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=524105

Fabio Massimo Di Nitto fdinitto@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- CC| |fdinitto@redhat.com AssignedTo|nobody@fedoraproject.org |fdinitto@redhat.com

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=524105

--- Comment #3 from Fabio Massimo Di Nitto fdinitto@redhat.com 2009-09-21 05:13:59 EDT --- Hi Lon,

let's start with the basic stuff from the checklist:

rpmlint SPECS/* SRPMS/* RPM*/*/* fence-virt-compat.x86_64: W: no-documentation fence-virt-compat.x86_64: W: dangling-relative-symlink /usr/sbin/fence_xvm fence_virt fence-virtd.x86_64: W: no-documentation fence-virtd-checkpoint.x86_64: W: no-documentation fence-virtd-libvirt.x86_64: W: no-documentation fence-virtd-multicast.x86_64: W: no-documentation 8 packages and 1 specfiles checked; 0 errors, 6 warnings.

The dangling-relative-symlink is OK, we will need to coordinate fence_xvm deprecation from fence-agents package, but be aware that while we go through the process, we will need a Conflicts (even if undesired), and later fence-agents will require fence-virt-compat (a two stage commit).

For the documentation, read my comments below about the spec file.

MUST:

* The package must be named according to the Package Naming Guidelines: OK * The spec file name must match the base package: OK * The package must meet the Packaging Guidelines: see below details. * The package must be licensed with a Fedora approved license: OK (GPLv2+) * The License field in the package spec file must match the actual licens: OK * If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package must be included in %doc: this needs to be done for each subpackage (and it will also kill all the no-documentation warnings). * The spec file must be written in American English: OK * The spec file for the package MUST be legible: OK * The sources used to build the package must match the upstream source: OK * The package MUST successfully compile and build into binary rpms on at least one primary architecture: OK * All build dependencies must be listed in BuildRequires: NOT OK. The package is missing some BuildRequires (nss and xml2 at least) * The spec file MUST handle locales properly: does not apply * Every binary RPM package (or subpackage) which stores shared library files: does not apply * Packages must NOT bundle copies of system libraries: OK * A package must own all directories that it creates: NOT OK. %{_libdir}/%{name}/ is not owned by the package or subpackages * A Fedora package must not list a file more than once in the spec file's %files listings: OK * Permissions on files must be set properly: OK * Each package must have a %clean section: OK * Each package must consistently use macros: OK * The package must contain code, or permissable conten: OK * Large documentation files must go in a -doc subpackage: does not apply * If a package includes something as %doc, it must not affect the runtime of the application: OK * Header files must be in a -devel package: does not apply. * Static libraries must be in a -static package: does not apply. * Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig': does not apply. * If a package contains library files with a suffix...: does not apply. * In the vast majority of cases, devel packages must...: does not apply. * Packages must NOT contain any .la libtool archives: OK * Packages containing GUI applications must include a %{name}.desktop file: does not apply * Packages must not own files or directories already owned by other packages: OK * At the beginning of %install, each package MUST run rm -rf: OK * All filenames in rpm packages must be valid UTF-8: OK

SHOULD: * If the source package does not include license text: OK. upstream ships licence file. * The description and summary sections in the package spec file should contain translations: does not apply * The reviewer should test that the package builds in mock: NOK (missing BuildRequires) * The package should compile and build into binary rpms on all supported architectures: (same as above) * The reviewer should test that the package functions as described: OK * If scriptlets are used, those scriptlets must be san: does not apply * Usually, subpackages other than devel should require the base package: At a first glance it looks ok with the exception of: fence-virtd-multicast Requires: %{name}-host = but there is no -host package? * The placement of pkgconfig(.pc) files depends on their usecase..: does not apply * If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, ..: OK.

A few notes on the spec file:

* URL should be in the form of: http://fence-virt.git.sourceforge.net/fence-virt/fence-virt/fence-virt-%%7Bv... to include the file name to download.

* the format %{buildroot} is preferred over $RPM_BUILD_ROOT

* %install section can be simpler. The whole installation of docs is redundant.

in the %files section use: %doc COPYING TODO README

and you will achieve the same result. The macro %doc looks for files in the unpacked source tree and copy them to the correct %{_docdir}/%{name}-%{version}/ for you.

this will also allow you to copy COPYING and README into subpackages easily.

* the %configure macro is best expressed as %{configure} for consistency. Both works.

Please fix the few remaining issues.

Fabio

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=524105

--- Comment #5 from Lon Hohberger lhh@redhat.com 2009-09-23 10:47:19 EDT --- I missed two things; I am addressing them now.

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=524105

--- Comment #7 from Lon Hohberger lhh@redhat.com 2009-09-23 14:50:55 EDT ---

New Files:

http://voxel.dl.sourceforge.net/project/fence-virt/fence-virt.spec http://voxel.dl.sourceforge.net/project/fence-virt/fence-virt-0.1.2-1.fc12.s...

[root@localhost ~]# rpmlint ~lhh/fence-virt.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings.

The source RPM is from the results of a mock run:

[root@localhost SPECS]# mock -r fedora-rawhide-x86_64 --rebuild /sandbox/lhh/rpm/SRPMS/fence-virt-0.1.2-1.fc11.src.rpm INFO: mock.py version 0.9.17 starting... State Changed: init plugins State Changed: start INFO: Start(/sandbox/lhh/rpm/SRPMS/fence-virt-0.1.2-1.fc11.src.rpm) Config(fedora-rawhide-x86_64) State Changed: lock buildroot State Changed: clean State Changed: init State Changed: lock buildroot Mock Version: 0.9.17 INFO: Mock Version: 0.9.17 INFO: enabled root cache State Changed: unpacking root cache INFO: enabled yum cache State Changed: cleaning yum metadata INFO: enabled ccache State Changed: running yum State Changed: setup State Changed: build INFO: Done(/sandbox/lhh/rpm/SRPMS/fence-virt-0.1.2-1.fc11.src.rpm) Config(fedora-rawhide-x86_64) 1 minutes 57 seconds

Rpmlint on the mock results:

[root@localhost result]# rpmlint *rpm fence-virtd.x86_64: W: no-documentation fence-virtd-checkpoint.x86_64: W: no-documentation fence-virtd-libvirt.x86_64: W: no-documentation fence-virtd-multicast.x86_64: W: no-documentation 7 packages and 0 specfiles checked; 0 errors, 4 warnings.

Ran this one through Koji --scratch build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=1701484

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=524105

--- Comment #9 from Lon Hohberger lhh@redhat.com 2009-09-23 14:56:23 EDT --- Oops -- sorry; wrong package spin. Here's the correct koji build:

http://koji.fedoraproject.org/koji/taskinfo?taskID=1701613

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=524105

Fabio Massimo Di Nitto fdinitto@redhat.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review+

--- Comment #11 from Fabio Massimo Di Nitto fdinitto@redhat.com 2009-09-24 00:24:45 EDT --- Verified that all the small glitches reported above have been fixed.

The package is good to go in.

Fabio

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=524105

--- Comment #13 from Lon Hohberger lhh@redhat.com 2009-09-24 12:08:06 EDT --- Strange; I've had fedora CVS package commit access for some time, yet I can't set fedora-cvs?

...

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=524105

--- Comment #15 from Lon Hohberger lhh@redhat.com 2009-09-24 15:47:44 EDT --- Oops - forgot devel branch.

New Package CVS Request ======================= Package Name: fence-virt Short Description: Pluggable Virtual Machine Fencing Owners: lon Branches: F-12, devel InitialCC: fabbione

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=524105

Kevin Fenzi kevin@tummy.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+

--- Comment #17 from Kevin Fenzi kevin@tummy.com 2009-09-25 12:15:39 EDT --- cvs done.

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=524105

--- Comment #18 from Lon Hohberger lhh@redhat.com 2009-12-07 09:44:45 EDT --- Package Change Request ====================== Package Name: fence-virt New Branches: F-11 Owners: lon fabbione

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=524105

Kevin Fenzi kevin@tummy.com changed:

What |Removed |Added ---------------------------------------------------------------------------- Flag|fedora-cvs? |fedora-cvs+

--- Comment #19 from Kevin Fenzi kevin@tummy.com 2009-12-07 11:36:22 EDT --- cvs done.