Skip to content

Commit

Permalink
Respect markers on them
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Sep 27, 2024
1 parent 09cc286 commit 320be30
Show file tree
Hide file tree
Showing 11 changed files with 515 additions and 42 deletions.
1 change: 1 addition & 0 deletions Cargo.lock
View file

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

39 changes: 37 additions & 2 deletions crates/pep508-rs/src/marker/tree.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ use std::ops::{Bound, Deref};
use std::str::FromStr;

use itertools::Itertools;
use pep440_rs::{Version, VersionParseError, VersionSpecifier};
use pubgrub::Range;
#[cfg(feature = "pyo3")]
use pyo3::{basic::CompareOp, pyclass, pymethods};
use serde::{de, Deserialize, Deserializer, Serialize, Serializer};

use pep440_rs::{Version, VersionParseError, VersionSpecifier};
use uv_normalize::ExtraName;

use crate::cursor::Cursor;
Expand Down Expand Up @@ -1631,6 +1630,20 @@ impl Serialize for MarkerTreeContents {
}
}

impl<'de> serde::Deserialize<'de> for MarkerTreeContents {
fn deserialize<D>(deserializer: D) -> Result<MarkerTreeContents, D::Error>
where
D: Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
let marker = MarkerTree::from_str(&s).map_err(de::Error::custom)?;
let marker = marker
.contents()
.ok_or_else(|| de::Error::custom("expected at least one marker expression"))?;
Ok(marker)
}
}

impl Display for MarkerTreeContents {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
// Normalize all `false` expressions to the same trivially false expression.
Expand Down Expand Up @@ -1667,6 +1680,28 @@ impl Display for MarkerTreeContents {
}
}

#[cfg(feature = "schemars")]
impl schemars::JsonSchema for MarkerTreeContents {
fn schema_name() -> String {
"MarkerTreeContents".to_string()
}

fn json_schema(_gen: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema {
schemars::schema::SchemaObject {
instance_type: Some(schemars::schema::InstanceType::String.into()),
metadata: Some(Box::new(schemars::schema::Metadata {
description: Some(
"A PEP 508-compliant marker expression, e.g., `sys_platform == 'Darwin'`"
.to_string(),
),
..schemars::schema::Metadata::default()
})),
..schemars::schema::SchemaObject::default()
}
.into()
}
}

#[cfg(test)]
mod test {
use std::ops::Bound;
Expand Down
98 changes: 73 additions & 25 deletions crates/uv-distribution/src/metadata/lowering.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use url::Url;

use distribution_filename::DistExtension;
use pep440_rs::VersionSpecifiers;
use pep508_rs::{VerbatimUrl, VersionOrUrl};
use pep508_rs::{MarkerTree, VerbatimUrl, VersionOrUrl};
use pypi_types::{ParsedUrlError, Requirement, RequirementSource, VerbatimParsedUrl};
use uv_git::GitReference;
use uv_normalize::PackageName;
Expand Down Expand Up @@ -48,8 +48,8 @@ impl LoweredRequirement {
// We require that when you use a package that's part of the workspace, ...
!workspace.packages().contains_key(&requirement.name)
// ... it must be declared as a workspace dependency (`workspace = true`), ...
|| source.as_ref().is_some_and(|source| source.inner().iter().all(|source| {
matches!(source, Source::Workspace { workspace: true })
|| source.as_ref().is_some_and(|source| source.iter().all(|source| {
matches!(source, Source::Workspace { workspace: true, .. })
}))
// ... except for recursive self-inclusion (extras that activate other extras), e.g.
// `framework[machine_learning]` depends on `framework[cuda]`.
Expand All @@ -75,41 +75,62 @@ impl LoweredRequirement {
return Either::Left(std::iter::once(Ok(Self(Requirement::from(requirement)))));
};

Either::Right(source.into_inner().into_iter().map(move |source| {
let source = match source {
Either::Right(source.into_iter().map(move |source| {
let (source, mut marker) = match source {
Source::Git {
git,
subdirectory,
rev,
tag,
branch,
marker,
} => {
if matches!(requirement.version_or_url, Some(VersionOrUrl::Url(_))) {
return Err(LoweringError::ConflictingUrls);
}
git_source(&git, subdirectory.map(PathBuf::from), rev, tag, branch)?
let source =
git_source(&git, subdirectory.map(PathBuf::from), rev, tag, branch)?;
let marker = marker.map(MarkerTree::from).unwrap_or_default();
(source, marker)
}
Source::Url { url, subdirectory } => {
Source::Url {
url,
subdirectory,
marker,
} => {
if matches!(requirement.version_or_url, Some(VersionOrUrl::Url(_))) {
return Err(LoweringError::ConflictingUrls);
}
url_source(url, subdirectory.map(PathBuf::from))?
let source = url_source(url, subdirectory.map(PathBuf::from))?;
let marker = marker.map(MarkerTree::from).unwrap_or_default();
(source, marker)
}
Source::Path { path, editable } => {
Source::Path {
path,
editable,
marker,
} => {
if matches!(requirement.version_or_url, Some(VersionOrUrl::Url(_))) {
return Err(LoweringError::ConflictingUrls);
}
path_source(
let source = path_source(
PathBuf::from(path),
origin,
project_dir,
workspace.install_path(),
editable.unwrap_or(false),
)?
)?;
let marker = marker.map(MarkerTree::from).unwrap_or_default();
(source, marker)
}
Source::Registry { index, marker } => {
let source = registry_source(&requirement, index)?;
let marker = marker.map(MarkerTree::from).unwrap_or_default();
(source, marker)
}
Source::Registry { index } => registry_source(&requirement, index)?,
Source::Workspace {
workspace: is_workspace,
marker,
} => {
if !is_workspace {
return Err(LoweringError::WorkspaceFalse);
Expand Down Expand Up @@ -148,7 +169,7 @@ impl LoweredRequirement {
))
})?;

if member.pyproject_toml().is_package() {
let source = if member.pyproject_toml().is_package() {
RequirementSource::Directory {
install_path,
url,
Expand All @@ -162,17 +183,22 @@ impl LoweredRequirement {
editable: false,
r#virtual: true,
}
}
};
let marker = marker.map(MarkerTree::from).unwrap_or_default();
(source, marker)
}
Source::CatchAll { .. } => {
// Emit a dedicated error message, which is an improvement over Serde's default error.
return Err(LoweringError::InvalidEntry);
}
};

marker.and(requirement.marker.clone());

Ok(Self(Requirement {
name: requirement.name.clone(),
extras: requirement.extras.clone(),
marker: requirement.marker.clone(),
marker,
source,
origin: requirement.origin.clone(),
}))
Expand All @@ -192,39 +218,59 @@ impl LoweredRequirement {
return Either::Left(std::iter::once(Ok(Self(Requirement::from(requirement)))));
};

Either::Right(source.into_inner().into_iter().map(move |source| {
let source = match source {
Either::Right(source.into_iter().map(move |source| {
let (source, mut marker) = match source {
Source::Git {
git,
subdirectory,
rev,
tag,
branch,
marker,
} => {
if matches!(requirement.version_or_url, Some(VersionOrUrl::Url(_))) {
return Err(LoweringError::ConflictingUrls);
}
git_source(&git, subdirectory.map(PathBuf::from), rev, tag, branch)?
let source =
git_source(&git, subdirectory.map(PathBuf::from), rev, tag, branch)?;
let marker = marker.map(MarkerTree::from).unwrap_or_default();
(source, marker)
}
Source::Url { url, subdirectory } => {
Source::Url {
url,
subdirectory,
marker,
} => {
if matches!(requirement.version_or_url, Some(VersionOrUrl::Url(_))) {
return Err(LoweringError::ConflictingUrls);
}
url_source(url, subdirectory.map(PathBuf::from))?
let source = url_source(url, subdirectory.map(PathBuf::from))?;
let marker = marker.map(MarkerTree::from).unwrap_or_default();
(source, marker)
}
Source::Path { path, editable } => {
Source::Path {
path,
editable,
marker,
} => {
if matches!(requirement.version_or_url, Some(VersionOrUrl::Url(_))) {
return Err(LoweringError::ConflictingUrls);
}
path_source(
let source = path_source(
PathBuf::from(path),
Origin::Project,
dir,
dir,
editable.unwrap_or(false),
)?
)?;
let marker = marker.map(MarkerTree::from).unwrap_or_default();
(source, marker)
}
Source::Registry { index, marker } => {
let source = registry_source(&requirement, index)?;
let marker = marker.map(MarkerTree::from).unwrap_or_default();
(source, marker)
}
Source::Registry { index } => registry_source(&requirement, index)?,
Source::Workspace { .. } => {
return Err(LoweringError::WorkspaceMember);
}
Expand All @@ -235,10 +281,12 @@ impl LoweredRequirement {
}
};

marker.and(requirement.marker.clone());

Ok(Self(Requirement {
name: requirement.name.clone(),
extras: requirement.extras.clone(),
marker: requirement.marker.clone(),
marker,
source,
origin: requirement.origin.clone(),
}))
Expand Down
3 changes: 2 additions & 1 deletion crates/uv-workspace/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ uv-options-metadata = { workspace = true }
either = { workspace = true }
fs-err = { workspace = true }
glob = { workspace = true }
itertools = { workspace = true }
owo-colors = { workspace = true }
rustc-hash = { workspace = true }
same-file = { workspace = true }
schemars = { workspace = true, optional = true }
Expand All @@ -36,7 +38,6 @@ toml = { workspace = true }
toml_edit = { workspace = true }
tracing = { workspace = true }
url = { workspace = true }
itertools = { workspace = true }

[dev-dependencies]
anyhow = { workspace = true }
Expand Down
Loading

0 comments on commit 320be30

Please sign in to comment.