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

Avoid validating extra and group sources in build-system.requires #9273

Merged
merged 1 commit into from
Nov 20, 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
24 changes: 9 additions & 15 deletions crates/uv-build-frontend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use tokio::sync::{Mutex, Semaphore};
use tracing::{debug, info_span, instrument, Instrument};

use uv_configuration::{BuildKind, BuildOutput, ConfigSettings, LowerBound, SourceStrategy};
use uv_distribution::RequiresDist;
use uv_distribution::BuildRequires;
use uv_distribution_types::{IndexLocations, Resolution};
use uv_fs::{PythonExt, Simplified};
use uv_pep440::Version;
Expand Down Expand Up @@ -464,15 +464,12 @@ impl SourceBuild {
.map(|project| &project.name)
.or(package_name)
{
// TODO(charlie): Add a type to lower requirements without providing
// empty extras.
let requires_dist = uv_pypi_types::RequiresDist {
let build_requires = uv_pypi_types::BuildRequires {
name: name.clone(),
requires_dist: build_system.requires,
provides_extras: vec![],
};
let requires_dist = RequiresDist::from_project_maybe_workspace(
requires_dist,
let build_requires = BuildRequires::from_project_maybe_workspace(
build_requires,
install_path,
None,
locations,
Expand All @@ -481,7 +478,7 @@ impl SourceBuild {
)
.await
.map_err(Error::Lowering)?;
requires_dist.requires_dist
build_requires.requires_dist
} else {
build_system
.requires
Expand Down Expand Up @@ -909,15 +906,12 @@ async fn create_pep517_build_environment(
let extra_requires = match source_strategy {
SourceStrategy::Enabled => {
if let Some(package_name) = package_name {
// TODO(charlie): Add a type to lower requirements without providing
// empty extras.
let requires_dist = uv_pypi_types::RequiresDist {
let build_requires = uv_pypi_types::BuildRequires {
name: package_name.clone(),
requires_dist: extra_requires,
provides_extras: vec![],
};
let requires_dist = RequiresDist::from_project_maybe_workspace(
requires_dist,
let build_requires = BuildRequires::from_project_maybe_workspace(
build_requires,
install_path,
None,
locations,
Expand All @@ -926,7 +920,7 @@ async fn create_pep517_build_environment(
)
.await
.map_err(Error::Lowering)?;
requires_dist.requires_dist
build_requires.requires_dist
} else {
extra_requires.into_iter().map(Requirement::from).collect()
}
Expand Down
4 changes: 3 additions & 1 deletion crates/uv-distribution/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ pub use distribution_database::{DistributionDatabase, HttpArchivePointer, LocalA
pub use download::LocalWheel;
pub use error::Error;
pub use index::{BuiltWheelIndex, RegistryWheelIndex};
pub use metadata::{ArchiveMetadata, LoweredRequirement, Metadata, MetadataError, RequiresDist};
pub use metadata::{
ArchiveMetadata, BuildRequires, LoweredRequirement, Metadata, MetadataError, RequiresDist,
};
pub use reporter::Reporter;
pub use source::prune;

Expand Down
152 changes: 152 additions & 0 deletions crates/uv-distribution/src/metadata/build_requires.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
use std::collections::BTreeMap;
use std::path::Path;

use uv_configuration::{LowerBound, SourceStrategy};
use uv_distribution_types::IndexLocations;
use uv_normalize::PackageName;
use uv_workspace::pyproject::ToolUvSources;
use uv_workspace::{DiscoveryOptions, ProjectWorkspace};

use crate::metadata::{GitWorkspaceMember, LoweredRequirement, MetadataError};

/// Lowered requirements from a `[build-system.requires]` field in a `pyproject.toml` file.
#[derive(Debug, Clone)]
pub struct BuildRequires {
pub name: PackageName,
pub requires_dist: Vec<uv_pypi_types::Requirement>,
}

impl BuildRequires {
/// Lower without considering `tool.uv` in `pyproject.toml`, used for index and other archive
/// dependencies.
pub fn from_metadata23(metadata: uv_pypi_types::BuildRequires) -> Self {
Self {
name: metadata.name,
requires_dist: metadata
.requires_dist
.into_iter()
.map(uv_pypi_types::Requirement::from)
.collect(),
}
}

/// Lower by considering `tool.uv` in `pyproject.toml` if present, used for Git and directory
/// dependencies.
pub async fn from_project_maybe_workspace(
metadata: uv_pypi_types::BuildRequires,
install_path: &Path,
git_member: Option<&GitWorkspaceMember<'_>>,
locations: &IndexLocations,
sources: SourceStrategy,
lower_bound: LowerBound,
) -> Result<Self, MetadataError> {
// TODO(konsti): Cache workspace discovery.
let discovery_options = if let Some(git_member) = &git_member {
DiscoveryOptions {
stop_discovery_at: Some(
git_member
.fetch_root
.parent()
.expect("git checkout has a parent"),
),
..Default::default()
}
} else {
DiscoveryOptions::default()
};
let Some(project_workspace) =
ProjectWorkspace::from_maybe_project_root(install_path, &discovery_options).await?
else {
return Ok(Self::from_metadata23(metadata));
};

Self::from_project_workspace(
metadata,
&project_workspace,
git_member,
locations,
sources,
lower_bound,
)
}

/// Lower the `build-system.requires` field from a `pyproject.toml` file.
fn from_project_workspace(
metadata: uv_pypi_types::BuildRequires,
project_workspace: &ProjectWorkspace,
git_member: Option<&GitWorkspaceMember<'_>>,
locations: &IndexLocations,
source_strategy: SourceStrategy,
lower_bound: LowerBound,
) -> Result<Self, MetadataError> {
// Collect any `tool.uv.index` entries.
let empty = vec![];
let project_indexes = match source_strategy {
SourceStrategy::Enabled => project_workspace
.current_project()
.pyproject_toml()
.tool
.as_ref()
.and_then(|tool| tool.uv.as_ref())
.and_then(|uv| uv.index.as_deref())
.unwrap_or(&empty),
SourceStrategy::Disabled => &empty,
};

// Collect any `tool.uv.sources` and `tool.uv.dev_dependencies` from `pyproject.toml`.
let empty = BTreeMap::default();
let project_sources = match source_strategy {
SourceStrategy::Enabled => project_workspace
.current_project()
.pyproject_toml()
.tool
.as_ref()
.and_then(|tool| tool.uv.as_ref())
.and_then(|uv| uv.sources.as_ref())
.map(ToolUvSources::inner)
.unwrap_or(&empty),
SourceStrategy::Disabled => &empty,
};

// Lower the requirements.
let requires_dist = metadata.requires_dist.into_iter();
let requires_dist = match source_strategy {
SourceStrategy::Enabled => requires_dist
.flat_map(|requirement| {
let requirement_name = requirement.name.clone();
let extra = requirement.marker.top_level_extra_name();
let group = None;
LoweredRequirement::from_requirement(
requirement,
&metadata.name,
project_workspace.project_root(),
project_sources,
project_indexes,
extra.as_ref(),
group,
locations,
project_workspace.workspace(),
lower_bound,
git_member,
)
.map(move |requirement| match requirement {
Ok(requirement) => Ok(requirement.into_inner()),
Err(err) => Err(MetadataError::LoweringError(
requirement_name.clone(),
Box::new(err),
)),
})
})
.collect::<Result<Vec<_>, _>>()?,
SourceStrategy::Disabled => requires_dist
.into_iter()
.map(uv_pypi_types::Requirement::from)
.collect(),
};

Ok(Self {
name: metadata.name,
requires_dist,
})
}
}
2 changes: 2 additions & 0 deletions crates/uv-distribution/src/metadata/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ use uv_pypi_types::{HashDigest, ResolutionMetadata};
use uv_workspace::dependency_groups::DependencyGroupError;
use uv_workspace::WorkspaceError;

pub use crate::metadata::build_requires::BuildRequires;
pub use crate::metadata::lowering::LoweredRequirement;
use crate::metadata::lowering::LoweringError;
pub use crate::metadata::requires_dist::RequiresDist;

mod build_requires;
mod lowering;
mod requires_dist;

Expand Down
13 changes: 13 additions & 0 deletions crates/uv-pypi-types/src/metadata/build_requires.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
use uv_normalize::PackageName;
use uv_pep508::Requirement;

use crate::VerbatimParsedUrl;

/// The `build-system.requires` field in a `pyproject.toml` file.
///
/// See: <https://peps.python.org/pep-0518/>
#[derive(Debug, Clone)]
pub struct BuildRequires {
pub name: PackageName,
pub requires_dist: Vec<Requirement<VerbatimParsedUrl>>,
}
9 changes: 7 additions & 2 deletions crates/uv-pypi-types/src/metadata/mod.rs
Original file line number Diff line number Diff line change
@@ -1,18 +1,23 @@
mod build_requires;
mod metadata10;
mod metadata12;
mod metadata23;
mod metadata_resolver;
mod pyproject_toml;
mod requires_txt;

use crate::VerbatimParsedUrl;
use mailparse::{MailHeaderMap, MailParseError};
use std::str::Utf8Error;

use mailparse::{MailHeaderMap, MailParseError};
use thiserror::Error;

use uv_normalize::InvalidNameError;
use uv_pep440::{VersionParseError, VersionSpecifiersParseError};
use uv_pep508::Pep508Error;

use crate::VerbatimParsedUrl;

pub use build_requires::BuildRequires;
pub use metadata10::Metadata10;
pub use metadata12::Metadata12;
pub use metadata23::Metadata23;
Expand Down
9 changes: 7 additions & 2 deletions crates/uv/tests/it/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4386,6 +4386,10 @@ fn sync_multiple_sources_index_disjoint_extras() -> Result<()> {
name = "torch-cu124"
url = "https://download.pytorch.org/whl/cu124"
explicit = true

[build-system]
requires = ["setuptools>=42"]
build-backend = "setuptools.build_meta"
"#,
)?;

Expand All @@ -4403,10 +4407,11 @@ fn sync_multiple_sources_index_disjoint_extras() -> Result<()> {

----- stderr -----
Resolved 4 packages in [TIME]
Prepared 2 packages in [TIME]
Installed 2 packages in [TIME]
Prepared 3 packages in [TIME]
Installed 3 packages in [TIME]
+ jinja2==3.1.3
+ markupsafe==2.1.5
+ project==0.1.0 (from file://[TEMP_DIR]/)
"###);

Ok(())
Expand Down
Loading