Vinzenz Feenstra has posted comments on this change.

Change subject: Updated spec file to follow packaging guide lines (BZ#772608)
......................................................................


Patch Set 1: (10 inline comments)

....................................................
File ovirt-guest-agent.spec
Line 1: 
Line 2: %global release_version 2
Line 3: 
Line 4: %global _moduledir /%{_lib}/security
Line 5: %global _kdmrc /etc/kde/kdm/kdmrc
This is required by the Fedora packaging guidelines.
Line 6: 
Line 7: Name: ovirt-guest-agent
Line 8: Version: 1.0.5
Line 9: Release: %{release_version}%{?dist}


Line 6: 
Line 7: Name: ovirt-guest-agent
Line 8: Version: 1.0.5
Line 9: Release: %{release_version}%{?dist}
Line 10: Summary: The oVirt Guest Agent
I can remove it again, I tried to keep the warnings on an absolute minimum. 
'The' would have solved it for this particular warning.
Line 11: Group: Applications/System
Line 12: License: GPLv2+
Line 13: URL: http://gerrit.ovirt.org/p/ovirt-guest-agent.git
Line 14: Source0: http://ghammer.fedorapeople.org/%{name}-%{version}.tar.bz2


Line 15: BuildRequires: libtool
Line 16: BuildRequires: pam-devel
Line 17: BuildRequires: python2-devel
Line 18: %if 0%{?fedora} >= 18
Line 19: BuildRequires: systemd
This is required according to fedora guidelines for packages which are 
installing systemd services. According to the fedora packaging guidelines this 
is providing the "_unitdir" macro. I noticed that it works without. However 
this is according the guidelines which are meant to be followed.
Line 20: %else
Line 21: BuildRequires: systemd-units
Line 22: %endif
Line 23: 


Line 31: Requires: kernel > 2.6.18-238.5.0
Line 32: Requires: usermode
Line 33: Provides: %{name} = %{version}-%{release}
Line 34: %if 0%{?fc16}
Line 35: Requires: selinux-policy >= 3.10.0-77
True, I have overlooked that. I will fix this.
Line 36: %endif
Line 37: %if 0%{?fedora} >= 17
Line 38: Requires: selinux-policy >= 3.10.0-89
Line 39: %endif


Line 69: run-time data from within the guest itself. The agent also accepts
Line 70: control commands to be run executed within the OS (like: shutdown and
Line 71: restart).
Line 72: 
Line 73: %description service
You don't have to type ovirt-guest-agent-service because it provides 
'ovirt-guest-agent' Therefore you would just require to install 
ovirt-guest-agent.
Line 74: This is the oVirt management agent running inside the guest. The agent
Line 75: interfaces with the oVirt manager, supplying heart-beat info as well as
Line 76: run-time data from within the guest itself. The agent also accepts
Line 77: control commands to be run executed within the OS (like: shutdown and


Line 109: # Update timestamps on Python files in order to avoid differences 
between
Line 110: # .pyc/.pyo files.
Line 111: touch -r %{SOURCE0} %{buildroot}%{_datadir}/ovirt-guest-agent/*.py
Line 112: 
Line 113: # Create needed folders
Ok I will remove it
Line 114: mkdir -p %{buildroot}%{_localstatedir}/log/ovirt-guest-agent
Line 115: mkdir -p %{buildroot}%{_localstatedir}/lock/subsys/ovirt-guest-agent
Line 116: 
Line 117: ln -sf /usr/bin/consolehelper 
%{buildroot}%{_datadir}/ovirt-guest-agent/ovirt-locksession


Line 177: %config(noreplace) %{_sysconfdir}/security/console.apps/ovirt-shutdown
Line 178: %config(noreplace) 
%{_sysconfdir}/security/console.apps/ovirt-hibernate
Line 179: %config(noreplace) %{_sysconfdir}/pam.d/ovirt-locksession
Line 180: %config(noreplace) %{_sysconfdir}/pam.d/ovirt-shutdown
Line 181: %config(noreplace) %{_sysconfdir}/pam.d/ovirt-hibernate
To which ones is it not applying? I will remove the others then. I will group 
those where noreplace is not applicable below the comment, for the review 
process this must be justified when this is not used and the best is to comment 
it in the spec file.
Line 182: 
Line 183: #This is intentionally NOT 'noreplace' it's a required udev rule. If 
this is
Line 184: #modified by an user this actually might break it.
Line 185: %config %attr (644,root,root) 
%{_sysconfdir}/udev/rules.d/55-ovirt-guest-agent.rules


Line 180: %config(noreplace) %{_sysconfdir}/pam.d/ovirt-shutdown
Line 181: %config(noreplace) %{_sysconfdir}/pam.d/ovirt-hibernate
Line 182: 
Line 183: #This is intentionally NOT 'noreplace' it's a required udev rule. If 
this is
Line 184: #modified by an user this actually might break it.
ok
Line 185: %config %attr (644,root,root) 
%{_sysconfdir}/udev/rules.d/55-ovirt-guest-agent.rules
Line 186: 
Line 187: %attr (755,root,root) 
%{_datadir}/ovirt-guest-agent/ovirt-guest-agent.py*
Line 188: 


Line 209: %doc AUTHORS COPYING NEWS README
Line 210: 
Line 211: %files pam-module
Line 212: %{_moduledir}/pam_ovirt_cred.so
Line 213: %doc AUTHORS COPYING NEWS README
Well, rpmlint warns you about not having documentation included. I have for now 
included these files. If you have a better suggestion I am open to suggestions. 
I would at a very minimum include the COPYING file because of the license.
Line 214: 
Line 215: %files gdm-plugin
Line 216: %config(noreplace) %{_sysconfdir}/pam.d/gdm-ovirtcred
Line 217: %{_datadir}/icons/hicolor/*/*/*.png


Line 226: %doc AUTHORS COPYING NEWS README
Line 227: 
Line 228: %changelog
Line 229: * Fri Oct 19 2012 Vinzenz Feenstra <vfeen...@redhat.com> - 1.0.5-2
Line 230: - introduced ovirt-guest-agent-service noarch package which provides 
will be removed
Line 231:   ovirt-guest-agent and avoids duplication of the same package content
Line 232: - fixed various rpmlint errors and warnings
Line 233: - added AUTHORS, COPYING, NEWS and README to all subpackages
Line 234: - changed conflicts to be requires, which is more accurate


--
To view, visit http://gerrit.ovirt.org/8715
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iec350b4027bffff8dff73db7986020fdc48261cd
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-guest-agent
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <vfeen...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Gal Hammer <gham...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to