From 0c4e1f3fdc3370a395aea35801e1fbc988c37dba Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Tue, 1 Jul 2025 07:55:42 -0500 Subject: [PATCH] Allow Python requests with missing segments --- crates/uv-python/src/downloads.rs | 409 +++++++++++++++++++++++++-- crates/uv/tests/it/python_install.rs | 4 +- 2 files changed, 389 insertions(+), 24 deletions(-) diff --git a/crates/uv-python/src/downloads.rs b/crates/uv-python/src/downloads.rs index 2b0a7e669..d5846ffa2 100644 --- a/crates/uv-python/src/downloads.rs +++ b/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 { + #[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> { + parts: P, + part: Option<&'a str>, + position: Position, + error: Option, + count: usize, + } + + impl<'a, P: Iterator> 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(_)))); + } +} diff --git a/crates/uv/tests/it/python_install.rs b/crates/uv/tests/it/python_install.rs index 7fd596cd8..c7868fbbc 100644 --- a/crates/uv/tests/it/python_install.rs +++ b/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 -----