Conversation
| let name = pyproject_toml | ||
| .project | ||
| .as_ref() | ||
| .map(|project| &project.name) | ||
| .or(package_name); | ||
| let extra_build_dependencies = name | ||
| .as_ref() | ||
| .and_then(|name| extra_build_dependencies.get(name).cloned()) | ||
| .unwrap_or_default(); | ||
|
|
||
| let backend = if let Some(mut build_system) = pyproject_toml.build_system { | ||
| // Apply extra-build-dependencies if there are any | ||
| build_system.requires.extend(extra_build_dependencies); |
There was a problem hiding this comment.
Happy path where I'm confident Sources/Indexes are appropriately handled.
| } | ||
| default_backend.clone() | ||
| let mut backend = default_backend.clone(); | ||
| // Apply extra_build_dependencies | ||
| // TODO(Gankra): should Sources/Indexes be applied on this path? | ||
| backend | ||
| .requirements | ||
| .extend(extra_build_dependencies.into_iter().map(Requirement::from)); | ||
| backend | ||
| }; |
There was a problem hiding this comment.
Sad path 1 where I probably need to handle sources where previously they were always irrelevant (pyproject.toml but missing [build-system])
| .requirements | ||
| .extend(extra_build_dependencies.into_iter().map(Requirement::from)); | ||
| Ok((backend, None)) | ||
| } |
There was a problem hiding this comment.
Sad path 2 where I am also probably mishandling sources/indexes (no pyproject.toml).
| [tool.uv.extra-build-dependencies] | ||
| fasttext = ["setuptools", "wheel", "pybind11"] | ||
| "#, | ||
| )?; | ||
|
|
||
| uv_snapshot!(&filters, context.sync(), @r" | ||
| success: true | ||
| exit_code: 0 | ||
| ----- stdout ----- | ||
|
|
||
| ----- stderr ----- | ||
| Resolved [N] packages in [TIME] | ||
| Prepared [N] packages in [TIME] | ||
| Installed [N] packages in [TIME] | ||
| + fasttext==0.9.2 | ||
| + myproject==0.1.0 (from file://[TEMP_DIR]/) | ||
| + numpy==1.26.4 | ||
| + pybind11==2.11.1 | ||
| + setuptools==69.2.0 | ||
| "); |
|
I think this effectively fixes #12447 |
|
The test fails to pass for me locally, with the same error as when I try to install asttext==0.9.2 on its own. Does it need a specific, older compiler version? |
|
Oh hrm that's annoying, there's other packages I could use for this problem though. |
|
We should pick something where the successful build is still fast, we had problems with slow builds in test the past. The fastest should be handrolling something with |
|
Any updates on this pull request? I am having build isolation issues (#2252). Thanks for your time and effort! |
b2caf47 to
c4245ca
Compare
59fe8f9 to
4ca5770
Compare
Replaces #14092 Adds `tool.uv.extra-build-dependencies = {package = [dependency, ...]}` which extends `build-system.requires` during package builds. These are lowered via workspace sources, are applied to transitive dependencies, and are included in the wheel cache shard hash. There are some features we need to follow-up on, but are out of scope here: - Preferring locked versions for build dependencies - Settings for requiring locked versions for build depencies There are some quality of life follow-ups we should also do: - Warn on `extra-build-dependencies` that do not apply to any packages - Add test cases and improve error messaging when the `extra-build-dependencies` resolve fails ------- There ~are~ were a few open decisions to be made here 1. Should we resolve these dependencies alongside the `build-system.requires` dependencies? Or should we resolve separately? (I think the latter is more powerful? because you can override things? but it opens the door to breaking your build) 2. Should we install these dependencies into the same environment? Or should we layer it on top as we do elsewhere? (I think it's fine to install into the same environment) 3. Should we respect sources defined in the parent project? (I think yes, but then we need to lower the dependencies earlier — I don't think that's a big deal, but it's not implemented) 4. Should we respect sources defined in the child project? (I think no, this gets really complicated and seems weird to allow) 5. Should we apply this to transitive dependencies? (I think so) --------- Co-authored-by: Aria Desires <aria.desires@gmail.com> Co-authored-by: konstin <konstin@mailbox.org>
This introduces a global setting
Which forces extra requirements into a build-isolated sdist build for any package that has the given name. Included is a test the demonstrates the feature is at all useful/functional using an example from
no-build-isolation-packagewas used as the template for all the places this setting should be threaded, as they both mess with the isolated build of a package by-name. There's a very high chance this implementation is insufficient, as I didn't go out of my way to do anything with caching (but maybe I ended up shoving it somewhere that does).