[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
https://bugzilla.redhat.com/show_bug.cgi?id=772608 Björn besser82 Esser bjoern.es...@gmail.com changed: What|Removed |Added Blocks|177841 (FE-NEEDSPONSOR) | Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=177841 [Bug 177841] Tracker: Review requests from new Fedora packagers who need a sponsor -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=772608 Steven Dake sd...@redhat.com changed: What|Removed |Added Flags|fedora-review? | -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=a8XAvBWPPYa=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=772608 Vinzenz Feenstra vfeen...@redhat.com changed: What|Removed |Added Status|ASSIGNED|CLOSED CC||vfeen...@redhat.com Resolution|--- |DUPLICATE Last Closed||2012-12-21 12:08:16 --- Comment #36 from Vinzenz Feenstra vfeen...@redhat.com --- Cloned to https://bugzilla.redhat.com/show_bug.cgi?id=889546 Closing this one. *** This bug has been marked as a duplicate of bug 889546 *** -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=T8BqSceuqTa=cc_unsubscribe ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #34 from Gal Hammer gham...@redhat.com --- New version (1.0.5-1): Spec URL: http://ghammer.fedorapeople.org/ovirt-guest-agent.spec SRPM URL: http://ghammer.fedorapeople.org/ovirt-guest-agent-1.0.5-1.fc16.src.rpm - fixed 'udevadm trigger' command line (bz#819945). - fixed various rpmlint errors and warnings. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #35 from Gal Hammer gham...@redhat.com --- (In reply to comment #33) I handled all the errors but one and reduce the number of warnings. [root@f17 x86_64]# rpmlint ovirt-guest-agent-1.0.4-1.fc17.x86_64.rpm ovirt-guest-agent.x86_64: E: no-binary Although the agent is no-arch (Python) the sub-packages are compiled (arch depended). AFAIK there is no way to create an arch depended sub-package with a main no-arch package. ovirt-guest-agent.x86_64: W: non-standard-uid /var/log/ovirt-guest-agent ovirtagent ovirt-guest-agent.x86_64: W: non-standard-gid /var/log/ovirt-guest-agent ovirtagent These uid/gid are assigned to this package. ovirt-guest-agent.x86_64: W: log-files-without-logrotate /var/log/ovirt-guest-agent I'm using the Python's build in log rotation. ovirt-guest-agent-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/ovirt-guest-agent-1.0.4/gdm-plugin/gdm-ovirtcred-extension.c ovirt-guest-agent-debuginfo.x86_64: E: incorrect-fsf-address /usr/src/debug/ovirt-guest-agent-1.0.4/gdm-plugin/gdm-ovirtcred-extension.h Fixed. The current rpmlint output is: ]$ rpmlint /var/lib/mock/fedora-16-x86_64/result/*.rpm ovirt-guest-agent.src: W: summary-not-capitalized C oVirt Guest Agent ovirt-guest-agent.x86_64: W: summary-not-capitalized C oVirt Guest Agent ovirt-guest-agent.x86_64: E: no-binary ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/pam.d/ovirt-locksession ovirt-guest-agent.x86_64: W: non-standard-uid /var/log/ovirt-guest-agent ovirtagent ovirt-guest-agent.x86_64: W: non-standard-gid /var/log/ovirt-guest-agent ovirtagent ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/dbus-1/system.d/org.ovirt.vdsm.Credentials.conf ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/pam.d/ovirt-shutdown ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/udev/rules.d/55-ovirt-guest-agent.rules ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/security/console.apps/ovirt-hibernate ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/security/console.apps/ovirt-shutdown ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/pam.d/ovirt-hibernate ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/security/console.apps/ovirt-locksession ovirt-guest-agent.x86_64: W: log-files-without-logrotate /var/log/ovirt-guest-agent ovirt-guest-agent.x86_64: W: dangerous-command-in-%post ln ovirt-guest-agent.x86_64: W: dangerous-command-in-%postun rm ovirt-guest-agent-gdm-plugin.x86_64: W: no-documentation ovirt-guest-agent-gdm-plugin.x86_64: W: non-conffile-in-etc /etc/pam.d/gdm-ovirtcred ovirt-guest-agent-kdm-plugin.x86_64: W: no-documentation ovirt-guest-agent-kdm-plugin.x86_64: W: non-conffile-in-etc /etc/pam.d/kdm-ovirtcred ovirt-guest-agent-pam-module.x86_64: W: summary-not-capitalized C oVirt Guest Agent PAM module ovirt-guest-agent-pam-module.x86_64: W: no-documentation 6 packages and 0 specfiles checked; 1 errors, 21 warnings. -- You are receiving this mail because: You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=772608
--- Comment #33 from Steven Dake sd...@redhat.com 2012-05-17 16:04:45 EDT ---
rpmlint spits out a bunch of errors:
Please review:
https://fedoraproject.org/wiki/ParagNemade/CommonRpmlintErrors
and fix as appropriate:
[root@f17 x86_64]# rpmlint ovirt-guest-agent-1.0.4-1.fc17.x86_64.rpm
ovirt-guest-agent.x86_64: W: summary-not-capitalized C oVirt Guest Agent
ovirt-guest-agent.x86_64: E: no-binary
ovirt-guest-agent.x86_64: W: only-non-binary-in-usr-lib
ovirt-guest-agent.x86_64: W: conffile-without-noreplace-flag
/etc/ovirt-guest-agent.conf
ovirt-guest-agent.x86_64: E: non-executable-script
/usr/share/ovirt-guest-agent/VirtIoChannel.py 0644L /usr/bin/python
ovirt-guest-agent.x86_64: E: wrong-script-end-of-line-encoding
/usr/share/ovirt-guest-agent/VirtIoChannel.py
ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/pam.d/ovirt-locksession
ovirt-guest-agent.x86_64: W: non-conffile-in-etc
/etc/security/console.apps/ovirt-hibernate
ovirt-guest-agent.x86_64: W: non-standard-uid /var/log/ovirt-guest-agent
ovirtagent
ovirt-guest-agent.x86_64: W: non-standard-gid /var/log/ovirt-guest-agent
ovirtagent
ovirt-guest-agent.x86_64: E: non-executable-script
/usr/share/ovirt-guest-agent/OVirtAgentLogic.py 0644L /usr/bin/python
ovirt-guest-agent.x86_64: W: non-conffile-in-etc
/etc/dbus-1/system.d/org.ovirt.vdsm.Credentials.conf
ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/pam.d/ovirt-shutdown
ovirt-guest-agent.x86_64: W: non-conffile-in-etc
/etc/udev/rules.d/55-ovirt-guest-agent.rules
ovirt-guest-agent.x86_64: E: non-executable-script
/usr/share/ovirt-guest-agent/GuestAgentLinux2.py 0644L /usr/bin/python
ovirt-guest-agent.x86_64: E: wrong-script-end-of-line-encoding
/usr/share/ovirt-guest-agent/GuestAgentLinux2.py
ovirt-guest-agent.x86_64: W: non-conffile-in-etc
/etc/security/console.apps/ovirt-shutdown
ovirt-guest-agent.x86_64: E: non-executable-script
/usr/share/ovirt-guest-agent/CredServer.py 0644L /usr/bin/python
ovirt-guest-agent.x86_64: W: non-conffile-in-etc /etc/pam.d/ovirt-hibernate
ovirt-guest-agent.x86_64: E: zero-length
/usr/share/doc/ovirt-guest-agent-1.0.4/NEWS
ovirt-guest-agent.x86_64: W: non-conffile-in-etc
/etc/security/console.apps/ovirt-locksession
ovirt-guest-agent.x86_64: W: log-files-without-logrotate
/var/log/ovirt-guest-agent
ovirt-guest-agent.x86_64: W: dangerous-command-in-%post ln
ovirt-guest-agent.x86_64: W: dangerous-command-in-%postun rm
1 packages and 0 specfiles checked; 8 errors, 16 warnings.
[root@f17 x86_64]# rpmlint ovirt-guest-agent-debuginfo-1.0.4-1.fc17.x86_64.rpm
ovirt-guest-agent-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/ovirt-guest-agent-1.0.4/gdm-plugin/gdm-ovirtcred-extension.c
ovirt-guest-agent-debuginfo.x86_64: E: incorrect-fsf-address
/usr/src/debug/ovirt-guest-agent-1.0.4/gdm-plugin/gdm-ovirtcred-extension.h
1 packages and 0 specfiles checked; 2 errors, 0 warnings.
[root@f17 x86_64]# rpmlint ovirt-guest-agent-gdm-plugin-1.0.4-1.fc17.x86_64.rpm
ovirt-guest-agent-gdm-plugin.x86_64: W: spelling-error %description -l en_US
login - loin, logic, lo gin
ovirt-guest-agent-gdm-plugin.x86_64: W: conffile-without-noreplace-flag
/etc/pam.d/gdm-ovirtcred
ovirt-guest-agent-gdm-plugin.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
[root@f17 x86_64]# rpmlint ovirt-guest-agent-kdm-plugin-1.0.4-1.fc17.x86_64.rpm
ovirt-guest-agent-kdm-plugin.x86_64: W: spelling-error %description -l en_US
login - loin, logic, lo gin
ovirt-guest-agent-kdm-plugin.x86_64: W: conffile-without-noreplace-flag
/etc/pam.d/kdm-ovirtcred
ovirt-guest-agent-kdm-plugin.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
[root@f17 x86_64]# rpmlint ovirt-guest-agent-pam-module-1.0.4-1.fc17.x86_64.rpm
ovirt-guest-agent-pam-module.x86_64: W: summary-not-capitalized C oVirt Guest
Agent PAM module
ovirt-guest-agent-pam-module.x86_64: W: spelling-error %description -l en_US
login - loin, logic, lo gin
ovirt-guest-agent-pam-module.x86_64: W: no-documentation
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
[root@f17 SRPMS]# rpmlint ovirt-guest-agent-1.0.4-1.fc17.src.rpm
ovirt-guest-agent.src: W: summary-not-capitalized C oVirt Guest Agent
ovirt-guest-agent.src:208: W: macro-in-%changelog %{_unitdir}
ovirt-guest-agent.src:212: W: macro-in-%changelog %clean
1 packages and 0 specfiles checked; 0 errors, 3 warnings.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=772608
--- Comment #31 from Gal Hammer gham...@redhat.com 2012-05-15 05:19:05 EDT ---
New version:
Spec URL: http://ghammer.fedorapeople.org/ovirt-guest-agent.spec
SRPM URL:
http://ghammer.fedorapeople.org/ovirt-guest-agent-1.0.4-1.fc16.src.rpm
- replaced with usage with a python 2.4 compatible way.
- added files to support RHEL-5 distribution.
- added more detailed memory statistics.
- fixed build on fc-17 (use the %{_unitdir} macro).
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #32 from Gal Hammer gham...@redhat.com 2012-05-15 05:21:58 EDT --- (In reply to comment #30) Gal, Trying to build your srpm in comment #25, I run into the following problem: RPM build errors: File not found: /root/rpmbuild/BUILDROOT/ovirt-guest-agent-1.0.3-2.fc17.x86_64/lib/systemd/system/ovirt-guest-agent.service Comment #27 may fix the problem, but I didn't try as the srpm must come from an upstream location to be merged. Please create a new spec file and rpm with the appropriate changes. Fixed. See Comment #31. Regards -steve -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #30 from Steven Dake sd...@redhat.com 2012-05-14 13:18:55 EDT --- Gal, Trying to build your srpm in comment #25, I run into the following problem: RPM build errors: File not found: /root/rpmbuild/BUILDROOT/ovirt-guest-agent-1.0.3-2.fc17.x86_64/lib/systemd/system/ovirt-guest-agent.service Comment #27 may fix the problem, but I didn't try as the srpm must come from an upstream location to be merged. Please create a new spec file and rpm with the appropriate changes. Regards -steve -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=772608
Gal Hammer gham...@redhat.com changed:
What|Removed |Added
CC||dw...@infradead.org
Component|Package Review |0x
Flag|fedora-review? |
--- Comment #28 from Gal Hammer gham...@redhat.com 2012-05-13 06:54:10 EDT ---
(In reply to comment #27)
Changing hardcoded path of said file in %files to
%{_unitdir}/ovirt-guest-agent.service fixed it for me:
Thanks. Applied.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 Steven Dake sd...@redhat.com changed: What|Removed |Added Flag||fedora-review? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 Steven Dake sd...@redhat.com changed: What|Removed |Added Component|0x |Package Review AssignedTo|sd...@redhat.com|nob...@fedoraproject.org -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #29 from Steven Dake sd...@redhat.com 2012-05-13 11:54:21 EDT --- Gal, I'll review today. Need bit of time to get an F17 system in order as this doesn't build on F16. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=772608
David Jaša dj...@redhat.com changed:
What|Removed |Added
CC||dj...@redhat.com
--- Comment #27 from David Jaša dj...@redhat.com 2012-05-10 15:49:27 EDT ---
When building on .fc17, I got this error:
Processing files: ovirt-guest-agent-1.0.3-2.fc17.x86_64
error: File not found:
/home/djasa/rpmbuild/BUILDROOT/ovirt-guest-agent-1.0.3-2.fc17.x86_64/lib/systemd/system/ovirt-guest-agent.service
Changing hardcoded path of said file in %files to
%{_unitdir}/ovirt-guest-agent.service fixed it for me:
--- rpmbuild/SPECS/ovirt-guest-agent.spec.orig 2012-04-15 09:42:39.0
+0200
+++ rpmbuild/SPECS/ovirt-guest-agent.spec 2012-05-10 21:45:16.498458778 +0200
@@ -178,7 +178,7 @@
%{_datadir}/ovirt-guest-agent/GuestAgentLinux2.py*
%attr (755,root,root) %{_datadir}/ovirt-guest-agent/LockActiveSession.py*
%attr (755,root,root) %{_datadir}/ovirt-guest-agent/hibernate
-/lib/systemd/system/ovirt-guest-agent.service
+%{_unitdir}/ovirt-guest-agent.service
%doc AUTHORS COPYING NEWS README
/mine fingers crossed, too! :)
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #25 from Gal Hammer gham...@redhat.com 2012-04-15 04:30:21 EDT --- New version: Spec URL: http://ghammer.fedorapeople.org/ovirt-guest-agent.spec SRPM URL: http://ghammer.fedorapeople.org/ovirt-guest-agent-1.0.3-2.fc16.src.rpm - removed the RHEL distribution support for the review process. - removed BuildRoot header and %clean section. - fixed user creation. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #26 from Gal Hammer gham...@redhat.com 2012-04-15 04:37:45 EDT --- (In reply to comment #24) 1) The BuildRoot is not necessary, Fedora figures this out automatically: Removed BuildRoot. 2) This looks a little suspicious. Especially in FC17+ the gdm release shouldn't be el6? Fixed by removing support for RHEL distribution for now. 3) Make spec files fedora specific please otherwise the packaging is very Fixed. 4) /var/run is mounted dynamically as a tempfs. As a result, you will not want Fixed. It was a leftover from RHEL 6. The /var/run path is not tempfs there. 5) The clean section isn't needed in fedora since about 12ish or so. It can be removed. Removed %clean section. 6) please do not use static IDS (such as 175) in useradd. Also the proper thing is not being done here re handling useradd failures. See http://fedoraproject.org/wiki/Packaging:UsersAndGroups for the proper mechanism. Fixed the user creation to match the guide lines in the link. I'm still using the static id. It is an assign id (see the setup package). After correcting the above, I'll go through an official review Fingers crossed! :-) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=772608
--- Comment #24 from Steven Dake sd...@redhat.com 2012-04-12 10:48:36 EDT ---
1)
The BuildRoot is not necessary, Fedora figures this out automatically:
BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XX)
2)
This looks a little suspicious. Especially in FC17+ the gdm release shouldn't
be el6?
%define gdm_version gdm-2.30.4
%define gdm_release %{gdm_version}-14.el6
Are these still needed now that your using the gdm-devel package?
# The following requirements were copied from the gdm.spec file.
BuildRequires: pkgconfig(libcanberra-gtk)
BuildRequires: scrollkeeper = 0:%{scrollkeeper_version}
BuildRequires: pango-devel = 0:%{pango_version}
BuildRequires: gtk2-devel = 0:%{gtk2_version}
BuildRequires: libglade2-devel = 0:%{libglade2_version}
BuildRequires: libgnomeui-devel = 0:%{libgnomeui_version}
BuildRequires: pam-devel = 0:%{pam_version}
BuildRequires: fontconfig = 0:%{fontconfig_version}
BuildRequires: desktop-file-utils = %{desktop_file_utils_version}
BuildRequires: gail-devel = 0:%{gail_version}
if they are still needed, please get rid of the defines:
%define libauditver 1.0.6
%define pango_version 1.2.0
%define gtk2_version 2.6.0
%define libglade2_version 2.0.0
%define libgnomeui_version 2.2.0
%define scrollkeeper_version 0.3.4
%define pam_version 0.99.8.1-11
%define desktop_file_utils_version 0.2.90
%define gail_version 1.2.0
%define nss_version 3.11.1
%define fontconfig_version 2.6.0
and put them directly in the buildrequires. The current spec file is very
difficult to read.
3)
Make spec files fedora specific please otherwise the packaging is very
difficult to read
ie:
%if 0%{?rhel}
--with-gdm-src-dir=%{_topdir}/BUILD/%{gdm_version} \
--with-simple-greeter-plugins-dir=%{_libdir}/gdm/simple-greeter/plugins \
%endif
%if 0%{?rhel}
sed -i
s~parent-setObjectName(\welcome\);~parent-setObjectName(\talker\);~
kdm-plugin/src/kgreet_ovirtcred.cpp
%endif
just assume systemd will be used
# Install systemd script.
install -Dm 0644 ovirt-guest-agent/ovirt-guest-agent.service
$RPM_BUILD_ROOT%{_unitdir}/ovirt-guest-agent.service
%if 0%{?rhel}
# No longer needed and is provided by the gdm package.
rm -f $RPM_BUILD_ROOT%{_libdir}/libgdmsimplegreeter.so
rm -f $RPM_BUILD_ROOT%{_libdir}/gdm/simple-greeter/plugins/ovirtcred.a
rm -f $RPM_BUILD_ROOT%{_libdir}/gdm/simple-greeter/plugins/ovirtcred.la
%else
4)
/var/run is mounted dynamically as a tempfs. As a result, you will not want it
in the package. If you need directories in /var/run, you will need to create
them. I don't like it either if it makes you feel better ;)
mkdir -p $RPM_BUILD_ROOT%{_localstatedir}/run/ovirt-guest-agent
5)
The clean section isn't needed in fedora since about 12ish or so. It can be
removed.
6) please do not use static IDS (such as 175) in useradd. Also the proper
thing is not being done here re handling useradd failures.
See http://fedoraproject.org/wiki/Packaging:UsersAndGroups for the proper
mechanism.
After correcting the above, I'll go through an official review
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #23 from Gal Hammer gham...@redhat.com 2012-04-10 10:22:12 EDT --- New version: Spec URL: http://ghammer.fedorapeople.org/ovirt-guest-agent.spec SRPM URL: http://ghammer.fedorapeople.org/ovirt-guest-agent-1.0.3-1.fc16.src.rpm - package was renamed to rhevm-guest-agent in RHEL distribution. - fixed gdm-plugin build requires. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #19 from Gal Hammer gham...@redhat.com 2012-03-28 06:22:42 EDT --- New version: Spec URL: http://ghammer.fedorapeople.org/ovirt-guest-agent.spec SRPM URL: http://ghammer.fedorapeople.org/ovirt-guest-agent-1.0.2-1.fc16.src.rpm - included a gpl-v2 copying file. - build the gdm-plugin using the gdm-devel package. - added a support for RHEL distribution. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #20 from Gal Hammer gham...@redhat.com 2012-03-28 06:29:25 EDT --- (In reply to comment #18) This package has a crazy number of build dependencies, but if you think you can keep it all working, more power to you :) That's all gdm staff :-). I would recommend changing BuildRequires: pkgconfig(libcanberra-gtk) to BuildRequires: libcanberra-devel The current mechanism is prone to error and hard to understand Along those same lines Change BuildRequires: pkgconfig(accountsservices) to to: BuildRequires accountsservices-devel # No gdm-devel package is available for plug-in development. So for now # we build the gdm package. Source1: gdm-3.2.1.1-6.fc16.src.rpm Hacking around the problem by bundling a source rpm is not acceptable. The proper solution is to fix the problem: 1. file a gdm bug 2a. if you want it fixed fast, sort out how to change gdm packaging to provide a devel package that exports what you need and feed it into the bugzilla or 2b. block on gdm maintainer providing devel package An alternative is not to build/provide the gdm plugin until this upstream problem is sorted out. This is why mock is not building (gdm requires xorg server, which the build system can't sort out) Even if this were permissible, I'm uncertain the build system could handle it properly. Since it is not permissible, whether the build system could be hacked to make it work is not relevant ;) The review is blocked on this issue. gdm-3 added a gdm-devel package, so I've fixed the build process to use it. It is no longer required to build the whole gdm source tree. The exception is for older distribution (e.g. rhel) which still use gdm-2. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #21 from Steven Dake sd...@redhat.com 2012-03-28 10:07:30 EDT --- I am a bit ill with a cold and head hurts. I'll execute a review when I'm feeling a bit better (couple days?). Regards -steve -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #22 from Gal Hammer gham...@redhat.com 2012-03-28 11:31:45 EDT --- (In reply to comment #21) I am a bit ill with a cold and head hurts. I'll execute a review when I'm feeling a bit better (couple days?). It sounds like the normal symptoms for someone who is working around the agent ;-). Get well soon. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #18 from Steven Dake sd...@redhat.com 2012-03-06 13:07:45 EST --- This package has a crazy number of build dependencies, but if you think you can keep it all working, more power to you :) I would recommend changing BuildRequires: pkgconfig(libcanberra-gtk) to BuildRequires: libcanberra-devel The current mechanism is prone to error and hard to understand Along those same lines Change BuildRequires: pkgconfig(accountsservices) to to: BuildRequires accountsservices-devel # No gdm-devel package is available for plug-in development. So for now # we build the gdm package. Source1: gdm-3.2.1.1-6.fc16.src.rpm Hacking around the problem by bundling a source rpm is not acceptable. The proper solution is to fix the problem: 1. file a gdm bug 2a. if you want it fixed fast, sort out how to change gdm packaging to provide a devel package that exports what you need and feed it into the bugzilla or 2b. block on gdm maintainer providing devel package An alternative is not to build/provide the gdm plugin until this upstream problem is sorted out. This is why mock is not building (gdm requires xorg server, which the build system can't sort out) Even if this were permissible, I'm uncertain the build system could handle it properly. Since it is not permissible, whether the build system could be hacked to make it work is not relevant ;) The review is blocked on this issue. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #16 from Gal Hammer gham...@redhat.com 2012-02-22 04:40:38 EST --- New version: Spec URL: http://ghammer.fedorapeople.org/ovirt-guest-agent.spec SRPM URL: http://ghammer.fedorapeople.org/ovirt-guest-agent-1.0.1-2.fc16.src.rpm - updated required selinux-policy version (related to rhbz#791113). - added a support to hibernate (s4) command. - renamed user name to ovirtguest. - reset version numbering after changing the package name. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #17 from Gal Hammer gham...@redhat.com 2012-02-22 04:50:41 EST --- (In reply to comment #14) That sounds weird, and actually sounds like a shortcoming of the GDM plugin framework and/or the gdm-devel package. I'd still say this is not permisible, but I'll leave the decision to a more seasoned packager (Steven?) AFAIK, there is no devel package to support GDM plugins development. From that wiki page: There are several cases where upstream is not providing the source to you in an upstream tarball. In these cases you must document how to generate the tarball used in the rpm either through a spec file comment or a script included as a separate SourceX Check that URL I pasted above again, you should actually describe how to create that tarball from git, not point to the tarball you created Since I'm providing the source as a tarball there is not reason to describe how to create that tarball. That's what the quote say. Try following Steven's advice and bump the rpm changelog every time you make changes (hint: rpmdev-bumpspec foo.spec). Also try submitting a koji scratch build (https://fedoraproject.org/wiki/Using_the_Koji_build_system) and paste the URL each time. In this case I just submitted it and the results are here: http://koji.fedoraproject.org/koji/taskinfo?taskID=3767959 As you can see, there are still some unmet build dependencies: error: Failed build dependencies: xorg-x11-server-Xorg is needed by gdm-1:3.2.1.1-6.fc17.x86_64 If you don't want to use koji you can use mock in your machine like this: mock -r fedora-16-x86_64 rebuild package-1.2-3.src.rpm. This should do a scratch build in a sanboxed chroot so you can see if all dependencies are met. I'm not using koji at the moment. I've checked the new version (1.0.1-2) using a mock build on my local machine and it worked. I hope this is enough for koji too. I was just pointing out the python packaging guidelines. Understood. However it would be nice to understand the rational behind it. I don't think I quite understand this last one... what does assigned mean? If I do in my system (before installing this package) `/usr/sbin/useradd -u 175 -o -r haproxy -c High Availability Proxy Daemon User -d / -s /sbin/nologin` and then try installing this package, the %prep step would likely fail, I think? Check the file /usr/share/doc/setup-2.8.36/uidgid (from the setup pacakge). It include the reserved uids. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #15 from Steven Dake sd...@redhat.com 2012-02-19 15:04:10 EST --- Gal, First please respond to questions in comment #14. Second, I'd like you to act as a reviewer for another package on your way to making into the packagers group. Please follow through with the entire process of getting the package into shape. The bugzilla is: https://bugzilla.redhat.com/show_bug.cgi?id=786860 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=772608
--- Comment #14 from Jorge A Gallegos k...@blegh.net 2012-02-07 01:27:30 EST
---
(In reply to comment #12)
See my responses inline
(In reply to comment #11)
First, I can see a glaring issue with your package, you are shipping GDM's
.src.rpm as part of your own source. That is probably not going to fly, even
though I can't really find anything in the packaging guidelines that
prevents
you from doing it (closest would be
http://fedoraproject.org/wiki/Packaging/Guidelines#No_inclusion_of_pre-built_binaries_or_libraries
but there's no mention about .src.rpm). I see some problems with your
approach:
- You are shipping a specific version of source that is already present in
fedora
- You will have to ship newer, static .src.rpm whenever GDM gets a newer
version
- You may end up fighting duplicated libraries
The approach, in my opinion, would be to get in touch with the GDM
packagers
and ask for your stuff to be included if it's not already. It is not
entirely
apparent why requiring gdm-devel wouldn't work right now.
The reason I include the gdm src.rpm file is because I didn't find another way
to compile a gdm plugin outside the gdm's source tree. It is used only during
compilation and linkage time and it is not shipped with the plugin itself.
That sounds weird, and actually sounds like a shortcoming of the GDM plugin
framework and/or the gdm-devel package. I'd still say this is not permisible,
but I'll leave the decision to a more seasoned packager (Steven?)
[BLOCKER] The License field in the package spec file must match the actual
license - the COPYING file in the source tarball has GPLv3, you have GPLv2+
in
the spec file
Fixed.
Cool!
[BLOCKER] The sources used to build the package must match the upstream
source,
as provided in the spec URL - You should take a look at
http://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control and
not use a self-hosted source tarball (Source0:
http://ghammer.fedorapeople.org/%{name}-%{version}.tar.bz2)
As far as I understood this is allowed
(http://fedoraproject.org/wiki/Packaging/SourceURL).
From that wiki page: There are several cases where upstream is not providing
the source to you in an upstream tarball. In these cases you must document how
to generate the tarball used in the rpm either through a spec file comment or a
script included as a separate SourceX
Check that URL I pasted above again, you should actually describe how to create
that tarball from git, not point to the tarball you created
[BLOCKER] The package MUST successfully compile and build into binary rpms
on
at least one primary architecture - I got this error when trying a mock
build:
xorg-x11-server-Xorg is needed by gdm-1:3.2.1.1-6.fc16.x86_64.
Probably a bug after fixes done after a review. I've updated a new version
(http://ghammer.fedorapeople.org/ovirt-guest-agent-1.0.1-1.fc16.src.rpm) which
compile on both 32 and 64 bits.
Try following Steven's advice and bump the rpm changelog every time you make
changes (hint: rpmdev-bumpspec foo.spec). Also try submitting a koji scratch
build (https://fedoraproject.org/wiki/Using_the_Koji_build_system) and paste
the URL each time. In this case I just submitted it and the results are here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3767959
As you can see, there are still some unmet build dependencies:
error: Failed build dependencies:
xorg-x11-server-Xorg is needed by gdm-1:3.2.1.1-6.fc17.x86_64
If you don't want to use koji you can use mock in your machine like this: mock
-r fedora-16-x86_64 rebuild package-1.2-3.src.rpm. This should do a scratch
build in a sanboxed chroot so you can see if all dependencies are met.
[BLOCKER] Each package must consistently use macros - I see a mix of $MACRO
and
%{macro}, use one style and stick with. Also, if you don't plan on
supporting
EPEL/RH, you should do away with the whole RPM_BUILD_ROOT [citation needed,
maybe?]
I'm not sure I understand this. I checked with the gdm spec file (as an
example) and it look pretty much like mine (using %macro expect for
$RPM_BUILD_ROOT).
Fair enough, I suppose if an existing accepted package follows the same tone is
fair to assume this is OK.
Additionally, it seems you include some python code in there, so, using
http://fedoraproject.org/wiki/Packaging:Python as guide:
[BLOCKER] To build a package containing python2 files, you need to have
BuildRequires: python2-devel
Why do I need to include it? My build machine doesn't have the python2-devel
package installed.
I was just pointing out the python packaging guidelines.
Some additional suggestions:
- defattr is not needed anymore, if I recall
The %files section normally begins with a
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=772608
--- Comment #12 from Gal Hammer gham...@redhat.com 2012-02-05 04:06:40 EST ---
(In reply to comment #11)
First, I can see a glaring issue with your package, you are shipping GDM's
.src.rpm as part of your own source. That is probably not going to fly, even
though I can't really find anything in the packaging guidelines that prevents
you from doing it (closest would be
http://fedoraproject.org/wiki/Packaging/Guidelines#No_inclusion_of_pre-built_binaries_or_libraries
but there's no mention about .src.rpm). I see some problems with your
approach:
- You are shipping a specific version of source that is already present in
fedora
- You will have to ship newer, static .src.rpm whenever GDM gets a newer
version
- You may end up fighting duplicated libraries
The approach, in my opinion, would be to get in touch with the GDM packagers
and ask for your stuff to be included if it's not already. It is not entirely
apparent why requiring gdm-devel wouldn't work right now.
The reason I include the gdm src.rpm file is because I didn't find another way
to compile a gdm plugin outside the gdm's source tree. It is used only during
compilation and linkage time and it is not shipped with the plugin itself.
[BLOCKER] The License field in the package spec file must match the actual
license - the COPYING file in the source tarball has GPLv3, you have GPLv2+ in
the spec file
Fixed.
[BLOCKER] The sources used to build the package must match the upstream
source,
as provided in the spec URL - You should take a look at
http://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control and
not use a self-hosted source tarball (Source0:
http://ghammer.fedorapeople.org/%{name}-%{version}.tar.bz2)
As far as I understood this is allowed
(http://fedoraproject.org/wiki/Packaging/SourceURL).
[BLOCKER] The package MUST successfully compile and build into binary rpms on
at least one primary architecture - I got this error when trying a mock build:
xorg-x11-server-Xorg is needed by gdm-1:3.2.1.1-6.fc16.x86_64.
Probably a bug after fixes done after a review. I've updated a new version
(http://ghammer.fedorapeople.org/ovirt-guest-agent-1.0.1-1.fc16.src.rpm) which
compile on both 32 and 64 bits.
[BLOCKER] Each package must consistently use macros - I see a mix of $MACRO
and
%{macro}, use one style and stick with. Also, if you don't plan on supporting
EPEL/RH, you should do away with the whole RPM_BUILD_ROOT [citation needed,
maybe?]
I'm not sure I understand this. I checked with the gdm spec file (as an
example) and it look pretty much like mine (using %macro expect for
$RPM_BUILD_ROOT).
Additionally, it seems you include some python code in there, so, using
http://fedoraproject.org/wiki/Packaging:Python as guide:
[BLOCKER] To build a package containing python2 files, you need to have
BuildRequires: python2-devel
Why do I need to include it? My build machine doesn't have the python2-devel
package installed.
Some additional suggestions:
- defattr is not needed anymore, if I recall
The %files section normally begins with a %defattr line which sets the default
file permissions.
(http://fedoraproject.org/wiki/How_to_create_an_RPM_package).
Maybe it is time to review/update the manual... :-)
- you should name your different targets appropriately like
ovirt-guest-agent-gdm-plugin instead of just gdm-plugin
The rpmbuild does that for me. The rpm file name is called:
ovirt-guest-agent-gdm-plugin-1.0.1-1.fc16.x86_64.
- in your %pre step, you add a user rhevagent if it doesn't exist, but you
are
also hardcoding the UID. You should leave that to the system to figure out and
just use -r for system users
This is an assigned uid.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #13 from Steven Dake sd...@redhat.com 2012-02-05 12:00:31 EST --- Gal, A short recommendation when making updates to spec files. After each revision of a spec file, please make a new bugzilla comment entry separate from questions for the reviewer that states the spec file location and src.rpm file location. Also take care to increase the version numbers and update changelog in the spec file. Example: Updated spec file to fix previous reported problems: SPEC: link SRPM: link -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 Steven Dake sd...@redhat.com changed: What|Removed |Added Flag||fedora-review? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=772608
Jorge A Gallegos k...@blegh.net changed:
What|Removed |Added
CC||k...@blegh.net
--- Comment #11 from Jorge A Gallegos k...@blegh.net 2012-02-01 03:19:25 EST
---
** I AM NOT A SPONSOR **
However, I'll try and submit a review as per Steven's request.
First, I can see a glaring issue with your package, you are shipping GDM's
.src.rpm as part of your own source. That is probably not going to fly, even
though I can't really find anything in the packaging guidelines that prevents
you from doing it (closest would be
http://fedoraproject.org/wiki/Packaging/Guidelines#No_inclusion_of_pre-built_binaries_or_libraries
but there's no mention about .src.rpm). I see some problems with your approach:
- You are shipping a specific version of source that is already present in
fedora
- You will have to ship newer, static .src.rpm whenever GDM gets a newer
version
- You may end up fighting duplicated libraries
The approach, in my opinion, would be to get in touch with the GDM packagers
and ask for your stuff to be included if it's not already. It is not entirely
apparent why requiring gdm-devel wouldn't work right now.
Now, on with the revision proper:
[kad@propane rpmbuild ]$ rpmlint SPECS/ovirt-guest-agent.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.
[kad@propane rpmbuild ]$ rpmlint SRPMS/ovirt-guest-agent-1.0.0-2.fc16.src.rpm
ovirt-guest-agent.src: W: summary-not-capitalized C oVirt Guest Agent
1 packages and 0 specfiles checked; 0 errors, 1 warnings.
(oVirt is not capitalized in the main oVirt package, so I guess that warning
is ok)
[BLOCKER] The License field in the package spec file must match the actual
license - the COPYING file in the source tarball has GPLv3, you have GPLv2+ in
the spec file
[BLOCKER] The sources used to build the package must match the upstream source,
as provided in the spec URL - You should take a look at
http://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control and
not use a self-hosted source tarball (Source0:
http://ghammer.fedorapeople.org/%{name}-%{version}.tar.bz2)
[BLOCKER] The package MUST successfully compile and build into binary rpms on
at least one primary architecture - I got this error when trying a mock build:
xorg-x11-server-Xorg is needed by gdm-1:3.2.1.1-6.fc16.x86_64.
[BLOCKER] Each package must consistently use macros - I see a mix of $MACRO and
%{macro}, use one style and stick with. Also, if you don't plan on supporting
EPEL/RH, you should do away with the whole RPM_BUILD_ROOT [citation needed,
maybe?]
Additionally, it seems you include some python code in there, so, using
http://fedoraproject.org/wiki/Packaging:Python as guide:
[BLOCKER] To build a package containing python2 files, you need to have
BuildRequires: python2-devel
Some additional suggestions:
- defattr is not needed anymore, if I recall
- you should name your different targets appropriately like
ovirt-guest-agent-gdm-plugin instead of just gdm-plugin
- in your %pre step, you add a user rhevagent if it doesn't exist, but you are
also hardcoding the UID. You should leave that to the system to figure out and
just use -r for system users
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 Steven Dake sd...@redhat.com changed: What|Removed |Added Blocks||177841(FE-NEEDSPONSOR) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 Steven Dake sd...@redhat.com changed: What|Removed |Added AssignedTo|nob...@fedoraproject.org|sd...@redhat.com -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 Steven Dake sd...@redhat.com changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #10 from Steven Dake sd...@redhat.com 2012-01-30 09:06:17 EST --- Gal, I will sponsor you as a packager. The first step is to review a Fedora contributor's package. Please execute the review process on this bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=784156 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #4 from Gal Hammer gham...@redhat.com 2012-01-25 03:12:36 EST --- (In reply to comment #3) - In the Requires and BuildRequires statements some use %define macros for the version and some hardcode them. It would probably be best to choose one method of specifying the versions and use it consistently. The %define macros were copied from the gdm.spec file. I moved them closer to the GDM's BuildRequires section to separate them from the guest's BuildRequires section. - The Source0 line is expected to contain a URL pointing to the archive or, where applicable, just the name of the archive accompanied by a comment explaining where it was generated from: http://fedoraproject.org/wiki/Packaging/SourceURL Fixed. - Given the ExclusiveArch directive I am not sure this macro is required?: %ifnarch s390 s390x ppc64 BuildRequires: xorg-x11-server-Xorg %endif It is just a left over from the GDM's spec file. I removed it. - There are a few spelling errors in the description, as well as mixed use of tabs/spaces, detected by rpmlint. Fixed. Spec URL: http://ghammer.fedorapeople.org/ovirt-guest-agent.spec SRPM URL: http://ghammer.fedorapeople.org/ovirt-guest-agent-1.0.0-2.fc16.src.rpm -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #5 from Stephen Gordon sgor...@redhat.com 2012-01-25 09:47:54 EST --- (In reply to comment #4) (In reply to comment #3) %ifnarch s390 s390x ppc64 BuildRequires: xorg-x11-server-Xorg %endif It is just a left over from the GDM's spec file. I removed it. To be clear I was suggesting that: %ifnarch s390 s390x ppc64 BuildRequires: xorg-x11-server-Xorg %endif Become: BuildRequires: xorg-x11-server-Xorg AFAIK the package is required to build on the supported architectures (i686, x86_64 thanks to the ExclusiveArch)), certainly mock suggests this is the case: error: Failed build dependencies: xorg-x11-server-Xorg is needed by gdm-1:3.2.1.1-6.fc16.x86_64 I'll admit I am a little confused as to why this package has to bundle the gdm SRPM rather than setting a BuildRequires on gdm but you have commented it and I will have to take your word for it for now. Another reviewer might want to look at this more closely :). -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #6 from Stephen Gordon sgor...@redhat.com 2012-01-25 10:08:54 EST --- Additionally the following files appear to be installed but not packaged (this comes out of the rpmbuild check): /etc/pam.d/ovirt-hibernate /etc/security/console.apps/ovirt-hibernate /usr/share/ovirt-guest-agent/hibernate -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #7 from Gal Hammer gham...@redhat.com 2012-01-25 14:15:36 EST --- (In reply to comment #5) To be clear I was suggesting that: %ifnarch s390 s390x ppc64 BuildRequires: xorg-x11-server-Xorg %endif Become: BuildRequires: xorg-x11-server-Xorg Fixed. I'll admit I am a little confused as to why this package has to bundle the gdm SRPM rather than setting a BuildRequires on gdm but you have commented it and I will have to take your word for it for now. Another reviewer might want to look at this more closely :). I hope someone will find a better solution. The agent's build time will be much faster. :-) The problem is that th gdm package doesn't include files which libtool requires. So I couldn't solve it with a BuildRequires on gdm. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #8 from Gal Hammer gham...@redhat.com 2012-01-25 14:17:04 EST --- (In reply to comment #6) Additionally the following files appear to be installed but not packaged (this comes out of the rpmbuild check): /etc/pam.d/ovirt-hibernate /etc/security/console.apps/ovirt-hibernate /usr/share/ovirt-guest-agent/hibernate These are files required for a feature that is not included in the reviewed version. Where did you see a reference to them? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #9 from Stephen Gordon sgor...@redhat.com 2012-01-25 15:10:33 EST --- When I added the BuildRequires: xorg-x11-server-XorgWhen entry back to this version and then attempted to build the package those files were listed in the output from rpmbuild. What it is (or was at least) effectively saying is that those files were detected as being deployed by the RPM but are not listed in the %files section (either explicitly or as part of the globs). -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 Stephen Gordon sgor...@redhat.com changed: What|Removed |Added CC||sgor...@redhat.com --- Comment #3 from Stephen Gordon sgor...@redhat.com 2012-01-24 16:57:18 EST --- - In the Requires and BuildRequires statements some use %define macros for the version and some hardcode them. It would probably be best to choose one method of specifying the versions and use it consistently. - The Source0 line is expected to contain a URL pointing to the archive or, where applicable, just the name of the archive accompanied by a comment explaining where it was generated from: http://fedoraproject.org/wiki/Packaging/SourceURL - Given the ExclusiveArch directive I am not sure this macro is required?: %ifnarch s390 s390x ppc64 BuildRequires: xorg-x11-server-Xorg %endif - There are a few spelling errors in the description, as well as mixed use of tabs/spaces, detected by rpmlint. Output of rpmlint on the SRPM (given oVirt is the project name I think it is probably fair to ignore the first warning): ovirt-guest-agent.src: W: summary-not-capitalized C oVirt Guest Agent ovirt-guest-agent.src: W: spelling-error %description -l en_US managment - management, engagement, Mantegna ovirt-guest-agent.src: W: spelling-error %description -l en_US runtime - run time, run-time, rudiment ovirt-guest-agent.src:185: W: mixed-use-of-spaces-and-tabs (spaces: line 185, tab: line 180) ovirt-guest-agent.src: W: invalid-url Source0: ovirt-guest-agent-1.0.0.tar.bz2 1 packages and 0 specfiles checked; 0 errors, 5 warnings. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 --- Comment #2 from Gal Hammer gham...@redhat.com 2012-01-17 08:04:49 EST --- (In reply to comment #1) Hello. I'm tring to use ovirt. Just few comments There is no need to include the following packages: redhat-rpm-config or their dependencies as BuildRequires because they would occur too often. These packages are considered the minimum build environment. http://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2 rpm-python and dbus-python depend from python. So there is no need to include python to Requires. automake depends from autoconf etc. Thanks. I removed the relevant lines from the spec file. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 772608] Review Request: ovirt-guest-agent - oVirt Guest Agent
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=772608 Pavel Zhukov pa...@zhukoff.net changed: What|Removed |Added CC||pa...@zhukoff.net --- Comment #1 from Pavel Zhukov pa...@zhukoff.net 2012-01-11 02:00:55 EST --- Hello. I'm tring to use ovirt. Just few comments There is no need to include the following packages: redhat-rpm-config or their dependencies as BuildRequires because they would occur too often. These packages are considered the minimum build environment. http://fedoraproject.org/wiki/Packaging:Guidelines#Exceptions_2 rpm-python and dbus-python depend from python. So there is no need to include python to Requires. automake depends from autoconf etc. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
