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: use downloaded archive in sdist #2091

Merged
merged 2 commits into from
Jul 26, 2024

Conversation

chrisirhc
Copy link
Contributor

@chrisirhc chrisirhc commented Jul 25, 2024

Before this PR the extra arguments added by the experimental_index_url
code paths were not used and the sdist was being redownloaded every time
we would build from sdist.

This PR fixes the code to not ignore the extra args added in
https://github.com/bazelbuild/rules_python/pull/2091/files#diff-c007ed21502bf8ea19b98b3f1b402e7071615f8520e4291b00a71bca2cd451e8R231

Fixes #2090

Copy link

google-cla bot commented Jul 25, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@aignas aignas requested a review from rickeylev as a code owner July 26, 2024 03:47
Copy link
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for highlighting this and sending in the fix.

@aignas aignas added this pull request to the merge queue Jul 26, 2024
Merged via the queue into bazelbuild:main with commit 6f9082f Jul 26, 2024
4 checks passed
@chrisirhc chrisirhc deleted the chua/07-25-extrapipargs branch July 29, 2024 08:19
@chrisirhc chrisirhc restored the chua/07-25-extrapipargs branch July 29, 2024 08:19
github-merge-queue bot pushed a commit that referenced this pull request Aug 24, 2024
…when build sdist (#2126)

Building sdist results in `Could not find a version that satisfies the
requirement setuptool` this regressed when a fix in parameter handling
got introduced in #2091.

Before this change the building from sdist when using
`experimental_index_url`
would break because `--no-index` is passed to `pip`. This means that
`pip`
would fail to locate build time dependencies needed for the packages and
would
just not work. In `whl_library` we setup `PYTHONPATH` to have some build
dependencies available (like `setuptools`) and we could use them during
building from `sdist` and to do so we need to add `--no-build-isolation`
flag.
However, for some cases we need to also add other build-time
dependencies (e.g.
`flit_core`) so that the building of the wheel in the `repository_rule`
context
is successfuly. Removing `--no-index` allows `pip` to silently fetch the
needed
build dependencies from PyPI if they are missing and continue with the
build.

This is not a perfect solution, but it does unblock users to use the
`sdist`
distributions with the experimental feature enabled by using
`experimental_index_url` (see #260 for tracking of the completion).

Fixes #2118
Fixes #2152

---------

Co-authored-by: aignas <240938+aignas@users.noreply.github.com>
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.

wheelinstaller does not use the archive when installing sdist
2 participants