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(pypi): translate wheel METADATA parsing to starlark #2629

Open
wants to merge 60 commits into
base: main
Choose a base branch
from

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Feb 24, 2025

This PR starts using the newly introduced (#2692) PEP508 compliant
requirement marker parser in starlark and moves the dependency
generation from the Python language (whl_installer) to the Starlark
in the whl_library repository rule.

This PR is (almost) a pure refactor where no bugs are fixed, but this is
foundational work that also adds notes on how things will be moved
to macros (i.e. analysis phase) so that we can fix a few long standing
bugs and prepare for stabilizing the experimental_index_url (#260).

I have migrated all of the unit tests from Python to starlark.

Non refactoring things:

  • Fix a bug in marker evaluation when we have (()) - a test has been added.
  • Add docs about the possible env property values.

Work towards #260, #2319, #2241
Fixes #2423

github-merge-queue bot pushed a commit that referenced this pull request Mar 30, 2025
This implements the PEP508 compliant marker evaluation in starlark and
removes the need for the Python interpreter when evaluating requirements
files passed to `pip.parse`. This makes the evaluation faster and allows
us to fix a few known issues (#2690).

In the future the intent is to move the `METADATA` parsing to pure
starlark so that the `RequiresDist` could be parsed in starlark at the
macro evaluation or analysis phases. This should make it possible to
more easily solve the design problem that more and more things need to
be passed to `whl_library` as args to have a robust dependency parsing:
* #2319 needs the full Python version to have correct cross-platform
compatible `METADATA` parsing and passing it to `Python` and back makes
  it difficult/annoying to implement.
* Parsing the `METADATA` file requires the precise list of target
  platform or the list of available packages in the `requirements.txt`.
  This means that without it we cannot trim the dependency tree in the
  `whl_library`. Doing this at macro loading phase allows us to depend
  on `.bzl` files in the `hub_repository` and more effectively pass
  information.

I can remotely see that this could become useful in `py_wheel` or an
building
wheels from sdists as the environment markers may be present in various
source
metadata as well. What is more `uv.lock` file has the env markers as
part of
the lock file information, so this might be useful there.

Work towards #2423
Work towards #260
Split from #2629
aignas added 7 commits April 4, 2025 09:34
This reverts commit 1de8269.
Since toolchain matching is done by matching the first target that
matches target settings, the `minor_mapping` config setting is special,
because e.g. all `3.11.X` toolchains match the `python_version = "3.11"`
setting.

This just reshuffles the list so that we have toolchains that are in the
`minor_mapping` before the rest.

Fixes bazel-contrib#2685
@aignas
Copy link
Collaborator Author

aignas commented Apr 4, 2025

For having unit tests working we need to first resolve #2735.

@aignas aignas changed the title WIP refactor(pypi): pep508 parsing in starlark refactor(pypi): parse wheel METADATA in starlark Apr 5, 2025
@aignas aignas force-pushed the feat/starlark-env-marker-and-metadata-parsing branch from 2d6d4c5 to c4b08ad Compare April 5, 2025 15:39
@aignas aignas changed the title refactor(pypi): parse wheel METADATA in starlark refactor(pypi): translate wheel METADATA parsing to starlark Apr 5, 2025
@aignas aignas marked this pull request as ready for review April 5, 2025 15:42
@aignas aignas requested review from rickeylev and groodt as code owners April 5, 2025 15:42
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.

[pypi] PEP508 Remove the need for Python in env_marker evaluation in hub repos
1 participant