1567642 - Flake8 fails on py3 only syntax (since it runs with py2)

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, defect, P1)

Description

[Steve Fink [:sfink] [:s:]]

In https://treeherder.mozilla.org/#/jobs?repo=try&selectedJob=257249986&author=sfink%40mozilla.com&fromchange=e38502952110ab3931767d17c61006bc6788d2ca I am getting:

TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/js/src/devtools/rootAnalysis/run-test.py:103:46 | SyntaxError: invalid syntax (E999)

It is complaining about this line:

    print("START TEST {}".format(name), flush=True)

The flush parameter has been available since Python3. Why is it being rejected?

I can switch to sys.stdout.flush() or something, but it's a workaround and I guess I'm wondering if there is some minimum python version I should be supporting?

Comment 1

[Tom Prince [:tomprince]]

You can add from __future__ import print_function to get the file to parse on python2 at least.

Comment 2

[Andrew Halberstadt [:ahal]]

If this file is only ever meant to be run with Python 3, the better fix would be to remove it from the py2 linter, which checks that files are parseable in Python 2 and have the proper __future__ imports.

You can add an exclusion here:
https://searchfox.org/mozilla-central/source/tools/lint/py2.yml

Please add a comment with something like "The following paths are excluded intentionally:" so that we can differentiate paths that are purposefully excluded and paths that just haven't been triaged yet.

Comment 3

[Steve Fink [:sfink] [:s:]]

Oh! I guess I was assuming that we'd already gotten to the py2->py3 point where we were intentionally abandoning python2 support when possible (well, convenient.)

As :tomprince says in comment 1, at the moment it's easy to make this script still work with python2. I just thought I was sort of supposed to be actively moving everything to python3 and abandoning python2 compatibility.

With respect to the exclusion, I'm confused. The script in question is js/src/devtools/rootAnalysis/run-test.py. But tools/lint/py2.yml already contains js/src within its exclude list. I would say that means that it isn't recursive, but that can't be right -- js/src has no .py files at all, they're all in subdirectories. So why is run-test.py being processed by a python2 lint in the first place?

(Hm... py3.yml also excludes js/src.)

Comment 4

[Andrew Halberstadt [:ahal]]

Yes, if this script doesn't need to run with Python 2 (e.g no mach support), then you might as well make it 3 only. The py2 linter is meant to:
A) Prep files for future Python 3 migration via use of __future__
B) Ensure things that do need to support both 2 and 3 don't accidentally break with Python 2.

This might be the first intentionally excluded module (the stuff in there now likely just isn't triaged), but as we convert more the intentional exclude list will grow longer.

(As for js/src and py3, I think there are patches to get it turned on, just hasn't been landed yet)

Comment 5

[Andrew Halberstadt [:ahal]]

Oh, guess I missed your actual question.. the answer is, I'm not sure. You can test it locally by running ./mach lint -l py2 <path>. Maybe it's not the py2 linter that's doing it after all. Possibly flake8?

Comment 6

[Andrew Halberstadt [:ahal]]

Ugh, yeah it's the flake8 linter (since we run it with Python 2). I think there's a bug on file to get it running with Python 3 instead, and if not I'll file one.

Comment 7

[Andrew Halberstadt [:ahal]]

Actually, let's just co-opt this bug (there are likely going to be many more of these types of errors until we fix this, so no point in keeping the title).

Basically we have two options:

1. Block on bug 1473498 and then convert mach lint itself to Python 3
2. Spawn a py3 subprocess from within the lint integration (note currently flake8 is invoked in-process)

I'm tempted to say let's do 1) as that's something we'll want to do anyway and it gives us more motivation to get around to it.. but this might actually be a pretty big issue as more things start becoming Python 3 only, so 2) might be necessary as a stop-gap. I guess we can wait and see how often people run into this as we work through mach stuff in the meantime.

Comment 10

[Andrew Halberstadt [:ahal]]

We're really close to being able to convert ./mach lint to Python 3, so I propose we just focus on that. We'll need virtualenv.py support first though.

Comment 12

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1567642 - [mozbase] Fix flake8 under py3 lint errors

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/216d19f500f2
[mozbase] Fix flake8 under py3 lint errors r=gbrown

Comment 14

[Andrew Halberstadt [:ahal]]

Fixing this will require converting all of mozlint and all of the lint integrations.. so this bug will track fixing/skipping the actual flake8 errors that happen when running under Python 3. And bug 1580280 will track getting |mach lint| to run with Python 3.

There will be a bit of a coordinated dance required between these bugs to get things landed in the right order.. so if reviewing anything please refrain from landing on r+.

Comment 15

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1567642 - [mach] Fix flake8 under Python 3 lint issues

Comment 16

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1567642 - [mozbuild] Fix flake8 under Python 3 lint issues

Depends on D45412

Comment 17

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1567642 - [mozharness] Fix flake8 under Python 3 lint issues

Depends on D45413

Comment 18

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1567642 - [telemetry] Fix flake8 under Python 3 lint issues

Depends on D45414

Comment 19

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1567642 - [reftest] Fix flake8 under Python 3 lint issues

Depends on D45415

Comment 20

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1567642 - [lint.flake8] Fix misc flake8 under Python 3 lint issues

Depends on D45416

Comment 21

[Andrew Halberstadt [:ahal]]

This bug will be fixed once this revision lands:
https://phabricator.services.mozilla.com/D45441

See stack for the dependency tree.

Comment 22

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/216d19f500f2

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1af32204172e
[mozharness] Fix flake8 under Python 3 lint issues r=Callek
https://hg.mozilla.org/integration/autoland/rev/d4bf347280dc
[telemetry] Fix flake8 under Python 3 lint issues r=chutten,janerik
https://hg.mozilla.org/integration/autoland/rev/6a18da136f76
[reftest] Fix flake8 under Python 3 lint issues r=gbrown

Comment 24

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1567642 - [mozharness] Fix missed print statement in 'test_base_script.py', r?Callek

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/522300b048fc
[mozharness] Fix missed print statement in 'test_base_script.py', r=Callek

Comment 26

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/1af32204172e
https://hg.mozilla.org/mozilla-central/rev/d4bf347280dc
https://hg.mozilla.org/mozilla-central/rev/6a18da136f76
https://hg.mozilla.org/mozilla-central/rev/522300b048fc

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b0aee58a4474
[mach] Fix flake8 under Python 3 lint issues r=firefox-build-system-reviewers,chmanchester
https://hg.mozilla.org/integration/autoland/rev/bb4b4714fe00
[mozbuild] Fix flake8 under Python 3 lint issues r=firefox-build-system-reviewers,chmanchester

Comment 28

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/b0aee58a4474
https://hg.mozilla.org/mozilla-central/rev/bb4b4714fe00

Comment 29

[Andrew Halberstadt [:ahal]]

Attachment: Bug 1567642 - [lint.flake8] Skip remaining flake8 under Python 3 lint issues

Depends on D45417

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb843f6f9594
[lint.flake8] Fix misc flake8 under Python 3 lint issues r=gbrown
https://hg.mozilla.org/integration/autoland/rev/7527ad7c590b
[lint.flake8] Skip remaining flake8 under Python 3 lint issues r=gbrown

Comment 31

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/bb843f6f9594
https://hg.mozilla.org/mozilla-central/rev/7527ad7c590b