Bug 887778
Summary: | Review Request: cutter - A Unit Testing Framework for C | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Kentaro Hayashi <kenhys> | ||||
Component: | Package Review | Assignee: | Michael Schwendt <bugs.michael> | ||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | unspecified | ||||||
Version: | rawhide | CC: | bugs.michael, notting, package-review | ||||
Target Milestone: | --- | Flags: | bugs.michael:
fedora-review+
gwync: fedora-cvs+ |
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2013-02-19 01:24:10 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Attachments: |
|
Description
Kentaro Hayashi
2012-12-17 10:26:27 UTC
Hi, I had found that cxxtest package style (no -devel package) is exceptional case, so I have split libraries and header files into -devel package. Here is the updated spec and SRPM. Spec URL: http://sourceforge.net/projects/cutter/files/tmp/cutter.spec SRPM URL: http://sourceforge.net/projects/cutter/files/tmp/cutter-1.2.2-2.fc17.src.rpm Here is the result of rpmlint -i: % rpmlint -i cutter-1.2.2-2.fc17.x86_64.rpm cutter-devel-1.2.2-2.fc17.x86_64.rpm cutter.x86_64: W: spelling-error %description -l en_US xUnit -> x Unit, unit The value of this tag appears to be misspelled. Please double-check. cutter-devel.x86_64: W: spelling-error %description -l en_US xUnit -> x Unit, unit The value of this tag appears to be misspelled. Please double-check. 2 packages and 0 specfiles checked; 0 errors, 2 warnings. There are two warnings above, but it's false detection. > cxxtest Misleading IMO. I've filed bug 888509 about that. Btw, there's "cpptest" as an example of how to package something like this. [...] A first look at the submitted package: > Summary: A Unit Testing Framework for C/C++ Similar to your -devel package %summary, omitting the leading article makes it more readable. Summary: Unit Testing Framework for C/C++ Once can more quickly focus on the importing part. > Summary: Libraries and header files for Cutter development And as one can see, here you also didn't start with "The libraries and ...". :-) > Group: Development/Tools Rather "Development/Libraries". Perhaps "Development/Languages". Afterall, this isn't just a utility/tool to install and run. > BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-%(%{__id_u} -n) https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag The "fedora-review" tool complains about this tag, too. > cutter-1.2.2/glib-compatible/pcre/pcre_fullinfo.c The fedore-review tool here discovers license "BSD (3 clause)", but a closer look reveals that this is an old bundled PCRE lib for glib 2.12, which isn't used for a newer glib2. glib-compatible/gregex.c: 28 #ifdef USE_SYSTEM_PCRE 29 #include <pcre.h> 30 #else 31 #include "pcre/pcre.h" 32 #endif So, only for completeness and in case any older target system features an old glib, I need to point at https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries and it may be safer to delete this bundled lib at the end of the %prep section, so if used it would cause a build failure. ;-) > BuildRequires: intltool > BuildRequires: glib2-devel > BuildRequires: libsoup-devel > checking for GTK+ - version >= 2.12.0... no > *** Could not run GTK+ test program, checking why... > *** The test program failed to compile or link. See the file config.log for the > *** exact error that occured. This usually means GTK+ is incorrectly installed. This warning in the build.log made me curious. What happens if one adds "BuildRequires: gtk2-devel"? The build fails: error: Installed (but unpackaged) file(s) found: /usr/lib64/libgdkcutter-pixbuf.so /usr/lib64/libgdkcutter-pixbuf.so.0 /usr/lib64/libgdkcutter-pixbuf.so.0.1.0 Why isn't gtk2-devel built with? Is anything else missing? The build output contains a few more "No" answers to configure existance checks. https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires For sake of "reproducible builds", it's common practice to either add missing BuildRequires or add --disable-foo options to explicitly build without certain features, even if the build dependencies are installed. > %package devel > Group: Development/Tools Same suggestion as above. > Requires: %{name} = %{version}-%{release} https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package > make %{?_smp_mflags} Please prepend V=1 for verbose build output. That makes build system build.log contents much more useful in case of errors. That's also the only way to see that the global compiler flags are used actually (not just prior for the configure check): https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags > %install > rm -rf %{buildroot} https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag > find %{buildroot} -name keep-me -size 0 -delete Hmmm? :) An excellent opportunity to add a brief comment to the spec file _why_ this is being done. $ find cutter-1.2.2|grep keep cutter-1.2.2/sample/stack/config/keep-me > %find_lang %{name} > %{_mandir}/ja/man1/* You could use %find_lang %{name} --with-man --all-name and not only would it find and add also the translated (currently only Japanese) manual pages (instead of you having to add then in the %files section manually), it would also flag them with the corresponding lang(…) marker. For example, the suggested %find_lang command would add these entries: %lang(ja) /usr/share/locale/ja/LC_MESSAGES/cutter.mo %lang(ja) /usr/share/man/ja/man1/* > cutter-1.2.2/test/ Sounds like this could/should be added to %check section. %check V=1 LD_LIBRARY_PATH=$(pwd)/cutter/.libs make check Objections? Should I take a second look? ;) $ rpmls -p cutter-1.2.2-2.fc18.3.x86_64.rpm|grep ^d drwxr-xr-x /usr/lib64/cutter/module drwxr-xr-x /usr/lib64/cutter/module/factory drwxr-xr-x /usr/lib64/cutter/module/factory/report drwxr-xr-x /usr/lib64/cutter/module/factory/stream drwxr-xr-x /usr/lib64/cutter/module/factory/ui drwxr-xr-x /usr/lib64/cutter/module/report drwxr-xr-x /usr/lib64/cutter/module/stream drwxr-xr-x /usr/lib64/cutter/module/ui drwxr-xr-x /usr/share/cutter/icons/kinotan drwxr-xr-x /usr/share/cutter/ui drwxr-xr-x /usr/share/doc/cutter-1.2.2 https://fedoraproject.org/wiki/Packaging:UnownedDirectories /usr/lib64/cutter is not included in the package. /usr/share/cutter is not included in the package. /usr/share/cutter/icons is not included in the package. > %{_datadir}/cutter/license/* /usr/share/cutter/license is not included in the package. The license files are not marked as %doc. https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text * /usr/share/cutter/ui is empty. An oversight or future-proof? * cutter.1 man page (also the translated one) refers to /usr/local/share/doc/cutter/ This path could be substituted at build-time. %{_defaultdocdir}/%{name}-%{version}-%{release} > %clean > rm -rf %{buildroot} Unless you want to build this for EL-5, %clean is not necessary anymore: https://fedoraproject.org/wiki/Packaging:Guidelines#.25clean Thank you for reviewing!! Here is the updated spec and SRPM. Spec URL: http://sourceforge.net/projects/cutter/files/tmp/cutter.spec/download SRPM URL: http://sourceforge.net/projects/cutter/files/tmp/cutter-1.2.2-3.fc17.src.rpm (In reply to comment #2) > > > Summary: A Unit Testing Framework for C/C++ > > Similar to your -devel package %summary, omitting the leading article makes > it more readable. > > Summary: Unit Testing Framework for C/C++ > > Once can more quickly focus on the importing part. > > > Summary: Libraries and header files for Cutter development > > And as one can see, here you also didn't start with "The libraries and ...". > :-) > Fixed! > > > Group: Development/Tools > > Rather "Development/Libraries". Perhaps "Development/Languages". Afterall, > this isn't just a utility/tool to install and run. > Changed to Development/Libraries category. > > > BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-%(%{__id_u} -n) > > https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag > > The "fedora-review" tool complains about this tag, too. > Removed BuildRoot tag. > > > cutter-1.2.2/glib-compatible/pcre/pcre_fullinfo.c > > The fedore-review tool here discovers license "BSD (3 clause)", but a closer > look reveals that this is an old bundled PCRE lib for glib 2.12, which isn't > used for a newer glib2. > > glib-compatible/gregex.c: > > 28 #ifdef USE_SYSTEM_PCRE > 29 #include <pcre.h> > 30 #else > 31 #include "pcre/pcre.h" > 32 #endif > > So, only for completeness and in case any older target system features an > old glib, I need to point at > https://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries and it may be > safer to delete this bundled lib at the end of the %prep section, so if used > it would cause a build failure. ;-) > Removed in %prep section. > > > BuildRequires: intltool > > BuildRequires: glib2-devel > > BuildRequires: libsoup-devel > > > checking for GTK+ - version >= 2.12.0... no > > *** Could not run GTK+ test program, checking why... > > *** The test program failed to compile or link. See the file config.log for the > > *** exact error that occured. This usually means GTK+ is incorrectly installed. > > This warning in the build.log made me curious. What happens if one adds > "BuildRequires: gtk2-devel"? The build fails: > > error: Installed (but unpackaged) file(s) found: > /usr/lib64/libgdkcutter-pixbuf.so > /usr/lib64/libgdkcutter-pixbuf.so.0 > /usr/lib64/libgdkcutter-pixbuf.so.0.1.0 > > Why isn't gtk2-devel built with? > Is anything else missing? The build output contains a few more "No" answers > to configure existance checks. > https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRequires > > For sake of "reproducible builds", it's common practice to either add > missing BuildRequires or add --disable-foo options to explicitly build > without certain features, even if the build dependencies are installed. > According to dependency, split cutter package into subpackages! -> cutter-report cutter-gui cutter-gstreamer > > > > %package devel > > Group: Development/Tools > > Same suggestion as above. > Fixed. > > Requires: %{name} = %{version}-%{release} > > https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package > Fixed. > > > make %{?_smp_mflags} > > Please prepend V=1 for verbose build output. That makes build system > build.log contents much more useful in case of errors. That's also the only > way to see that the global compiler flags are used actually (not just prior > for the configure check): > https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags > > > > %install > > rm -rf %{buildroot} > > https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag > Fixed not to rm -rf. > > > find %{buildroot} -name keep-me -size 0 -delete > > Hmmm? :) An excellent opportunity to add a brief comment to the spec file > _why_ this is being done. > > $ find cutter-1.2.2|grep keep > cutter-1.2.2/sample/stack/config/keep-me > 'keep-me' file was added to distribute config directory purpose, but that directory would be kind of automatically generated. So I've fixed in upstream. (patch will be dropped in the next release) > > > %find_lang %{name} > > %{_mandir}/ja/man1/* > > You could use > > %find_lang %{name} --with-man --all-name > > and not only would it find and add also the translated (currently only > Japanese) manual pages (instead of you having to add then in the %files > section manually), it would also flag them with the corresponding lang(…) > marker. For example, the suggested %find_lang command would add these > entries: > > %lang(ja) /usr/share/locale/ja/LC_MESSAGES/cutter.mo > %lang(ja) /usr/share/man/ja/man1/* > Fixed. > > > cutter-1.2.2/test/ > > Sounds like this could/should be added to %check section. > > %check > V=1 LD_LIBRARY_PATH=$(pwd)/cutter/.libs make check > > Objections? Should I take a second look? ;) > > Added %check entry. > $ rpmls -p cutter-1.2.2-2.fc18.3.x86_64.rpm|grep ^d > drwxr-xr-x /usr/lib64/cutter/module > drwxr-xr-x /usr/lib64/cutter/module/factory > drwxr-xr-x /usr/lib64/cutter/module/factory/report > drwxr-xr-x /usr/lib64/cutter/module/factory/stream > drwxr-xr-x /usr/lib64/cutter/module/factory/ui > drwxr-xr-x /usr/lib64/cutter/module/report > drwxr-xr-x /usr/lib64/cutter/module/stream > drwxr-xr-x /usr/lib64/cutter/module/ui > drwxr-xr-x /usr/share/cutter/icons/kinotan > drwxr-xr-x /usr/share/cutter/ui > drwxr-xr-x /usr/share/doc/cutter-1.2.2 > > https://fedoraproject.org/wiki/Packaging:UnownedDirectories > > /usr/lib64/cutter is not included in the package. > > /usr/share/cutter is not included in the package. > > /usr/share/cutter/icons is not included in the package. > Fixed. > > %{_datadir}/cutter/license/* > > /usr/share/cutter/license is not included in the package. > > The license files are not marked as %doc. > https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#License_Text > Fixed. > > * /usr/share/cutter/ui is empty. An oversight or future-proof? > When cutter is built with GTK+, GTK+ frontend module contains definition file under that directory. now there is gtk-menu.ui (cutter-gui package). > > * cutter.1 man page (also the translated one) refers to > /usr/local/share/doc/cutter/ > This path could be substituted at build-time. > %{_defaultdocdir}/%{name}-%{version}-%{release} Fixed. 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. Thank you for reviewing! Here is the updated(1.2.2-3 -> 1.2.2-4) spec and SRPM. Spec URL: http://sourceforge.net/projects/cutter/files/tmp/cutter.spec/download SRPM URL: http://sourceforge.net/projects/cutter/files/tmp/cutter-1.2.2-4.fc18.src.rpm (In reply to comment #5) Here is the result of rpmlint: $ rpmlint RPMS/x86_64/* |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. > Fixed! (fedora-review works) > > > %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. > Fixed! > > > %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. > Updated package description. > > > +%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) > Fixed to aggregate %dir entries. > 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. > Fixed %doc entry for gtk-doc. > [...] > > 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. Fixed not to provide private module soname. There is no private module soname such as 'pdf.so' or 'gtk.so'. $ rpm -qp --provides RPMS/x86_64/cutter-*.rpm cutter = 1.2.2-4.fc18 cutter(x86-64) = 1.2.2-4.fc18 libcppcutter.so.0()(64bit) libcutter.so.0()(64bit) libgdkcutter-pixbuf.so.0()(64bit) libsoupcutter.so.0()(64bit) cutter-debuginfo = 1.2.2-4.fc18 cutter-debuginfo(x86-64) = 1.2.2-4.fc18 cutter-devel = 1.2.2-4.fc18 cutter-devel(x86-64) = 1.2.2-4.fc18 pkgconfig(cppcutter) = 1.2.2 pkgconfig(cutter) = 1.2.2 pkgconfig(gcutter) = 1.2.2 pkgconfig(gdkcutter-pixbuf) = 1.2.2 pkgconfig(libcutter) = 1.2.2 pkgconfig(soupcutter) = 1.2.2 cutter-gstreamer = 1.2.2-4.fc18 cutter-gstreamer(x86-64) = 1.2.2-4.fc18 gstreamer0.10(element-cutter-console-output)()(64bit) gstreamer0.10(element-cutter-server)()(64bit) gstreamer0.10(element-cutter-test-runner)()(64bit) libgstcuttertest.so()(64bit) cutter-gui = 1.2.2-4.fc18 cutter-gui(x86-64) = 1.2.2-4.fc18 cutter-report = 1.2.2-4.fc18 cutter-report(x86-64) = 1.2.2-4.fc18 Created attachment 693385 [details]
remaining spec issues
APPROVED
Here are comments on a few last issues, which need not be fixed with another src.rpm, but can be fixed when/after importing into Fedora package git:
* The "rm -fr %{buildroot}/glib-compatible/pcre" after %prep effectively is a no-op, because %buildroot doesn't exist at that time yet. And to delete files from the extracted source archive, it would need to happen _after_ %setup, not before it. That's why I moved %setup a few lines in the attached patch. However, I had to comment out the "rm -rf …" line, because one cannot simply remove the directory, because that breaks the build very early (missing the Makefile.in template) or (if only removing the source files) very late during %check:
Making check in po
make[1]: Entering directory `/home/personal/tmp/rpm/BUILD/cutter-1.2.2/po'
make[1]: *** No rule to make target `../glib-compatible/pcre/pcre_chartables.c', needed by `cutter.pot'. Stop.
make[1]: Leaving directory `/home/personal/tmp/rpm/BUILD/cutter-1.2.2/po'
make: *** [check-recursive] Error 1
So, with regard to the unbundling policy, it's okay if you ensure that the included PCRE is not built with in favour of the system lib.
* "%dir %{_libdir}/gstreamer-0.10" is owned by the "gstreamer" package already, which cutter-gstreamer depends on automatically via libraries.
* rpmbuild for Fedora 19 development notices a few "bogus date" warnings in the %changelog, which I've corrected with the help of "date -d …".
Thank you for reviewing!! but I'm a bit confused. (In reply to comment #7) > Created attachment 693385 [details] > remaining spec issues > > APPROVED > > Here are comments on a few last issues, which need not be fixed with another > src.rpm, but can be fixed when/after importing into Fedora package git: > I will apply attachment 693385 [details] when importing package, but fedora-review remains set to "?" flag. So the next thing for this review process is fixing remaining issues and update spec (1.2.2-5) and SRPM, isn't it? (not making Package SCM admin requests) Oh, I've simply toggled a wrong flag (for the attachment), because I thought it would be the same one. Should have looked more closer. :-/ fedora-review flag is '+' now. (In reply to comment #9) > Oh, I've simply toggled a wrong flag (for the attachment), because I thought > it would be the same one. Should have looked more closer. :-/ > > fedora-review flag is '+' now. Oh, I see, thanks! New Package SCM Request ======================= Package Name: cutter Short Description: Unit Testing Framework for C/C++ Owners: kenhys Branches: f18 InitialCC: Git done (by process-git-requests). cutter-1.2.2-4.fc18 has been submitted as an update for Fedora 18. https://admin.fedoraproject.org/updates/cutter-1.2.2-4.fc18 cutter-1.2.2-4.fc18 has been pushed to the Fedora 18 testing repository. cutter-1.2.2-4.fc18 has been pushed to the Fedora 18 stable repository. |