Improve mac Python 2 toolchain story #8547

brandjon · 2019-06-03T14:30:41Z

Discussed this with @sergiocampama on Friday. He's traveling now so +cc @thomasvl to represent rules_apple.

Bazel 0.27 flips --incompatible_use_python_toolchains. The default Python toolchain is the autodetecting toolchain, which searches for a Python interpreter of the appropriate version (PY2 vs PY3) on PATH. Previously Bazel would default to using python on PATH, regardless of whether or not it was the correct version. Since python is usually PY2, and Bazel Python targets default to PY3 when no python_version attribute is given, this means that the default behavior on Mac and Linux was to analyze a target as PY3 but run it as PY2.

The problem is that most Mac users do not even have Python 3 installed, but they may have upstream dependencies on rules that require Python code (e.g. as a tool). This code can be written to be compatible with both Python 2 and Python 3, as is the case for rules_apple. But unless those targets are also annotated with python_version = "PY2" (which shouldn't otherwise be necessary), they will be analyzed as PY3, and with the toolchain flag, attempt to run using a non-existent Python 3 runtime.

I see the following options:

Revert to the old behavior on mac of using whatever python is on PATH, irrespective of whether the target is PY2 or PY3, but only doing so when there's no python3 runtime available. I really don't want to do this, because it's exactly the behavior we're trying to avoid in py_binary doesn't automatically pick a Python runtime matching the requested version #4815, and if we don't break this antipattern now it'll only be harder to break later.
Add a non-strict version of the autodetecting toolchain that does this fallback, but don't make it the default behavior. This way, users have to opt in to these bad semantics with --extra_toolchains on the command line or register_toolchains in the WORKSPACE. Users could be directed to opt into the non-strict toolchain from the error message in the default strict toolchain.

I think I'll start work on 2 and see if I can get it cherrypicked into 0.27.

The text was updated successfully, but these errors were encountered:

This toolchain works just like the standard autodetecting toolchain, except that if it falls back on the `python` command, it doesn't care what version that interpreter is. This allows users to opt into the legacy behavior of #4815 while still enabling Python toolchains. This is particularly useful for Mac users who do not have a Python 3 runtime installed. Naturally, such users can only have Python 2 code in their builds, but it's still possible that these Python targets get analyzed by Bazel as PY3. For example, the target author may have forgotten to set the `python_version = "PY2"` attribute, or the downstream user may have not added `--host_force_python=PY2` to their bazelrc. Previously this worked because under #4815 Bazel would just use Python 2 for PY3 targets. Strict version checking breaks these builds, but the new non-strict toolchain provides a workaround. Fixes #8547 RELNOTES: None PiperOrigin-RevId: 251535571

1. Change the Python workspace suffix from registering all toolchains (via `:all`) in the @bazel_tools//tools/python package, to just registering the normal autodetecting toolchain. This ensures that the non-strict toolchain never inadvertently gets higher precedence than the standard toolchain. (This may have been working before by chance, but let's be safe and not rely on target pattern expansion order.) A side-effect of this is that we need to make our analysis and shell integration tests use the standard toolchain's name. 2. Fix bad quoting that caused the suggested command line flag to not be printed 3. Now that the wrapper is shared, there's two possible names for the toolchain, so output the right one in error messages. Fixes #8576, follow-up to #8547. RELNOTES: None PiperOrigin-RevId: 251937305

This toolchain works just like the standard autodetecting toolchain, except that if it falls back on the `python` command, it doesn't care what version that interpreter is. This allows users to opt into the legacy behavior of #4815 while still enabling Python toolchains. This is particularly useful for Mac users who do not have a Python 3 runtime installed. Naturally, such users can only have Python 2 code in their builds, but it's still possible that these Python targets get analyzed by Bazel as PY3. For example, the target author may have forgotten to set the `python_version = "PY2"` attribute, or the downstream user may have not added `--host_force_python=PY2` to their bazelrc. Previously this worked because under #4815 Bazel would just use Python 2 for PY3 targets. Strict version checking breaks these builds, but the new non-strict toolchain provides a workaround. Fixes #8547 RELNOTES: None PiperOrigin-RevId: 251535571

1. Change the Python workspace suffix from registering all toolchains (via `:all`) in the @bazel_tools//tools/python package, to just registering the normal autodetecting toolchain. This ensures that the non-strict toolchain never inadvertently gets higher precedence than the standard toolchain. (This may have been working before by chance, but let's be safe and not rely on target pattern expansion order.) A side-effect of this is that we need to make our analysis and shell integration tests use the standard toolchain's name. 2. Fix bad quoting that caused the suggested command line flag to not be printed 3. Now that the wrapper is shared, there's two possible names for the toolchain, so output the right one in error messages. Fixes #8576, follow-up to #8547. RELNOTES: None PiperOrigin-RevId: 251937305

This toolchain works just like the standard autodetecting toolchain, except that if it falls back on the `python` command, it doesn't care what version that interpreter is. This allows users to opt into the legacy behavior of bazelbuild#4815 while still enabling Python toolchains. This is particularly useful for Mac users who do not have a Python 3 runtime installed. Naturally, such users can only have Python 2 code in their builds, but it's still possible that these Python targets get analyzed by Bazel as PY3. For example, the target author may have forgotten to set the `python_version = "PY2"` attribute, or the downstream user may have not added `--host_force_python=PY2` to their bazelrc. Previously this worked because under bazelbuild#4815 Bazel would just use Python 2 for PY3 targets. Strict version checking breaks these builds, but the new non-strict toolchain provides a workaround. Fixes bazelbuild#8547 RELNOTES: None PiperOrigin-RevId: 251535571

1. Change the Python workspace suffix from registering all toolchains (via `:all`) in the @bazel_tools//tools/python package, to just registering the normal autodetecting toolchain. This ensures that the non-strict toolchain never inadvertently gets higher precedence than the standard toolchain. (This may have been working before by chance, but let's be safe and not rely on target pattern expansion order.) A side-effect of this is that we need to make our analysis and shell integration tests use the standard toolchain's name. 2. Fix bad quoting that caused the suggested command line flag to not be printed 3. Now that the wrapper is shared, there's two possible names for the toolchain, so output the right one in error messages. Fixes bazelbuild#8576, follow-up to bazelbuild#8547. RELNOTES: None PiperOrigin-RevId: 251937305

This toolchain works just like the standard autodetecting toolchain, except that if it falls back on the `python` command, it doesn't care what version that interpreter is. This allows users to opt into the legacy behavior of bazelbuild#4815 while still enabling Python toolchains. This is particularly useful for Mac users who do not have a Python 3 runtime installed. Naturally, such users can only have Python 2 code in their builds, but it's still possible that these Python targets get analyzed by Bazel as PY3. For example, the target author may have forgotten to set the `python_version = "PY2"` attribute, or the downstream user may have not added `--host_force_python=PY2` to their bazelrc. Previously this worked because under bazelbuild#4815 Bazel would just use Python 2 for PY3 targets. Strict version checking breaks these builds, but the new non-strict toolchain provides a workaround. Fixes bazelbuild#8547 RELNOTES: None PiperOrigin-RevId: 251535571

1. Change the Python workspace suffix from registering all toolchains (via `:all`) in the @bazel_tools//tools/python package, to just registering the normal autodetecting toolchain. This ensures that the non-strict toolchain never inadvertently gets higher precedence than the standard toolchain. (This may have been working before by chance, but let's be safe and not rely on target pattern expansion order.) A side-effect of this is that we need to make our analysis and shell integration tests use the standard toolchain's name. 2. Fix bad quoting that caused the suggested command line flag to not be printed 3. Now that the wrapper is shared, there's two possible names for the toolchain, so output the right one in error messages. Fixes bazelbuild#8576, follow-up to bazelbuild#8547. RELNOTES: None PiperOrigin-RevId: 251937305

brandjon added type: feature request P1 I'll work on this now. (Assignee required) team-Rules-Python Native rules for Python labels Jun 3, 2019

brandjon self-assigned this Jun 3, 2019

brandjon mentioned this issue Jun 3, 2019

Breaks with Python toolchains (to be flipped in Bazel 0.27) bazelbuild/rules_apple#456

Closed

brandjon added the release blocker label Jun 3, 2019

brandjon mentioned this issue Jun 3, 2019

Release 0.27 - June 2019 #7816

Closed

bazel-io closed this as completed in 052167e Jun 4, 2019

brandjon mentioned this issue Jun 6, 2019

Fix non-strict toolchain's precedence and diagnostic message #8576

Closed

brandjon mentioned this issue Jun 10, 2019

make_deb.py does not seem to be Python3-compatible #8443

Closed

brandjon mentioned this issue Jul 3, 2019

Update make_deb.py to work for Python2 or 3 bazelbuild/rules_pkg#47

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve mac Python 2 toolchain story #8547

Improve mac Python 2 toolchain story #8547

brandjon commented Jun 3, 2019 •

edited

Loading

Improve mac Python 2 toolchain story #8547

Improve mac Python 2 toolchain story #8547

Comments

brandjon commented Jun 3, 2019 • edited Loading

brandjon commented Jun 3, 2019 •

edited

Loading