[Bug 887778] Review Request: cutter - A Unit Testing Framework for C

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=887778

--- Comment #6 from HAYASHI Kentaro <kenhys@xxxxxxxxx> ---
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

-- 
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=ILQn4dWF2f&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]