A few people have wanted to experiment with Python 3 in the build system. We're not ready to require Python 3 to build a Firefox distribution just yet. But we will someday. And we're certainly ready for supplemental tools to use Python 3 and for parts of the build system to support running with Python 3. Let's add some code to the build system to detect Python 3 so Python 3 processes can easily be invoked.

(In reply to Gregory Szorc [:gps] from comment #0) > A few people have wanted to experiment with Python 3 in the build system. > We're not ready to require Python 3 to build a Firefox distribution just > yet. But we will someday. And we're certainly ready for supplemental tools > to use Python 3 and for parts of the build system to support running with > Python 3. > > Let's add some code to the build system to detect Python 3 so Python 3 > processes can easily be invoked. At the all-hands, we also talked about linting existing python2 code to see what proportion would need to work if we decide to switch to python3. Are those the experiments we're trying to enable here, and hence should we file a bug for that?

I'm not super eager to port the build system to Python 3 just yet. The work I'm trying to unblock right now are groups that want to e.g. port mozbase, support using Python 3 for some mach commands, etc. We should probably have some lint for files that need to be Python 3 clean. I'm not sure where to begin on that though. Our Python testing situation isn't great. I'm hoping whoever does the first major Python 3 porting work will undertake some of that work.

Comment on attachment 8891510 [details] Bug 1385381 - Only call @checking callback if no error occurred; https://reviewboard.mozilla.org/r/162634/#review168558

Comment on attachment 8891511 [details] Bug 1385381 - Detect and expose Python 3 to the build system; https://reviewboard.mozilla.org/r/162636/#review168562 ::: build/moz.configure/init.configure:315 (Diff revision 1) > +# Python 3 > +# ======== > + > +option(env='PYTHON3', nargs=1, help='Python 3 interpreter') > + > +@depends('PYTHON3', virtualenv_python) Why depend on `virtualenv_python`? ::: build/moz.configure/init.configure:333 (Diff revision 1) > + except Exception as e: > + raise FatalCheckError('could not determine version of PYTHON ' > + '(%s): %s' % (python, e)) > + > + if version < (3, 5, 0): > + raise FatalCheckError('PYTHON must point to Python 3.5 or newer; ' Should be 'PYTHON3 must point to...' ::: build/moz.configure/init.configure:347 (Diff revision 1) > + return None > + > + # The API returns a bytes whereas everything in configure is unicode. > + python = python.decode('utf-8') > + > + return python, version Using a `namespace` might be nicer here, then we could just do `set_config('PYTHON3', python3.path)` below. ::: build/moz.configure/init.configure:359 (Diff revision 1) > +@checking('for Python 3 version') > +def python3_version(python): > + return '.'.join(str(v) for v in python[1]) > + > +set_config('PYTHON3', python3_path) > +set_config('PYTHON3_VERSION', python3_version) What's the use case for version sniffing python 3? We don't set the python 2 version from configure. ::: python/mozbuild/mozbuild/base.py:295 (Diff revision 1) > + Returns a tuple of an executable path and its version (as a tuple). Both > + tuple entries can be None. More to the point either both will be `None` or neither will.

Comment on attachment 8891511 [details] Bug 1385381 - Detect and expose Python 3 to the build system; https://reviewboard.mozilla.org/r/162636/#review168562 > Why depend on `virtualenv_python`? This is because that dependency will re-execute configure in the virtualenv. I wanted to be sure the virtualenv was activated before importing some mozbuild modules. Plus, it makes sense for the virtualenv to be one of the first things that configure does. But, due to the ordering in this file, I think virtualenv_python will always be executed first anyway. So I'll drop this. > Using a `namespace` might be nicer here, then we could just do `set_config('PYTHON3', python3.path)` below. glandium hit me with this feedback on another patch. Count me among the "now I know what a `namespace` is" crowd. > What's the use case for version sniffing python 3? We don't set the python 2 version from configure. We don't /have/ to set it. But it's nice to report in logs. We explicitly report the Python and Mercurial versions in mozharness because we want to know when we're using an old, buggy version for example. Plus, I imagine we'll want to start reporting the versions of things in the automated build telemetry so we can better identify sub-optimal environments.

Comment on attachment 8891511 [details] Bug 1385381 - Detect and expose Python 3 to the build system; https://reviewboard.mozilla.org/r/162636/#review172840 ::: build/moz.configure/init.configure:317 (Diff revisions 1 - 2) > > -option(env='PYTHON3', nargs=1, help='Python 3 interpreter') > +option(env='PYTHON3', nargs=1, help='Python 3 interpreter (3.5 or later)') > > -@depends('PYTHON3', virtualenv_python) > +@depends('PYTHON3') > @checking('for Python 3', > - callback=lambda x: x[0] if x else 'no') > + callback=lambda x: x.path if x else 'no') Doesn't this callback handle the case x is None, making the previous patch a little redundant? ::: build/moz.configure/init.configure:356 (Diff revisions 1 - 2) > @depends_if(python3_info) > @checking('for Python 3 version') > def python3_version(python): > - return '.'.join(str(v) for v in python[1]) > + return '.'.join(str(v) for v in python.version) > > set_config('PYTHON3', python3_path) This should work as `python3_info.path` now that python3_info returns a namespace instead of having a separate definition for python3_path.

Comment on attachment 8891511 [details] Bug 1385381 - Detect and expose Python 3 to the build system; https://reviewboard.mozilla.org/r/162636/#review172840 > This should work as `python3_info.path` now that python3_info returns a namespace instead of having a separate definition for python3_path. I did it this way to get a reasonable `@checking` message displayed. Let me look into refactoring it now that I use a `namespace`.

Comment on attachment 8891511 [details] Bug 1385381 - Detect and expose Python 3 to the build system; https://reviewboard.mozilla.org/r/162636/#review172840 > Doesn't this callback handle the case x is None, making the previous patch a little redundant? Yes, the previous patch is no longer needed for this patch. However, I still view it as a legitimate bug fix, since it doesn't make sense to call the callback if an exception is being raised from the check function.

Comment on attachment 8891511 [details] Bug 1385381 - Detect and expose Python 3 to the build system; https://reviewboard.mozilla.org/r/162636/#review172840 > I did it this way to get a reasonable `@checking` message displayed. Let me look into refactoring it now that I use a `namespace`. Bleh - that `@checking` change breaks a bunch of test. I'm too lazy and will just drop it.

Comment on attachment 8891511 [details] Bug 1385381 - Detect and expose Python 3 to the build system; This needs a re-review since it changed substantially.

Comment on attachment 8891511 [details] Bug 1385381 - Detect and expose Python 3 to the build system; https://reviewboard.mozilla.org/r/162636/#review173716 ::: build/moz.configure/init.configure:351 (Diff revision 4) > +set_config('PYTHON3', depends(python3)(lambda p: p.path)) > +set_config('PYTHON3_VERSION', depends(python3)(lambda p: p.str_version)) I guess these should be `depends_if` until we require python 3.

Pushed by gszorc@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/eee2776f7c7f Detect and expose Python 3 to the build system; r=chmanchester