Bug 526126

Summary: Review Request: python3 - Python 3.x (backwards incompatible version)
Product: [Fedora] Fedora Reporter: Andrew McNabb <amcnabb>
Component: Package ReviewAssignee: Tim Lauridsen <tla>
Status: CLOSED DUPLICATE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: a.badger, che666, dmalcolm, fedora-package-review, ivazqueznet, james.antill, jdy, linuxdonald, lmacken, notting, phuang, pmatilai, steve.traylen, sundaram, tla, tomspur
Target Milestone: ---Flags: tla: fedora‑review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-01-12 12:43:46 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 226342    
Bug Blocks: 530636, 531648, 545124    
Attachments:
Description Flags
Script to locate collisions between python 2 and python 3 rpms
none
Output from the script
none
patch to version 9 of the spec file
none
Updated version of script to locate collisions between python2/python3 none

Description Andrew McNabb 2009-09-28 18:41:13 EDT
Spec URL: http://aml.cs.byu.edu/~amcnabb/python3.spec
SRPM URL: http://aml.cs.byu.edu/~amcnabb/python3-3.1.1-1.fc11.src.rpm
Description: Python 3.x (backwards incompatible with the 2.x series)

Python 3.x is somewhat controversial in the Fedora community.  Since so many of the system tools are written in Python, it may be a very long time before Fedora can be migrated from Python 2.x to Python 3.x.  Unlike previous updates to Python, it will be impossible to switch over to a new version in one day.

I believe it will be important to have a Python 3.x RPM available.  Note that I do NOT propose creating any separate library RPMs for Python 3.  There are several reasons for creating an early RPM for Python 3.x without creating new RPMs for all of the various Python packages.  First, Fedora developers will eventually need to experiment with Python 3.x, and having an RPM available would help with this.  Second, people learning Python are now finding resources that are specific to Python 3.x (for example, the famous book "Dive Into Python" is now available for Python 3.x).  Third, Python developers (like myself) may wish to use Python 3.x for simple scripts that don't rely on third-party libraries (there are plenty of batteries included in the standard library).

This new Python 3.x RPM is loosely based on the current Python 2.x RPMs.  I'm sure it still isn't perfect, but I'm at the point where I would appreciate feedback.  I really think that this RPM will be helpful for many people.
Comment 1 Andrew McNabb 2009-09-28 18:42:50 EDT
By the way, this is my first Fedora package, so I would greatly appreciate help from a sponsor.  Thanks.
Comment 2 Thomas Kowaliczek 2009-09-28 21:13:50 EDT
The files are not available. An reviewer can't review it ;)
And i can't review it because it's to big for me.
Comment 3 Rahul Sundaram 2009-09-29 02:18:04 EDT
Some work has already been done 

http://ivazquez.fedorapeople.org/packages/python3000/

Might want to compare.
Comment 4 Andrew McNabb 2009-09-29 12:15:08 EDT
Thomas, I've tried downloading the files from a few different places, and everywhere has worked.  What problem are you seeing?
Comment 5 Thomas Kowaliczek 2009-09-29 20:01:58 EDT
Ah i see now it works :)
Comment 6 Dave Malcolm 2009-10-01 15:45:18 EDT
I also had a go at packaging 3.1.1, before I saw either of your efforts, so we now have 3 separate SRPMS...  I'll have a look and compare the 3 SRPMs.  

To liven things up further, I think it would be good if the python 3 specfile closely resembled the python 2 specfile.  (It may be necessary to clean up the python 2 specfile to do this, see bug 226342).

I've written up some thoughts about getting Python 3 into Fedora here:
https://www.redhat.com/archives/fedora-devel-list/2009-October/msg00054.html
Comment 7 Dave Malcolm 2009-10-13 16:48:29 EDT
Adding ivazquez to the CC list.  Ignacio: I hope that's OK.

Andrew: thanks for your work on packaging Python 3

I feel a little awkward about reviewing this.  As far as I can see based on the link in comment #3, ivazquez has been packaging Python 3000 since 2007-09-01, with "3.0-0.a1.1":
Sat Sep  1 2007 Ignacio Vazquez-Abrams <ivazqueznet+rpm@gmail.com> 3.0-0.a1.1

and he's been steadily updating his rpms since then.  However, I don't think he ever filed a package review request on those rpms.  (Please correct me if I'm wrong!)  Ignacio: is there a reason you didn't file a review request?

I've posted a proposal on Python 3 in Fedora 13.  See this proposed feature page:
https://fedoraproject.org/wiki/Features/Python3F13

I'm very keen to see a python 3 package in Fedora, either based on Andrew's srpm, or on Ignacio's.  I also tried packaging it, but I'm happy to abandon my specfile in favor of your work.  I would like to be a comaintainer of the resulting package.

For me, the most important thing is that the python 3 rpms should be installable in parallel with the python 2 rpms.  I hope I captured the other kinds of expected behavior on the feature page above.

I'd be happy to review your package (though we need to find a sponsor for you), but I'm worried that Ignacio may feel put out if we use your package rather than his (and likewise, vice versa).  I'm curious why Ignacio never filed a package review request on his packages.

Thoughts?

I tried generating a pure textual "diff" of the two specfiles (Andrew's vs Ignacio's), but too much is different for the result to be meaningful.

So I tried comparing individual aspects of the packaging (no doubt omitting much):

= Name =
"python3" vs "python3000"

I prefer "python3"; I feel that "python 3000" was a good development codename during the development towards 3.0, but now that it's been released, "3" seems to be the name.  I'd prefer us to use "python3-" as a naming prefix throughout.

= Source and Patches =
ivazquez's srpm makes use of various ".list" source files to control various parts of the build/install/etc, and I think this approach works well.

amcnabb's patches:
Patch0: python-3.1.1-config.patch
This heavily patches Modules/Setup.dist, like we do on the python 2 rpm.  I haven't gone though and checked the various modules in detail yet, but it looks reasonable, a "batteries-included" approach.

Patch102: python-3.1.1-lib64.patch
It's only applied on 64-bit archs; it reworks lib to lib64 in many places.

# http://bugs.python.org/issue6999 -- fixed in r75062
Patch200: python-3.1.1-pathfix.patch
Fixes an encoding issue with the pathfix script, used when fixing up shebangs.  Good to see that you upstreamed it.


ivazquez's patches:
Patch0:         Python-3.1.1-rpath.patch
Removes standard library path from rpath.

Patch1:         Python-3.0a5-libprep.patch

Ignacio's version fix up the shebangs uses sed (see "sedcmd" in the specfile), rather than fixing the broken "pathfix" tool.

= BuildRequires =
Both have BuildRequires on:
  bzip2-devel
  db4-devel (amcnabb's has a ">= 4.7" here)
  gdbm-devel
  openssl-devel
  ncurses-devel
  readline-devel
  sqlite-devel
  tk-devel
(alphabetized them for ease of comparison)

amcnabb's has an additional:
BuildRequires: gmp-devel
BuildRequires: zlib-devel, expat-devel
BuildRequires: libGL-devel tk tix gcc-c++ libX11-devel glibc-devel
BuildRequires: bzip2 tar /usr/bin/find pkgconfig tcl-devel
BuildRequires: tix-devel 
BuildRequires: autoconf
BuildRequires: libffi-devel

Some of these are implicitly assumed in the buildroot, and don't need to be explicitly listed.  The others presumably reflect the very broad batteries-included approach taken when patching Modules/Setup.dist

ivazquez's has an additional:
BuildRequires: gdbm-devel
BuildRequires: python

= Description =
amcnabb's seems to be taken directly from our python 2 specfile; I think the one in ivazquez's specfile is better (although as I said, I feel we should refer to Python 3 not to Python 3000).  In particular, the description in ivazquez' specfile deprecates the use of TkInter, which I think is wise advice!

= Subpacakages =
amcnabb's specfile has a -test subpackage, whereas ivazquez's is -tests.  Our python 2 specfile uses the singular form, so perhaps we should use the singular form for consistency (also, the main component of it is the "Lib/test" directory from the upstream tarball, again a singular)

= Configuration =
Both specfiles have %configure with:
  --enable-ipv6
  --enable-shared

amcnabb's adds: --with-wide-unicode --with-system-ffi
ivazquez's adds: --enable-unicode=ucs4

I definitely want us to have "--with-system-ffi"


I hope that's a useful start on reviewing this.

Should I begin a formal package review of Andrew's package?
Comment 8 Andrew McNabb 2009-10-13 17:26:22 EDT
Dave, thank you very much for your insightful thoughts.  I agree with you on almost all points.  The important thing for me is getting a working RPM that can be installed alongside Python 2.  As you noticed, a lot of things in my specfile exist purely to be similar to Python 2 (although I changed anything that seemed wrong or inapplicable).  I wouldn't have any hard feelings over which specfile is used.  However, there are some areas where I think mine is more "right" and other areas where I think ivazquez's is probably better.  Obviously, we should want to combine the best of both.  Here are a few specific thoughts:

1) I think that the name python3 is clearly preferable to python3000.

2) I'm confused by the .list thing.  The .list files are really short, and I don't see how they make it any more clear.  In fact, I think they make the specfile more difficult to read.  Is this a standard way to do things?

3) The patch python-3.1.1-config.patch follows the approach of Python 2.  This is really important (whoever did Python 2 really knew what they were doing).  Python's default build configuration (without the patch applied) will fail silently if a module can't be built.  This is really evil.  It means that if a dependency is missing, then files will just disappear.  This patch gets rid of silent failures by explicitly specifying which modules must be built.

4) The patch python-3.1.1-lib64.patch ensures that Python is installed into /usr/lib64 (as per Fedora's packaging guidelines).  It looks like the python3000 specfile does the same work with sed instead of a patch.  I don't know which approach is better, and I could be easily persuaded either way.  I picked to make a patch to more closely match the Python 2 specfile, but I can definitely see some advantages to using sed.

5) I have no strong feelings on the package description or build dependencies.  However, I don't get the disparaging remarks about Tkinter.  Not that I have any attachment to Tkinter, but I'm not aware of it being considered in any way deprecated.

6) The configure option --with-wide-unicode seems to be the correct spelling for the option, where --enable-unicode=ucs4 is the old way to say it.

7) By the way, the python3000 package defines a python3000-libs subpackage, but the python3 package just includes the .so file in the main package.  When I tried to have a python3-libs package, it ended up being a prerequisite for python3 anyway, so I couldn't see the point in keeping it separate.  However, it's entirely possible that I did something wrong to make this happen. :)

ivazquez, do you have any thoughts on these issues?  Is there any particular reason that you didn't submit your package for review?  Thanks for your work!
Comment 9 James Antill 2009-10-14 01:35:27 EDT
 I'm pretty worried about multiple installable python's, so I've tried to keep out of this (hey, it's David's problem now when he breaks the world anyway ... I just get to complain :)
 Anyway, I think I can clear some stuff up:

4) IMO use a patch instead of sed, the advantage of sed is that a patch often breaks as new upstreams of python are done. The disadvantage of sed is that it'll keep almost working, silently, when new upstreams of python change something.

5) The problems with TK are that it looks like gack, and has no i18n support. So pygtk2 is _much_ prefered.

7) python-libs is separate for multilib, x86_64 has just:

python-devel.i586                        2.6-9.fc11                    updates
python-libs.i586                         2.6-9.fc11                    updates
Comment 10 Andrew McNabb 2009-10-14 12:47:38 EDT
James, thanks for your thoughts.  By the way, why are you worried about multiple installable pythons?  Unless someone runs "python3", they will get the normal (python2) system python.  The python3 package would never install a binary named "python".  Anyway, I would love to understand the concerns.

In response to your comments:

4) Thanks for clarifying the difference.  I'm already using patches instead of sed in my specfile, so I'll just keep on doing that.

5) I agree that Tk isn't the prettiest thing in the world, but upstream hasn't deprecated it (and it's the only GUI in the stdlib).  I don't think we have to like it, but that doesn't mean we should make misleading comments in the package descriptions.

7) Thanks for the clarification about multilib (this is one of those areas where I still make lots of mistakes).  I'll plan on putting a libs package back in.
Comment 11 Ignacio Vazquez-Abrams 2009-10-15 16:31:12 EDT
2) The .list thing was experimental. Feel free to ignore it if you find it impedes progress.

4) My SRPM uses both a patch and sed. The patch annotates where files should be poked in order to enable multilib, and sed does the actual poking.

5) I do understand that TkInter is still available, regardless of the fact that I want it to die a flaming, screaming death. However, I do not believe that my description is in any way misleading.
Comment 12 Andrew McNabb 2009-10-15 19:06:52 EDT
Ignacio, thanks for your help.  For the record, I'm really not trying to step on your toes with this specfile.  What are your thoughts on combining and merging our efforts?
Comment 13 James Antill 2009-10-16 17:20:57 EDT
> James, thanks for your thoughts.  By the way, why are you worried about
multiple installable pythons?

I think I've written down all of the big concerns, but I reserve the right to have forgotten things:)

http://illiterat.livejournal.com/7660.html
Comment 14 Andrew McNabb 2009-10-16 18:22:58 EDT
James, thanks for the informative link.

One of the concerns was: "Of course humans are lazy, so to get around doing some of this work someone will decide it's a good idea to have a single python-blah package that works with either/both versions of python ... and then you'll have more problems."  A package that works with both python2 and python3 would have to write files to both /usr/lib/python2.6/site-packages and /usr/lib/python3.1/site-packages.  This is clearly a bad idea, and it seems reasonable to ban it in the packaging guidelines.

There is also the worry about having to support two versions of each package.  For right now, we're just doing the interpreter, and you make a great argument for waiting to package libraries for Python 3.  However, this will eventually be unavoidable, and ignoring Python 3 won't make it go away.

Is it inaccurate to summarize the rest of your concerns as: "two versions of Python will lead to user complaints"?  I think all of the complaints that you enumerated are unavoidable and not particularly severe.  I could see an argument for making some additions to the release notes.
Comment 15 Steve Traylen 2009-10-16 18:54:29 EDT
For comment #14 and the problem of two versions of say, SOAPpy for
2.6 and 3.0 I think we just need to have standard variable so a package content
can just be swinged and duplicated from default to special e.g 3.0 in 
this context.
... As you say it all comes down to guidelines basically.
Steve... 
P.S I would love to see python3 available.
Comment 16 James Antill 2009-10-17 01:40:32 EDT
> A package that works with both python2 and python3 ... is clearly a bad idea, and it seems reasonable to ban it in the packaging guidelines.

Well it's clear to you because you don't have to do 3x the work :) ... but, yeh, in Fedora the obvious solution to this is to ban it (I've seen 3rd party repos. do it for php modules so they work with the RHEL-5 version and a php-5.3 version).

> For right now, we're just doing the interpreter, and you make a great argument
> for waiting to package libraries for Python 3.

Right, as I said ... the problem with just doing the interpreter is that it doesn't really help anyway because very few things use no extra modules. For this reason alone I don't think you can pretend this will be a one off, it's very likely to avalanche into RFEs for everything possible to be available in both versions. So, again IMO, you need to start from that assumption not assume that you can just do a py2k package.

> However, this will eventually be unavoidable ignoring Python 3 won't make it go away.

I'm not saying to ignore it, we do semi-significant GCC updates every few releases which break things ... yes, python3 breaks pretty much every piece of python ever. But I'd still hope we could do all the bits that needed to be done during a single release (esp. with time to prepare for it). But as I also said, it's kinda David's problem as he gets to keep all the pieces if/when it breaks.

> Is it inaccurate to summarize the rest of your concerns as: "two versions of
Python will lead to user complaints"

Well there was the minor bit about how everyone has to do 3x the testing :).
Comment 17 Dave Malcolm 2009-10-19 15:19:53 EDT
Created attachment 365267 [details]
Script to locate collisions between python 2 and python 3 rpms

The python 2 and python 3 packages must be parallel-installable.

In order to verify this, I've written a script to attempt to identify areas where the two sets of packages could interfere with each other.

Specifically, the attached script takes the locally-installed set of python (2) rpms as one PackageSet, and a locally-built set of python3 rpms as another PackageSet, and attempts to locate
  - names listed in the "Provides" in one set of packages that also occur in the "Provides" of a package in the other set
  - paths owned by packages in both sets

Is the script a reasonable test of the parallel-installability of the rpms?  Is there anything else it should test for?

Running this script on my F11 machine's python rpms, together with a local build of the candidate python3 rpms outputs some collisions:

==  Colliding "Provides" items: ==
Lots of "foo.so" lines appear (e.g. "_bisectmodule.so"), but probably shouldn't.  The python modules have numerous auto-generated lines e.g. "Provides: _bisectmodule.so".  I believe they're coming from the SONAME entries in the built c extension modules, from the script "/usr/lib/rpm/find-provides".

Does anything actually use these provides lines?  My feeling is that they should be suppressed for the Python 3 rpms, and probably for Python 2 rpms also: the .so files aren't in the regular system search path for DSOs (instead, being loaded by Python's module loading mechanism).

Looking at /usr/lib/rpm/macros, it looks like this can be overridden by setting __find_provides to another script, or empty, though we probably shouldn't do this; don't want to hide the python library itself.

== Colliding filesystem items: ==
I see the following collisions; duplicate ownership within the -debuginfo packages.  I believe that these aren't a problem:
/usr/lib/debug
/usr/lib/debug/.build-id
/usr/lib/debug/.build-id/00
/usr/lib/debug/.build-id/0c
/usr/lib/debug/.build-id/2b
/usr/lib/debug/.build-id/39
/usr/lib/debug/.build-id/47
/usr/lib/debug/.build-id/61
/usr/lib/debug/.build-id/76
/usr/lib/debug/.build-id/7b
/usr/lib/debug/.build-id/94
/usr/lib/debug/.build-id/9b
/usr/lib/debug/.build-id/a1
/usr/lib/debug/.build-id/ba
/usr/lib/debug/.build-id/d3
/usr/lib/debug/.build-id/d4
/usr/lib/debug/.build-id/eb
/usr/lib/debug/.build-id/ed
/usr/lib/debug/.build-id/fd
/usr/lib/debug/usr
/usr/lib/debug/usr/bin
/usr/lib/debug/usr/lib
Comment 18 Dave Malcolm 2009-10-19 15:20:44 EDT
Created attachment 365268 [details]
Output from the script
Comment 19 Andrew McNabb 2009-10-20 12:05:57 EDT
Dave, I think it makes sense to block the private libraries from being added to provides, but I'm not experienced enough to know the best way to do this.
Comment 20 Dave Malcolm 2009-10-20 19:45:09 EDT
I've had a go at merging Andrew and Ignacio's specfiles, using Andrew's specfile as a base.  I've also attempted to address some of the issues already raised in this review.

Specfile: http://dmalcolm.fedorapeople.org/python3.spec
SRPM: http://dmalcolm.fedorapeople.org/python3-3.1.1-2.fc11.src.rpm

To ease review, a diff from Andrew's 3.1.1-1 specfile to my 3.1.1-2 specfile can be seen here:
http://dmalcolm.fedorapeople.org/python3-from-3.1.1-1-to-3.1.1-2.diff

I wrote a custom __find_provides hook, filtering away paths matching /usr/lib/python (which appears to require setting _use_internal_dependency_generator to 0).  See http://dmalcolm.fedorapeople.org/find-provides-without-python-sonames.sh

This successfully gets rid of the various _collectionsmodule.so etc provides lines:
$ rpm -q --provides python3
python3 = 3.1.1-2.fc11
python3(x86-32) = 3.1.1-2.fc11
and thus my script to find collisions with the regular "python" rpms no longer detects any "Provides" collisions .

Note that SONAME provides still work for regular libraries:
$ rpm -q --provides python3-libs
libpython3.1.so.1.0  
python3-libs = 3.1.1-2.fc11
python3-libs(x86-32) = 3.1.1-2.fc11

See the %changelog in the specfile for the other changes.

rpmlint output isn't clean yet (but it's getting late here):
python3.i586: E: binary-or-shlib-defines-rpath /usr/lib/python3.1/lib-dynload/_sqlite3.so ['/usr/lib']
python3.i586: E: zero-length /usr/lib/python3.1/build_class.py
python3-libs.i586: W: no-documentation
python3-test.i586: W: no-documentation
python3-test.i586: E: zero-length /usr/lib/python3.1/test/nullcert.pem
python3-test.i586: E: script-without-shebang /usr/lib/python3.1/distutils/tests/Setup.sample
python3-test.i586: W: uncompressed-zip /usr/lib/python3.1/test/zipdir.zip
python3-tkinter.i586: W: no-documentation
python3-tools.i586: E: script-without-shebang /usr/lib/python3.1/Demo/comparisons/patterns
python3-tools.i586: E: script-without-shebang /usr/lib/python3.1/Demo/rpc/test
python3-tools.i586: W: file-not-utf8 /usr/lib/python3.1/Demo/distutils/test2to3/setup.py
python3-tools.i586: E: zero-length /usr/lib/python3.1/Tools/modulator/Templates/copyright
python3-tools.i586: W: wrong-file-end-of-line-encoding /usr/lib/python3.1/Demo/turtle/tdemo_round_dance.py
python3-tools.i586: W: file-not-utf8 /usr/lib/python3.1/Demo/rpc/README
python3-tools.i586: E: script-without-shebang /usr/lib/python3.1/Tools/README
python3-tools.i586: E: script-without-shebang /usr/lib/python3.1/Demo/scripts/newslist.doc
python3-tools.i586: E: script-without-shebang /usr/lib/python3.1/Demo/md5test/foo
python3-tools.i586: W: wrong-file-end-of-line-encoding /usr/lib/python3.1/Demo/turtle/tdemo_nim.py
7 packages and 0 specfiles checked; 10 errors, 8 warnings.
Comment 21 Dave Malcolm 2009-10-21 16:07:11 EDT
I've fixed some of the rpmlint issues.

Updated specfile here: http://dmalcolm.fedorapeople.org/python3.spec
Updated SRPM here: http://dmalcolm.fedorapeople.org/python3-3.1.1-3.fc11.src.rpm
Diff of specfile since comment #20 can be seen here: http://dmalcolm.fedorapeople.org/python3-from-3.1.1-2-to-3.1.1-3.diff

rpmlint output is now:
python3.i586: E: binary-or-shlib-defines-rpath /usr/lib/python3.1/lib-dynload/_sqlite3.so ['/usr/lib']
python3.i586: E: zero-length /usr/lib/python3.1/build_class.py
python3-libs.i586: W: no-documentation
python3-test.i586: W: no-documentation
python3-test.i586: E: zero-length /usr/lib/python3.1/test/nullcert.pem
python3-test.i586: W: uncompressed-zip /usr/lib/python3.1/test/zipdir.zip
python3-tkinter.i586: W: no-documentation
python3-tools.i586: W: file-not-utf8 /usr/lib/python3.1/Demo/distutils/test2to3/setup.py
python3-tools.i586: E: zero-length /usr/lib/python3.1/Tools/modulator/Templates/copyright
7 packages and 0 specfiles checked; 4 errors, 5 warnings.
Comment 22 Andrew McNabb 2009-10-21 18:07:44 EDT
Since Dave is working so hard on this, I think it would be great to switch to him as the submitter.  Ignacio, would you have any complaints with this?

Dave, would you need to clone this bug report, or is there some way to switch the reporter field?
Comment 23 Dave Malcolm 2009-10-21 18:27:13 EDT
> Since Dave is working so hard on this, I think it would be great to switch to
> him as the submitter.  Ignacio, would you have any complaints with this?

From my point-of-view, I'm happy to take over as the "requester" role within this review request; I have plenty of time to work on this package.

> Dave, would you need to clone this bug report, or is there some way to switch
> the reporter field?  
I don't think it's possible to switch the "reporter" field via the web UI.

I have a preference for not cloning: we'd lose the handy links for all of the comments so far.  But if you'd prefer me to go that route, I'll do it (potentially we could continue the review here, but if the import process requires a fresh clone we could do the clone at that point, to avoid splitting the conversation).
Comment 24 Rahul Sundaram 2009-10-21 18:51:06 EDT
The review process does depend on the reporter field to be set correctly so just before you import, you can clone, file this as a duplicate and then request cvs access.
Comment 25 Dave Malcolm 2009-10-22 11:44:32 EDT
Updated specfile: http://dmalcolm.fedorapeople.org/python3.spec
Updated SRPM: http://dmalcolm.fedorapeople.org/python3-3.1.1-4.fc11.src.rpm
Diff between 3.1.1-3 and 3.1.1-4: http://dmalcolm.fedorapeople.org/python3-from-3.1.1-3-to-3.1.1-4.diff

The above fixes a couple of compiled modules that were broken (spotted whilst running regrtest.py)

"import crypt" was failing with this error:
/usr/lib/python3.1/lib-dynload/cryptmodule.so: undefined symbol: crypt

"import datetime" was failing with this error:
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: /usr/lib/python3.1/lib-dynload/datetimemodule.so: undefined symbol: _PyTime_DoubleToTimet

The latter symbol is defined in timemodule.c; the fix is to build that module using "setup.py".

The updated srpm fixes both of these issues.

rpmlint output is the same as in comment #21
Comment 26 Andrew McNabb 2009-10-22 14:59:56 EDT
Dave, I've downloaded your new specfile and taken a look at it.

1) I love the description for __os_install_post.  It explains the problem clearly and is very helpful.

2) Are there any of our changes that should be submitted upstream?  For example, the iconv command on line 280 seems like something that should be fixed upstream.  The same is true for the chmod on line 255, the find/sed on line 277, and the find command on line 292.

3) The %description probably shouldn't refer to Python 3000, since that was just a code name and hasn't been used recently.  I would recommend rephrasing to "Python 3 is a new version of...".

I think you're making great progress.  This spec file is looking better and better all the time.
Comment 27 Dave Malcolm 2009-10-22 18:05:11 EDT
(Andrew, thanks; I'll respond to comment #26, but here's what I've done in the meantime)

Updated specfile: http://dmalcolm.fedorapeople.org/python3.spec
Updated SRPM: http://dmalcolm.fedorapeople.org/python3-3.1.1-5.fc11.src.rpm
Diff between 3.1.1-4 and 3.1.1-5: http://dmalcolm.fedorapeople.org/python3-from-3.1.1-4-to-3.1.1-5.diff

This version:
(a) adds a patch to fix what appears to be an upstream bug in permissions handling of .pyc files, which shows up during the regression test suite; I've sent the patch upstream (http://bugs.python.org/issue7187)

This improves the results of the regression test suite (/usr/lib/python3.1/test/regrtest.py)

Results with 3.1.1-4:
276 tests OK.
39 tests failed:
    test_cmd_line test_codeccallbacks test_codecencodings_cn
    test_codecencodings_hk test_codecencodings_jp
    test_codecencodings_kr test_codecencodings_tw test_cprofile
    test_distutils test_docxmlrpc test_email test_heapq
    test_htmlparser test_httpservers test_imp test_lib2to3
    test_linecache test_modulefinder test_multiprocessing test_osx_env
    test_plistlib test_pyclbr test_pydoc test_runpy test_socket
    test_sqlite test_sundry test_tcl test_threading_local test_tk
    test_ttk_guionly test_ttk_textonly test_uuid test_warnings
    test_wsgiref test_xml_etree_c test_xmlrpc test_xmlrpc_net
    test_zipfile

Results with 3.1.1-5:
304 tests OK.
10 tests failed:
    test_email test_httpservers test_imp test_lib2to3 test_linecache
    test_socket test_tk test_ttk_guionly test_ttk_textonly
    test_zipfile
   
(b) actually applies ivazquez's patch to fixup RPATH directives, fixing one error found by rpmlint:
binary-or-shlib-defines-rpath /usr/lib/python3.1/lib-dynload/_sqlite3.so ['/usr/lib']

The remaining rpmlint errors are:
python3.i586: E: zero-length /usr/lib/python3.1/build_class.py
python3-libs.i586: W: no-documentation
python3-test.i586: W: no-documentation
python3-test.i586: E: zero-length /usr/lib/python3.1/test/nullcert.pem
python3-test.i586: W: uncompressed-zip /usr/lib/python3.1/test/zipdir.zip
python3-tkinter.i586: W: no-documentation
python3-tools.i586: W: file-not-utf8 /usr/lib/python3.1/Demo/distutils/test2to3/setup.py
python3-tools.i586: E: zero-length /usr/lib/python3.1/Tools/modulator/Templates/copyright
7 packages and 0 specfiles checked; 3 errors, 5 warnings.
Comment 28 Dave Malcolm 2009-10-22 18:16:50 EDT
Of the rpmlint warnings, I think that the following are ignorable:
- 3 "zero-length" errors: I believe these are deliberate:
  - I believe that "build_class.py" must exist for legacy compat reasons (so that "import build_class" succeeds)
  - nullcert.pem is a test datafile
  - Templates/copyright is an empty placeholder file
- 1 "uncompressed-zip" warning: this is an upstream test data file for exercising the zip handling code.  It seems better to me to leave it as is.
- 1 "file-not-utf8" warning: this file ("Demo/distutils/test2to3/setup.py") is an upstream test file for the 2to3 code, with a non-utf8 encoding.  One of the purposes of the 2to3 code is to do character set conversions on .py files, so having this file as non-utf8 verifies that the 2to3 tool is capable of doing this correctly.

That leaves the 3 subpackages that show "no-documentation" warnings.  These aren't especially serious IMHO, but I can fix them if others perceive them as a problem.
Comment 29 Dave Malcolm 2009-10-22 19:43:24 EDT
(In reply to comment #26)
> Dave, I've downloaded your new specfile and taken a look at it.
> 
> 1) I love the description for __os_install_post.  It explains the problem
> clearly and is very helpful
Thanks.

I want to build up a full python 3 stack, and a problem is that every python3-foo module package will need to have a similar construction, and this could get messy fast.  So this probably needs fixing in rpmbuild, fixing things so that the /usr/lib/rpm/brp-python-bytecompile script can somehow be told which python implementation to use (it can accept an argument, but that's not how it's getting invoked).  Perhaps an environment variable can be used?  "RPMBUILD_PYTHON_INTERPRETER", or somesuch?

> 2) Are there any of our changes that should be submitted upstream?  For
> example, the iconv command on line 280 seems like something that should be
> fixed upstream.  The same is true for the chmod on line 255, the find/sed on
> line 277, and the find command on line 292.

Probably.

Unfortunately, the find/sed/chmod is corrupting
  /usr/lib/python3.1/test/test_httpservers.py
due to a line beginning with a shebang embedded deep in the file, which is causing
  grep -m 1 -q '^#!' ~/rpmbuild/BUILD/Python-3.1.1/Lib/test/test_httpservers.py
to match, and then the "sed" command strips out the first line of the file, which isn't a shebang line (and thus the file becomes syntactically invalid :-( )

grep's "-m" option doesn't seem to be quite what's needed here for restricting the shebang search to the first line in the file; any suggestions on fixing this?  (I tried using "head -n1 | grep", but am unsure of the correct way to embed that in the "find -exec" clause.

> 3) The %description probably shouldn't refer to Python 3000, since that was
> just a code name and hasn't been used recently.  I would recommend rephrasing
> to "Python 3 is a new version of...".
Sounds good

> I think you're making great progress.  This spec file is looking better and
> better all the time.  
Thanks!  FWIW I think the biggest remaining issue will be to figure out a sane way of building out a python3- stack on top of this package, which will involve thinking through packaging practices/guidelines (and probably just trying it for some packages).
Comment 30 Andrew McNabb 2009-10-22 20:47:22 EDT
(In reply to comment #29)
> 
> I want to build up a full python 3 stack, and a problem is that every
> python3-foo module package will need to have a similar construction, and this
> could get messy fast.  So this probably needs fixing in rpmbuild, fixing things
> so that the /usr/lib/rpm/brp-python-bytecompile script can somehow be told
> which python implementation to use (it can accept an argument, but that's not
> how it's getting invoked).  Perhaps an environment variable can be used? 
> "RPMBUILD_PYTHON_INTERPRETER", or somesuch?

I think that sounds like a good idea.  The only problem is that within the python3.spec, we couldn't just set `RPMBUILD_PYTHON_INTERPRETER=./python` because this interpreter also needs to be called with `LD_LIBRARY_PATH=.`.  And it would be a shame to change brp-python-bytecompile in a way that didn't help at all for python3.spec.  Hmm.


> Unfortunately, the find/sed/chmod is corrupting
>   /usr/lib/python3.1/test/test_httpservers.py
> due to a line beginning with a shebang embedded deep in the file, which is
> causing
>   grep -m 1 -q '^#!' ~/rpmbuild/BUILD/Python-3.1.1/Lib/test/test_httpservers.py
> to match, and then the "sed" command strips out the first line of the file,
> which isn't a shebang line (and thus the file becomes syntactically invalid :-(
> )

Hmm.  Is it really necessary to remove the shebang lines from .py files that aren't executable?  I think it's a complicated (and error-prone) step that doesn't have any tangible value.


> grep's "-m" option doesn't seem to be quite what's needed here for restricting
> the shebang search to the first line in the file; any suggestions on fixing
> this?  (I tried using "head -n1 | grep", but am unsure of the correct way to
> embed that in the "find -exec" clause.

If this step is really necessary, the only way to embed "head -n1 |grep" is to offload it into a standalone shell script.  Alternatively, you could use sed:

sed -e '/^#!/Q 0' -e 'Q 1' {}

It will return 0 if the shebang is found and 1 otherwise.  Hey, that's actually pretty cool, in a sick way. :)
Comment 31 Dave Malcolm 2009-10-23 19:51:58 EDT
(In reply to comment #29)
> (In reply to comment #26)
> > Dave, I've downloaded your new specfile and taken a look at it.
> > 
> > 1) I love the description for __os_install_post.  It explains the problem
> > clearly and is very helpful
> Thanks.

Actually, I now think my description is wrong; I now think the definition of __os_install_post comes from /usr/lib/rpm/redhat/macros (from our downstream redhat-rpm-config rpm).

I'll send a patch to add a %{__python} argument there, which should cover building rpms on top of this one; they should be able to a
  %define __python /usr/bin/python3
and rpmbuild should then bytecompile .py files in those rpms with that binary.

I don't think doing so covers the case of this core python3 package though, so maybe we still need the current workaround (we'd have to somehow define %{__python} to be the freshly-built python3.1, and set LD_LIBRARY_PATH etc, which I don't see how to do sanely)
Comment 32 Dave Malcolm 2009-10-26 19:23:44 EDT
(In reply to comment #31)
> (In reply to comment #29)
> > (In reply to comment #26)
> > > Dave, I've downloaded your new specfile and taken a look at it.
> > > 
> > > 1) I love the description for __os_install_post.  It explains the problem
> > > clearly and is very helpful
> > Thanks.
> 
> Actually, I now think my description is wrong; I now think the definition of
> __os_install_post comes from /usr/lib/rpm/redhat/macros (from our downstream
> redhat-rpm-config rpm).
> 
> I'll send a patch to add a %{__python} argument there, which should cover
> building rpms on top of this one; they should be able to a
>   %define __python /usr/bin/python3
> and rpmbuild should then bytecompile .py files in those rpms with that binary.

I didn't like this approach, as it would restrict us to having all .py files within one srpm build be for the same python implementation.

I've created a patch for rpmbuild which doesn't have this restriction, and it thus can support e.g. both a python-foo and python3-foo subpackage emitted from the same build.  I don't yet know if this is sane, but I don't want to rule it out due to tool bugs.

See bug 531117 for the gory details.


I've also created an rpmlint test to verify timestamps and ABI versions of .pyc/.pyo files (this is bug 531102).

Finally, I've patched "file" so it can identify Python 3 bytecode files (see bug 531082).

(all of the above bugs along with this review are on a Python 3 tracker bug; bug 530636).

> I don't think doing so covers the case of this core python3 package though, so
> maybe we still need the current workaround (we'd have to somehow define
> %{__python} to be the freshly-built python3.1, and set LD_LIBRARY_PATH etc,
> which I don't see how to do sanely)  

FWIW I think this is still true with my patch; IIRC "make install" does the byte-compilation using the freshly built python binary; I just need to sort out the timestamps.
Comment 33 Andrew McNabb 2009-10-27 12:45:12 EDT
(In reply to comment #32)
> 
> FWIW I think this is still true with my patch; IIRC "make install" does the
> byte-compilation using the freshly built python binary; I just need to sort out
> the timestamps.  

The install target doesn't do byte-compilation.  This step is performed manually, with the following line (on line 289 of my slightly outdated download of the spec file):

LD_LIBRARY_PATH=. /usr/lib/rpm/brp-python-bytecompile ./python
Comment 34 Dave Malcolm 2009-10-27 20:39:59 EDT
Updated specfile: http://dmalcolm.fedorapeople.org/python3.spec
Updated SRPM: http://dmalcolm.fedorapeople.org/python3-3.1.1-6.fc11.src.rpm
Diff between 3.1.1-5 and 3.1.1-6: http://dmalcolm.fedorapeople.org/python3-from-3.1.1-5-to-3.1.1-6.diff

This incorporates Andrew's description suggestion from comment #26 and his "sed" suggestion from comment #30.

This version improves the regrtest results.   When run as root, the only failures I get are now "test_httpservers" and "test_socket" (the latter due to not having my hostname set up properly in DNS/hosts)  Some additional tests fail when run as non-root due to permissions issues.  I don't intend to fix the latter.

rpmlint errors are as before (see comment #27 and comment #28).
Comment 35 Andrew McNabb 2009-10-28 00:13:23 EDT
Wow.  You've made tremendous progress.  The remaining rpmlint warnings/errors seem to all be false positives, and it looks like it's passing almost all of the regression tests.  You mentioned why "test_socket" is failing, but do you know why "test_httpservers" is failing?

What's the justification for removing the shebang lines from .py files that
aren't executable?

In Comment #26, I mentioned a few changes made in the spec file that look upstreamable.  Although it's doesn't seem like a high priority issue, upstreaming would probably declutter the spec file a bit.

Whenever you think everything looks good, I'd be happy to do some more testing and come up with any final suggestions.  However, I'm not an official reviewer.  Do we have anyone lined up for the reviewer role?  I think the package is just about ready.
Comment 36 Ignacio Vazquez-Abrams 2009-10-28 01:46:45 EDT
I'm fine with switching it to whoever come the appropriate time.

As for the spec, it looks sane to me; I haven't really tried building it yet
due to lack of time, but I trust your observations. The only things I really
pick up on are style/cosmetic things, such as find -exec vs. find |
xargs; I think one should be picked and stuck with throughout the entire spec.
Comment 37 Andrew McNabb 2009-10-29 15:16:08 EDT
The current specfile puts %{pylibdir}/config/* into the devel subpackage.  However, distutils needs to load %{pylibdir}/config/Makefile (see the get_makefile_filename() function in distutils/sysconfig.py.  Since distutils is and should be in the main python3 package, then %{pylibdir}/config/Makefile also needs to be in the main python3 package instead of the devel subpackage.  Based on _init_posix() in distutils/sysconfig.py, it looks like this is also true for /usr/include/python2.6/pyconfig.h.  The only alternative would be to put distutils into the python3-devel subpackage, but that doesn't seem right at all.
Comment 38 Dave Malcolm 2009-10-29 23:23:43 EDT
(In reply to comment #37)
> The current specfile puts %{pylibdir}/config/* into the devel subpackage. 
> However, distutils needs to load %{pylibdir}/config/Makefile (see the
> get_makefile_filename() function in distutils/sysconfig.py.  Since distutils is
> and should be in the main python3 package, then %{pylibdir}/config/Makefile
> also needs to be in the main python3 package instead of the devel subpackage. 
> Based on _init_posix() in distutils/sysconfig.py, it looks like this is also
> true for /usr/include/python2.6/pyconfig.h.  The only alternative would be to
> put distutils into the python3-devel subpackage, but that doesn't seem right at
> all.  
(For reference, Andrew also filed a report about this for the main python package as bug 531901)

Thanks; I've fixed this in the latest version of the file:
Updated specfile: http://dmalcolm.fedorapeople.org/python3.spec
Updated SRPM: http://dmalcolm.fedorapeople.org/python3-3.1.1-7.fc11.src.rpm
Diff between 3.1.1-6 and 3.1.1-7:
http://dmalcolm.fedorapeople.org/python3-from-3.1.1-6-to-3.1.1-7.diff

This adds an additional rpmlint warning:

python3.i686: W: devel-file-in-non-devel-package /usr/include/python3.1/pyconfig-32.h

but clearly this is deliberate, to address the issue in comment #37/bug 531901
Comment 39 Toshio Ernie Kuratomi 2009-10-30 20:14:13 EDT
Python 3.* isn't very standard across packages.  Since we're saying that python2 and python3 are different language versions, I think we should use Python 3 (or Python3) in the descriptions.


We're unconditionally BuildRequireing openssl-devel so why are we conditionalizing this:
  if pkg-config openssl ; then
    export CFLAGS="$CFLAGS `pkg-config --cflags openssl`"
    export LDFLAGS="$LDFLAGS `pkg-config --libs-only-L openssl`"
  fi


We don't need this::
  [ -d $RPM_BUILD_ROOT ] && rm -fr $RPM_BUILD_ROOT


Change this::
  - mkdir -p $RPM_BUILD_ROOT/usr $RPM_BUILD_ROOT%{_mandir}
  + mkdir -p $RPM_BUILD_ROOT%{_prefix} $RPM_BUILD_ROOT%{_mandir}


Why do we still have this in the spec file?  there's more and more work being done on utilizing the information in the egg-info metadata so I don't think we can keep removing it and stay compatible with upstream's intentions::
  # Get rid of egg-info files (core python modules are installed through rpms)
  rm $RPM_BUILD_ROOT%{pylibdir}/*.egg-info


Unless there's a good reason we probably want to make this change::
  # Switch all shebangs to refer to the specific Python version.
  - LD_LIBRARY_PATH=. ./python Tools/scripts/pathfix.py -i "%{_bindir}/env python%{pybasever}" $RPM_BUILD_ROOT
  + LD_LIBRARY_PATH=. ./python Tools/scripts/pathfix.py -i "%{_bindir}/python%{pybasever}" $RPM_BUILD_ROOT


What files are affected here?
  # Remove shebang lines from .py files that aren't executable
I just glanced at python2.6 and saw several different kinds of files that this falls to:
  difflib.py - can run a unittest.. no harm done
  unittest.py - can be run as a script and do something useful
For find out which this is you have to check what the file does when run as a script.


.cvsignore removal shouldn't be needed anymore as upstream hasswitched to svn and then to hg.  remove lines like this::
  find $RPM_BUILD_ROOT/ -name ".cvsignore"|xargs rm -f
  find . -name ".cvsignore"|xargs rm -f


Fedora standard has %post and %postun before %files.


I'd like us to think about installing rpm macros like python_sitelib from this and the main python package so we don't have to add it as boilerplate to each spec file.
Comment 40 Toshio Ernie Kuratomi 2009-10-30 20:16:40 EDT
Clarification:
> We don't need this::
>  [ -d $RPM_BUILD_ROOT ] && rm -fr $RPM_BUILD_ROOT

Change it to::
  rm -fr $RPM_BUILD_ROOT

Also, we should probably do work on the python2 merge review as well.
Comment 41 Dave Malcolm 2009-11-03 13:56:50 EST
Thanks; I've addressed most of the above; here's the latest work-in-progress:

Updated specfile: http://dmalcolm.fedorapeople.org/python3.spec
Updated SRPM: http://dmalcolm.fedorapeople.org/python3-3.1.1-8.fc11.src.rpm
Diff between 3.1.1-7 and 3.1.1-8: 
http://dmalcolm.fedorapeople.org/python3-from-3.1.1-7-to-3.1.1-8.diff

rpmlint results are as before.

Still to do:
  (i) analysis of shebang files still 
  (ii) installation of rpm macros
  (iii) anything else I've missed

Re (ii), is there a standard way for a -devel package to drop macros into a directory (e.g. /usr/lib/rpm ) in such a way that rpm will automatically use them?  Is this acceptable practice?
Comment 42 Toshio Ernie Kuratomi 2009-11-03 20:24:50 EST
(In reply to comment #41)
> Re (ii), is there a standard way for a -devel package to drop macros into a
> directory (e.g. /usr/lib/rpm ) in such a way that rpm will automatically use
> them?  Is this acceptable practice?  

Yes.  Create the macros in a file named macros.python3 (and macros.python for the python2 macros) and drop them into %{_sysconfdir}/rpm/.

For instance, /etc/rpm/macros.perl in the perl-devel file.
Comment 43 Dave Malcolm 2009-11-04 12:49:18 EST
(In reply to comment #42)
> (In reply to comment #41)
> > Re (ii), is there a standard way for a -devel package to drop macros into a
> > directory (e.g. /usr/lib/rpm ) in such a way that rpm will automatically use
> > them?  Is this acceptable practice?  
> 
> Yes.  Create the macros in a file named macros.python3 (and macros.python for
> the python2 macros) and drop them into %{_sysconfdir}/rpm/.
> 
> For instance, /etc/rpm/macros.perl in the perl-devel file.  

Updated specfile: http://dmalcolm.fedorapeople.org/python3.spec
Updated SRPM: http://dmalcolm.fedorapeople.org/python3-3.1.1-9.fc11.src.rpm
Diff between 3.1.1-8 and 3.1.1-9: http://dmalcolm.fedorapeople.org/python3-from-3.1.1-8-to-3.1.1-9.diff

I've tested this with a sample specfile and it works.

One possible drawback is that the macro files are read (with rpmInitMacros) and expanded on every rpm operation, which means that when python3-devel is installed, every "rpm" invocation is going to be quietly invoking /usr/bin/python3 twice on startup.  None of the existing macro files seem to invoke subprocess during expansion.

Having said that, it only affects systems with python-devel installed, so I don't see this as a problem.

I'll look at doing something similar for the main "python-devel" package.

rpmlint output is as before (see comment #38)
Comment 44 Dave Malcolm 2009-11-04 12:50:08 EST
> Having said that, it only affects systems with python-devel installed, so I
"python3-devel", rather than "python-devel", obviously
Comment 45 Dave Malcolm 2009-11-04 13:09:39 EST
(In reply to comment #43)
> I'll look at doing something similar for the main "python-devel" package.
Filed as bug 533022
Comment 46 Dave Malcolm 2009-11-17 15:52:43 EST
I read through the history here, and tried to summarize the remaining review issues.

Here's what I think remains:
  - a reviewer needs to go through the full review guidelines on this
  - fixup the factual errors in the comment describing redefinition of __os_install_post
  - perhaps fixup rpm-build to avoid needing find-provides-without-python-sonames.sh
  - fixup macros.python3 to bake in the definitions, avoid invoking python3 each time
  - verify the script in comment #17 still works and that it verifies the 2 and 3 packages are independent
  - cosmetic issue: exec vs find | xargs noted in comment #36
  - what files are affected when modifying shebangs, and how (see commment #39)
  - anything else I've missed
Comment 47 Andrew McNabb 2009-11-17 16:44:32 EST
Created attachment 369966 [details]
patch to version 9 of the spec file
Comment 48 Andrew McNabb 2009-11-17 16:56:49 EST
(In reply to comment #46)
>   - fixup the factual errors in the comment describing redefinition of
> __os_install_post
>   - cosmetic issue: exec vs find | xargs noted in comment #36

These two issues should be fixed in python3-10.patch, which I just attached.

>   - what files are affected when modifying shebangs, and how (see commment #39)

I wouldn't worry too much about this problem.  The shebang doesn't actually do anything for a file without executable permissions, so removing the shebang line doesn't make it any less executable than it already was.
Comment 49 Andrew McNabb 2009-11-17 17:02:27 EST
(In reply to comment #48)
> 
> I wouldn't worry too much about this problem.  The shebang doesn't actually do
> anything for a file without executable permissions, so removing the shebang
> line doesn't make it any less executable than it already was.  

I forgot to mention that leaving the shebang lines in doesn't make a file any more executable than it already was.  It probably wouldn't hurt to just remove the shebang removal.
Comment 50 Dave Malcolm 2009-11-17 22:00:25 EST
(In reply to comment #47)
> Created an attachment (id=369966) [details]
> patch to version 9 of the spec file  

Thanks Andrew!

Updated specfile: http://dmalcolm.fedorapeople.org/python3.spec
Updated SRPM: http://dmalcolm.fedorapeople.org/python3-3.1.1-10.fc11.src.rpm
Diff: purely the changes from your attachment
rpmlint output as before
Comment 51 Rudolf Kastl 2009-11-18 03:23:28 EST
hmm i am curious why not call python3 -> python and the old version python-compat2.6 or whatever?
Comment 52 Dave Malcolm 2009-11-18 10:30:40 EST
(In reply to comment #51)
> hmm i am curious why not call python3 -> python and the old version
> python-compat2.6 or whatever?  
Python 3 is intended by upstream to be the future of Python, but we have many critical components that use Python 2. Python 2 and Python 3 are sufficiently different that we need both (try writing "print" in each). Python 2 will be around for a long time.

Changing the meaning of "python" to mean python 3 rather than python 2 in specfiles/yum/rpmdb would have a very high chance of breaking something during updates, and I don't see any real benefit.

Hence the plan is to continue to use "python-" to mean the existing python 2 stack, and "python3-" for the new parallel-installable python3 stack.

For more information see https://fedoraproject.org/wiki/Features/Python3F13
Comment 53 Thomas Spura 2009-11-18 16:17:35 EST
I can't get runtests.sh working...

When running, all tests are failing because of:
"./python: error while loading shared libraries: libpython3.1.so.1.0: cannot open shared object file: No such file or directory"

If this is fixed, you could add a %check section and run the tests there.
Comment 54 Dave Malcolm 2009-11-20 14:17:58 EST
(In reply to comment #53)
> I can't get runtests.sh working...
I'm not sure what you mean by "runtests.sh" here.


> When running, all tests are failing because of:
> "./python: error while loading shared libraries: libpython3.1.so.1.0: cannot
> open shared object file: No such file or directory"
> 
> If this is fixed, you could add a %check section and run the tests there.  

I think you need to prefix the invocation of "./python" with this:
  LD_LIBRARY_PATH=.
so that it can find the libpython library from the build.
Comment 55 Thomas Spura 2009-11-23 17:10:22 EST
Sorry for the delay:

You could add a %check section:

%check
LD_LIBRARY_PATH=. ./runtests.sh

This runs the python3 testsuite on each build to verify nothing is broken.

Currently here on my pc:
  18 BAD
 296 GOOD
  23 SKIPPED
 337 insgesamt
Comment 56 Dave Malcolm 2010-01-07 21:12:33 EST
Replying to comment #55:
Thanks!  I've added a %check to the specfile.  I used
  LD_LIBRARY_PATH=$(pwd)
rather than
  LD_LIBRARY_PATH=.
as I ran into problems that appeared to occur when a subprocess had changed working directory and could no longer find libpython.  See the diff below for more info.

Updated specfile: http://dmalcolm.fedorapeople.org/python3.spec
Updated SRPM: http://dmalcolm.fedorapeople.org/python3-3.1.1-11.fc12.src.rpm
Diff: http://dmalcolm.fedorapeople.org/python3-from-3.1.1-10-to-3.1.1.11.diff
rpmlint output as before (see comment #27, comment #28, and comment #38)
Koji scratch build into dist-f13 here: http://koji.fedoraproject.org/koji/taskinfo?taskID=1908523

Following up on comment #46:

Issues addressed in release 10:
>  - fixup the factual errors in the comment describing redefinition of
>__os_install_post
>  - cosmetic issue: exec vs find | xargs noted in comment #36

Remaining issues:
>  - a reviewer needs to go through the full review guidelines on this
TODO
>  - perhaps fixup rpm-build to avoid needing
> find-provides-without-python-sonames.sh
>  - fixup macros.python3 to bake in the definitions, avoid invoking python3
> each time
>  - verify the script in comment #17 still works and that it verifies the 2 and
> 3 packages are independent
>  - what files are affected when modifying shebangs, and how (see commment #39)
>  - anything else I've missed
Comment 57 Dave Malcolm 2010-01-09 12:28:03 EST
The remaining issues from comment #56:
>  - a reviewer needs to go through the full review guidelines on this
Looking for a volunteer here.

>  - perhaps fixup rpm-build to avoid needing
> find-provides-without-python-sonames.sh
Deferred: I don't think this is needed before package import.

>  - fixup macros.python3 to bake in the definitions, avoid invoking python3
> each time
Deferred: currently the script is arch-independent and goes in sysconfdir; it will lead to different results on 32-bit vs 64-bit archs.  It's not clear to me how to "bake in" the result at build time in a way that works in a cross-arch way.  Suggestions welcome.  So I plan to punt this for now: I don't think it's necessary to fix this to pass package review, something to be fixed after package import.

>  - verify the script in comment #17 still works and that it verifies the 2 and
> 3 packages are independent
I'm working on this

>  - what files are affected when modifying shebangs, and how (see commment #39)
I'm working on this

>  - anything else I've missed  
Does anyone have other concerns?
Comment 58 Dave Malcolm 2010-01-09 12:29:51 EST
> how to "bake in" the result at build time in a way that works in a cross-arch
"cross-arch" should read "multilib-safe" here
Comment 59 Thomas Spura 2010-01-09 13:08:06 EST
(In reply to comment #57)
> The remaining issues from comment #56:
> >  - a reviewer needs to go through the full review guidelines on this
> Looking for a volunteer here.

I'm unsure, if my practice in packaging is good enought to be sure, this won't break the build system, when you import it laterly...
This is the main reason, what's preventing me a bit from taking this ;-)
Going throught the review guidelines is not the problem at all...

> >  - anything else I've missed  
> Does anyone have other concerns?    

- BR tix tk not needed, because both -devel packages are in BR

- %global vs %define:
  https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define

- tools and tkinter contain tests -> should go into tests

- tools just contains *.py, but is installed in lib64.
  I don't see why atm... Otherwise this could be noarch.
  Probably because there is only one pylibdir.
  Is it worth adding? I don't think so... Would be great, but probably too complicated in new releases.

- Why do you force gcc?
Comment 60 Thomas Spura 2010-01-09 13:10:29 EST
And by the way: because you are not the requester of this review request, you need to open your own one and close this as dublicate.
Comment 61 Tim Lauridsen 2010-01-10 08:43:10 EST
I will give it a try, but it is a very complex .spec, so second opinions are welcome.
Comment 62 Tim Lauridsen 2010-01-10 08:44:07 EST
rpmlint output:

$ rpmlint python3-3.1.1-11.fc12.src.rpm 
python3.src: W: strange-permission find-provides-without-python-sonames.sh 0775
python3.src:220: E: hardcoded-library-path in $RPM_BUILD_ROOT/usr/lib/python%{pybasever}/site-packages
python3.src:416: E: hardcoded-library-path in /usr/lib/python%{pybasever}
python3.src:417: E: hardcoded-library-path in /usr/lib/python%{pybasever}/site-packages
1 packages and 0 specfiles checked; 3 errors, 1 warnings.

rpmlint ~/rpmbuild/RPMS/i686/python3-*
python3.i686: E: zero-length /usr/lib/python3.1/build_class.py
python3.i686: W: devel-file-in-non-devel-package /usr/include/python3.1/pyconfig-32.h
python3-libs.i686: W: no-documentation
python3-test.i686: W: no-documentation
python3-test.i686: E: zero-length /usr/lib/python3.1/test/nullcert.pem
python3-test.i686: W: uncompressed-zip /usr/lib/python3.1/test/zipdir.zip
python3-tkinter.i686: W: no-documentation
python3-tools.i686: W: file-not-utf8 /usr/lib/python3.1/Demo/distutils/test2to3/setup.py
python3-tools.i686: E: zero-length /usr/lib/python3.1/Tools/modulator/Templates/copyright
7 packages and 0 specfiles checked; 3 errors, 6 warnings.
Comment 63 Tim Lauridsen 2010-01-10 08:47:01 EST
python3.src: W: strange-permission find-provides-without-python-sonames.sh 0775

Should this be 755 ?

python3-tools.i686: W: file-not-utf8
/usr/lib/python3.1/Demo/distutils/test2to3/setup.py

python3.i686: W: devel-file-in-non-devel-package
/usr/include/python3.1/pyconfig-32.h

Please comment on these ones.
Comment 64 Tim Lauridsen 2010-01-10 08:48:25 EST
MUST:
* package is named according to the Package Naming Guidelines .
* spec file name match base package
* package meet Packaging Guidelines .
* package is licensed with a Fedora approved license and meet the Licensing Guidelines .
* License field match the actual license.
* available license(s) file(s) is included in %doc.
* spec file is written in American English. 
* spec file is legible. 
* sources match upstream (md5sum)
    d1ddd9f16e3c6a51c7208f33518cd674  Python-3.1.1.tar.bz2 (upstream)
    d1ddd9f16e3c6a51c7208f33518cd674  Python-3.1.1.tar.bz2 (srpm)
* package compile on x86
* build dependencies is listed in BuildRequires
* no locales
* available shared library files calls ldconfig in %post and %postun. 
* package not relocatable
* package own all directories that it creates.
* no duplicate files in the %files listing. 
* Permissions on files must be set properly. (%defattr(...) line)
* %clean section present and contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT)
* package is consistently using macros
* package contain code, or permissable content
* no large doc
* %doc does not affect runtime
? available Header files go into -devel package. 
    What about this one ?
        python3.i686: W: devel-file-in-non-devel-package /usr/include/python3.1/pyconfig-32.h
* no static libs
X available pkgconfig(.pc) files 'Requires: pkgconfig' 
    There is a buildrequire on pkgconfig, but no Requires:
    https://fedoraproject.org/wiki/Packaging/Guidelines#PkgconfigFiles
* no unversioned *.so.* libs
* available devel packages uses fully versioned dependency: Requires: %{name} = %{version}-%{release} 
* package dont contain .la libtool archives.
* not a GUI app.
* no files or directories already owned by other packages.
* %install begins with rm -rf %{buildroot} (or $RPM_BUILD_ROOT)
* filenames is valid UTF-8

SHOULD:
* source package include license text(s) in separate file from upstream
* builds in mock. 
    Not tried, but koji build has been done so it should be ok
* compile and build into binary rpms on all supported architectures. 
* package functions as described.
* scriptlets is sane.
* subpackages require the base package using a fully versioned dependency.
Comment 65 Tim Lauridsen 2010-01-10 08:50:56 EST
Fix/comment on the X/? and i will approve it, i nobody has any issues i have mixed
Comment 66 Tim Lauridsen 2010-01-10 10:28:26 EST
i nobody = if nobody :)

have mixed = have missed :)
Comment 67 Dave Malcolm 2010-01-11 18:42:54 EST
(In reply to comment #59)
> (In reply to comment #57)
> > The remaining issues from comment #56:
> > >  - a reviewer needs to go through the full review guidelines on this
> > Looking for a volunteer here.
> 
> I'm unsure, if my practice in packaging is good enought to be sure, this won't
> break the build system, when you import it laterly...
> This is the main reason, what's preventing me a bit from taking this ;-)
> Going throught the review guidelines is not the problem at all...

Bravo!  Thanks for the feedback!


> > >  - anything else I've missed  
> > Does anyone have other concerns?    
> 
> - BR tix tk not needed, because both -devel packages are in BR
I've removed these two.


> - %global vs %define:
>  
> https://fedoraproject.org/wiki/Packaging/Guidelines#.25global_preferred_over_.25define
Fixed; I did a search-and-replace throughout the specfile


> - tools and tkinter contain tests -> should go into tests
"rpm -qlv python3-tools|grep test" shows the majority of these files in these subidrectories:
- /usr/lib/python3.1/lib2to3/tests subdirectory
- /usr/lib/python3.1/Demo/distutils with just a test2to3 (the latter with many files)
- /usr/lib/python3.1/Demo/md5test
and a few other files

"rpm -qlv python3-tkinter|grep test" shows the tests in this subpackage are all below "/usr/lib/python3.1/tkinter/test"

In both cases I've moved the ones listed above into the "test" subpackage. I've added a requirement to the "test" subpackage for the "tools" subpackage due to the way the directory hierarchy is set up.


> - tools just contains *.py, but is installed in lib64.
>   I don't see why atm... Otherwise this could be noarch.
>   Probably because there is only one pylibdir.
>   Is it worth adding? I don't think so... Would be great, but probably too
> complicated in new releases.
This sounds like bug 543756.  We've lived with this for Python 2.  I'd prefer to leave dealing with this as an open RFE, rather than block package import on it.


> - Why do you force gcc?    
These lines:
  # Force CC
  export CC=gcc
came from Python 2's python.spec, and was introduced in this version
* Wed Nov 12 2003 Mihai Ibanescu <misa@redhat.com> 2.3.2-5
- force CC (#109268)
(way back in the Fedora Core 2 days)
https://bugzilla.redhat.com/show_bug.cgi?id=109268

If I'm reading that bug correctly, it appears that what happened was that the Makefile (generated by "configure" during the build) would include a reference to "x86_64-redhat-linux-gcc", rather than to "gcc".  This generated Makefile is shipped in the RPM (originally in the -devel subpackage, more recently in the main subpackage; see bug 531901) and is parsed by the "distutils" module.   It appears that a generated libtool script specialcased detection for "gcc" and couldn't handle such compiler names.

I've tried removing the lines, and it seems to work fine.  We can reinstate them if it causes trouble.   (Also, if in the future Unladen Swallow gets merged into Python 3.x, we may want to build Python 3 with LLVM's compiler instead of gcc).


Updated specfile: http://dmalcolm.fedorapeople.org/python3.spec
Updated SRPM: http://dmalcolm.fedorapeople.org/python3-3.1.1-10.fc11.src.rpm
Diff: http://dmalcolm.fedorapeople.org/python3-from-3.1.1-11-to-3.1.1-12.diff
rpmlint output as before (see comment #27, comment #28, and comment #38)
Successful scratch build here: http://koji.fedoraproject.org/koji/taskinfo?taskID=1915081
Comment 68 Dave Malcolm 2010-01-11 18:44:07 EST
(In reply to comment #61)
> I will give it a try, but it is a very complex .spec, so second opinions are
> welcome.    
Thanks!

(In reply to comment #63)
> python3.src: W: strange-permission find-provides-without-python-sonames.sh 0775
> 
> Should this be 755 ?
Fixed in -13
Updated specfile here: http://dmalcolm.fedorapeople.org/python3.spec
updated SRPM here: http://dmalcolm.fedorapeople.org/python3-3.1.1-13.fc12.src.rpm
Updated scratch build here: http://koji.fedoraproject.org/koji/taskinfo?taskID=1915305
No change to specfile other than to %changelog


> python3-tools.i686: W: file-not-utf8
> /usr/lib/python3.1/Demo/distutils/test2to3/setup.py
From comment #28: this file ("Demo/distutils/test2to3/setup.py") is an upstream test file for the 2to3 code, with a non-utf8 encoding.  One of the purposes of the 2to3 code is to do character set conversions on .py files, so having this file as non-utf8 verifies that the 2to3 tool is capable of doing this correctly.

 
> python3.i686: W: devel-file-in-non-devel-package
> /usr/include/python3.1/pyconfig-32.h
This is deliberate, as the file is actually used at run-time by the distutils package (as well as the more normal usage at build-time) - see comment #37, bug 531901, and upstream issue http://bugs.python.org/issue4359
Comment 69 Dave Malcolm 2010-01-11 19:04:19 EST
From comment #57
>  - verify the script in comment #17 still works and that it verifies the 2 and
> 3 packages are independent
I slightly reworked my script, to compare scratch builds.

"koki download-build" can't download scratch builds, as they only have a task_id, not a build_id.
mbonnet has a script for this:
  http://people.redhat.com/mikeb/scripts/download-scratch.py
and I use this to download all of the files into the PWD
Comment 70 Dave Malcolm 2010-01-11 19:05:15 EST
Created attachment 383125 [details]
Updated version of script to locate collisions between python2/python3
Comment 71 Dave Malcolm 2010-01-11 19:11:01 EST
The output of this script contains:
(i) the various /usr/lib/debug filesystem collisions described in comment #17, which aren't a problem
(ii) Provides: "python(abi)", which could be a problem

$ rpm -q --provides python | grep abi
python(abi) = 2.6
python-abi = 2.6

$ rpm -q --provides python3 | grep abi
python(abi) = 3.1

Both python and python3 provide "python(abi)", with different values.  Add-on Python rpms will have a Requires on "python(abi)", some requiring this to equal "2.6", others requiring it to equal "3.1"

Is this a problem?  IIRC I was able to install both sets of packages during my testing.

James: is this a no-no from the point of view of Yum?
Comment 72 James Antill 2010-01-11 23:32:04 EST
It's probably not been tested much in the wild, but it will almost certainly work. Just ignore the warning.
But don't do "obviously" insane stuff like:

Conflicts: python(abi) < 3.1

...because that has to be true for all providers.
Comment 73 James Antill 2010-01-11 23:46:39 EST
Also note:

% cat /tmp/python-abi-wtf.py
#! /usr/bin/python -tt

import yum

yb=yum.YumBase()
for pkg in sorted(yb.pkgSack.returnPackages()):
    for req in pkg.requires_print:
        if not req.startswith("python(abi)"): continue
        if req == "python(abi) = 2.6": continue
        print pkg.repo.id,pkg,req
% sudo python /tmp/python-abi-wtf.py
Loaded plugins: local, presto
fedora cobbler-2.0.0-1.fc12.noarch python(abi) >= 2.6
fedora cobbler-web-2.0.0-1.fc12.noarch python(abi) >= 2.6
rpmfusion-free compat-python24-imaging-1.1.6-4.fc11.x86_64 python(abi) = 2.4
rpmfusion-free compat-python24-libxml2-2.7.6-1.fc12.x86_64 python(abi) = 2.4
fedora koan-2.0.0-1.fc12.noarch python(abi) >= 2.6
fedora revisor-cli-2.1.8-1.fc12.noarch python(abi) >= 2.4
updates revisor-cli-2.1.10-3.fc12.noarch python(abi) >= 2.4
fedora ris-linux-0.4-6.fc12.noarch python(abi) >= 2.1

...what yum does with >= is much less well defined, but you'll probably be fine ... and most of the above are probably bugs anyway (Eg. revisor-cli also has a Requires: python(abi) = 2.6).
 But I'd save the above script and run it periodically to make sure no one does anything weird you aren't expecting (being a trail blazer and all :).
Comment 74 Tim Lauridsen 2010-01-12 01:03:08 EST
David:

What about this one ?

X available pkgconfig(.pc) files 'Requires: pkgconfig' 
    There is a buildrequire on pkgconfig, but no Requires:
    https://fedoraproject.org/wiki/Packaging/Guidelines#PkgconfigFiles
Comment 75 Panu Matilainen 2010-01-12 05:28:18 EST
Rpm adds pkgconfig dependencies automatically, no need to duplicate it manually.
Comment 76 Tim Lauridsen 2010-01-12 08:41:27 EST
OK, Fine with me, maybe this path of the guidelines is outdated.

APPROVED
Comment 77 Tim Lauridsen 2010-01-12 08:42:15 EST
path = part :)
Comment 78 Toshio Ernie Kuratomi 2010-01-12 10:14:10 EST
(In reply to comment #76)
> OK, Fine with me, maybe this path of the guidelines is outdated.
> 
It is.  http://fedoraproject.org/wiki/PackagingDrafts/PkgconfigAutoRequires

I think ratification by FESCo/FPC writeup has gotten lost in the holiday-vacation-no-meeting shuffle.  I'll look into it.
Comment 79 Toshio Ernie Kuratomi 2010-01-12 10:41:24 EST
Tim, thanks for taking on a large and complex review!

Something I noticed, can be changed after import:

%if "%{_lib}" == "lib64"
%attr(0755,root,root) %dir /usr/lib/python%{pybasever}
%attr(0755,root,root) %dir /usr/lib/python%{pybasever}/site-packages
%endif
[..]
%dir /usr/include/python%{pybasever}
/usr/include/python%{pybasever}/%{_pyconfig_h}

Those should be:
%attr(0755,root,root) %dir %{_prefix}/lib/python%{pybasever}
%attr(0755,root,root) %dir %{_prefix}/lib/python%{pybasever}/site-packages
%dir %{_includedir}/python%{pybasever}
%{_includedir}/python%{pybasever}/%{_pyconfig_h}
Comment 80 Dave Malcolm 2010-01-12 12:43:22 EST
Tim: thanks for reviewing this.

James: thanks for doublechecking the python(abi) question

Toshio: yes, I'll fix the issue in comment #79 after import

As discussed in:
https://bugzilla.redhat.com/show_bug.cgi?id=526126#c24
https://bugzilla.redhat.com/show_bug.cgi?id=526126#c60
I'm opening a fresh review in order to set myself as "Reporter", and I'll mark the old review as a duplicate of this one.
Comment 81 Dave Malcolm 2010-01-12 12:43:46 EST

*** This bug has been marked as a duplicate of bug 554799 ***
Comment 82 Dave Malcolm 2010-01-13 17:55:28 EST
The package review in bug 554799 is now done; the package is in CVS and I've built the package into dist-f13 (yay!)

If anyone wants to be added as a comaintainer etc, please let me know.

Thanks Andrew, Ignacio, James, Steve, Toshio, Tim, and everyone else for your work on this! (hope I didn't miss anyone)