Patchwork D7258: makefile: use python3 by default (BC)

login
register
mail settings
Submitter phabricator
Date Nov. 6, 2019, 6:43 p.m.
Message ID <differential-rev-PHID-DREV-hmtj7vcgrm3gzy2motsg-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/42809/
State New
Headers show

Comments

phabricator - Nov. 6, 2019, 6:43 p.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION 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`.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D7258

AFFECTED FILES
  Makefile

CHANGE DETAILS




To: indygreg, #hg-reviewers
Cc: mercurial-devel
phabricator - Nov. 6, 2019, 6:52 p.m.
This revision is now accepted and ready to land.
martinvonz added a comment.
martinvonz accepted this revision.


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

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7258/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7258

To: indygreg, #hg-reviewers, martinvonz
Cc: martinvonz, mercurial-devel
Raphaël Gomès - Nov. 6, 2019, 10:40 p.m.
I think it's a good idea to try to rip the band-aid sooner than later.

+1 for what Martin said about Linux tests, it would make sense to know 
where we're starting to not have the test suite break on our series for 
Python3 reasons.

On 11/6/19 7:43 PM, indygreg (Gregory Szorc) wrote:
> indygreg created this revision.
> Herald added a subscriber: mercurial-devel.
> Herald added a reviewer: hg-reviewers.
>
> REVISION 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`.
>
> REPOSITORY
>    rHG Mercurial
>
> REVISION DETAIL
>    https://phab.mercurial-scm.org/D7258
>
> AFFECTED FILES
>    Makefile
>
> CHANGE DETAILS
>
> diff --git a/Makefile b/Makefile
> --- a/Makefile
> +++ b/Makefile
> @@ -5,7 +5,7 @@
>   # % make PREFIX=/opt/ install
>   
>   export PREFIX=/usr/local
> -PYTHON?=python
> +PYTHON?=python3
>   $(eval HGROOT := $(shell pwd))
>   HGPYTHONS ?= $(HGROOT)/build/pythons
>   PURE=
>
>
>
> To: indygreg, #hg-reviewers
> Cc: mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
phabricator - Nov. 7, 2019, 8:03 a.m.
indygreg added a comment.


  In D7258#106380 <https://phab.mercurial-scm.org/D7258#106380>, @martinvonz wrote:
  
  > 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/.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7258/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7258

To: indygreg, #hg-reviewers, martinvonz
Cc: dlax, martinvonz, mercurial-devel
phabricator - Nov. 7, 2019, 4:36 p.m.
mharbison72 added a comment.
mharbison72 added subscribers: durin42, mharbison72.


  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

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7258/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7258

To: indygreg, #hg-reviewers, martinvonz
Cc: mharbison72, durin42, dlax, martinvonz, mercurial-devel
phabricator - Nov. 7, 2019, 8:29 p.m.
durin42 added a comment.


  In D7258#106949 <https://phab.mercurial-scm.org/D7258#106949>, @mharbison72 wrote:
  
  > 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

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7258/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7258

To: indygreg, #hg-reviewers, martinvonz
Cc: mharbison72, durin42, dlax, martinvonz, mercurial-devel
phabricator - Nov. 8, 2019, 1:21 a.m.
mharbison72 added a comment.


  In D7258#106995 <https://phab.mercurial-scm.org/D7258#106995>, @durin42 wrote:
  
  > In D7258#106949 <https://phab.mercurial-scm.org/D7258#106949>, @mharbison72 wrote:
  >
  >> 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.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7258/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7258

To: indygreg, #hg-reviewers, martinvonz
Cc: mharbison72, durin42, dlax, martinvonz, mercurial-devel
Yuya Nishihara - Nov. 8, 2019, 2:53 p.m.
>     ifeq ($(shell uname -o), Msys)
>     PYTHON?=py -2
>     else
>     PYTHON?=python3
>     endif

Perhaps, `ifeq($(OS),Windows_NT)` would work. My .profile had one.
phabricator - Nov. 8, 2019, 2:54 p.m.
yuja added a comment.


  >   ifeq ($(shell uname -o), Msys)
  >   PYTHON?=py -2
  >   else
  >   PYTHON?=python3
  >   endif
  
  Perhaps, `ifeq($(OS),Windows_NT)` would work. My .profile had one.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7258/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7258

To: indygreg, #hg-reviewers, martinvonz
Cc: yuja, mharbison72, durin42, dlax, martinvonz, mercurial-devel
phabricator - Nov. 8, 2019, 4:35 p.m.
indygreg added a comment.
indygreg planned changes to this revision.


  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.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7258/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7258

To: indygreg, #hg-reviewers, martinvonz
Cc: yuja, mharbison72, durin42, dlax, martinvonz, mercurial-devel
phabricator - Nov. 8, 2019, 4:59 p.m.
mharbison72 added a comment.


  In D7258#107444 <https://phab.mercurial-scm.org/D7258#107444>, @indygreg wrote:
  
  > 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.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7258/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7258

To: indygreg, #hg-reviewers, martinvonz
Cc: yuja, mharbison72, durin42, dlax, martinvonz, mercurial-devel

Patch

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -5,7 +5,7 @@ 
 # % make PREFIX=/opt/ install
 
 export PREFIX=/usr/local
-PYTHON?=python
+PYTHON?=python3
 $(eval HGROOT := $(shell pwd))
 HGPYTHONS ?= $(HGROOT)/build/pythons
 PURE=