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

fix(pypi): fix the whl selection algorithm after #2069 #2078

Merged
merged 10 commits into from
Jul 20, 2024

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Jul 19, 2024

It seems that a few things broke in recent commits:

  • We are not using the MODULE.bazel.lock file and it seems that it is easy to
    miss when the components in the PyPI extension stop integrating well
    together. This happened during the switch to {abi}_{os}_{plat} target
    platform passing within the code.
  • The logger code stopped working in the extension after the recent additions
    to add the rule_name.
  • repo_utils.getenv was always getting PATH env var on bazel 6.x.

This PR fixes both cases and updates docs to serve as a better reminder. By
fixing the select_whls code and we can just rely on target platform triples
(ABI, OS, CPU). This gets one step closer to maybe supporting optional
python_version which would address #1708. Whilst at it we are also adding
different status messages for building the wheel from sdist vs just
extracting or downloading the wheel.

Tests:

  • Added more unit tests and brought them in line with the rest of the code.
  • Checked manually for differences between the MODULE.bazel.lock files in our
    rules_python extension before refactor(pypi): split out more utils and start passing 'abi_os_arch' around #2069 and after this PR and there are no
    differences except in the experimental_target_platforms attribute in
    whl_library. Before this PR you would see that we do not select any wheels
    for e.g. MarkupSafe and we are always building from sdist.

Work towards #260.

@rickeylev
Copy link
Contributor

Before this PR you would see that we do not select any wheels for e.g. MarkupSafe and we are always building from sdist.

Would this show up as the "Fetching repo bla bla ..." step taking a long time? When I was working on the docs yesterday, there were a couple pypi deps (markup safe sounds familiar) that seemed to be taking a long time to become available. I chalked it up slow downloads or because I was modifying the pypi blz code, but maybe it was building it from source in the repo phase.

python/private/repo_utils.bzl Outdated Show resolved Hide resolved
@aignas
Copy link
Collaborator Author

aignas commented Jul 20, 2024

Correct, it was building MarkupSafe from source, that is why it was taking longer than usual, I'll add a better log message to indicate that, because we can distinguish between that when we pass url to whl_library.

@aignas aignas added this pull request to the merge queue Jul 20, 2024
Merged via the queue into bazelbuild:main with commit 4a262fa Jul 20, 2024
4 checks passed
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