Skip to content

Commit

Permalink
Simplify
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Sep 2, 2024
1 parent d76a560 commit 4bc9b6a
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 61 deletions.
34 changes: 28 additions & 6 deletions crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ impl Lock {
/// Convert the [`Lock`] to a [`Resolution`] using the given marker environment, tags, and root.
pub fn to_resolution(
&self,
project: &InstallTarget,
project: InstallTarget<'_>,
marker_env: &ResolverMarkerEnvironment,
tags: &Tags,
extras: &ExtrasSpecification,
Expand All @@ -439,8 +439,12 @@ impl Lock {
for root_name in project.packages() {
let root = self
.find_by_name(root_name)
.expect("found too many packages matching root")
.expect("could not find root");
.map_err(|_| LockErrorKind::MultipleRootPackages {
name: root_name.clone(),
})?
.ok_or_else(|| LockErrorKind::MissingRootPackage {
name: root_name.clone(),
})?;

// Add the base package.
queue.push_back((root, None));
Expand All @@ -466,10 +470,15 @@ impl Lock {
for group in dev {
for dependency in project.group(group) {
if dependency.marker.evaluate(marker_env, &[]) {
let root_name = &dependency.name;
let root = self
.find_by_markers(&dependency.name, marker_env)
.expect("found too many packages matching root")
.expect("could not find root");
.find_by_markers(root_name, marker_env)
.map_err(|_| LockErrorKind::MultipleRootPackages {
name: root_name.clone(),
})?
.ok_or_else(|| LockErrorKind::MissingRootPackage {
name: root_name.clone(),
})?;

// Add the base package.
queue.push_back((root, None));
Expand Down Expand Up @@ -3605,6 +3614,19 @@ enum LockErrorKind {
/// An error that occurs when converting a URL to a path
#[error("failed to convert URL to path")]
UrlToPath,
/// An error that occurs when multiple packages with the same
/// name were found when identifying the root packages.
#[error("found multiple packages matching `{name}`")]
MultipleRootPackages {
/// The ID of the package.
name: PackageName,
},
/// An error that occurs when a root package can't be found.
#[error("could not find root package `{name}`")]
MissingRootPackage {
/// The ID of the package.
name: PackageName,
},
}

/// An error that occurs when a source string could not be parsed.
Expand Down
48 changes: 12 additions & 36 deletions crates/uv-workspace/src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -874,11 +874,6 @@ impl ProjectWorkspace {
&self.workspace
}

/// Convert the [`ProjectWorkspace`] into a [`Workspace`].
pub fn into_workspace(self) -> Workspace {
self.workspace
}

/// Returns the current project as a [`WorkspaceMember`].
pub fn current_project(&self) -> &WorkspaceMember {
&self.workspace().packages[&self.project_name]
Expand Down Expand Up @@ -1369,39 +1364,20 @@ impl VirtualProject {
}

/// A target that can be installed.
///
/// The project could be a package within a workspace, a real workspace root, a frozen member within
/// a workspace or a (legacy) non-project workspace root, which can define its own dev dependencies.
#[derive(Debug, Clone)]
pub enum InstallTarget {
#[derive(Debug, Clone, Copy)]
pub enum InstallTarget<'env> {
/// A project (which could be a workspace root or member).
Project(ProjectWorkspace),
Project(&'env ProjectWorkspace),
/// A (legacy) non-project workspace root.
NonProject(Workspace),
NonProject(&'env Workspace),
/// A frozen member within a [`Workspace`].
FrozenMember(Workspace, PackageName),
FrozenMember(&'env Workspace, &'env PackageName),
}

impl InstallTarget {
/// Find the current [`InstallTarget`], given the current directory.
pub async fn discover(
path: &Path,
options: &DiscoveryOptions<'_>,
) -> Result<Self, WorkspaceError> {
match VirtualProject::discover(path, options).await? {
VirtualProject::Project(project) => Ok(InstallTarget::Project(project)),
VirtualProject::NonProject(workspace) => Ok(InstallTarget::NonProject(workspace)),
}
}

/// Set the [`InstallTarget`] to a frozen member within the workspace.
#[must_use]
pub fn with_frozen_member(self, package_name: PackageName) -> Self {
match self {
Self::Project(project) => Self::FrozenMember(project.into_workspace(), package_name),
Self::NonProject(workspace) => Self::FrozenMember(workspace, package_name),
Self::FrozenMember(workspace, _) => Self::FrozenMember(workspace, package_name),
}
impl<'env> InstallTarget<'env> {
/// Create an [`InstallTarget`] for a frozen member within a workspace.
pub fn frozen_member(project: &'env VirtualProject, package_name: &'env PackageName) -> Self {
Self::FrozenMember(project.workspace(), package_name)
}

/// Return the [`Workspace`] of the target.
Expand All @@ -1418,7 +1394,7 @@ impl InstallTarget {
match self {
Self::Project(project) => Either::Left(std::iter::once(project.project_name())),
Self::NonProject(workspace) => Either::Right(workspace.packages().keys()),
Self::FrozenMember(_, package_name) => Either::Left(std::iter::once(package_name)),
Self::FrozenMember(_, package_name) => Either::Left(std::iter::once(*package_name)),
}
}

Expand Down Expand Up @@ -1468,8 +1444,8 @@ impl InstallTarget {
}
}

impl From<VirtualProject> for InstallTarget {
fn from(project: VirtualProject) -> Self {
impl<'env> From<&'env VirtualProject> for InstallTarget<'env> {
fn from(project: &'env VirtualProject) -> Self {
match project {
VirtualProject::Project(project) => Self::Project(project),
VirtualProject::NonProject(workspace) => Self::NonProject(workspace),
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/project/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ pub(crate) async fn add(
let install_options = InstallOptions::default();

if let Err(err) = project::sync::do_sync(
&InstallTarget::from(project.clone()),
InstallTarget::from(&project),
&venv,
&lock,
&extras,
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/project/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ pub(crate) async fn remove(
let state = SharedState::default();

project::sync::do_sync(
&InstallTarget::from(project.clone()),
InstallTarget::from(&project),
&venv,
&lock,
&extras,
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/project/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ pub(crate) async fn run(
let install_options = InstallOptions::default();

project::sync::do_sync(
&InstallTarget::from(project.clone()),
InstallTarget::from(&project),
&venv,
result.lock(),
&extras,
Expand Down
32 changes: 17 additions & 15 deletions crates/uv/src/commands/project/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,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, InstallTarget, MemberDiscovery, Workspace};
use uv_workspace::{DiscoveryOptions, InstallTarget, MemberDiscovery, VirtualProject, Workspace};

use crate::commands::pip::loggers::{DefaultInstallLogger, DefaultResolveLogger, InstallLogger};
use crate::commands::pip::operations::Modifications;
Expand Down Expand Up @@ -44,30 +44,32 @@ pub(crate) async fn sync(
cache: &Cache,
printer: Printer,
) -> Result<ExitStatus> {
// Identify the target.
let target = if frozen {
let project = InstallTarget::discover(
// Identify the project.
let project = if frozen {
VirtualProject::discover(
&CWD,
&DiscoveryOptions {
members: MemberDiscovery::None,
..DiscoveryOptions::default()
},
)
.await?;
if let Some(package) = package {
project.with_frozen_member(package)
} else {
project
}
} else if let Some(package) = package {
InstallTarget::Project(
.await?
} else if let Some(package) = package.as_ref() {
VirtualProject::Project(
Workspace::discover(&CWD, &DiscoveryOptions::default())
.await?
.with_current_project(package.clone())
.with_context(|| format!("Package `{package}` not found in workspace"))?,
)
} else {
InstallTarget::discover(&CWD, &DiscoveryOptions::default()).await?
VirtualProject::discover(&CWD, &DiscoveryOptions::default()).await?
};

// Identify the target.
let target = if let Some(package) = package.as_ref().filter(|_| frozen) {
InstallTarget::frozen_member(&project, package)
} else {
InstallTarget::from(&project)
};

// Discover or create the virtual environment.
Expand Down Expand Up @@ -114,7 +116,7 @@ pub(crate) async fn sync(

// Perform the sync operation.
do_sync(
&target,
target,
&venv,
&lock,
&extras,
Expand All @@ -138,7 +140,7 @@ pub(crate) async fn sync(
/// Sync a lockfile with an environment.
#[allow(clippy::fn_params_excessive_bools)]
pub(super) async fn do_sync(
target: &InstallTarget,
target: InstallTarget<'_>,
venv: &PythonEnvironment,
lock: &Lock,
extras: &ExtrasSpecification,
Expand Down
12 changes: 11 additions & 1 deletion crates/uv/tests/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1125,7 +1125,7 @@ fn no_install_workspace() -> Result<()> {
+ sniffio==1.3.1
"###);

// Unless `--package` is used.
// Even if `--package` is used.
uv_snapshot!(context.filters(), context.sync().arg("--package").arg("child").arg("--no-install-workspace").arg("--frozen"), @r###"
success: true
exit_code: 0
Expand All @@ -1138,6 +1138,16 @@ fn no_install_workspace() -> Result<()> {
- sniffio==1.3.1
"###);

// Unless the package doesn't exist.
uv_snapshot!(context.filters(), context.sync().arg("--package").arg("fake").arg("--no-install-workspace").arg("--frozen"), @r###"
success: false
exit_code: 2
----- stdout -----
----- stderr -----
error: could not find root package `fake`
"###);

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

Expand Down

0 comments on commit 4bc9b6a

Please sign in to comment.