Ensure Python 2 and 3 are both available on all platforms

[brandjon]

Last time I checked, our linux workers had both Python 2 and 3 available, but windows only had Python 2 and Mac only Python 3. Can we ensure that all platforms have both versions available?

This affects what platforms some shell integration tests run on. It's possible to rewrite some of the tests as analysis-time tests, or analysis+execution-time tests. But even if I do that, I'll still want something with end-to-end coverage ensuring that py_binarys can in fact locate an interpreter using the (future) default Python toolchain.

For Windows, please ensure that the "py.exe" wrapper is installed, which I believe comes standard with Python 3. It shouldn't matter whether "python.exe" itself is accessible on PATH so long as the wrapper is available. (Whatever tests I add for launching "py.exe" and "python.exe" will probably use a mocked out PATH and fake binaries anyway.)

[brandjon]

Ping @philwo, this would be handy to have now that we're pushing Python toolchainization.

[fweikert]

Philwo is currently OOO. We can look into this issue this week.

[philwo]

@brandjon What is "py.exe"? I've never heard of it.

We cannot easily install Python 2 and 3 on Windows, because both versions install their binaries with the same names, whereas on other platforms they're usually installed as /usr/bin/python and /usr/bin/python3.

I mean, we can install both, but how would you find them, if they're not on the PATH?

[brandjon]

That's the problem that the py.exe launcher is intended to solve. It handles the file association for .py and dispatches to the right version of Python based on shebang. Or it can be passed flags to use a specific version.

I intend to call py.exe in the toolchain of last resort that autodetects an interpreter at execution time. This toolchain may be used to run our end-to-end Python tests on Windows, or alternatively we may have the test workspace register a toolchain that hardcodes the absolute paths of the installed Python runtimes. I don't believe it's necessary for the runtimes to be on PATH -- looks like py.exe searches the registry.

[brandjon]

Ping?

[brandjon]

Note that I had to opt out of the new Python toolchains flag in android_instrumentation_test_integration_test since it had the effect of making the test actually care that it got the right Python version.

[philwo]

I'm rolling out new Windows images that have Python 2.7 and Python 3.6 installed:

Python 2.7 is in C:\Python2
Python 3.6 is in C:\Python3
PATH is set to include Python 3 before Python 2.

[brandjon]

Excellent! To confirm, is py.exe available in C:\Windows (I believe the standard location if py.exe is installed)?

[brandjon]

(Github has a very interesting notion of time. Your comment is currently appearing below the one I just made, with the timestamp "philwo commented 3 hours from now".)

[philwo]

Your comment is currently appearing below the one I just made, with the timestamp "philwo commented 3 hours from now"

Time travel - it's the only way I can handle the workload. No, but lol - I have no idea what's up with GitHub :D

Yes, py.exe should be available - I gave it a quick try in my test image. :)

[brandjon]

Who would own the Mac image? Once we have both runtimes there, I should be able to modify our test workspace to set up appropriate toolchains so we can run Python integration tests on all platforms.

[philwo]

macOS comes with Python 2.7.10 by default (in /usr/bin/python) and we additionally install the latest Python 3.x from Homebrew in /usr/local/bin/python3. It's currently Python 3.7.3, I think.

Does that work for you?

[brandjon]

Sure! Are both on PATH? If not I can just define a toolchain with the absolute path you just gave me.

[philwo]

Yes, they're both on the PATH :) If it doesn't work, let me know.

[brandjon]

See this downstream project CI failure, because PATH at action execution time doesn't contain /usr/local/bin/python3.

I understand when you said it's on PATH you meant at client invocation time. But is there a good way to make it available to the action by default, so that we don't need to work around this on every single downstream project's configuration?

• One possibility is to put --action_env in a common .bazelrc accessible by the test runner user. But this doesn't seem composable with other uses of --action_env or other modifications to PATH.

• Registering an explicit Python toolchain would be nice but I can't see how to do that without modifying the WORKSPACE of downstream projects (or hacking up some kind of extra source root containing the definition and passing --extra_toolchains).

• A symlink of /usr/bin/python3 -> /usr/local/bin/python3` might be the easiest solution. I don't know what mac users would do on their own systems if homebrew paths aren't available to actions by default, but I guess that's a separate problem.

[brandjon]

Note that this will be a breaking failure when --incompatible_use_python_toolchains is flipped (bazelbuild/bazel#7899), and the breakage will be in our CI rather than any particular downstream project.

[philwo]

One possibility is to put --action_env in a common .bazelrc accessible by the test runner user. But this doesn't seem composable with other uses of --action_env or other modifications to PATH.

Yeah, that sounds problematic. What about using --test_env=PATH?

But I thought we had already removed the broken "PATH normalizing" from Bazel and no longer replace it with a stripped down default PATH... I have to investigate what's happening there tomorrow.

Registering an explicit Python toolchain would be nice but I can't see how to do that without modifying the WORKSPACE of downstream projects (or hacking up some kind of extra source root containing the definition and passing --extra_toolchains).

I don't know much about toolchains. We can modify the WORKSPACE file of projects before we run CI tests on them, just like we already have to do for the android_sdk_repository enablement: https://github.com/bazelbuild/bazel/blob/master/.bazelci/postsubmit.yml#L5 - but it's not pretty and it would become a burden if we had to do this for every single project.

A symlink of /usr/bin/python3 -> /usr/local/bin/python3` might be the easiest solution. I don't know what mac users would do on their own systems if homebrew paths aren't available to actions by default, but I guess that's a separate problem.

That's unfortunately impossible - macOS System Integrity Protection prevents even the root user from writing to protected paths like /usr/bin:

$ sudo touch /usr/bin/python3
touch: /usr/bin/python3: Operation not permitted

(I know that it's possible to disable this by using an undocumented tool from the recovery system, but that's not really an option for a production system.)

[brandjon]

--test_env may not be enough for projects that run their own integration tests over bazel, and that expect Python 3 to work in bazel run commands or in tools used within the build. Ideally we could sneak this homebrew path into whatever is setting up core stuff like /usr/bin on this machine.

[philwo]

I verified that for a simple sh_test, Bazel does not modify the PATH on my Mac and the test gets the same PATH env as in my user shell.

That means, /usr/local/bin should be in the PATH.

I looked at the above linked failure and noticed that the test ran with this PATH:

Failure reason: Cannot locate 'python3' or 'python' on the target platform's PATH, which is:
/usr/gnu/bin:/usr/local/bin:/bin:/usr/bin:.

This is wrong for various reasons: a) There is no /usr/gnu/bin on the system and I don't have the slightest idea where that comes from. b) It includes the current directory in the PATH (see the trailing ":.").

Any idea where that weird PATH comes from?

[brandjon]

I'm as stumped as you about the gnu path, I did a code search and github search for that before and found nothing. The Python stub script doesn't manipulate PATH in the environment, so it's not that. Could it maybe be a consequence of the pywrapper script using #!/bin/sh instead of #!/bin/bash?

[brandjon]

Minimal repro:

# //pkg:BUILD

load(":stuff.bzl", "stuff_rule")

py_binary(
    name = "foo",
    srcs = [":foo.py"],
)

stuff_rule(
    name = "stuff",
    outfile = "stuff.txt",
)

# //pkg:stuff.bzl

def _stuff_rule_impl(ctx):
    ctx.actions.run(
        inputs = [],
        outputs = [ctx.outputs.outfile],
        executable = ctx.executable._tool,
    )

stuff_rule = rule(
    implementation = _stuff_rule_impl,
    attrs = {
        # cfg param is required, but doesn't matter whether it's "host" or "target"
        "_tool": attr.label(executable=True, default="//pkg:foo", cfg="target"),
        "outfile": attr.output(),
    },
)

touch pkg/foo.py
bazel build //pkg:stuff --incompatible_use_python_toolchains

That gives you the error message about not being able to locate a python command on PATH. The really interesting thing is that if I don't use the toolchain flag, and I have foo.py try to print out PATH, it doesn't exist!

# foo.py

import os
print(os.environ['PATH'])
# We should actually direct it to a file whose name is passed in argv[1],
# but for the purposes of this example we don't get that far.

bazel build //pkg:stuff
[...]
KeyError: 'PATH'

So the problem appears to be that the new toolchain assumes PATH is in the environment, and that's apparently not true for build actions, even if it's true for targets of bazel run, bazel test, and genrule tools.

Now to figure out why we're seeing these test failures on mac even though I can apparently repro the absence of PATH locally. Somehow the pywrapper script seems to run fine running on my machine as a tool.

[brandjon]

Ok, a key part to this is that the PATH env variable is unset before invoking the py3wrapper.sh script. On my linux box this is fine but on the mac it repros the failure. (Note that unset is different from set to empty string, so PATH= some_command doesn't work.)

My guess is the shell comes up with its default PATH differently between the two platforms, so that python3/python is found on one and not the other. This raises interesting questions about whether/how PATH should be propagated to the action from bazel.

Fortunately, at least it means that the absence of PATH from the underlying Python program's os.environ is WAI.

[brandjon]

This also explains where the gnu path came from.

[brandjon]

Oh.

Ok, now I got it.

When PATH is unset, the shell comes up with its own default PATH, which is not exported. The which command is of course a separate binary, not a shell built-in, so it gets the empty path and always returns no results.

bk-macpro-1:~ ci$ bash
bash-3.2$ echo $PATH
/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin
bash-3.2$ unset PATH
bash-3.2$ echo $PATH

bash-3.2$ /bin/bash
bash-3.2$ echo $PATH
/usr/gnu/bin:/usr/local/bin:/bin:/usr/bin:.
bash-3.2$ python3
Python 3.7.3 (default, Mar 27 2019, 09:23:15) 
[Clang 10.0.1 (clang-1001.0.46.3)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>>              
bash-3.2$ which python3
bash-3.2$ which which

[brandjon]

And that's the difference: On my machine, when the shell initializes its own PATH it also exports it, and on the mac it doesn't export it.

[brandjon]

Re my last comment: I'm not actually sure my machine exports it. It could be an undocumented behavior of the which command. I do observe that on my machine, the payload Python process doesn't see the PATH set by default by the wrapper script's shell, unless I add the export call.