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

Add support for reading PEP 735 dependency groups #8104

Merged
merged 1 commit into from
Oct 16, 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
4 changes: 4 additions & 0 deletions crates/uv-workspace/src/pyproject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ pub struct PyProjectToml {
pub project: Option<Project>,
/// Tool-specific metadata.
pub tool: Option<Tool>,
/// Non-project dependency groups, as defined in PEP 735.
pub dependency_groups: Option<BTreeMap<ExtraName, Vec<String>>>,
Copy link
Member

@konstin konstin Oct 15, 2024

Choose a reason for hiding this comment

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

I'd put it in a newtype so we can implement a get_group_resolved on top of it. PyO3/pyproject-toml-rs#24 is good reference implementation, we can reuse it.

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'll add support for the include-group syntax separately. This just copies our existing patterns.

I can hold of on merging though since it'd break people using the syntax.

Copy link
Member Author

@zanieb zanieb Oct 15, 2024

Choose a reason for hiding this comment

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

Honestly pretty hesitant to change the pattern here? We probably want to change all the structures separately if you want a different approach.

Copy link
Member

Choose a reason for hiding this comment

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

We're using both styles in uv, it's not that much of a difference in the end

/// The raw unserialized document.
#[serde(skip)]
pub raw: String,
Expand Down Expand Up @@ -1098,6 +1100,8 @@ pub enum DependencyType {
Dev,
/// A dependency in `project.optional-dependencies.{0}`.
Optional(ExtraName),
/// A dependency in `dependency-groups.{0}`.
Group(ExtraName),
}

/// <https://github.com/serde-rs/serde/issues/1316#issue-332908452>
Expand Down
18 changes: 17 additions & 1 deletion crates/uv-workspace/src/pyproject_mut.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,14 +578,30 @@ impl PyProjectTomlMut {
}
}

// Check `dependency-groups`.
if let Some(groups) = self.doc.get("dependency-groups").and_then(Item::as_table) {
for (group, dependencies) in groups {
let Some(dependencies) = dependencies.as_array() else {
continue;
};
let Ok(group) = ExtraName::new(group.to_string()) else {
continue;
};

if !find_dependencies(name, marker, dependencies).is_empty() {
types.push(DependencyType::Group(group));
}
}
}

// Check `tool.uv.dev-dependencies`.
if let Some(dev_dependencies) = self
.doc
.get("tool")
.and_then(Item::as_table)
.and_then(|tool| tool.get("uv"))
.and_then(Item::as_table)
.and_then(|tool| tool.get("dev-dependencies"))
.and_then(|uv| uv.get("dev-dependencies"))
.and_then(Item::as_array)
{
if !find_dependencies(name, marker, dev_dependencies).is_empty() {
Expand Down
50 changes: 40 additions & 10 deletions crates/uv-workspace/src/workspace/tests.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
use std::env;

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

use anyhow::Result;
use assert_fs::fixture::ChildPath;
use assert_fs::prelude::*;
use insta::assert_json_snapshot;

use uv_pep508::ExtraName;

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

async fn workspace_test(folder: &str) -> (ProjectWorkspace, String) {
Expand Down Expand Up @@ -76,7 +79,8 @@ async fn albatross_in_example() {
],
"optional-dependencies": null
},
"tool": null
"tool": null,
"dependency-groups": null
}
}
}
Expand Down Expand Up @@ -128,7 +132,8 @@ async fn albatross_project_in_excluded() {
],
"optional-dependencies": null
},
"tool": null
"tool": null,
"dependency-groups": null
}
}
}
Expand Down Expand Up @@ -237,7 +242,8 @@ async fn albatross_root_workspace() {
"override-dependencies": null,
"constraint-dependencies": null
}
}
},
"dependency-groups": null
}
}
}
Expand Down Expand Up @@ -326,7 +332,8 @@ async fn albatross_virtual_workspace() {
"override-dependencies": null,
"constraint-dependencies": null
}
}
},
"dependency-groups": null
}
}
}
Expand Down Expand Up @@ -377,7 +384,8 @@ async fn albatross_just_project() {
],
"optional-dependencies": null
},
"tool": null
"tool": null,
"dependency-groups": null
}
}
}
Expand Down Expand Up @@ -528,7 +536,8 @@ async fn exclude_package() -> Result<()> {
"override-dependencies": null,
"constraint-dependencies": null
}
}
},
"dependency-groups": null
}
}
}
Expand Down Expand Up @@ -629,7 +638,8 @@ async fn exclude_package() -> Result<()> {
"override-dependencies": null,
"constraint-dependencies": null
}
}
},
"dependency-groups": null
}
}
}
Expand Down Expand Up @@ -743,7 +753,8 @@ async fn exclude_package() -> Result<()> {
"override-dependencies": null,
"constraint-dependencies": null
}
}
},
"dependency-groups": null
}
}
}
Expand Down Expand Up @@ -831,7 +842,8 @@ async fn exclude_package() -> Result<()> {
"override-dependencies": null,
"constraint-dependencies": null
}
}
},
"dependency-groups": null
}
}
}
Expand All @@ -840,3 +852,21 @@ async fn exclude_package() -> Result<()> {

Ok(())
}

#[test]
fn read_dependency_groups() {
let toml = r#"
[dependency-groups]
test = ["a"]
"#;

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(&ExtraName::from_str("test").unwrap())
.expect("Group `test` should be present");
assert_eq!(test, &["a".to_string()]);
}
6 changes: 6 additions & 0 deletions crates/uv/src/commands/project/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ pub(crate) async fn add(
bail!("Project is missing a `[project]` table; add a `[project]` table to use optional dependencies, or run `{}` instead", "uv add --dev".green())
}
DependencyType::Dev => (),
DependencyType::Group(_) => (),
}
}

Expand Down Expand Up @@ -469,6 +470,7 @@ pub(crate) async fn add(
DependencyType::Optional(ref group) => {
toml.add_optional_dependency(group, &requirement, source.as_ref())?
}
DependencyType::Group(_) => todo!("adding dependencies to groups is not yet supported"),
};

// If the edit was inserted before the end of the list, update the existing edits.
Expand Down Expand Up @@ -742,6 +744,9 @@ async fn lock_and_sync(
DependencyType::Optional(ref group) => {
toml.set_optional_dependency_minimum_version(group, *index, minimum)?;
}
DependencyType::Group(_) => {
todo!("adding dependencies to groups is not yet supported")
}
}

modified = true;
Expand Down Expand Up @@ -813,6 +818,7 @@ async fn lock_and_sync(
let dev = DevMode::Exclude;
(extras, dev)
}
DependencyType::Group(_) => todo!("adding dependencies to groups is not yet supported"),
};

project::sync::do_sync(
Expand Down
6 changes: 6 additions & 0 deletions crates/uv/src/commands/project/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ pub(crate) async fn remove(
);
}
}
DependencyType::Group(_) => {
todo!("removing dependencies from groups is not yet supported")
}
}
}

Expand Down Expand Up @@ -249,6 +252,9 @@ fn warn_if_present(name: &PackageName, pyproject: &PyProjectTomlMut) {
"`{name}` is an optional dependency; try calling `uv remove --optional {group}`",
);
}
DependencyType::Group(_) => {
// TODO(zanieb): Once we support `remove --group`, add a warning here.
}
}
}
}
4 changes: 2 additions & 2 deletions crates/uv/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -843,8 +843,8 @@ impl AddSettings {
python,
} = args;

let dependency_type = if let Some(group) = optional {
DependencyType::Optional(group)
let dependency_type = if let Some(extra) = optional {
DependencyType::Optional(extra)
} else if dev {
DependencyType::Dev
} else {
Expand Down
Loading