Opened 4 years ago

Closed 2 years ago

#4244 enhancement closed fixed (fixed)

Allow running setup.py in Python 3

Reported by: loewis Owned by: thijs
Priority: normal Milestone: Python-3.x
Component: core Keywords: py3k
Cc: spiv, thijs, exarkun Branch: branches/setup-py3k-4244-2
(diff, github, buildbot, log)
Author: thijs, loewis Launchpad Bug:

Description (last modified by thijs)

The attached patch changes trunk to support running setup.py in Python 3.1 (tested on r77847). setup.py will not complete, as it will break when trying to compile extensions modules. I'll look into porting the extension modules next, however, that is likely independent from this change.

The changes fall into two parts:

  • adjust all code run by setup.py so that it works on both 2.x and 3.x unmodified
  • integrate with distribute's use_2to3 flag

The patch is intended for use with distribute in Python 3.x, usage of setuptools should continue to work in 2.x unmodified.

The 3.x changes can be summarized as follows:

  • replace file with open (#5785). This will work on all 2.x versions, and 3.x doesn't have the file builtin
  • provide an implementation for execfile in 3.x. 2to3 will also fix all execfile usage, however, a few cases of execfile are used during the setup process (#5129)
  • parenthetise print and exec
  • replace an unlink/EnvironmentError check in dist.py with a os.path.exists check. This, again, should work on all Python versions, whereas the except syntax doesn't work in 3.x.

The use_2to3 changes have two pieces:

  • activate use_2to3
  • disable a fixer that caused problems. I haven't checked 3.1.1; it may be that the problem only exists in the Python release31-maint branch.

Attachments (4)

setup3k.diff (4.5 KB) - added by loewis 4 years ago.
4244.feature (441 bytes) - added by loewis 4 years ago.
execfile.diff (1.5 KB) - added by loewis 4 years ago.
setup-4244.patch (6.3 KB) - added by thijs 2 years ago.

Download all attachments as: .zip

Change History (39)

Changed 4 years ago by loewis

comment:1 Changed 4 years ago by spiv

  • Cc spiv added
  • Keywords review added

Changed 4 years ago by loewis

comment:2 Changed 4 years ago by loewis

  • Owner glyph deleted

comment:3 Changed 4 years ago by thijs

  • Cc thijs added
  • Keywords review removed
  • Milestone set to Python-3.x
  • Owner set to thijs
  • Status changed from new to assigned

I'll put the patch in a branch. #4246 is blocked by this ticket.

comment:4 Changed 4 years ago by thijs

  • Author set to thijs
  • Branch set to branches/setup-py3k-4244

(In [28148]) Branching to 'setup-py3k-4244'

comment:5 Changed 4 years ago by thijs

(In [28149]) Apply setup3k.diff by loewis, refs #4244

comment:6 Changed 4 years ago by thijs

  • Author changed from thijs to loewis, thijs

Running trial shows everything's fine on py2.6.4:

PASSED (skips=889, expectedFailures=18, successes=5179)

And this works as well:

$ python3.1 setup.py --version
9.0.0+r28148

Where it threw this error before:

$ python3.1 setup.py --version
  File "setup.py", line 87
    """
      ^
SyntaxError: invalid syntax

Will apply some coding standards now and put it back up for review.

comment:7 Changed 4 years ago by thijs

(In [28150]) Apply coding standard, refs #4244

comment:8 Changed 4 years ago by thijs

(In [28151]) Remove dedicated compat module for py3k, refs #4244

comment:9 Changed 4 years ago by thijs

Will now test the code from #4246 against this branch, but also putting this ticket back up for review. If that code fails to build against this branch I'll assign this ticket back to loewis.

comment:10 Changed 4 years ago by thijs

  • Owner changed from thijs to loewis
  • Status changed from assigned to new

Well, I didn't get as far as testing the code from #4246 because of this:

$ trial --help
Traceback (most recent call last):
  File "/Users/thijstriemstra/Sites/Twisted/branches/setup-py3k-4244/bin/trial", line 21, in <module>
    from twisted.scripts.trial import run
  File "/Users/thijstriemstra/Sites/Twisted/branches/setup-py3k-4244/twisted/__init__.py", line 18, in <module>
    from twisted.python import compat
  File "/Users/thijstriemstra/Sites/Twisted/branches/setup-py3k-4244/twisted/python/compat.py", line 19
    return exec(compile(open(filename).read(), filename, 'exec'), *args)
              ^
SyntaxError: invalid syntax

Do you have a fix for this loewis? :)

comment:11 Changed 4 years ago by thijs

(In [28152]) Add news file, refs #4244

comment:12 follow-up: Changed 4 years ago by loewis

I was originally proposing to have a separate compat file for code that is syntactically invalid 2.x code, and only meant for use in 3.x. This could then get conditionally imported. The specific problem is that the exec statement would consider parenthesis to start a tuple, and then dislike an asterisk. In addition, "return exec" is incorrect syntax in 2.x, but execfile should return None, anyway.

If you don't want to create such a file, here is a work-around that arranges execfile to be part of compat.py directly. If that approach is followed, I'd recommend to always define execfile in compat; then the conditional imports can go away. I've added these changes also to the patch.

Changed 4 years ago by loewis

comment:13 Changed 4 years ago by exarkun

  • Keywords review added
  • Owner changed from loewis to thijs

comment:14 Changed 4 years ago by thijs

(In [28234]) Apply execfile.diff, refs #4244

comment:15 Changed 4 years ago by thijs

  • Owner thijs deleted

Thanks loewis! Trial's happy now:

PASSED (skips=889, expectedFailures=18, successes=5179)

Leaving it in review for someone to check out the changes, but it's fine by me to merge this. Will check how #4246 works with this branch.

comment:16 Changed 4 years ago by exarkun

  • Cc exarkun added
  • Keywords review removed
  • Owner set to thijs
  1. It looks like twisted.python.compat should have execfile added to its __all__.
  2. Also, I guess our convention for snarfing builtins is name = name rather than from __builtin__ import name, so we may as well try to be consistent there.
  3. I'm not sure about the distribute-related stuff in setup.py. It seems like this is an extra feature, not directly required for basic 3.1 support in setup.py (although I can see why it is a desirable feature). It might be better to split this off into a separate ticket, because I think I'm inclined to block it until we get a slave set up that exercises it.

comment:17 Changed 4 years ago by exarkun

Also, I think we should go with .misc files for 3.x stuff until we get to the point where Twisted does something useful on 3.x.

comment:18 Changed 4 years ago by loewis

IMO, there is little point in being able to run setup.py on 3.x if the resulting installation hasn't been run through 2to3 - the resulting code *will* fail to do anything useful. If the dependency on distribute is a concern, I could try rewriting the patch to work on plain distutils as shipped with 3.1; much of the functionality is already available. Please let me know whether that's desirable.

As for the additional buildslave - unless #4246, #4247, #4248 are also accepted, a build under 3.x will break (i.e. not complete). In that sense, the extension modules aren't truly optional, as a failure to build them will cause the entire build to abort.

comment:19 follow-up: Changed 4 years ago by exarkun

If the dependency on distribute is a concern, I could try rewriting the patch to work on plain distutils as shipped with 3.1; much of the functionality is already available. Please let me know whether that's desirable.

It sounds like that might not be the best use of time. And I have no objection to using distribute per se, I just want to make sure all the code we're accepting is tested. :) I'll see what I can do about finding a machine where we can have a builder run setup.py with distribute so that the code actually gets exercised.

comment:20 in reply to: ↑ 19 ; follow-ups: Changed 4 years ago by thijs

Replying to exarkun:

I'll see what I can do about finding a machine where we can have a builder run setup.py with distribute so that the code actually gets exercised.

Feel free to use the 'thijs-bot' slave for that.

comment:21 Changed 4 years ago by loewis

I can also host a 3.x build slave if desired.

comment:22 in reply to: ↑ 20 Changed 4 years ago by thijs

Replying to thijs:

Replying to exarkun:

I'll see what I can do about finding a machine where we can have a builder run setup.py with distribute so that the code actually gets exercised.

Feel free to use the 'thijs-bot' slave for that.

I've installed distribute-0.6.10 on that build slave.

comment:23 in reply to: ↑ 20 Changed 4 years ago by thijs

Replying to thijs:

Feel free to use the 'thijs-bot' slave for that.

I'm currently not able to provide a buildslave, looks like this ticket is stuck on that?

comment:24 Changed 3 years ago by <automation>

  • Owner thijs deleted

comment:25 in reply to: ↑ 12 Changed 3 years ago by thijs

Replying to loewis:

I was originally proposing to have a separate compat file for code that is syntactically invalid 2.x code, and only meant for use in 3.x. This could then get conditionally imported. The specific problem is that the exec statement would consider parenthesis to start a tuple, and then dislike an asterisk. In addition, "return exec" is incorrect syntax in 2.x, but execfile should return None, anyway.

If you don't want to create such a file, here is a work-around that arranges execfile to be part of compat.py directly. If that approach is followed, I'd recommend to always define execfile in compat; then the conditional imports can go away. I've added these changes also to the patch.

I created a separate ticket for execfile at #5129. This ticket could focus on the setuptools/distribute/2to3 fixes in setup.py that 'allow running setup.py in Python 3.1'.

comment:26 Changed 2 years ago by thijs

  • Description modified (diff)

comment:27 in reply to: ↑ description Changed 2 years ago by thijs

  • Description modified (diff)
  • Summary changed from Allow running setup.py in Python 3.1 to Allow running setup.py in Python 3

Replying to loewis:

  • replace file with open. This will work on all 2.x versions, and 3.x doesn't have the file builtin

I've opened #5785 for this case in twisted/python/versions.py

  • parenthetise print and exec

There's one of those in setup.py that can be fixed in this ticket.

  • replace an unlink/EnvironmentError check in dist.py with a os.path.exists check. This, again, should work on all Python versions, whereas the except syntax doesn't work in 3.x.

This can also be fixed in this ticket, but there is no code coverage for build_scripts_twisted class so this will need test(s).

When you fix those points manually setup.py can be run in Python 3.2! Getting there..

$ python3.2 --version
12.1.0+r34835

$ python3.2 setup.py build
...
twisted/internet/_sigchld.c:101:1: warning: control reaches end of non-void function [-Wreturn-type]
gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-Bsymbolic-functions -Wl,-z,relro build/temp.linux-x86_64-3.2/twisted/internet/_sigchld.o -o build/lib.linux-x86_64-3.2/twisted/internet/_sigchld.cpython-32mu.so
running build_scripts
creating build/scripts-3.2
copying and adjusting bin/trial -> build/scripts-3.2
copying and adjusting bin/tap2deb -> build/scripts-3.2
copying and adjusting bin/pyhtmlizer -> build/scripts-3.2
copying and adjusting bin/tap2rpm -> build/scripts-3.2
copying and adjusting bin/manhole -> build/scripts-3.2
copying and adjusting bin/twistd -> build/scripts-3.2
copying and adjusting bin/tapconvert -> build/scripts-3.2
copying and adjusting bin/mail/mailmail -> build/scripts-3.2
copying and adjusting bin/conch/tkconch -> build/scripts-3.2
copying and adjusting bin/conch/ckeygen -> build/scripts-3.2
copying and adjusting bin/conch/conch -> build/scripts-3.2
copying and adjusting bin/conch/cftp -> build/scripts-3.2
copying and adjusting bin/lore/lore -> build/scripts-3.2
changing mode of build/scripts-3.2/trial from 664 to 775
changing mode of build/scripts-3.2/tap2deb from 664 to 775
changing mode of build/scripts-3.2/pyhtmlizer from 664 to 775
changing mode of build/scripts-3.2/tap2rpm from 664 to 775
changing mode of build/scripts-3.2/manhole from 664 to 775
changing mode of build/scripts-3.2/twistd from 664 to 775
changing mode of build/scripts-3.2/tapconvert from 664 to 775
changing mode of build/scripts-3.2/mailmail from 664 to 775
changing mode of build/scripts-3.2/tkconch from 664 to 775
changing mode of build/scripts-3.2/ckeygen from 664 to 775
changing mode of build/scripts-3.2/conch from 664 to 775
changing mode of build/scripts-3.2/cftp from 664 to 775
changing mode of build/scripts-3.2/lore from 664 to 775

Changed 2 years ago by thijs

comment:28 Changed 2 years ago by thijs

  • Keywords review added

Attached patch adds tests for build_scripts_twisted, fixes the print() in setup.py and the exception handling in dist.py. When both this ticket and #5785 land it's possible to build twisted in py3k, running trial is the next step..

comment:29 follow-up: Changed 2 years ago by antoine

This was already in the old code, but:

if e.args[1] == 'No such file or directory'

is ugly. One should really use "e.errno == errno.ENOENT" instead. Also, the code there is buggy: it always passes and never re-raises the exception.

In the tests, instead of doing:

    f = open(...)
    try:
        # things with f
    finally:
        f.close()

You could do the more idiomatic (and shorter):

    with open(...) as f:
        # things with f

comment:30 Changed 2 years ago by thijs

  • Author changed from loewis, thijs to thijs, loewis
  • Branch changed from branches/setup-py3k-4244 to branches/setup-py3k-4244-2

(In [34858]) Branching to 'setup-py3k-4244-2'

comment:31 Changed 2 years ago by thijs

(In [34861]) Address review comments, add news file. refs #4244

comment:32 in reply to: ↑ 29 Changed 2 years ago by thijs

Thanks for the review.

Replying to antoine:

This was already in the old code, but:

if e.args[1] == 'No such file or directory'

is ugly. One should really use "e.errno == errno.ENOENT" instead. Also, the code there is buggy: it always passes and never re-raises the exception.

I went with the reporter's approach in setup3k.diff, using os.path.exists..

In the tests, instead of doing:
..
You could do the more idiomatic (and shorter):

Fixed.

comment:33 Changed 2 years ago by antoine

  • Keywords review removed
  • Owner set to thijs

I haven't run the tests, but the latest diff looks good enough to me.

comment:34 Changed 2 years ago by thijs

  • Status changed from new to assigned

Thanks for the review. build results.

comment:35 Changed 2 years ago by thijs

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [34959]) Merge setup-py3k-4244-2: Make the print statement in setup.py compatible for Python 2.6/3.0

Author: loewis, thijs
Reviewer: thijs, exarkun, antoine
Fixes: #4244

This also adds additional test coverage for twisted.python.dist.

Note: See TracTickets for help on using tickets.