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

Warn when missing minimal bounds when using tool.uv.sources #3452

Merged
merged 6 commits into from
May 8, 2024

Conversation

konstin
Copy link
Member

@konstin konstin commented May 8, 2024

When using tool.uv.sources, we warn that requirements have a bound, i.e. at least a lower version constraint.

When using a library, the symbols you import were introduced in different versions, creating an implicit lower bound. This warning makes this explicit. This is crucial to prevent backtracking resolvers from selecting an ancient versions that is not compatible (or worse, doesn't build), and a performance optimization on top.

This feature is gated to tool.uv.sources (as it should have been to begin with for #3263/#3443) to not unnecessarily break legacy workflows. It is also helpful specifically when using a tool.uv.sources section that contains constraints that are not published to pypi, e.g. for workspace dependencies. We can adjust those later to e.g. not constrain workspace dependencies with publish = false, but i think it's the right setting to start with.

@zanieb
Copy link
Member

zanieb commented May 8, 2024

I think I have a strong preference for this being a warning?

Maybe it'd be configurable in the future.. i.e. unconstrainted-requirements = error | warn | allow

A warning seems less likely to block adoption.

When using `tool.uv.sources`, we warn that requirements have a bound, i.e. at least a lower version constraint.

When using a library, the symbols you import were introduced in different versions, creating an implicit lower bound. This forces to make that bound explicit. This is crucial to prevent backtracking resolvers from selecting an ancient versions that is not compatible (or worse, doesn't build), and a performance optimization on top.

This feature is gated to `tool.uv.sources` (as it should have been for #3263/#3443) to not unnecessarily break legacy workflows. It is also helpful specifically when using a `tool.uv.sources` section that contains constraints that are not published to pypi, e.g. for workspace dependencies. We can adjust those later to e.g. not constrain workspace dependencies with `publish = false`, but i think it's the right setting to start with.
@konstin konstin changed the title Enforce minimal bounds when using tool.uv.sources Warn when missing minimal bounds when using tool.uv.sources May 8, 2024
@konstin konstin added the preview Experimental behavior label May 8, 2024
@konstin
Copy link
Member Author

konstin commented May 8, 2024

This is a correctness check (no lower bound is almost certainly incorrect), you should not be able to silence correctness checks; the correct way to get rid of the warning is to add the lower bound your project has, either implicitly or by policy.

@charliermarsh
Copy link
Member

I'm not totally convinced that I want this, but I guess it's fine for now if it's limited to tool.uv.sources and doesn't show up for non-sources users.

@@ -494,7 +494,7 @@ pub(crate) fn lower_requirement(
Source::Registry { index } => match requirement.version_or_url {
None => {
warn_user_once!(
"Missing version constraint (e.g. a lower bound) for {}",
"Missing version constraint (e.g. a lower bound) for `{}`",
Copy link
Member

Choose a reason for hiding this comment

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

Please change "e.g. a lower bound" to "e.g., lower bound".

@charliermarsh charliermarsh merged commit 7c7c9e2 into main May 8, 2024
43 checks passed
@charliermarsh charliermarsh deleted the konsti/ensure-bound branch May 8, 2024 17:16
@denkasyanov
Copy link
Contributor

denkasyanov commented Aug 22, 2024

Is there a way to add lower bounds automatically for installed dependencies that doesn't have them?

Not sure what exactly triggered the warning, update to 0.3.1 or (I guess more likely) adding a dependency from GitHub.

@konstin
Copy link
Member Author

konstin commented Aug 23, 2024

@denkasyanov Can you describe steps to reproduce your problem and the exact error you're seeing?

@denkasyanov
Copy link
Contributor

Sorry, should have added more context from the beginning.

I updated from uv 0.3.0 to 0.3.1 AND replaced the spyne package from pypi with spyne package from GitHub like that:

uv remove spyne
uv add git+https://github.com/arskom/spyne@f105ec2f41495485fef1211fe73394231b3f76e5

After that I tried to run uv sync and received warnings

warning: Missing version constraint (e.g., a lower bound) for `celery`
warning: Missing version constraint (e.g., a lower bound) for `django`

My dependencies at the moment:

[project]
dependncies = [
  "celery",
  "django",
 ]

This was expected, so no error (not even a real problem) here.

But since I had a ton of dependencies, I thought, maybe there is a way to "half-lock" them in the [project] table.

So that I could turn the example above into something like:

[project]
dependncies = [
  "celery>=5.4.0",
  "django>=5.1",
 ]

for all regular and dev dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preview Experimental behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants