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.

Sign up for GitHub

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

Add --group support to uv add and uv remove #8108

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Add --group support to uv add and uv remove #8108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 15 additions & 5 deletions crates/uv-cli/src/lib.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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<_, _>>()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this correctly merge keys? Like if dev is included twice, does this concatenate the values? I sort of think it might replace them...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it probably replaces them, yeah. I wasn't a goal to resolve that here (per the description) since this implemented before I wrote a proposal. I figured I'd write some test cases and address it separately for review purposes.

};

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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was somewhat surprised that this is only used once. I guess the other "dependency types" (like production) don't match this exact pattern?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I actually moved it out because I thought I would need to apply it twice, like.. to the dev-dependencies group but I ended up structuring things differently. I think we can move it back in, or make it an impl method somewhere.

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 @@ -424,7 +494,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 @@ -443,7 +513,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
View file
Open in desktop
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,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
View file
Open in desktop
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
Loading