Product: Fedora https://bugzilla.redhat.com/show_bug.cgi?id=887778 --- Comment #5 from Michael Schwendt <mschwendt@xxxxxxxxx> --- Great update with many changes and a detailed %changelog entry! A few issues remain or have been introduced with the new subpackages, however. More about that further below. Some of the issues are found in hundreds of other packages, too, despite the existance of related packaging guidelines. Sometimes the issues are not noticed during review. In other cases, they are reintroduced with updates to a package. Not all issues are trivial to fix. Sometimes a fix is nice to have, but no more than that. [...] rpmlint doesn't report any important errors or warnings for the update and its builds: $ rpmlint *|grep -v spell|grep -v no-doc 6 packages and 0 specfiles checked; 0 errors, 8 warnings. [...] So, let's examine the spec changes: > BuildRequires: autoconf > > %prep > ... > autoconf Rebuilding Autotools' files typically belongs into the %build section. That's the more appropriate/convenient place, because one could still run "rpmbuild -bp cutter.spec" (or even with --nodeps if the build requirements are not installed) and get only the patched/prepared source tree, e.g. when rediffing patches. The "fedora-review" script also fails when the %prep section runs tools that are not available outside the Mock build environment. > %package report > Requires: goffice08 Explicitly depending on the "goffice08" package name is not necessary and bears a higher risk of needing an unnecessary rebuild/update in the future or being wrong. Building with goffice08 libraries results in automatically detected dependencies on the actual libraries: https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires For example: $ rpm -qpR cutter-report-1.2.2-3.fc18.x86_64.rpm |grep off goffice08 libgoffice-0.8.so.8()(64bit) $ repoquery --whatprovides 'libgoffice-0.8.so.8()(64bit)' goffice08-0:0.8.17-8.fc18.x86_64 Imagine the lib moving to package goffice08-libs. > %package gui > Requires: gtk2 > %package gstreamer > Requires: gstreamer Similarly for those two subpackages. > %package gstreamer > Summary: Cutter GStreamer plugin > %description gstreamer > Cutter GStreamer plugin. Not a blocker, but the description (and %summary) could be a bit more verbose and try to tell _what_ this package does related to GStreamer. That is not obvious even with the two lines of the description copied from the base package. As not to confuse ordinary GStreamer users, who search for "real" GStreamer plug-ins. > +%files gui > +%defattr(-, root, root, -) %defattr is optional for Fedora and EL >= 5: https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions ( https://fedoraproject.org/wiki/EPEL/GuidelinesAndPolicies#Distribution_specific_guidelines ) > +%dir %{_libdir}/cutter > +%dir %{_libdir}/cutter/module > +%dir %{_libdir}/cutter/module/factory > +%dir %{_libdir}/cutter/module/factory/ui > +%dir %{_libdir}/cutter/module/ui Here it gets a little bit complicated, because there are multiple subpackages that store files in these directories. The guidelines say: https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership | Directory ownership is a little more complex than file | ownership. Packages must own all directories they put files in, | except for: | | [...] | | any directories owned by the filesystem, man, or other explicitly | created -filesystem packages | | any directories owned by other packages in your package's natural | dependency chain For cutter this means that since the subpackages depend on the base package, it is sufficient that the base package contains the needed %dir entries. Example: %files report %{_libdir}/cutter/module/factory/report/pdf_factory.so %{_libdir}/cutter/module/report/pdf.so $ rpm -qpR cutter-report-1.2.2-3.fc18.x86_64.rpm |grep cut cutter(x86-64) = 1.2.2-3.fc18 libcutter.so.0()(64bit) If there are many (more) subpackages, it can make sense to move directory ownership into a *-filesystem package and have all subpackages depend on that one, but this would be overhead for the Cutter packages so far. > %doc %{_datadir}/gtk-doc/html/cutter/ The file/directory ownership guidelines covers this special directory. It is okay for cutter-devel to include the two leading directories instead of depending on "gtk-doc": %dir %{_datadir}/gtk-doc/ %dir %{_datadir}/gtk-doc/html/ %doc %{_datadir}/gtk-doc/html/cutter/ Or simply this to include the dir and everything below it: %doc %{_datadir}/gtk-doc/ https://fedoraproject.org/wiki/Packaging:Guidelines#The_directory_is_owned_by_a_package_which_is_not_required_for_your_package_to_function Note that several packages don't get it right at all, since directory ownership isn't simple. [...] Finally, and probably the most advanced packaging topic related to Cutter, is this in the guidelines: https://fedoraproject.org/wiki/Packaging:Guidelines#Filtering_Auto-Generated_Requires https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering | MUST: Packages must not provide RPM dependency information when that | information is not global in nature, or are otherwise handled | (e.g. through a virtual provides system). e.g. a plugin package | containing a binary shared library must not "provide" that library | unless it is accessible through the system library paths. It refers to the automatic Provides for Cutter's private module libs, e.g. cutter-report and cutter-gui: $ rpm -qp --provides cutter-report-1.2.2-3.fc18.x86_64.rpm cutter-report = 1.2.2-3.fc18 cutter-report(x86-64) = 1.2.2-3.fc18 pdf.so()(64bit) pdf_factory.so()(64bit) $ rpm -qp --provides cutter-gui-1.2.2-3.fc18.x86_64.rpm cutter-gui = 1.2.2-3.fc18 cutter-gui(x86-64) = 1.2.2-3.fc18 gtk.so()(64bit) gtk_factory.so()(64bit) It's not an immediate problem/blocker, and the plugin names don't start with "lib" either (fortunately!). But packagers need to be aware of this, because it can lead to issues when any other package depends on a library with the same name, and then the Cutter subpackage would be the wrong one to satisfy such a dependency. Again, other packages don't get this right either despite the "MUST" in the guidelines. This is because it's easily overlooked, misunderstood, or not considered more than a potential problem. $ repoquery --whatprovides 'pdf.so()(64bit)' zathura-pdf-poppler-0:0.2.1-4.fc18.x86_64 GraphicsMagick-0:1.3.17-1.fc18.x86_64 zathura-pdf-poppler-0:0.2.1-5.fc18.x86_64 ImageMagick-0:6.7.7.5-3.fc18.x86_64 GraphicsMagick-0:1.3.17-1.fc18.x86_64 libabiword-1:2.8.6-18.fc18.x86_64 $ repoquery --whatprovides 'gtk.so()(64bit)' ekg2-gtk2-0:0.3.1-5.fc18.x86_64 rep-gtk-0:0.90.8-2.fc18.x86_64 $ repoquery --whatrequires 'gtk.so()(64bit)' $ Afterall, most real system libraries are named "libsomething" and are versioned, too, so "pdf.so" and other plugin libs are unlikely to disturb real library dependencies. There's also the following in the guidelines, which tells when the Provides/Requires filtering macros must not be used: https://fedoraproject.org/wiki/Packaging:AutoProvidesAndRequiresFiltering#Usage A work-around since RPM 4.9 would be this line in the spec file: %global __provides_exclude_from ^%{_libdir}/%{name}/module/.*\\.so$ I cannot tell what the Fedora Packaging Committee says about that, however. It is not covered by the guidelines yet. -- 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=w4628xUiMB&a=cc_unsubscribe _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review