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

Remove Python 2 support from Python rules #15684

Closed
rickeylev opened this issue Jun 15, 2022 · 3 comments
Closed

Remove Python 2 support from Python rules #15684

rickeylev opened this issue Jun 15, 2022 · 3 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-Python Native rules for Python type: feature request

Comments

@rickeylev
Copy link
Contributor

rickeylev commented Jun 15, 2022

Description of the feature request:

With Python 2 having been deprecated 2+ years ago, I think it's time to drop support for it in the Python rules.

The main motivator here is to make re-implementing the rules in Starlark easier: there's a lot of tests and special code that just deals with Python 2: srcs_version compatibility checking, propagation of that, an aspect to identify mismatches, the python_version transition, etc. Re-implementing all that just doesn't make sense for a dead platform.

So, I'm filing this issue as a canonical tracking issue to figure out how to go about dropping this support in Bazel.

Analogous rules_python issue: bazelbuild/rules_python#886

It's been decided that Python 2 support will be dropped as part of Bazel 7. This is also to simplify Starlarkifying the Python rules.

Flag flip tracking issue: #17293

Various refs:

copybara-service bot pushed a commit that referenced this issue Jun 17, 2022
Summary of changes:

* Remove usage of Python rules where not necessary; the Python rules
  were/are being used to test features that aren't specific to the Python
  rules.
* Use regex matching for failures instead of exact strings. The Starlark
  rules have slightly different phrasing or orderings of elements that
  don't affect behavior. This also makes the tests less brittle overall.
* Disable attempting to use the Starlark implementation in all the tests
  requiring Python 2 (e.g. srcs_version checking etc). The Starlark
  implementation doesn't currently (and probably never will) support Python
  2. #15684 filed to track removal
  of it in Bazel.
* Disable attempting to use the Starlark implementation where toolchain
  resolution is required. This will eventually be implemented, but isn't
  is use at Google and otherwise blocks switching internally.
* Tests verifying warnings are printed were removed; Starlark doesn't
  provide a warning facility.
* Stamping is disabled in various tests because, with the Starlark implementation,
  it requires remote execution of actions, which some tests aren't setup for
  (they never needed it previously). Rather than set this up, stamping was
  disabled (affected tests don't require stamping anyways).

PiperOrigin-RevId: 455695015
Change-Id: I82822eff05b6c0a66e65f131c9e1c8784b1573ac
@comius comius added P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Jun 20, 2022
aranguyen pushed a commit to aranguyen/bazel that referenced this issue Jun 27, 2022
Summary of changes:

* Remove usage of Python rules where not necessary; the Python rules
  were/are being used to test features that aren't specific to the Python
  rules.
* Use regex matching for failures instead of exact strings. The Starlark
  rules have slightly different phrasing or orderings of elements that
  don't affect behavior. This also makes the tests less brittle overall.
* Disable attempting to use the Starlark implementation in all the tests
  requiring Python 2 (e.g. srcs_version checking etc). The Starlark
  implementation doesn't currently (and probably never will) support Python
  2. bazelbuild#15684 filed to track removal
  of it in Bazel.
* Disable attempting to use the Starlark implementation where toolchain
  resolution is required. This will eventually be implemented, but isn't
  is use at Google and otherwise blocks switching internally.
* Tests verifying warnings are printed were removed; Starlark doesn't
  provide a warning facility.
* Stamping is disabled in various tests because, with the Starlark implementation,
  it requires remote execution of actions, which some tests aren't setup for
  (they never needed it previously). Rather than set this up, stamping was
  disabled (affected tests don't require stamping anyways).

PiperOrigin-RevId: 455695015
Change-Id: I82822eff05b6c0a66e65f131c9e1c8784b1573ac
@rickeylev rickeylev self-assigned this Oct 16, 2022
@rickeylev
Copy link
Contributor Author

After some discussion with Bazel leads, it's been decided that support for Python 2 in the native rules will be dropped in Bazel 7.0. I'll be adding a --incompatible_python_disable_py2 in an upcoming commit and defaulting it to true in a subsequent one.

copybara-service bot pushed a commit that referenced this issue Jan 14, 2023
…cified.

When true, using Python 2 only attribute values will cause an error.

More specifically, using `python_version=PY2`, `srcs_version=PY2` or `srcs_version=PY2ONLY` with `py_binary`, `py_test`, `py_library`, `py_runtime`,
or `py_runtime_pair` will result in an error.

Work towards #15684

PiperOrigin-RevId: 501959931
Change-Id: I7bc089a2f7b46f2538e4a92d8753c788193a71d5
@rickeylev
Copy link
Contributor Author

Flag flip tracked in #17293

copybara-service bot pushed a commit that referenced this issue Jan 23, 2023
This is in preparation for disabling Python 2 functionality. There is more code
and tests related to Python 2 support, but these are the ones that fail when
`--incompatible_python_disable_py2=true`.

This also revealed that rejecting Python 2 settings for `py_runtime` and
`py_runtime_pair` will have to wait until after the flag flip. This is because
the auto-detecting toolchains define some py2 toolchains, so in order to make
tests pass, those have to be removed, but doing so would break Python 2 support
prior to the flag flip. The other rules still reject them, though, which is
plenty sufficient.

Work towards #15684

PiperOrigin-RevId: 504086890
Change-Id: Ib39e3b32076072f5510418241cd0ca4765e6ab44
@rickeylev
Copy link
Contributor Author

Flag has been flipped!

copybara-service bot pushed a commit that referenced this issue Feb 10, 2023
This also removes the last tests that require the Java implementation of the
rules.

Work towards #15684

PiperOrigin-RevId: 508747691
Change-Id: Id976c1f51cb67fbe446074af546cc93ef997e4c4
hvadehra pushed a commit that referenced this issue Feb 14, 2023
…cified.

When true, using Python 2 only attribute values will cause an error.

More specifically, using `python_version=PY2`, `srcs_version=PY2` or `srcs_version=PY2ONLY` with `py_binary`, `py_test`, `py_library`, `py_runtime`,
or `py_runtime_pair` will result in an error.

Work towards #15684

PiperOrigin-RevId: 501959931
Change-Id: I7bc089a2f7b46f2538e4a92d8753c788193a71d5
hvadehra pushed a commit that referenced this issue Feb 14, 2023
This is in preparation for disabling Python 2 functionality. There is more code
and tests related to Python 2 support, but these are the ones that fail when
`--incompatible_python_disable_py2=true`.

This also revealed that rejecting Python 2 settings for `py_runtime` and
`py_runtime_pair` will have to wait until after the flag flip. This is because
the auto-detecting toolchains define some py2 toolchains, so in order to make
tests pass, those have to be removed, but doing so would break Python 2 support
prior to the flag flip. The other rules still reject them, though, which is
plenty sufficient.

Work towards #15684

PiperOrigin-RevId: 504086890
Change-Id: Ib39e3b32076072f5510418241cd0ca4765e6ab44
hvadehra pushed a commit that referenced this issue Feb 14, 2023
This also removes the last tests that require the Java implementation of the
rules.

Work towards #15684

PiperOrigin-RevId: 508747691
Change-Id: Id976c1f51cb67fbe446074af546cc93ef997e4c4
copybara-service bot pushed a commit that referenced this issue Apr 5, 2023
Because providers are based on identity, and there are Java tests that need
to reference the provider, it's hard to split up the changes, so it's all
in a single change.

The Java class is kept to make usage by the Java tests easier; it isn't
actually used outside of tests.

Work towards #15684

PiperOrigin-RevId: 522119112
Change-Id: I3fef9b0477c110b5f32f8e26612c04bbb92ecb7b
fweikert pushed a commit to fweikert/bazel that referenced this issue May 25, 2023
Because providers are based on identity, and there are Java tests that need
to reference the provider, it's hard to split up the changes, so it's all
in a single change.

The Java class is kept to make usage by the Java tests easier; it isn't
actually used outside of tests.

Work towards bazelbuild#15684

PiperOrigin-RevId: 522119112
Change-Id: I3fef9b0477c110b5f32f8e26612c04bbb92ecb7b
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: feature request
Projects
None yet
Development

No branches or pull requests

3 participants