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(whl_library): move bazel file generation to Starlark #1336

Merged

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Jul 22, 2023

Before this PR, the wheel_installer was doing three things:

  1. Downloading the right wheel.
  2. Extracting it into the output directory.
  3. Generating BUILD.bazel files based on the extracted contents.

This PR is moving the third part into the whl_library repository rule
and it has the following benefits:

  • We can reduce code duplication and label sanitization functions in
    rules_python.
  • There are many things that the wheel_installer does not care anymore
    and we don't need to change less code when extending whl_library as
    we can now do many things in starlark directly.
  • It becomes easier to change the API of how we expose the generated
    BUILD.bazel patching because we only need to change the Starlark
    functions.

Work towards #1330.

@aignas
Copy link
Collaborator Author

aignas commented Jul 22, 2023

FYI @chrislovecnm, I decided to see how this idea of generating BUILD.bazel and other extra files worked out in reality and I think I am happy with the solution. It makes working on the #1262 and extending the API easier, hence me doing this refactor before extending the whl_library more.

Copy link
Collaborator

@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.

Mostly lgtm

aignas and others added 5 commits August 1, 2023 09:08
Before this PR, the `wheel_installer` was doing three things:
1. Downloading the right wheel.
2. Extracting it into the output directory.
3. Generating BUILD.bazel files based on the extracted contents.

This PR is moving the third part into the `whl_library` repository rule
and it has the following benefits:
* We can reduce code duplication and label sanitization functions in
  rules_python.
* There are many things that the `wheel_installer` does not care anymore
  and we don't need to change less code when extending `whl_library` as
  we can now do many things in starlark directly.
* It becomes easier to change the API of how we expose the generated
  BUILD.bazel patching because we only need to change the Starlark
  functions.

Work towards bazelbuild#1330.
@aignas aignas force-pushed the refactor/1330/whl_library_refactor branch from 9210aea to d6a7707 Compare August 1, 2023 00:13
@rickeylev rickeylev added this pull request to the merge queue Aug 2, 2023
Merged via the queue into bazelbuild:main with commit 608ddb7 Aug 2, 2023
@aignas aignas deleted the refactor/1330/whl_library_refactor 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
Development

Successfully merging this pull request may close these issues.

2 participants