Bug 825517 - Review Request: egtk - The eGTK (elementary GTK) themes for GTK+2, GTK+3, Metacity and Xfwm4
Summary: Review Request: egtk - The eGTK (elementary GTK) themes for GTK+2, GTK+3, Met...
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Mario Blättermann
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-05-27 09:34 UTC by Mattia M.
Modified: 2012-07-05 23:41 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-07-05 23:29:57 UTC
Type: ---
Embargoed:
mario.blaettermann: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Mattia M. 2012-05-27 09:34:41 UTC
Spec URL: http://odysseus.fedorapeople.org/packages/eGTK/2012.05.27/egtk.spec
SRPM URL: http://odysseus.fedorapeople.org/packages/eGTK/2012.05.27/egtk-3.1-1.fc17.src.rpm
Description: The official elementary GTK theme designed to be smooth, attractive, fast, and usable.
Fedora Account System Username: odysseus

Comment 1 Mattia M. 2012-05-27 09:37:35 UTC
RPM packages for Fedora 17 (for those willing to try the theme):

GTK+2 theme: http://odysseus.fedorapeople.org/packages/eGTK/2012.05.27/RPMs/egtk-gtk2-theme-3.1-1.fc17.noarch.rpm
GTK+3 theme: http://odysseus.fedorapeople.org/packages/eGTK/2012.05.27/RPMs/egtk-gtk3-theme-3.1-1.fc17.noarch.rpm
Metacity theme: http://odysseus.fedorapeople.org/packages/eGTK/2012.05.27/RPMs/egtk-metacity-theme-3.1-1.fc17.noarch.rpm
Xfwm4 theme: http://odysseus.fedorapeople.org/packages/eGTK/2012.05.27/RPMs/egtk-xfwm4-theme-3.1-1.fc17.noarch.rpm


The execution of rpmlint on the Spec, SRPM and RPMs returns the following output:

egtk.src: W: name-repeated-in-summary C eGTK
5 packages and 1 specfiles checked; 0 errors, 1 warnings.

Comment 2 Mario Blättermann 2012-06-15 13:55:18 UTC
All the four subpackages own the folder %{_datadir}/themes/%{short_name}/ and contain the same doc files. Currently, there's no base package named egtk. I recommend to add a package named egtk-common and let it own the aforementioned folder and the doc files.

Moreover, it is not needed to correct a wrong FSF address. Give the developers an appropriate hint and leave the file untouched for the time being.

%setup -q -c -n %{name}-%{version}

You may drop "-n %{name}-%{version}" in %setup. Unless you need a different name for the build folder, you don't need to add this pattern explicitely.

Comment 3 Mattia M. 2012-06-15 14:02:16 UTC
(In reply to comment #2)
> All the four subpackages own the folder %{_datadir}/themes/%{short_name}/
> and contain the same doc files. Currently, there's no base package named
> egtk. I recommend to add a package named egtk-common and let it own the
> aforementioned folder and the doc files.
 
> %setup -q -c -n %{name}-%{version}
> 
> You may drop "-n %{name}-%{version}" in %setup. Unless you need a different
> name for the build folder, you don't need to add this pattern explicitely.

OK, I'll follow your suggestions.

> Moreover, it is not needed to correct a wrong FSF address. Give the
> developers an appropriate hint and leave the file untouched for the time
> being.

I've already filed a bug report to the eGTK developers about the FSF address issue: https://bugs.launchpad.net/egtk/+bug/1005780
I'll remove the line  that corrects it from the Spec file.

Thank you.

Comment 4 Mattia M. 2012-06-17 06:38:34 UTC
Spec URL: http://odysseus.fedorapeople.org/packages/eGTK/2012.06.17/egtk.spec

SRPM URL: http://odysseus.fedorapeople.org/packages/eGTK/2012.06.17/egtk-3.1-2.fc17.src.rpm

RPM packages for Fedora 17: http://odysseus.fedorapeople.org/packages/eGTK/2012.06.17/RPMs/

Output of rpmlint on the Spec file, SRPM and RPMs:

egtk-common.noarch: E: incorrect-fsf-address /usr/share/doc/egtk-common-3.1/COPYING
egtk-gtk2-theme.noarch: W: no-documentation
egtk-gtk3-theme.noarch: W: no-documentation
egtk-metacity-theme.noarch: W: no-documentation
egtk-xfwm4-theme.noarch: W: no-documentation
egtk.src: W: name-repeated-in-summary C eGTK
6 packages and 1 specfiles checked; 1 errors, 5 warnings.

Comment 5 Mario Blättermann 2012-06-22 18:29:57 UTC
Your packages have to depend on the base package:

Requires:       %{name} = %{version}-%{release}

See http://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

Comment 6 Mattia M. 2012-06-23 06:44:03 UTC
(In reply to comment #5)
> Your packages have to depend on the base package:
> 
> Requires:       %{name} = %{version}-%{release}
> 
> See http://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

So, what I should do is adding that "Requires" line to all of the "%package" sections (including that related to the "common" subpackage)?

But then, when I try to install the packages built from the so-edited Spec file, Yum complains about them requiring "egtk = 3.1-3.fc17" and doesn't allow me to install.

Isn't "Requires: egtk-common" in all "%package" sections (except "common") enough?
For example, have a look at the Spec file of this package: http://kojipkgs.fedoraproject.org//packages/gnome-shell-extensions/3.4.0/1.fc17/src/gnome-shell-extensions-3.4.0-1.fc17.src.rpm

Comment 7 Mario Blättermann 2012-06-23 17:27:42 UTC
(In reply to comment #6)
> But then, when I try to install the packages built from the so-edited Spec
> file, Yum complains about them requiring "egtk = 3.1-3.fc17" and doesn't
> allow me to install.
> 
> Isn't "Requires: egtk-common" in all "%package" sections (except "common")
> enough?
No, it is usually not enough. In the current state, Yum could use any version of an egtk-common package as a dependency. The mentioned line was an example, directly taken from the guidelines. In our case, you have to change it as follows:

Requires:       %{name}-common = %{version}-%{release}

Imagine, the developers decide to stop with the xfce part, then the egtk-xfce subpackage remains in an older version and could become buggy. The explicite version number in the "Requires" line avoids this.

Comment 8 Mattia M. 2012-06-24 16:57:20 UTC
(In reply to comment #7)

> No, it is usually not enough. In the current state, Yum could use any
> version of an egtk-common package as a dependency. The mentioned line was an
> example, directly taken from the guidelines. In our case, you have to change
> it as follows:
> 
> Requires:       %{name}-common = %{version}-%{release}
> 
> Imagine, the developers decide to stop with the xfce part, then the
> egtk-xfce subpackage remains in an older version and could become buggy. The
> explicite version number in the "Requires" line avoids this.

OK, this makes everything clear. Thank you.
So, here's the revised version:

Spec URL: http://odysseus.fedorapeople.org/packages/eGTK/2012.06.24/egtk.spec

SRPM URL: http://odysseus.fedorapeople.org/packages/eGTK/2012.06.24/egtk-3.1-3.fc17.src.rpm

RPMs for Fedora 17: http://odysseus.fedorapeople.org/packages/eGTK/2012.06.24/RPMs/

Output of rpmlint is the same as in comment #4.

Comment 9 Mario Blättermann 2012-06-24 17:48:48 UTC
Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=4191661

$ rpmlint -i -v *
egtk.src: I: checking
egtk.src: I: enchant-dictionary-not-found en_US
A dictionary for the Enchant spell checking library is not available for the
language given in the info message.  Spell checking will proceed with
rpmlint's built-in implementation for localized tags in this language. For
better spell checking results in this language, install the appropriate
dictionary that Enchant will use for this language, often for example
hunspell-* or aspell-*.

egtk.src: W: name-repeated-in-summary C eGTK
The name of the package is repeated in its summary.  This is often redundant
information and looks silly in various programs' output.  Make the summary
brief and to the point without including redundant information in it.

egtk.src: I: checking-url https://launchpad.net/egtk (timeout 10 seconds)
egtk.src: I: checking-url https://launchpad.net/egtk/3.x/3.1/+download/elementary.tar.gz (timeout 10 seconds)
egtk-common.noarch: I: checking
egtk-common.noarch: I: checking-url https://launchpad.net/egtk (timeout 10 seconds)
egtk-common.noarch: E: incorrect-fsf-address /usr/share/doc/egtk-common-3.1/COPYING
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

egtk-gtk2-theme.noarch: I: checking
egtk-gtk2-theme.noarch: I: checking-url https://launchpad.net/egtk (timeout 10 seconds)
egtk-gtk2-theme.noarch: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

egtk-gtk3-theme.noarch: I: checking
egtk-gtk3-theme.noarch: I: checking-url https://launchpad.net/egtk (timeout 10 seconds)
egtk-gtk3-theme.noarch: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

egtk-metacity-theme.noarch: I: checking
egtk-metacity-theme.noarch: I: checking-url https://launchpad.net/egtk (timeout 10 seconds)
egtk-metacity-theme.noarch: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

egtk.spec: I: checking-url https://launchpad.net/egtk/3.x/3.1/+download/elementary.tar.gz (timeout 10 seconds)
egtk-xfwm4-theme.noarch: I: checking
egtk-xfwm4-theme.noarch: I: checking-url https://launchpad.net/egtk (timeout 10 seconds)
egtk-xfwm4-theme.noarch: W: no-documentation
The package contains no documentation (README, doc, etc). You have to include
documentation files.

6 packages and 1 specfiles checked; 1 errors, 5 warnings.


All is OK so far. The warnings could be ignored, you don't have to write docs if upstream doesn't ship any.


---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
    GPLv2
[+] MUST: The License field in the package spec file must match the actual
license.
[+] MUST: The file containing the text of the license(s) for the package must
be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum *
    bc28c1734f7210ebdd927808f2e7397e  elementary.tar.gz
    bc28c1734f7210ebdd927808f2e7397e  elementary.tar.gz.packaged

[+] MUST: The package MUST successfully compile and build into binary rpms on
at least one primary architecture.
- See Koji build above.
[.] MUST: If the package does not successfully compile, build or work on an
architecture, ...
[.] MUST: All build dependencies must be listed in BuildRequires.
[+] MUST: The spec file MUST handle locales properly.
[+] MUST: If a package installs files below %{_datadir}/icons, the icon cache
must be updated.
[.] MUST: Packages storing shared library files (not just symlinks) must call
ldconfig in %post and %postun.
[.] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Packages must not provide RPM dependency information when that
information is not global in nature, or are otherwise handled.
[.] MUST: When filtering automatically generated RPM dependency information,
the filtering system implemented by Fedora must be used.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: If a package contains library files with a suffix (e.g.
libfoo.so.1.1), ...
[.] MUST: devel packages must require the base package using a fully versioned
dependency.
[.] MUST: Packages must NOT contain any .la libtool archives.
[.] MUST: Packages containing GUI applications must include a %{name}.desktop
file
[.] MUST: .desktop files must be properly installed with desktop-file-install
in the %install section.
[+] MUST: Packages must not own files or directories already owned by other
packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

[.] SHOULD: If the source package does not include license text(s) as a
    separate file from upstream, the packager SHOULD query upstream...

[+] SHOULD: Timestamps of files should be preserved.
[+] SHOULD: The reviewer should test that the package builds in mock.
    See Koji build above (which uses mock anyway)
[+] SHOULD: The reviewer should test that the package functions as described.
    Works properly on my machine (metacity part not tested)
[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base
package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
[.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin,
/usr/bin, or /usr/sbin ...
[.] SHOULD: Your package should contain man pages for binaries/scripts.
    Currently no man page available.

--------------------------------------

PACKAGE APPROVED

--------------------------------------

Comment 10 Mattia M. 2012-06-25 07:17:04 UTC
(In reply to comment #9)
> $ rpmlint -i -v *
> egtk-common.noarch: E: incorrect-fsf-address
> /usr/share/doc/egtk-common-3.1/COPYING
> The Free Software Foundation address in this file seems to be outdated or
> misspelled.  Ask upstream to update the address, or if this is a license
> file,
> possibly the entire file with a new copy available from the FSF.
> 
> All is OK so far. The warnings could be ignored, you don't have to write
> docs if upstream doesn't ship any.

And what about the error caused by the incorrect FSF address?
Can you confirm there's no need to correct it and all I have to do is notifying upstream about?

> PACKAGE APPROVED

Thank you!

Comment 11 Mattia M. 2012-06-25 07:24:28 UTC
New Package SCM Request
=======================
Package Name: egtk
Short Description: The eGTK (elementary GTK) themes for GTK+2, GTK+3, Metacity and Xfwm4
Owners: odysseus
Branches: f16 f17
InitialCC:

Comment 12 Mario Blättermann 2012-06-25 17:54:44 UTC
(In reply to comment #10)
> And what about the error caused by the incorrect FSF address?
> Can you confirm there's no need to correct it and all I have to do is
> notifying upstream about?

https://bugzilla.redhat.com/show_bug.cgi?id=690919#c5

This is what "spot" said. I'm unsure whether we should touch license declarations in general. If upstream isn't dead yet, you shouldn't change anything, and wait until the address has been corrected in future releases. Keep in mind, we don't speak about the license itself, but about an address where users can get a printed copy of the license text from, if in doubt. But who would do this really...?

Comment 13 Gwyn Ciesla 2012-06-26 14:47:46 UTC
Git done (by process-git-requests).

Comment 14 Fedora Update System 2012-06-26 20:02:54 UTC
egtk-3.1-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/egtk-3.1-1.fc16

Comment 15 Fedora Update System 2012-06-26 20:11:45 UTC
egtk-3.1-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/egtk-3.1-1.fc17

Comment 16 Fedora Update System 2012-06-28 03:43:19 UTC
egtk-3.1-1.fc16 has been pushed to the Fedora 16 testing repository.

Comment 17 Fedora Update System 2012-07-05 23:29:57 UTC
egtk-3.1-1.fc17 has been pushed to the Fedora 17 stable repository.

Comment 18 Fedora Update System 2012-07-05 23:41:52 UTC
egtk-3.1-1.fc16 has been pushed to the Fedora 16 stable repository.


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