Skip to content

Commit

Permalink
Add --group support to uv add and uv remove (#8108)
Browse files Browse the repository at this point in the history
Part of #8090

Adds the ability to add and remove dependencies from arbitrary groups
using `uv add` and `uv remove`. Does not include resolving with the new
dependencies — tackling that in #8110.

Additionally, this does not yet resolve interactions with the existing
`dev` group — we'll tackle that separately as well. I probably won't
merge the stack until that design is resolved.
  • Loading branch information
zanieb authored and charliermarsh committed Oct 23, 2024
1 parent a8038e5 commit 71c663c
Show file tree
Hide file tree
Showing 22 changed files with 653 additions and 124 deletions.
20 changes: 15 additions & 5 deletions crates/uv-cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use uv_configuration::{
ProjectBuildBackend, TargetTriple, TrustedHost, TrustedPublishing, VersionControlSystem,
};
use uv_distribution_types::{Index, IndexUrl, Origin, PipExtraIndex, PipFindLinks, PipIndex};
use uv_normalize::{ExtraName, PackageName};
use uv_normalize::{ExtraName, GroupName, PackageName};
use uv_pep508::Requirement;
use uv_pypi_types::VerbatimParsedUrl;
use uv_python::{PythonDownloads, PythonPreference, PythonVersion};
Expand Down Expand Up @@ -2946,7 +2946,7 @@ pub struct AddArgs {
pub requirements: Vec<PathBuf>,

/// Add the requirements as development dependencies.
#[arg(long, conflicts_with("optional"))]
#[arg(long, conflicts_with("optional"), conflicts_with("group"))]
pub dev: bool,

/// Add the requirements to the specified optional dependency group.
Expand All @@ -2956,9 +2956,15 @@ pub struct AddArgs {
///
/// To enable an optional dependency group for this requirement instead, see
/// `--extra`.
#[arg(long, conflicts_with("dev"))]
#[arg(long, conflicts_with("dev"), conflicts_with("group"))]
pub optional: Option<ExtraName>,

/// Add the requirements to the specified local dependency group.
///
/// These requirements will not be included in the published metadata for the project.
#[arg(long, conflicts_with("dev"), conflicts_with("optional"))]
pub group: Option<GroupName>,

/// Add the requirements as editable.
#[arg(long, overrides_with = "no_editable")]
pub editable: bool,
Expand Down Expand Up @@ -3064,13 +3070,17 @@ pub struct RemoveArgs {
pub packages: Vec<PackageName>,

/// Remove the packages from the development dependencies.
#[arg(long, conflicts_with("optional"))]
#[arg(long, conflicts_with("optional"), conflicts_with("group"))]
pub dev: bool,

/// Remove the packages from the specified optional dependency group.
#[arg(long, conflicts_with("dev"))]
#[arg(long, conflicts_with("dev"), conflicts_with("group"))]
pub optional: Option<ExtraName>,

/// Remove the packages from the specified local dependency group.
#[arg(long, conflicts_with("dev"), conflicts_with("optional"))]
pub group: Option<GroupName>,

/// Avoid syncing the virtual environment after re-locking the project.
#[arg(long, env = EnvVars::UV_NO_SYNC, value_parser = clap::builder::BoolishValueParser::new(), conflicts_with = "frozen")]
pub no_sync: bool,
Expand Down
24 changes: 17 additions & 7 deletions crates/uv-configuration/src/dev.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use either::Either;
use uv_normalize::GroupName;
use uv_normalize::{GroupName, DEV_DEPENDENCIES};

#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
pub enum DevMode {
Expand Down Expand Up @@ -27,17 +27,17 @@ impl DevMode {
}
}

#[derive(Debug, Copy, Clone)]
pub enum DevSpecification<'group> {
#[derive(Debug, Clone)]
pub enum DevSpecification {
/// Include dev dependencies from the specified group.
Include(&'group [GroupName]),
Include(Vec<GroupName>),
/// Do not include dev dependencies.
Exclude,
/// Include dev dependencies from the specified group, and exclude all non-dev dependencies.
Only(&'group [GroupName]),
/// Include dev dependencies from the specified groups, and exclude all non-dev dependencies.
Only(Vec<GroupName>),
}

impl<'group> DevSpecification<'group> {
impl DevSpecification {
/// Returns an [`Iterator`] over the group names to include.
pub fn iter(&self) -> impl Iterator<Item = &GroupName> {
match self {
Expand All @@ -51,3 +51,13 @@ impl<'group> DevSpecification<'group> {
matches!(self, Self::Exclude | Self::Include(_))
}
}

impl From<DevMode> for DevSpecification {
fn from(mode: DevMode) -> Self {
match mode {
DevMode::Include => Self::Include(vec![DEV_DEPENDENCIES.clone()]),
DevMode::Exclude => Self::Exclude,
DevMode::Only => Self::Only(vec![DEV_DEPENDENCIES.clone()]),
}
}
}
17 changes: 12 additions & 5 deletions crates/uv-distribution/src/metadata/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use uv_configuration::{LowerBound, SourceStrategy};
use uv_distribution_types::IndexLocations;
use uv_normalize::{ExtraName, GroupName, PackageName};
use uv_pep440::{Version, VersionSpecifiers};
use uv_pypi_types::{HashDigest, ResolutionMetadata};
use uv_pep508::Pep508Error;
use uv_pypi_types::{HashDigest, ResolutionMetadata, VerbatimParsedUrl};
use uv_workspace::WorkspaceError;

pub use crate::metadata::lowering::LoweredRequirement;
Expand All @@ -21,10 +22,16 @@ mod requires_dist;
pub enum MetadataError {
#[error(transparent)]
Workspace(#[from] WorkspaceError),
#[error("Failed to parse entry for: `{0}`")]
LoweringError(PackageName, #[source] LoweringError),
#[error(transparent)]
Lower(#[from] LoweringError),
#[error("Failed to parse entry: `{0}`")]
LoweringError(PackageName, #[source] Box<LoweringError>),
#[error("Failed to parse entry in `{0}`: `{1}`")]
GroupLoweringError(GroupName, PackageName, #[source] Box<LoweringError>),
#[error("Failed to parse entry in `{0}`: `{1}`")]
GroupParseError(
GroupName,
String,
#[source] Box<Pep508Error<VerbatimParsedUrl>>,
),
}

#[derive(Debug, Clone)]
Expand Down
146 changes: 108 additions & 38 deletions crates/uv-distribution/src/metadata/requires_dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,13 @@ use crate::Metadata;

use std::collections::BTreeMap;
use std::path::Path;
use std::str::FromStr;

use uv_configuration::{LowerBound, SourceStrategy};
use uv_distribution_types::IndexLocations;
use uv_normalize::{ExtraName, GroupName, PackageName, DEV_DEPENDENCIES};
use uv_workspace::pyproject::ToolUvSources;
use uv_pypi_types::VerbatimParsedUrl;
use uv_workspace::pyproject::{Sources, ToolUvSources};
use uv_workspace::{DiscoveryOptions, ProjectWorkspace};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -97,48 +100,71 @@ impl RequiresDist {
};

let dev_dependencies = {
// First, collect `tool.uv.dev_dependencies`
let dev_dependencies = project_workspace
.current_project()
.pyproject_toml()
.tool
.as_ref()
.and_then(|tool| tool.uv.as_ref())
.and_then(|uv| uv.dev_dependencies.as_ref())
.into_iter()
.and_then(|uv| uv.dev_dependencies.as_ref());

// Then, collect `dependency-groups`
let dependency_groups = project_workspace
.current_project()
.pyproject_toml()
.dependency_groups
.iter()
.flatten()
.cloned();
let dev_dependencies = match source_strategy {
SourceStrategy::Enabled => dev_dependencies
.flat_map(|requirement| {
let requirement_name = requirement.name.clone();
LoweredRequirement::from_requirement(
requirement,
&metadata.name,
project_workspace.project_root(),
.map(|(name, requirements)| {
(
name.clone(),
requirements
.iter()
.map(|requirement| {
match uv_pep508::Requirement::<VerbatimParsedUrl>::from_str(
requirement,
) {
Ok(requirement) => Ok(requirement),
Err(err) => Err(MetadataError::GroupParseError(
name.clone(),
requirement.clone(),
Box::new(err),
)),
}
})
.collect::<Result<Vec<_>, _>>(),
)
})
.chain(
// Only add the `dev` group if `dev-dependencies` is defined
dev_dependencies
.into_iter()
.map(|requirements| (DEV_DEPENDENCIES.clone(), Ok(requirements.clone()))),
)
.map(|(name, requirements)| {
// Apply sources to the requirements
match requirements {
Ok(requirements) => match apply_source_strategy(
source_strategy,
requirements,
&metadata,
project_sources,
project_indexes,
locations,
project_workspace.workspace(),
project_workspace,
lower_bound,
)
.map(move |requirement| match requirement {
Ok(requirement) => Ok(requirement.into_inner()),
Err(err) => {
Err(MetadataError::LoweringError(requirement_name.clone(), err))
}
})
})
.collect::<Result<Vec<_>, _>>()?,
SourceStrategy::Disabled => dev_dependencies
.into_iter()
.map(uv_pypi_types::Requirement::from)
.collect(),
};
if dev_dependencies.is_empty() {
BTreeMap::default()
} else {
BTreeMap::from([(DEV_DEPENDENCIES.clone(), dev_dependencies)])
}
&name,
) {
Ok(requirements) => Ok((name, requirements)),
Err(err) => Err(err),
},
Err(err) => Err(err),
}
})
.collect::<Result<Vec<_>, _>>()?;

dependency_groups.into_iter().collect::<BTreeMap<_, _>>()
};

let requires_dist = metadata.requires_dist.into_iter();
Expand All @@ -158,9 +184,10 @@ impl RequiresDist {
)
.map(move |requirement| match requirement {
Ok(requirement) => Ok(requirement.into_inner()),
Err(err) => {
Err(MetadataError::LoweringError(requirement_name.clone(), err))
}
Err(err) => Err(MetadataError::LoweringError(
requirement_name.clone(),
Box::new(err),
)),
})
})
.collect::<Result<Vec<_>, _>>()?,
Expand Down Expand Up @@ -190,6 +217,49 @@ impl From<Metadata> for RequiresDist {
}
}

fn apply_source_strategy(
source_strategy: SourceStrategy,
requirements: Vec<uv_pep508::Requirement<VerbatimParsedUrl>>,
metadata: &uv_pypi_types::RequiresDist,
project_sources: &BTreeMap<PackageName, Sources>,
project_indexes: &[uv_distribution_types::Index],
locations: &IndexLocations,
project_workspace: &ProjectWorkspace,
lower_bound: LowerBound,
group_name: &GroupName,
) -> Result<Vec<uv_pypi_types::Requirement>, MetadataError> {
match source_strategy {
SourceStrategy::Enabled => requirements
.into_iter()
.flat_map(|requirement| {
let requirement_name = requirement.name.clone();
LoweredRequirement::from_requirement(
requirement,
&metadata.name,
project_workspace.project_root(),
project_sources,
project_indexes,
locations,
project_workspace.workspace(),
lower_bound,
)
.map(move |requirement| match requirement {
Ok(requirement) => Ok(requirement.into_inner()),
Err(err) => Err(MetadataError::GroupLoweringError(
group_name.clone(),
requirement_name.clone(),
Box::new(err),
)),
})
})
.collect::<Result<Vec<_>, _>>(),
SourceStrategy::Disabled => Ok(requirements
.into_iter()
.map(uv_pypi_types::Requirement::from)
.collect()),
}
}

#[cfg(test)]
mod test {
use std::path::Path;
Expand Down Expand Up @@ -255,7 +325,7 @@ mod test {
"#};

assert_snapshot!(format_err(input).await, @r###"
error: Failed to parse entry for: `tqdm`
error: Failed to parse entry: `tqdm`
Caused by: Can't combine URLs from both `project.dependencies` and `tool.uv.sources`
"###);
}
Expand Down Expand Up @@ -422,7 +492,7 @@ mod test {
"#};

assert_snapshot!(format_err(input).await, @r###"
error: Failed to parse entry for: `tqdm`
error: Failed to parse entry: `tqdm`
Caused by: Can't combine URLs from both `project.dependencies` and `tool.uv.sources`
"###);
}
Expand All @@ -441,7 +511,7 @@ mod test {
"#};

assert_snapshot!(format_err(input).await, @r###"
error: Failed to parse entry for: `tqdm`
error: Failed to parse entry: `tqdm`
Caused by: Package is not included as workspace package in `tool.uv.workspace`
"###);
}
Expand Down
11 changes: 10 additions & 1 deletion crates/uv-normalize/src/group_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::fmt::{Display, Formatter};
use std::str::FromStr;
use std::sync::LazyLock;

use serde::{Deserialize, Deserializer};
use serde::{Deserialize, Deserializer, Serialize, Serializer};

use crate::{validate_and_normalize_owned, validate_and_normalize_ref, InvalidNameError};

Expand Down Expand Up @@ -41,6 +41,15 @@ impl<'de> Deserialize<'de> for GroupName {
}
}

impl Serialize for GroupName {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
self.0.serialize(serializer)
}
}

impl Display for GroupName {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
self.0.fmt(f)
Expand Down
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ impl Lock {
marker_env: &ResolverMarkerEnvironment,
tags: &Tags,
extras: &ExtrasSpecification,
dev: DevSpecification<'_>,
dev: &DevSpecification,
build_options: &BuildOptions,
install_options: &InstallOptions,
) -> Result<Resolution, LockError> {
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 @@ -43,7 +43,7 @@ impl<'lock> RequirementsTxtExport<'lock> {
lock: &'lock Lock,
root_name: &PackageName,
extras: &ExtrasSpecification,
dev: DevSpecification<'_>,
dev: &DevSpecification,
editable: EditableMode,
hashes: bool,
install_options: &'lock InstallOptions,
Expand Down
Loading

0 comments on commit 71c663c

Please sign in to comment.