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

Support uv --override option (#668) #1015

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

idlsoft
Copy link

@idlsoft idlsoft commented Apr 13, 2024

This change will read an override-dependencies array from [tool.rye] and propagate it as an --override option to uv

Example used in test_overrides:

[project]
...
dependencies = ["flask==3.0.0"]

[tool.rye]
managed = true
override-dependencies = [ "werkzeug==2.3.8" ]

flask 3.0.0 requires werkzeug>=3.0.0 but the override will enforce 2.3.8.

@bluss
Copy link
Contributor

bluss commented Apr 20, 2024

I'm not a fan of expanding scope, so don't do that, but rye add can add deps, dev-deps, optional deps and excluded deps. Extending the pattern it seems like it would fit that rye add can add overrides as well, which could be a later PR.

rye/src/lock.rs Outdated Show resolved Hide resolved
rye/src/lock.rs Outdated Show resolved Hide resolved
@idlsoft idlsoft force-pushed the uv_overrides branch 2 times, most recently from a019cd7 to a8937dc Compare April 20, 2024 13:23
@idlsoft
Copy link
Author

idlsoft commented Apr 20, 2024

I'm not a fan of expanding scope, so don't do that, but rye add can add deps, dev-deps, optional deps and excluded deps. Extending the pattern it seems like it would fit that rye add can add overrides as well, which could be a later PR.

Done

@idlsoft idlsoft force-pushed the uv_overrides branch 10 times, most recently from 6830807 to 501ed8f Compare April 26, 2024 13:25
@idlsoft idlsoft force-pushed the uv_overrides branch 2 times, most recently from e9d17a9 to 639bffe Compare May 6, 2024 14:20
@idlsoft idlsoft requested a review from bluss May 10, 2024 19:34
Copy link
Contributor

@bluss bluss left a comment

Choose a reason for hiding this comment

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

lgtm. I can review, but note I'm just a contributor and can't do more in this repo.

&requirements_file,
lock_options.pre,
env::var("__RYE_UV_EXCLUDE_NEWER").ok(),
upgrade,
keyring_provider,
)?;
} else {
if overrides_file_in.is_some() {
bail!("dependency overrides require the uv backend");
Copy link
Contributor

Choose a reason for hiding this comment

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

can this maybe be a warning? Don't know

Copy link
Author

Choose a reason for hiding this comment

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

it would ignore an explicit instruction to overwrite a dependency, I feel like it should be treated as an error.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a question of design, I don't know, on one hand the project is a pyproject.toml file which can be interpreted by standard python tools, not just Rye. And those tools would ignore everything inside tool.rye anyway. Maybe error when rye is managing it is good yeah.

@idlsoft
Copy link
Author

idlsoft commented May 10, 2024

lgtm. I can review, but note I'm just a contributor and can't do more in this repo.

I don't think I can ask for reviewers. Your name was there because you've commented earlier.
Thx :)

@idlsoft idlsoft force-pushed the uv_overrides branch 2 times, most recently from c67478c to 5585eb6 Compare May 22, 2024 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants