Page MenuHomePhabricator

makefile: use python3 by default (BC)
Changes PlannedPublic

Authored by indygreg on Wed, Nov 6, 1:43 PM.

Details

Reviewers
martinvonz
Group Reviewers
hg-reviewers
Summary

I think the time has come. My main reason for wanting to do this is
to force Mercurial developers to use Python 3 in their day-to-day work.
This should help flush out any remaining Python 3 bugs.

If this change is too controversial, we can revert it before the next
release.

.. bc::

Makefile now uses `python3` instead of `python` by default. This
means Mercurial will be built and run with Python 3 instead of
Python 2.7 by default.

To continue using Python 2, set the PYTHON variable. e.g.
`make install PYTHON=python2.7`.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Wed, Nov 6, 1:43 PM
martinvonz accepted this revision.Wed, Nov 6, 1:52 PM
martinvonz added a subscriber: martinvonz.

All tests except for test-check-py3-compat.t pass on py3 (on Linux), right?

This revision is now accepted and ready to land.Wed, Nov 6, 1:52 PM
dlax added a subscriber: dlax.Wed, Nov 6, 3:04 PM

All tests except for test-check-py3-compat.t pass on py3 (on Linux), right?

Yes, tests are clean on Python 3.6 and 3.7. See https://ci.hg.gregoryszorc.com/.

This will break Windows by default, because py3 is still named python.exe. Alternately, I've been using using PYTHON="py -3" to invoke make.

The command issued to the buildbot will need to explicitly use PYTHON="py -2" to keep that happy. Probably also need to invoke run-tests.py with the right python too, so that $PYTHON is correct. It looks like the shebang lines are already $PYTHON, through some aren't quoted.

CC @durin42

This will break Windows by default, because py3 is still named python.exe. Alternately, I've been using using PYTHON="py -3" to invoke make.

I'm not sure what we should do here. We probably can't change the default only for Windows somehow? (My make-fu is not awesome)

The command issued to the buildbot will need to explicitly use PYTHON="py -2" to keep that happy. Probably also need to invoke run-tests.py with the right python too, so that $PYTHON is correct. It looks like the shebang lines are already $PYTHON, through some aren't quoted.

Okay, I'll see about wiring that up tonight or tomorrow then. I believe that's the only (other) thing blocking this?

CC @durin42

This will break Windows by default, because py3 is still named python.exe. Alternately, I've been using using PYTHON="py -3" to invoke make.

I'm not sure what we should do here. We probably can't change the default only for Windows somehow? (My make-fu is not awesome)

ifeq ($(shell uname -o), Msys)
PYTHON?=py -2
else
PYTHON?=python3
endif

Generally I just test uname, but that prints "MINGW32_NT-6.2" on Windows 10 and "MINGW32_NT-6.1" on Win7.

The command issued to the buildbot will need to explicitly use PYTHON="py -2" to keep that happy. Probably also need to invoke run-tests.py with the right python too, so that $PYTHON is correct. It looks like the shebang lines are already $PYTHON, through some aren't quoted.

Okay, I'll see about wiring that up tonight or tomorrow then. I believe that's the only (other) thing blocking this?

I think that's correct. I vaguely recall that some test helpers were run like ./foo.py at one point, which works on Windows because the registered file handler for *.py is python2. But that's more of an issue with py3 porting, and shouldn't block this. And maybe it was fixed with all of the $PYTHON work you did.

yuja added a subscriber: yuja.Fri, Nov 8, 9:54 AM
ifeq ($(shell uname -o), Msys)
PYTHON?=py -2
else
PYTHON?=python3
endif

Perhaps, ifeq($(OS),Windows_NT) would work. My .profile had one.

indygreg planned changes to this revision.Fri, Nov 8, 11:34 AM

I've seen ifeq($(OS),Windows_NT) used a lot. Although OS may be something else under msys. I'm away from my Windows machine until Sunday. I'll send a revision once I've had time to test on Windows.

I've seen ifeq($(OS),Windows_NT) used a lot. Although OS may be something else under msys. I'm away from my Windows machine until Sunday. I'll send a revision once I've had time to test on Windows.

I can confirm that this is the correct environment variable under msys on Windows 7 and 10, and it works in the makefile as suggested.