1580280 - Run |mach lint| with Python 3

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, task)

Description

[Andrew Halberstadt [:ahal]]

Apart from furthering the Python 3 migration, the main motivation for this is to get flake8 running with Python 3 (which will help the migration immensely everywhere else).

Comment 1

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1580280 - [mozlint] Support Python 3 in the mozlint library

Comment 2

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1580280 - [lint] Support Python 3 in the flake8 integration

Depends on D45435

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1580280 - [mozbuild] Fix Python 3 issues in nodeutil.py

Depends on D45436

Comment 4

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1580280 - [lint] Support Python 3 in the eslint integration

Depends on D45437

Comment 5

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1580280 - [lint] Support Python 3 in the wpt integration

Depends on D45438

Comment 6

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1580280 - [lint] Support Python 3 in other lint integrations

Depends on D45439

Comment 7

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1580280 - [mozlint] Run |mach lint| with Python 3 and drop support for Python 2

Depends on D45440

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/02afa7cfcd1a
[mozlint] Support Python 3 in the mozlint library r=egao
https://hg.mozilla.org/integration/autoland/rev/2e3c166dfab4
[lint] Support Python 3 in the flake8 integration r=sylvestre
https://hg.mozilla.org/integration/autoland/rev/155072fb5c73
[mozbuild] Fix Python 3 issues in nodeutil.py r=nalexander
https://hg.mozilla.org/integration/autoland/rev/784780820d7f
[lint] Support Python 3 in the eslint integration r=Standard8

Comment 9

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/02afa7cfcd1a
https://hg.mozilla.org/mozilla-central/rev/2e3c166dfab4
https://hg.mozilla.org/mozilla-central/rev/155072fb5c73
https://hg.mozilla.org/mozilla-central/rev/784780820d7f

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7498eb95728f
[lint] Support Python 3 in the wpt integration r=jgraham
https://hg.mozilla.org/integration/autoland/rev/bc708e13e2ca
[lint] Support Python 3 in other lint integrations r=sylvestre

Comment 11

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/7498eb95728f
https://hg.mozilla.org/mozilla-central/rev/bc708e13e2ca

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/371641b1010b
[mozlint] Run |mach lint| with Python 3 and drop support for Python 2 r=mars

Comment 13

[Oana Pop-Rus]

Backed out for lints failure

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=superseded%2Ctestfailed%2Cbusted%2Cexception%2Crunnable&revision=371641b1010b834f4144fdb8688a511c381a6776&selectedJob=268438281

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=268438281&repo=autoland

Backout: https://hg.mozilla.org/integration/autoland/rev/8a3dbb1b5e0e263709e412112583ca3ad4a2017a

Comment 14

[Andrew Halberstadt [:ahal]]

Hey Nick,

This patch gets mach lint running with Python 3. I got backed out over the android-lints (forgot to test them) hitting this line:
https://searchfox.org/mozilla-central/rev/f43ae7e1c43a4a940b658381157a6ea6c5a185c1/build/moz.configure/init.configure#319

I guess gradle invokes configure at some point. Interestingly, that line shouldn't get hit in automation, but under Python 3 it does. I'm having a bit of difficulty figuring out where that automation value comes from, do you have any insight?

Although even after fixing that, I suspect we might run into other problems. I might need to create a little shim that runs the android-lints in a Python 2 subprocess :/

Comment 15

[Andrew Halberstadt [:ahal]]

(Note: getting this landed is fairly important to the migration effort, so I'd prefer not to block on configure support for Python 3)

Comment 16

[Nick Alexander :nalexander [he/him]]

(In reply to Andrew Halberstadt [:ahal] from comment #14)

Hey Nick,

This patch gets mach lint running with Python 3. I got backed out over the android-lints (forgot to test them) hitting this line:
https://searchfox.org/mozilla-central/rev/f43ae7e1c43a4a940b658381157a6ea6c5a185c1/build/moz.configure/init.configure#319

I guess gradle invokes configure at some point. Interestingly, that line shouldn't get hit in automation, but under Python 3 it does. I'm having a bit of difficulty figuring out where that automation value comes from, do you have any insight?

The Android lints expect to be run with an already built tree, and the automation task arranges that. I think this issue demonstrates that mach commands under Python 3 (updated mach lint) are not correctly invoking other mach commands that might require Python 2 (mach gradle ..., which might invoke mach environment, mach build, etc).

Now, I don't understand why MOZ_AUTOMATION isn't set for this task and you see the issue at all. I conclude there's something funky happening with the environment handling when running under Python 3.

Although even after fixing that, I suspect we might run into other problems. I might need to create a little shim that runs the android-lints in a Python 2 subprocess :/

Given that we need a build, and the Gradle tasks that the Android lints invoke themselves can invoke mach build, and that builds don't work under Python 3, yes, I think we need to make these things happen under Python 2 in some way. If that's very challenging then I could try to make sure that Gradle doesn't invoke mach build; that is an active worksite, what with mach build also packaging so that the order of tasks can be improved and Fennec being removed allowing significant simplifications.

Comment 17

[Andrew Halberstadt [:ahal]]

Ok, so we might still need to run gradle with Python 2, but turns out that's not what was causing the error from the backout. The error occurs when processing the conditional_names:
https://searchfox.org/mozilla-central/rev/f1e99da78fe6c3c68696358dac06aed90f8112d3/python/mach/mach/registrar.py#64

Conditional names were introduced by bug 1521996 and were meant to provide the ability for commands e.g install or run to have different implementations based on a build condition (e.g whether a build is android or not). But they end up calling into the build config to evaluate them, which I assume is why configure gets invoked.

Why MOZ_AUTOMATION isn't being forwarded here is still a mystery to me.

Comment 18

[Andrew Halberstadt [:ahal]]

I filed bug 1584567 to test out removing conditional names to see if that solves the issue. But it doesn't and that's because you were right. It's not about the conditions after all.

From bug 1521996 comment 19:

(In reply to Nick Alexander :nalexander [he/him] from comment #19)
Regardless of what we do with conditional names, there's still an issue, 'cuz just asking for the build configuration shouldn't trigger configure. I think this is something new with the transition to Python 3. Definitely I've tested running mach lint --linter android-lint with a Desktop mozconfig and with no mozconfig at all and I didn't see mach configure running. Do you see this when running with Python 3 as well?

This is being triggered when instantiating the MachCommandBase object here:
https://searchfox.org/mozilla-central/source/python/mozbuild/mozbuild/base.py#898

Which triggers this:
https://searchfox.org/mozilla-central/source/python/mozbuild/mozbuild/base.py#307

Still haven't quite groked the missing environment, but it's starting to make sense.

Comment 19

[Andrew Halberstadt [:ahal]]

Took longer than I would have liked, but finally figured it out. Python 3 uses __bool__ instead of __nonzero__:
https://searchfox.org/mozilla-central/source/python/mozbuild/mozbuild/configure/options.py#110

Comment 20

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1580280 - [gradle] Stop redirecting stderr into stdout when calling 'mach environment', r?nalexander

Print debugging a task that runs gradle has been really annoying as gradle
reads the output of 'mach environment' and fails as soon as a debug line shows
up.

What's worse, is it redirects stderr into stdout so even printing to
'sys.stderr' fails. This fixes that so writing to stderr will at least work.

Comment 21

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1580280 - [configure] Define '__bool__' on PositiveOptionValue for Python 3 compatibility, r?#firefox-build-system-reviewers

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3282de8d3e51
[gradle] Stop redirecting stderr into stdout when calling 'mach environment', r=nalexander
https://hg.mozilla.org/integration/autoland/rev/b1aec2f85194
[configure] Define '__bool__' on PositiveOptionValue for Python 3 compatibility, r=nalexander
https://hg.mozilla.org/integration/autoland/rev/fbc81f842739
[mozlint] Run |mach lint| with Python 3 and drop support for Python 2 r=mars

Comment 23

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/3282de8d3e51
https://hg.mozilla.org/mozilla-central/rev/b1aec2f85194
https://hg.mozilla.org/mozilla-central/rev/fbc81f842739