-
-
Notifications
You must be signed in to change notification settings - Fork 593
compile_pip_requirements should be robust against upstream wheel breakages #2763
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
Comments
@groodt Can I ask why you closed this as not planned? |
This is an upstream issue. rules_python does pin the versions of dependencies that it uses. However, sdist will pull in their own dependencies for their build environment. Those dependencies are outside rules_python control. Workarounds include:
|
It sounds like you're saying the issue is when building dependencies from a source distribution, but that's not the case here. The issue here occurred when pip-compile is parsing the You're right in that this is not directly controlled by rules_python, so I opened an issue on pip-tools: jazzband/pip-tools#2173. Can we keep this issue open as blocked on that issue, so that if that issue is fixed, Also, requests 1 and 2 in the original description seem to be within |
You may be misunderstanding how sdist and metadata is produced. Let's keep an eye on what piptools says, but it will likely be the same outcome. To produce metadata for an sdist, you need to build the sdist. To build the sdist, you invoke the PEP 517 hooks to prepare the build environment. The build environment dependencies are defined by the sdist project itself (generally not pinned). Not piptools (and not rules_python). |
Oh I see. So if I'll update my requests, then, to:
What do you think? |
Almost. But you're still misunderstanding. This isn't a Hope that helps. |
Maybe I was unclear in my original post, so I apologize if so. But no, the issue was our project's The "Invalid command: 'bdist_wheel'` error occurred here, which is getting the metadata of our pyproject.toml. The "Failed to parse /.../pyproject.toml" message in the output was pointing to our pyproject.toml file. I do see your point; theoretically, all of our transitive dependencies could be using unpinned build-system configs, so the build is inherently non-hermetic in that we're at the mercy of the hermeticity of all our transitive deps. But that's a broader issue with the Python ecosystem and not solvable here. |
Hmmm... maybe you can create a repro repository so we can understand your use-case. But it's not an expected or supported use-case to be using bazel or pip-tools to be building in-tree sdist. If you have in-tree code, the supported approach and expectation is that you'd be using bazel rules for the build, not delegating it to pip and pip-tools. |
Repro: https://github.com/brandonchinn178/rules-python-2763-repro This failed for us even if I commented out all dependencies. Since our pyproject.toml didn't specify |
@groodt Also, it'd be great to bump the version of setuptools; newer versions of setuptools vendors the So current list of requests:
|
Isn't (1) already possible by with extra_args? https://rules-python.readthedocs.io/en/latest/api/rules_python/python/pip.html#compile_pip_requirements.extra_args (2) is probably pretty easy: just some exception handling to this code to print extra text on certain errors: https://github.com/bazel-contrib/rules_python/blob/main/python/private/pypi/dependency_resolver/dependency_resolver.py |
ah I guess you could use extra_args. Although kind of a pain to edit BUILD.bazel just for a quick debug. If
Regardless, either way would be fine, as long as there are docs/error messages recommending it. |
Oh yeah, that's much nicer, good idea. I'd accept PRs for both |
oh! Looks like |
#2792) Resolution failure is the most common error from pip-compile, so we should make sure the error message is as clean as it can be. Previously, the output was cluttered with the exception traceback, which makes the actual error hard to see (several nested traceback). The new output shortens it with a nicer message: ``` Checking _main/requirements_lock.txt ERROR: Cannot install requests<2.24 and requests~=2.25.1 because these package versions have conflicting dependencies. ResolutionImpossible: for help visit https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflicts ``` Fixes #2763 --------- Co-authored-by: Richard Levasseur <rlevasseur@google.com>
🐞 bug report
Yesterday around 2p PDT,
bazel test //:requirements_test
inexplicably started failing with an unhelpful error:The first source of friction was debugging the underlying error. After finding the underlying
dependency_resolver.py
command, running it manually with--verbose
shows a more helpful message:But this still remains perplexing, as we saw the
wheel
package being installed earlier in the output. Then it started inexplicably working again this morning, which led me to pypa/wheel#662.wheel
released a version that broke here and they yanked it, causing it to work again.Overall, this was very confusing because we don't expect to encounter these types of issues anymore with a hermetic build system.
Requests:
pip-compile
with--verbose
pip-compile
exits with anything other than "incompatible dependencies", suggest rerunning with--verbose
wheel
/setuptools
that are being installedAffected Rule
Is this a regression?
Description
🔬 Minimal Reproduction
🔥 Exception or Error
🌍 Your Environment
Operating System:
Output of
bazel version
:Rules_python version:
Anything else relevant?
The text was updated successfully, but these errors were encountered: