Skip to content

Commit

Permalink
Fix feature scoping for pep508 wasm32 support for ruff (astral-sh#8694)
Browse files Browse the repository at this point in the history
  • Loading branch information
konstin authored Oct 30, 2024
1 parent c1a0fb3 commit 94fc35e
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 82 deletions.
33 changes: 22 additions & 11 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions crates/uv-pep508/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,8 @@ version-ranges = { workspace = true }

[dev-dependencies]
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"]
Expand Down
5 changes: 2 additions & 3 deletions crates/uv-pep508/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down
89 changes: 48 additions & 41 deletions crates/uv-pep508/src/marker/tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:?}"))
}
});
}

Expand Down
2 changes: 2 additions & 0 deletions crates/uv-pep508/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -748,6 +749,7 @@ fn no_space_after_operator() {
}

#[test]
#[cfg(feature = "non-pep508-extensions")]
fn path_with_fragment() {
let requirements = if cfg!(windows) {
&[
Expand Down
64 changes: 41 additions & 23 deletions crates/uv-pep508/src/verbatim_url.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf, VerbatimUrlError> {
self.url
.to_file_path()
Expand Down Expand Up @@ -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<Self, Self::Err> {
#[cfg_attr(not(feature = "non-pep508-extensions"), allow(unused_variables))]
fn parse_url(url: &str, working_dir: Option<&Path>) -> Result<Self, Self::Err> {
// Expand environment variables in the URL.
let expanded = expand_env_vars(url);

Expand All @@ -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`
Expand All @@ -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()))
}
}
}
Expand All @@ -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.
Expand Down
3 changes: 1 addition & 2 deletions crates/uv-pypi-types/src/metadata/requires_txt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <https://setuptools.pypa.io/en/latest/deprecated/python_eggs.html#dependency-metadata>.
///
Expand Down

0 comments on commit 94fc35e

Please sign in to comment.