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=527488 --- Comment #50 from LINBIT <partner@xxxxxxxxxx> 2009-10-19 03:50:49 EDT --- Spec file and SRPM: http://people.linbit.com/~florian/drbd.spec http://people.linbit.com/~florian/drbd-8.3.4-10.src.rpm (In reply to comment #48) > Well, > > * %description > - I don't think writing the authors of this software in > %description is needed. Gone. > - And the description "Please report bugs to drbd-dev@xxxxxxxxxxxxxxxx" > is in my opinion wrong (because we have our BTS) Gone. > * BuildRequires > - BR: gcc is redundant: > https://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2 I realize that; however I still prefer to list the full build dependencies. The packaging guidelines state that the packages are considered "the minimum build environment", but if a clueless user happens to run rpmbuild without gcc installed, then I consider it proper to fail with an unsatisfied build dependency, rather than with a relatively obscure error from configure, midway during %prep. > * Dependency between subpackages > - Usually the dependency between packages rebuilt from a srpm should > be strict EVR (not just version) specific > (i.e. usually the dependency should be like: > Requires: %{name}-utils = %{version}-%{release} ) > https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package Fixed. > * Parallel make > - Use %{?_smp_mflags} instead of %{_smp_mflags} because %{_smp_mflags} macro > may not be defined. Fixed. > * %bcond_without > - If you want to really use %bcond_without/%bcond_with, please support > all possible patterns carefully, or just drop to use %bcond_ method. > ! For example currently passing "--without utils" to your srpm fails > because all other subpackages depend on -utils subpackage but > with "--without utils" -utils subpackage won't be rebuilt. I had considered it convenient to still be able to provide a shortcut for rolling, for example, a drbd-udev package without having to compile utils. Which would absolutely be possible as far as the configure/make/make install procedure is concerned. Apparently this convenience is not needed, thus, the "--with utils" conditional is gone. > * %defattr > - Now we recommend to use %defattr(-,root,root,-) Fixed. > * About -rgmanager subpackage > - Is -rgmanager part really needed? From the description in the spec file > currently this subpackage can be rebuilt only for F-10, which is about to > be EOL and on F-11/12/13 this cannot be supported. Addressed by Fabio -- see comment #49. > * License tag > - scripts/drbd.ocf is under GPLv2 and installed as > /usr/lib/ocf/resource.d/linbit/drbd, -pacemaker subpackage should have > "GPLv2 and GPLv2+" license tag (or just GPLv2) Good catch. Both the Linux-HA and Pacemaker projects seem, for the time being, to be GPLv2 exclusive, so I fixed the License tag for both drbd-pacemaker and drbd-heartbeat. > * sysvinit script treatment > - Some Requires(post) or so are missing. Fixed. > - Would you explain why condrestart is not needed at %postun (on upgrade)? The drbd init script merely configures DRBD resources listed in drbd.conf. It does not start or stop any daemon or similar. The only situation in which a restart would be needed during a package upgrade would be if a) new features (and hence, new drbd.conf keywords) would be introduced in the new release, AND b) a resource listed in the drbd.conf configuration file would be changed to actually use one of those new features. Since b) is impossible as the configuration file is listed as %config(noreplace) and is hence not touched upon upgrade, a condrestart is not required on upgrade. > https://fedoraproject.org/wiki/Packaging/SysVInitScript#InitscriptScriptlets > > - Please use %{_initddir} Fixed. I now use %{_initddir} and if that is not defined, I fall back to %{_initrddir} -- the spec has to build on CentOS 5, which does not have %{_initddir} defined. > https://fedoraproject.org/wiki/Packaging/SysVInitScript#Initscripts_on_the_filesystem > > * Duplicate files > - You don't have to include COPYING file as %doc other than -utils > subpackage because all other packages depends on -utils subpackage. Now I have two conflicting reviewer comments. Fabio (in comment #17) tells me that all subpackages should contain the COPYING file. Mamoru (in comment #48) tells me they shouldn't. http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text is inconclusive and doesn't mention sub-packages specifically. What should I do? Build logs: http://koji.fedoraproject.org/koji/taskinfo?taskID=1753544 (f12) http://koji.fedoraproject.org/koji/taskinfo?taskID=1753549 (f13) Changelog: 093e841... drbd.spec.in: remove reference to authors and bug reports 38114ad... drbd.spec.in: fix dependencies to include release number 546e733... drbd.spec.in: do not require defined %{_smp_mflags} macro 9de2c41... drbd.spec.in: fix %defattr to include directory mode 29f1771... drbd.spec.in: remove --with utils e40e9ee... drbd.spec.in: change License tag for drbd-heartbeat and drbd-pacemaker 8ad0220... configure.ac, drbd.spec.in: add --with-initdir 20671ee... drbd.spec.in: add missing Requires tags 600c73b... RPM spec files: bump release number -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review