Repository navigation
Hint when a build failure is caused by a requires-python mismatch - #19673
SuryanshSS1011 wants to merge 1 commit into
Conversation
|
Friendly bump on this one. It's been sitting since early June with green CI and no reviewer assigned yet. Anything I can do to help move it along, or is there someone who'd be the right person to take a look? Happy to rebase if it's gone stale. |
|
I can take a look |
| } | ||
| Some(format!( | ||
| "`{}` declares `requires-python = \"{}\"`, but you are using Python {}. \ | ||
| This is the most likely cause of the build failure; try a newer version of `{}` \ |
There was a problem hiding this comment.
I'm not sure we can say it's the most likely cause unless we also have sniffed for a SyntaxError in the text or something. I'll try to think of some wording I prefer.
There was a problem hiding this comment.
It might work to just say "Consider adding a constraint to to select a different version"?
We could track further specializations in separate issues.
There was a problem hiding this comment.
I have reworded to state the mismatch in a neutral way and drop the causation claim. Happy to take whatever phrasing you land on if you'd prefer something specific.
I also looked into the direction-aware approach from the other comment and it's quite tractable. RequiresPython::from_specifiers(requires_python).range() gives lower()/upper() bounds, so !lower().contains(version) would mean the interpreter is below the range (too old, a newer package release is the usual fix) and otherwise it's above the upper bound (too new, where a newer release won't help). So we could give a directional suggestion instead of the neutral one. Would you want that to be worked on in this PR, or as a separate issue?
There was a problem hiding this comment.
I don't have strong feelings, but it's probably easiest to do it in a separate pull request.
| /// Target interpreter version, used to detect `requires-python` mismatches. | ||
| python_version: Option<Version>, |
There was a problem hiding this comment.
How would this work for universal resolution? Can't the build environments use arbitrary different interpreter versions in that case? (I'd need to investigate our behavior to confirm)
There was a problem hiding this comment.
I looked into this. The hint is only wired into pip install and pip sync, and it's fed interpreter.python_version() from the single running interpreter. Neither command supports universal resolution (no --universal; both resolve with ResolverEnvironment::specific), and sdists built during resolution build against that same interpreter, so there's one well-defined version in scope here. It'd be a real concern if this hint were added to pip compile --universal, but it isn't currently.
There was a problem hiding this comment.
I think these build failures could happen in uv lock or uv pip compile too. Are we perhaps attaching the hint in the wrong location? e.g., should we be generating this hint immediately following the build failure where we have information about the interpreter used for the build environment rather than the interpreter used for the resolve?
| let report = miette::Report::new(Diagnostic { kind, dist, cause }); | ||
| anstream::eprint!("{report:?}"); | ||
| anstream::eprint!("{hints}"); | ||
| } | ||
|
|
||
| /// Format a hint when a build failure is caused by a `requires-python` mismatch. |
There was a problem hiding this comment.
We'll want some sort of test coverage for this. It should be doable by creating a test package with a custom build backend. We have other tests that do so for reference.
There was a problem hiding this comment.
I have added unit tests on the message builder. I tried for an end-to-end test too but got a bit stuck as the hint only fires when a registry dist fails to build, i.e. one with a File that carries requires-python, and I couldn't get the fixtures to line that up. packse packages build fine so there's no failure to trigger it, and the path-dep / custom build backend tests don't go through a registry File. Is there a reference test you had in mind for the registry-sdist-build-failure case? If it needs a new fixture I'm happy to add one, but wanted to verify that I'm following the right pattern first.
There was a problem hiding this comment.
Why do we only fire the hint for a registry distribution? We should have requires-python metadata for all distributions, right?
If you require a registry distribution, then yeah it's harder to test — but I'm not sure we actually want to limit it to that scope.
There was a problem hiding this comment.
I looked into it further and the cleaner fix seems to be generating the hint at the build-failure site in uv-distribution instead of in the resolve-side diagnostics. At that point we have the build environment's interpreter (source/mod.rs has it via build_context.interpreter()), and we can get requires-python for any dist type, either from the File for registry dists or statically from the source tree's pyproject.toml for path/git/etc. That'd make the hint fire for uv lock and uv pip compile too, use the right interpreter, and it's testable with the custom build backend approach you mentioned.
Does this direction sound right to you before I rework it?
There was a problem hiding this comment.
That sounds correct yeah, that's the same as my suggestion at #19673 (comment)
| } | ||
| Some(format!( | ||
| "`{}` declares `requires-python = \"{}\"`, but you are using Python {}. \ | ||
| This is the most likely cause of the build failure; try a newer version of `{}` \ |
There was a problem hiding this comment.
What if the python_version is below the requires_python range? Wouldn't we need to suggest they use an older version then? (This could be hard to detect, I'm not sure if we should attempt to do so, but I am wary of a case where we suggest the wrong thing)
|
Thanks for the pull! Always appreciate improvements to errors :) |
|
Thanks for the quick and thorough review @zanieb! Working through all of these now. Will follow up with the updates soon. |
12423db to
d215b22
Compare
uv test inventory changesThis PR changes the tests when compared with the
|
|
@zanieb, I have pushed the rework. The hint is now generated at the build-failure site in There were a couple of things I found while doing it, listed here:
I also removed the old resolve-side plumbing ( |
3a21350 to
3997bb7
Compare
|
Bumping this @zanieb. The rework is pushed (hint generated at the build-failure site, covers all distribution types and uses the build-env interpreter, per what we agreed). Let me know if the approach looks right or if you'd like anything adjusted. |
29112ec to
1f170e5
Compare
|
I'll try to take a look next week, feel free to ping me if I forget again 😬 |
49ef355 to
190be2d
Compare
|
Thanks! No worries on timing. Here's a quick heads-up on what changed since you last looked, so it's fresh when you get to it: I rebased on main and reworked the hint to follow the same shape as #20157. The requires-python context is attached to the build error and the hint is produced in the The wording is now neutral, matching the |
|
@zanieb pinging, per your direction. The changes since last time are summarized in my note above. Thanks! |
| /// Registry distributions carry `requires-python` in their file metadata. For source trees | ||
| /// (path, Git, directory, direct URL) it isn't available there, so we read it statically from | ||
| /// the `pyproject.toml` at `source_root` instead. Returns `None` if it can't be determined |
There was a problem hiding this comment.
I would move the first two sentences above the corresponding code blocks below as inline comments and just have the "Returns..." sentence here.
When would we be unable to determine the requires-python?
There was a problem hiding this comment.
build_requires_python returns None in three cases, all on source trees since registry dists always carry it in their File:
- no readable or parseable
pyproject.toml, e.g. a legacysetup.pyorsetup.cfgonly project, or a read error - a
pyproject.tomlwith norequires-python, or a dynamic one (declared inproject.dynamic, so there is no static value to read) - a malformed
requires-pythonspecifier, which we log atdebug!and treat as absent
In all three cases annotate_build_failure returns the original build error unchanged and attaches no hint. This is since if we cannot establish requires-python we do not want to guess at the cause.
|
The structure of this is not quite what I'd expect, I'm looking into a refactor to share. |
2bc7bf1 to
5346e25
Compare
|
Sounds good! One heads-up so it doesn't cost you time: the red CI we got earlier was because #20630 changed the |
|
I think I'm expecting something more like 099ac36 |
5346e25 to
65ee187
Compare
|
Took your branch and rebased it onto main, squashed into one commit. Two things on top of it. First, the Computing |
When a source distribution build fails, uv gives no indication that the build interpreter may simply be outside the package's `requires-python` range, which is a common cause for older packages whose build backends predate the running Python. Carry the `requires-python` and build-environment interpreter version on the build error itself and emit the hint from its `Hint` impl. Because the context is attached where the build fails, the hint covers every command that builds (`pip install`, `pip sync`, `pip compile`, `lock`) and uses the build environment's interpreter rather than the resolve interpreter. The existing pre-build `requires-python` guard only consulted registry file metadata, so it never fired for source trees. It now reads `requires-python` statically from `pyproject.toml` when the distribution has no file metadata, which also makes the hint reachable for path, Git, and direct URL sources. Closes astral-sh#7035 Co-authored-by: Zanie Blue <[email protected]>
65ee187 to
30a4467
Compare
Summary
This PR closes #7035.
Many transitive build failures on old package versions are caused by the failing distribution's declared requires-python excluding the active interpreter. The underlying build-backend error (often a deep SyntaxError or RuntimeError) doesn't say this directly, and users frequently file confused bug reports as a result.
This PR surfaces a targeted hint when a distribution fails to build and its requires-python doesn't include the interpreter used for the build environment. The hint is attached at the point of the build failure, so it applies to any build path rather than being wired into individual commands. It follows the same approach as #20157: the requires-python context is attached to the build error, and the hint is produced in the error's
Hintimplementation, so the error itself renders unchanged apart from the added hint.The hint is prepended so it appears before the existing derivation-chain hint and the generic build-failure hint, since it diagnoses the likely root cause.
Example
Installing
numba==0.53.1on Python 3.12. Numba transitively depends onllvmlite==0.36.0, whose declared requires-python is>=3.6,<3.10:Before
After
Scope
Because the hint is attached at the build-failure site rather than in a specific command, it fires wherever a build fails with a requires-python mismatch, including
uv pip installanduv pip sync. The wording is kept neutral rather than telling the user to go up or down a version, since either direction can be correct depending on the mismatch. The minor version is shown (3.12), sincerequires-pythonis expressed in minor versions; happy to show the full version if you'd prefer.Test plan
cargo fmtcleancargo clippyclean on the touched cratesrequire_hashes_find_links_no_hashandbuild_requires_python_hint)