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

feat(py_console_script_binary)!: entry points with custom dependencies #1363

Merged
merged 86 commits into from
Aug 25, 2023

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Aug 5, 2023

Add py_console_script_binary, a macro/rule that allows better customization of how
entry points are generated. Notable features of it are:

  • It allows passing in additional dependencies, which makes it easier for plugin
    dependencies to be added to tools such as pylint or sphinx.
  • The underlying py_binary rule can be passed in, allowing custom rules,
    such as the version-aware rules, to be used for the resulting binary.
  • Entry point generation is based upon a wheel's entry_points.txt file. This helps
    avoid loading external repositories unless they're actually used, allows entry
    points to have better version-aware support, and allows bzlmod to provide a
    supportable mechanism for entry points.

Because the expected common use case is an entry point for our pip generated repos,
there is special logic to make that easy and concisely do. Usage of
py_console_script_binary is not tied to our pip code generation, though, and users can
manually specify dependencies if they need to.

BREAKING CHANGE: This is a breaking change, but only for bzlmod users. Note that
bzlmod support is still beta. Bzlmod users will need to replace using entry_point
from requirements.bzl with loading py_console_script_binary and defining the
entry point locally:

load("@rules_python//python/entry_points:py_console_script_binary.bzl, "py_console_script_binary")

py_console_script_binary(name="foo", pkg="@mypip//pylint")

For workspace users, this new macro is available to be used, but the old code is still
present.

Fixes #1362
Fixes #543
Fixes #979
Fixes #1262
Closes #980
Closes #1294
Closes #1055

@aignas
Copy link
Collaborator Author

aignas commented Aug 5, 2023

Ideally, I would love to reuse the implementation for py_binary and have entry_point as an actual rule, but genrule can also work here initially.

@rickeylev, @groodt, this may have some rough edges still (I am wondering if the workaround around the sys.path entries yields to correct behaviour, but I am not sure if there is a better solution yet), but it is ready for initial feedback. The benefit of this approach is that it does not require very clever tricks explored in #1294 in order to expose spoke contents via the hub repo and we can just use the stuff exposed in the hub repo directly.

@aignas aignas marked this pull request as ready for review August 5, 2023 15:05
@aignas aignas changed the title feat! add entry_point macro to rules_python feat(entry_point)! add entry_point macro to rules_python Aug 5, 2023
@aignas aignas requested a review from f0rmiga as a code owner August 5, 2023 15:26
@chrislovecnm
Copy link
Collaborator

chrislovecnm commented Aug 5, 2023

@aignas I'm getting the same error on Windows. Not sure if this helps:

PS C:\Users\chris\Workspace\rules_python\examples\bzlmod\bazel-bin\entry_point> c:\\b\\r4du22do\\execroot\\_main\\bazel-out\\x64_windows-fastbuild\\bin\\entry_point\\entry_point_test.exe.runfiles\\_main\\entry_point\\yamllint.exe -v
Traceback (most recent call last):
  File "C:\Users\chris\AppData\Local\Temp\Bazel.runfiles_mbclvpp2\runfiles\_main\entry_point\yamllint.py", line 12, in <module>
    from yamllint.cli import run
  File "C:\Users\chris\AppData\Local\Temp\Bazel.runfiles_mbclvpp2\runfiles\_main\entry_point\yamllint.py", line 12, in <module>
    from yamllint.cli import run
ModuleNotFoundError: No module named 'yamllint.cli'; 'yamllint' is not a package
PS C:\Users\chris\Workspace\rules_python\examples\bzlmod\bazel-bin\entry_point

Although running pylint.exe does work.

@rickeylev
Copy link
Contributor

FYI: I have some pubic API-surface feedback to give, but gtg at the moment.

Skimming the PR, I'll also likely have some implementation feedback, but need to look more closely.

@aignas aignas changed the title feat(entry_point)! add entry_point macro to rules_python feat(py_entry_point_binary)! add entry_point macro to rules_python Aug 7, 2023
Copy link
Collaborator

@chrislovecnm chrislovecnm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good! I know that @rickeylev has some input as well. Mostly my comments are about documentation. I want to include more documentation to simplify code maintenance and our users' lives.

@rickeylev and I have talked about how much we appreciate all of the work you have done!

examples/bzlmod/entry_point/BUILD.bazel Outdated Show resolved Hide resolved
examples/bzlmod/entry_point/BUILD.bazel Outdated Show resolved Hide resolved
examples/bzlmod/entry_point/test_entry_point.py Outdated Show resolved Hide resolved
examples/bzlmod/entry_point/test_entry_point.py Outdated Show resolved Hide resolved
examples/pip_parse_vendored/BUILD.bazel Show resolved Hide resolved
python/private/py_entry_point_gen.bzl Outdated Show resolved Hide resolved
python/private/toolchains_repo.bzl Outdated Show resolved Hide resolved
python/py_entry_point_binary.bzl Outdated Show resolved Hide resolved
python/py_entry_point_binary.bzl Outdated Show resolved Hide resolved
python/py_entry_point_binary.bzl Outdated Show resolved Hide resolved
@chrislovecnm
Copy link
Collaborator

@aignas are we fixing #1055 as well?

@aignas
Copy link
Collaborator Author

aignas commented Aug 10, 2023

I'll be going on vacation from tomorrow, so will not respond to questions for a week and a bit.

Copy link
Contributor

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see you've gone through the trouble of implementing the "read from entry points config file" approach I originally was thinking to defer on :).

Well, ok, but lets we need to put some more thought into the design and better document what parts of an entry point config we do and don't support. Lets have that discussion on #979 instead of this PR. I'll post further thoughts on it there. I've tried to refrain from over-commenting on things relating to that.

python/entry_point/BUILD.bazel Outdated Show resolved Hide resolved
python/entry_point/BUILD.bazel Outdated Show resolved Hide resolved
python/entry_point/BUILD.bazel Outdated Show resolved Hide resolved
python/entry_point/BUILD.bazel Outdated Show resolved Hide resolved
python/entry_point/py_console_script_binary.bzl Outdated Show resolved Hide resolved
python/entry_point/generator_test.py Outdated Show resolved Hide resolved
python/entry_point/generator_test.py Outdated Show resolved Hide resolved
python/entry_point/generator.py Outdated Show resolved Hide resolved
@aignas
Copy link
Collaborator Author

aignas commented Aug 20, 2023

I'd be grateful if somebody could enlighten me as to why the Windows tests are failing.

@rickeylev
Copy link
Contributor

I'd be grateful if somebody could enlighten me as to why the Windows tests are failing.

Given the error, it looks like the entry point's executable (the "windows launcher" wrapper thing) is running, and that the launcher is failing to find...something. There's a lot of something it could be :(. Do you have a windows machine?

The basic way the windows+zip mechanism work is...

A native windows executable is created. This is called the "windows launcher". This binary has the path to python interpreter to use embedded into it by the py_binary build process. That interpreter path is uses comes from...it's supposed to be the toolchain. However, I seem to recall that --python_path could override it (and might have on windows?), but I'm having trouble finding the logic that did that.

Additionally, it looks like, in the launcher source code, it does if <desired path doesn't exist>: use python.exe.

A few ideas:

  1. Strip down the test to just call the entry point without any args. That eliminates any of the other args being passed as potential issues.
  2. Try running subprocess.run("python.exe", shell=True). It's not clear to me if it's invoking python.exe via a shell (which would trigger PATH searching), or directly (e.g. via exec, which wouldn't perform PATH searching. I have to assume the former; I don't see how it could have worked without PATH lookup.
  3. The launcher source seems to indicate it has a special --print_launcher_command flag
  4. Replace pylint with a simple example. This eliminates any issues due to pylint.
  5. Check the runfiles environment; nested binaries can interact poorly sometimes
  6. I see the test changes the current working directory? If something is looking for a path relative to the execroot, that could confuse things.

The windows launcher source code is here: https://github.com/bazelbuild/bazel/tree/master/src/tools/launcher

To see how the launcher is built, see here. Note that is the starlarkified code from Bazel head. The error is Bazel 6, which would be using the Java impl. The two should be functionally the same, though.

@aignas aignas marked this pull request as draft August 23, 2023 00:16
Introduce a new `entry_point` macro to `rules_python` as opposed to the
`hub` repository which allows users to generate an `entry_point` script
for a given package. This will check the `console_scripts` key in the
`entry_points.txt` dist-info file and avoids eager fetching of third
party repositories because it is a `genrule` and a `py_binary`
underneath the hood and exists in `rules_python`.

This is a breaking change for bzlmod users as they will have to start
using `@rules_python//python:entry_point.bzl`. For others this new macro
is available to be used, but the old code is still present.

Fixes bazelbuild#1362
Fixes bazelbuild#543
Fixes bazelbuild#979
Fixes bazelbuild#1262
Closes bazelbuild#980
Closes bazelbuild#1294
Copy link
Contributor

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alrighty, almost there!

examples/bzlmod/entry_points/BUILD.bazel Outdated Show resolved Hide resolved
examples/bzlmod/entry_points/pylint_deps_test.py Outdated Show resolved Hide resolved
python/entry_points/py_console_script_binary.bzl Outdated Show resolved Hide resolved
python/private/py_console_script_binary.bzl Outdated Show resolved Hide resolved
tests/entry_points/BUILD.bazel Outdated Show resolved Hide resolved
python/private/py_console_script_binary.bzl Outdated Show resolved Hide resolved
python/private/py_console_script_binary.bzl Show resolved Hide resolved
python/private/py_console_script_gen.py Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
doc why str(pkg) is called on a seemingly already string value
@rickeylev rickeylev dismissed chrislovecnm’s stale review August 25, 2023 20:08

Comments have been addressed; ready to merge

See if buildifier is happy with this...
@rickeylev rickeylev added this pull request to the merge queue Aug 25, 2023
Merged via the queue into bazelbuild:main with commit 9818a60 Aug 25, 2023
2 checks passed
@aignas aignas mentioned this pull request Sep 20, 2023
12 tasks
renovate bot added a commit to bazel-contrib/rules_bazel_integration_test that referenced this pull request 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
[bazelbuild/rules_python#1391
- fix: bcr releaser email by
[@&#8203;f0rmiga](https://togithub.com/f0rmiga) in
[bazelbuild/rules_python#1392
- Adding kwargs to gazelle_python_manifest by
[@&#8203;linzhp](https://togithub.com/linzhp) in
[bazelbuild/rules_python#1289
- docs: Use correct link to build badge image and build status page. by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1390
- feat(py_console_script_binary)!: entry points with custom dependencies
by [@&#8203;aignas](https://togithub.com/aignas) in
[bazelbuild/rules_python#1363
- fix(whl_library): avoid unnecessary repository rule restarts by
[@&#8203;aignas](https://togithub.com/aignas) in
[bazelbuild/rules_python#1400
- refactor: add missing `//python/config_settings/private:distribution`
target by [@&#8203;philsc](https://togithub.com/philsc) in
[bazelbuild/rules_python#1402
- Import pycross_wheel_library by
[@&#8203;philsc](https://togithub.com/philsc) in
[bazelbuild/rules_python#1403
- refactor: upgrade certifi by
[@&#8203;cflewis](https://togithub.com/cflewis) in
[bazelbuild/rules_python#1397
- fix: don't set distribs in version transitioning rule by
[@&#8203;comius](https://togithub.com/comius) in
[bazelbuild/rules_python#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
[bazelbuild/rules_python#1410
- fix: gazelle: Fix non-hermetic runfiles lookup by
[@&#8203;fmeum](https://togithub.com/fmeum) in
[bazelbuild/rules_python#1415
- feat: create toolchain type for py_proto_library by
[@&#8203;comius](https://togithub.com/comius) in
[bazelbuild/rules_python#1416
- internal: copy Starlark rule implementation from Bazel by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1418
- feat: add new Python toolchain versions by
[@&#8203;aignas](https://togithub.com/aignas) in
[bazelbuild/rules_python#1414
- internal(pystar): make starlark impl (mostly) loadable by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1422
- feat: generate py_library per file by
[@&#8203;raylu](https://togithub.com/raylu) in
[bazelbuild/rules_python#1398
- chore: bump default python versions by
[@&#8203;aignas](https://togithub.com/aignas) in
[bazelbuild/rules_python#1425
- feat: Support netrc-based authentication for python_repository rule by
[@&#8203;LINKIWI](https://togithub.com/LINKIWI) in
[bazelbuild/rules_python#1417
- refactor(pystar): load (but don't use) Starlark implementation. by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1428
- fix(gazelle): runfiles discovery by
[@&#8203;aignas](https://togithub.com/aignas) in
[bazelbuild/rules_python#1429
- feat, refactor(pystar): bzl_library for packaging.bzl; fix pystar doc
building and py_wheel by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#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
[bazelbuild/rules_python#1423
- pycross: Rename `pycross_wheel_library` and make it work by
[@&#8203;philsc](https://togithub.com/philsc) in
[bazelbuild/rules_python#1413
- fix: Skip printing unneccesary warning. by
[@&#8203;matts1](https://togithub.com/matts1) in
[bazelbuild/rules_python#1407
- refactor(bzlmod)!: simplify pip.parse repository layout by
[@&#8203;aignas](https://togithub.com/aignas) in
[bazelbuild/rules_python#1395
- feat(bzlmod): mark pip extension as os/arch dependent by
[@&#8203;aignas](https://togithub.com/aignas) in
[bazelbuild/rules_python#1433
- chore: bump internal_deps by
[@&#8203;aignas](https://togithub.com/aignas) in
[bazelbuild/rules_python#1322
- tests(pystar): CI configs that uses Starlark implementation of rules
by [@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1435
- internal(pystar): Copy @&#8203;bazel_tools//tools/python files to
rules_python by [@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1437
- internal(pystar): Make py_runtime_pair and autodetecting toolchain
mostly loadable. by [@&#8203;rickeylev](https://togithub.com/rickeylev)
in
[bazelbuild/rules_python#1439
- tests: Move base rule tests under tests instead of
//tools/build_defs/python by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1440
- tests(pystar): py_runtime_pair and py_runtime analysis tests by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1441
- fix(pystar): Use py_internal for runfiles_enabled,
declare_shareable_artifact, share_native_deps by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#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
[bazelbuild/rules_python#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
[bazelbuild/rules_python#1444
- fix: add missing `@bazel_tools` files to bzl_library dependencies. by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1457
- tests(pystar): add analysis tests to cover basic windows building by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1452
- docs: move dependency management into respective bzl packages by
[@&#8203;rickeylev](https://togithub.com/rickeylev) in
[bazelbuild/rules_python#1459
- feat(py_wheel): Normalize name and version by
[@&#8203;vonschultz](https://togithub.com/vonschultz) in
[bazelbuild/rules_python#1331
- chore: add new Python toolchains from indygreg by
[@&#8203;aignas](https://togithub.com/aignas) in
[bazelbuild/rules_python#1461

#### New Contributors

- [@&#8203;cflewis](https://togithub.com/cflewis) made their first
contribution in
[bazelbuild/rules_python#1397
- [@&#8203;sgowroji](https://togithub.com/sgowroji) made their first
contribution in
[bazelbuild/rules_python#1410
- [@&#8203;raylu](https://togithub.com/raylu) made their first
contribution in
[bazelbuild/rules_python#1398
- [@&#8203;LINKIWI](https://togithub.com/LINKIWI) made their first
contribution in
[bazelbuild/rules_python#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>
@aignas aignas deleted the exp/979/entry_point-rule branch May 13, 2024 06:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants