Skip to content
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

Use PEP 517 to extract non-static pyproject.toml metadata #2633

Merged
merged 1 commit into from
Mar 25, 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
1 change: 1 addition & 0 deletions Cargo.lock

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

183 changes: 182 additions & 1 deletion crates/pep508-rs/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,88 @@ impl MarkerTree {
}
}

/// Remove the extras from a marker, returning `None` if the marker tree evaluates to `true`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe @BurntSushi should review this marker code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, feel free @BurntSushi.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Messaged with Andrew, I want to build on this branch (but not related to the markers) so I'm going to merge and address any review comments post-merge.

///
/// Any `extra` markers that are always `true` given the provided extras will be removed.
/// Any `extra` markers that are always `false` given the provided extras will be left
/// unchanged.
///
/// For example, if `dev` is a provided extra, given `sys_platform == 'linux' and extra == 'dev'`,
/// the marker will be simplified to `sys_platform == 'linux'`.
pub fn simplify_extras(self, extras: &[ExtraName]) -> Option<MarkerTree> {
/// Returns `true` if the given expression is always `true` given the set of extras.
pub fn is_true(expression: &MarkerExpression, extras: &[ExtraName]) -> bool {
// Ex) `extra == 'dev'`
if expression.l_value == MarkerValue::Extra {
if let MarkerValue::QuotedString(r_string) = &expression.r_value {
if let Ok(r_extra) = ExtraName::from_str(r_string) {
return extras.contains(&r_extra);
}
}
}
// Ex) `'dev' == extra`
if expression.r_value == MarkerValue::Extra {
if let MarkerValue::QuotedString(l_string) = &expression.l_value {
if let Ok(l_extra) = ExtraName::from_str(l_string) {
return extras.contains(&l_extra);
}
}
}
false
}

match self {
Self::Expression(expression) => {
// If the expression is true, we can remove the marker entirely.
if is_true(&expression, extras) {
None
} else {
// If not, return the original marker.
Some(Self::Expression(expression))
}
}
Self::And(expressions) => {
// Remove any expressions that are _true_ due to the presence of an extra.
let simplified = expressions
.into_iter()
.filter_map(|marker| marker.simplify_extras(extras))
.collect::<Vec<_>>();

// If there are no expressions left, return None.
if simplified.is_empty() {
None
} else if simplified.len() == 1 {
// If there is only one expression left, return the remaining expression.
simplified.into_iter().next()
} else {
// If there are still expressions left, return the simplified marker.
Some(Self::And(simplified))
}
}
Self::Or(expressions) => {
let num_expressions = expressions.len();

// Remove any expressions that are _true_ due to the presence of an extra.
let simplified = expressions
.into_iter()
.filter_map(|marker| marker.simplify_extras(extras))
.collect::<Vec<_>>();

// If _any_ of the expressions are true (i.e., if any of the markers were filtered
// out in the above filter step), the entire marker is true.
if simplified.len() < num_expressions {
None
} else if simplified.len() == 1 {
// If there is only one expression left, return the remaining expression.
simplified.into_iter().next()
} else {
// If there are still expressions left, return the simplified marker.
Some(Self::Or(simplified))
}
}
}
}

/// Same as [`Self::evaluate`], but instead of using logging to warn, you can pass your own
/// handler for warnings
pub fn evaluate_reporter(
Expand Down Expand Up @@ -1368,9 +1450,13 @@ fn parse_markers(markers: &str) -> Result<MarkerTree, Pep508Error> {
#[cfg(test)]
mod test {
use crate::marker::{MarkerEnvironment, StringVersion};
use crate::{MarkerExpression, MarkerOperator, MarkerTree, MarkerValue, MarkerValueString};
use crate::{
MarkerExpression, MarkerOperator, MarkerTree, MarkerValue, MarkerValueString,
MarkerValueVersion,
};
use insta::assert_snapshot;
use std::str::FromStr;
use uv_normalize::ExtraName;

fn parse_err(input: &str) -> String {
MarkerTree::from_str(input).unwrap_err().to_string()
Expand Down Expand Up @@ -1616,4 +1702,99 @@ mod test {
)
.unwrap();
}

#[test]
fn test_simplify_extras() {
// Given `os_name == "nt" and extra == "dev"`, simplify to `os_name == "nt"`.
let markers = MarkerTree::from_str(r#"os_name == "nt" and extra == "dev""#).unwrap();
let simplified = markers.simplify_extras(&[ExtraName::from_str("dev").unwrap()]);
assert_eq!(
simplified,
Some(MarkerTree::Expression(MarkerExpression {
l_value: MarkerValue::MarkerEnvString(MarkerValueString::OsName),
operator: MarkerOperator::Equal,
r_value: MarkerValue::QuotedString("nt".to_string()),
}))
);

// Given `os_name == "nt" or extra == "dev"`, remove the marker entirely.
let markers = MarkerTree::from_str(r#"os_name == "nt" or extra == "dev""#).unwrap();
let simplified = markers.simplify_extras(&[ExtraName::from_str("dev").unwrap()]);
assert_eq!(simplified, None);

// Given `extra == "dev"`, remove the marker entirely.
let markers = MarkerTree::from_str(r#"extra == "dev""#).unwrap();
let simplified = markers.simplify_extras(&[ExtraName::from_str("dev").unwrap()]);
assert_eq!(simplified, None);

// Given `extra == "dev" and extra == "test"`, simplify to `extra == "test"`.
let markers = MarkerTree::from_str(r#"extra == "dev" and extra == "test""#).unwrap();
let simplified = markers.simplify_extras(&[ExtraName::from_str("dev").unwrap()]);
assert_eq!(
simplified,
Some(MarkerTree::Expression(MarkerExpression {
l_value: MarkerValue::Extra,
operator: MarkerOperator::Equal,
r_value: MarkerValue::QuotedString("test".to_string()),
}))
);

// Given `os_name == "nt" and extra == "test"`, don't simplify.
let markers = MarkerTree::from_str(r#"os_name == "nt" and extra == "test""#).unwrap();
let simplified = markers.simplify_extras(&[ExtraName::from_str("dev").unwrap()]);
assert_eq!(
simplified,
Some(MarkerTree::And(vec![
MarkerTree::Expression(MarkerExpression {
l_value: MarkerValue::MarkerEnvString(MarkerValueString::OsName),
operator: MarkerOperator::Equal,
r_value: MarkerValue::QuotedString("nt".to_string()),
}),
MarkerTree::Expression(MarkerExpression {
l_value: MarkerValue::Extra,
operator: MarkerOperator::Equal,
r_value: MarkerValue::QuotedString("test".to_string()),
}),
]))
);

// Given `os_name == "nt" and (python_version == "3.7" or extra == "dev")`, simplify to
// `os_name == "nt".
let markers = MarkerTree::from_str(
r#"os_name == "nt" and (python_version == "3.7" or extra == "dev")"#,
)
.unwrap();
let simplified = markers.simplify_extras(&[ExtraName::from_str("dev").unwrap()]);
assert_eq!(
simplified,
Some(MarkerTree::Expression(MarkerExpression {
l_value: MarkerValue::MarkerEnvString(MarkerValueString::OsName),
operator: MarkerOperator::Equal,
r_value: MarkerValue::QuotedString("nt".to_string()),
}))
);

// Given `os_name == "nt" or (python_version == "3.7" and extra == "dev")`, simplify to
// `os_name == "nt" or python_version == "3.7"`.
let markers = MarkerTree::from_str(
r#"os_name == "nt" or (python_version == "3.7" and extra == "dev")"#,
)
.unwrap();
let simplified = markers.simplify_extras(&[ExtraName::from_str("dev").unwrap()]);
assert_eq!(
simplified,
Some(MarkerTree::Or(vec![
MarkerTree::Expression(MarkerExpression {
l_value: MarkerValue::MarkerEnvString(MarkerValueString::OsName),
operator: MarkerOperator::Equal,
r_value: MarkerValue::QuotedString("nt".to_string()),
}),
MarkerTree::Expression(MarkerExpression {
l_value: MarkerValue::MarkerEnvVersion(MarkerValueVersion::PythonVersion),
operator: MarkerOperator::Equal,
r_value: MarkerValue::QuotedString("3.7".to_string()),
}),
]))
);
}
}
1 change: 1 addition & 0 deletions crates/uv-requirements/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ rustc-hash = { workspace = true }
serde = { workspace = true }
toml = { workspace = true }
tracing = { workspace = true }
url = { workspace = true }

[lints]
workspace = true
2 changes: 2 additions & 0 deletions crates/uv-requirements/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
pub use crate::resolver::*;
pub use crate::source_tree::*;
pub use crate::sources::*;
pub use crate::specification::*;

mod confirm;
mod resolver;
mod source_tree;
mod sources;
mod specification;
pub mod upgrade;
127 changes: 127 additions & 0 deletions crates/uv-requirements/src/source_tree.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
use std::borrow::Cow;
use std::path::{Path, PathBuf};
use std::sync::Arc;

use anyhow::{Context, Result};
use futures::{StreamExt, TryStreamExt};
use url::Url;

use distribution_types::{BuildableSource, PathSourceUrl, SourceUrl};
use pep508_rs::Requirement;
use uv_client::RegistryClient;
use uv_distribution::{Reporter, SourceDistCachedBuilder};
use uv_traits::BuildContext;

use crate::ExtrasSpecification;

/// A resolver for requirements specified via source trees.
///
/// Used, e.g., to determine the the input requirements when a user specifies a `pyproject.toml`
/// file, which may require running PEP 517 build hooks to extract metadata.
pub struct SourceTreeResolver<'a> {
/// The requirements for the project.
source_trees: Vec<PathBuf>,
/// The extras to include when resolving requirements.
extras: &'a ExtrasSpecification<'a>,
/// The reporter to use when building source distributions.
reporter: Option<Arc<dyn Reporter>>,
}

impl<'a> SourceTreeResolver<'a> {
/// Instantiate a new [`SourceTreeResolver`] for a given set of `source_trees`.
pub fn new(source_trees: Vec<PathBuf>, extras: &'a ExtrasSpecification<'a>) -> Self {
Self {
source_trees,
extras,
reporter: None,
}
}

/// Set the [`Reporter`] to use for this resolver.
#[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self {
let reporter: Arc<dyn Reporter> = Arc::new(reporter);
Self {
reporter: Some(reporter),
..self
}
}

/// Resolve the requirements from the provided source trees.
pub async fn resolve<T: BuildContext>(
self,
context: &T,
client: &RegistryClient,
) -> Result<Vec<Requirement>> {
let requirements: Vec<_> = futures::stream::iter(self.source_trees.iter())
.map(|source_tree| async {
self.resolve_source_tree(source_tree, context, client).await
})
.buffered(50)
.try_collect()
.await?;
Ok(requirements.into_iter().flatten().collect())
}

/// Infer the package name for a given "unnamed" requirement.
async fn resolve_source_tree<T: BuildContext>(
&self,
source_tree: &Path,
context: &T,
client: &RegistryClient,
) -> Result<Vec<Requirement>> {
// Convert to a buildable source.
let path = fs_err::canonicalize(source_tree).with_context(|| {
format!(
"Failed to canonicalize path to source tree: {}",
source_tree.display()
)
})?;
let Ok(url) = Url::from_directory_path(&path) else {
return Err(anyhow::anyhow!("Failed to convert path to URL"));
};
let source = BuildableSource::Url(SourceUrl::Path(PathSourceUrl {
url: &url,
path: Cow::Owned(path),
}));

// Run the PEP 517 build process to extract metadata from the source distribution.
let builder = if let Some(reporter) = self.reporter.clone() {
SourceDistCachedBuilder::new(context, client).with_reporter(reporter)
} else {
SourceDistCachedBuilder::new(context, client)
};

let metadata = builder
.download_and_build_metadata(&source)
.await
.context("Failed to build source distribution")?;

// Determine the appropriate requirements to return based on the extras. This involves
// evaluating the `extras` expression in any markers, but preserving the remaining marker
// conditions.
match self.extras {
ExtrasSpecification::None => Ok(metadata.requires_dist),
ExtrasSpecification::All => Ok(metadata
.requires_dist
.into_iter()
.map(|requirement| Requirement {
marker: requirement
.marker
.and_then(|marker| marker.simplify_extras(&metadata.provides_extras)),
..requirement
})
.collect()),
ExtrasSpecification::Some(extras) => Ok(metadata
.requires_dist
.into_iter()
.map(|requirement| Requirement {
marker: requirement
.marker
.and_then(|marker| marker.simplify_extras(extras)),
..requirement
})
.collect()),
}
}
}
4 changes: 4 additions & 0 deletions crates/uv-requirements/src/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,8 @@ impl ExtrasSpecification<'_> {
ExtrasSpecification::Some(extras) => extras.contains(name),
}
}

pub fn is_empty(&self) -> bool {
matches!(self, ExtrasSpecification::None)
}
}
Loading
Loading