Skip to content

Commit

Permalink
Avoid building dynamic versions when validating lockfile
Browse files Browse the repository at this point in the history
  • Loading branch information
@charliermarsh
charliermarsh committed Jan 17, 2025
1 parent 3344609 commit f2ddc03
Show file tree
Hide file tree
Showing 4 changed files with 166 additions and 62 deletions.
9 changes: 7 additions & 2 deletions crates/uv-distribution/src/distribution_database.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,8 +496,13 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
}

/// Return the [`RequiresDist`] from a `pyproject.toml`, if it can be statically extracted.
pub async fn requires_dist(&self, project_root: &Path) -> Result<Option<RequiresDist>, Error> {
self.builder.source_tree_requires_dist(project_root).await
pub async fn requires_dist(
&self,
source_tree: impl AsRef<Path>,
) -> Result<Option<RequiresDist>, Error> {
self.builder
.source_tree_requires_dist(source_tree.as_ref())
.await
}

/// Stream a wheel from a URL, unzipping it into the cache as it's downloaded.
Expand Down
10 changes: 5 additions & 5 deletions crates/uv-distribution/src/source/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1344,18 +1344,18 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
/// Return the [`RequiresDist`] from a `pyproject.toml`, if it can be statically extracted.
pub(crate) async fn source_tree_requires_dist(
&self,
project_root: &Path,
source_tree: &Path,
) -> Result<Option<RequiresDist>, Error> {
// Attempt to read static metadata from the `pyproject.toml`.
match read_requires_dist(project_root).await {
match read_requires_dist(source_tree).await {
Ok(requires_dist) => {
debug!(
"Found static `requires-dist` for: {}",
project_root.display()
source_tree.display()
);
let requires_dist = RequiresDist::from_project_maybe_workspace(
requires_dist,
project_root,
source_tree,
None,
self.build_context.locations(),
self.build_context.sources(),
Expand All @@ -1375,7 +1375,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
) => {
debug!(
"No static `requires-dist` available for: {} ({err:?})",
project_root.display()
source_tree.display()
);
Ok(None)
}
Expand Down
130 changes: 80 additions & 50 deletions crates/uv-resolver/src/lock/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use url::Url;

use uv_cache_key::RepositoryUrl;
use uv_configuration::BuildOptions;
use uv_distribution::DistributionDatabase;
use uv_distribution::{DistributionDatabase, RequiresDist};
use uv_distribution_filename::{
BuildTag, DistExtension, ExtensionError, SourceDistExtension, WheelFilename,
};
Expand Down Expand Up @@ -1213,58 +1213,80 @@ impl Lock {
continue;
}

// Get the metadata for the distribution.
let dist = package.to_dist(
root,
// When validating, it's okay to use wheels that don't match the current platform.
TagPolicy::Preferred(tags),
// When validating, it's okay to use (e.g.) a source distribution with `--no-build`.
// We're just trying to determine whether the lockfile is up-to-date. If we end
// up needing to build a source distribution in order to do so, below, we'll error
// there.
&BuildOptions::default(),
)?;

// Fetch the metadata for the distribution.
//
// TODO(charlie): We don't need the version here, so we could avoid running a PEP 517
// build if only the version is dynamic.
let metadata = {
let id = dist.version_id();
if let Some(archive) =
index
.distributions()
.get(&id)
.as_deref()
.and_then(|response| {
if let MetadataResponse::Found(archive, ..) = response {
Some(archive)
} else {
None
}
})
{
// If the metadata is already in the index, return it.
archive.metadata.clone()
} else {
// Run the PEP 517 build process to extract metadata from the source distribution.
let archive = database
.get_or_build_wheel_metadata(&dist, hasher.get(&dist))
.await
.map_err(|err| LockErrorKind::Resolution {
id: package.id.clone(),
err,
})?;

let metadata = archive.metadata.clone();
// If the distribution is a source tree, attempt to extract the requirements from the
// `pyproject.toml` directly. The distribution database will do this too, but we can be
// even more aggressive here since we _only_ need the requirements. So, for example,
// even if the version is dynamic, we can still extract the requirements without
// performing a build, unlike in the database where we typically construct a "complete"
// metadata object.
let metadata = if let Some(source_tree) = package.id.source.as_source_tree() {
database
.requires_dist(root.join(source_tree))
.await
.map_err(|err| LockErrorKind::Resolution {
id: package.id.clone(),
err,
})?
} else {
None
};

// Insert the metadata into the index.
index
.distributions()
.done(id, Arc::new(MetadataResponse::Found(archive)));
let metadata = if let Some(metadata) = metadata {
metadata
} else {
// Get the metadata for the distribution.
let dist = package.to_dist(
root,
// When validating, it's okay to use wheels that don't match the current platform.
TagPolicy::Preferred(tags),
// When validating, it's okay to use (e.g.) a source distribution with `--no-build`.
// We're just trying to determine whether the lockfile is up-to-date. If we end
// up needing to build a source distribution in order to do so, below, we'll error
// there.
&BuildOptions::default(),
)?;

let metadata = {
let id = dist.version_id();
if let Some(archive) =
index
.distributions()
.get(&id)
.as_deref()
.and_then(|response| {
if let MetadataResponse::Found(archive, ..) = response {
Some(archive)
} else {
None
}
})
{
// If the metadata is already in the index, return it.
archive.metadata.clone()
} else {
// Run the PEP 517 build process to extract metadata from the source distribution.
let archive = database
.get_or_build_wheel_metadata(&dist, hasher.get(&dist))
.await
.map_err(|err| LockErrorKind::Resolution {
id: package.id.clone(),
err,
})?;

let metadata = archive.metadata.clone();

// Insert the metadata into the index.
index
.distributions()
.done(id, Arc::new(MetadataResponse::Found(archive)));

metadata
}
};

metadata
}
RequiresDist::from(metadata)
};

// Validate the `requires-dist` metadata.
Expand Down Expand Up @@ -2770,13 +2792,21 @@ impl Source {
}

/// Returns `true` if the source is that of a source tree.
pub(crate) fn is_source_tree(&self) -> bool {
fn is_source_tree(&self) -> bool {
match self {
Source::Directory(..) | Source::Editable(..) | Source::Virtual(..) => true,
Source::Path(..) | Source::Git(..) | Source::Registry(..) | Source::Direct(..) => false,
}
}

/// Returns the path to the source tree, if the source is a source tree.
fn as_source_tree(&self) -> Option<&Path> {
match self {
Source::Directory(path) | Source::Editable(path) | Source::Virtual(path) => Some(path),
Source::Path(..) | Source::Git(..) | Source::Registry(..) | Source::Direct(..) => None,
}
}

fn to_toml(&self, table: &mut Table) {
let mut source_table = InlineTable::new();
match *self {
Expand Down
79 changes: 74 additions & 5 deletions crates/uv/tests/it/lock.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7281,14 +7281,14 @@ fn lock_relative_lock_deserialization() -> Result<()> {

uv_snapshot!(context.filters(), context.lock().current_dir(&child), @r###"
success: false
exit_code: 2
exit_code: 1
----- stdout -----

----- stderr -----
Using CPython 3.12.[X] interpreter at: [PYTHON-3.12]
error: Failed to generate package metadata for `child==0.1.0 @ editable+.`
Caused by: Failed to parse entry: `member`
Caused by: `member` references a workspace in `tool.uv.sources` (e.g., `member = { workspace = true }`), but is not a workspace member
× Failed to build `child @ file://[TEMP_DIR]/packages/child`
├─▶ Failed to parse entry: `member`
╰─▶ `member` references a workspace in `tool.uv.sources` (e.g., `member = { workspace = true }`), but is not a workspace member
"###);

Ok(())
Expand Down Expand Up @@ -14936,11 +14936,13 @@ fn lock_explicit_default_index() -> Result<()> {
DEBUG Using Python request `>=3.12` from `requires-python` metadata
DEBUG The virtual environment's Python version satisfies `>=3.12`
DEBUG Using request timeout of [TIME]
DEBUG Found static `pyproject.toml` for: project @ file://[TEMP_DIR]/
DEBUG Found static `requires-dist` for: [TEMP_DIR]/
DEBUG No workspace root found, using project root
DEBUG Ignoring existing lockfile due to mismatched requirements for: `project==0.1.0`
Requested: {Requirement { name: PackageName("anyio"), extras: [], groups: [], marker: true, source: Registry { specifier: VersionSpecifiers([]), index: None, conflict: None }, origin: None }}
Existing: {Requirement { name: PackageName("iniconfig"), extras: [], groups: [], marker: true, source: Registry { specifier: VersionSpecifiers([VersionSpecifier { operator: Equal, version: "2.0.0" }]), index: Some(Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("test.pypi.org")), port: None, path: "/simple", query: None, fragment: None }), conflict: None }, origin: None }}
DEBUG Found static `pyproject.toml` for: project @ file://[TEMP_DIR]/
DEBUG No workspace root found, using project root
DEBUG Solving with installed Python version: 3.12.[X]
DEBUG Solving with target Python version: >=3.12
DEBUG Adding direct dependency: project*
Expand Down Expand Up @@ -19915,6 +19917,7 @@ fn lock_dynamic_version() -> Result<()> {
name = "project"
requires-python = ">=3.12"
dynamic = ["version"]
dependencies = []

[build-system]
requires = ["setuptools"]
Expand Down Expand Up @@ -20011,6 +20014,72 @@ fn lock_dynamic_version() -> Result<()> {
Ok(())
}

/// Validating a lockfile with a dynamic version (but static dependencies) shouldn't require
/// building the package.
#[test]
fn lock_dynamic_version_no_build() -> Result<()> {
let context = TestContext::new("3.12");

let pyproject_toml = context.temp_dir.child("pyproject.toml");
pyproject_toml.write_str(
r#"
[project]
name = "project"
requires-python = ">=3.12"
dynamic = ["version"]
dependencies = []

[build-system]
requires = ["setuptools"]
build-backend = "setuptools.build_meta"

[tool.uv]
cache-keys = [{ file = "pyproject.toml" }, { file = "src/project/__init__.py" }]

[tool.setuptools.dynamic]
version = { attr = "project.__version__" }

[tool.setuptools]
package-dir = { "" = "src" }

[tool.setuptools.packages.find]
where = ["src"]
"#,
)?;

context
.temp_dir
.child("src")
.child("project")
.child("__init__.py")
.write_str("__version__ = '0.1.0'")?;

context.temp_dir.child("uv.lock").write_str(indoc::indoc! {
r#"
version = 1
requires-python = ">=3.12"

[options]
exclude-newer = "2024-03-25T00:00:00Z"

[[package]]
name = "project"
source = { editable = "." }
"#})?;

// Validate the lockfile with `--offline` to ensure that the package itself isn't built.
uv_snapshot!(context.filters(), context.lock().arg("--locked").arg("--offline"), @r###"
success: true
exit_code: 0
----- stdout -----

----- stderr -----
Resolved 1 package in [TIME]
"###);

Ok(())
}

/// Lock a package that depends on a package with a dynamic version using a `workspace` source.
#[test]
fn lock_dynamic_version_workspace_member() -> Result<()> {
Expand Down

0 comments on commit f2ddc03

Please sign in to comment.