Bug 608973

Builds fine in mock, spec file looks good, engine seems to work.
rpmlint is complaining a bit about permissions in the debuginfo package.

I'll do a full review soon.

Thank you Julian!

Regards,
Germán.

Now when installing it under GNOME I notice that the choosable themes in the Appearance menu complain about not finding the "Faenza" and "Faenza-Dark" icon themes. (Equinox, -Dark and -Light)

The engine itself works, but this is a bit annoying.

Well...what do you suggest? Contact upstream to change it?

Regards,
German.

I don't know how active upstream is. Looking around on gnome-look.org etc. I didn't even find a Faenza icon theme.

I'd just use sed and replace IconTheme=Faenza-Whatever with IconTheme=gnome in the index.theme files for the themes.

sed -i 's/Faenza-Dark/%{replace_theme}/g' %{buildroot}%{_datadir}/themes/Equinox/index.theme
sed -i 's/Faenza Dark/%{replace_theme}/g' %{buildroot}%{_datadir}/themes/Equinox\ Glass/index.theme
sed -i 's/Faenza/%{replace_theme}/g' %{buildroot}%{_datadir}/themes/Equinox\ Light/index.theme

in %install should do the trick. replace_theme could be any icon theme, gnome or Tango or whatever, just be sure to add the Requires for it.

(In reply to comment #5)
> I don't know how active upstream is. Looking around on gnome-look.org etc. I
> didn't even find a Faenza icon theme.
> 
> I'd just use sed and replace IconTheme=Faenza-Whatever with IconTheme=gnome in
> the index.theme files for the themes.    

Faenza is an icon theme under development by the same creator of the engine. In gnome-look.org he said that Faenza is not ready yet.

(In reply to comment #6)
> sed -i 's/Faenza-Dark/%{replace_theme}/g'
> %{buildroot}%{_datadir}/themes/Equinox/index.theme
> sed -i 's/Faenza Dark/%{replace_theme}/g'
> %{buildroot}%{_datadir}/themes/Equinox\ Glass/index.theme
> sed -i 's/Faenza/%{replace_theme}/g' %{buildroot}%{_datadir}/themes/Equinox\
> Light/index.theme
> 
> in %install should do the trick. replace_theme could be any icon theme, gnome
> or Tango or whatever, just be sure to add the Requires for it.    

Hello Julian:

I'm going to change the icon theme in the spec file as you suggest.

Thanks,
German.

Okay, by the way the BuildRoot tag is not needed anymore so you can just remove it.

Hello Julian:

I've informed upstream about the changes we'll make in the Fedora package with respect to the icon theme. We'll use Gnome icon theme while Faenza is not ready. Because of this, I've added the corresponding Requires in the spec file.

I've also corrected the release number because I've made a mistake before, and I've removed the BuildRoot tag as you suggested.

Please find the updated files here:

Spec URL: http://skytux.fedorapeople.org/packages/gtk-equinox-engine.spec

SRPM URL: http://skytux.fedorapeople.org/packages/gtk-equinox-engine-1.20-2.fc13.i686.rpm
________________________________________________________________________________

$ rpmlint SRPMS/gtk-equinox-engine-1.20-2.fc13.src.rpm 
gtk-equinox-engine.src: W: no-buildroot-tag
1 packages and 0 specfiles checked; 0 errors, 1 warnings.

$ rpmlint RPMS/i686/gtk-equinox-engine-1.20-2.fc13.i686.rpm 
gtk-equinox-engine.i686: W: unstripped-binary-or-object /usr/lib/gtk-2.0/2.10.0/engines/libequinox.so

$ koji build --scratch dist-f13 rpmbuild/SRPMS/gtk-equinox-engine-1.20-2.fc13.src.rpm
Task info: http://koji.fedoraproject.org/koji/taskinfo?taskID=2297296

(In reply to comment #10)

> SRPM URL:
> http://skytux.fedorapeople.org/packages/gtk-equinox-engine-1.20-2.fc13.i686.rpm

Sorry, that url is for the rpm package (that I removed). The source rpm is here:

SRPM URL: http://skytux.fedorapeople.org/packages/gtk-equinox-engine-1.20-2.fc13.src.rpm

MUST: The package does not yet exist in Fedora. The Review Request is not a
duplicate. OK
MUST: The spec file for the package is legible and macros are used
consistently. OK
MUST: The package must be named according to the Package Naming Guidelines. OK
MUST: The spec file name must match the base package %{name}. OK
MUST: The package must be licensed with a Fedora approved license and meet the 
Licensing Guidelines. OK
MUST: The License field in the package spec file must match the actual license.
OK
MUST: The sources used to build the package must match the upstream source, as
provided in the spec URL. OK
MUST: The package MUST successfully compile and build into binary rpms. OK
MUST: The spec file MUST handle locales properly. N/A
MUST: Optflags are used and time stamps preserved. OK
MUST: Packages containing shared library files must call ldconfig. N/A
MUST: A package must own all directories that it creates or require the package
that owns the directory. OK
MUST: Files only listed once in %files listings. OK
MUST: Debuginfo package is complete. OK
MUST: Permissions on files must be set properly. OK
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. OK
MUST: Header files must be in a -devel package. N/A
MUST: Static libraries must be in a -static package. N/A
MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A
MUST: If a package contains library files with a suffix then library files
ending in .so must go in a -devel package. N/A
MUST: In the vast majority of cases, devel packages must require the base
package using a fully versioned dependency. N/A
MUST: Packages does not contain any .la libtool archives. OK
MUST: Desktop files are installed properly. OK
MUST: No file conflicts with other packages and no general names. OK
MUST: Buildroot cleaned before install. OK
SHOULD: %{?dist} tag is used in release. OK
SHOULD: If the package does not include license text(s) as separate files from
upstream, the packager should query upstream to include it. N/A
SHOULD: The package builds in mock.
- builds fine for fedora-13-i386, fedora-rawhide-i386 and fedora-12-i386


NOTE: All files in equinox-1.20/src/ are marked executable. rpmlint doesn't like that when checking the debuginfo package. A chmod 644 on these files in %prep should fix it though.

Hi Julian:

Many thanks for the full review. Executable bits are fixed now, you told me abut that the first time, but I forgot to correct it...sorry :-(

Please find updated files here:

Spec URL: http://skytux.fedorapeople.org/packages/gtk-equinox-engine.spec

SRPM URL: http://skytux.fedorapeople.org/packages/gtk-equinox-engine-1.20-3.fc13.src.rpm

Cheers,
Germán.

(In reply to comment #13)
> Many thanks for the full review. Executable bits are fixed now, you told me
> abut that the first time, but I forgot to correct it...sorry :-(

No problem at all :)

APPROVED

Thanks for the review Julian!

Germán.

New Package CVS Request
=======================
Package Name: gtk-equinox-engine
Short Description: Equinox theme engine for GTK+ 2.x
Owners: skytux
Branches: F-12 F-13
InitialCC:

CVS done (by process-cvs-requests.py).

gtk-equinox-engine-1.20-3.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/gtk-equinox-engine-1.20-3.fc12

gtk-equinox-engine-1.20-3.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/gtk-equinox-engine-1.20-3.fc13

gtk-equinox-engine-1.20-3.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.

gtk-equinox-engine-1.20-3.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.

Package Change Request
======================
Package Name: gtk-equinox-engine
New Branches: el6
Owners: skytux

Many people asked me to maintain this package for EPEL.

Git done (by process-git-requests).