Bug 372161 - Review Request: gnome-themes-extras - extra themes for gnome
Review Request: gnome-themes-extras - extra themes for gnome
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Alex Lancaster
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 412081
  Show dependency treegraph
 
Reported: 2007-11-09 00:21 EST by Marc Wiriadisastra
Modified: 2008-01-14 07:02 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-01-14 07:02:51 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
alexl: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
GNOME Desktop 496096 None None None Never

  None (edit)
Description Marc Wiriadisastra 2007-11-09 00:21:26 EST
Spec URL: http://www.mwiriadi.id.au/fedora-spec/gnome-themes-extras/gnome-themes-extras.spec
SRPM URL: http://www.mwiriadi.id.au/fedora-spec/gnome-themes-extras/gnome-themes-extras-2.20-1.fc8.src.rpm

Description: The Gnome themes extras package is a collection of metathemes for the Gnome desktop environment. This package requires that you use a Gnome 2.2 release or newer to work properly. The design goal of this package is to give Gnome users
an extra set of themes that are not only functional, but also eye catching.
Comment 1 Alex Lancaster 2007-11-12 04:23:23 EST
Full review to come, but this is a blocker, according to configure.ac need at
least 2.11.7 of gtk-engines-2 which means that BuildRequires needs to be upped to:
 gtk2-engines >= 2.11.7

This will also mean that it must be build on F-8 and above (F-7 only has
gtk2-engines-2.10.2).
Comment 2 Alex Lancaster 2007-11-12 05:02:06 EST
Full review follows: the first section are all MUST fix items:

       - MUST: rpmlint output:
gnome-themes-extras.noarch: W: invalid-license LGPL
gnome-themes-extras.noarch: W: obsolete-not-provided gnome-themes-extras-0.9.0
      - MUST: remove Obsoletes: gnome-themes-themes, don't need to obsolete
packages that have the same name, yum does that by default
      - MUST: don't use %makeinstall, see:
http://fedoraproject.org/wiki/Packaging/Guidelines?highlight=%28%25makeinstall%29
use make install DESTDIR=%{buildroot} instead
      - MUST: License information in the tarball is not clear because README,
COPYING are empty and those that mention specific licenses, e.g Gion and Neu are
GPL not LGPL, please clarify with upstream and read
http://fedoraproject.org/wiki/Licensing if the package contains themes with
different licenses they must each be listed in the license tag, e.g. "License:
GPLv2+ and LGPLv2+" (see also
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines)
      - MUST: As above, License field is currently NOT valid, LGPL is not valid
see: http://fedoraproject.org/wiki/Licensing
      - MUST: COPYING is empty: file bug with upstream asking to include it in
package
      - MUST: BuildRequires: should increase gtk2-engines >= 2.11.7

The following is a checklist of all the OK items:

      - MUST: Meets Package Naming Guidelines.: OK
      - MUST: The spec file name: OK
      - MUST: Meets the Packaging Guidelines: OK
      - MUST: Spec file in American English: OK
      - MUST: The spec file legible: OK
      - MUST: MD5sum matches:
36698ee94c2281d5beab9b0a681f3350  gnome-themes-extras-2.20.tar.gz
http://ftp.gnome.org/pub/GNOME/sources/gnome-themes-extras/2.20/gnome-themes-extras-2.20.md5sum
36698ee94c2281d5beab9b0a681f3350  gnome-themes-extras-2.20.tar.gz
      - MUST: Builds in binary RPM on F-8 i386: OK
      - MUST: uses %find_lang, although not yet needed: OK
      - MUST: ldconfig: not required (noarch): OK
      - MUST: not relocatable: OK
      - MUST: Own all directories that it creates: OK
      - MUST:No duplicate files: OK
      - MUST: %defattr(...): OK
      - MUST: Each package must have a %clean section: OK
      - MUST: Consistently use macros: OK
      - MUST: Contains mainly permissable content, themes: OK
      - MUST: No large doc: OK
      - MUST: %doc not critical for run-time: OK
      - MUST: no devel package: OK
      - MUST: no static libraries: OK
      - MUST: No pkgconfig(.pc) files: OK
      - MUST: no library files with suffix: OK
      - MUST: no .la libtool archives: OK
      - MUST: not a GUI app: OK
      - MUST: Does not own files or directories already owned by other packages: OK
      - MUST: Runs rm -rf %{buildroot}: OK
      - MUST: Filenames valid UTF-8: OK
Comment 3 Alex Lancaster 2007-11-12 05:07:09 EST
Hmm, looks like this was orphaned, the old branches are in CVS:

http://cvs.fedoraproject.org/viewcvs/rpms/gnome-themes-extras/

but haven't been marked as dead.package.  Either way, we'll need to re-review
it, so this is good.
Comment 4 Alex Lancaster 2007-11-12 05:11:53 EST
The orphaning happened back at FC-6 by the looks of things.  Also: do you have a
sponsor?  I cannot act as a sponsor, although I can act as a reviewer, I believe.
Comment 5 Marc Wiriadisastra 2007-11-12 05:22:46 EST
I have fixed all the issues and will upload a new version when I get feedback
from upstream about the licensing and hopefully an explanation relating to the
COPYING file.
Comment 6 Marc Wiriadisastra 2007-11-12 05:25:16 EST
No I do not have a sponsor as of yet.  Another package I have submitted is
getting review by Kevin Fenzi.

Comment 7 Marc Wiriadisastra 2007-11-12 05:53:46 EST
http://bugzilla.gnome.org/show_bug.cgi?id=496096 as a reference I had to apply
it to a specific theme.

With that in mind and the process of people separating themes out should the
packages be done accordingly as well separated out and licenses applied
individually?
Comment 8 Alex Lancaster 2007-11-12 06:03:39 EST
(In reply to comment #7)
> http://bugzilla.gnome.org/show_bug.cgi?id=496096 as a reference I had to apply
> it to a specific theme.
> 
> With that in mind and the process of people separating themes out should the
> packages be done accordingly as well separated out and licenses applied
> individually?

Yes, perhaps they could be split out as subpackages with a separate License: tag
for each one, that might work.  We already have some theme packages like that,
e.g. gnome-theme-clearlooks-bigpack-0.6-6.fc7
Comment 9 Alex Lancaster 2007-11-12 06:05:46 EST
(In reply to comment #8)

> Yes, perhaps they could be split out as subpackages with a separate License: tag
> for each one, that might work.  We already have some theme packages like that,
> e.g. gnome-theme-clearlooks-bigpack-0.6-6.fc7

Although that package is actually a set of themes, but you get the idea. 

Comment 10 Marc Wiriadisastra 2007-11-12 07:32:36 EST
Updated the package less the license with the spec file and splitting the source
into separate packages.

I'm assuming the docs are going to be the same as what they are now but
adjustments will be made if need to.

http://www.mwiriadi.id.au/fedora-spec/gnome-themes-extras
Comment 11 Alex Lancaster 2007-11-14 02:20:26 EST
(In reply to comment #10)
> Updated the package less the license with the spec file and splitting the source
> into separate packages.

- The License tag in the main package "LGPL" is still wrong and each subpackage
should have it's own License tag if they are different, I think.

- Each of the subpackages should probably not have the same documentation files,
either, that should be included in the main package

- The main package doesn't currently have it's own %files section, which would
make it an empty package.

I'm actually beginning to think to sticking to a single monolithic package, and
including multiple tags in License to cover each one.
Comment 12 Marc Wiriadisastra 2007-11-14 02:32:40 EST
I should have clarified I'm still waiting for feedback from gnome so I have left
the files as is. I won't put the same docs in the same section when I have the
different docs.

While the license tag is incorrect I can't change it till gnome gets back to me.
 The package I have listed is just waiting for there alterations.
Comment 13 Mamoru TASAKA 2008-01-02 20:42:11 EST
(Removing NEEDSPONSOR: bug 426733)
Comment 14 Alex Lancaster 2008-01-03 18:38:17 EST
(In reply to comment #12)
> I should have clarified I'm still waiting for feedback from gnome so I have left
> the files as is. I won't put the same docs in the same section when I have the
> different docs.
> 
> While the license tag is incorrect I can't change it till gnome gets back to me.
>  The package I have listed is just waiting for there alterations.

Ping?  Is it possible make updates to this package now?  I'd like to finish up
the review so you can get this into Fedora.
Comment 15 Marc Wiriadisastra 2008-01-03 18:50:04 EST
I have pinged upstream 2-3 times with the same bug listed above.  They have
advised be that they will be fixing it however since the first bug request they
have not fixed it.

So I'm not to sure what else I can do to request upstream to include the
licensing information since they are not being to helpful at the moment.
Comment 16 Alex Lancaster 2008-01-03 18:57:45 EST
(In reply to comment #15)

> So I'm not to sure what else I can do to request upstream to include the
> licensing information since they are not being to helpful at the moment.

Perhaps just package the theme(s) that do have licenses for the moment (and
strip out the packages that don't from the tarball) and you can add them back as
upstream confirms licenses.
Comment 17 Marc Wiriadisastra 2008-01-03 19:19:54 EST
Only COPYING files I can find is Gion and Neu so is it ok to remove it after
building or do I need to strip it out of the source? If I have to strip it out
of Source that would mean I would possibly need to patch the configure script?
Comment 18 Marc Wiriadisastra 2008-01-04 20:01:57 EST
Major issues with this package.

All the patches in http://mwiriadi.fedorapeople.org/packages/gnome-themes-extras/

This doesn't build.

make[2]: Entering directory
`/home/marc/rpmbuild/BUILD/gnome-themes-extras-2.20/icon-themes'
make[2]: *** No rule to make target `all'.  Stop.
make[2]: Leaving directory
`/home/marc/rpmbuild/BUILD/gnome-themes-extras-2.20/icon-themes'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/marc/rpmbuild/BUILD/gnome-themes-extras-2.20'
make: *** [all] Error 2
error: Bad exit status from /var/tmp/rpm-tmp.18227 (%build)

I didn't adjust any of the makefiles apart from the patches.  I did a visual
comparison of the two sections for make all and there is no difference apart
from what is patched.

Any suggestions?
Comment 19 Marc Wiriadisastra 2008-01-07 07:33:05 EST
Here is the SRPM without the patches, as discussed sadly it doesn't build with
the patches.

http://mwiriadi.fedorapeople.org/packages/gnome-themes-extras/gnome-themes-extras-2.20-2.fc8.src.rpm

Comment 20 Alex Lancaster 2008-01-07 22:28:01 EST
Here's the upstream license clarification request bug:

http://bugzilla.gnome.org/show_bug.cgi?id=496096

It looks like we are go tentatively for GPLv2, but I'd like to like to check
whether they mean GPLv2+ (i.e. or later version), which is the most likely. 
Judging from some of the comments it seems that upstream doesn't really
understand the meaning of GPL versions very well.
Comment 22 Alex Lancaster 2008-01-09 07:16:45 EST
MUST FIX items:

1) OK, I think that we should go back to the monolithic package for the moment,
to avoid duplication of docs and to make sure upgrades from the old
gnome-themes-extras goes smoothly.

The good news is that I did a koji build on rawhide that worked:

http://koji.fedoraproject.org/koji/taskinfo?taskID=336678

Also there are some (mostly) small issues with running rpmlint on the packages

$ rpmlint *.rpm

gnome-themes-extras.src: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab:
line 20)
gnome-themes-extras-darklooks.noarch: W: summary-not-capitalized darklooks theme
gnome-themes-extras-darklooks.noarch: E: description-line-too-long Darklooks is
a meta-theme that is part of the gnome-themes-extras package. This package
gnome-themes-extras-foxtrot.noarch: E: description-line-too-long Foxtrot is a
meta-theme that is part of the gnome-themes-extras package. This package
gnome-themes-extras-gion.noarch: E: description-line-too-long Gion is a
meta-theme that is part of the gnome-themes-extras package.  This package
gnome-themes-extras-neu.noarch: W: spelling-error-in-description pacakge package
gnome-themes-extras-neu.noarch: E: description-line-too-long Neu is a meta-theme
that is part of the gnome-themes-extras pacakge. This package

2) Remember, Description lines should be no longer than 80 chars, and that
Summary should start with a capital letter.

3) You are also mixing tabs and spaces in the spec file which makes the spec
file look odd in different editors, for example in Emacs it looks like this:

%package		foxtrot
Requires:		gnome-icon-theme, gnome-themes
Group:		User Interface/Desktops
Summary:		Foxtrot metatheme
%description	foxtrot
Foxtrot is a meta-theme that is part of the gnome-themes-extras package. This
package requires that you use a Gnome 2.2 release or newer.

4) * Mon Jan 7 2008 Marc Wiriadisastra <marc@mwiriadi.id.au> - 2.20-3
- Added patches to remove non-licensed themes

isn't true any more, so should be removed/updated.
Comment 23 Alex Lancaster 2008-01-09 07:18:29 EST
Also put a link to the upstream GNOME bugzilla link in a comment in the spec
file with a note saying upstream clarified the license on the bug and are
(hopefully) working to include it in the next release.  Please keep checking to
see whether they mean GPLv2+ (which they probably do) rather than GPLv2 and
update spec accordingly.
Comment 24 Alex Lancaster 2008-01-09 07:23:54 EST
You watch to see they actually update SVN here:

http://svn.gnome.org/viewvc/gnome-themes-extras/trunk/
Comment 25 Marc Wiriadisastra 2008-01-09 16:45:04 EST
Done all of that I added the link to the gnome bug in the changelog thats the
only visual place that it is.

http://mwiriadi.fedorapeople.org/packages/gnome-themes-extras/gnome-themes-extras-2.20-4.fc8.src.rpm
http://mwiriadi.fedorapeople.org/packages/gnome-themes-extras/gnome-themes-extras.spec
Comment 26 Alex Lancaster 2008-01-10 09:51:25 EST
All looks good, so this package is:

APPROVED
Comment 27 Marc Wiriadisastra 2008-01-10 15:48:53 EST
This package is in Fedora but ownership has been dropped I submitted this as a
new review originally because of the time and version numbers.

Should I just add myself as a maintainer and just do an update?  Do I have to
fill out the cvs form to add myself as a maintainer?
Comment 28 Marc Wiriadisastra 2008-01-10 15:53:10 EST
Package Change Request
======================
Package Name: gnome-themes-extras
Updated Fedora Owners: mwiriadi
New Branches: F-8 devel
Comment 29 Kevin Fenzi 2008-01-11 00:32:47 EST
Looks like it's still owned in devel... 

You can go to: 

https://admin.fedoraproject.org/pkgdb/packages/name/gnome-themes-extras

and take ownership of the F-8 branch and ask for ownership of the devel branch. 
Comment 30 Alex Lancaster 2008-01-11 00:51:21 EST
(In reply to comment #29)
> Looks like it's still owned in devel... 
> 
> You can go to: 
> 
> https://admin.fedoraproject.org/pkgdb/packages/name/gnome-themes-extras
> 
> and take ownership of the F-8 branch and ask for ownership of the devel branch. 

But there isn't an F-8 branch there.
Comment 31 Alex Lancaster 2008-01-11 01:51:33 EST
Matthias: would you be willing to orphan the devel branch so that Marc can
maintain the entire package?
Comment 32 Kevin Fenzi 2008-01-11 13:08:44 EST
F-8 branch added. 
Comment 33 Marc Wiriadisastra 2008-01-11 18:45:24 EST
I have added myself as part of the group members but Matthias automatically has
gotten ownership.
Comment 34 Alex Lancaster 2008-01-12 20:30:37 EST
Just sent Matthias a private e-mail (which I Cc'ed you on) to ask him to release
the package.  Hopefully he'll be more likely to read that than
bugzilla/packagedb spam
Comment 35 Matthias Saou 2008-01-14 05:49:37 EST
I finally took the time to release the devel branch. Sorry about that, I was
really planning on reviving that package... but if someone else can do it *now*,
then that's even better :-)
Comment 36 Marc Wiriadisastra 2008-01-14 05:57:12 EST
It's ready now yes I can upload it now and release it for Fedora 8 and devel
though it's up to you whether you want to do that.

Comment 37 Marc Wiriadisastra 2008-01-14 07:02:51 EST
Thanks heaps for all the help it's been uploaded and release for F-8 stable
since it's a 'new' package.

Marking this as next release.

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