refactor: make the channel in the matchspec struct an actual channel (c…

…onda#401) This should really help make installation more correct. I got the issue that `conda-forge::python` and `conda::python` got the same result after installation. So I changed the type of the channel to be a channel from the start. --------- Co-authored-by: Bas Zalmstra <[email protected]>
Wackyator · Nov 14, 2023 · f839abd · f839abd
1 parent e75bbe2
commit f839abd
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 24 deletions.
diff --git a/crates/rattler_conda_types/Cargo.toml b/crates/rattler_conda_types/Cargo.toml
@@ -19,7 +19,7 @@ itertools = "0.11.0"
 lazy-regex = "3.0.2"
 nom = "7.1.3"
 regex = "1.9.6"
-serde = { version = "1.0.188", features = ["derive"] }
+serde = { version = "1.0.188", features = ["derive", "rc"] }
 serde_json = "1.0.107"
 serde_yaml = "0.9.25"
 serde_with = { version = "3.3.0", features = ["indexmap_2"] }

diff --git a/crates/rattler_conda_types/src/match_spec/mod.rs b/crates/rattler_conda_types/src/match_spec/mod.rs
@@ -1,9 +1,13 @@
 use crate::{build_spec::BuildNumberSpec, PackageName, PackageRecord, VersionSpec};
 use rattler_digest::{serde::SerializableHash, Md5Hash, Sha256Hash};
-use serde::{Deserialize, Serialize};
+use serde::{Deserialize, Deserializer, Serialize};
 use serde_with::{serde_as, skip_serializing_none, DisplayFromStr};
 use std::fmt::{Debug, Display, Formatter};
 use std::hash::Hash;
+use std::sync::Arc;
+
+use crate::Channel;
+use crate::ChannelConfig;
 
 pub mod matcher;
 pub mod parse;
@@ -64,8 +68,9 @@ use matcher::StringMatcher;
 /// # Examples:
 ///
 /// ```rust
-/// use rattler_conda_types::{MatchSpec, VersionSpec, StringMatcher, PackageName};
+/// use rattler_conda_types::{MatchSpec, VersionSpec, StringMatcher, PackageName, Channel};
 /// use std::str::FromStr;
+/// use std::sync::Arc;
 ///
 /// let spec = MatchSpec::from_str("foo 1.0 py27_0").unwrap();
 /// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo")));
@@ -80,18 +85,18 @@ use matcher::StringMatcher;
 /// let spec = MatchSpec::from_str(r#"conda-forge::foo[version="1.0.*"]"#).unwrap();
 /// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo")));
 /// assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap()));
-/// assert_eq!(spec.channel, Some("conda-forge".to_string()));
+/// assert_eq!(spec.channel, Some(Channel::from_str("conda-forge", &Default::default()).map(|channel| Arc::new(channel)).unwrap()));
 ///
 /// let spec = MatchSpec::from_str("conda-forge/linux-64::foo>=1.0").unwrap();
 /// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo")));
 /// assert_eq!(spec.version, Some(VersionSpec::from_str(">=1.0").unwrap()));
-/// assert_eq!(spec.channel, Some("conda-forge".to_string()));
+/// assert_eq!(spec.channel, Some(Channel::from_str("conda-forge", &Default::default()).map(|channel| Arc::new(channel)).unwrap()));
 /// assert_eq!(spec.subdir, Some("linux-64".to_string()));
 ///
 /// let spec = MatchSpec::from_str("*/linux-64::foo>=1.0").unwrap();
 /// assert_eq!(spec.name, Some(PackageName::new_unchecked("foo")));
 /// assert_eq!(spec.version, Some(VersionSpec::from_str(">=1.0").unwrap()));
-/// assert_eq!(spec.channel, Some("*".to_string()));
+/// assert_eq!(spec.channel, Some(Channel::from_str("*", &Default::default()).map(|channel| Arc::new(channel)).unwrap()));
 /// assert_eq!(spec.subdir, Some("linux-64".to_string()));
 ///
 /// let spec = MatchSpec::from_str(r#"foo[build="py2*"]"#).unwrap();
@@ -125,7 +130,7 @@ pub struct MatchSpec {
     /// Match the specific filename of the package
     pub file_name: Option<String>,
     /// The channel of the package
-    pub channel: Option<String>,
+    pub channel: Option<Arc<Channel>>,
     /// The subdir of the channel
     pub subdir: Option<String>,
     /// The namespace of the package (currently not used)
@@ -141,8 +146,10 @@ pub struct MatchSpec {
 impl Display for MatchSpec {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         if let Some(channel) = &self.channel {
-            // TODO: namespace
-            write!(f, "{}", channel)?;
+            if let Some(name) = &channel.name {
+                // TODO: namespace
+                write!(f, "{}", name)?;
+            }
         }
 
         if let Some(subdir) = &self.subdir {
@@ -264,7 +271,8 @@ pub struct NamelessMatchSpec {
     /// Match the specific filename of the package
     pub file_name: Option<String>,
     /// The channel of the package
-    pub channel: Option<String>,
+    #[serde(deserialize_with = "deserialize_channel")]
+    pub channel: Option<Arc<Channel>>,
     /// The subdir of the channel
     pub subdir: Option<String>,
     /// The namespace of the package (currently not used)
@@ -371,6 +379,27 @@ impl MatchSpec {
     }
 }
 
+/// Deserialize channel from string
+/// TODO: This should be refactored so that the front ends are the one setting the channel config,
+/// and rattler only takes care of the url.
+fn deserialize_channel<'de, D>(deserializer: D) -> Result<Option<Arc<Channel>>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let s: Option<String> = Option::deserialize(deserializer)?;
+
+    match s {
+        Some(str_val) => {
+            let config = ChannelConfig::default();
+
+            Channel::from_str(str_val, &config)
+                .map(|channel| Some(Arc::new(channel)))
+                .map_err(serde::de::Error::custom)
+        }
+        None => Ok(None),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::str::FromStr;

diff --git a/crates/rattler_conda_types/src/match_spec/parse.rs b/crates/rattler_conda_types/src/match_spec/parse.rs
@@ -5,7 +5,8 @@ use crate::package::ArchiveType;
 use crate::version_spec::version_tree::{recognize_constraint, recognize_version};
 use crate::version_spec::{is_start_of_version_constraint, ParseVersionSpecError};
 use crate::{
-    InvalidPackageNameError, NamelessMatchSpec, PackageName, ParseChannelError, VersionSpec,
+    Channel, InvalidPackageNameError, NamelessMatchSpec, PackageName, ParseChannelError,
+    VersionSpec,
 };
 use nom::branch::alt;
 use nom::bytes::complete::{tag, take_till1, take_until, take_while, take_while1};
@@ -397,10 +398,12 @@ fn parse(input: &str) -> Result<MatchSpec, ParseMatchSpecError> {
 
     if let Some(channel_str) = channel_str {
         if let Some((channel, subdir)) = channel_str.rsplit_once('/') {
-            nameless_match_spec.channel = Some(channel.to_string());
+            nameless_match_spec.channel =
+                Some(Channel::from_str(channel, &Default::default())?.into());
             nameless_match_spec.subdir = Some(subdir.to_string());
         } else {
-            nameless_match_spec.channel = Some(channel_str.to_string());
+            nameless_match_spec.channel =
+                Some(Channel::from_str(channel_str, &Default::default())?.into());
         }
     }
 
@@ -468,12 +471,13 @@ mod tests {
     use serde::Serialize;
     use std::collections::BTreeMap;
     use std::str::FromStr;
+    use std::sync::Arc;
 
     use super::{
         split_version_and_build, strip_brackets, BracketVec, MatchSpec, ParseMatchSpecError,
     };
     use crate::match_spec::parse::parse_bracket_list;
-    use crate::{BuildNumberSpec, NamelessMatchSpec, VersionSpec};
+    use crate::{BuildNumberSpec, Channel, NamelessMatchSpec, VersionSpec};
     use smallvec::smallvec;
 
     #[test]
@@ -573,18 +577,39 @@ mod tests {
         let spec = MatchSpec::from_str("conda-forge::foo[version=\"1.0.*\"]").unwrap();
         assert_eq!(spec.name, Some("foo".parse().unwrap()));
         assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap()));
-        assert_eq!(spec.channel, Some("conda-forge".to_string()));
+        assert_eq!(
+            spec.channel,
+            Some(
+                Channel::from_str("conda-forge", &Default::default())
+                    .map(|channel| Arc::new(channel))
+                    .unwrap()
+            )
+        );
 
         let spec = MatchSpec::from_str("conda-forge::foo[version=1.0.*]").unwrap();
         assert_eq!(spec.name, Some("foo".parse().unwrap()));
         assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap()));
-        assert_eq!(spec.channel, Some("conda-forge".to_string()));
+        assert_eq!(
+            spec.channel,
+            Some(
+                Channel::from_str("conda-forge", &Default::default())
+                    .map(|channel| Arc::new(channel))
+                    .unwrap()
+            )
+        );
 
         let spec =
             MatchSpec::from_str(r#"conda-forge::foo[version=1.0.*, build_number=">6"]"#).unwrap();
         assert_eq!(spec.name, Some("foo".parse().unwrap()));
         assert_eq!(spec.version, Some(VersionSpec::from_str("1.0.*").unwrap()));
-        assert_eq!(spec.channel, Some("conda-forge".to_string()));
+        assert_eq!(
+            spec.channel,
+            Some(
+                Channel::from_str("conda-forge", &Default::default())
+                    .map(|channel| Arc::new(channel))
+                    .unwrap()
+            )
+        );
         assert_eq!(
             spec.build_number,
             Some(BuildNumberSpec::from_str(">6").unwrap())

diff --git a/crates/rattler_solve/src/resolvo/mod.rs b/crates/rattler_solve/src/resolvo/mod.rs
@@ -259,15 +259,19 @@ impl<'a> CondaDependencyProvider<'a> {
                     }) {
                         // Check if the spec has a channel, and compare it to the repodata channel
                         if let Some(spec_channel) = &spec.channel {
-                            if !&record.channel.contains(spec_channel) {
+                            if record.channel != spec_channel.base_url.to_string() {
                                 tracing::debug!("Ignoring {} from {} because it was not requested from that channel.", &record.package_record.name.as_normalized(), &record.channel);
                                 // Add record to the excluded with reason of being in the non requested channel.
-                                candidates.excluded.push((
-                                    solvable_id,
-                                    pool.intern_string(format!(
-                                        "candidate not in requested channel: '{spec_channel}'"
-                                    )),
-                                ));
+                                let message = format!(
+                                    "candidate not in requested channel: '{}'",
+                                    spec_channel
+                                        .name
+                                        .clone()
+                                        .unwrap_or(spec_channel.base_url.to_string())
+                                );
+                                candidates
+                                    .excluded
+                                    .push((solvable_id, pool.intern_string(message)));
                                 continue;
                             }
                         }