-
Notifications
You must be signed in to change notification settings - Fork 541
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
[Regression] Can't upgrade from 0.33.1 to 0.35.0 because of change to how sdists are processed #2152
Comments
As a local workaround we're just applying this patch. diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl
index 2300eb35..777aadda 100644
--- a/python/private/pypi/whl_library.bzl
+++ b/python/private/pypi/whl_library.bzl
@@ -123,7 +123,7 @@ def _parse_optional_attrs(rctx, args, extra_pip_args = None):
"--extra_pip_args",
json.encode(struct(arg = [
envsubst(pip_arg, rctx.attr.envsubst, getenv)
- for pip_arg in extra_pip_args
+ for pip_arg in rctx.attr.extra_pip_args
])),
] #2090 does apply to us and we'd like it to be fixed, but it's not causing any major performance hits or preventing us from working, so for now we're fine with un-fixing it with the above patch. |
This is fixed by #2126. |
github-merge-queue bot
pushed a commit
that referenced
this issue
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>
Confirmed that b99bb61 works 👍. Thanks! And sorry for the noise 🙃 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
🐞 bug report
Affected Rule
pip.parse
, I guess?Is this a regression?
Yes. Things were working in 0.33.1 and failed in 0.35.0.
A
git bisect
shows that 6f9082f (#2091 "fix: use downloaded archive in sdist") is the cause.I confirmed that reverting the change resolves the issue and we're able to update to 0.35.0. Given that the PR was originally intended to fix a different issue, a simple revert is not the likely solution.
Description
A simple
rules_python
version bump should not require any changes to python dependencies (requirements.txt
lockfile). However, I've noticed that I do have to bump some deps, namely these so far:These are packages that did not have wheels for our python version (3.11) and then subsequently added wheels in later releases. So they would download sdists instead.
🔬 Minimal Reproduction
It should be enough to:
experimental_index_url
lazy-object-proxy
(as it has no deps) 1.7.1bazel run //:gazelle_python_manifest.update
lazy-object-proxy
to 1.8.0 (they added a 3.11 wheel)And you'll see the error.
🔥 Exception or Error
🌍 Your Environment
Operating System:
gLinux (Debian Bookworm-based)
Output of
bazel version
:7.2.0
Rules_python version:
0.33.1
Anything else relevant?
The text was updated successfully, but these errors were encountered: