Fedora Merge Review: cairo http://cvs.fedora.redhat.com/viewcvs/devel/cairo/ Initial Owner: besfahbo
GOOD ==== MUST: Naming Guidelines followed MUST: Packaging Guidelines generally followed (except SMP flags below) MUST: spec filename is fine MUST: License of the library itself is open source (LGPL/MPL) MUST: License filed matches actual licenses MUST: spec file legible MUST: sources match upstream (md5sum used) MUST: RPM build for i386 MUST: no ExcludeArch MUST: build dependencies fine MUST: no locales MUST: ldconfig called fine MUST: not relocatable MUST: no duplicate files MUST: file permissions fine (except for -debuginfo, mentioned below) MUST: %clean section fine MUST: macro use consistent MUST: package contains code MUST: large documentation files in -devel instead of -doc (acceptable) MUST: %doc does not affect run-time MUST: header files in -devel MUST: -devel requries pkgconfig MUST: *.so in -devel MUST: -devel has fully versioned dependency MUST: *.la removed explicitly MUST: not GUI MUST: doesn't own others' files or dirs SHOULD: no scriptlets SHOULD: no subpackage other than -devel SHOULD: pkgconfig file in -devel BAD === MUST: rpmlint output: $ rpmlint cairo-1.3.12-1.i386.rpm E: cairo obsolete-not-provided libpixman E: cairo obsolete-not-provided libpixman-devel E: cairo obsolete-not-provided libpixman-debuginfo (Provides should be added, -devel and -debuginfo should be obsoleted and provided by subpackages) $ rpmlint cairo-devel-1.3.12-1.i386.rpm W: cairo-devel no-documentation (The HTML files provided in the package are definitely documentation and should be marked as such.) $ rpmlint cairo-debuginfo-1.3.12-1.i386.rpm W: cairo-debuginfo spurious-executable-perm /usr/src/debug/cairo-1.3.12/src/cairo-scaled-font.c W: cairo-debuginfo spurious-executable-perm /usr/src/debug/cairo-1.3.12/src/cairoint.h (These two files are wrongly set to be executables.) MUST: licensing 1) The license files COPYING-LGPL-2.1 and COPYING-MPL-1.1 are not included in the binary RPM. 2) Some auxiliary files refer to GPL (like test/pdiff/pdiff.c), and say that one should have received a copy of the license with the file, while it is not available in the package itself. Upstream problem, but a SHOULD in review list (contact upstream). 3) Although the COPYING file mentions that all "auxiliary components" (test files etc.) are free software and refers to the files' headers, some are not, because their headers doesn't mention any such licensing, which makes them proprietary. Examples: composite-integer-translate-over-repeat.c, composite-integer-translate-source.c, ... in the tests subdirectory. MUST: US English Suggestions: replace "eg." with "e.g." or "for example"; capitalize "cairo" to "Cairo" in the -devel package description. MUST: owning directories The -devel subpackage installs files in /usr/share/gtk-doc/html without owning the directory or depending on anything that owns it. MUST: packaging guidelines * make should use %{?_smp_mflags} flag. SUGGESTIONS =========== * Consider shipping some of the not-shipped docs, like PORTING_GUIDE or TODO as documentation.
(In reply to comment #1) > BAD > === > MUST: rpmlint output: > > $ rpmlint cairo-1.3.12-1.i386.rpm > E: cairo obsolete-not-provided libpixman > E: cairo obsolete-not-provided libpixman-devel > E: cairo obsolete-not-provided libpixman-debuginfo > > (Provides should be added, -devel and -debuginfo should be obsoleted and > provided by subpackages) This is intentional and not error. We are not providing libpixman in cairo. Hiding it. Anybody depending on libpixman is doomed. But the only user was cairo. So that's not a problem in reality. > $ rpmlint cairo-debuginfo-1.3.12-1.i386.rpm > W: cairo-debuginfo spurious-executable-perm > /usr/src/debug/cairo-1.3.12/src/cairo-scaled-font.c > W: cairo-debuginfo spurious-executable-perm > /usr/src/debug/cairo-1.3.12/src/cairoint.h Weird. Not sure howthis is happening. > MUST: US English > > Suggestions: replace "eg." with "e.g." or "for example"; capitalize "cairo" > to "Cairo" in the -devel package description. Carl advertises for non-capitalized name usage. Mostly to differentiate from the lesser known Cairo.
> * Consider shipping some of the not-shipped docs, like PORTING_GUIDE or > TODO as documentation. Neither PORTING_GUIDE nor TODO are useful documentation for users of cairo.
PORTING_GUIDE makes sense in -devel, if it was not so obsolete these days. Some packages ship TODO, other don't.
(In reply to comment #2) > This is intentional and not error. We are not providing libpixman in cairo. > Hiding it. Anybody depending on libpixman is doomed. But the only user was > cairo. So that's not a problem in reality. What is the intended user experience here? * What should happen if a user has libpixman and old cairo installed and then tries to update his box to a repository that contains the present version of cairo? * What should happen if he only has libpixman and tries to install cairo from the same repository? * What should happen if a user has a new cairo and then tries to install a libpixman RPM (using a .rpm file he found somewhere that was copied from rawhide once)? Having answers for these three cases will help finding the best settings for the Obsoletes lines.
I don't see libpixman in any of FC2, FC3, FC4, FC5, FC6, thus we should be able to simply drop this obsolete Obsoletes: - Toshio said that we should not worry about going back further than 2 Fedora releases.
> Toshio said that we should not worry > about going back further than 2 Fedora releases. Generally yes. But to be on the very safe side, we may wish to go as far back as FC3 and FC4 (since they were dropped pre-maturely and may have some people who have not switched yet), but not further. No difference to libpixman of course.
libpixman was just available in rawhide before FC5, when cairo had not reached 1.0.
(In reply to comment #8) > libpixman was just available in rawhide before FC5, when cairo had not reached 1.0. OK. If you want to keep the Obsoletes, please provide answers to comment #5 so we can make sure the best is specified in the spec. If you want to remove them, feel free.
now you are being a bit too picky. I'm going to remove the Obsoletes, but here are the answers anyway: (In reply to comment #5) > What is the intended user experience here? > * What should happen if a user has libpixman and old cairo installed and then > tries to update his box to a repository that contains the present version of > cairo? libpixman removed, as is apparently what happens. > * What should happen if he only has libpixman and tries to install cairo from > the same repository? "Don't care". Removed. > * What should happen if a user has a new cairo and then tries to install a > libpixman RPM (using a .rpm file he found somewhere that was copied from > rawhide once)? "Don't care". Conflict. > Having answers for these three cases will help finding the best settings for the > Obsoletes lines. I only can think of two settings: keep them or remove them. Removing defeats the purpose of what it was there for: to clean up an unneeded package. Keeping them does just that. There's nothing special here as the only package that evern depended on libpixman was cairo. But as I said, I'll remove them anyway. (the situation is similar to pango obsoleting pango-gtkbeta and fribidi-gtkbeta).
FWIW, I've removed the Obsoletes: glib-gtkbeta from glib2.
Behdad, did you want to make any changes here ?
Just wanted to remove the Obsoletes. I'll go over my merge review bugs next week.
Mostly fixed. Remaining issues are: $ rpmlint cairo-devel-1.3.12-1.i386.rpm W: cairo-devel no-documentation (The HTML files provided in the package are definitely documentation and should be marked as such.) MUST: owning directories The -devel subpackage installs files in /usr/share/gtk-doc/html without owning the directory or depending on anything that owns it. MUST: packaging guidelines * make should use %{?_smp_mflags} flag. None of the few packages I looked at (pango, gtk+) does any of them, so I'm not sure about to do them in the right way. For one thing, we pass --disable-gtk-doc to configure, so requiring gtk-doc doesn't make much sense.
Stalled review ?
FWIW, I have started to require gtk-doc in -devel packages now, to get over the directory ownership annoyance.
How should we proceed. rpmlint is happy about cairo. Someone can send a patch for the minor issues left?
Mass reassigning all merge reviews to their component. For more details, see this FESCO ticket: https://fedorahosted.org/fesco/ticket/1269 If you don't know what merge reviews are about, please see: https://fedoraproject.org/wiki/Merge_Reviews How to handle this bug is left to the discretion of the package maintainer.
This bug appears to have been reported against 'rawhide' during the Fedora 23 development cycle. Changing version to '23'. (As we did not run this process for some time, it could affect also pre-Fedora 23 development cycle bugs. We are very sorry. It will help us with cleanup during Fedora 23 End Of Life. Thank you.) More information and reason for this action is here: https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora23
Closing the ticket, I don't think there's anything left to do here.