Bug 591222 - Review Request: gtk3 - version 3 of GTK+
Summary: Review Request: gtk3 - version 3 of GTK+
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Bastien Nocera
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-05-11 17:15 UTC by Matthias Clasen
Modified: 2010-06-21 14:31 UTC (History)
8 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-06-21 14:31:51 UTC
Type: ---
Embargoed:
bnocera: fedora-review+
dennis: fedora-cvs+


Attachments (Terms of Use)

Description Matthias Clasen 2010-05-11 17:15:12 UTC
srpm: http://mclasen.fedorapeople.org/gtk3-2.90.0-1.fc13.src.rpm
spec: http://mclasen.fedorapeople.org/gtk3.spec

The spec is based on the gtk2 spec, with some cleanups.
I would have built this in mock, but mock is uncooperative:

    pkg = self.searchNevra(n, e, v, r, a)[0]
IndexError: list index out of range

So, I can just say that this builds fine with rpmbuild

Comment 1 Terje Røsten 2010-05-12 07:53:29 UTC
koji was happy:

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

Some pedantic comments:

 o spec seems to follow one req/buildreq per line.
   However, that pattern are not used in %package devel 
   Requires: lines.

 o changelog needs updating
 
 o is these lines needed (in %package devel)

   Provides: gail-devel = %{version}-%{release}
   Obsoletes: gail-devel < 2.13.0-1

 o use %global over %define
 
 o %defattr(-, root, root) -> %defattr(-, root, root, -)

 o I am bit surprised about various hacks, push at least some
   of the changes upstream?

Comment 2 Matthias Clasen 2010-05-12 14:32:00 UTC
> Some pedantic comments:

Yeah, some were too pedantic for me. The others have been fixed in:


srpm: http://mclasen.fedorapeople.org/gtk3-2.90.0-2.fc13.src.rpm
spec: http://mclasen.fedorapeople.org/gtk3.spec

Comment 3 Thomas Spura 2010-05-14 21:45:42 UTC
(In reply to comment #2)
> > Some pedantic comments:
> 
> Yeah, some were too pedantic for me. The others have been fixed in:

It would be nice to know which one, this way it's double work...

(In reply to comment #1)
> Some pedantic comments:
>  o changelog needs updating

From rpmlint:
gtk3.x86_64: W: incoherent-version-in-changelog 2.9.0.0-2 ['2.90.0-2.fc13', '2.90.0-2']

>  o use %global over %define

Done

>  o %defattr(-, root, root) -> %defattr(-, root, root, -)

Done

>  o I am bit surprised about various hacks, push at least some
>    of the changes upstream?    

Indeed. Or is this not (easyly) possible?


Some other comments:
- rpmlint (just the relevant ones):
gtk3.src: W: strange-permission update-gdk-pixbuf-loaders-3.0 0775L
gtk3.src: W: strange-permission update-gtk-immodules-3.0 0775L

* Why not 755?

gtk3.src:125: W: mixed-use-of-spaces-and-tabs (spaces: line 125, tab: line 125)
gtk3.src: W: invalid-url Source0: http://download.gnome.org/sources/gtk+/3.0/gtk+-2.90.0.tar.bz2 HTTP Error 404: Not Found

* Will there be a source online? I currently get only 404s.

gtk3.x86_64: W: incoherent-version-in-changelog 2.9.0.0-2 ['2.90.0-2.fc13', '2.90.0-2']
gtk3-devel-docs.x86_64: W: file-not-utf8 /usr/share/doc/gtk3-devel-docs-2.90.0/examples/calendar/calendar.c


- Could you please use INSTALL="install -p" to preserve timestamps?
  (also when installing the examples)

- It's a bit confusing to use $host and %{_host}, almost impossible to verify as a reviewer...
  Could you explain them a bit?

The rest looks a bit hacky and it would be *nice* to get that parts upstream, but I don't see them as blocker...

Comment 4 Matthias Clasen 2010-05-15 00:38:40 UTC
(In reply to comment #3)
>
> 
> >  o I am bit surprised about various hacks, push at least some
> >    of the changes upstream?    
> 
> Indeed. Or is this not (easyly) possible?

Spell out what you consider 'hacks', maybe ? The multilib support crap is not upstreamable, because multilib is a RHism that I don't want to spread upstream.
 
> 
> Some other comments:
> - rpmlint (just the relevant ones):
> gtk3.src: W: strange-permission update-gdk-pixbuf-loaders-3.0 0775L
> gtk3.src: W: strange-permission update-gtk-immodules-3.0 0775L
> 
> * Why not 755?

No specific reason. I can change it.

> 
> gtk3.src:125: W: mixed-use-of-spaces-and-tabs (spaces: line 125, tab: line 125)
> gtk3.src: W: invalid-url Source0:
> http://download.gnome.org/sources/gtk+/3.0/gtk+-2.90.0.tar.bz2 HTTP Error 404:

I'll fix that.

> - Could you please use INSTALL="install -p" to preserve timestamps?
>   (also when installing the examples)

I still have to find somebody how can show me an actual benefit of timestamp pedantry. (Beyond making reviewers happy...)

> - It's a bit confusing to use $host and %{_host}, almost impossible to verify
> as a reviewer...
>   Could you explain them a bit?

It has plenty of comments in the spec:

# Deriving /etc/gtk-3.0/$host location
...
# Make sure that the host value that is passed to the compile
# is the same as the host that we're using in the spec file

So:
GTK+ is looking up files there at runtime. Some of the files are arch-dependent, thus we must factor $host into the path. The rest of the
'hack' deals with the fact that rpm has its own idea of host triplet, and
we need to make sure that the host triplet that GTK+ uses at runtime (which is the one coming from config.guess and compiled into the library) is the same that we use to when putting the files in their place.

Comment 5 Matthias Clasen 2010-05-15 01:48:44 UTC
I'm pondering how to get rid of the multilib hacks altogether: 
http://mail.gnome.org/archives/gtk-devel-list/2010-May/msg00076.html

Comment 6 Matthias Clasen 2010-05-17 16:08:46 UTC
These fixes the wrapper script file permissions and the source url:

srpm: http://mclasen.fedorapeople.org/gtk3-2.90.0-3.fc13.src.rpm
spec: http://mclasen.fedorapeople.org/gtk3.spec

Comment 7 Bastien Nocera 2010-05-20 15:37:19 UTC
The latest SRPM's build failed:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2199494

Comment 8 Matthias Clasen 2010-05-20 17:21:47 UTC
Grr, thats %check failing. 

New versions:

srpm: http://mclasen.fedorapeople.org/gtk3-2.90.0-4.fc13.src.rpm
spec: http://mclasen.fedorapeople.org/gtk3.spec

Comment 9 Bastien Nocera 2010-05-21 15:37:13 UTC
Builds properly:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2201276

Comment 10 Bastien Nocera 2010-05-21 16:24:32 UTC
210 # Input method frameworks want this•
211 mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/X11/xinit/xinput.d•
212 cp %{SOURCE3} $RPM_BUILD_ROOT%{_sysconfdir}/X11/xinit/xinput.d•

This is already install in gtk2.

Comment 11 Bastien Nocera 2010-05-21 17:04:42 UTC
$ rpmlint /home/hadess/Projects/packages/SRPMS/gtk3-2.90.0-4.fc13.src.rpm
gtk3.src: W: spelling-error %description -l en_US multi -> mulch, mufti
gtk3.src: W: strange-permission update-gdk-pixbuf-loaders-3.0 0755L
gtk3.src: W: strange-permission update-gtk-immodules-3.0 0755L
gtk3.src:286: W: macro-in-comment %{_mandir}
gtk3.src:287: W: macro-in-comment %{_mandir}
gtk3.src:288: W: macro-in-comment %{_mandir}
gtk3.src:325: W: macro-in-comment %{_mandir}
gtk3.src:326: W: macro-in-comment %{_mandir}
gtk3.src: W: no-cleaning-of-buildroot %clean
gtk3.src: W: no-buildroot-tag
gtk3.src: W: no-%clean-section
gtk3.src:123: W: mixed-use-of-spaces-and-tabs (spaces: line 123, tab: line 123)
1 packages and 0 specfiles checked; 0 errors, 12 warnings.

Apart from the tabs/spaces mix, this is all fine.

$ rpmlint gtk3*rpm
gtk3.x86_64: W: spelling-error %description -l en_US multi -> mulch, mufti
gtk3.x86_64: W: incoherent-version-in-changelog 2.9.0.0-4 ['2.90.0-4.fc14', '2.90.0-4']
gtk3.x86_64: W: shared-lib-calls-exit /usr/lib64/libgdk-x11-3.0.so.0.9000.0 exit.5
gtk3.x86_64: W: shared-lib-calls-exit /usr/lib64/libgdk_pixbuf_xlib-3.0.so.0.9000.0 exit.5
gtk3.x86_64: W: shared-lib-calls-exit /usr/lib64/libgtk-x11-3.0.so.0.9000.0 exit.5
gtk3.x86_64: W: no-manual-page-for-binary gtk-query-immodules-3.0-64
gtk3.x86_64: W: no-manual-page-for-binary update-gtk-immodules-3.0
gtk3.x86_64: W: no-manual-page-for-binary gtk-update-icon-cache-3.0
gtk3.x86_64: W: no-manual-page-for-binary update-gdk-pixbuf-loaders-3.0
gtk3.x86_64: W: no-manual-page-for-binary gdk-pixbuf-query-loaders-3.0-64
gtk3-devel.x86_64: W: spelling-error %description -l en_US amd -> mad, and, am
gtk3-devel.x86_64: W: no-documentation
gtk3-devel.x86_64: W: no-manual-page-for-binary gdk-pixbuf-csource-3.0
gtk3-devel.x86_64: W: no-manual-page-for-binary gtk3-demo
gtk3-devel.x86_64: W: no-manual-page-for-binary gtk-builder-convert-3.0
gtk3-devel-docs.x86_64: W: file-not-utf8 /usr/share/doc/gtk3-devel-docs-2.90.0/examples/calendar/calendar.c
gtk3-immodules.x86_64: W: spelling-error %description -l en_US gtk -> gt, gt k, GTE
gtk3-immodules.x86_64: W: non-coherent-filename getfile?taskID=2201277&name=gtk3-immodules-2.90.0-4.fc14.x86_64.rpm gtk3-immodules-2.90.0-4.fc14.x86_64.rpm
gtk3-immodules.x86_64: W: no-documentation
gtk3-immodules.x86_64: W: non-conffile-in-etc /etc/X11/xinit/xinput.d/im-cedilla.conf
gtk3-immodule-xim.x86_64: W: spelling-error %description -l en_US gtk -> gt, gt k, GTE
gtk3-immodule-xim.x86_64: W: no-documentation

The typo is real, and calendar.c should be fixed as well (probably upstream).

package name: ok
spec file name: ok
packaging guidelines: see above
license: ok
license field: ok
license file: ok
spec file language: ok
spec file legibility: ok

Do this upstream?

# fight unused direct deps
sed -i -e 's/ -shared / -Wl,-O1,--as-needed\0/g' libtool

# truncate NEWS
awk '/^Overview of Changes/ { seen+=1 }
{ if (seen < 2) print }
{ if (seen == 2) { print "For older news, see http://git.gnome.org/cgit/gtk+/plain/NEWS"; exit } }' NEWS > tmp; mv tmp NEWS

#
# Make cleaned-up versions of tutorials, examples, and faq for installation
#
mkdir -p tmpdocs
cp -aR docs/tutorial/html tmpdocs/tutorial
cp -aR docs/faq/html tmpdocs/faq

for dir in examples/* ; do
  if [ -d $dir ] ; then
     mkdir -p tmpdocs/$dir
     for file in $dir/* ; do
       install -m 0644 $file tmpdocs/$dir
     done
  fi
done

Use "install -D"
# Input method frameworks want this
mkdir -p $RPM_BUILD_ROOT%{_sysconfdir}/X11/xinit/xinput.d
cp %{SOURCE3} $RPM_BUILD_ROOT%{_sysconfdir}/X11/xinit/xinput.d


upstream sources: nak
buildable: yes, runs fine in scratch
excludearch: n/a
build deps: ok
locale handling: ok
ldconfig: ok
relocatable: n/a
directory ownership: ok
duplicate files: ok
file permissions: ok
%clean: ok
content: permissible
documentation: ok
headers: ok
static libs: ok
pkgconfig files: ok
shared libraries: ok
devel package: ok
libtool archives: ok
gui apps: n/a
file ownership: ok
%install: ok
utf8 filenames: ok

Looks fine apart from the items mentioned above, and some upstream TODO items mentioned in the spec file.

Feel free to ask for CVS when those items are fixed.

Comment 12 Matthias Clasen 2010-05-22 01:07:03 UTC
New versions:

srpm: http://mclasen.fedorapeople.org/gtk3-2.90.0-5.fc14.src.rpm
spec: http://mclasen.fedorapeople.org/gtk3.spec    


I have fixed the typo, fixed calendar.c encoding upstream, got rid of tabs, used install -D. Some more spec file cleanups will happen with the next upstream release, which changes the location of the immodule and loader cache files.

Comment 13 Matthias Clasen 2010-05-22 01:08:25 UTC
New Package CVS Request
=======================
Package Name: gtk3
Short Description: Version 3 of the GTK+ toolkit
Owners: mclasen
Branches:

Comment 14 Chen Lei 2010-05-25 04:17:04 UTC
I'll be much better to not require gtk-doc in gtk3-devel-docs
Simply own /usr/share/gtk-doc/html in %file will be more appropiate, actually html documentations in /usr/share/gtk-doc/html is used by devhelp or webbrower not by gtk-doc. gtk-doc is a tool to generate those html files.

Requires: automake is also not necessrary, it'll pull in some additional dependency which is useless for most SRPMs.

Comment 15 Chen Lei 2010-05-25 04:32:03 UTC
Some many suggestion:

Most IM packages(scim, ibus) add %config to its conf file.


%{_sysconfdir}/X11/xinit/xinput.d/im-cedilla.conf
->%config  %{_sysconfdir}/X11/xinit/xinput.d/im-cedilla.conf


I suggest to add a gnome-filessystem to fedora, like kde-filesystem, mozilla-filesystem to own some common directories in gnome. So that, gnome packages can only depends on this filesystem package instead of owning the directories.
e.g.
%{_datadir}/gtk-doc
%{_datadir}/gtk-doc/html 
%{_datadir}/gir-1.0
%{_datadir}/gnome/help

Comment 16 Dennis Gilmore 2010-05-25 20:52:33 UTC
CVS Done

Comment 17 Bastien Nocera 2010-06-21 14:31:51 UTC
gtk3-2.90.1-1.fc14 and subsequent versions were built.


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