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

uv lock to use overrides from tool.uv (#4108) #4369

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

idlsoft
Copy link
Contributor

@idlsoft idlsoft commented Jun 17, 2024

Summary

This will make uv lock read override-dependencies from the [tool.uv] section of pyproject.toml.
Resolves #4108

This other implementation touches more code but seems more consistent.

Test Plan

Unit test

@idlsoft idlsoft force-pushed the lock_with_overrides branch 8 times, most recently from 6fe8f49 to 1505dce Compare June 17, 2024 21:15
@idlsoft idlsoft marked this pull request as ready for review June 17, 2024 21:23
Copy link
Member

@BurntSushi BurntSushi left a comment

Choose a reason for hiding this comment

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

I think this LGTM, but I think @konstin should sign off on this before merging.

}
}
});
let overrides = overrides;
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this can be removed?

Wanted to make it explicit that it's now immutable

Copy link
Member

Choose a reason for hiding this comment

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

We don't usually do this in our code. It would add a lot of noise.

Copy link
Member

@konstin konstin Jun 19, 2024

Choose a reason for hiding this comment

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

fyi i removed it and also changed the iteration style to align more with what we do elsewhere

@BurntSushi BurntSushi requested a review from konstin June 19, 2024 13:29
@konstin
Copy link
Member

konstin commented Jun 19, 2024

Currently, this can have effects on neighbouring packages: Say you have a non-project workspace root and in it two package foo, bar and baz.

foo has:

tool.uv.override-dependencies = [
   "anyio==4.3.0"
]

bar has:

dependencies = [
    "anyio==4.4.0"
]

baz has:

tool.uv.override-dependencies = [
   "lib_that_uses_anyio=1.2.3"
]

This means foo changes the resolution of both bar and baz, once directly and once indirectly. Since overrides apply globally, i think we should only allow them in the workspace root.

@konstin konstin added the needs-design Needs discussion, investigation, or design label Jun 19, 2024
@idlsoft
Copy link
Contributor Author

idlsoft commented Jun 19, 2024

This means foo changes the resolution of both bar and baz, once directly and once indirectly. Since overrides apply globally, i think we should only allow them in the workspace root.

I assume you mean foo, bar and baz don't depend on each other.
You're right, but it's also not entirely new. The fact that one virtualenv is shared by all workspace members is bound to have side-effects.

As convinient as a shared virtualenv is, I kinda wish there was a way to require a private one per member.
As it stands right now dependency versions are negotiated by unrelated workspace members.
Also you can have a line in bar that references lib_that_uses_anyio or foo, and it'll work fine in the virtualenv, but not when bar is shipped as a standalone library.

On an unrelated note, if someone could also take a look at astral-sh/rye#1015 I'd appreciate it. I'm a bit stuck with one of my projects.

@idlsoft
Copy link
Contributor Author

idlsoft commented Jun 21, 2024

I've tried a different implementation, I think it's more in line with existing code.

@idlsoft
Copy link
Contributor Author

idlsoft commented Jun 24, 2024

Updated, it now gets overrides from the workspace only.

@charliermarsh
Copy link
Member

I think this looks correct.

@charliermarsh charliermarsh merged commit 03cfdc2 into astral-sh:main Jun 24, 2024
47 checks passed
charliermarsh pushed a commit that referenced this pull request Jul 21, 2024
Resolves #4467.

## Summary

This PR implements the following

1. Add `tool.uv.constraint-dependencies` to pyproject.toml
1. Support to refer `tool.uv.constraint-dependencies` in `uv lock`
1. Support to refer `tool.uv.constraint-dependencies` in `uv pip
compile/install`

These are analogues of the override features implemented in #3839 and
#4369.

## Test Plan

Add test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-design Needs discussion, investigation, or design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pyproject defined override dependencies ignored by uv lock and uv sync
4 participants