Apply `--no-install` options when constructing resolution (#7277)

## Summary

We need to apply the `--no-install` filters earlier, such that we don't
error if we only have a source distribution for a given package when
`--no-build` is provided but that package is _omitted_.

Closes #7247.
This commit is contained in:
Charlie Marsh 2024-09-11 14:31:24 -04:00 committed by GitHub
parent 7021b15a42
commit 15792a3775
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 114 additions and 82 deletions

1
Cargo.lock generated
View File

@ -4700,7 +4700,6 @@ dependencies = [
"anyhow",
"cache-key",
"clap",
"distribution-types",
"either",
"pep508_rs",
"platform-tags",

View File

@ -14,7 +14,6 @@ workspace = true
[dependencies]
cache-key = { workspace = true }
distribution-types = { workspace = true }
pep508_rs = { workspace = true, features = ["schemars"] }
platform-tags = { workspace = true }
pypi-types = { workspace = true }

View File

@ -1,15 +1,16 @@
use std::collections::BTreeSet;
use rustc_hash::FxHashSet;
use tracing::debug;
use distribution_types::{Name, Resolution};
use pep508_rs::PackageName;
#[derive(Debug, Clone, Default)]
pub struct InstallOptions {
/// Omit the project itself from the resolution.
pub no_install_project: bool,
/// Omit all workspace members (including the project itself) from the resolution.
pub no_install_workspace: bool,
/// Omit the specified packages from the resolution.
pub no_install_package: Vec<PackageName>,
}
@ -26,81 +27,48 @@ impl InstallOptions {
}
}
pub fn filter_resolution(
&self,
resolution: Resolution,
project_name: Option<&PackageName>,
members: &BTreeSet<PackageName>,
) -> Resolution {
// If `--no-install-project` is set, remove the project itself.
let resolution = self.apply_no_install_project(resolution, project_name);
// If `--no-install-workspace` is set, remove the project and any workspace members.
let resolution = self.apply_no_install_workspace(resolution, members);
// If `--no-install-package` is provided, remove the requested packages.
self.apply_no_install_package(resolution)
}
fn apply_no_install_project(
&self,
resolution: Resolution,
project_name: Option<&PackageName>,
) -> Resolution {
if !self.no_install_project {
return resolution;
}
let Some(project_name) = project_name else {
debug!("Ignoring `--no-install-project` for virtual workspace");
return resolution;
};
resolution.filter(|dist| dist.name() != project_name)
}
fn apply_no_install_workspace(
&self,
resolution: Resolution,
members: &BTreeSet<PackageName>,
) -> Resolution {
if !self.no_install_workspace {
return resolution;
}
resolution.filter(|dist| !members.contains(dist.name()))
}
fn apply_no_install_package(&self, resolution: Resolution) -> Resolution {
if self.no_install_package.is_empty() {
return resolution;
}
let no_install_packages = self.no_install_package.iter().collect::<FxHashSet<_>>();
resolution.filter(|dist| !no_install_packages.contains(dist.name()))
}
/// Returns `true` if a package passes the install filters.
pub fn include_package(
&self,
package: &PackageName,
project_name: &PackageName,
project_name: Option<&PackageName>,
members: &BTreeSet<PackageName>,
) -> bool {
// If `--no-install-project` is set, remove the project itself. The project is always
// part of the workspace.
if (self.no_install_project || self.no_install_workspace) && package == project_name {
return false;
// If `--no-install-project` is set, remove the project itself.
if self.no_install_project {
if let Some(project_name) = project_name {
if package == project_name {
debug!("Omitting `{package}` from resolution due to `--no-install-project`");
return false;
}
}
}
// If `--no-install-workspace` is set, remove the project and any workspace members.
if self.no_install_workspace && members.contains(package) {
return false;
if self.no_install_workspace {
// In some cases, the project root might be omitted from the list of workspace members
// encoded in the lockfile. (But we already checked this above if `--no-install-project`
// is set.)
if !self.no_install_project {
if let Some(project_name) = project_name {
if package == project_name {
debug!(
"Omitting `{package}` from resolution due to `--no-install-workspace`"
);
return false;
}
}
}
if members.contains(package) {
debug!("Omitting `{package}` from resolution due to `--no-install-workspace`");
return false;
}
}
// If `--no-install-package` is provided, remove the requested packages.
if self.no_install_package.contains(package) {
debug!("Omitting `{package}` from resolution due to `--no-install-package`");
return false;
}

View File

@ -30,7 +30,7 @@ use pypi_types::{
redact_git_credentials, HashDigest, ParsedArchiveUrl, ParsedGitUrl, Requirement,
RequirementSource, ResolverMarkerEnvironment,
};
use uv_configuration::{BuildOptions, ExtrasSpecification};
use uv_configuration::{BuildOptions, ExtrasSpecification, InstallOptions};
use uv_distribution::DistributionDatabase;
use uv_fs::{relative_to, PortablePath, PortablePathBuf};
use uv_git::{GitReference, GitSha, RepositoryReference, ResolvedRepositoryReference};
@ -547,6 +547,7 @@ impl Lock {
extras: &ExtrasSpecification,
dev: &[GroupName],
build_options: &BuildOptions,
install_options: &InstallOptions,
) -> Result<Resolution, LockError> {
let mut queue: VecDeque<(&Package, Option<&ExtraName>)> = VecDeque::new();
let mut seen = FxHashSet::default();
@ -633,15 +634,21 @@ impl Lock {
}
}
}
map.insert(
dist.id.name.clone(),
ResolvedDist::Installable(dist.to_dist(
project.workspace().install_path(),
tags,
build_options,
)?),
);
hashes.insert(dist.id.name.clone(), dist.hashes());
if install_options.include_package(
&dist.id.name,
project.project_name(),
&self.manifest.members,
) {
map.insert(
dist.id.name.clone(),
ResolvedDist::Installable(dist.to_dist(
project.workspace().install_path(),
tags,
build_options,
)?),
);
hashes.insert(dist.id.name.clone(), dist.hashes());
}
}
let diagnostics = vec![];
Ok(Resolution::new(map, hashes, diagnostics))

View File

@ -134,7 +134,7 @@ impl<'lock> RequirementsTxtExport<'lock> {
let mut nodes: Vec<Node> = petgraph
.node_references()
.filter(|(_index, package)| {
install_options.include_package(&package.id.name, root_name, lock.members())
install_options.include_package(&package.id.name, Some(root_name), lock.members())
})
.map(|(index, package)| Node {
package,

View File

@ -219,15 +219,19 @@ pub(super) async fn do_sync(
let tags = venv.interpreter().tags()?;
// Read the lockfile.
let resolution = lock.to_resolution(target, &markers, tags, extras, &dev, build_options)?;
let resolution = lock.to_resolution(
target,
&markers,
tags,
extras,
&dev,
build_options,
&install_options,
)?;
// Always skip virtual projects, which shouldn't be built or installed.
let resolution = apply_no_virtual_project(resolution);
// Filter resolution based on install-specific options.
let resolution =
install_options.filter_resolution(resolution, target.project_name(), lock.members());
// Add all authenticated sources to the cache.
for url in index_locations.urls() {
store_credentials_from_url(url);

View File

@ -1222,6 +1222,61 @@ fn no_install_package() -> Result<()> {
Ok(())
}
/// Ensure that `--no-build` isn't enforced for projects that aren't installed in the first place.
#[test]
fn no_install_project_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"
version = "0.1.0"
requires-python = ">=3.12"
dependencies = ["anyio==3.7.0"]
[build-system]
requires = ["setuptools>=42"]
build-backend = "setuptools.build_meta"
"#,
)?;
// Generate a lockfile.
context.lock().assert().success();
// `--no-build` should raise an error, since we try to install the project.
uv_snapshot!(context.filters(), context.sync().arg("--no-build"), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
warning: Failed to validate existing lockfile: distribution project==0.1.0 @ editable+. can't be installed because it is marked as `--no-build` but has no binary distribution
Resolved 4 packages in [TIME]
error: distribution project==0.1.0 @ editable+. can't be installed because it is marked as `--no-build` but has no binary distribution
"###);
// But it's fine to combine `--no-install-project` with `--no-build`. We shouldn't error, since
// we aren't building the project.
uv_snapshot!(context.filters(), context.sync().arg("--no-install-project").arg("--no-build"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
warning: Failed to validate existing lockfile: distribution project==0.1.0 @ editable+. can't be installed because it is marked as `--no-build` but has no binary distribution
Resolved 4 packages in [TIME]
Prepared 3 packages in [TIME]
Installed 3 packages in [TIME]
+ anyio==3.7.0
+ idna==3.6
+ sniffio==1.3.1
"###);
Ok(())
}
/// Convert from a package to a virtual project.
#[test]
fn convert_to_virtual() -> Result<()> {