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.

Sign up for GitHub

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

Jump to bottom

[pypi] PEP508 Remove the need for Python in env_marker evaluation in hub repos #2423

Open
aignas opened this issue Nov 18, 2024 · 0 comments
Open

[pypi] PEP508 Remove the need for Python in env_marker evaluation in hub repos #2423

aignas opened this issue Nov 18, 2024 · 0 comments

Comments

@aignas
Copy link
Collaborator

aignas commented Nov 18, 2024

Related #2319

This is to add a parser for PEP508 in starlark and to use it in the hub_repository instead of evaluating markers with Python in the module extension evaluation phase.

This will be needed in other places as well:

  • Using the same starlark parser in parsing the METADATA.
  • Parsing uv.lock and other lock files that have the marker as part of their deps.
  • Allowing us to push the env_marker evaluation to the analysis phase together with the other config setting evaluation.
  • Allowing to properly support the full spec of PEP508 in the whl_library without exploding the number of args we need to pass to whl_library.
  • Allowing to create something similar to a whl_archive repository rule that would not depend on Python that we could dog-food ourselves for the repo rules that need Python at the repo phase (i.e. the current whl_library).
@aignas aignas mentioned this issue Nov 18, 2024
Open
aignas added a commit to aignas/rules_python that referenced this issue Nov 18, 2024
@aignas
refactor(pypi): move config setting processing to the macro
ae80da5 
Before hand we would pass around `whl_alias` struct instances and it
would go through rounds of starlark functions which would end up
rendered as BUILD.bazel files in the hub repository. This means that
every time the output of the said functions would change, the
`MODULE.bazel.lock` would also possibly change.

Since we now have much better unit testing framework, we start relying
on these functions being evaluated at loading phase instead of repo rule
phase, which means that we can make the `BUILD.bazel` files not change a
lot when the underlying code changes, this should make the code more
maintainable in the long run and unblocks the code to accept extra
things, like env marker values or similar.

Summary:
- Remove the `repo` field from `whl_alias`.
- Rename `whl_alias` to `whl_config_setting`
- Move the tests around
- Simplify the `pkg_aliases` tests to use `contains` instead of exact
  equality to make things less brittle.
- Simplify the `pkg_aliases` tests to use different mocks to make
  expectations easier to understand.
- Make `whl_config_setting` hashable by using tuples instead of lists.

This is definitely needed for bazelbuild#2319 and bazelbuild#2423.
@aignas aignas mentioned this issue Nov 18, 2024
Merged
aignas added a commit to aignas/rules_python that referenced this issue Nov 18, 2024
@aignas
refactor(pypi): move config setting processing to the macro
16cf803 
Before hand we would pass around `whl_alias` struct instances and it
would go through rounds of starlark functions which would end up
rendered as BUILD.bazel files in the hub repository. This means that
every time the output of the said functions would change, the
`MODULE.bazel.lock` would also possibly change.

Since we now have much better unit testing framework, we start relying
on these functions being evaluated at loading phase instead of repo rule
phase, which means that we can make the `BUILD.bazel` files not change a
lot when the underlying code changes, this should make the code more
maintainable in the long run and unblocks the code to accept extra
things, like env marker values or similar.

Summary:
- Remove the `repo` field from `whl_alias`.
- Rename `whl_alias` to `whl_config_setting`
- Move the tests around
- Simplify the `pkg_aliases` tests to use `contains` instead of exact
  equality to make things less brittle.
- Simplify the `pkg_aliases` tests to use different mocks to make
  expectations easier to understand.
- Make `whl_config_setting` hashable by using tuples instead of lists.

This is definitely needed for bazelbuild#2319 and bazelbuild#2423.
github-merge-queue bot pushed a commit that referenced this issue Nov 22, 2024
@aignas
refactor(pypi): move config setting processing to the macro (#2424)
2abca35 
Before hand we would pass around `whl_alias` struct instances and it
would go through rounds of starlark functions which would end up
rendered as BUILD.bazel files in the hub repository. This means that
every time the output of the said functions would change, the
`MODULE.bazel.lock` would also possibly change.

Since we now have much better unit testing framework, we start relying
on these functions being evaluated at loading phase instead of repo rule
phase, which means that we can make the `BUILD.bazel` files not change a
lot when the underlying code changes, this should make the code more
maintainable in the long run and unblocks the code to accept extra
things, like env marker values or similar.

Summary:
- Remove the `repo` field from `whl_alias`.
- Rename `whl_alias` to `whl_config_setting`
- Move the tests around
- Simplify the `pkg_aliases` tests to use `contains` instead of exact
  equality to make things less brittle.
- Simplify the `pkg_aliases` tests to use different mocks to make
  expectations easier to understand.
- Make `whl_config_setting` hashable by using tuples instead of lists.
- Adjust how we store the `whl_config_setting` in the PyPI extension
  and optimize the passing to the `hub_repository`.

This is needed for #2319 and #2423.

To be in future PRs:
* Remove the need to pass `osx_versions`, etc to the `pkg_aliases`
macro.
ewianda pushed a commit to ewianda/rules_python that referenced this issue Nov 22, 2024
@aignas @ewianda
refactor(pypi): move config setting processing to the macro (bazelbui…
2f090ad 
…ld#2424)

Before hand we would pass around `whl_alias` struct instances and it
would go through rounds of starlark functions which would end up
rendered as BUILD.bazel files in the hub repository. This means that
every time the output of the said functions would change, the
`MODULE.bazel.lock` would also possibly change.

Since we now have much better unit testing framework, we start relying
on these functions being evaluated at loading phase instead of repo rule
phase, which means that we can make the `BUILD.bazel` files not change a
lot when the underlying code changes, this should make the code more
maintainable in the long run and unblocks the code to accept extra
things, like env marker values or similar.

Summary:
- Remove the `repo` field from `whl_alias`.
- Rename `whl_alias` to `whl_config_setting`
- Move the tests around
- Simplify the `pkg_aliases` tests to use `contains` instead of exact
  equality to make things less brittle.
- Simplify the `pkg_aliases` tests to use different mocks to make
  expectations easier to understand.
- Make `whl_config_setting` hashable by using tuples instead of lists.
- Adjust how we store the `whl_config_setting` in the PyPI extension
  and optimize the passing to the `hub_repository`.

This is needed for bazelbuild#2319 and bazelbuild#2423.

To be in future PRs:
* Remove the need to pass `osx_versions`, etc to the `pkg_aliases`
macro.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@aignas