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 Nils Philippsen <nphilipp@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo? | --- Comment #21 from Nils Philippsen <nphilipp@xxxxxxxxxx> 2009-08-05 08:47:24 EDT --- Sorry for the late response. The xsane-0.997-3.fc12 package is building right now with the changes below. (In reply to comment #16) > (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 :) Yuck ;-). > > (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!). Oh, I'd rather not have that ;-). It seems we need to fix the autofoo stuff in the package to use off-root builds, but I guess this is "out of scope" for this review anyway. > > > - 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". Now I understand you ;-). Fixed. > > > 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. Fixed. > > > 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. No, I can't as xsane-gimp won't work properly without it in that exact location ;-). Anyway, I've moved the eula and documentation to a -common subpackage which gets pulled in by both xsane and xsane-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). > > 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. As I read the guideline I'm already compliant, because the one and the other mean exactly the same thing. There is no tangible benefit from substituting the short-hand version with the long-hand one other than the latter matches verbatim what's written in 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review