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 pip compile --universal fails when dependency's python-requires is incompatible with -p, even with markers #6730

Closed
Tracked by #11548
alex opened this issue Aug 27, 2024 · 20 comments
Assignees
Labels
bug Something isn't working

Comments

@alex
Copy link
Contributor

alex commented Aug 27, 2024

~ ❯❯❯ uv version
uv 0.3.5 (Homebrew 2024-08-27)

When you use a marker to express a dependency, uv pip compile --universal "forwards" that to the output. For example:

~ ❯❯❯ echo 'argcomplete; python_version >= "3.8"' | uv pip compile --universal -p 3.7 -
warning: The requested Python version 3.7 is not available; 3.12.5 will be used to build dependencies instead.
Resolved 4 packages in 3ms
# This file was autogenerated by uv via the following command:
#    uv pip compile --universal -p 3.7 -
argcomplete==3.1.2 ; python_full_version >= '3.8'
importlib-metadata==6.7.0 ; python_version < '0'
    # via argcomplete
typing-extensions==4.7.1 ; python_version < '0'
    # via importlib-metadata
zipp==3.15.0 ; python_version < '0'
    # via importlib-metadata

When Python 3.7 is actually used, no dependencies will be installed.

However, when this same marker is used for a dependency whose python-requires mean there is no resolution for the lower version, this fails:

~ ❯❯❯ echo 'check-sdist; python_version >= "3.8"' | uv pip compile --universal -p 3.7 -
warning: The requested Python version 3.7 is not available; 3.12.5 will be used to build dependencies instead.
  × No solution found when resolving dependencies:
  ╰─▶ Because only check-sdist{python_full_version >= '3.8'}<=0.1.3 is available and the requested Python version (>=3.7)
      does not satisfy Python>=3.8, we can conclude that all versions of check-sdist{python_full_version >= '3.8'} are
      incompatible.
      And because you require check-sdist{python_full_version >= '3.8'}, we can conclude that your requirements are
      unsatisfiable.

The fact that check-sdist only supports Python 3.8+ should be ok, because the dependency has the ; python_version >= "3.8" marker. But instead uv pip compile rejects it because it's not compatible with -p.

@charliermarsh
Copy link
Member

Ah yeah. You can solve this with the environments field, I think? By setting environments = ["python_version >= '3.8'"]. I think it is similar to #4668

@alex
Copy link
Contributor Author

alex commented Aug 27, 2024

Hmm, this may in fact be a duplicate of that issue.

I'm not sure environments does what I want here -- I want to have a lockfile that installs correctly on Python 3.7, but does not include this dependency.

@alex
Copy link
Contributor Author

alex commented Aug 27, 2024

Ah, environments = ["python_version < '3.8'", "python_version >= '3.8'"] causes it to work though.

@charliermarsh
Copy link
Member

I think (or hope) that environments = ["python_version < '3.8'", "python_version >= '3.8'"] would work though. That should work on any.

@charliermarsh
Copy link
Member

Yeah. The reason this happens is related to implementation details of the resolver (that we're considering changing). We only "fork" the resolver if we see multiple requirements for the same package. Once we fork, we (e.g.) do the right thing with respect to narrowing the supported Python range. But if we only see argcomplete; python_version >= "3.8", we don't fork (at least, not right now -- we have a PR open that does do this).

@alex
Copy link
Contributor Author

alex commented Aug 27, 2024

This may be a seperate issue, or not an issue at all, but it also seems that markers don't influence version resolution? Compare:

~ ❯❯❯ echo 'argcomplete; python_version >= "3.8"' | uv pip compile --universal -p 3.7 -
warning: The requested Python version 3.7 is not available; 3.12.5 will be used to build dependencies instead.
Resolved 4 packages in 3ms
# This file was autogenerated by uv via the following command:
#    uv pip compile --universal -p 3.7 -
argcomplete==3.1.2 ; python_full_version >= '3.8'
importlib-metadata==6.7.0 ; python_version < '0'
    # via argcomplete
typing-extensions==4.7.1 ; python_version < '0'
    # via importlib-metadata
zipp==3.15.0 ; python_version < '0'
    # via importlib-metadata
~ ❯❯❯ echo 'argcomplete; python_version >= "3.8"' | uv pip compile --universal -p 3.8 -
warning: The requested Python version 3.8 is not available; 3.12.5 will be used to build dependencies instead.
Resolved 1 package in 1ms
# This file was autogenerated by uv via the following command:
#    uv pip compile --universal -p 3.8 -
argcomplete==3.5.0

@charliermarsh
Copy link
Member

Can you explain that comment a bit more? Sorry, trying to follow. Do you mean the -p 3.7 vs. -p 3.8?

@alex
Copy link
Contributor Author

alex commented Aug 27, 2024

Sorry, I should have written more words.

In both of these invocations, argcomplete is only for Python 3.8. In one version this is done with -p 3.8 and in the other it's done with ; python_version >= "3.8". But these resolve to different argcomplete versions.

@charliermarsh
Copy link
Member

Ohhh yes, I didn't notice that. I think that's because in the non-forking case (without environments), we're still trying to select a version that works on Python 3.7.

environments is kind of a workaround for this right now; the resolver should actually do the right thing here, and select the maximum compatible version for Python 3.8 or later.

@alex
Copy link
Contributor Author

alex commented Aug 28, 2024

Should this be a separate issue, or is this the right place?

@alex
Copy link
Contributor Author

alex commented Sep 2, 2024

I seem to have found another variant on this -- when you have additional environments:

(tempenv-70b9155915899) ~/.v/tempenv-70b9155915899 ❯❯❯ cat uv.toml 
environments = ["python_version >= '3.10'", "python_version >= '3.8' and python_version < '3.10'", "python_version < '3.8'"]

(tempenv-70b9155915899) ~/.v/tempenv-70b9155915899 ❯❯❯ echo 'check-sdist; python_version >= "3.8"' | uv pip compile --universal -p 3.7 -
warning: The requested Python version 3.7 is not available; 3.12.5 will be used to build dependencies instead.
  × No solution found when resolving dependencies for split (python_full_version >= '3.8' and python_full_version <
  │ '3.10'):
  ╰─▶ Because only check-sdist{python_full_version >= '3.8'}<=0.1.3 is available and the requested Python version (>=3.7)
      does not satisfy Python>=3.8, we can conclude that all versions of check-sdist{python_full_version >= '3.8'} are
      incompatible.
      And because you require check-sdist{python_full_version >= '3.8'}, we can conclude that your requirements are
      unsatisfiable.

@alex
Copy link
Contributor Author

alex commented Sep 6, 2024

The bug described ^ appears to be fixed now.

@BurntSushi
Copy link
Member

It looks like uv 0.4.5 fixed it:

[andrew@duff i6730]$ cat uv.toml
environments = ["python_version >= '3.10'", "python_version >= '3.8' and python_version < '3.10'", "python_version < '3.8'"]
[andrew@duff i6730]$ echo 'check-sdist; python_version >= "3.8"' | uv-0.4.4 pip compile --universal -p 3.7 -
warning: uv is only compatible with Python >=3.8, found Python 3.7.9
  x No solution found when resolving dependencies for split (python_full_version >= '3.8' and python_full_version < '3.10'):
  `-> Because only check-sdist{python_full_version >= '3.8'}<=0.1.3 is available and the requested Python version (>=3.7) does not satisfy Python>=3.8, we can conclude that all versions of check-sdist{python_full_version >= '3.8'}
      are incompatible.
      And because you require check-sdist{python_full_version >= '3.8'}, we can conclude that your requirements are unsatisfiable.
[andrew@duff i6730]$ echo 'check-sdist; python_version >= "3.8"' | uv-0.4.5 pip compile --universal -p 3.7 -
warning: uv is only compatible with Python >=3.8, found Python 3.7.9
Resolved 10 packages in 5ms
# This file was autogenerated by uv via the following command:
#    uv pip compile --universal -p 3.7 -
build==1.2.1 ; python_full_version >= '3.8'
    # via check-sdist
check-sdist==0.1.3 ; python_full_version >= '3.8'
colorama==0.4.6 ; python_full_version >= '3.8' and os_name == 'nt'
    # via build
importlib-metadata==8.4.0 ; python_full_version >= '3.8' and python_full_version < '3.10.2'
    # via build
importlib-resources==6.4.4 ; python_full_version == '3.8.*'
    # via check-sdist
packaging==24.1 ; python_full_version >= '3.8'
    # via build
pathspec==0.12.1 ; python_full_version >= '3.8'
    # via check-sdist
pyproject-hooks==1.1.0 ; python_full_version >= '3.8'
    # via build
tomli==2.0.1 ; python_full_version >= '3.8' and python_full_version < '3.11'
    # via
    #   build
    #   check-sdist
zipp==3.20.1 ; python_full_version >= '3.8' and python_full_version < '3.10.2'
    # via
    #   importlib-metadata
    #   importlib-resources

I'm actually not totally sure which commit fixed this. I haven't done a bisect. I see multiple possible candidates I think?

Also, the python_version < '0' marker is weird. It can never be true, so I wonder what leads to us writing that out. That might be worth investigating too. (I know we can just filter such things out towards the end, but I'd like to better understand why they show up in the first place.)

@charliermarsh
Copy link
Member

I believe Konsti dropped those in his unreachable PRs, maybe?

@BurntSushi
Copy link
Member

They are definitely still around (using the example in the OP):

[andrew@duff i6730]$ echo 'argcomplete; python_version >= "3.8"' | uv-main pip compile --universal -p 3.7 -
warning: uv is only compatible with Python >=3.8, found Python 3.7.9
Resolved 4 packages in 32ms
# This file was autogenerated by uv via the following command:
#    uv pip compile --universal -p 3.7 -
argcomplete==3.1.2 ; python_full_version >= '3.8'
importlib-metadata==6.7.0 ; python_full_version < '0'
    # via argcomplete
typing-extensions==4.7.1 ; python_full_version < '0'
    # via importlib-metadata
zipp==3.15.0 ; python_full_version < '0'
    # via importlib-metadata

@charliermarsh
Copy link
Member

charliermarsh commented Sep 6, 2024

But I don’t see them in your reproduced snippet, and those changes came after this issue.

@charliermarsh
Copy link
Member

Oh sorry, you re-ran there, I see. Do those packages appear if you use uv lock instead? We probably just need to apply whatever Konsti did in the lockfile to the universal output case \cc @konstin

@konstin
Copy link
Member

konstin commented Sep 6, 2024

I believe Konsti dropped those in his unreachable PRs, maybe?

The pruning currently only happens in the transformation from graph to uv.lock, we should also prune requirements.txt entries with a false marker.

@charliermarsh
Copy link
Member

Gonna track that last part here: #7196

@charliermarsh charliermarsh added the bug Something isn't working label Sep 8, 2024
@charliermarsh
Copy link
Member

Should be fixed in the next release (see: #8628).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants