feat: Add no-build to pypi-options (#2997)

Co-authored-by: Ruben Arts <[email protected]>
Co-authored-by: Ruben Arts <[email protected]>

crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__workspace__tests__pypi_options_default_feature.snap

@@ -1,6 +1,6 @@
 ---
 source: crates/pixi_manifest/src/manifests/workspace.rs
-expression: "toml_edit::de::from_str::<WorkspaceManifest>(&contents).expect(\"parsing should succeed!\").workspace.pypi_options.clone().unwrap()"
+expression: "WorkspaceManifest::from_toml_str(&contents).expect(\"parsing should succeed!\").workspace.pypi_options.clone().unwrap()"
 ---
 index-url: "https://pypi.org/simple"
 extra-index-urls:
@@ -10,3 +10,4 @@ find-links:
   - url: "https://example.com/bar"
 no-build-isolation: ~
 index-strategy: ~
+no-build: ~

crates/pixi_manifest/src/manifests/snapshots/pixi_manifest__manifests__workspace__tests__pypy_options_project_and_default_feature.snap

@@ -8,3 +8,4 @@ extra-index-urls:
 find-links: ~
 no-build-isolation: ~
 index-strategy: ~
+no-build: ~

crates/pixi_manifest/src/pypi/pypi_options.rs

@@ -1,4 +1,4 @@
-use std::{hash::Hash, path::PathBuf};
+use std::{collections::HashSet, hash::Hash, path::PathBuf};
 
 use indexmap::IndexSet;
 use serde::Serialize;
@@ -48,6 +48,40 @@ pub enum FindLinksUrlOrPath {
     Url(Url),
 }
 
+/// Don't build sdist for all or certain packages
+#[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, strum::Display)]
+#[strum(serialize_all = "kebab-case")]
+#[serde(rename_all = "kebab-case")]
+pub enum NoBuild {
+    /// Build any sdist we come across
+    #[default]
+    None,
+    /// Don't build any sdist
+    All,
+    /// Don't build sdist for specific packages
+    // Todo: would be nice to check if these are actually used at some point
+    Packages(HashSet<String>),
+}
+
+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
+    /// - If both are `Packages`, the result is the union of the two
+    pub fn union(&self, other: &NoBuild) -> NoBuild {
+        match (self, other) {
+            (NoBuild::All, _) | (_, NoBuild::All) => NoBuild::All,
+            (NoBuild::None, _) => other.clone(),
+            (_, NoBuild::None) => self.clone(),
+            (NoBuild::Packages(packages), NoBuild::Packages(other_packages)) => {
+                let mut packages = packages.clone();
+                packages.extend(other_packages.iter().cloned());
+                NoBuild::Packages(packages)
+            }
+        }
+    }
+}
+
 /// Specific options for a PyPI registries
 #[derive(Debug, Clone, PartialEq, Serialize, Eq, Default)]
 #[serde(rename_all = "kebab-case")]
@@ -63,6 +97,8 @@ pub struct PypiOptions {
     pub no_build_isolation: Option<Vec<String>>,
     /// The strategy to use when resolving against multiple index URLs.
     pub index_strategy: Option<IndexStrategy>,
+    /// Don't build sdist for all or certain packages
+    pub no_build: Option<NoBuild>,
 }
 
 /// Clones and deduplicates two iterators of values
@@ -85,13 +121,15 @@ impl PypiOptions {
         flat_indexes: Option<Vec<FindLinksUrlOrPath>>,
         no_build_isolation: Option<Vec<String>>,
         index_strategy: Option<IndexStrategy>,
+        no_build: Option<NoBuild>,
     ) -> Self {
         Self {
             index_url: index,
             extra_index_urls: extra_indexes,
             find_links: flat_indexes,
             no_build_isolation,
             index_strategy,
+            no_build,
         }
     }
 
@@ -190,12 +228,21 @@ impl PypiOptions {
             })
             .or_else(|| other.no_build_isolation.clone());
 
+        // Set the no-build option
+        let no_build = match (self.no_build.as_ref(), other.no_build.as_ref()) {
+            (Some(a), Some(b)) => Some(a.union(b)),
+            (Some(a), None) => Some(a.clone()),
+            (None, Some(b)) => Some(b.clone()),
+            (None, None) => None,
+        };
+
         Ok(PypiOptions {
             index_url: index,
             extra_index_urls: extra_indexes,
             find_links: flat_indexes,
             no_build_isolation,
             index_strategy,
+            no_build,
         })
     }
 }
@@ -278,6 +325,7 @@ mod tests {
             ]),
             no_build_isolation: Some(vec!["foo".to_string(), "bar".to_string()]),
             index_strategy: None,
+            no_build: None,
         };
 
         // Create the second set of options
@@ -290,6 +338,7 @@ mod tests {
             ]),
             no_build_isolation: Some(vec!["foo".to_string()]),
             index_strategy: None,
+            no_build: Some(NoBuild::All),
         };
 
         // Merge the two options
@@ -298,6 +347,40 @@ mod tests {
         insta::assert_yaml_snapshot!(merged_opts);
     }
 
+    #[test]
+    fn test_no_build_union() {
+        // Case 1: One is `All`, result should be `All`
+        assert_eq!(NoBuild::All.union(&NoBuild::None), NoBuild::All);
+        assert_eq!(NoBuild::None.union(&NoBuild::All), NoBuild::All);
+        assert_eq!(
+            NoBuild::All.union(&NoBuild::Packages(HashSet::from_iter(["pkg1".to_string()]))),
+            NoBuild::All
+        );
+
+        // Case 2: One is `None`, result should be the other
+        assert_eq!(NoBuild::None.union(&NoBuild::None), NoBuild::None);
+        assert_eq!(
+            NoBuild::None.union(&NoBuild::Packages(HashSet::from_iter(["pkg1".to_string()]))),
+            NoBuild::Packages(HashSet::from_iter(["pkg1".to_string()]))
+        );
+        assert_eq!(
+            NoBuild::Packages(HashSet::from_iter(["pkg1".to_string()])).union(&NoBuild::None),
+            NoBuild::Packages(HashSet::from_iter(["pkg1".to_string()]))
+        );
+
+        // Case 3: Both are `Packages`, result should be the union of the two
+        assert_eq!(
+            NoBuild::Packages(HashSet::from_iter(["pkg1".to_string(), "pkg2".to_string()])).union(
+                &NoBuild::Packages(HashSet::from_iter(["pkg2".to_string(), "pkg3".to_string()]))
+            ),
+            NoBuild::Packages(HashSet::from_iter([
+                "pkg1".to_string(),
+                "pkg2".to_string(),
+                "pkg3".to_string()
+            ]))
+        );
+    }
+
     #[test]
     fn test_error_on_multiple_primary_indexes() {
         // Create the first set of options
@@ -307,6 +390,7 @@ mod tests {
             find_links: None,
             no_build_isolation: None,
             index_strategy: None,
+            no_build: Default::default(),
         };
 
         // Create the second set of options
@@ -316,6 +400,7 @@ mod tests {
             find_links: None,
             no_build_isolation: None,
             index_strategy: None,
+            no_build: Default::default(),
         };
 
         // Merge the two options
@@ -333,6 +418,7 @@ mod tests {
             find_links: None,
             no_build_isolation: None,
             index_strategy: Some(IndexStrategy::FirstIndex),
+            no_build: Default::default(),
         };
 
         // Create the second set of options
@@ -342,6 +428,7 @@ mod tests {
             find_links: None,
             no_build_isolation: None,
             index_strategy: Some(IndexStrategy::UnsafeBestMatch),
+            no_build: Default::default(),
         };
 
         // Merge the two options

crates/pixi_manifest/src/pypi/snapshots/pixi_manifest__pypi__pypi_options__tests__merge_pypi_options.snap

@@ -15,3 +15,4 @@ no-build-isolation:
   - foo
   - bar
 index-strategy: ~
+no-build: all

crates/pixi_manifest/src/toml/pypi_options.rs

@@ -1,4 +1,4 @@
-use std::path::PathBuf;
+use std::{collections::HashSet, path::PathBuf};
 
 use pixi_toml::{TomlEnum, TomlFromStr, TomlWith};
 use toml_span::{
@@ -8,7 +8,45 @@ use toml_span::{
 };
 use url::Url;
 
-use crate::pypi::pypi_options::{FindLinksUrlOrPath, PypiOptions};
+use crate::pypi::pypi_options::{FindLinksUrlOrPath, NoBuild, PypiOptions};
+
+impl<'de> toml_span::Deserialize<'de> for NoBuild {
+    fn deserialize(value: &mut Value<'de>) -> Result<Self, DeserError> {
+        // It can be either `true` or `false` or an array of strings
+        if value.as_bool().is_some() {
+            if bool::deserialize(value)? {
+                return Ok(NoBuild::All);
+            } else {
+                return Ok(NoBuild::None);
+            }
+        }
+        // We assume it's an array of strings
+        if value.as_array().is_some() {
+            match value.take() {
+                ValueInner::Array(array) => {
+                    let mut packages = HashSet::with_capacity(array.len());
+                    for mut value in array {
+                        packages.insert(value.take_string(None)?.into_owned());
+                    }
+                    Ok(NoBuild::Packages(packages))
+                }
+                _ => Err(expected(
+                    "an array of packages e.g. [\"foo\", \"bar\"]",
+                    value.take(),
+                    value.span,
+                )
+                .into()),
+            }
+        } else {
+            Err(expected(
+                r#"either "all", "none" or an array of packages e.g. ["foo", "bar"] "#,
+                value.take(),
+                value.span,
+            )
+            .into())
+        }
+    }
+}
 
 impl<'de> toml_span::Deserialize<'de> for PypiOptions {
     fn deserialize(value: &mut Value<'de>) -> Result<Self, DeserError> {
@@ -26,6 +64,8 @@ impl<'de> toml_span::Deserialize<'de> for PypiOptions {
             .optional::<TomlEnum<_>>("index-strategy")
             .map(TomlEnum::into_inner);
 
+        let no_build = th.optional::<NoBuild>("no-build");
+
         th.finalize(None)?;
 
         Ok(Self {
@@ -34,6 +74,7 @@ impl<'de> toml_span::Deserialize<'de> for PypiOptions {
             find_links,
             no_build_isolation,
             index_strategy,
+            no_build,
         })
     }
 }
@@ -149,6 +190,7 @@ mod test {
                 ]),
                 no_build_isolation: Some(vec!["pkg1".to_string(), "pkg2".to_string()]),
                 index_strategy: None,
+                no_build: Default::default(),
             },
         );
     }
@@ -164,6 +206,16 @@ mod test {
         ]
         no-build-isolation = ["sigma"]
         index-strategy = "first-index"
+        no-build = true
+        "#;
+        let options = PypiOptions::from_toml_str(input).unwrap();
+        assert_debug_snapshot!(options);
+    }
+
+    #[test]
+    fn test_no_build_packages() {
+        let input = r#"
+        no-build = ["package1"]
         "#;
         let options = PypiOptions::from_toml_str(input).unwrap();
         assert_debug_snapshot!(options);
@@ -257,4 +309,19 @@ mod test {
         "###
         )
     }
+
+    #[test]
+    fn test_wrong_build_option_type() {
+        let input = r#"no-build = 3"#;
+        assert_snapshot!(format_parse_error(
+            input,
+            PypiOptions::from_toml_str(input).unwrap_err()
+        ), @r###"
+         × expected either "all", "none" or an array of packages e.g. ["foo", "bar"] , found integer
+          ╭─[pixi.toml:1:12]
+        1 │ no-build = 3
+          ·            ─
+          ╰────
+        "###)
+    }
 }

crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__pypi_options__test__full.snap

@@ -82,4 +82,7 @@ PypiOptions {
     index_strategy: Some(
         FirstIndex,
     ),
+    no_build: Some(
+        All,
+    ),
 }

crates/pixi_manifest/src/toml/snapshots/pixi_manifest__toml__pypi_options__test__no_build_packages.snap

@@ -0,0 +1,18 @@
+---
+source: crates/pixi_manifest/src/toml/pypi_options.rs
+expression: options
+---
+PypiOptions {
+    index_url: None,
+    extra_index_urls: None,
+    find_links: None,
+    no_build_isolation: None,
+    index_strategy: None,
+    no_build: Some(
+        Packages(
+            {
+                "package1",
+            },
+        ),
+    ),
+}

crates/pixi_manifest/src/validation.rs

@@ -9,8 +9,8 @@ use std::{
 
 use super::pypi::pypi_options::PypiOptions;
 use crate::{
-    Environment, Feature, FeatureName, KnownPreviewFeature, SystemRequirements, TargetSelector,
-    WorkspaceManifest,
+    pypi::pypi_options::NoBuild, Environment, Feature, FeatureName, KnownPreviewFeature,
+    SystemRequirements, TargetSelector, WorkspaceManifest,
 };
 
 impl WorkspaceManifest {
@@ -236,7 +236,7 @@ impl WorkspaceManifest {
         }
 
         // Check if there are no conflicts in pypi options between features
-        features
+        let opts = features
             .iter()
             .chain(default)
             .filter_map(|feature| {
@@ -250,6 +250,23 @@ impl WorkspaceManifest {
             .try_fold(PypiOptions::default(), |acc, opts| acc.union(opts))
             .into_diagnostic()?;
 
+        // If no-build is set, check if the package names are pep508 compliant
+        if let Some(NoBuild::Packages(packages)) = opts.no_build {
+            let packages = packages
+                .iter()
+                .map(|p| pep508_rs::PackageName::new(p.clone()))
+                .collect::<Result<Vec<_>, _>>();
+            if let Err(e) = packages {
+                return Err(miette::miette!(
+                    labels = vec![LabeledSpan::at(
+                        env.features_source_loc.clone().unwrap_or_default(),
+                        "while resolving no-build packages array"
+                    )],
+                    "{e}",
+                ));
+            }
+        }
+
         Ok(())
     }
 }