-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 pip-compatible --group
flag to uv pip install
and uv pip compile
#11686
base: main
Are you sure you want to change the base?
Conversation
// Deduplicate in a stable way to get deterministic behaviour | ||
let deduped_paths = groups | ||
.iter() | ||
.map(|group| &group.path) | ||
.collect::<BTreeSet<_>>(); | ||
group_requirements = deduped_paths | ||
.into_iter() | ||
.map(|path| RequirementsSource::PyprojectToml(path.to_owned())) | ||
.collect::<Vec<_>>(); | ||
requirements = &group_requirements[..]; |
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 started doing a cleanup pass to compute a BTreeMap<PathBuf, Vec<GroupName>>
that would be used by subsequent steps but it was just more code and value threading for no real benefit over the current impl that just does a filter(group.path == path)
later.
FYI, pip support was merged. |
crates/uv-cli/src/lib.rs
Outdated
/// Ignore the package and it's dependencies, only install from the specified dependency group. | ||
/// | ||
/// May be provided multiple times. | ||
#[arg(long, group = "sources", conflicts_with_all = ["requirements", "package", "editable"])] |
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.
Are these conflicts present in the pip
implementation?
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 doesn't really look like it?)
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 thought -r
was exclusive to our CLI (I didn't check the others though).
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.
Yeah -r
is exclusive to us, but I'd expect pip install -e . --group foo
and pip install anyio --group foo
to work unless they explicitly choose for it not to upstream.
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 -r
should have the same semantics as these other options, I'm not sure)
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.
One problem is that uv pip install -r pyproject.toml --extra foo
is the "correct" use of --extra
but uv pip install -r pyproject.toml --group foo
would be invalid? Even more confusing, if we allow it, uv pip install -r foo/pyproject.toml --group bar
would install bar
from ./pyproject.toml
instead of foo/pyproject.toml
.
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.
Yeah that's part of why I opted to disallow it. The upstream pip semantics are really confusing to pair with anything else.
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 it's important that we're compatible with pip on whether --group
can be used with <package>
and --editable
though, which introduces all the same complexities.
I implemented/pushed the following changes, because I needed to wrap my head around them in code/tests to think them through semantically:
|
Zanie convinced me that pip-compile is fine actually, so I just loosened the CLI restriction to make it accepted. I'm just adding more tests now. |
6e578d9
to
9376d4f
Compare
--group
flag to uv pip install
--group
flag to uv pip install
and uv pip compile
Can you share the current state of this? It looks like you updated the PR summary to cover the behavior we discussed? |
9376d4f
to
4d3b843
Compare
I believe the PR is in the final state I'd like. Possibly it could have Even More tests but I'm not sure it would add much. (Just rebased) |
4d3b843
to
1cc738b
Compare
1cc738b
to
0dfe672
Compare
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.
This looks pretty good. I have mostly minor comments, though the --project
bit might be blocking?
We should add this to the documentation at
We may want to mark --group
on uv pip compile
as "in preview" until support lands in pip-tools
. See jazzband/pip-tools#2062. It looks like they're just considering using ::
instead of :
as a separator? Moving this to preview would involve a runtime warning and a note in the doc.
requirement_sources.push(source); | ||
} | ||
|
||
// pip `--group` flags specify their own sources, which we need to process here |
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 pretty sure this makes sense, but it might be relevant for Charlie to take a look at before merging.
Ooookay I think that's all review feedback addressed! |
```console | ||
$ uv pip compile --group some/path/pyproject.toml:foo | ||
``` | ||
|
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 should also mention --project
here, especially for multiple groups.
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 may want to add a !!! note admonition noting that this is not yet implemented in pip-tools
yet and link to the issue?
To lock dependency groups in the current project directory's pyproject.toml: | ||
|
||
```console | ||
$ uv pip compile --group foo | ||
``` |
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.
"To lock a dependency group... For example, to lock the group foo
"?
We also use backticks for pyproject.toml
.
@@ -107,6 +107,18 @@ Install from a `pyproject.toml` file with all optional dependencies enabled: | |||
$ uv pip install -r pyproject.toml --all-extras | |||
``` | |||
|
|||
To install dependency groups in the current project directory's pyproject.toml: |
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.
My above comments for pip compile
apply here too.
To install dependency groups for an arbitrary pyproject.toml: | ||
|
||
```console | ||
$ uv pip install --group some/path/pyproject.toml:foo | ||
``` |
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 need to talk about how --group
does not apply to other sources, like -e .
etc. here. Maybe under an !!! note
admonition? We could track this separately, but this is an important thing to teach in the docs.
----- stdout ----- | ||
|
||
----- stderr ----- | ||
error: invalid value './:foo' for '--group <GROUP>': The --group path is required to end in pyproject.toml for compatibility with pip; got: ./ |
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.
Should we quote 'pyproject.toml' for consistency with the rest?
----- stdout ----- | ||
|
||
----- stderr ----- | ||
error: invalid value 'subdir/:foo' for '--group <GROUP>': The --group path is required to end in pyproject.toml for compatibility with pip; got: subdir/ |
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.
--group should be wrapped in backticks
----- stdout ----- | ||
|
||
----- stderr ----- | ||
error: The dependency group 'bar' was not found in pyproject.toml |
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.
Probably want "in the pyproject.toml
"
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 this is ready. It's hard to keep track of everything with the scope of the changes.
I have a few stylistic nits remaining and want to see more user-facing documentation explaining how --group
interacts with other options and how it differs from the rest of the uv interface. We can push that into a separate pull request, but it's very important. I'm happy to help with that if you need it.
This is a minimal redux of #10861 to be compatible with
uv pip
.This implements the interface described in: pypa/pip#13065 (comment) for
uv pip install
anduv pip compile
. Namely--group <[path:]name>
, wherepath
when not defined defaults topyproject.toml
.In that interface they add
--group
topip install
,pip download
, andpip wheel
. Notably we do not defineuv pip download
anduv pip wheel
, so for parity we only need to implementuv pip install
. However, we also supportuv pip compile
which is not part of pip itself, and--group
makes sense there too.The behaviour of
--group
foruv pip
commands makes sense for the cases upstream pip supports, but has confusing meanings in cases that only we support (because reading pyproject.tomls is New Tech to them but heavily supported by us). Specifically case (h) below is a concerning footgun, and case (e) below may get complaints from people who aren't well-versed in dependency-groups-as-they-pertain-to-wheels.Only Group Flags
Group flags on their own work reasonably and uncontroversially, except perhaps that they don't do very clever automatic project discovery.
a)
uv pip install --group path/to/pyproject.toml:mygroup
pulls uppath/to/project.toml
and installs all the packages listed by itsmygroup
dependency-group (essentially treating it like another kind of requirements.txt). In this regard it functions similarly to--only-group
in the rest of uv's interface.b)
uv pip install --group mygroup
is just sugar foruv pip install --group pyproject.toml:mygroup
(note that no project discovery occurs, upstream pip simply hardcodes the path "pyproject.toml" here and we reproduce that.)c)
uv pip install --group a/pyproject.toml:groupx --group b/pyproject.toml:groupy
, and any other instance of multiple--group
flags, can be understood as completely independent requests for the given groups at the given files.Groups With Named Packages
Groups being mixed with named packages also work in a fairly unsurprising way, especially if you understand that things like dependency-groups are not really supposed to exist on pypi, they're just for local development.
d)
uv pip install mypackage --group path/to/pyproject.toml:mygroup
much like multiple instances of--group
the two requests here are essentially completely independent: pleases installmypackage
, and please also installpath/to/pyproject.toml:mygroup
.e)
uv pip install mypackage --group mygroup
is exactly the same, but this is where it becomes possible for someone to be a little confused, as you might thinkmygroup
is supposed to refer tomypackage
in some way (it can't). But no, it's sourcingpyproject.toml:mygroup
from the current working directory.Groups With Requirements/Sourcetrees/Editables
Requirements and sourcetrees are where I expect users to get confused. It behaves exactly the same as it does in the previous sections but you would absolutely be forgiven for expecting a different behaviour. Especially because
--group
with the rest of uv does do something different.f)
uv pip install -r a/pyproject.toml --group b/pyproject.toml:mygroup
is again just two independent requests (installa/pyproject.toml
's dependencies, andb/pyproject.toml
'smygroup
).g)
uv pip install -r pyproject.toml --group mygroup
is exactly like the previous case but incidentally the two requests refer to the same file. What the user wanted to happen is almost certainly happening, but they are likely getting "lucky" here that they're requesting something simple.h)
uv pip install -r a/pyproject.toml --group mygroup
is again exactly the same but the user is likely to get surprised and upset as this invocation actually sources two different files (installa/pyproject.toml
's dependencies, andpyproject.toml
'smygroup
)! I would expect most people to assume the--group
flag here is covering all applicable requirements/sourcetrees/editables, but no, it continues to be a totally independent reference to a file with a hardcoded relative path.Fixes #8590
Fixes #8969