[Bug 226658] Merge Review: xsane

[bugzilla at redhat.com]

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=226658





--- Comment #16 from Jussi Lehtola <jussi.lehtola at iki.fi>  2009-05-29 08:59:17 EDT ---
(In reply to comment #15)
> Thanks for your effort, but I'll not use your patch because it's not very
> readable (better use unified diffs: diff -u) and I want to discuss some things:

Right. You could have generated an unified diff by patching a local copy of the
spec file, though :)

> (In reply to comment #11)
> > - Instead of make clean'ing in between building the different versions, I'd
> > suggest using off-root builds.
> 
> Later you said you had problems with that, what were these?

The automake stuff isn't configured properly. First of all, configure reads
some xsane. files (at least xsane.NEWS), so one needs to copy them to the build
dir. After that things got even messier since it seemed that there were
problems with header files etc as well.

This could of course be overcome rather simply: do a manual setup of the build
tree by extracting the tarball twice into separate subdirs (gimp and nogimp)
where the builds can be made separately.

This of course doubles the size of the -debuginfo package (although I'm not
sure if debugging xsane-gimp is even possible now!).


> > - No need to define desktop_vendor as it is only used in two places and is not
> > even supposed to be changed.
> 
> "Existing packages that use a vendor tag must continue to do so for the life of
> the package." As this is a merge review, i.e. the package isn't new, I'll leave
> it as it is.

Yes, that's exactly my point. As there are only two instances where the macro
is used, I'd replace them both with "fedora".

> > MUST: A package must own all directories that it creates or require the package
> > that owns the directory. NEEDSWORK
> > - Package must not own
> >  %dir %{_datadir}/applications
> > which is a standard system directory.
> > - gimp package must Requires: gimp for dir ownership and not own
> >  %dir %{_sysconfdir}/gimp
> >  %dir %{_sysconfdir}/gimp/plugins.d
> 
> fixed (-gimp subpackage already requires gimp)

Only in %post and %postun phases.

> > MUST: Files only listed once in %files listings. NEEDSWORK
> > - %{_datadir}/sane, %{_datadir}/sane/xsane and
> > %{_datadir}/sane/xsane/xsane-eula.txt are owned by both xsane and xsane-gimp.
> > Is there a good reason for this? If xsane-gimp needs those files, it'd be wiser
> > to let xsane own them and put a Requires: %{name} = %{version}-%{release} to
> > xsane-gimp.
> 
> xsane and xsane-gimp can be installed independently, they both need the same
> xsane-eula.txt file. I believe the rule you're hinting on means files listed
> twice for the same package (e.g. twice in "%files gimp")

http://fedoraproject.org/wiki/Packaging/Guidelines#DuplicateFiles

"A Fedora package must not list a file more than once in the spec file's %files
listings. If you think your package is a valid exception to this, please bring
it to the attention of the Packaging Committee so they can improve on this
Guideline."

Now you have xsane-eula.txt and the directories listed twice. Of course, you
can get around this by putting xsane-eula.txt in %doc of the xsane-gimp
package.


> > MUST: Permissions on files must be set properly. NEEDSWORK
> > - Use %defattr(-,root,root,-) instead of %defattr(-,root,root).
> 
> The latter (which I use) is functionally equivalent to the former (which isn't
> mandatory either).

Hmm, the guideline at
 http://fedoraproject.org/wiki/Packaging/Guidelines#FilePermissions
states:
"Unless you have a very good reason to deviate from that, you should use
%defattr(-,root,root,-) for all %files sections in your package."
Please, just use the version of the guideline.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.