Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

select() on the Python mode should only see "PY2" or "PY3" #6501

Closed
brandjon opened this issue Oct 24, 2018 · 3 comments
Closed

select() on the Python mode should only see "PY2" or "PY3" #6501

brandjon opened this issue Oct 24, 2018 · 3 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Python Native rules for Python type: bug

Comments

@brandjon
Copy link
Member

Currently the configuration value corresponding to --force_python can never transition from null to PY2, due to a quirk in how the transition logic is implemented. (Basically, it sees that PY2 is the default value when null is specified, and decides that keeping it null is good enough.) This means that the same target never appears in both the PY2 and null configurations within the same build.

This would have changed in my fix for #1446. As it turns out, once you start to have targets built in both modes, you can get action conflicts because the build results are slightly different, even though both modes mean "use the Python 2 interpreter". This is because transitive dependencies can behave differently in null mode than in PY2 mode, for instance when you have a select() on "force_python".

The most straightforward immediate solution is to use a separate output path root for PY2 mode, like we already do with "-py3". This would have performance implications in that there'd now be three different ways you could redundantly rebuild the same targets, instead of just two.

A better solution is likely to eliminate null as a valid value for this mode entirely...

@brandjon brandjon added type: bug P2 We'll consider working on this in future. (Assignee optional) team-Rules-Python Native rules for Python labels Oct 24, 2018
@brandjon brandjon self-assigned this Oct 24, 2018
@brandjon
Copy link
Member Author

Action conflicts will no longer be possible (at least, not via this mechanism) under the new model; see #6583.

@brandjon brandjon changed the title PY2 and null Python modes can have action conflicts select() on the Python mode should only see "PY2" or "PY3" Dec 20, 2018
@brandjon
Copy link
Member Author

754dddc added a new select()-able Starlark-defined flag, @bazel_tools//tools/python:python_version. All code that selects on "force_python" should switch to this instead, in order to avoid action conflicts. Under --experimental_better_python_version_mixing, selecting on "force_python" will be disallowed.

bazel-io pushed a commit that referenced this issue Dec 27, 2018
The options fragment gets to decide whether its flags are selectable. This way, selectability can be guarded by an experimental or incompatible change flag, provided that it's declared in the same options fragment as the flag it's guarding.

Work toward #6583 and #6501.

RELNOTES: None
PiperOrigin-RevId: 227040456
bazel-io pushed a commit that referenced this issue Dec 27, 2018
The proper way to select on the python version is to have a config_setting depend (via the flag_values attribute) on @bazel_tools//tools/python:python_version. Reading the value of the native flags --python_version, --force_python, or --host_force_python is not guaranteed to give the actual Python version mode, and can lead to action conflicts.

This CL makes it an error to select on any of these three native flags when --experimental_better_python_version_mixing is enabled. For --python_version, which is currently experimental, this CL also disallows it even when the experimental flag is not enabled.

Work toward #6583 and #6501.

RELNOTES: None
PiperOrigin-RevId: 227046222
@brandjon
Copy link
Member Author

brandjon commented Apr 1, 2019

--incompatible_remove_old_python_version_api is flipped in Bazel 0.25, so it is no longer possible to select() on "force_python".

@brandjon brandjon closed this as completed Apr 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Python Native rules for Python type: bug
Projects
None yet
Development

No branches or pull requests

1 participant