fix: parse ~= as version not as path (#804)

By adding the path parsing to the nameless matchspec I broke the parsing of `~= 1.2.3`. This PR changes a few things to better deal with that. - It now checks for `~/` to verify if it is a path - We can now parse `~/channel_name::package` as the tilde is now actually parsed instead of seen as part of the string. - Added a bunch more test cases to catch more issues in the future. --------- Co-authored-by: Bas Zalmstra <[email protected]>
conda · Aug 6, 2024 · 98a555d · 98a555d
1 parent a635839
commit 98a555d
Show file tree

Hide file tree

Showing 9 changed files with 314 additions and 63 deletions.
diff --git a/crates/rattler_conda_types/Cargo.toml b/crates/rattler_conda_types/Cargo.toml
@@ -38,10 +38,11 @@ typed-path = { workspace = true }
 url = { workspace = true, features = ["serde"] }
 indexmap = { workspace = true }
 rattler_redaction = { version = "0.1.0", path = "../rattler_redaction" }
+dirs = { workspace = true }
 
 [dev-dependencies]
 rand = { workspace = true }
-insta = { workspace = true, features = ["yaml", "redactions", "toml", "glob"] }
+insta = { workspace = true, features = ["yaml", "redactions", "toml", "glob", "filters"] }
 rattler_package_streaming = { path = "../rattler_package_streaming", default-features = false, features = ["rustls-tls"] }
 tempfile = { workspace = true }
 rstest = { workspace = true }

diff --git a/crates/rattler_conda_types/src/channel/mod.rs b/crates/rattler_conda_types/src/channel/mod.rs
@@ -6,13 +6,12 @@ use std::{
 };
 
 use file_url::directory_path_to_url;
+use rattler_redaction::Redact;
 use serde::{Deserialize, Serialize, Serializer};
 use thiserror::Error;
 use typed_path::{Utf8NativePathBuf, Utf8TypedPath, Utf8TypedPathBuf};
 use url::Url;
 
-use rattler_redaction::Redact;
-
 use super::{ParsePlatformError, Platform};
 use crate::utils::{
     path::is_path,
@@ -107,7 +106,9 @@ impl NamedChannelOrUrl {
             NamedChannelOrUrl::Name(name) => {
                 let mut base_url = config.channel_alias.clone();
                 if let Ok(mut segments) = base_url.path_segments_mut() {
-                    segments.push(&name);
+                    for segment in name.split(&['/', '\\']) {
+                        segments.push(segment);
+                    }
                 }
                 base_url
             }
@@ -391,11 +392,11 @@ pub enum ParseChannelError {
     InvalidName(String),
 
     /// The root directory is not an absolute path
-    #[error("root directory from channel config is not an absolute path")]
+    #[error("root directory: '{0}' from channel config is not an absolute path")]
     NonAbsoluteRootDir(PathBuf),
 
     /// The root directory is not UTF-8 encoded.
-    #[error("root directory of channel config is not utf8 encoded")]
+    #[error("root directory: '{0}' of channel config is not utf8 encoded")]
     NotUtf8RootDir(PathBuf),
 }
 
@@ -443,12 +444,24 @@ pub(crate) const fn default_platforms() -> &'static [Platform] {
 }
 
 /// Returns the specified path as an absolute path
-fn absolute_path(path: &str, root_dir: &Path) -> Result<Utf8TypedPathBuf, ParseChannelError> {
-    let path = Utf8TypedPath::from(path);
+fn absolute_path(path_str: &str, root_dir: &Path) -> Result<Utf8TypedPathBuf, ParseChannelError> {
+    let path = Utf8TypedPath::from(path_str);
     if path.is_absolute() {
         return Ok(path.normalize());
     }
 
+    // Parse the `~/` as the home folder
+    if let Ok(user_path) = path.strip_prefix("~/") {
+        return Ok(Utf8TypedPathBuf::from(
+            dirs::home_dir()
+                .ok_or(ParseChannelError::InvalidPath(path.to_string()))?
+                .to_str()
+                .ok_or(ParseChannelError::NotUtf8RootDir(PathBuf::from(path_str)))?,
+        )
+        .join(user_path)
+        .normalize());
+    }
+
     let root_dir_str = root_dir
         .to_str()
         .ok_or_else(|| ParseChannelError::NotUtf8RootDir(root_dir.to_path_buf()))?;
@@ -467,6 +480,7 @@ fn absolute_path(path: &str, root_dir: &Path) -> Result<Utf8TypedPathBuf, ParseC
 mod tests {
     use std::str::FromStr;
 
+    use typed_path::{NativePath, Utf8NativePath};
     use url::Url;
 
     use super::*;
@@ -515,6 +529,22 @@ mod tests {
             absolute_path("../foo", &current_dir).as_ref(),
             Ok(&parent_dir.join("foo"))
         );
+
+        let home_dir = dirs::home_dir()
+            .unwrap()
+            .into_os_string()
+            .into_encoded_bytes();
+        let home_dir = Utf8NativePath::from_bytes_path(NativePath::new(&home_dir))
+            .unwrap()
+            .to_typed_path();
+        assert_eq!(
+            absolute_path("~/unix_dir", &current_dir).unwrap(),
+            home_dir.join("unix_dir")
+        );
+        assert_eq!(
+            absolute_path("~/unix_dir/test/../test2", &current_dir).unwrap(),
+            home_dir.join("unix_dir").join("test2")
+        );
     }
 
     #[test]
@@ -665,18 +695,6 @@ mod tests {
         assert_eq!(channel.name.as_deref(), Some("conda-forge/label/rust_dev"));
     }
 
-    #[test]
-    fn test_is_path() {
-        assert!(is_path("./foo"));
-        assert!(is_path("/foo"));
-        assert!(is_path("~/foo"));
-        assert!(is_path("../foo"));
-        assert!(is_path("/C:/foo"));
-        assert!(is_path("C:/foo"));
-
-        assert!(!is_path("conda-forge/label/rust_dev"));
-    }
-
     #[test]
     fn channel_canonical_name() {
         let config = ChannelConfig::default_with_root_dir(std::env::current_dir().unwrap());
@@ -767,4 +785,25 @@ mod tests {
             assert!(!channel.base_url().as_str().ends_with("//"));
         }
     }
+
+    #[test]
+    fn test_compare_channel_and_named_channel_or_url() {
+        let channel_config = ChannelConfig {
+            channel_alias: Url::from_str("https://conda.anaconda.org").unwrap(),
+            root_dir: std::env::current_dir().expect("No current dir set"),
+        };
+        let named = NamedChannelOrUrl::Name("conda-forge".to_string());
+        let channel = Channel::from_str("conda-forge", &channel_config).unwrap();
+        assert_eq!(
+            &channel.base_url,
+            named.into_channel(&channel_config).base_url()
+        );
+
+        let named = NamedChannelOrUrl::Name("nvidia/label/cuda-11.8.0".to_string());
+        let channel = Channel::from_str("nvidia/label/cuda-11.8.0", &channel_config).unwrap();
+        assert_eq!(
+            channel.base_url(),
+            named.into_channel(&channel_config).base_url()
+        );
+    }
 }
diff --git a/crates/rattler_conda_types/src/match_spec/parse.rs b/crates/rattler_conda_types/src/match_spec/parse.rs
@@ -23,7 +23,7 @@ use super::{
 use crate::{
     build_spec::{BuildNumberSpec, ParseBuildNumberSpecError},
     package::ArchiveIdentifier,
-    utils::{path::is_path, url::parse_scheme},
+    utils::{path::is_absolute_path, url::parse_scheme},
     version_spec::{
         is_start_of_version_constraint,
         version_tree::{recognize_constraint, recognize_version},
@@ -251,8 +251,8 @@ fn parse_bracket_vec_into_components(
                 let url = if parse_scheme(value).is_some() {
                     Url::parse(value)?
                 }
-                // 2 Is the spec a path, parse it as an url
-                else if is_path(value) {
+                // 2 Is the spec an absolute path, parse it as an url
+                else if is_absolute_path(value) {
                     let path = Utf8TypedPath::from(value);
                     file_url::file_path_to_url(path)
                         .map_err(|_error| ParseMatchSpecError::InvalidPackagePathOrUrl)?
@@ -286,7 +286,7 @@ pub fn parse_url_like(input: &str) -> Result<Option<Url>, ParseMatchSpecError> {
             .map_err(ParseMatchSpecError::from);
     }
     // Is the spec a path, parse it as an url
-    if is_path(input) {
+    if is_absolute_path(input) {
         let path = Utf8TypedPath::from(input);
         return file_url::file_path_to_url(path)
             .map(Some)
@@ -966,6 +966,10 @@ mod tests {
             // subdir in brackets take precedence
             "conda-forge/linux-32::python[version=3.9, subdir=linux-64]",
             "conda-forge/linux-32::python ==3.9[subdir=linux-64, build_number=\"0\"]",
+            "rust ~=1.2.3",
+            "~/channel/dir::package",
+            "~\\windows_channel::package",
+            "./relative/channel::package",
         ];
 
         let evaluated: IndexMap<_, _> = specs
@@ -982,7 +986,20 @@ mod tests {
                 )
             })
             .collect();
-        insta::assert_yaml_snapshot!(format!("test_from_string_{strictness:?}"), evaluated);
+
+        // Strip absolute paths to this crate from the channels for testing
+        let crate_root = env!("CARGO_MANIFEST_DIR");
+        let crate_path = Url::from_directory_path(std::path::Path::new(crate_root)).unwrap();
+        let home = Url::from_directory_path(dirs::home_dir().unwrap()).unwrap();
+        insta::with_settings!({filters => vec![
+            (crate_path.as_str(), "file://<CRATE>/"),
+            (home.as_str(), "file://<HOME>/"),
+        ]}, {
+            insta::assert_yaml_snapshot!(
+            format!("test_from_string_{strictness:?}"),
+            evaluated
+        );
+        });
     }
 
     #[rstest]
@@ -1001,6 +1018,28 @@ mod tests {
         let specs = [
             "2.7|>=3.6",
             "https://conda.anaconda.org/conda-forge/linux-64/_libgcc_mutex-0.1-conda_forge.tar.bz2",
+            "~=1.2.3",
+            "*.* mkl",
+            "C:\\Users\\user\\conda-bld\\linux-64\\foo-1.0-py27_0.tar.bz2",
+            "=1.0=py27_0",
+            "==1.0=py27_0",
+            "https://conda.anaconda.org/conda-forge/linux-64/py-rattler-0.6.1-py39h8169da8_0.conda",
+            "https://repo.prefix.dev/ruben-arts/linux-64/boost-cpp-1.78.0-h75c5d50_1.tar.bz2",
+            "3.8.* *_cpython",
+            "=*=cuda*",
+            ">=1!164.3095,<1!165",
+            "/home/user/conda-bld/linux-64/foo-1.0-py27_0.tar.bz2",
+            "[version=1.0.*]",
+            "[version=1.0.*, build_number=\">6\"]",
+            "==2.7.*.*|>=3.6",
+            "3.9",
+            "*",
+            "[version=3.9]",
+            "[version=3.9]",
+            "[version=3.9, subdir=linux-64]",
+            // subdir in brackets take precedence
+            "[version=3.9, subdir=linux-64]",
+            "==3.9[subdir=linux-64, build_number=\"0\"]",
         ];
 
         let evaluated: IndexMap<_, _> = specs
@@ -1145,9 +1184,6 @@ mod tests {
         let err = MatchSpec::from_str("bla/bla", Strict)
             .expect_err("Should try to parse as name not url");
         assert_eq!(err.to_string(), "'bla/bla' is not a valid package name. Package names can only contain 0-9, a-z, A-Z, -, _, or .");
-
-        let err = MatchSpec::from_str("./test/file", Strict).expect_err("Invalid url");
-        assert_eq!(err.to_string(), "invalid package path or url");
     }
 
     #[test]
@@ -1172,40 +1208,33 @@ mod tests {
 
     #[test]
     fn test_parse_channel_subdir() {
-        let (channel, subdir) = parse_channel_and_subdir("conda-forge").unwrap();
-        assert_eq!(
-            channel.unwrap(),
-            Channel::from_str("conda-forge", &channel_config()).unwrap()
-        );
-        assert_eq!(subdir, None);
-
-        let (channel, subdir) = parse_channel_and_subdir("conda-forge/linux-64").unwrap();
-        assert_eq!(
-            channel.unwrap(),
-            Channel::from_str("conda-forge", &channel_config()).unwrap()
-        );
-        assert_eq!(subdir, Some("linux-64".to_string()));
-
-        let (channel, subdir) = parse_channel_and_subdir("conda-forge/label/test").unwrap();
-        assert_eq!(
-            channel.unwrap(),
-            Channel::from_str("conda-forge/label/test", &channel_config()).unwrap()
-        );
-        assert_eq!(subdir, None);
-
-        let (channel, subdir) =
-            parse_channel_and_subdir("conda-forge/linux-64/label/test").unwrap();
-        assert_eq!(
-            channel.unwrap(),
-            Channel::from_str("conda-forge/linux-64/label/test", &channel_config()).unwrap()
-        );
-        assert_eq!(subdir, None);
+        let test_cases = vec![
+            ("conda-forge", Some("conda-forge"), None),
+            (
+                "conda-forge/linux-64",
+                Some("conda-forge"),
+                Some("linux-64"),
+            ),
+            (
+                "conda-forge/label/test",
+                Some("conda-forge/label/test"),
+                None,
+            ),
+            (
+                "conda-forge/linux-64/label/test",
+                Some("conda-forge/linux-64/label/test"),
+                None,
+            ),
+            ("*/linux-64", Some("*"), Some("linux-64")),
+        ];
 
-        let (channel, subdir) = parse_channel_and_subdir("*/linux-64").unwrap();
-        assert_eq!(
-            channel.unwrap(),
-            Channel::from_str("*", &channel_config()).unwrap()
-        );
-        assert_eq!(subdir, Some("linux-64".to_string()));
+        for (input, expected_channel, expected_subdir) in test_cases {
+            let (channel, subdir) = parse_channel_and_subdir(input).unwrap();
+            assert_eq!(
+                channel.unwrap(),
+                Channel::from_str(expected_channel.unwrap(), &channel_config()).unwrap()
+            );
+            assert_eq!(subdir, expected_subdir.map(|s| s.to_string()));
+        }
     }
 }
diff --git a/...ec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Lenient.snap b/...ec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Lenient.snap
@@ -98,3 +98,18 @@ python=*:
     base_url: "https://conda.anaconda.org/conda-forge/"
     name: conda-forge
   subdir: linux-64
+rust ~=1.2.3:
+  name: rust
+  version: ~=1.2.3
+"~/channel/dir::package":
+  name: package
+  channel:
+    base_url: "file://<HOME>/channel/dir/"
+    name: ~/channel/dir
+"~\\windows_channel::package":
+  error: invalid channel
+"./relative/channel::package":
+  name: package
+  channel:
+    base_url: "file://<CRATE>/relative/channel/"
+    name: "./relative/channel"
diff --git a/...pec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Strict.snap b/...pec/snapshots/rattler_conda_types__match_spec__parse__tests__test_from_string_Strict.snap
@@ -90,3 +90,18 @@ python=*:
     base_url: "https://conda.anaconda.org/conda-forge/"
     name: conda-forge
   subdir: linux-64
+rust ~=1.2.3:
+  name: rust
+  version: ~=1.2.3
+"~/channel/dir::package":
+  name: package
+  channel:
+    base_url: "file://<HOME>/channel/dir/"
+    name: ~/channel/dir
+"~\\windows_channel::package":
+  error: invalid channel
+"./relative/channel::package":
+  name: package
+  channel:
+    base_url: "file://<CRATE>/relative/channel/"
+    name: "./relative/channel"