Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 225798 - Merge Review: gimp-help
Summary: Merge Review: gimp-help
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Orcan Ogetbil
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2007-01-31 18:44 UTC by Nobody's working on this, feel free to take it
Modified: 2008-12-13 19:11 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2008-12-13 19:11:01 UTC
oget.fedora: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 18:44:15 UTC
Fedora Merge Review: gimp-help

http://cvs.fedora.redhat.com/viewcvs/devel/gimp-help/
Initial Owner: nphilipp@redhat.com

Comment 1 Orcan Ogetbil 2008-10-30 18:48:57 UTC
I reviewed this package. The following issues need to be addressed for compilance with the guidelines:

* The files AUTHORS, ChangeLog, COPYING, HACKING, NEWS, README, TERMINOLOGY need to be listed under %doc

* The package owns
   %dir %{_datadir}/gimp
   %dir %{_datadir}/gimp/%{gimpsubver}
which are also owned by gimp. Note that the package Requires: gimp. The above ownerships need to be dropped.

* %defattr(-,root,root,-) is preferred. 

* BuildRequires: libxml2
is not required since
   BuildRequires: libxslt
will pull that up

* Similarly
   BuildRequires: docbook-style-xsl
is not required. It will be pulled up by other dependencies. Am I wrong?

Comment 2 Orcan Ogetbil 2008-12-10 19:05:55 UTC
ping?

Comment 3 Nils Philippsen 2008-12-11 13:29:35 UTC
Sorry for the late reply.

I've fixed the following in CVS, gimp-2.4.2-2 is building with these changes for Rawhide right now.

(In reply to comment #1)
> I reviewed this package. The following issues need to be addressed for
> compilance with the guidelines:
> 
> * The files AUTHORS, ChangeLog, COPYING, HACKING, NEWS, README, TERMINOLOGY
> need to be listed under %doc

These files get shipped except HACKING which is documentation about how to change the documentation for which you need the source anyway.

> * The package owns
>    %dir %{_datadir}/gimp
>    %dir %{_datadir}/gimp/%{gimpsubver}
> which are also owned by gimp. Note that the package Requires: gimp. The above
> ownerships need to be dropped.

Not owned anymore.

> * %defattr(-,root,root,-) is preferred. 

Fixed.

> * BuildRequires: libxml2
> is not required since
>    BuildRequires: libxslt
> will pull that up
> 
> * Similarly
>    BuildRequires: docbook-style-xsl
> is not required. It will be pulled up by other dependencies. Am I wrong?

The configure script of gimp-help checks directly for e.g. xmllint (included in libxml2) and .../xhtml/docbook.xsl which is included in docbook-style-xsl. I.e. both packages are required directly, therefore I keep listing them as build requirements. It doesn't matter that they in turn are required by other build requirements, I don't want to rely on that this fact doesn't change in the future.

Comment 4 Orcan Ogetbil 2008-12-11 20:17:18 UTC
Thanks for the update. There are two more little things:

* From the SPEC file:
    cat << EOF > files.list
    %%defattr (-, root, root, -)
    %%dir %{_datadir}/gimp/%{gimpsubver}/help
    %{_datadir}/gimp/%{gimpsubver}/help/images
    EOF
and
    echo "%%lang($dir) %{_datadir}/gimp/%{gimpsubver}/help/$dir" >> "$f"

The macro entries with single % expand into the files.list . This is equivalent to putting hard-coded paths into %files . I think those macro entries need double-% too.

* Parallel make must be supported whenever possible (If it is not supported, this should be noted in the SPEC file as a comment.). Sorry that I missed this in my initial review. Enabling the parallel make didn't cause a problem with my dual core. It certainly speeds things up.

Comment 5 Nils Philippsen 2008-12-12 10:53:46 UTC
(In reply to comment #4)
> Thanks for the update. There are two more little things:
> 
> * From the SPEC file:
>     cat << EOF > files.list
>     %%defattr (-, root, root, -)
>     %%dir %{_datadir}/gimp/%{gimpsubver}/help
>     %{_datadir}/gimp/%{gimpsubver}/help/images
>     EOF
> and
>     echo "%%lang($dir) %{_datadir}/gimp/%{gimpsubver}/help/$dir" >> "$f"
> 
> The macro entries with single % expand into the files.list . This is equivalent
> to putting hard-coded paths into %files . I think those macro entries need
> double-% too.

Technically they needn't be, as it doesn't really matter if they're expanded while files.list is built or while it is evaluated by rpmbuild. I've changed it to be more consistent, though.

> * Parallel make must be supported whenever possible (If it is not supported,
> this should be noted in the SPEC file as a comment.). Sorry that I missed this
> in my initial review. Enabling the parallel make didn't cause a problem with my
> dual core. It certainly speeds things up.

I've changed that and started building gimp-2.4.2-3.fc11 with these changes.

Comment 6 Orcan Ogetbil 2008-12-12 16:31:39 UTC
(In reply to comment #5)
> (In reply to comment #4)
> > The macro entries with single % expand into the files.list . This is equivalent
> > to putting hard-coded paths into %files . I think those macro entries need
> > double-% too.
> 
> Technically they needn't be, as it doesn't really matter if they're expanded
> while files.list is built or while it is evaluated by rpmbuild. I've changed it
> to be more consistent, though.
> 
You're right. I thought more about it. It shouldn't matter.

> > * Parallel make must be supported whenever possible (If it is not supported,
> > this should be noted in the SPEC file as a comment.). Sorry that I missed this
> > in my initial review. Enabling the parallel make didn't cause a problem with my
> > dual core. It certainly speeds things up.
> 
> I've changed that and started building gimp-2.4.2-3.fc11 with these changes.

I checked koji. It looks all good. 

-------------------------------------------
The package "gimp-help" is APPROVED by oget
-------------------------------------------

Comment 7 Orcan Ogetbil 2008-12-13 19:11:01 UTC
Package is in rawhide. We can close the bug now.


Note You need to log in before you can comment on or make changes to this bug.