Skip to content

Commit

Permalink
Respect sources in build requirements
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Oct 15, 2024
1 parent 738d2be commit 629e500
Show file tree
Hide file tree
Showing 13 changed files with 385 additions and 38 deletions.
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.

1 change: 1 addition & 0 deletions crates/uv-build-frontend/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ workspace = true

[dependencies]
uv-configuration = { workspace = true }
uv-distribution = { workspace = true }
uv-distribution-types = { workspace = true }
uv-fs = { workspace = true }
uv-pep440 = { workspace = true }
Expand Down
2 changes: 2 additions & 0 deletions crates/uv-build-frontend/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ static DISTUTILS_NOT_FOUND_RE: LazyLock<Regex> =
pub enum Error {
#[error(transparent)]
Io(#[from] io::Error),
#[error(transparent)]
Lowering(#[from] uv_distribution::MetadataError),
#[error("{} does not appear to be a Python project, as neither `pyproject.toml` nor `setup.py` are present in the directory", _0.simplified_display())]
InvalidSourceDist(PathBuf),
#[error("Invalid `pyproject.toml`")]
Expand Down
95 changes: 85 additions & 10 deletions crates/uv-build-frontend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ use tokio::sync::{Mutex, Semaphore};
use tracing::{debug, info_span, instrument, Instrument};

pub use crate::error::{Error, MissingHeaderCause};
use uv_configuration::{BuildKind, BuildOutput, ConfigSettings};
use uv_configuration::{BuildKind, BuildOutput, ConfigSettings, SourceStrategy};
use uv_distribution::{LowerBound, RequiresDist};
use uv_distribution_types::Resolution;
use uv_fs::{rename_with_retry, PythonExt, Simplified};
use uv_pep440::Version;
Expand Down Expand Up @@ -249,6 +250,7 @@ impl SourceBuild {
build_context: &impl BuildContext,
source_build_context: SourceBuildContext,
version_id: Option<String>,
source_strategy: SourceStrategy,
config_settings: ConfigSettings,
build_isolation: BuildIsolation<'_>,
build_kind: BuildKind,
Expand All @@ -267,8 +269,14 @@ impl SourceBuild {
let default_backend: Pep517Backend = DEFAULT_BACKEND.clone();

// Check if we have a PEP 517 build backend.
let (pep517_backend, project) =
Self::extract_pep517_backend(&source_tree, &default_backend).map_err(|err| *err)?;
let (pep517_backend, project) = Self::extract_pep517_backend(
&source_tree,
fallback_package_name,
source_strategy,
&default_backend,
)
.await
.map_err(|err| *err)?;

let package_name = project
.as_ref()
Expand Down Expand Up @@ -363,6 +371,7 @@ impl SourceBuild {
package_name.as_ref(),
package_version.as_ref(),
version_id.as_deref(),
source_strategy,
build_kind,
level,
&config_settings,
Expand Down Expand Up @@ -421,8 +430,10 @@ impl SourceBuild {
}

/// Extract the PEP 517 backend from the `pyproject.toml` or `setup.py` file.
fn extract_pep517_backend(
async fn extract_pep517_backend(
source_tree: &Path,
package_name: Option<&PackageName>,
source_strategy: SourceStrategy,
default_backend: &Pep517Backend,
) -> Result<(Pep517Backend, Option<Project>), Box<Error>> {
match fs::read_to_string(source_tree.join("pyproject.toml")) {
Expand All @@ -433,7 +444,48 @@ impl SourceBuild {
let pyproject_toml: PyProjectToml =
PyProjectToml::deserialize(pyproject_toml.into_deserializer())
.map_err(Error::InvalidPyprojectTomlSchema)?;

let backend = if let Some(build_system) = pyproject_toml.build_system {
// If necessary, lower the requirements.
let requirements = match source_strategy {
SourceStrategy::Enabled => {
if let Some(name) = pyproject_toml
.project
.as_ref()
.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 {
name: name.clone(),
requires_dist: build_system.requires,
provides_extras: vec![],
};
let requires_dist = RequiresDist::from_project_maybe_workspace(
requires_dist,
source_tree,
source_strategy,
LowerBound::Allow,
)
.await
.map_err(Error::Lowering)?;
requires_dist.requires_dist
} else {
build_system
.requires
.into_iter()
.map(Requirement::from)
.collect()
}
}
SourceStrategy::Disabled => build_system
.requires
.into_iter()
.map(Requirement::from)
.collect(),
};

Pep517Backend {
// If `build-backend` is missing, inject the legacy setuptools backend, but
// retain the `requires`, to match `pip` and `build`. Note that while PEP 517
Expand All @@ -446,11 +498,7 @@ impl SourceBuild {
.build_backend
.unwrap_or_else(|| "setuptools.build_meta:__legacy__".to_string()),
backend_path: build_system.backend_path,
requirements: build_system
.requires
.into_iter()
.map(Requirement::from)
.collect(),
requirements,
}
} else {
// If a `pyproject.toml` is present, but `[build-system]` is missing, proceed with
Expand Down Expand Up @@ -755,6 +803,7 @@ async fn create_pep517_build_environment(
package_name: Option<&PackageName>,
package_version: Option<&Version>,
version_id: Option<&str>,
source_strategy: SourceStrategy,
build_kind: BuildKind,
level: BuildOutput,
config_settings: &ConfigSettings,
Expand Down Expand Up @@ -851,7 +900,33 @@ async fn create_pep517_build_environment(
version_id,
)
})?;
let extra_requires: Vec<_> = extra_requires.into_iter().map(Requirement::from).collect();

// If necessary, lower the requirements.
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 {
name: package_name.clone(),
requires_dist: extra_requires,
provides_extras: vec![],
};
let requires_dist = RequiresDist::from_project_maybe_workspace(
requires_dist,
source_tree,
source_strategy,
LowerBound::Allow,
)
.await
.map_err(Error::Lowering)?;
requires_dist.requires_dist
} else {
extra_requires.into_iter().map(Requirement::from).collect()
}
}
SourceStrategy::Disabled => extra_requires.into_iter().map(Requirement::from).collect(),
};

// Some packages (such as tqdm 4.66.1) list only extra requires that have already been part of
// the pyproject.toml requires (in this case, `wheel`). We can skip doing the whole resolution
Expand Down
4 changes: 3 additions & 1 deletion crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
} = Planner::new(resolution).build(
site_packages,
&Reinstall::default(),
&BuildOptions::default(),
self.build_options,
self.hasher,
self.index_locations,
self.config_settings,
Expand Down Expand Up @@ -312,6 +312,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
subdirectory: Option<&'data Path>,
version_id: Option<String>,
dist: Option<&'data SourceDist>,
sources: SourceStrategy,
build_kind: BuildKind,
build_output: BuildOutput,
) -> Result<SourceBuild> {
Expand Down Expand Up @@ -349,6 +350,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
self,
self.source_build_context.clone(),
version_id,
sources,
self.config_settings.clone(),
self.build_isolation,
build_kind,
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, RequiresDist};
pub use metadata::{
ArchiveMetadata, LowerBound, LoweredRequirement, Metadata, MetadataError, RequiresDist,
};
pub use reporter::Reporter;
pub use source::prune;

Expand Down
29 changes: 20 additions & 9 deletions crates/uv-distribution/src/metadata/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ impl LoweredRequirement {
project_dir: &'data Path,
project_sources: &'data BTreeMap<PackageName, Sources>,
workspace: &'data Workspace,
lower_bound: LowerBound,
) -> impl Iterator<Item = Result<LoweredRequirement, LoweringError>> + 'data {
let (source, origin) = if let Some(source) = project_sources.get(&requirement.name) {
(Some(source), Origin::Project)
Expand Down Expand Up @@ -62,15 +63,17 @@ impl LoweredRequirement {

let Some(source) = source else {
let has_sources = !project_sources.is_empty() || !workspace.sources().is_empty();
// Support recursive editable inclusions.
if has_sources
&& requirement.version_or_url.is_none()
&& &requirement.name != project_name
{
warn_user_once!(
"Missing version constraint (e.g., a lower bound) for `{}`",
requirement.name
);
if matches!(lower_bound, LowerBound::Warn) {
// Support recursive editable inclusions.
if has_sources
&& requirement.version_or_url.is_none()
&& &requirement.name != project_name
{
warn_user_once!(
"Missing version constraint (e.g., a lower bound) for `{}`",
requirement.name
);
}
}
return Either::Left(std::iter::once(Ok(Self(Requirement::from(requirement)))));
};
Expand Down Expand Up @@ -533,3 +536,11 @@ fn path_source(
})
}
}

#[derive(Debug, Copy, Clone)]
pub enum LowerBound {
/// Allow missing lower bounds.
Allow,
/// Warn about missing lower bounds.
Warn,
}
3 changes: 2 additions & 1 deletion crates/uv-distribution/src/metadata/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use uv_pep440::{Version, VersionSpecifiers};
use uv_pypi_types::{HashDigest, ResolutionMetadata};
use uv_workspace::WorkspaceError;

pub use crate::metadata::lowering::LoweredRequirement;
use crate::metadata::lowering::LoweringError;
pub use crate::metadata::lowering::{LowerBound, LoweredRequirement};
pub use crate::metadata::requires_dist::RequiresDist;

mod lowering;
Expand Down Expand Up @@ -77,6 +77,7 @@ impl Metadata {
},
install_path,
sources,
LowerBound::Warn,
)
.await?;

Expand Down
9 changes: 8 additions & 1 deletion crates/uv-distribution/src/metadata/requires_dist.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::metadata::{LoweredRequirement, MetadataError};
use crate::Metadata;

use crate::metadata::lowering::LowerBound;
use std::collections::BTreeMap;
use std::path::Path;
use uv_configuration::SourceStrategy;
Expand Down Expand Up @@ -38,6 +39,7 @@ impl RequiresDist {
metadata: uv_pypi_types::RequiresDist,
install_path: &Path,
sources: SourceStrategy,
lower_bound: LowerBound,
) -> Result<Self, MetadataError> {
// TODO(konsti): Limit discovery for Git checkouts to Git root.
// TODO(konsti): Cache workspace discovery.
Expand All @@ -48,13 +50,14 @@ impl RequiresDist {
return Ok(Self::from_metadata23(metadata));
};

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

fn from_project_workspace(
metadata: uv_pypi_types::RequiresDist,
project_workspace: &ProjectWorkspace,
source_strategy: SourceStrategy,
lower_bound: LowerBound,
) -> Result<Self, MetadataError> {
// Collect any `tool.uv.sources` and `tool.uv.dev_dependencies` from `pyproject.toml`.
let empty = BTreeMap::default();
Expand Down Expand Up @@ -92,6 +95,7 @@ impl RequiresDist {
project_workspace.project_root(),
sources,
project_workspace.workspace(),
lower_bound,
)
.map(move |requirement| match requirement {
Ok(requirement) => Ok(requirement.into_inner()),
Expand Down Expand Up @@ -124,6 +128,7 @@ impl RequiresDist {
project_workspace.project_root(),
sources,
project_workspace.workspace(),
lower_bound,
)
.map(move |requirement| match requirement {
Ok(requirement) => Ok(requirement.into_inner()),
Expand Down Expand Up @@ -170,6 +175,7 @@ mod test {
use uv_workspace::pyproject::PyProjectToml;
use uv_workspace::{DiscoveryOptions, ProjectWorkspace};

use crate::metadata::lowering::LowerBound;
use crate::RequiresDist;

async fn requires_dist_from_pyproject_toml(contents: &str) -> anyhow::Result<RequiresDist> {
Expand All @@ -193,6 +199,7 @@ mod test {
requires_dist,
&project_workspace,
SourceStrategy::Enabled,
LowerBound::Warn,
)?)
}

Expand Down
Loading

0 comments on commit 629e500

Please sign in to comment.