Return a structured result from Lock::satisfies (#6119)

## Summary

Gives the caller control over how messages are reported back to the
user. Also merges the index-location validation into the lock, since
we're already iterating over the packages.
This commit is contained in:
Charlie Marsh 2024-08-15 13:19:40 -04:00 committed by GitHub
parent b627c9f5e1
commit 9d514cbbe0
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 151 additions and 56 deletions

View file

@ -3,7 +3,7 @@ pub use error::{NoSolutionError, NoSolutionHeader, ResolveError};
pub use exclude_newer::ExcludeNewer;
pub use exclusions::Exclusions;
pub use flat_index::FlatIndex;
pub use lock::{Lock, LockError, ResolverManifest, TreeDisplay};
pub use lock::{Lock, LockError, ResolverManifest, SatisfiesResult, TreeDisplay};
pub use manifest::Manifest;
pub use options::{Options, OptionsBuilder};
pub use preferences::{Preference, PreferenceError, Preferences};

View file

@ -17,9 +17,10 @@ use cache_key::RepositoryUrl;
use distribution_filename::{DistExtension, ExtensionError, SourceDistExtension, WheelFilename};
use distribution_types::{
BuiltDist, DirectUrlBuiltDist, DirectUrlSourceDist, DirectorySourceDist, Dist,
DistributionMetadata, FileLocation, GitSourceDist, HashPolicy, IndexUrl, Name, PathBuiltDist,
PathSourceDist, RegistryBuiltDist, RegistryBuiltWheel, RegistrySourceDist, RemoteSource,
Resolution, ResolvedDist, ToUrlError, UrlString,
DistributionMetadata, FileLocation, FlatIndexLocation, GitSourceDist, HashPolicy,
IndexLocations, IndexUrl, Name, PathBuiltDist, PathSourceDist, RegistryBuiltDist,
RegistryBuiltWheel, RegistrySourceDist, RemoteSource, Resolution, ResolvedDist, ToUrlError,
UrlString,
};
use pep440_rs::Version;
use pep508_rs::{MarkerEnvironment, MarkerTree, VerbatimUrl, VerbatimUrlError};
@ -630,9 +631,10 @@ impl Lock {
members: &[PackageName],
constraints: &[Requirement],
overrides: &[Requirement],
indexes: Option<&IndexLocations>,
tags: &Tags,
database: &DistributionDatabase<'_, Context>,
) -> Result<bool, LockError> {
) -> Result<SatisfiesResult<'_>, LockError> {
let mut queue: VecDeque<&Package> = VecDeque::new();
let mut seen = FxHashSet::default();
@ -645,7 +647,7 @@ impl Lock {
"Mismatched members:\n expected: {:?}\n found: {:?}",
expected, actual
);
return Ok(false);
return Ok(SatisfiesResult::MismatchedMembers(expected, actual));
}
}
@ -668,7 +670,7 @@ impl Lock {
"Mismatched constraints:\n expected: {:?}\n found: {:?}",
expected, actual
);
return Ok(false);
return Ok(SatisfiesResult::MismatchedConstraints(expected, actual));
}
}
@ -691,10 +693,20 @@ impl Lock {
"Mismatched overrides:\n expected: {:?}\n found: {:?}",
expected, actual
);
return Ok(false);
return Ok(SatisfiesResult::MismatchedOverrides(expected, actual));
}
}
// Collect the set of available indexes (both `--index-url` and `--find-links` entries).
let indexes = indexes.map(|locations| {
locations
.indexes()
.map(IndexUrl::redacted)
.chain(locations.flat_index().map(FlatIndexLocation::redacted))
.map(UrlString::from)
.collect::<BTreeSet<_>>()
});
// Add the workspace packages to the queue.
for root_name in workspace.packages().keys() {
let root = self
@ -704,7 +716,7 @@ impl Lock {
let Some(root) = root else {
// The package is not in the lockfile, so it can't be satisfied.
debug!("Workspace package `{root_name}` not found in lockfile");
return Ok(false);
return Ok(SatisfiesResult::MissingRoot(root_name.clone()));
};
// Add the base package.
@ -712,6 +724,20 @@ impl Lock {
}
while let Some(package) = queue.pop_front() {
// If the lockfile references an index that was not provided, we can't validate it.
if let Source::Registry(index) = &package.id.source {
if indexes
.as_ref()
.is_some_and(|indexes| !indexes.contains(index))
{
return Ok(SatisfiesResult::MissingIndex(
&package.id.name,
&package.id.version,
index,
));
}
}
// If the package is immutable, we don't need to validate it (or its dependencies).
if package.id.source.is_immutable() {
continue;
@ -725,7 +751,10 @@ impl Lock {
.await
else {
debug!("Failed to get metadata for: {}", package.id);
return Ok(false);
return Ok(SatisfiesResult::MissingMetadata(
&package.id.name,
&package.id.version,
));
};
// Validate the `requires-dist` metadata.
@ -749,7 +778,12 @@ impl Lock {
"Mismatched `requires-dist` for {}:\n expected: {:?}\n found: {:?}",
package.id, expected, actual
);
return Ok(false);
return Ok(SatisfiesResult::MismatchedRequiresDist(
&package.id.name,
&package.id.version,
expected,
actual,
));
}
}
@ -790,7 +824,12 @@ impl Lock {
"Mismatched `requires-dev` for {}:\n expected: {:?}\n found: {:?}",
package.id, expected, actual
);
return Ok(false);
return Ok(SatisfiesResult::MismatchedDevDependencies(
&package.id.name,
&package.id.version,
expected,
actual,
));
}
}
@ -823,10 +862,43 @@ impl Lock {
}
}
Ok(true)
Ok(SatisfiesResult::Satisfied)
}
}
/// The result of checking if a lockfile satisfies a set of requirements.
#[derive(Debug)]
pub enum SatisfiesResult<'lock> {
/// The lockfile satisfies the requirements.
Satisfied,
/// The lockfile uses a different set of workspace members.
MismatchedMembers(BTreeSet<PackageName>, &'lock BTreeSet<PackageName>),
/// The lockfile uses a different set of constraints.
MismatchedConstraints(BTreeSet<Requirement>, BTreeSet<Requirement>),
/// The lockfile uses a different set of overrides.
MismatchedOverrides(BTreeSet<Requirement>, BTreeSet<Requirement>),
/// The lockfile is missing a workspace member.
MissingRoot(PackageName),
/// The lockfile referenced an index that was not provided
MissingIndex(&'lock PackageName, &'lock Version, &'lock UrlString),
/// The resolver failed to generate metadata for a given package.
MissingMetadata(&'lock PackageName, &'lock Version),
/// A package in the lockfile contains different `requires-dist` metadata than expected.
MismatchedRequiresDist(
&'lock PackageName,
&'lock Version,
BTreeSet<Requirement>,
BTreeSet<Requirement>,
),
/// A package in the lockfile contains different `dev-dependencies` metadata than expected.
MismatchedDevDependencies(
&'lock PackageName,
&'lock Version,
BTreeMap<GroupName, BTreeSet<Requirement>>,
BTreeMap<GroupName, BTreeSet<Requirement>>,
),
}
/// We discard the lockfile if these options match.
#[derive(Clone, Debug, Default, serde::Deserialize, PartialEq, Eq)]
#[serde(rename_all = "kebab-case")]

View file

@ -8,9 +8,7 @@ use owo_colors::OwoColorize;
use rustc_hash::{FxBuildHasher, FxHashMap};
use tracing::debug;
use distribution_types::{
FlatIndexLocation, IndexLocations, IndexUrl, UnresolvedRequirementSpecification, UrlString,
};
use distribution_types::{IndexLocations, UnresolvedRequirementSpecification};
use pep440_rs::Version;
use pypi_types::Requirement;
use uv_auth::store_credentials_from_url;
@ -29,7 +27,7 @@ use uv_requirements::upgrade::{read_lock_requirements, LockedRequirements};
use uv_requirements::NamedRequirementsResolver;
use uv_resolver::{
FlatIndex, Lock, Options, OptionsBuilder, PythonRequirement, RequiresPython, ResolverManifest,
ResolverMarkers,
ResolverMarkers, SatisfiesResult,
};
use uv_types::{BuildContext, BuildIsolation, EmptyInstalledPackages, HashStrategy};
use uv_warnings::{warn_user, warn_user_once};
@ -627,55 +625,81 @@ impl ValidatedLock {
// However, iIf _no_ indexes were provided, we assume that the user wants to reuse the existing
// distributions, even though a failure to reuse the lockfile will result in re-resolving
// against PyPI by default.
if !index_locations.is_none() {
// Collect the set of available indexes (both `--index-url` and `--find-links` entries).
let indexes = index_locations
.indexes()
.map(IndexUrl::redacted)
.chain(
index_locations
.flat_index()
.map(FlatIndexLocation::redacted),
)
.map(UrlString::from)
.collect::<BTreeSet<_>>();
// Find any packages in the lockfile that reference a registry that is no longer included in
// the current configuration.
for package in lock.packages() {
let Some(index) = package.index() else {
continue;
let indexes = if index_locations.is_none() {
None
} else {
Some(index_locations)
};
if !indexes.contains(index) {
let _ = writeln!(
printer.stderr(),
"Ignoring existing lockfile due to removal of referenced registry: {index}"
);
// It's fine to prefer the existing versions, though.
return Ok(Self::Preferable(lock));
}
}
}
// Determine whether the lockfile satisfies the workspace requirements.
if lock
match lock
.satisfies(
workspace,
members,
constraints,
overrides,
indexes,
interpreter.tags()?,
database,
)
.await?
{
SatisfiesResult::Satisfied => {
debug!("Existing `uv.lock` satisfies workspace requirements");
Ok(Self::Satisfies(lock))
} else {
debug!("Existing `uv.lock` does not satisfy workspace requirements; ignoring...");
}
SatisfiesResult::MismatchedMembers(expected, actual) => {
debug!(
"Ignoring existing lockfile due to mismatched members:\n Expected: {:?}\n Actual: {:?}",
expected, actual
);
Ok(Self::Preferable(lock))
}
SatisfiesResult::MismatchedConstraints(expected, actual) => {
debug!(
"Ignoring existing lockfile due to mismatched constraints:\n Expected: {:?}\n Actual: {:?}",
expected, actual
);
Ok(Self::Preferable(lock))
}
SatisfiesResult::MismatchedOverrides(expected, actual) => {
debug!(
"Ignoring existing lockfile due to mismatched overrides:\n Expected: {:?}\n Actual: {:?}",
expected, actual
);
Ok(Self::Preferable(lock))
}
SatisfiesResult::MissingRoot(name) => {
debug!("Ignoring existing lockfile due to missing root package: `{name}`");
Ok(Self::Preferable(lock))
}
SatisfiesResult::MissingIndex(name, version, index) => {
debug!(
"Ignoring existing lockfile due to missing index: `{name}` `{version}` from `{index}`"
);
Ok(Self::Preferable(lock))
}
SatisfiesResult::MissingMetadata(name, version) => {
debug!(
"Ignoring existing lockfile due to missing metadata for: `{name}=={version}`"
);
Ok(Self::Preferable(lock))
}
SatisfiesResult::MismatchedRequiresDist(name, version, expected, actual) => {
debug!(
"Ignoring existing lockfile due to mismatched `requires-dist` for: `{name}=={version}`\n Expected: {:?}\n Actual: {:?}",
expected, actual
);
Ok(Self::Preferable(lock))
}
SatisfiesResult::MismatchedDevDependencies(name, version, expected, actual) => {
debug!(
"Ignoring existing lockfile due to mismatched dev dependencies for: `{name}=={version}`\n Expected: {:?}\n Actual: {:?}",
expected, actual
);
Ok(Self::Preferable(lock))
}
}
}
/// Return the inner [`Lock`].
@ -699,7 +723,7 @@ impl ValidatedLock {
}
/// Write the lockfile to disk.
pub(crate) async fn commit(lock: &Lock, workspace: &Workspace) -> Result<(), ProjectError> {
async fn commit(lock: &Lock, workspace: &Workspace) -> Result<(), ProjectError> {
let encoded = lock.to_toml()?;
fs_err::tokio::write(workspace.install_path().join("uv.lock"), encoded).await?;
Ok(())
@ -708,7 +732,7 @@ pub(crate) async fn commit(lock: &Lock, workspace: &Workspace) -> Result<(), Pro
/// Read the lockfile from the workspace.
///
/// Returns `Ok(None)` if the lockfile does not exist.
pub(crate) async fn read(workspace: &Workspace) -> Result<Option<Lock>, ProjectError> {
async fn read(workspace: &Workspace) -> Result<Option<Lock>, ProjectError> {
match fs_err::tokio::read_to_string(&workspace.install_path().join("uv.lock")).await {
Ok(encoded) => match toml::from_str(&encoded) {
Ok(lock) => Ok(Some(lock)),

View file

@ -8203,7 +8203,6 @@ fn lock_change_index() -> Result<()> {
----- stderr -----
warning: `uv lock` is experimental and may change without warning
Ignoring existing lockfile due to removal of referenced registry: https://pypi-proxy.fly.dev/basic-auth/simple
Resolved 2 packages in [TIME]
"###);