Skip to content

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

Closed
brandonchinn178 opened this issue Apr 9, 2025 · 14 comments · Fixed by #2792
Closed

compile_pip_requirements should be robust against upstream wheel breakages #2763

brandonchinn178 opened this issue Apr 9, 2025 · 14 comments · Fixed by #2792

Comments

@brandonchinn178
Copy link
Contributor

🐞 bug report

Yesterday around 2p PDT, bazel test //:requirements_test inexplicably started failing with an unhelpful error:

==================== Test output for //:requirements_test:
Backend subprocess exited when trying to invoke prepare_metadata_for_build_wheel
Failed to parse /.../pyproject.toml
pip-compile exited with code 2. This means that pip-compile found incompatible requirements or could not find a version that matches the install requirement in one of ['pyproject.toml'].
Checking requirements.txt
================================================================================

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:

error: invalid command 'bdist_wheel'

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:

  1. Expose a mechanism to run pip-compile with --verbose
  2. If pip-compile exits with anything other than "incompatible dependencies", suggest rerunning with --verbose
  3. Pin the version of wheel/setuptools that are being installed

Affected 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?

@groodt groodt closed this as not planned Won't fix, can't repro, duplicate, stale Apr 9, 2025
@brandonchinn178
Copy link
Contributor Author

@groodt Can I ask why you closed this as not planned?

@groodt
Copy link
Collaborator

groodt commented Apr 9, 2025

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:

  • Use a wheel-only / binary-only set of dependencies
  • Vendor and build the source in-tree
  • Request that the upstream package pins their build dependencies

@brandonchinn178
Copy link
Contributor Author

brandonchinn178 commented Apr 9, 2025

However, sdist will pull in their own dependencies for their build environment

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 pyproject.toml file, which includes a call to read the wheel metadata, which involves running bdist_wheel on the current project.

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, rules_python could start pinning those versions?


Also, requests 1 and 2 in the original description seem to be within rules_python control, could we keep the issue open to resolve at least those?

@groodt
Copy link
Collaborator

groodt commented Apr 9, 2025

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).

@brandonchinn178
Copy link
Contributor Author

Oh I see. So if build-system is not specified in pyproject.toml, the default is setuptools.build_meta:__legacy__, as recommended by PEP 517. And that build system is the one that doesn't pin wheel. This isn't an upstream issue, this is "I didn't specify build-system in my pyproject.toml".

I'll update my requests, then, to:

  1. Expose a mechanism to run pip-compile with --verbose
  2. If pip-compile exits with anything other than "incompatible dependencies", suggest rerunning with --verbose
  3. Pin the version of wheel/setuptools that are being installed
  4. Add documentation that pyproject.toml should contain build-system
    a. IMO this is in scope of rules_python because if you're using Bazel, you expect things to be hermetic, and this is an easy pitfall to make your build non-hermetic

What do you think?

@groodt
Copy link
Collaborator

groodt commented Apr 10, 2025

Almost. But you're still misunderstanding. This isn't a pyproject.toml that your project has access to. Your that rules_python has access to. This is an unpinned dependency in a transitive dependency you have (that is an sdist). It's also not necessarily even a pyproject.toml. It could be a setup.py or a setup.cfg too. In general, if you have an sdist, your environment is at the mercy of the sdist author.

Hope that helps.

@brandonchinn178
Copy link
Contributor Author

Maybe I was unclear in my original post, so I apologize if so. But no, the issue was our project's pyproject.toml.

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.

@groodt
Copy link
Collaborator

groodt commented Apr 10, 2025

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.

@brandonchinn178
Copy link
Contributor Author

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 build-system, it uses the setuptools.meta:__legacy__ build system by default, which fetches the latest wheel. Uncommenting the build-system section pins the versions. IMO rules_python docs should mention this pitfall, that pyproject.toml should pin build-system deps.

@brandonchinn178
Copy link
Contributor Author

@groodt Also, it'd be great to bump the version of setuptools; newer versions of setuptools vendors the wheel dependency, which is one less version we need to worry about pinning here: jazzband/pip-tools#2173 (comment)

So current list of requests:

  1. Expose a mechanism to run pip-compile with --verbose
  2. If pip-compile exits with anything other than "incompatible dependencies", suggest rerunning with --verbose
  3. Bump setuptools version
  4. Document that pyproject.toml should contain a build-system table with pinned dependencies

@rickeylev
Copy link
Collaborator

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

@brandonchinn178
Copy link
Contributor Author

ah I guess you could use extra_args. Although kind of a pain to edit BUILD.bazel just for a quick debug. If bazel run //:requirements.update forwarded args, you could just do

bazel run //:requirements.update -- --verbose

Regardless, either way would be fine, as long as there are docs/error messages recommending it.

@rickeylev
Copy link
Collaborator

forward on the command line args

Oh yeah, that's much nicer, good idea.

I'd accept PRs for both

@brandonchinn178
Copy link
Contributor Author

brandonchinn178 commented Apr 19, 2025

oh! Looks like dependency_resolver.py already forwards extra args to pip-compile, so bazel run //:requirements.update -- --verbose already works!

github-merge-queue bot pushed a commit that referenced this issue Apr 23, 2025
#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>
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 a pull request may close this issue.

3 participants