Skip to content

Commit

Permalink
Avoid validating extra and group sources in build-system.requires (#…
Browse files Browse the repository at this point in the history
…9273)

## Summary

This was an oversight in the initial implementation. We shouldn't
validate sources for the `build-system.requires` field, since extras and
groups can _never_ be active.

Closes #9259.
  • Loading branch information
charliermarsh authored Nov 20, 2024
1 parent ccc0962 commit 289771e
Show file tree
Hide file tree
Showing 7 changed files with 193 additions and 20 deletions.
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

0 comments on commit 289771e

Please sign in to comment.