tox-dev / tox-uv

Use https://github.com/astral-sh/uv with tox
MIT License
114 stars 18 forks source link

uv in commands does not resolve to the version packaged with tox-uv #17

Closed hugovk closed 5 months ago

hugovk commented 8 months ago

Follow on from https://github.com/tox-dev/tox-uv/issues/16.

I need to hardcode uv pip instead of {envpython} -m pip when using tox-uv:

 commands_pre =
-    {envpython} -m pip install -r requirements.txt
+    uv pip install -r requirements.txt

This works on:

This does not work on:

py: commands_pre[0]> uv pip install -r requirements.txt
py: failed with uv is not allowed, use allowlist_externals to allow it

With regular tox, I don't need to allowlist pip.

gaborbernat commented 8 months ago

Yes, but uv pip is not pip 🤔

Czaki commented 8 months ago

It happens because this plugin do not install uv in virtualenv (use external one) and pip is installed in virtualenv.

gaborbernat commented 8 months ago

We do use uv from the same Python environment as tox... so we do install it in virtualenv assuming you install tox in a virtual environment.

Czaki commented 8 months ago

We do use uv from the same Python environment as tox... so we do install it in virtualenv assuming you install tox in a virtual environment.

I was writing about virtual environment created by tox for the purpose of testing.

paddyroddy commented 8 months ago

Why would this issue only affect 3.11-3.13?

gaborbernat commented 8 months ago

Yeah that's not clear to me 🤔

Czaki commented 8 months ago

I ran tests with this plugin on python 3.11 and its works

paddyroddy commented 8 months ago

Very possibly completely unrelated, but I'm having a different version specific issue. My tests are working on ubuntu 3.10/3.11 and macos 3.11, but fail on macos 3.10. I'm not doing anything custom besides installing tox-uv. The error I'm getting is below, and an example test run is here.

py310-macos: venv> /Users/runner/hostedtoolcache/Python/3.10.13/x64/bin/uv venv -p 3.10 /Users/runner/work/sleplet/sleplet/.tox/py310-macos/.venv
py310-macos: install_deps> /Users/runner/hostedtoolcache/Python/3.10.13/x64/bin/uv pip install pytest-cov
.pkg: _optional_hooks> python /Users/runner/hostedtoolcache/Python/3.10.13/x64/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: get_requires_for_build_sdist> python /Users/runner/hostedtoolcache/Python/3.10.13/x64/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta
.pkg: freeze> /Users/runner/hostedtoolcache/Python/3.10.13/x64/bin/uv pip freeze
error: failed to canonicalize path `/Users/runner/work/sleplet/sleplet/.tox/.pkg/.venv/bin/python`
  Caused by: No such file or directory (os error 2)
.pkg: exit 2 (0.01 seconds) /Users/runner/work/sleplet/sleplet> /Users/runner/hostedtoolcache/Python/3.10.13/x64/bin/uv pip freeze pid=3069
.pkg: _exit> python /Users/runner/hostedtoolcache/Python/3.10.13/x64/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta
  py310-macos: FAIL code 2 (1.37 seconds)
  evaluation failed :( (2.12 seconds)
hugovk commented 8 months ago

I ran tests with this plugin on python 3.11 and its works

@Czaki What version of tox-uv and uv do you have? Are you calling uv pip?

Here's a fresh CI repro with tox-uv 1.5.0 and uv 0.1.15:

https://github.com/hugovk/tinytext/actions/runs/8169413347/job/22333447159

Czaki commented 8 months ago

your problem is in this line https://github.com/hugovk/tinytext/blob/c5bc2febf914cff2a2248cf8c51807796e272a1b/tox.ini#L13

This plugin does not install uv in the test environment but uses one from the parent environment. you may see this in this line for example:

https://github.com/hugovk/tinytext/actions/runs/8169413347/job/22333447159#step:5:30

If you want use uv in your commands_pre you need to add uv to deps or to allowlist external (I thinkt that this should work. But I think that the best solution for your problem will be just adding requirements.txt to deps section by add -rrequirements.txt

I do not know why it works for python 3.10 and bellow.

hugovk commented 8 months ago

Yes, I will switch to -r requirements.txt in deps (re: https://github.com/tox-dev/tox-uv/issues/16#issuecomment-1952289561) instead of [uv] pip install, and we we could close this issue for my specific use case.

I guess the general question this issue is really asking:

Should uv be available in tox-uv, in a similar way that pip is available in regular tox?

Czaki commented 8 months ago

I think yes, but I'm not a maintainer

gaborbernat commented 8 months ago

We already do https://github.com/tox-dev/tox-uv/blob/main/src/tox_uv/_venv.py#L129, if it's not working is a bug. It could be that find_uv_bin and the PATH resolve uv, resolve to different thing.

gaborbernat commented 8 months ago

Adding the host Python path could cause problems, so perhaps we want to explcitly allow the PATH resolve uv too, on top of this. But now we'd have two uv versions alive... so perhaps we want to patch the path resolve to resolve to find_uv_bin for uv as a better solution.

griels commented 7 months ago

How about an option to install uv in the virtualenv?

uv_seed_self = True perhaps... or uv_seed = False|True|UV(just uv)|FULL (everything)?

I need to set custom options on install_command which tox doesn't seem to support via setting opts, so I tried working around this with uv_seed = True and a wrapper that does subprocess.check_output([sys.executable, '-m', 'pip', 'install', 'uv']) and then subprocess.check_output([uv.find_uv_bin(), *pip_command]) but it's messy and seems to be unreliable, and obviously not all pip install args are supported, making debugging those cases tricky.

gaborbernat commented 5 months ago

I think the right approach would be to resolve uv to the plugins uv. The simple one would be to just install/copy uv to the created virtual environments.

gaborbernat commented 5 months ago

Done via https://github.com/tox-dev/tox-uv/pull/59

Azureuse commented 4 months ago

I might be misunderstanding the conversation above. I have the following section in my tox.ini:

commands_pre =
    poetry export --without-hashes -f requirements.txt --only test --output {env_dir}{/}requirements_test.txt
    {envpython} -m pip install --no-deps -c {env_dir}{/}constraints.txt -r {env_dir}{/}requirements_test.txt

I have tox installed via pipx and the above commands_pre section runs fine.

After adding tox-uv to the same environment as tox using pipx inject tox tox-uv it is clear that the normal pip commands executed by tox are being intercepted / modified to run using uv (which is awesome). However the {envpython} -m pip install isn't successfully redirected to uv and I get an error

> /.../.tox/py311-pandas15/bin/python: No module named pip

Changing {envpython} -m pip install to uv pip install resolved the problem but I would like my tox file to be agnostic to whether a user has tox-uv installed or not.

Is that possible, or I have I misunderstood the above? Many thanks

gaborbernat commented 4 months ago

But pip is not uv... you can't just swap them around and expect it to work interchangeably.

Azureuse commented 4 months ago

Sure, that's true. And I guess my question is more aligned with #16.

It looks tox-uv reroutes calls to 'normal' pip to uv pip - though I appreciate this may not be what is actually happening in the plugin. But for example without tox-uv I see

py311-pandas21: install_package_deps> python -I -m pip install ...

and with tox-uv the same line appears as

py311-pandas21: install_package_deps> ~/.local/pipx/venvs/tox/bin/uv pip install ...

Its really useful that a 'simple' tox.ini file will run with or without tox-uv without any modification. I would love it if there is a way that I could get the same agnostic behaviour when I need to run a [uv] pip install in the commands_pre.

Might this be possible?

Azureuse commented 4 months ago

I see tox-uv overrides the default_install_command method of the Installer, replacing ["python", isolated_flag, "-m", "pip", "install", "{opts}", "{packages}"] with [self.uv, "pip", "install", "{opts}", "{packages}"].

I can get close to the behaviour I want by replacing

commands_pre =
    ...
    {envpython} -m pip install --no-deps -c {env_dir}{/}constraints.txt -r {env_dir}{/}requirements_test.txt

with

commands_pre =
    ...
    {install_command} --no-deps -c {env_dir}{/}constraints.txt -r {env_dir}{/}requirements_test.txt

That allows me to access the different python pip install vs uv pip install command BUT the '{packages}' bit is still there...

py311-pandas21: commands_pre[1]> ~/.local/pipx/venvs/tox/bin/uv pip install '{packages}' --no-deps -c ~/project/.tox/py311-pandas21/constraints.txt -r ~/project/.tox/py311-pandas21/requirements_test.txt
error: Failed to parse: `{packages}`

I assume it is not possible to somehow remove / substitute in a value for '{packages}' here?

I am new to tox (and uv). Hopefully someone with more experience has some suggestions. Let me know if anything is unclear. Many thanks.

gaborbernat commented 4 months ago

Sure, that's true. And I guess my question is more aligned with #16.

It looks tox-uv reroutes calls to 'normal' pip to uv pip - though I appreciate this may not be what is actually happening in the plugin. But for example without tox-uv I see

py311-pandas21: install_package_deps> python -I -m pip install ...

and with tox-uv the same line appears as

py311-pandas21: install_package_deps> ~/.local/pipx/venvs/tox/bin/uv pip install ...

Its really useful that a 'simple' tox.ini file will run with or without tox-uv without any modification. I would love it if there is a way that I could get the same agnostic behaviour when I need to run a [uv] pip install in the commands_pre.

Might this be possible?

It is possible, however you will need to do something gymnastic to rewrite the command. The reason why we do this with the plugin for the core operation is because we know that those use only as much of the pip CLI That's compatible between the two tools. I don't think we should or want to do this automatically for user specified commands, because we cannot be sure that they are not using some pip only flags... That being said I'm not against someone adding opt-in flag to enable such behavior. PR welcome.