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

Do not require workspace members to sync with --frozen #6737

Merged
merged 1 commit into from
Aug 28, 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 @@ -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
Loading