feat: no-build option #2997
feat: no-build option #2997
Conversation
|
@ruben-arts This should be ready for a review, and should already work without the other functionality mentioned in this PR. |
| @@ -342,6 +397,7 @@ mod tests { | |||
| find_links: None, | |||
| no_build_isolation: None, | |||
| index_strategy: Some(IndexStrategy::UnsafeBestMatch), | |||
| no_build: Default::default(), | |||
There was a problem hiding this comment.
Please also test a merge of these options
There was a problem hiding this comment.
Now testing a lot more!
| pub enum NoBuild { | ||
| /// Build any sdist we come across | ||
| #[default] | ||
| None, |
There was a problem hiding this comment.
I thought you wanted to make this either true or false or a list of names. I don't believe all and none are going to make it easier, and it makes it less aligned with uv, although it already isn't ;)
no-build = trueor
no-build = ["numpy"]There was a problem hiding this comment.
Changed it to like you said!
| impl NoBuild { | ||
| /// Merges two `NoBuild` together, according to the following rules | ||
| /// - If either is `All`, the result is `All` | ||
| /// - If either is `None`, the result is the other |
There was a problem hiding this comment.
I think we're missing the "undefined" case here.
I believe this function is most important for the feature merging. The priority of the other merges should be followed. So the one with the higher priority should win. Apart from the list of packages, there I believe we should extend the list.
In the current case if some feature makes it no-build, you can't override it with a higher priority feature.
If my explanation is not clear, let's call about it.
There was a problem hiding this comment.
Indeed, that was a clear oversight corrected this. And in conjunction also made it a HashSet of packages to avoid duplication, although that should not have mattered much.
|
@ruben-arts I also some some incosistencies in the uv conversions, because I had to touch these I also:
|
| [pypi-options] | ||
| no-build = ["package1", "package2"] | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Please document what the override logic is of these options
There was a problem hiding this comment.
I tried to do so can you give it a check? Tried to err on the side of being succinct.
Co-authored-by: Ruben Arts <[email protected]>
| @@ -20,8 +20,8 @@ use pixi_manifest::FeaturesExt; | |||
| use pixi_record::{LockedGitUrl, ParseLockFileError, PixiRecord, SourceMismatchError}; | |||
| use pixi_spec::{PixiSpec, SourceSpec, SpecConversionError}; | |||
| use pixi_uv_conversions::{ | |||
| as_uv_req, as_uv_specifiers, as_uv_version, into_pixi_reference, to_normalize, | |||
There was a problem hiding this comment.
If an environment is solved with no-build = false and later that changes to true, we should resolve if we depend on sdists. Assuming we can just look for tar.gz that shouldn't be to hard right?
We can skip the other way around as that doesn't invalidate the manifest with the lockfile and a pixi update should fix those issues.
This PR adds the
no-buildoption for pypi-dependencies that dis-allow the building of sdists, we are planning in using this in conjunction with: #2994 to have a much faster solve for environments that do not need the full conda prefix and will not build PyPI source distributions.This PR will be standalone in the fact that it can be used without the features listed above.
This is what the error looks like when no-build is enabled: