Skip to content

Commit

Permalink
Add support for reading include-group in dependency groups
Browse files Browse the repository at this point in the history
  • Loading branch information
zanieb committed Oct 16, 2024
1 parent e960e73 commit 4be3873
Show file tree
Hide file tree
Showing 6 changed files with 697 additions and 29 deletions.
2 changes: 2 additions & 0 deletions crates/uv-distribution/src/metadata/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ pub enum MetadataError {
String,
#[source] Box<Pep508Error<VerbatimParsedUrl>>,
),
#[error("Failed to find group `{0}` included by `{1}`")]
GroupNotFound(GroupName, GroupName),
}

#[derive(Debug, Clone)]
Expand Down
121 changes: 99 additions & 22 deletions crates/uv-distribution/src/metadata/requires_dist.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use crate::metadata::{LoweredRequirement, MetadataError};
use crate::Metadata;

use std::collections::BTreeMap;
use std::collections::{BTreeMap, BTreeSet};
use std::path::Path;
use std::str::FromStr;
use uv_configuration::SourceStrategy;
use uv_normalize::{ExtraName, GroupName, PackageName, DEV_DEPENDENCIES};
use uv_pypi_types::VerbatimParsedUrl;
use uv_workspace::pyproject::{Sources, ToolUvSources};
use uv_workspace::pyproject::{DependencyGroupSpecifier, Sources, ToolUvSources};
use uv_workspace::{DiscoveryOptions, ProjectWorkspace};

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -90,32 +90,23 @@ impl RequiresDist {
.dependency_groups
.iter()
.flatten()
.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<_>, _>>(),
)
})
.collect::<BTreeMap<_, _>>();

// Resolve inclusions
let dependency_groups = resolve_dependency_groups(&dependency_groups);

let dependency_groups = dependency_groups
.into_iter()
.chain(
// Only add the `dev` group if `dev-dependencies` is defined
dev_dependencies
.into_iter()
.map(|requirements| (DEV_DEPENDENCIES.clone(), Ok(requirements.clone()))),
)
.collect::<BTreeMap<_, _>>();

let dependency_groups = dependency_groups
.into_iter()
.map(|(name, requirements)| {
// Apply sources to the requirements
match requirements {
Expand Down Expand Up @@ -222,6 +213,92 @@ fn apply_source_strategy(
}
}

/// Resolve the dependency groups (which may contain references to other groups) into concrete
/// lists of requirements.
fn resolve_dependency_groups(
groups: &BTreeMap<&GroupName, &Vec<DependencyGroupSpecifier>>,
) -> BTreeMap<GroupName, Result<Vec<uv_pep508::Requirement<VerbatimParsedUrl>>, MetadataError>> {
fn resolve_group(
resolved: &mut BTreeMap<
GroupName,
Result<Vec<uv_pep508::Requirement<VerbatimParsedUrl>>, MetadataError>,
>,
groups: &BTreeMap<&GroupName, &Vec<DependencyGroupSpecifier>>,
name: &GroupName,
parents: &BTreeSet<GroupName>,
) -> Result<Vec<uv_pep508::Requirement<VerbatimParsedUrl>>, MetadataError> {
let Some(specifiers) = groups.get(name) else {
// Missing group
let parent_name = parents
.iter()
.last()
.expect("We should always have a parent when a group is missing");
return Err(MetadataError::GroupNotFound(
name.clone(),
parent_name.clone(),
));
};

if parents.contains(name) {
// A cycle was detected
// TODO(zanieb): We should probably add an error variant for this, we probably want to
// show the full `parents` list to demonstrate the cycle?
return Ok(Vec::new());
}

if let Some(result) = resolved.get(name) {
// Already resolved this group
// TODO(zanieb): Can we avoid cloning here? Will we want to clone anyway?
return Ok(result.as_ref().unwrap().clone());
}

let mut parents = parents.clone();
parents.insert(name.clone());

let mut requirements = Vec::new();
for specifier in *specifiers {
match specifier {
DependencyGroupSpecifier::Requirement(requirement) => {
match uv_pep508::Requirement::<VerbatimParsedUrl>::from_str(requirement) {
Ok(requirement) => requirements.push(requirement),
Err(err) => {
return Err(MetadataError::GroupParseError(
name.clone(),
requirement.clone(),
Box::new(err),
));
}
}
}
DependencyGroupSpecifier::Table {
include_group: Some(include_name),
} => {
let include_name = GroupName::from_str(include_name).unwrap();
let include_requirements =
resolve_group(resolved, groups, &include_name, &parents)?;
requirements.extend(include_requirements);
}
DependencyGroupSpecifier::Table {
include_group: None,
} => {}
}
}

resolved.insert(name.clone(), Ok(requirements.clone()));
Ok(requirements)
}

let mut resolved = BTreeMap::new();

for name in groups.keys() {
// TODO(zanieb): Raise errors? or defer them until a group is needed per the PEP's
// suggestion?
let _ = resolve_group(&mut resolved, groups, name, &BTreeSet::new());
}

resolved
}

#[cfg(test)]
mod test {
use std::path::Path;
Expand Down
16 changes: 15 additions & 1 deletion crates/uv-workspace/src/pyproject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub struct PyProjectToml {
/// Tool-specific metadata.
pub tool: Option<Tool>,
/// Non-project dependency groups, as defined in PEP 735.
pub dependency_groups: Option<BTreeMap<GroupName, Vec<String>>>,
pub dependency_groups: Option<BTreeMap<GroupName, Vec<DependencyGroupSpecifier>>>,
/// The raw unserialized document.
#[serde(skip)]
pub raw: String,
Expand Down Expand Up @@ -113,6 +113,20 @@ impl AsRef<[u8]> for PyProjectToml {
}
}

/// A specifier item in a PEP 735 Dependency Group
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
#[serde(rename_all = "kebab-case", untagged)]
pub enum DependencyGroupSpecifier {
/// PEP 508 requirement string
Requirement(String),
/// Include another dependency group
#[serde(rename_all = "kebab-case")]
Table {
/// The name of the group to include
include_group: Option<String>,
},
}

/// PEP 621 project metadata (`project`).
///
/// See <https://packaging.python.org/en/latest/specifications/pyproject-toml>.
Expand Down
30 changes: 24 additions & 6 deletions crates/uv-workspace/src/workspace/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use insta::assert_json_snapshot;

use uv_normalize::GroupName;

use crate::pyproject::PyProjectToml;
use crate::pyproject::{DependencyGroupSpecifier, PyProjectToml};
use crate::workspace::{DiscoveryOptions, ProjectWorkspace};

async fn workspace_test(folder: &str) -> (ProjectWorkspace, String) {
Expand Down Expand Up @@ -842,16 +842,34 @@ async fn exclude_package() -> Result<()> {
fn read_dependency_groups() {
let toml = r#"
[dependency-groups]
test = ["a"]
foo = ["a", {include-group = "bar"}]
bar = ["b"]
"#;

let result =
PyProjectToml::from_string(toml.to_string()).expect("Deserialization should succeed");

let groups = result
.dependency_groups
.expect("`dependency-groups` should be present");
let test = groups
.get(&GroupName::from_str("test").unwrap())
.expect("Group `test` should be present");
assert_eq!(test, &["a".to_string()]);
let foo = groups
.get(&GroupName::from_str("foo").unwrap())
.expect("Group `foo` should be present");
assert_eq!(
foo,
&[
DependencyGroupSpecifier::Requirement("a".to_string()),
DependencyGroupSpecifier::Table {
include_group: Some("bar".to_string())
}
]
);

let bar = groups
.get(&GroupName::from_str("bar").unwrap())
.expect("Group `bar` should be present");
assert_eq!(
bar,
&[DependencyGroupSpecifier::Requirement("b".to_string()),]
);
}
Loading

0 comments on commit 4be3873

Please sign in to comment.