Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply --no-install options when constructing resolution #7277

Merged
merged 2 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion crates/uv-configuration/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
96 changes: 32 additions & 64 deletions crates/uv-configuration/src/install_options.rs
Original file line number Diff line number Diff line change
@@ -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>,
}

Expand All @@ -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;
}

Expand Down
27 changes: 17 additions & 10 deletions crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/lock/requirements_txt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
14 changes: 9 additions & 5 deletions crates/uv/src/commands/project/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
55 changes: 55 additions & 0 deletions crates/uv/tests/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()> {
Expand Down
Loading