Bug 731851

Summary: Review Request: pygobject3 - Parallel installable python module for GObject Introspection
Product: [Fedora] Fedora Reporter: John (J5) Palmieri <johnp>
Component: Package ReviewAssignee: Michel Lind <michel>
Status: CLOSED CURRENTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: icq, jkeck, michel, notting, package-review
Target Milestone: ---Flags: michel: fedora-review+
gwync: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-09-15 08:42:01 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 719791    

Description John (J5) Palmieri 2011-08-18 19:53:32 UTC
Spec URL: http://johnp.fedorapeople.org/downloads/pygobject3/pygobject3.spec
SRPM URL: http://johnp.fedorapeople.org/downloads/pygobject3/pygobject3-2.90.2-1.fc15.src.rpm
Description: This packages installs in parallel with pygobject2 to provide GObject Introspection support.  pygobject2 sticks around to provide legacy support for the static python gobject bindings.

Comment 1 John (J5) Palmieri 2011-08-18 20:42:22 UTC
You will need the latest version of pygobject2 from rawhide if you wish to install this otherwise you will get conflicts.

Comment 2 Michel Lind 2011-08-19 15:20:01 UTC
Reviewing using these scratch builds, for F-16:

pygobject2:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3286561

pygobject3:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3286567

Comment 3 Michel Lind 2011-08-19 17:04:22 UTC
Some changes are necessary; please see the unchecked boxes:


#+TODO: TODO(t) WAIT(w@/!) FAIL(f@) | DONE(d) N/A(n)

* TODO Review [70%]
  - [X] Names [2/2]
    - [X] Package name
    - [X] Spec name
  - [X] Package version [2/2]
	http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Package_Versioning
    - [X] Version number
	  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Version_Tag
    - [X] Release tag
	  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Release_Tag
	  http://fedoraproject.org/wiki/Packaging/NamingGuidelines#Pre-Release_packages
  - [X] Meets [[http://fedoraproject.org/wiki/Packaging/Guidelines][guidelines]]
  - [X] Source files match upstream
        $ sha1sum pygobject-2.90.2.tar.bz2 ~/rpmbuild/SOURCES/pygobject-2.90.2.tar.bz2 
        ea56c46e2820578e18520d2ecd97547bbd8fc196  pygobject-2.90.2.tar.bz2
        ea56c46e2820578e18520d2ecd97547bbd8fc196  /home/michel/rpmbuild/SOURCES/pygobject-2.90.2.tar.bz2
  - [X] [[http://fedoraproject.org/wiki/Packaging:No_Bundled_Libraries][No bundled libraries]]
  - [-] License [2/4]
    - [X] License is Fedora-approved
    - [X] No licensing conflict
    - [ ] License field accurate
          some files are actually under MIT license; update field to say
          LGPLv2+ and MIT

          $ grep -rl "WITHOUT WARRANTY OF ANY KIND" *
          gi/pygi-property.c
          gi/pygi-foreign-gvariant.h
          gi/pygi-foreign-gvariant.c
          gi/pygi-foreign.c
          gi/pygi-signal-closure.h
          gi/pygi-foreign.h
          gi/pygi-property.h
          gi/pygi-foreign-cairo.c
          install-sh <-- irrelevant

    - [ ] License included iff packaged by upstream
          Please package COPYING file in pygobject3 and python3-gobject
  - [-] rpmlint [1/2]
    - [X] on src.rpm
          1 packages and 0 specfiles checked; 0 errors, 0 warnings.
    - [ ] on x86_64.rpm
          pygobject3.x86_64: W: incoherent-version-in-changelog 2.28.6-1 ['2.90.2-1.fc16', '2.90.2-1']
          \=> version number needs updating
          pygobject3.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/gi/_gi_cairo.so _gi_cairo.so()(64bit)
          pygobject3.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/gi/_glib/_glib.so _glib.so()(64bit)
          pygobject3.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/gi/_gi.so _gi.so()(64bit)
          pygobject3.x86_64: W: private-shared-object-provides /usr/lib64/python2.7/site-packages/gi/_gobject/_gobject.so _gobject.so()(64bit)
          \=> normal for Python modules
          pygobject3.x86_64: E: library-without-ldconfig-postin /usr/lib64/libpyglib-gi-2.0-python.so.0.0.0
          pygobject3.x86_64: E: library-without-ldconfig-postun /usr/lib64/libpyglib-gi-2.0-python.so.0.0.0
          \=> ldconfig needs to be added
          pygobject3.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libpyglib-gi-2.0-python.so
          \=> can this be moved to -devel?
          pygobject3.x86_64: W: doc-file-dependency /usr/share/doc/pygobject3-2.90.2/examples/cairo-demo.py /usr/bin/env
          \=> harmless
          pygobject3-devel.x86_64: W: spelling-error Summary(en_US) embeding -> embedding, bedimming, bedding
          \=> please correct spelling (wow, aspell actually works this time)
          pygobject3-devel.x86_64: W: no-documentation
          \=> harmless
          pygobject3-doc.x86_64: E: non-executable-script /usr/share/pygobject/xsl/fixxref.py 0644L /usr/bin/env
          \=> is this even needed? might want to exclude it
          4 packages and 0 specfiles checked; 3 errors, 9 warnings.

  - [X] Language & locale [2/2]
    - [X] Spec in US English
    - [X] Spec legible
          field values could probably be better aligned, but that's a nitpick
  - [X] Build [2/2]
    - [X] BRs complete
    - [X] Directory ownership
  - [-] Spec inspection [7/10]
    - [ ] ldconfig for libraries
          Missing; see rpmlint comment
    - [X] No duplicate files
    - [X] File permissions
    - [X] Filenames must be UTF-8
    - [X] no BuildRoot ([[https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag][except if targeting RHEL5]])
          OPTIONAL: buildroot could be removed
    - [X] [RHEL 5] %buildroot cleaned on %install
          OPTIONAL: rm -rf no longer necessary
    - [ ] Macro usage consistent
          Question: why is %{_smp_mflags} not used for building python3-gobject?
    - [X] Documentation [2/2]
      - [X] If large docs, separate -doc
      - [X] %doc files are non-essential
    - [-] Development [3/4]
      - [X] Headers in -devel
      - [ ] If versioned .so's, unversioned in -devel
	    see rpmlint comment above
      - [X] -devel, -static requires main
      - [X] No .la
    - [X] Other subpackages

Comment 4 Michel Lind 2011-08-19 17:18:07 UTC
Problems experienced when attempting to install pygobject3:
- the -codegen subpackage is still defined, and marked as a dependency of the -devel subpackage, but it is no longer generated (and has no %files codegen section).

This should probably be excised; right now the -devel subpackage is not installable

- pygobject3-doc and pygobject2-doc are not parallel-installable; either make them install files into versioned directories (e.g. %{_datadir}/gtk-doc/html/pygobject{2,3}/) or make them conflict with each other. The former is probably better

== Usage testing ==
gnome-tweak-tool now works again! Will update the dependency once this review is done

Comment 5 John (J5) Palmieri 2011-08-19 19:38:56 UTC
Fixed all errors except the .so version warning as this should be fixed upstream.  There is no reason to link directly to it anymore and I don't want to create a -devel package for python3-gobject just for this.

As for the warnings they can be waved.

Docs were removed because they are a holdover from the static bindings and are being reworked upstream.

new spec -->  http://johnp.fedorapeople.org/downloads/pygobject3/pygobject3.spec
new srpm -->  http://johnp.fedorapeople.org/downloads/pygobject3/pygobject3-2.90.2-2.fc15.src.rpm

I tried scratch building for f16-candidate but there is a dep on a newer version of python3-pycairo.  How did you build it?

Rawhide build --> http://koji.fedoraproject.org/koji/taskinfo?taskID=3287559

Comment 6 Michel Lind 2011-08-19 20:58:42 UTC
So the python3-pycairo available to f16-candidate is too old? Not sure how I managed to get past it, maybe it was picking a different mirror that happens to have it.

You have apparently unused BuildRequires: on autoconf/automake/libtool -- leftover from a spec used with a snapshot tarball?

And -devel still depends on %{name}-doc, that should be commented out.

But apart from that, this looks good -- please make those changes before committing.

APPROVED

Comment 7 Michel Lind 2011-08-20 08:33:34 UTC
J5, any idea how many other packages actually make use of pygobject2's introspection? They'd need to be updated for F-16 and Rawhide...

e.g. gnome-tweak-tool uses this, but relies on gnome-shell to pull in pygobject2, and it's not possible ATM to find out, just by examining its RPM's requirements, to see that it makes use of Python's 'gi' module.

Comment 8 Ignacio Casal Quinteiro (nacho) 2011-08-20 12:47:41 UTC
libpeas was using pygobject 2 but it will use pygobject 3 in the new version, this means, gedit, totem etc etc will need a rebuild.

Comment 9 John (J5) Palmieri 2011-08-20 14:10:42 UTC
The only way to tell if the package is using gi is to search for everything that requires pygobject2 and check them manually.  The thing is only a few modules are fully ported so it shouldn't be that much of an issue.

Comment 10 John (J5) Palmieri 2011-08-20 14:13:50 UTC
New Package SCM Request
=======================
Package Name: pygobject3
Short Description: Python bindings for GObject Introspection
Owners: johnp nacho
Branches: f16 f16-candidate
InitialCC:

Comment 11 John (J5) Palmieri 2011-08-20 14:26:38 UTC
BTW, I'm pretty sure we haven't broken ABI so I don't think a recompile is needed.  Libpeas has to change its requires so it will get a recomile anyway.  Does dependencies of libpeas need to be recompiled everytime libpeas is?

Comment 12 Ignacio Casal Quinteiro (nacho) 2011-08-20 14:39:44 UTC
well, in the last dicussion we had afaik there was an ABI break in pygobject. So if so we will need to recompile everything. Anyway at least gedit will need to be recompiled as we need to use the introspected glib.

Comment 13 Gwyn Ciesla 2011-08-20 15:22:50 UTC
Git done (by process-git-requests).

Removed invalid f16-candidate.