Skip to content

Commit

Permalink
Do not require workspace members to sync with --frozen (#6737)
Browse files Browse the repository at this point in the history
## Summary

Closes #6685.
  • Loading branch information
charliermarsh committed Aug 28, 2024
1 parent 485e0d2 commit 53ef633
Show file tree
Hide file tree
Showing 9 changed files with 85 additions and 27 deletions.
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 @@ -20,7 +20,6 @@ pypi-types = { workspace = true }
uv-auth = { workspace = true }
uv-cache = { workspace = true }
uv-normalize = { workspace = true }
uv-workspace = { workspace = true }

clap = { workspace = true, features = ["derive"], optional = true }
either = { workspace = true }
Expand Down
22 changes: 10 additions & 12 deletions crates/uv-configuration/src/install_options.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
use std::collections::BTreeSet;

use rustc_hash::FxHashSet;
use tracing::debug;

use distribution_types::{Name, Resolution};
use pep508_rs::PackageName;
use uv_workspace::VirtualProject;

#[derive(Debug, Clone, Default)]
pub struct InstallOptions {
Expand All @@ -28,13 +29,14 @@ impl InstallOptions {
pub fn filter_resolution(
&self,
resolution: Resolution,
project: &VirtualProject,
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);
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, project);
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)
Expand All @@ -43,13 +45,13 @@ impl InstallOptions {
fn apply_no_install_project(
&self,
resolution: Resolution,
project: &VirtualProject,
project_name: Option<&PackageName>,
) -> Resolution {
if !self.no_install_project {
return resolution;
}

let Some(project_name) = project.project_name() else {
let Some(project_name) = project_name else {
debug!("Ignoring `--no-install-project` for virtual workspace");
return resolution;
};
Expand All @@ -60,17 +62,13 @@ impl InstallOptions {
fn apply_no_install_workspace(
&self,
resolution: Resolution,
project: &VirtualProject,
members: &BTreeSet<PackageName>,
) -> Resolution {
if !self.no_install_workspace {
return resolution;
}

let workspace_packages = project.workspace().packages();
resolution.filter(|dist| {
!workspace_packages.contains_key(dist.name())
&& Some(dist.name()) != project.project_name()
})
resolution.filter(|dist| !members.contains(dist.name()))
}

fn apply_no_install_package(&self, resolution: Resolution) -> Resolution {
Expand Down
5 changes: 5 additions & 0 deletions crates/uv-resolver/src/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,11 @@ impl Lock {
&self.supported_environments
}

/// Returns the workspace members that were used to generate this lock.
pub fn members(&self) -> &BTreeSet<PackageName> {
&self.manifest.members
}

/// If this lockfile was built from a forking resolution with non-identical forks, return the
/// markers of those forks, otherwise `None`.
pub fn fork_markers(&self) -> &[MarkerTree] {
Expand Down
4 changes: 2 additions & 2 deletions crates/uv-workspace/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pub use workspace::{
check_nested_workspaces, DiscoveryOptions, ProjectWorkspace, VirtualProject, Workspace,
WorkspaceError, WorkspaceMember,
check_nested_workspaces, DiscoveryOptions, MemberDiscovery, ProjectWorkspace, VirtualProject,
Workspace, WorkspaceError, WorkspaceMember,
};

pub mod pyproject;
Expand Down
22 changes: 19 additions & 3 deletions crates/uv-workspace/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,23 @@ pub enum WorkspaceError {
Normalize(#[source] std::io::Error),
}

#[derive(Debug, Default, Clone)]
pub enum MemberDiscovery<'a> {
/// Discover all workspace members.
#[default]
All,
/// Don't discover any workspace members.
None,
/// Discover workspace members, but ignore the given paths.
Ignore(FxHashSet<&'a Path>),
}

#[derive(Debug, Default, Clone)]
pub struct DiscoveryOptions<'a> {
/// The path to stop discovery at.
pub stop_discovery_at: Option<&'a Path>,
/// The set of member paths to ignore.
pub ignore: FxHashSet<&'a Path>,
/// The strategy to use when discovering workspace members.
pub members: MemberDiscovery<'a>,
}

/// A workspace, consisting of a root directory and members. See [`ProjectWorkspace`].
Expand Down Expand Up @@ -546,7 +557,12 @@ impl Workspace {
.clone();

// If the directory is explicitly ignored, skip it.
if options.ignore.contains(member_root.as_path()) {
let skip = match &options.members {
MemberDiscovery::All => false,
MemberDiscovery::None => true,
MemberDiscovery::Ignore(ignore) => ignore.contains(member_root.as_path()),
};
if skip {
debug!(
"Ignoring workspace member: `{}`",
member_root.simplified_display()
Expand Down
4 changes: 2 additions & 2 deletions crates/uv/src/commands/project/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use uv_python::{
};
use uv_resolver::RequiresPython;
use uv_workspace::pyproject_mut::{DependencyTarget, PyProjectTomlMut};
use uv_workspace::{DiscoveryOptions, Workspace, WorkspaceError};
use uv_workspace::{DiscoveryOptions, MemberDiscovery, Workspace, WorkspaceError};

use crate::commands::project::find_requires_python;
use crate::commands::reporters::PythonDownloadReporter;
Expand Down Expand Up @@ -141,7 +141,7 @@ async fn init_project(
match Workspace::discover(
parent,
&DiscoveryOptions {
ignore: std::iter::once(path).collect(),
members: MemberDiscovery::Ignore(std::iter::once(path).collect()),
..DiscoveryOptions::default()
},
)
Expand Down
17 changes: 13 additions & 4 deletions crates/uv/src/commands/project/sync.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use anyhow::{Context, Result};
use itertools::Itertools;

use distribution_types::{Dist, ResolvedDist, SourceDist};
use itertools::Itertools;
use pep508_rs::MarkerTree;
use uv_auth::store_credentials_from_url;
use uv_cache::Cache;
Expand All @@ -14,7 +13,7 @@ use uv_normalize::{PackageName, DEV_DEPENDENCIES};
use uv_python::{PythonDownloads, PythonEnvironment, PythonPreference, PythonRequest};
use uv_resolver::{FlatIndex, Lock};
use uv_types::{BuildIsolation, HashStrategy};
use uv_workspace::{DiscoveryOptions, VirtualProject, Workspace};
use uv_workspace::{DiscoveryOptions, MemberDiscovery, VirtualProject, Workspace};

use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger, InstallLogger};
use crate::commands::pip::operations::Modifications;
Expand Down Expand Up @@ -52,6 +51,15 @@ pub(crate) async fn sync(
.with_current_project(package.clone())
.with_context(|| format!("Package `{package}` not found in workspace"))?,
)
} else if frozen {
VirtualProject::discover(
&CWD,
&DiscoveryOptions {
members: MemberDiscovery::None,
..DiscoveryOptions::default()
},
)
.await?
} else {
VirtualProject::discover(&CWD, &DiscoveryOptions::default()).await?
};
Expand Down Expand Up @@ -201,7 +209,8 @@ pub(super) async fn do_sync(
let resolution = apply_no_virtual_project(resolution);

// Filter resolution based on install-specific options.
let resolution = install_options.filter_resolution(resolution, project);
let resolution =
install_options.filter_resolution(resolution, project.project_name(), lock.members());

// Add all authenticated sources to the cache.
for url in index_locations.urls() {
Expand Down
36 changes: 34 additions & 2 deletions crates/uv/tests/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1103,10 +1103,30 @@ fn no_install_workspace() -> Result<()> {
+ sniffio==1.3.1
"###);

// However, we do require the `pyproject.toml`.
// Remove the virtual environment.
fs_err::remove_dir_all(&context.venv)?;

// We don't require the `pyproject.toml` for non-root members, if `--frozen` is provided.
fs_err::remove_file(child.join("pyproject.toml"))?;

uv_snapshot!(context.filters(), context.sync().arg("--no-install-workspace"), @r###"
uv_snapshot!(context.filters(), context.sync().arg("--no-install-workspace").arg("--frozen"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
Creating virtualenv at: .venv
Prepared 4 packages in [TIME]
Installed 4 packages in [TIME]
+ anyio==3.7.0
+ idna==3.6
+ iniconfig==2.0.0
+ sniffio==1.3.1
"###);

// Unless `--package` is used.
uv_snapshot!(context.filters(), context.sync().arg("--package").arg("child").arg("--no-install-workspace").arg("--frozen"), @r###"
success: false
exit_code: 2
----- stdout -----
Expand All @@ -1115,6 +1135,18 @@ fn no_install_workspace() -> Result<()> {
error: Workspace member `[TEMP_DIR]/child` is missing a `pyproject.toml` (matches: `child`)
"###);

// But we do require the root `pyproject.toml`.
fs_err::remove_file(context.temp_dir.join("pyproject.toml"))?;

uv_snapshot!(context.filters(), context.sync().arg("--no-install-workspace").arg("--frozen"), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: No `pyproject.toml` found in current directory or any parent directory
"###);

Ok(())
}

Expand Down

0 comments on commit 53ef633

Please sign in to comment.