From 25b4764422c8ab63133b67eec7df204ab9dbc105 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 30 Oct 2024 10:32:06 +0100 Subject: [PATCH 1/4] Fix pep508 re-exports --- crates/uv-pep508/src/lib.rs | 5 ++--- crates/uv-pypi-types/src/metadata/requires_txt.rs | 3 +-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/uv-pep508/src/lib.rs b/crates/uv-pep508/src/lib.rs index ef02f502e360..c1319b288653 100644 --- a/crates/uv-pep508/src/lib.rs +++ b/crates/uv-pep508/src/lib.rs @@ -26,13 +26,12 @@ use serde::{de, Deserialize, Deserializer, Serialize, Serializer}; use thiserror::Error; use url::Url; -use crate::marker::MarkerValueExtra; use cursor::Cursor; pub use marker::{ ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, MarkerEnvironment, MarkerEnvironmentBuilder, MarkerExpression, MarkerOperator, MarkerTree, MarkerTreeContents, - MarkerTreeKind, MarkerValue, MarkerValueString, MarkerValueVersion, MarkerWarningKind, - StringMarkerTree, StringVersion, VersionMarkerTree, + MarkerTreeKind, MarkerValue, MarkerValueExtra, MarkerValueString, MarkerValueVersion, + MarkerWarningKind, StringMarkerTree, StringVersion, VersionMarkerTree, }; pub use origin::RequirementOrigin; #[cfg(feature = "non-pep508-extensions")] diff --git a/crates/uv-pypi-types/src/metadata/requires_txt.rs b/crates/uv-pypi-types/src/metadata/requires_txt.rs index fbfa4fe6d112..f84d36c89cfe 100644 --- a/crates/uv-pypi-types/src/metadata/requires_txt.rs +++ b/crates/uv-pypi-types/src/metadata/requires_txt.rs @@ -3,8 +3,7 @@ use serde::Deserialize; use std::io::BufRead; use std::str::FromStr; use uv_normalize::ExtraName; -use uv_pep508::marker::MarkerValueExtra; -use uv_pep508::{ExtraOperator, MarkerExpression, MarkerTree, Requirement}; +use uv_pep508::{ExtraOperator, MarkerExpression, MarkerTree, MarkerValueExtra, Requirement}; /// `requires.txt` metadata as defined in . /// From 550828a7160ba90d5fbe2ff99816db235f564547 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 30 Oct 2024 10:41:11 +0100 Subject: [PATCH 2/4] Fix pep508 tests --- Cargo.lock | 32 +++++++---- crates/uv-pep508/Cargo.toml | 2 +- crates/uv-pep508/src/marker/tree.rs | 89 ++++++++++++++++------------- 3 files changed, 71 insertions(+), 52 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0094ec132720..3761f92eaa5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3632,15 +3632,6 @@ dependencies = [ "syn", ] -[[package]] -name = "testing_logger" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6d92b727cb45d33ae956f7f46b966b25f1bc712092aeef9dba5ac798fc89f720" -dependencies = [ - "log", -] - [[package]] name = "textwrap" version = "0.16.1" @@ -3967,6 +3958,27 @@ dependencies = [ "tracing-serde", ] +[[package]] +name = "tracing-test" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "557b891436fe0d5e0e363427fc7f217abf9ccd510d5136549847bdcbcd011d68" +dependencies = [ + "tracing-core", + "tracing-subscriber", + "tracing-test-macro", +] + +[[package]] +name = "tracing-test-macro" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "04659ddb06c87d233c566112c1c9c5b9e98256d9af50ec3bc9c8327f873a7568" +dependencies = [ + "quote", + "syn", +] + [[package]] name = "tracing-tree" version = "0.4.0" @@ -4917,9 +4929,9 @@ dependencies = [ "serde", "serde_json", "smallvec", - "testing_logger", "thiserror", "tracing", + "tracing-test", "unicode-width", "url", "uv-fs", diff --git a/crates/uv-pep508/Cargo.toml b/crates/uv-pep508/Cargo.toml index 0968cbc1ce81..f8f3465f587f 100644 --- a/crates/uv-pep508/Cargo.toml +++ b/crates/uv-pep508/Cargo.toml @@ -43,7 +43,7 @@ version-ranges = { workspace = true } insta = { version = "1.40.0" } log = { version = "0.4.22" } serde_json = { version = "1.0.128" } -testing_logger = { version = "0.1.1" } +tracing-test = { version = "0.2.5" } [features] tracing = ["dep:tracing", "uv-pep440/tracing"] diff --git a/crates/uv-pep508/src/marker/tree.rs b/crates/uv-pep508/src/marker/tree.rs index db31e50e2f16..febf3dbcf0f6 100644 --- a/crates/uv-pep508/src/marker/tree.rs +++ b/crates/uv-pep508/src/marker/tree.rs @@ -1937,59 +1937,66 @@ mod test { #[test] #[cfg(feature = "tracing")] - fn warnings() { + #[tracing_test::traced_test] + fn warnings1() { let env37 = env37(); - testing_logger::setup(); let compare_keys = MarkerTree::from_str("platform_version == sys_platform").unwrap(); compare_keys.evaluate(&env37, &[]); - testing_logger::validate(|captured_logs| { - assert_eq!( - captured_logs[0].body, - "Comparing two markers with each other doesn't make any sense, will evaluate to false" - ); - assert_eq!(captured_logs[0].level, log::Level::Warn); - assert_eq!(captured_logs.len(), 1); - }); + logs_contain( + "Comparing two markers with each other doesn't make any sense, will evaluate to false", + ); + } + + #[test] + #[cfg(feature = "tracing")] + #[tracing_test::traced_test] + fn warnings2() { + let env37 = env37(); let non_pep440 = MarkerTree::from_str("python_version >= '3.9.'").unwrap(); non_pep440.evaluate(&env37, &[]); - testing_logger::validate(|captured_logs| { - assert_eq!( - captured_logs[0].body, - "Expected PEP 440 version to compare with python_version, found `3.9.`, \ - will evaluate to false: after parsing `3.9`, found `.`, which is \ - not part of a valid version" - ); - assert_eq!(captured_logs[0].level, log::Level::Warn); - assert_eq!(captured_logs.len(), 1); - }); + logs_contain( + "Expected PEP 440 version to compare with python_version, found `3.9.`, \ + will evaluate to false: after parsing `3.9`, found `.`, which is \ + not part of a valid version", + ); + } + + #[test] + #[cfg(feature = "tracing")] + #[tracing_test::traced_test] + fn warnings3() { + let env37 = env37(); let string_string = MarkerTree::from_str("'b' >= 'a'").unwrap(); string_string.evaluate(&env37, &[]); - testing_logger::validate(|captured_logs| { - assert_eq!( - captured_logs[0].body, - "Comparing two quoted strings with each other doesn't make sense: 'b' >= 'a', will evaluate to false" - ); - assert_eq!(captured_logs[0].level, log::Level::Warn); - assert_eq!(captured_logs.len(), 1); - }); + logs_contain( + "Comparing two quoted strings with each other doesn't make sense: 'b' >= 'a', will evaluate to false" + ); + } + + #[test] + #[cfg(feature = "tracing")] + #[tracing_test::traced_test] + fn warnings4() { + let env37 = env37(); let string_string = MarkerTree::from_str(r"os.name == 'posix' and platform.machine == 'x86_64' and platform.python_implementation == 'CPython' and 'Ubuntu' in platform.version and sys.platform == 'linux'").unwrap(); string_string.evaluate(&env37, &[]); - testing_logger::validate(|captured_logs| { - let messages: Vec<_> = captured_logs + logs_assert(|lines: &[&str]| { + let lines: Vec<_> = lines .iter() - .map(|message| { - assert_eq!(message.level, log::Level::Warn); - &message.body - }) + .map(|s| s.split_once(" ").unwrap().1) .collect(); - let expected = [ - "os.name is deprecated in favor of os_name", - "platform.machine is deprecated in favor of platform_machine", - "platform.python_implementation is deprecated in favor of platform_python_implementation", - "platform.version is deprecated in favor of platform_version", - "sys.platform is deprecated in favor of sys_platform" + let expected = [ + "WARN warnings4: uv_pep508: os.name is deprecated in favor of os_name", + "WARN warnings4: uv_pep508: platform.machine is deprecated in favor of platform_machine", + "WARN warnings4: uv_pep508: platform.python_implementation is deprecated in favor of", + "WARN warnings4: uv_pep508: sys.platform is deprecated in favor of sys_platform", + "WARN warnings4: uv_pep508: Comparing linux and posix lexicographically" ]; - assert_eq!(messages, &expected); + if lines == expected { + Ok(()) + } else { + Err(format!("{:?}", lines)) + } }); } From f7df1f164de6109d686e3b771a51eba82e35f333 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 30 Oct 2024 10:57:23 +0100 Subject: [PATCH 3/4] Fix feature scoping for pep508 wasm32 support for ruff --- crates/uv-pep508/src/tests.rs | 2 + crates/uv-pep508/src/verbatim_url.rs | 64 ++++++++++++++++++---------- 2 files changed, 43 insertions(+), 23 deletions(-) diff --git a/crates/uv-pep508/src/tests.rs b/crates/uv-pep508/src/tests.rs index 970202ff4976..8ac718847d86 100644 --- a/crates/uv-pep508/src/tests.rs +++ b/crates/uv-pep508/src/tests.rs @@ -712,6 +712,7 @@ fn error_invalid_extra_unnamed_url() { /// Check that the relative path support feature toggle works. #[test] +#[cfg(feature = "non-pep508-extensions")] fn non_pep508_paths() { let requirements = &[ "foo @ file://./foo", @@ -748,6 +749,7 @@ fn no_space_after_operator() { } #[test] +#[cfg(feature = "non-pep508-extensions")] fn path_with_fragment() { let requirements = if cfg!(windows) { &[ diff --git a/crates/uv-pep508/src/verbatim_url.rs b/crates/uv-pep508/src/verbatim_url.rs index f75539d2ed2b..5d2f62597cbd 100644 --- a/crates/uv-pep508/src/verbatim_url.rs +++ b/crates/uv-pep508/src/verbatim_url.rs @@ -140,6 +140,7 @@ impl VerbatimUrl { } /// Return the underlying [`Path`], if the URL is a file URL. + #[cfg(feature = "non-pep508-extensions")] pub fn as_path(&self) -> Result { self.url .to_file_path() @@ -212,11 +213,8 @@ impl Pep508Url for VerbatimUrl { type Err = VerbatimUrlError; /// Create a `VerbatimUrl` to represent the requirement. - fn parse_url( - url: &str, - #[cfg_attr(not(feature = "non-pep508-extensions"), allow(unused_variables))] - working_dir: Option<&Path>, - ) -> Result { + #[cfg_attr(not(feature = "non-pep508-extensions"), allow(unused_variables))] + fn parse_url(url: &str, working_dir: Option<&Path>) -> Result { // Expand environment variables in the URL. let expanded = expand_env_vars(url); @@ -225,18 +223,24 @@ impl Pep508Url for VerbatimUrl { // Ex) `file:///home/ferris/project/scripts/...`, `file://localhost/home/ferris/project/scripts/...`, or `file:../ferris/` Some(Scheme::File) => { // Strip the leading slashes, along with the `localhost` host, if present. - let path = strip_host(path); // Transform, e.g., `/C:/Users/ferris/wheel-0.42.0.tar.gz` to `C:\Users\ferris\wheel-0.42.0.tar.gz`. - let path = normalize_url_path(path); - #[cfg(feature = "non-pep508-extensions")] - if let Some(working_dir) = working_dir { - return Ok(VerbatimUrl::from_path(path.as_ref(), working_dir)? - .with_given(url.to_string())); - } + { + let path = strip_host(path); + + let path = normalize_url_path(path); - Ok(VerbatimUrl::from_absolute_path(path.as_ref())?.with_given(url.to_string())) + if let Some(working_dir) = working_dir { + return Ok(VerbatimUrl::from_path(path.as_ref(), working_dir)? + .with_given(url.to_string())); + } + + Ok(VerbatimUrl::from_absolute_path(path.as_ref())? + .with_given(url.to_string())) + } + #[cfg(not(feature = "non-pep508-extensions"))] + Ok(VerbatimUrl::parse_url(expanded)?.with_given(url.to_string())) } // Ex) `https://download.pytorch.org/whl/torch_stable.html` @@ -248,24 +252,33 @@ impl Pep508Url for VerbatimUrl { // Ex) `C:\Users\ferris\wheel-0.42.0.tar.gz` _ => { #[cfg(feature = "non-pep508-extensions")] - if let Some(working_dir) = working_dir { - return Ok(VerbatimUrl::from_path(expanded.as_ref(), working_dir)? - .with_given(url.to_string())); + { + if let Some(working_dir) = working_dir { + return Ok(VerbatimUrl::from_path(expanded.as_ref(), working_dir)? + .with_given(url.to_string())); + } + + Ok(VerbatimUrl::from_absolute_path(expanded.as_ref())? + .with_given(url.to_string())) } - - Ok(VerbatimUrl::from_absolute_path(expanded.as_ref())? - .with_given(url.to_string())) + #[cfg(not(feature = "non-pep508-extensions"))] + Err(Self::Err::NotAUrl(expanded.to_string())) } } } else { // Ex) `../editable/` #[cfg(feature = "non-pep508-extensions")] - if let Some(working_dir) = working_dir { - return Ok(VerbatimUrl::from_path(expanded.as_ref(), working_dir)? - .with_given(url.to_string())); + { + if let Some(working_dir) = working_dir { + return Ok(VerbatimUrl::from_path(expanded.as_ref(), working_dir)? + .with_given(url.to_string())); + } + + Ok(VerbatimUrl::from_absolute_path(expanded.as_ref())?.with_given(url.to_string())) } - Ok(VerbatimUrl::from_absolute_path(expanded.as_ref())?.with_given(url.to_string())) + #[cfg(not(feature = "non-pep508-extensions"))] + Err(Self::Err::NotAUrl(expanded.to_string())) } } } @@ -288,6 +301,11 @@ pub enum VerbatimUrlError { /// Received a path that could not be normalized. #[error("path could not be normalized: {0}")] Normalization(PathBuf, #[source] std::io::Error), + + /// Received a path that could not be normalized. + #[cfg(not(feature = "non-pep508-extensions"))] + #[error("Not a URL (missing scheme): {0}")] + NotAUrl(String), } /// Expand all available environment variables. From 8eb655d77ad7f3270f79a43c371f2a8660134af8 Mon Sep 17 00:00:00 2001 From: konstin Date: Wed, 30 Oct 2024 11:07:15 +0100 Subject: [PATCH 4/4] Lint fixes --- Cargo.lock | 1 - crates/uv-pep508/Cargo.toml | 1 - crates/uv-pep508/src/marker/tree.rs | 2 +- 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3761f92eaa5e..b14c3a431115 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4922,7 +4922,6 @@ dependencies = [ "indexmap", "insta", "itertools 0.13.0", - "log", "regex", "rustc-hash", "schemars", diff --git a/crates/uv-pep508/Cargo.toml b/crates/uv-pep508/Cargo.toml index f8f3465f587f..15e0e0095b2c 100644 --- a/crates/uv-pep508/Cargo.toml +++ b/crates/uv-pep508/Cargo.toml @@ -41,7 +41,6 @@ version-ranges = { workspace = true } [dev-dependencies] insta = { version = "1.40.0" } -log = { version = "0.4.22" } serde_json = { version = "1.0.128" } tracing-test = { version = "0.2.5" } diff --git a/crates/uv-pep508/src/marker/tree.rs b/crates/uv-pep508/src/marker/tree.rs index febf3dbcf0f6..5959de032293 100644 --- a/crates/uv-pep508/src/marker/tree.rs +++ b/crates/uv-pep508/src/marker/tree.rs @@ -1995,7 +1995,7 @@ mod test { if lines == expected { Ok(()) } else { - Err(format!("{:?}", lines)) + Err(format!("{lines:?}")) } }); }