Edgewall Software
Modify

Opened 4 years ago

Last modified 2 months ago

#12130 new enhancement

Add Python 3 compatibility using six

Reported by: Tim Graham <timograham@…> Owned by:
Priority: normal Milestone: next-dev-1.5.x
Component: general Version:
Severity: normal Keywords: python3
Cc: Jun Omae, gabriele.svelto@…, t17-0by@… Branch: log:rjollos.git@t12130_python3.1
Release Notes:
API Changes:

Description

As discussed on trac-dev, I'll implement Python 3 compatibility using the six library. There is also #10083 "Make Trac work with Python 3", but Ryan suggested a separate ticket to track my work.

Attachments (0)

Change History (61)

comment:1 by Tim Graham <timograham@…>, 4 years ago

Cleanups that don't require six (commits in this branch be merged ASAP): https://github.com/timgraham/trac/pull/1

Commits that require six (probably best to wait to merge this branch until the test suite is passing on Python 3): https://github.com/timgraham/trac/pull/2

comment:2 by Ryan J Ollos, 4 years ago

I have no direct experience with six, but it makes sense to me that we should use it. The main thing I would keep in mind is to organize the imports in a way that it will be easy to remove the dependency on six in the future, when we drop Python 2.7 support.

comment:3 by Ryan J Ollos, 4 years ago

Milestone: 1.2

comment:4 by Tim Graham <timograham@…>, 4 years ago

I didn't see anything explicit in the style guide, but the order of imports seems to be [builtins] [third-party] [internal] with import ... sorted before from ... import.

For example, would this be correct (under the third-party section):

import six
from genshi.builder import tag, Element
from six import StringIO

in reply to:  4 comment:5 by Ryan J Ollos, 4 years ago

Replying to Tim Graham <timograham@…>:

I didn't see anything explicit in the style guide, but the order of imports seems to be [builtins] [third-party] [internal] with import ... sorted before from ... import.

Yeah, we follow PEP-0008. Although you'll probably find a bit of code that doesn't comply.

For example, would this be correct (under the third-party section):

import six
from genshi.builder import tag, Element
from six import StringIO

That's how I'd order them, but I don't think we have been explicit about that anywhere and you'll see some variation in the codebase. I say, go with that unless anyone says differently in this ticket.

comment:6 by Jun Omae, 4 years ago

Cc: Jun Omae added

Updated https://github.com/jun66j5/trac/commits/python3 branch (rebased on current trunk and reworked with six module).

  • initenv works well.
  • tracd doesn't work.
    AttributeError: 'HTTPMessage' object has no attribute 'typeheader'
    
  • unit tests can run on Python 2.6, 2.7 and 3.3.
    Python: /home/jun66j5/venv/py33/bin/python
    
      Package        Version
      ------------------------------------------------------------------
      Python       : 3.3.6 (default, Jan 28 2015, 16:27:12)
      [GCC 4.6.3]
      Setuptools   : 3.5.1
      Genshi       : 0.8
      Babel        : not installed
      sqlite3      : 2.6.0
      PySqlite     : not installed
      MySQLdb      : not installed
      Psycopg2     : not installed
      SVN bindings : not installed
      Mercurial    : not installed
      Pygments     : 2.0.2
      Pytz         : 2015.4
      Docutils     : not installed
      Twill        : not installed
      LXML         : not installed
      coverage     : 3.7.1
      figleaf      : not installed
    
    ...
    
    FAILED (failures=182, errors=128)
    SKIP: utils/tests/datefmt.py (no babel installed)
    SKIP: tracopt/versioncontrol/svn/tests/svn_fs.py (no svn bindings)
    make: *** [unit-test] Error 1
    

comment:7 by Tim Graham <timograham@…>, 4 years ago

Should I abandon work on this then?

in reply to:  7 comment:8 by Ryan J Ollos, 4 years ago

Replying to Tim Graham <timograham@…>:

Should I abandon work on this then?

Or just base any future changes on the latest work that Jun has done.

comment:9 by Jun Omae, 4 years ago

Updated https://github.com/jun66j5/trac/commits/python3.

  • Unit tests:
    • FAILED (failures=2) on Python 3.3 with sqlite3 and psycopg2.
        Package        Version
        ------------------------------------------------------------------
        Python       : 3.3.6 (default, Jan 28 2015, 16:27:12)
        [GCC 4.6.3]
        Setuptools   : 3.5.1
        Genshi       : 0.8
        Babel        : not installed
        sqlite3      : 2.6.0
        PySqlite     : not installed
        MySQLdb      : not installed
        Psycopg2     : 2.6.1 (dt dec pq3 ext)
        SVN bindings : not installed
        Mercurial    : not installed
        Pygments     : 2.0.2
        Pytz         : 2015.4
        Docutils     : 0.12
        Twill        : not installed
        LXML         : 3.4.4
        coverage     : 3.7.1
        figleaf      : not installed
      ...
      Ran 1719 tests in 22.166s
      
      FAILED (failures=2)
      make: *** [unit-test] Error 1
      
    • Unit and functional tests pass on Python 2.6.
  • tracd works well with digest authentication.
  • Git repository probably works.

Known issues:

  • Babel leads RuntimeError: maximum recursion depth exceeded. I have no idea to fix it.
    ....
      File "/venv/py33/lib/python3.3/site-packages/babel/support.py", line 252, in __getattr__
        return getattr(self.value, name)
      File "/venv/py33/lib/python3.3/site-packages/babel/support.py", line 182, in value
        value = self._func(*self._args, **self._kwargs)
      File "trac/util/translation.py", line 218, in _ngettext
        trans = self.active.ungettext(singular, plural, num)
    RuntimeError: maximum recursion depth exceeded
    make: *** [unit-test] Error 1
    

in reply to:  9 comment:10 by Jun Omae, 4 years ago

Known issues:

  • Babel leads RuntimeError: maximum recursion depth exceeded. I have no idea to fix it.

I noticed that ugettext and ungettext methods have been removed from gettext.NullTranslations since Python 3. The issue has been fixed.

Last edited 4 years ago by Jun Omae (previous) (diff)

comment:11 by Ryan J Ollos, 4 years ago

Milestone: 1.21.3.1

comment:12 by jim.amberger@…, 4 years ago

I apologize for not being able to follow this conversation but may I inquire as to the status of trac on python3? Is there a working branch I can clone that basically works?

Thanks for your work on this.

comment:13 by Ryan J Ollos, 4 years ago

I doubt the branch discussed in comment:6 is suitable for use in production by even a very proficient Python developer. What's your motivation for running with Python 3?

comment:14 by Jun Omae, 4 years ago

Rebased https://github.com/jun66j5/trac/commits/python3 with current trunk (r14555). Unit tests pass with SQLite, PostgreSQL and MySQL (using pymysql).

in reply to:  14 ; comment:15 by Ryan J Ollos, 4 years ago

Replying to Jun Omae:

Rebased https://github.com/jun66j5/trac/commits/python3 with current trunk (r14555). Unit tests pass with SQLite, PostgreSQL and MySQL (using pymysql).

Nice. Do you have a general idea of when the changes would be committed? Do you think they are close enough that they might be pushed in milestone:1.3.1, or are there major outstanding issues?

in reply to:  13 comment:16 by jim.amberger@…, 4 years ago

My motivation is wanting to use mod-wsgi3 for everything I host rather than go through the configuration headaches of running python2 and python3 applications on the same machine/apache2 install. I will clone Jun Omae's repo in comment:14 and see what happens.

in reply to:  15 ; comment:17 by Jun Omae, 4 years ago

Replying to Ryan J Ollos:

Replying to Jun Omae:

Rebased https://github.com/jun66j5/trac/commits/python3 with current trunk (r14555). Unit tests pass with SQLite, PostgreSQL and MySQL (using pymysql).

Nice. Do you have a general idea of when the changes would be committed? Do you think they are close enough that they might be pushed in milestone:1.3.1, …

Yeah. I think we are ready to push those changes in 1.3.1.

General idea is that we could push with three steps:

  1. 64d9bd475...d1e22ca17: Add Python 3 compatibilities without six module while keeping unit tests pass on Python 2.
  2. 28a607ccc...c7c238121: Add six module to the dependencies. Add Python 3 compatibilities using six module while keeping unit tests pass on Python 2. As the result, it would be able to run unit tests on Python 3 with many failures.
  3. 493b9ccd9...e6d782ad3: Remove test failures on Python 2 and 3 until zero failures.

, or are there major outstanding issues?

There might be good solutions for the following incompatibilities.

  1. dict iterates items with different order between Python 2 and 3. It makes difficult to write unit tests between 2 and 3, for especially Genshi rendering. E.g. 74884b16d and 938118032.
  2. file instance with text mode on Python 3 reads and writes unicode strings. However, bytes strings on Python 2. We could use io.open for text mode on Python 2. In the branch, uses of open and fdopen with text mode are still remaining. E.g. f4e36082c.

in reply to:  17 comment:18 by Christian Boos, 4 years ago

Replying to Jun Omae:

Replying to Ryan J Ollos:

Replying to Jun Omae:

Rebased https://github.com/jun66j5/trac/commits/python3 with current trunk (r14555). Unit tests pass with SQLite, PostgreSQL and MySQL (using pymysql).

Nice. Do you have a general idea of when the changes would be committed? Do you think they are close enough that they might be pushed in milestone:1.3.1, …

Yeah. I think we are ready to push those changes in 1.3.1.

I think Ryan also has a long standing series mostly ready to push on 1.3.1 (#11901), probably that should land first as it will mostly remove stuff we won't have then to convert.

General idea is that we could push with three steps:

General impression, great work!

  1. 64d9bd475...d1e22ca17: Add Python 3 compatibilities without six module while keeping unit tests pass on Python 2.
  1. why not just:
    try:
        get_thread_id = threading.get_ident  # since Python 3.3
    except AttributeError:
        get_thread_id = threading._get_ident
    
  1. there's an import six in trac/versioncontrol/web_ui/log.py
  1. 28a607ccc...c7c238121: Add six module to the dependencies. Add Python 3 compatibilities using six module while keeping unit tests pass on Python 2. As the result, it would be able to run unit tests on Python 3 with many failures.
  • in trac/db/sqlite_backend.py, there's a six.iteritems but not import six

I wonder if it wouldn't be better looking to use from six import even for the iteritems etc.

I see you removed filter(None,...), but filter seems to be still there in 3.4, is it deprecated somehow? Also, by the way, which 3.x version are we targeting? 3.3?

  1. 493b9ccd9...e6d782ad3: Remove test failures on Python 2 and 3 until zero failures.
  • with the jinja2 branch, the attributes will be sorted, so no worries for the tests in this area.
  • lots of the __repr__ methods look alike (with the class name, some value and the value[value.startswith('u'):] trick); maybe we can come up with a helper function
  • all in all, it looks that it will require some practice to get the proper usage of bytes/b''/to_utf8/io.BytesIO vs. unicode/u''/to_unicode/io.StringIO, maybe some guidelines in our TracDev/CodingStyle will help (e.g. hashlib expects bytes, most of email also)

, or are there major outstanding issues?

There might be good solutions for the following incompatibilities.

  1. dict iterates items with different order between Python 2 and 3. It makes difficult to write unit tests between 2 and 3, for especially Genshi rendering. E.g. 74884b16d and 938118032.

Yes, see above, and [a5d0cf70/cboos.git].

  1. file instance with text mode on Python 3 reads and writes unicode strings. However, bytes strings on Python 2. We could use io.open for text mode on Python 2. In the branch, uses of open and fdopen with text mode are still remaining. E.g. f4e36082c.

comment:19 by Ryan J Ollos, 3 years ago

Ubuntu 16.04 is now packaging a fork of MySQL-python (TracDev/ApiChanges/1.3@19):

# apt search python-mysql
Sorting... Done
Full Text Search... Done
bibus/xenial,xenial 1.5.2-4 all
  bibliographic database

python-mysql.connector/xenial,xenial 2.0.4-1 all
  pure Python implementation of MySQL Client/Server protocol

python-mysqldb/xenial 1.3.7-1build2 amd64
  Python interface to MySQL

python-mysqldb-dbg/xenial 1.3.7-1build2 amd64
  Python interface to MySQL (debug extension)

We might want to add mysqlclient to our test matrix in 1.3.x, unless we plan to switch to supporting only pymysql.

comment:20 by Christian Boos, 3 years ago

Milestone: 1.3.11.3.3

I think we should address this after the Jinja2 merge (1.3.2):

  • less work to do with the tests w.r.t. dict ordering differences (comment:18)
  • no conversion needed for the Python code in the templates, as the expressions in Jinja2 are actually not Python, simply "Python-like" and obey to the same syntax regardless of the actual version of Python being used

comment:21 by Ryan J Ollos, 3 years ago

I'm working on rebasing Jun's changes on the latest trunk, which is non-trivial due to all the changes (#11901 in particular). I've started pushing some of the obvious changes that appear appropriate for Python 2.7: [15326-15335]. I'll post most of the rest, including all that depend on the six module, for additional review and testing.

Last edited 3 years ago by Ryan J Ollos (previous) (diff)

comment:22 by Christian Boos, 3 years ago

If I may, I know I've been somehow slow on rebasing my Jinja2 changes, but now that I've pushed my CSS changes and mostly finished with the t.e.o "clean-up", I'm going to tackle that now. And there are some parts of the code that have changed a lot (e.g. trac/util/html.py), or are no longer necessary (all the code in the Genshi template). So maybe at least don't bother fixing the Genshi templates, as the Python code there will just go away.

OTOH, the changes you committed so far all look nice to have!

Last edited 3 years ago by Christian Boos (previous) (diff)

comment:23 by Ryan J Ollos, 3 years ago

Okay, I'll only push changes to py files, and also defer the changes that depend on six.

comment:24 by Ryan J Ollos, 3 years ago

Additional changes in [15336-15337].

in reply to:  21 comment:25 by Ryan J Ollos, 3 years ago

Replying to Ryan J Ollos:

I'm working on rebasing Jun's changes on the latest trunk, which is non-trivial due to all the changes (#11901 in particular).

Now rebasing on the latest trunk with Jinja2, so a few more days.

comment:26 by Ryan J Ollos, 3 years ago

The get_all_columns refactoring from r15332 was applied to 1.2-stable in r15590, merged to trunk in r15591.

comment:27 by Ryan J Ollos, 3 years ago

Additional changes in [15804:15813].

comment:28 by Ryan J Ollos, 3 years ago

I've rebased the changes in log:rjollos.git:python3.2. There are a few test failures for Python 2.7 (due to [93029820/rjollos.git]). There are many test failures with Python 3.5+. Feel free to fork and propose additional changes. I'll keep rebasing this against the trunk and post rebased changes every few weeks or so (like in #11901).

I've squashes the original changes into two primary changesets:

  1. [21dabbf6/rjollos.git]: refactoring that doesn't depend on six
  2. [352d3af3/rjollos.git]: changes that use six

If we can get all the tests passing, I was thinking to push (1) much sooner than (2) (see also comment:17). I could keep rebasing (2) to be sure the tests pass, and push that in milestone 1.3.3 or 1.3.4.

We should update the AppVeyor and TravisCI configs in the branch to run tests with pymysql and Python 3.5, 3.6. I'm unsure what we are going to do for SVN bindings in Python3 (comment:32:ticket:10083 and comment:36:ticket:10083). It also appears there is no twill for Python3, which may necessitate work on #11988, long overdue anyway.

Last edited 3 years ago by Ryan J Ollos (previous) (diff)

comment:29 by Ryan J Ollos, 3 years ago

#10083 closed as a duplicate, but some the ticket has some useful information.

in reply to:  29 comment:30 by ilewismsl, 3 years ago

Replying to Ryan J Ollos:

#10083 closed as a duplicate, but some the ticket has some useful information.

Not sure whether I am missing something, but your note here says that you closed #10083 as duplicate, but it is still open.

comment:31 by Gabriele Svelto <gabriele.svelto@…>, 3 years ago

Cc: gabriele.svelto@… added

comment:32 by Ryan J Ollos, 3 years ago

Regression in r15812 reported and fixed in #12800.

comment:33 by Ryan J Ollos, 2 years ago

Addition of pymysql will be considered in #12821. I'll post a rebase of comment:28 changes after changes in #12821 are committed.

Last edited 2 years ago by Ryan J Ollos (previous) (diff)

in reply to:  33 comment:34 by Ryan J Ollos, 2 years ago

Replying to Ryan J Ollos:

Addition of pymysql will be considered in #12821. I'll post a rebase of comment:28 changes after changes in #12821 are committed.

log:rjollos.git:t12130_python3. Test passing on Python 2.7. Needs work for Python 3.5+.

comment:35 by Ryan J Ollos, 2 years ago

Milestone: 1.3.31.5.1

comment:36 by Ryan J Ollos, 2 years ago

comment:37 by Thomas Lae <t17-0by@…>, 2 years ago

Cc: t17-0by@… added

comment:38 by Tim Graham <timograham@…>, 18 months ago

DONE PR#12 to remove usage of Python's deprecated dircache module (removed in Python 3).

Last edited 18 months ago by Ryan J Ollos (previous) (diff)

comment:39 by Tim Graham <timograham@…>, 18 months ago

DONE PR#15 to remove use of method argument unpacking.

DONE PR#16 to remove ur string literals.

Last edited 18 months ago by Ryan J Ollos (previous) (diff)

comment:40 by Tim Graham <timograham@…>, 18 months ago

DONE PR#17 to make some exception catching Python 3 compatible.

Last edited 18 months ago by Ryan J Ollos (previous) (diff)

in reply to:  38 ; comment:41 by Jun Omae, 18 months ago

Replying to Tim Graham <timograham@…>:

PR to remove usage of Python's deprecated dircache module (removed in Python 3).

Thanks. However, the same changes are already in [644892e1e/rjollos.git] of rjollos.git@t12130_python3.1 branch.

Last edited 18 months ago by Jun Omae (previous) (diff)

in reply to:  40 comment:42 by Jun Omae, 18 months ago

Replying to Tim Graham <timograham@…>:

PR#17 to make some exception catching Python 3 compatible.

Fixed in [16608-16609] on 1.2-stable and trunk. Thanks.

in reply to:  41 ; comment:43 by Tim Graham <timograham@…>, 18 months ago

Replying to Jun Omae:

Replying to Tim Graham <timograham@…>:

PR to remove usage of Python's deprecated dircache module (removed in Python 3).

Thanks. However, the same changes are already in [644892e1e/rjollos.git] of rjollos.git@t12130_python3.1 branch.

I saw that, but I think my PR is simpler. It filters out directories in the first loop rather than keeping them around and filtering them out later.

I'm trying to offer smaller commits that can be more easily reviewed and merged rather than one huge patch.

in reply to:  43 comment:44 by Ryan J Ollos, 18 months ago

Replying to Tim Graham <timograham@…>:

I'm trying to offer smaller commits that can be more easily reviewed and merged rather than one huge patch.

Thank you, that is much appreciated! I will rebase my t12130_python3 branch soon. It needs a lot of work still.

comment:45 by Ryan J Ollos, 18 months ago

Removed use of unpack argument in r16611 (comment:39).

Replaced use of ur prefix in r16622 (comment:39).

Replaced use of dircache in r16612 (comment:38).

Last edited 18 months ago by Ryan J Ollos (previous) (diff)

comment:46 by Tim Graham <timograham@…>, 17 months ago

DONE PR #18 to make numeric literals compatible with Python 3.

Last edited 17 months ago by Ryan J Ollos (previous) (diff)

comment:47 by Ryan J Ollos, 17 months ago

Made numeric literals compatible with Python3 in r16658 (comment:46).

comment:48 by Ryan J Ollos, 17 months ago

Removed PermissionError inheritance from StandardError in r16675. Edited TracDev/Exceptions@21.

Last edited 17 months ago by Ryan J Ollos (previous) (diff)

comment:49 by Jun Omae, 11 months ago

async is a reserved word in Python 3.7:

$ make python=37 status

Python: /venv/py37/bin/python

Traceback (most recent call last):
  File "contrib/make_status.py", line 14, in <module>
    from trac.util.text import print_table, printout
  File "/src/tracdev/git/trac/util/__init__.py", line 45, in <module>
    from trac.util.datefmt import time_now, to_datetime, to_timestamp, utc
  File "/src/tracdev/git/trac/util/datefmt.py", line 56, in <module>
    from trac.util.translation import _, ngettext
  File "/src/tracdev/git/trac/util/translation.py", line 23, in <module>
    from trac.util.html import tag
  File "/src/tracdev/git/trac/util/html.py", line 229
    async=None, autofocus=None, autoplay=None, checked=None, controls=None,
        ^
SyntaxError: invalid syntax
Makefile:92: recipe for target 'status' failed

comment:50 by Christian Boos, 11 months ago

Branch: log:rjollos.git@t12130_python3

comment:51 by Jun Omae, 11 months ago

Branch: log:rjollos.git@t12130_python3log:rjollos.git@t12130_python3.1

in reply to:  49 comment:52 by Christian Boos, 10 months ago

Replying to Jun Omae:

async is a reserved word in Python 3.7:

Fixed in r16846.

comment:53 by Jun Omae, 9 months ago

Recently, Subversion swig-py3 branch is active. I worked to make compatible with Python 2 and 3 using the swig-py3 branch, in https://github.com/jun66j5/trac/tree/python3+swig-py3+r1853738.

comment:54 by anonymous, 9 months ago

Mercurial would also be interesting: https://www.mercurial-scm.org/wiki/Python3

comment:55 by anonymous, 8 months ago

Mercurial 5.0 is scheduled for May 1 as a Python 3 beta release.

in reply to:  55 comment:56 by Jun Omae, 7 months ago

Mercurial 5.0 is released and has beta support for Python 3.

comment:57 by anonymous, 4 months ago

Hi, Fedora Trac rpm maintainer here. How is this coming? Python2 is EOL in 2020 and won't be an option in Fedora in a release or two.

comment:58 by gwync@…, 4 months ago

Bah. Commenting with my email address. Need coffee. :)

comment:59 by Ryan J Ollos, 3 months ago

I've proposed a cut-over to Python 3 for Trac 1.6: gmessage:trac-dev:cgfUUFY1CVo/n4m0cxl1BgAJ. So far, there have been no objections raised. If that's the case, I will pursue that route once Trac 1.4 is released.

comment:60 by gwync@…, 3 months ago

Hopefully there's at least a test release I can use before Fedora 32 is GA, otherwise I might have to drop Trac, which I'd like to avoid, since I use it. :)

comment:61 by Ryan J Ollos, 2 months ago

Milestone: 1.5.1next-dev-1.5.x

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned. Next status will be 'new'.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.