Python 3 compatibility

[vadmium]

It looks like the current code uses features that were new in Python 2.6, so I tried to keep it compatible with 2.6, while adding Python 3 support.

These changes were primarily for Python 3, but along the way I noticed quite a few bugs. So I also fixed them. I also added tests exercising the code changes that I made.

If nobody needs the 2.6 compatibility (or nobody is interested in using this even with 2.7) I could remove some of the compatibility hacks. Personally, I am only interested in using this with recent versions of Python 3.

[bjoernricks]

Sorry for not noticing your pull request. I didn't get an email from github. Thanks a lot for your contribution. I'll try to review your patches at the next weekend.

[vadmium]

@bjoernricks, could you indicate if it is okay to drop for support for Python 2.6 (or even 2.7)? Any old version that can be dropped would make these changes and other work simpler.

[bjoernricks]

I am all for dropping python 2.6 support. I am really not sure if there is some code especially for 2.6.

[bjoernricks]

Regarding your merge request, I really like your changes and would love to merge them but having py3 support and py3 unrelated stuff in one PR doesn't feel right. Could you please split fixes and features into several PRs? That would make it easier for me to merge them.

[bjoernricks]

Additionally do you have objections against switching from LGPLv2+ to MIT license?

[vadmium]

No objections to MIT. (Actually I would prefer that, being simpler than the LGPL :)

I will work on splitting up my changes. Perhaps a few PRs adding tests and bug fixes against Python 2. After that I will use this PR to add only the Py 3 support, which is verified by the new tests.

[bjoernricks]

Sounds great! I'll change to MIT after your PRs have been merged.

[vadmium]

Hi @bjoernricks, I have rebased this onto your version of my bug fixes to avoid the spurious conflicts. I think it is ready to merge again.

The first half of my commits are necessary to make the library work with Python 3. I added some common things to “quilt.utils” that are imported into other modules to keep the Py 2 + 3 compatibility workarounds to a minimum:

• _basestring: Python 2’s basestring class, or Python 3’s str class
• _encode_str: In Python 3, encode a text string using the user’s locale; in Python 2 this is passed a “str” object which is already encoded. I have a commit on another branch to fix a problem on Windows that will also use this.

The next four commits are mainly to avoid various warnings related to Python 3 porting. These include the new ResourceWarning in Python 3, other deprecation warnings added to Python 3 (both enabled with “python3 -W default”), and a Py 3 porting warning enabled with “python2 -3”.

The end result should support Python 3.2+ (and 2.7), though I have only tested on 2.7, 3.5 and 3.6. (The 3.2 limitation is due to using the “exception” attribute from unittest.TestCase.assertRaises in one of the tests.)

[bjoernricks]

I am just curious about why the from future import print_function is missing in the "Make syntax compatible with Python 3, so that modules will import" 69dd6b779f490668ed15dc0705d0bcc22623be97 commit. Isn't it necessary for py2.7 anymore?

Besides I am also thinking about using (or even including) six to handle the basestring and metaclass py2/3 changes.

[vadmium]

In commit 69dd6b7, (almost) all the print calls have a single argument inside round brackets. It is valid Python 2 syntax because the round brackets are just redundant brackets around an expression. It may be more obvious if you add a space:

print "Available commands:"  # Python 2 only
print ("Available commands:")  # Still a print statement in Py 2

The “future” statement is still necessary if you have more than one argument to print though. In one case I avoided it by joining the arguments together: print("\t" + cmd[0]). In the second commit (507f7ed), I did use the future import in cases that originally had “print >> sys.stderr”. It is trickier to detect these cases because they are valid Py 3 syntax (so the modules import), but they raise an exception at run time (bit-shifting a function object).

I can try using or adding Six if you prefer. Last time I checked, it looked like a decent library, though I am used to rolling my own compatibility code and never tried using it. Other projects that I’ve helped with tend to either add custom code to an existing common module like I did with “quilt.utils”, or to a new submodule, e.g. https://github.com/python-excel/xlrd/blob/master/xlrd/timemachine.py.

[bjoernricks]

Thanks a lot for the clarification! Makes sense now.

Regarding six, most of the stuff (if not all) others are writing themselves for py2/3 compatibility is already included in six. So imho it's mostly NIH syndrome. If library authors don't want to add another external dependency they could include six's sources instead like django did. It's only one python file. Therefore I am really in favor for using six.

[bjoernricks]

Additionally I really would like to merge your fixes not related to python 3 changes now. Therefore splitting this pull request (e.g. commit d5030a9defdfecb69971fd2661346e21071b09ac is not related to py3) into several distinct pull requests with only 1-3 commits per topic would help me to review and merge your code a lot faster.

[bjoernricks]

As you have already noticed I also replaced optparse with argparse. So imho there aren't any major changes for python 3 compatibility missing.

I have thought about making a 1.0 release but I think the code isn't stable enough yet. It's still missing a lot of tests and feature flags from the original quilt. At least the tests should be more complete before doing a real stable release. If have found already some small glitches which should have been covered by test cases.

Is there anything you would like to merge or work on before doing another minor release?

[vadmium]

I am adjusting my changes and plan to soon present two more pull requests to avoid various warnings that are new in Py 3. That will leave the final commit in this pull request, where I mentioned Py 3 support in Changes and setup.py. Though since you have mentioned Python 3 in the readme (revision bbb1d7b), my change is not so important.

I have some more bug fixes queued up, some with tests, including tests for some of the bugs you fixed. They depend on (or would have conflicted with) my Py 3 changes, which is why I haven’t made pull requests yet. These include fixes for spawn failure, push -f, Windows quirks, exception names, pushing a specific patch, push failure handling, delete -n, and delete -r. Some bug fixes depend on others as well. It would be good to get these into a release.

My main usage is as a Quilt CLI on Windows on top of Subversion. The original Quilt written for Bash is slow on Windows, while the Python version is fast enough to not be annoying. I have already added a “series -v” implementation (will have to port it to your version of argparse; currently it uses my own argparse framework that introspects parameters from the run method signature). I also have a preliminary “diff” implementation, using Python’s built-in difflib module rather than external commands, and other features to implement. But if you need to make a release it doesn’t have to wait for for any of this.

[bjoernricks]

Wow a lot of fantastic additions! I would be very glad to integrate them. Yeah preparing and stashing changes is the most missing feature in subversion. Therefore I am using git-svn at work.

I am not in a hurry for a new release.I just thought having a py3 compatible release would be nice. Just prepare your changes and I'll make a release afterwards.

[vadmium]

The remaining commit here was just to document that Python 3 works. In the meantime you added a note to the readme, which is a reasonable indicator. So I don't mind if you take or leave this commit. (I have an unrelated commit on top of this one, but I can rebase if necessary.)

[bjoernricks]

Thanks a lot for your work!