Skip to content

Fix feature scoping for pep508 wasm32 support for ruff #8694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading