-
Notifications
You must be signed in to change notification settings - Fork 1k
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 dependency-group cli flags to uv pip install
and uv pip compile
(--group
, --no-group
, --only-group
, --all-groups
)
#10861
Conversation
No description provided. |
crates/uv-settings/src/settings.rs
Outdated
/// Include optional dependencies from the specified group; may be provided more than once. | ||
/// | ||
/// Only applies to `pyproject.toml`, `setup.py`, and `setup.cfg` sources. | ||
#[option( | ||
default = "[]", | ||
value_type = "list[str]", | ||
example = r#" | ||
group = ["dev", "docs"] | ||
"# | ||
)] | ||
pub group: Option<Vec<GroupName>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whether these were supposed to pass through the shared PipOptions and not a side-thing was an uncertainty for me. Overall I assumed they're equivalentish to the extra
settings and those are here, so, seems fine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems right to me, though the option abstractions are not my forte.
/// Returns `true` if the specification will have no effect. | ||
pub fn is_empty(&self) -> bool { | ||
let GroupsSpecification::Include { | ||
include: IncludeGroups::Some(includes), | ||
exclude, | ||
} = self | ||
else { | ||
return false; | ||
}; | ||
includes.is_empty() && exclude.is_empty() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure about the correctness of this. Is this relying on some assumptions about the construction of GroupsSpecification
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, but it's a relatively mundane assumption imo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth a comment saying what the assumption is, but don't feel strongly.
crates/uv-settings/src/settings.rs
Outdated
/// Include optional dependencies from the specified group; may be provided more than once. | ||
/// | ||
/// Only applies to `pyproject.toml`, `setup.py`, and `setup.cfg` sources. | ||
#[option( | ||
default = "[]", | ||
value_type = "list[str]", | ||
example = r#" | ||
group = ["dev", "docs"] | ||
"# | ||
)] | ||
pub group: Option<Vec<GroupName>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems right to me, though the option abstractions are not my forte.
crates/uv/tests/it/pip_install.rs
Outdated
@@ -8257,3 +8257,161 @@ fn cyclic_build_dependency() { | |||
"### | |||
); | |||
} | |||
|
|||
#[test] | |||
fn install_group() -> Result<()> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if multiple pyproject.toml
files are provided? What happens if a group is present in one but not the other? If a group is present in both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similarly, what about uv pip install <dir> --group <name>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe as currently implemented we:
- don't complain if we fail to find a group in the toml
- would independently match and include groups from every toml
This falls out of the implementation basically being "for each sourcefile, for each group in that sourcefile, if the selector built from cli flags matches the name of this group, append that group to the requirements".
not sure about the dir usecase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should fail (or at the very least, warn) if we cannot find a group.
I don't mind including groups from every toml, but we should have test coverage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We fail in uv sync
fwiw
❯ uv sync --group test
error: Group `test` is not defined in the project's `dependency-group` table
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added warnings.
docs/reference/settings.md
Outdated
#### [`all-groups`](#pip_all-groups) {: #pip_all-groups } | ||
<span id="all-groups"></span> | ||
|
||
Include all groups. | ||
|
||
Only applies to `pyproject.toml`, `setup.py`, and `setup.cfg` sources. | ||
|
||
**Default value**: `false` | ||
|
||
**Type**: `bool` | ||
|
||
**Example usage**: | ||
|
||
=== "pyproject.toml" | ||
|
||
```toml | ||
[tool.uv.pip] | ||
all-groups = true | ||
``` | ||
=== "uv.toml" | ||
|
||
```toml | ||
[pip] | ||
all-groups = true | ||
``` | ||
|
||
--- | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I'm not sure why we support requesting extras in settings file like this. It seems correct to match the extras options for groups though?
Perhaps @charliermarsh understands the use-case?
264867c
to
15e22dc
Compare
uv pip install
and uv pip compile
(--group
, --no-group
, --no-default-groups
, --only-group
, --all-groups
)uv pip install
and uv pip compile
(--group
, --no-group
, --only-group
, --all-groups
)
Thanks @Gankra |
Not that this is does not match the proposed API for |
Wow incredible timing. This won't get released in uv until we're happy with our interop story (will back this out if need be). |
This is intended to be compatible with the proposed |
Is there another way of compiling development groups dependencies into a requirements.txt if this is no longer planned? I don't see anything in the docs or issues so just wondering if it's possible. |
If you use |
And pypa/pip#13065 should be about ready to go in, it's already got an approval. You can also use |
Ultimately this is a lot of settings plumbing and a couple minor pieces of Actual Logic (which are so simple I have to assume there's something missing, but maybe not!).
Note this "needlessly" use DevDependencyGroup since it costs nothing, is more futureproof, and lets us maintain one primary interface (we just pass
false
for all the dev arguments).Fixes #8590
Fixes #8969