Allow Python requests with missing segments

crates/uv-python/src/downloads.rs

@@ -49,6 +49,8 @@ pub enum Error {
     MissingExtension(String, ExtensionError),
     #[error("Invalid Python version: {0}")]
     InvalidPythonVersion(String),
+    #[error("Invalid request key (empty request)")]
+    EmptyRequest,
     #[error("Invalid request key (too many parts): {0}")]
     TooManyParts(String),
     #[error("Failed to download {0}")]
@@ -527,42 +529,194 @@ impl Display for PythonDownloadRequest {
         write!(f, "{}", parts.join("-"))
     }
 }
-
 impl FromStr for PythonDownloadRequest {
     type Err = Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
+        #[derive(Debug, Clone)]
+        enum Position {
+            Start,
+            Implementation,
+            Version,
+            Os,
+            Arch,
+            Libc,
+            End,
+        }
+
+        impl Position {
+            pub(crate) fn next(&self) -> Self {
+                match self {
+                    Position::Start => Position::Implementation,
+                    Position::Implementation => Position::Version,
+                    Position::Version => Position::Os,
+                    Position::Os => Position::Arch,
+                    Position::Arch => Position::Libc,
+                    Position::Libc => Position::End,
+                    Position::End => Position::End,
+                }
+            }
+        }
+
+        #[derive(Debug)]
+        struct State<'a, P: Iterator<Item = &'a str>> {
+            parts: P,
+            part: Option<&'a str>,
+            position: Position,
+            error: Option<Error>,
+            count: usize,
+        }
+
+        impl<'a, P: Iterator<Item = &'a str>> State<'a, P> {
+            fn new(parts: P) -> Self {
+                Self {
+                    parts,
+                    part: None,
+                    position: Position::Start,
+                    error: None,
+                    count: 0,
+                }
+            }
+
+            fn next_part(&mut self) {
+                self.next_position();
+                self.part = self.parts.next();
+                self.count += 1;
+                self.error.take();
+            }
+
+            fn next_position(&mut self) {
+                self.position = self.position.next();
+            }
+
+            fn record_err(&mut self, err: Error) {
+                // For now, we only record the first error encountered. We could record all of the
+                // errors for a given part, then pick the most appropriate one later.
+                self.error.get_or_insert(err);
+            }
+        }
+
+        if s.is_empty() {
+            return Err(Error::EmptyRequest);
+        }
+
         let mut parts = s.split('-');
-        let mut version = None;
+
         let mut implementation = None;
+        let mut version = None;
         let mut os = None;
         let mut arch = None;
         let mut libc = None;
 
-        let mut position = 0;
+        let mut state = State::new(parts.by_ref());
+        state.next_part();
+
         loop {
-            // Consume each part
-            let Some(part) = parts.next() else { break };
-            position += 1;
-
-            if part.eq_ignore_ascii_case("any") {
-                continue;
-            }
-
-            match position {
-                1 => implementation = Some(ImplementationName::from_str(part)?),
-                2 => {
-                    version = Some(
-                        VersionRequest::from_str(part)
-                            .map_err(|_| Error::InvalidPythonVersion(part.to_string()))?,
-                    );
+            let Some(part) = state.part else { break };
+            match state.position {
+                Position::Start => unreachable!("We start before the loop"),
+                Position::Implementation => {
+                    if part.eq_ignore_ascii_case("any") {
+                        state.next_part();
+                        continue;
+                    }
+                    match ImplementationName::from_str(part) {
+                        Ok(val) => {
+                            implementation = Some(val);
+                            state.next_part();
+                        }
+                        Err(err) => {
+                            state.next_position();
+                            state.record_err(err.into());
+                        }
+                    }
+                }
+                Position::Version => {
+                    if part.eq_ignore_ascii_case("any") {
+                        state.next_part();
+                        continue;
+                    }
+                    match VersionRequest::from_str(part)
+                        .map_err(|_| Error::InvalidPythonVersion(part.to_string()))
+                    {
+                        // Err(err) if !first_part => return Err(err),
+                        Ok(val) => {
+                            version = Some(val);
+                            state.next_part();
+                        }
+                        Err(err) => {
+                            state.next_position();
+                            state.record_err(err);
+                        }
+                    }
+                }
+                Position::Os => {
+                    if part.eq_ignore_ascii_case("any") {
+                        state.next_part();
+                        continue;
+                    }
+                    match Os::from_str(part) {
+                        Ok(val) => {
+                            os = Some(val);
+                            state.next_part();
+                        }
+                        Err(err) => {
+                            state.next_position();
+                            state.record_err(err.into());
+                        }
+                    }
+                }
+                Position::Arch => {
+                    if part.eq_ignore_ascii_case("any") {
+                        state.next_part();
+                        continue;
+                    }
+                    match Arch::from_str(part) {
+                        Ok(val) => {
+                            arch = Some(ArchRequest::Explicit(val));
+                            state.next_part();
+                        }
+                        Err(err) => {
+                            state.next_position();
+                            state.record_err(err.into());
+                        }
+                    }
+                }
+                Position::Libc => {
+                    if part.eq_ignore_ascii_case("any") {
+                        state.next_part();
+                        continue;
+                    }
+                    match Libc::from_str(part) {
+                        Ok(val) => {
+                            libc = Some(val);
+                            state.next_part();
+                        }
+                        Err(err) => {
+                            state.next_position();
+                            state.record_err(err.into());
+                        }
+                    }
+                }
+                Position::End => {
+                    if state.count > 5 {
+                        return Err(Error::TooManyParts(s.to_string()));
+                    }
+
+                    // Throw the first error for the current part
+                    //
+                    // TODO(zanieb): It's plausible another error variant is a better match but it
+                    // sounds hard to explain how? We could peek at the next item in the parts, and
+                    // see if that informs the type of this one, or we could use some sort of
+                    // similarity or common error matching, but this sounds harder.
+                    if let Some(err) = state.error {
+                        return Err(err);
+                    }
+                    state.next_part();
                 }
-                3 => os = Some(Os::from_str(part)?),
-                4 => arch = Some(ArchRequest::Explicit(Arch::from_str(part)?)),
-                5 => libc = Some(Libc::from_str(part)?),
-                _ => return Err(Error::TooManyParts(s.to_string())),
             }
         }
+
         Ok(Self::new(version, implementation, arch, os, libc, None))
     }
 }
@@ -1299,3 +1453,214 @@ async fn read_url(
         Ok((Either::Right(stream.compat()), size))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// Parse a request with all of its fields.
+    #[test]
+    fn test_python_download_request_from_str_complete() {
+        let request = PythonDownloadRequest::from_str("cpython-3.12.0-linux-x86_64-gnu")
+            .expect("Test request should be parsed");
+
+        assert_eq!(request.implementation, Some(ImplementationName::CPython));
+        assert_eq!(
+            request.version,
+            Some(VersionRequest::from_str("3.12.0").unwrap())
+        );
+        assert_eq!(request.os, Some(Os(target_lexicon::OperatingSystem::Linux)));
+        assert_eq!(
+            request.arch,
+            Some(ArchRequest::Explicit(Arch {
+                family: target_lexicon::Architecture::X86_64,
+                variant: None,
+            }))
+        );
+        assert_eq!(
+            request.libc,
+            Some(Libc::Some(target_lexicon::Environment::Gnu))
+        );
+    }
+
+    /// Parse a request with `any` in various positions.
+    #[test]
+    fn test_python_download_request_from_str_with_any() {
+        let request = PythonDownloadRequest::from_str("any-3.11-any-x86_64-any")
+            .expect("Test request should be parsed");
+
+        assert_eq!(request.implementation, None);
+        assert_eq!(
+            request.version,
+            Some(VersionRequest::from_str("3.11").unwrap())
+        );
+        assert_eq!(request.os, None);
+        assert_eq!(
+            request.arch,
+            Some(ArchRequest::Explicit(Arch {
+                family: target_lexicon::Architecture::X86_64,
+                variant: None,
+            }))
+        );
+        assert_eq!(request.libc, None);
+    }
+
+    /// Parse a request with `any` implied by the omission of segments.
+    #[test]
+    fn test_python_download_request_from_str_missing_segment() {
+        let request =
+            PythonDownloadRequest::from_str("pypy-linux").expect("Test request should be parsed");
+
+        assert_eq!(request.implementation, Some(ImplementationName::PyPy));
+        assert_eq!(request.version, None);
+        assert_eq!(request.os, Some(Os(target_lexicon::OperatingSystem::Linux)));
+        assert_eq!(request.arch, None);
+        assert_eq!(request.libc, None);
+    }
+
+    #[test]
+    fn test_python_download_request_from_str_version_only() {
+        let request =
+            PythonDownloadRequest::from_str("3.10.5").expect("Test request should be parsed");
+
+        assert_eq!(request.implementation, None);
+        assert_eq!(
+            request.version,
+            Some(VersionRequest::from_str("3.10.5").unwrap())
+        );
+        assert_eq!(request.os, None);
+        assert_eq!(request.arch, None);
+        assert_eq!(request.libc, None);
+    }
+
+    #[test]
+    fn test_python_download_request_from_str_implementation_only() {
+        let request =
+            PythonDownloadRequest::from_str("cpython").expect("Test request should be parsed");
+
+        assert_eq!(request.implementation, Some(ImplementationName::CPython));
+        assert_eq!(request.version, None);
+        assert_eq!(request.os, None);
+        assert_eq!(request.arch, None);
+        assert_eq!(request.libc, None);
+    }
+
+    /// Parse a request with the OS and architecture specified.
+    #[test]
+    fn test_python_download_request_from_str_os_arch() {
+        let request = PythonDownloadRequest::from_str("windows-x86_64")
+            .expect("Test request should be parsed");
+
+        assert_eq!(request.implementation, None);
+        assert_eq!(request.version, None);
+        assert_eq!(
+            request.os,
+            Some(Os(target_lexicon::OperatingSystem::Windows))
+        );
+        assert_eq!(
+            request.arch,
+            Some(ArchRequest::Explicit(Arch {
+                family: target_lexicon::Architecture::X86_64,
+                variant: None,
+            }))
+        );
+        assert_eq!(request.libc, None);
+    }
+
+    /// Parse a request with a pre-release version.
+    #[test]
+    fn test_python_download_request_from_str_prerelease() {
+        let request = PythonDownloadRequest::from_str("cpython-3.13.0rc1")
+            .expect("Test request should be parsed");
+
+        assert_eq!(request.implementation, Some(ImplementationName::CPython));
+        assert_eq!(
+            request.version,
+            Some(VersionRequest::from_str("3.13.0rc1").unwrap())
+        );
+        assert_eq!(request.os, None,);
+        assert_eq!(request.arch, None,);
+        assert_eq!(request.libc, None);
+    }
+
+    /// We fail on extra parts in the request.
+    #[test]
+    fn test_python_download_request_from_str_too_many_parts() {
+        let result = PythonDownloadRequest::from_str("cpython-3.12-linux-x86_64-gnu-extra");
+
+        assert!(matches!(result, Err(Error::TooManyParts(_))));
+    }
+
+    /// We don't allow an empty request.
+    #[test]
+    fn test_python_download_request_from_str_empty() {
+        let result = PythonDownloadRequest::from_str("");
+
+        assert!(matches!(result, Err(Error::EmptyRequest)), "{result:?}");
+    }
+
+    /// Parse a request with all "any" segments.
+    #[test]
+    fn test_python_download_request_from_str_all_any() {
+        let request = PythonDownloadRequest::from_str("any-any-any-any-any")
+            .expect("Test request should be parsed");
+
+        assert_eq!(request.implementation, None);
+        assert_eq!(request.version, None);
+        assert_eq!(request.os, None);
+        assert_eq!(request.arch, None);
+        assert_eq!(request.libc, None);
+    }
+
+    /// Test that "any" is case-insensitive in various positions.
+    #[test]
+    fn test_python_download_request_from_str_case_insensitive_any() {
+        let request = PythonDownloadRequest::from_str("ANY-3.11-Any-x86_64-aNy")
+            .expect("Test request should be parsed");
+
+        assert_eq!(request.implementation, None);
+        assert_eq!(
+            request.version,
+            Some(VersionRequest::from_str("3.11").unwrap())
+        );
+        assert_eq!(request.os, None);
+        assert_eq!(
+            request.arch,
+            Some(ArchRequest::Explicit(Arch {
+                family: target_lexicon::Architecture::X86_64,
+                variant: None,
+            }))
+        );
+        assert_eq!(request.libc, None);
+    }
+
+    /// Parse a request with an invalid leading segment.
+    #[test]
+    fn test_python_download_request_from_str_invalid_leading_segment() {
+        let result = PythonDownloadRequest::from_str("foobar-3.14-windows");
+
+        assert!(
+            matches!(result, Err(Error::ImplementationError(_))),
+            "{result:?}"
+        );
+    }
+
+    /// Parse a request with segments in an invalid order.
+    #[test]
+    fn test_python_download_request_from_str_out_of_order() {
+        let result = PythonDownloadRequest::from_str("3.12-cpython");
+
+        assert!(
+            matches!(result, Err(Error::InvalidRequestPlatform(_))),
+            "{result:?}"
+        );
+    }
+
+    /// Parse a request with too many "any" segments.
+    #[test]
+    fn test_python_download_request_from_str_too_many_any() {
+        let result = PythonDownloadRequest::from_str("any-any-any-any-any-any");
+
+        assert!(matches!(result, Err(Error::TooManyParts(_))));
+    }
+}

crates/uv/tests/it/python_install.rs

@@ -1623,7 +1623,7 @@ fn python_install_emulated_macos() {
     ");
 
     // Install an x86_64 version (assuming an aarch64 host)
-    uv_snapshot!(context.filters(), context.python_install().arg("cpython-3.13-macos-x86_64"), @r"
+    uv_snapshot!(context.filters(), context.python_install().arg("3.13-x86_64"), @r"
     success: true
     exit_code: 0
     ----- stdout -----
@@ -1654,7 +1654,7 @@ fn python_install_emulated_macos() {
     ----- stderr -----
     ");
 
-    uv_snapshot!(context.filters(), context.python_install().arg("cpython-3.13-macos-aarch64"), @r"
+    uv_snapshot!(context.filters(), context.python_install().arg("3.13-aarch64"), @r"
     success: true
     exit_code: 0
     ----- stdout -----