Convert to Python 3 #11201

olekw · 2020-04-23T01:12:31Z

Python 2 has reached end-of-life.
This change was imported from Debian packaging.

aiuto · 2020-04-23T01:30:31Z

I am dubious about updating third_party except to pull down fresh releases or cherrypicks from from upstream. Unless this is scriptable from an update, it is going to introduce a maintenance burden.

We would be better off eliminating the dependency on gflags. A very quick grep on the source tree shows no use of it. I wonder if this package is vestigial from before conversion to absl flags?

aiuto · 2020-05-19T17:18:08Z

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java

@@ -237,7 +237,7 @@ public void createExecutable(
        PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext);
        // TODO(#8685): Remove this special-case handling as part of making the proper shebang a
        // property of the Python toolchain configuration.
-        String pythonExecutableName = OS.getCurrent() == OS.OPENBSD ? "python3" : "python";
+        String pythonExecutableName = "python3";


Won't this break anyone who is still using Python3?

cc: @brandjon

@aiuto assuming you mean Python 2 then, yes. If a user has only Python 2 installed and not Python 3 then this would fail for them. However, I'm not sure how many users, even those who use Python 2 as default, don't have Python 3 installed at all. (Or couldn't easily install it) Debian ships it even in our oldoldstable (which is really obsolete).

FYI macOS before Catalina only had python2 installed by default, so there is no global /usr/bin/python3, some users might have that installed through homebrew at /usr/local/bin/python3, but it's not a requirement today and arguably relying on brewed python is an issue since users could have any sub-version there depending on when they installed it

We can't require that Python3 be installed on all target platforms where Python 2 targets run.

The proper solution is probably to review #11434 and make the autodetecting toolchain for Python 3 emit a python3 shebang. The only case I can think of where this would be a breaking change is where there's no python3 command in the environment, but there is a python command that resolve to a Python 3 interpreter. In which case I'm tempted to just say add an explicit toolchain to your build.

This code path only affects the disabled-by-default "launchable Python zips" in the deprecated native Python rules of Bazel. For example, if you build something like this:

philwo@philwo-macbookpro2:src/pytest $ cat BUILD py_binary( name = "test", srcs = ["test.py"], ) philwo@philwo-macbookpro2:src/pytest $ cat test.py print("Hello world!") philwo@philwo-macbookpro2:src/pytest $ bazel build --build_python_zip :test Target //:test up-to-date: bazel-bin/test bazel-bin/test.zip

Then the bazel-bin/test would be a ZIP file with the shebang prepended, so you can just run it as an executable, but only if you have a python3 executable in your path. Without this change you can only run it if you have a python executable in your path.

The python executable on the path means "Python 2" on almost all distros (only Arch Linux was bold enough to migrate that to Python 3 :)). So currently we require everyone to install an outdated, EOL'd version of Python to run these binaries, even if their own code is Python 3, as it should be.

With the change, we no longer require Python 2.x installed, but Python 3. IMHO this is fine, especially considering https://python3statement.org/.

In the meantime I think allowing configuring the shebang should suffice.

Yes, that sounds like a nice way. We could let it default to the current python one and introduce an incompatible flag that flips it to python3, WDYT?

Admittedly we wrote there that we'd still support building Python 2.x code. :/

Which was a very intentional policy point, not an oversight. Google isn't even free of Python 2 code, so we can't expect all our users to be. (Of course, we're talking about the ability to build and run Python 2 targets, which isn't the same thing as requiring Python 3 to also be present.)

What would dropping Python 2 support look like in your ideal vision? Would we no longer have a python_version attribute that chose between PY2 and PY3? To drop support of the whole concept of running PY2 code from the Python rules, I'd want to see virtually no users having any PY2 code in their builds. Given that only last year we changed the default and it broke people, I think we have some time before that happens. Perhaps enough that we'll first migrate the rules entirely to Starlark and it'll be a different calculus (e.g. if the version selection system gets more granularity).

We could let it default to the current python one and introduce an incompatible flag that flips it to python3, WDYT?

If people are not setting up an explicit Python toolchain in their build, then they're using the autodetecting toolchain, which will look for Python 2 or Python 3 depending on the configuration of the target. It's easy for these toolchains to specify custom shebangs that use python and python3 respectively. (Of course, the python command on Arch will yield a Python 3 interpreter, but the stub script doesn't care about the version so long as it's at least present.)

If people are setting up a custom toolchain, they can modify their toolchain definition to explicitly specify a shebang.

For the default value, of a toolchain that is not the autodetecting one, we can probably use an incompatible flag to nudge people along. But this would be for convenience; it doesn't block anything.

Ah, just looked at #11434 and I can now understand the %shebang% functionality better. Is there a consensus here to change the default to Python 3 in that mechanism as well? I would personally recommend that but I'll hold off on adding that to this PR lacking said consensus.
@aiuto @philwo @brandjon @oquenchil

@olekw Yes, let's do that and change the default there to python3.

I'll import this and send it for internal review, then we can do a follow-up change on that line.

olekw · 2020-05-19T17:40:59Z

I am dubious about updating third_party except to pull down fresh releases or cherrypicks from from upstream. Unless this is scriptable from an update, it is going to introduce a maintenance burden.

That's a fair point. Regarding scripting, with the exception of BazelPythonSemantics.java, all the other changes were made by using sed (obviously, double-checked afterwards!) I used the whole "#! /usr/bin/env python" string to make sure I didn't accidentally catch something extraneous. That would probably be fairly trivial to script if you wanted to go that route. I searched and (at the time at least) did not find any other references for python vs python3.

aiuto · 2020-05-21T19:30:35Z

I think we should drop this PR and replace it with 3 independent ones.

Drop the dependency on third_party/py/gflags. That is a large part of this PR and it is a bigger win
Convert out nanopb use to py3
Convert mock to py3

Circle back on which python gets invoked in a few months, after we see more migration to rules_python.

See bazelbuild#11201

It is unused. See #11201 Closes #11564. PiperOrigin-RevId: 315438708

aiuto · 2020-06-09T15:48:01Z

Hi Olek: #11564 removed //third_party/gflags, so much of this can go away.

werkt · 2020-07-06T16:10:55Z

@aiuto related to #11564

https://github.com/bazelbuild/rules_docker/blob/faaa10a72fa9abde070e2a20d6046e9f9b849e9a/container/BUILD#L47 (current master) is still referencing this package - this breaks rules_docker for bazel master.

philwo · 2020-09-23T14:38:31Z

What's the status of this PR?

olekw · 2020-09-28T01:44:01Z

What's the status of this PR?

Hi @philwo. I'm happy to refactor this in light of the other changes. As I mentioned in the meeting last week, I've been heavily occupied with things much less fun than working on FOSS. But if the consensus is that you'd like to merge (an updated version of) this PR then I can certainly fix it up in the next couple days.

lberki · 2020-10-21T11:21:33Z

@brandjon , I'm inclined to merge this change as it is. WDYT? (then it'll need to be rebased, though)

aiuto · 2021-01-06T05:16:07Z

Is this still a work in progress?
third_party/py/gflags is no longer part of Bazel, so half of this PR can be deleted.

aiuto · 2021-06-24T04:40:53Z

Friendly ping. This is very stale now. Much of it refers to deleted code.
@olekw Are you going to refresh or close this?

oquenchil · 2021-07-14T13:15:50Z

@olekw If you update this PR, we can merge.

olekw · 2021-07-19T01:04:27Z

Thanks for the pings, @aiuto and @oquenchil! Day job has been keeping me busy but I can definitely update this PR. Will try to do that in the next couple days.

Python 2 has reached end-of-life. This change was adapted from Debian packaging.

olekw · 2021-07-23T19:06:37Z

Ok, went through line by line and this (much smaller) PR is updated for HEAD.

I did notice that there is now a %shebang% in python_stub_template.txt. From what I can tell, this new code appears to Do The Right Thing but I'll leave it to whoever implemented that functionality to verify. :)

Update: See requested change conversation

oquenchil · 2021-07-26T10:24:35Z

@fweikert Hi Florian, IIRC you have experience with Py2 to Py3 migration. Could you please answer olekw's question and take over merging this PR?

fweikert · 2021-07-27T17:00:01Z

Happy to merge this PR once the discussion is resolved. However, I'm, not familiar with the particular problem.

This reflects the change in recent Linux distributions that no longer ship a /usr/bin/python binary. I'm not sure where this is actually used, now that if I understand correctly the shebang defaults to the Python binary found via auto-detection, but we should probably still fix this path (or remove this default fallback completely?). Related to #11201. Closes #13851. PiperOrigin-RevId: 392427248

This reflects the change in recent Linux distributions that no longer ship a /usr/bin/python binary. I'm not sure where this is actually used, now that if I understand correctly the shebang defaults to the Python binary found via auto-detection, but we should probably still fix this path (or remove this default fallback completely?). Related to bazelbuild#11201. Closes bazelbuild#13851. PiperOrigin-RevId: 392427248

googlebot added the cla: yes label Apr 23, 2020

olekw mentioned this pull request Apr 23, 2020

Replace Python 2 with Python 3 #11202

Closed

jin mentioned this pull request May 19, 2020

Bazel binary for aarch64 architecture #8833

Closed

jin added the team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website label May 19, 2020

jin assigned philwo May 19, 2020

philwo added P1 I'll work on this now. (Assignee required) type: feature request labels May 19, 2020

philwo self-requested a review May 19, 2020 17:08

aiuto suggested changes May 19, 2020

View reviewed changes

aiuto added a commit to aiuto/bazel that referenced this pull request Jun 8, 2020

Remove //third_party/gflags

d076da6

See bazelbuild#11201

aiuto mentioned this pull request Jun 8, 2020

Remove //third_party/gflags #11564

Closed

bazel-io pushed a commit that referenced this pull request Jun 9, 2020

Remove //third_party/gflags

2d0defc

It is unused. See #11201 Closes #11564. PiperOrigin-RevId: 315438708

lberki added team-Rules-Python Native rules for Python and removed team-OSS Issues for the Bazel OSS team: installation, release processBazel packaging, website labels Oct 21, 2020

meisterT mentioned this pull request Oct 31, 2020

Migrate tools to PY3 #10127

Closed

jimmyhoran mentioned this pull request Mar 2, 2021

mobile-install is failing on a python import error #12741

Closed

Convert to Python 3

68380a1

Python 2 has reached end-of-life. This change was adapted from Debian packaging.

olekw force-pushed the patch-1 branch from 0f4b13e to 68380a1 Compare July 23, 2021 18:58

oquenchil requested a review from fweikert July 26, 2021 10:23

meisterT requested a review from meteorcloudy July 26, 2021 10:24

philwo approved these changes Aug 10, 2021

View reviewed changes

bazel-io closed this in a5d2973 Aug 11, 2021

olekw mentioned this pull request Aug 15, 2021

Update default stub shebang to Python 3 #13851

Closed

kersson mentioned this pull request Mar 17, 2022

Use python3 on macOS #15045

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Convert to Python 3 #11201

Convert to Python 3 #11201

olekw commented Apr 23, 2020

aiuto commented Apr 23, 2020

aiuto May 19, 2020

olekw May 19, 2020

keith May 19, 2020

brandjon May 19, 2020

philwo May 19, 2020

philwo May 19, 2020 •

edited

Loading

brandjon May 19, 2020

olekw Jul 23, 2021 •

edited

Loading

philwo Aug 10, 2021

philwo Aug 10, 2021 •

edited

Loading

olekw commented May 19, 2020

aiuto commented May 21, 2020

aiuto commented Jun 9, 2020

werkt commented Jul 6, 2020 •

edited

Loading

philwo commented Sep 23, 2020

olekw commented Sep 28, 2020

lberki commented Oct 21, 2020

aiuto commented Jan 6, 2021

aiuto commented Jun 24, 2021

oquenchil commented Jul 14, 2021

olekw commented Jul 19, 2021

olekw commented Jul 23, 2021 •

edited

Loading

oquenchil commented Jul 26, 2021

fweikert commented Jul 27, 2021

Convert to Python 3 #11201

Convert to Python 3 #11201

Conversation

olekw commented Apr 23, 2020

aiuto commented Apr 23, 2020

aiuto May 19, 2020

Choose a reason for hiding this comment

olekw May 19, 2020

Choose a reason for hiding this comment

keith May 19, 2020

Choose a reason for hiding this comment

brandjon May 19, 2020

Choose a reason for hiding this comment

philwo May 19, 2020

Choose a reason for hiding this comment

philwo May 19, 2020 • edited Loading

Choose a reason for hiding this comment

brandjon May 19, 2020

Choose a reason for hiding this comment

olekw Jul 23, 2021 • edited Loading

Choose a reason for hiding this comment

philwo Aug 10, 2021

Choose a reason for hiding this comment

philwo Aug 10, 2021 • edited Loading

Choose a reason for hiding this comment

olekw commented May 19, 2020

aiuto commented May 21, 2020

aiuto commented Jun 9, 2020

werkt commented Jul 6, 2020 • edited Loading

philwo commented Sep 23, 2020

olekw commented Sep 28, 2020

lberki commented Oct 21, 2020

aiuto commented Jan 6, 2021

aiuto commented Jun 24, 2021

oquenchil commented Jul 14, 2021

olekw commented Jul 19, 2021

olekw commented Jul 23, 2021 • edited Loading

oquenchil commented Jul 26, 2021

fweikert commented Jul 27, 2021

philwo May 19, 2020 •

edited

Loading

olekw Jul 23, 2021 •

edited

Loading

philwo Aug 10, 2021 •

edited

Loading

werkt commented Jul 6, 2020 •

edited

Loading

olekw commented Jul 23, 2021 •

edited

Loading