Avoid validating extra and group sources in `build-system.requires` (#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 https://github.com/astral-sh/uv/issues/9259.
This commit is contained in:
Charlie Marsh 2024-11-20 09:05:50 -05:00 committed by GitHub
parent ccc0962cbd
commit 289771e311
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 193 additions and 20 deletions

View File

@ -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;
@ -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,
@ -481,7 +478,7 @@ impl SourceBuild {
)
.await
.map_err(Error::Lowering)?;
requires_dist.requires_dist
build_requires.requires_dist
} else {
build_system
.requires
@ -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,
@ -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()
}

View File

@ -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;

View File

@ -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,
})
}
}

View File

@ -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;

View File

@ -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>>,
}

View File

@ -1,3 +1,4 @@
mod build_requires;
mod metadata10;
mod metadata12;
mod metadata23;
@ -5,14 +6,18 @@ 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;

View File

@ -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"
"#,
)?;
@ -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(())