[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 #15 from Nils Philippsen <nphilipp at redhat.com>  2009-05-29 08:41:24 EDT ---
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:

(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?

> **
> 
> rpmlint output:
> xsane.x86_64: W: file-not-utf8 /usr/share/doc/xsane-0.996/xsane.CHANGES
> xsane.x86_64: W: file-not-utf8 /usr/share/doc/xsane-0.996/xsane.PROBLEMS
> xsane.x86_64: W: file-not-utf8 /usr/share/doc/xsane-0.996/xsane.INSTALL
> xsane-gimp.x86_64: W: no-documentation
> 4 packages and 0 specfiles checked; 0 errors, 4 warnings.
> 
> - You need to convert these to UTF-8 in the setup phase:
> 
> for doc in xsane.{CHANGES,PROBLEMS,INSTALL}; do
>  iconv -f ISO-8859-1 -t utf8 $doc -o $doc.new && \
>  touch -r $doc $doc.new && \
>  mv $doc.new $doc
> done

applied

> - You don't need BR: sed, it is in the standard buildroot.

removed

> - 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.

> 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)

> 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")

> 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).

> MUST: Clean section exists. OK
> MUST: Large documentation files must go in a -doc subpackage. N/A
> 
> MUST: All relevant items are included in %doc. Items in %doc do not affect
> runtime of application. NEEDSWORK
> - Remove unneeded files from doc (rm in setup or install phase): xsane.RPM,
> xsane.conf, xsane.spec*, xsane.INSTALL, xsane.REMOVE

I've changed the file list to only include the other documentation files.

I've commited my changes to Rawhide CVS.

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