feat(bzlmod): support entry_point in bzlmod hub repos by aignas · Pull Request #1294 · bazel-contrib/rules_python

aignas · 2023-07-05T11:52:19Z

Before this PR users would need to get the Python version specific entry_point
which meant that the user may have issues when migrating from one default
version of Python to another. In this PR we:

Move bzlmod hub repo implementation to a private directory to avoid
generating docs.
Remove the old python version specific hub_repository with its template.
Add python version handling in the alias generation to decrease the number of
external repositories we create.
Create an entrypoints.bzl file whilst initializing the whl_library which
contains a dictionary of entrypoints available in this package.
Create python version-specific aliases for entry_points by utilising the new
entrypoints.bzl file.
Create an entry_point starlark macro, that is used from the hub repo macro.

This makes the bazel query @pip//... fetch all of the targets, but this might
be an acceptable price to pay given that there is now an easy way to query the
available entry_points in the hub repo.

Fixes #1262

python/extensions/private/requirements.bzl.tmpl

python/pip_install/entrypoint.bzl

python/private/create_pkg_aliases.bzl

aignas · 2023-07-07T09:46:09Z

It seems that that the CI failure might have been a flake.

python/private/test/render_pkg_aliases_test.bzl

aignas · 2023-07-10T03:15:47Z

This is ready for an initial review, I think.

chrislovecnm

Looks great overall. I love the tests! Some basic notes for you.

@rickeylev or another more senior person needs to look at the PR.

python/extensions/private/hub_repository.bzl

python/pip_install/entrypoint.bzl

python/private/test/render_pkg_aliases_test.bzl

chrislovecnm · 2023-07-10T16:46:29Z

@aignas I have the test failing locally, but any ideas on how to debug the diff_test easier? Also, can you give me a breakdown of what your update script is doing? I can rewrite it in Powershell.

rickeylev

Can you post a pastebin of the generated BUILD files? Or otherwise describe them with some examples? It's hard to grok the overall build layout. I think I know what's going on, but an example would make sure we're on the same page. In a nutshell:

The whl_alias repos are removed. It now goes piphub -> underlying pip package repos
entry_point() is implemented by returning an inline select() expression

python/private/version_label.bzl

python/private/sanitize_name.bzl

python/private/render_pkg_aliases.bzl

python/pip_install/entrypoint.bzl

python/extensions/private/hub_repository.bzl

python/extensions/private/requirements.bzl.tmpl

aignas

I have refactored the code to use rules_testing and in the process I needed to upgrade the version from 0.0.5 and add rules_license to the internal dependencies. Overall the package is a great improvement over skylib and the assertion output when the dictionaries do not match is great. The reason why I was using diff_test is to actually workaround the limitations in ergonomics within skylib.

python/extensions/pip.bzl

python/extensions/private/requirements.bzl.tmpl

aignas · 2023-07-11T09:15:27Z

@rickeylev, you are correct about the overall layout, the best is to checkout, run bzlmod example tests and inspect the bazel-bzlmod/external directory. I upload the folder tomorrow if it is still needed tomorrow.

rickeylev

Looking pretty good overall

tests/pip_hub_repository/entry_point/entry_point_test.bzl

tests/pip_hub_repository/render_pkg_aliases/render_pkg_aliases_test.bzl

python/extensions/private/requirements.bzl.tmpl

python/pip_install/tools/wheel_installer/wheel_installer.py

rickeylev · 2023-07-11T21:45:30Z

overall layout

I think a have a decent sense of the layout after looking at the render_pkg_aliases test -- that fairly nicely described the different build files, their content, and how they're referencing each other.

rickeylev

Just some naming and doc nits, otherwise lgtm.

python/pip_install/entry_point.bzl

python/pip_install/tools/wheel_installer/wheel_installer.py

tests/pip_hub_repository/entry_point/BUILD.bazel

tests/pip_hub_repository/entry_point/entry_point_test.bzl

python/extensions/private/requirements.bzl.tmpl

rickeylev · 2023-07-12T18:56:30Z

Question -- what happens for the case where the available entry points differ? e.g. one has [foo], and the has [bar]? The resulting select expression would be e.g.

select({
  "is_py3.8": "@pip_38_pylint//:foo", # exists
  "is_py3.9": "@pip_39_pylint//:foo", # doesn't exists

Is that going to work out OK? Or cause issues? I'm thinking of e.g. bazel query behavior specifically. A regular build or cquery should only evaluate the necessary dependencies.

aignas · 2023-07-14T08:48:38Z

@rickeylev whilst looking at all this I've found a bug in the transition code and this PR is getting really big, so I have submitted #1316. Because py_binary in the whl_library generated rules do not specify main attribute it would be great to merge that before we have entry_point ready.

I think I would also like to make a PR for the whl_library accept a python_version arg so that if it is present, the rule can use the py_binary from @rules_python//python/config_settings:transitions.bzl. Let me know of your preference about how you would like to review this work.

I am not a huge fan of the entry_point macro either, but I am not sure how I would go about not using it at all. Maybe the @pip//pylint/bin_py39:epylint is a better user experience than using the entry_point macro. Previously the hub repo would not expose this sort of alias and the entry_point macro was certainly much better than @pip_pylint//:rules_python_wheel_entry_point_epylint.

…1329) Before this PR there were at least 2 places where such a helper function existed and it made it very easy to make another copy. This PR introduces a hardened version, that follows conventions from upstream PyPI and tests have been added. Split from #1294, work towards #1262.

Before this PR there would be at least a few places where we would be converting a `X.Y.Z` version string to a shortened `X_Y` or `XY` string segment to be used in repository rule labels. This PR adds a small utility function that helps making things consistent. Work towards #1262, split from #1294.

aignas · 2023-07-25T09:20:27Z

examples/bzlmod/MODULE.bazel

    },
 )
 pip.parse(
+    entry_points = {


@rickeylev, instead of entry_point macros, what do you think about the following extension API? You can see the bzlmod example entry_point test for how it works.

) Before this PR the only way to render aliases for PyPI package repos using the version-aware toolchain was to use the `whl_library_alias` repo. However, we have code that is creating aliases for packages within the hub repo and it is natural to merge the two approaches to keep the number of layers of indirection to minimum. - feat: support alias rendering for python aware toolchain targets. - refactor: use render_pkg_aliases everywhere. - refactor: move the function to a private `.bzl` file. - test: add unit tests for rendering of the aliases. Split from #1294 and work towards #1262 with ideas taken from #1320.

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 bazel-contrib#1362 Fixes bazel-contrib#543 Fixes bazel-contrib#979 Fixes bazel-contrib#1262 Closes bazel-contrib#980 Closes bazel-contrib#1294

#1363) 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 --------- Co-authored-by: Richard Levasseur <richardlev@gmail.com>

aignas requested review from hrfuller and rickeylev as code owners July 5, 2023 11:52

aignas marked this pull request as draft July 6, 2023 01:13

aignas changed the title ~~refactor: unify bzlmod hub repos and make them private~~ feat: support entry_point in bzlmod hub repos Jul 7, 2023

aignas force-pushed the refactor/remove-bzlmod-requirements-usage branch 2 times, most recently from 4a8a2b5 to a15a6e8 Compare July 7, 2023 05:21

aignas marked this pull request as ready for review July 7, 2023 05:21

aignas force-pushed the refactor/remove-bzlmod-requirements-usage branch 3 times, most recently from 04584d4 to 6ad185b Compare July 7, 2023 08:11

aignas commented Jul 7, 2023

View reviewed changes

python/extensions/private/requirements.bzl.tmpl Outdated Show resolved Hide resolved

aignas commented Jul 7, 2023

View reviewed changes

python/pip_install/entrypoint.bzl Show resolved Hide resolved

aignas commented Jul 7, 2023

View reviewed changes

python/private/create_pkg_aliases.bzl Outdated Show resolved Hide resolved

aignas force-pushed the refactor/remove-bzlmod-requirements-usage branch 2 times, most recently from 51baeb2 to 7cb90b2 Compare July 10, 2023 00:04

aignas commented Jul 10, 2023

View reviewed changes

python/private/test/render_pkg_aliases_test.bzl Outdated Show resolved Hide resolved

chrislovecnm suggested changes Jul 10, 2023

View reviewed changes

chrislovecnm changed the title ~~feat: support entry_point in bzlmod hub repos~~ feat(bzlmod): support entry_point in bzlmod hub repos Jul 10, 2023

chrislovecnm added the type: bzlmod bzlmod work label Jul 10, 2023

rickeylev reviewed Jul 10, 2023

View reviewed changes

aignas commented Jul 11, 2023

View reviewed changes

python/extensions/pip.bzl Outdated Show resolved Hide resolved

python/extensions/private/requirements.bzl.tmpl Outdated Show resolved Hide resolved

aignas mentioned this pull request Jul 11, 2023

chore: Bump rules_testing to 0.4.0 from 0.0.5 #1306

Merged

rickeylev requested changes Jul 11, 2023

View reviewed changes

rickeylev requested changes Jul 12, 2023

View reviewed changes

aignas marked this pull request as draft July 16, 2023 15:30

aignas force-pushed the refactor/remove-bzlmod-requirements-usage branch from 3fd01c2 to 922905f Compare July 16, 2023 15:30

This was referenced Jul 19, 2023

refactor: add a version label function for consistent labels #1328

Merged

refactor: have a single function for normalized PyPI package names #1329

Merged

aignas force-pushed the refactor/remove-bzlmod-requirements-usage branch from 922905f to 683ebec Compare July 25, 2023 07:14

aignas added 2 commits July 25, 2023 16:19

feat: support alias rendering for python aware toolchain targets

36199b0

refactor: use render_pkg_aliases

6313822

aignas mentioned this pull request Jul 25, 2023

refactor: support rendering pkg aliases without whl_library_alias #1346

Merged

aignas and others added 7 commits July 25, 2023 17:04

test: add a helper to distinguish between bazel 6

5eab849

feat: add a full_version helper

79fc058

refactor: use full_version

590830b

feat: generate entry_points.bzl for each whl_library

d1f62cb

feat: consolidate bzlmod pip hub repos and support entry_points

b9a2f5f

docs: regenerate docs

9d28b0f

A different external API for entrypoints without a macro

1b88ae3

aignas force-pushed the refactor/remove-bzlmod-requirements-usage branch from 683ebec to 1b88ae3 Compare July 25, 2023 09:19

aignas commented Jul 25, 2023

View reviewed changes

This was referenced Aug 5, 2023

entry_point does not respect version aware toolchains #1362

Closed

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

Merged

aignas mentioned this pull request Aug 7, 2023

fix: Fixing select when we don't have specific platforms #1364

Closed

rickeylev closed this in #1363 Aug 25, 2023

aignas deleted the refactor/remove-bzlmod-requirements-usage branch May 13, 2024 06:48

Uh oh!

Conversation

aignas commented Jul 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aignas commented Jul 7, 2023

Uh oh!

Uh oh!

aignas commented Jul 10, 2023

Uh oh!

chrislovecnm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chrislovecnm commented Jul 10, 2023

Uh oh!

rickeylev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aignas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aignas commented Jul 11, 2023

Uh oh!

rickeylev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rickeylev commented Jul 11, 2023

Uh oh!

rickeylev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rickeylev commented Jul 12, 2023

Uh oh!

aignas commented Jul 14, 2023

Uh oh!

aignas Jul 25, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

aignas commented Jul 5, 2023 •

edited

Loading