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

refactor(pystar): load (but don't use) Starlark implementation. #1428

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

rickeylev
Copy link
Contributor

@rickeylev rickeylev commented Sep 21, 2023

Always loading the code provides several benefits:

  • It's easier to reason about what code paths are taken.
  • Iteratively working on them is simply changing an environment variable instead of editing several files.
  • Ensures the files are loadable on older versions of Bazel.

Usage of the Starlark implemenation is controlled by an environment variable, RULES_PYTHON_ENABLE_PYSTAR=1. An environment variable must be used because the decision
about which implementation to use must be made before regular build flags are able to
run (loading phase logic is affected).

The Starlark implementation is almost entirely compatible with pre-Bazel 7, except for the py_internal symbol. This symbol is special in a couple ways:

  • It only exists within the @rules_python repo
  • It does not exist prior to Bazel 7.

This requires using a repo rule, @rules_python_internal, to do some feature/version detection to generate a shim bzl file so that the py_internal symbol is always loadable. Regular rules_python code then loads the shim and can act accordingly.

Also fixes some other loading-time issues (beyond simply py_internal being None):

  • configuration_field() args are validated at time of call, so those must
    be guarded so Bazel 5.4 doesn't fail on them.
  • The init arg of provider() isn't supported under Bazel 5.4; change them
    to no-op stubs behind a guard.
  • The | operator for dicts isn't supported under Bazel 5.4; change to use
    skylib's dicts.add

Work towards #1069

@rickeylev rickeylev force-pushed the pystar-conditional-usage branch 14 times, most recently from e30d600 to e194415 Compare September 24, 2023 20:44
@rickeylev rickeylev marked this pull request as ready for review September 24, 2023 20:44
@rickeylev rickeylev removed the request for review from f0rmiga September 24, 2023 20:45
Always loading the code provides several benefits:
  * It's easier to reason about what code paths are taken.
  * Iteratively working on them is simply changing an environment
    variable instead of editing several files.
  * Ensures the files are loadable on older versions of Bazel.

Usage of the Starlark implemenation is controlled by an environment
variable, `RULES_PYTHON_USE_PYSTAR=1`. An environment variable must be
used the decision on what implemenation to use must be made before a
regular build flag is able to run.

The Starlark implementation is almost entirely compatible with pre-Bazel
7, except for the `py_internal` symbol. This symbol is special in
a couple ways:
  * It only exists within the `@rules_python` repo
  * It does not exist prior to Bazel 7.

This requires using a repo rule, `@rules_python_internal`, to do some
feature/version detection to generate a shim bzl file so that the
`py_internal` symbol is always loadable. Regular rules_python code then
loads the shim and can act accordingly.

Work towards bazelbuild#1069
@f0rmiga
Copy link
Collaborator

f0rmiga commented Sep 24, 2023

Don't need my review on this?

@rickeylev
Copy link
Contributor Author

I don't think so? Nothing behavior changing AFAIK. If you see an issue, though, feel free to point it out.

@rickeylev rickeylev added this pull request to the merge queue Sep 25, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Sep 25, 2023
@rickeylev rickeylev added this pull request to the merge queue Sep 25, 2023
Merged via the queue into bazelbuild:main with commit d8966b8 Sep 25, 2023
1 check passed
renovate bot referenced this pull request in bazel-contrib/rules_bazel_integration_test Oct 6, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [rules_python](https://togithub.com/bazelbuild/rules_python) |
http_archive | minor | `0.25.0` -> `0.26.0` |

---

### Release Notes

<details>
<summary>bazelbuild/rules_python (rules_python)</summary>

###
[`v0.26.0`](https://togithub.com/bazelbuild/rules_python/releases/tag/0.26.0)

[Compare
Source](https://togithub.com/bazelbuild/rules_python/compare/0.25.0...0.26.0)

#### Using Bzlmod with Bazel 6

**NOTE: bzlmod support is still beta. APIs subject to change.**

Add to your `MODULE.bazel` file:

```starlark
bazel_dep(name = "rules_python", version = "0.26.0")

pip = use_extension("@&#8203;rules_python//python/extensions:pip.bzl", "pip")

pip.parse(
    name = "pip",
    requirements_lock = "//:requirements_lock.txt",
)

use_repo(pip, "pip")
```

#### Using WORKSPACE

Paste this snippet into your `WORKSPACE` file:

```starlark
load("@&#8203;bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

http_archive(
    name = "rules_python",
    sha256 = "9d04041ac92a0985e344235f5d946f71ac543f1b1565f2cdbc9a2aaee8adf55b",
    strip_prefix = "rules_python-0.26.0",
    url = "https://github.com/bazelbuild/rules_python/releases/download/0.26.0/rules_python-0.26.0.tar.gz",
)

load("@&#8203;rules_python//python:repositories.bzl", "py_repositories")

py_repositories()
```

##### Gazelle plugin

Paste this snippet into your `WORKSPACE` file:

```starlark
load("@&#8203;bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
http_archive(
    name = "rules_python_gazelle_plugin",
    sha256 = "9d04041ac92a0985e344235f5d946f71ac543f1b1565f2cdbc9a2aaee8adf55b",
    strip_prefix = "rules_python-0.26.0/gazelle",
    url = "https://github.com/bazelbuild/rules_python/releases/download/0.26.0/rules_python-0.26.0.tar.gz",
)

### To compile the rules_python gazelle extension from source,
### we must fetch some third-party go dependencies that it uses.

load("@&#8203;rules_python_gazelle_plugin//:deps.bzl", _py_gazelle_deps = "gazelle_deps")

_py_gazelle_deps()
```

#### What's Changed

- doc: Note Python version changes in CHANGELOG by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1391](https://togithub.com/bazelbuild/rules_python/pull/1391)
- fix: bcr releaser email by
[@&#8203;f0rmiga](https://togithub.com/f0rmiga) in
[https://github.com/bazelbuild/rules_python/pull/1392](https://togithub.com/bazelbuild/rules_python/pull/1392)
- Adding kwargs to gazelle_python_manifest by
[@&#8203;linzhp](https://togithub.com/linzhp) in
[https://github.com/bazelbuild/rules_python/pull/1289](https://togithub.com/bazelbuild/rules_python/pull/1289)
- docs: Use correct link to build badge image and build status page. by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1390](https://togithub.com/bazelbuild/rules_python/pull/1390)
- feat(py_console_script_binary)!: entry points with custom dependencies
by [@&#8203;aignas](https://togithub.com/aignas) in
[https://github.com/bazelbuild/rules_python/pull/1363](https://togithub.com/bazelbuild/rules_python/pull/1363)
- fix(whl_library): avoid unnecessary repository rule restarts by
[@&#8203;aignas](https://togithub.com/aignas) in
[https://github.com/bazelbuild/rules_python/pull/1400](https://togithub.com/bazelbuild/rules_python/pull/1400)
- refactor: add missing `//python/config_settings/private:distribution`
target by [@&#8203;philsc](https://togithub.com/philsc) in
[https://github.com/bazelbuild/rules_python/pull/1402](https://togithub.com/bazelbuild/rules_python/pull/1402)
- Import pycross_wheel_library by
[@&#8203;philsc](https://togithub.com/philsc) in
[https://github.com/bazelbuild/rules_python/pull/1403](https://togithub.com/bazelbuild/rules_python/pull/1403)
- refactor: upgrade certifi by
[@&#8203;cflewis](https://togithub.com/cflewis) in
[https://github.com/bazelbuild/rules_python/pull/1397](https://togithub.com/bazelbuild/rules_python/pull/1397)
- fix: don't set distribs in version transitioning rule by
[@&#8203;comius](https://togithub.com/comius) in
[https://github.com/bazelbuild/rules_python/pull/1412](https://togithub.com/bazelbuild/rules_python/pull/1412)
- fix(gazelle): upgrade rules_go: 0.39.1 -> 0.41.0 to work with upcoming
Bazel versions by [@&#8203;sgowroji](https://togithub.com/sgowroji) in
[https://github.com/bazelbuild/rules_python/pull/1410](https://togithub.com/bazelbuild/rules_python/pull/1410)
- fix: gazelle: Fix non-hermetic runfiles lookup by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[https://github.com/bazelbuild/rules_python/pull/1415](https://togithub.com/bazelbuild/rules_python/pull/1415)
- feat: create toolchain type for py_proto_library by
[@&#8203;comius](https://togithub.com/comius) in
[https://github.com/bazelbuild/rules_python/pull/1416](https://togithub.com/bazelbuild/rules_python/pull/1416)
- internal: copy Starlark rule implementation from Bazel by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1418](https://togithub.com/bazelbuild/rules_python/pull/1418)
- feat: add new Python toolchain versions by
[@&#8203;aignas](https://togithub.com/aignas) in
[https://github.com/bazelbuild/rules_python/pull/1414](https://togithub.com/bazelbuild/rules_python/pull/1414)
- internal(pystar): make starlark impl (mostly) loadable by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1422](https://togithub.com/bazelbuild/rules_python/pull/1422)
- feat: generate py_library per file by
[@&#8203;raylu](https://togithub.com/raylu) in
[https://github.com/bazelbuild/rules_python/pull/1398](https://togithub.com/bazelbuild/rules_python/pull/1398)
- chore: bump default python versions by
[@&#8203;aignas](https://togithub.com/aignas) in
[https://github.com/bazelbuild/rules_python/pull/1425](https://togithub.com/bazelbuild/rules_python/pull/1425)
- feat: Support netrc-based authentication for python_repository rule by
[@&#8203;LINKIWI](https://togithub.com/LINKIWI) in
[https://github.com/bazelbuild/rules_python/pull/1417](https://togithub.com/bazelbuild/rules_python/pull/1417)
- refactor(pystar): load (but don't use) Starlark implementation. by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1428](https://togithub.com/bazelbuild/rules_python/pull/1428)
- fix(gazelle): runfiles discovery by
[@&#8203;aignas](https://togithub.com/aignas) in
[https://github.com/bazelbuild/rules_python/pull/1429](https://togithub.com/bazelbuild/rules_python/pull/1429)
- feat, refactor(pystar): bzl_library for packaging.bzl; fix pystar doc
building and py_wheel by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1432](https://togithub.com/bazelbuild/rules_python/pull/1432)
- refactor(toolchain): use a helper method to convert an X.Y version to
X.Y.Z by [@&#8203;aignas](https://togithub.com/aignas) in
[https://github.com/bazelbuild/rules_python/pull/1423](https://togithub.com/bazelbuild/rules_python/pull/1423)
- pycross: Rename `pycross_wheel_library` and make it work by
[@&#8203;philsc](https://togithub.com/philsc) in
[https://github.com/bazelbuild/rules_python/pull/1413](https://togithub.com/bazelbuild/rules_python/pull/1413)
- fix: Skip printing unneccesary warning. by
[@&#8203;matts1](https://togithub.com/matts1) in
[https://github.com/bazelbuild/rules_python/pull/1407](https://togithub.com/bazelbuild/rules_python/pull/1407)
- refactor(bzlmod)!: simplify pip.parse repository layout by
[@&#8203;aignas](https://togithub.com/aignas) in
[https://github.com/bazelbuild/rules_python/pull/1395](https://togithub.com/bazelbuild/rules_python/pull/1395)
- feat(bzlmod): mark pip extension as os/arch dependent by
[@&#8203;aignas](https://togithub.com/aignas) in
[https://github.com/bazelbuild/rules_python/pull/1433](https://togithub.com/bazelbuild/rules_python/pull/1433)
- chore: bump internal_deps by
[@&#8203;aignas](https://togithub.com/aignas) in
[https://github.com/bazelbuild/rules_python/pull/1322](https://togithub.com/bazelbuild/rules_python/pull/1322)
- tests(pystar): CI configs that uses Starlark implementation of rules
by [@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1435](https://togithub.com/bazelbuild/rules_python/pull/1435)
- internal(pystar): Copy @&#8203;bazel_tools//tools/python files to
rules_python by [@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1437](https://togithub.com/bazelbuild/rules_python/pull/1437)
- internal(pystar): Make py_runtime_pair and autodetecting toolchain
mostly loadable. by [@&#8203;rickeylev](https://togithub.com/rickeylev)
in
[https://github.com/bazelbuild/rules_python/pull/1439](https://togithub.com/bazelbuild/rules_python/pull/1439)
- tests: Move base rule tests under tests instead of
//tools/build_defs/python by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1440](https://togithub.com/bazelbuild/rules_python/pull/1440)
- tests(pystar): py_runtime_pair and py_runtime analysis tests by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1441](https://togithub.com/bazelbuild/rules_python/pull/1441)
- fix(pystar): Use py_internal for runfiles_enabled,
declare_shareable_artifact, share_native_deps by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1443](https://togithub.com/bazelbuild/rules_python/pull/1443)
- build(deps): bump urllib3 from 1.26.13 to 1.26.17 in
/examples/pip_repository_annotations by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/bazelbuild/rules_python/pull/1447](https://togithub.com/bazelbuild/rules_python/pull/1447)
- build(deps): bump urllib3 from 1.25.11 to 1.26.17 in
/examples/pip_install by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[https://github.com/bazelbuild/rules_python/pull/1444](https://togithub.com/bazelbuild/rules_python/pull/1444)
- fix: add missing `@bazel_tools` files to bzl_library dependencies. by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1457](https://togithub.com/bazelbuild/rules_python/pull/1457)
- tests(pystar): add analysis tests to cover basic windows building by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1452](https://togithub.com/bazelbuild/rules_python/pull/1452)
- docs: move dependency management into respective bzl packages by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[https://github.com/bazelbuild/rules_python/pull/1459](https://togithub.com/bazelbuild/rules_python/pull/1459)
- feat(py_wheel): Normalize name and version by
[@&#8203;vonschultz](https://togithub.com/vonschultz) in
[https://github.com/bazelbuild/rules_python/pull/1331](https://togithub.com/bazelbuild/rules_python/pull/1331)
- chore: add new Python toolchains from indygreg by
[@&#8203;aignas](https://togithub.com/aignas) in
[https://github.com/bazelbuild/rules_python/pull/1461](https://togithub.com/bazelbuild/rules_python/pull/1461)

#### New Contributors

- [@&#8203;cflewis](https://togithub.com/cflewis) made their first
contribution in
[https://github.com/bazelbuild/rules_python/pull/1397](https://togithub.com/bazelbuild/rules_python/pull/1397)
- [@&#8203;sgowroji](https://togithub.com/sgowroji) made their first
contribution in
[https://github.com/bazelbuild/rules_python/pull/1410](https://togithub.com/bazelbuild/rules_python/pull/1410)
- [@&#8203;raylu](https://togithub.com/raylu) made their first
contribution in
[https://github.com/bazelbuild/rules_python/pull/1398](https://togithub.com/bazelbuild/rules_python/pull/1398)
- [@&#8203;LINKIWI](https://togithub.com/LINKIWI) made their first
contribution in
[https://github.com/bazelbuild/rules_python/pull/1417](https://togithub.com/bazelbuild/rules_python/pull/1417)

**Full Changelog**:
bazelbuild/rules_python@0.25.0...0.26.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/bazel-contrib/rules_bazel_integration_test).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4wLjMiLCJ1cGRhdGVkSW5WZXIiOiIzNy4wLjMiLCJ0YXJnZXRCcmFuY2giOiJtYWluIn0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants