tox-dev / tox-uv

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

Make uv dependency optional #82

Closed RomainBrault closed 2 weeks ago

RomainBrault commented 2 weeks ago

What's the problem this feature will solve?

Currently uv is installed in tox's virtualenv.

If a user have its own uv in another virutalenv, it might cause confusion and more management (e.g. updates)

Describe the solution you'd like

Follow PyPA's build implementation:

https://github.com/pypa/build/blob/main/src/build/env.py

        try:
            import uv

            self._uv_bin = uv.find_uv_bin()
        except (ModuleNotFoundError, FileNotFoundError):
            uv_bin = shutil.which('uv')
            if uv_bin is None:
                msg = 'uv executable not found'
                raise RuntimeError(msg) from None

            _ctx.log(f'Using external uv from {uv_bin}')
            self._uv_bin = uv_bin

In tox uv we could modify in https://github.com/tox-dev/tox-uv/blob/main/src/tox_uv/_venv.py:

    @property
    def uv(self) -> str:
        return find_uv_bin()

by

    @property
    def uv(self) -> str:
        try:
            import uv

            return uv.find_uv_bin()
        except (ModuleNotFoundError, FileNotFoundError):
            uv_bin = shutil.which('uv')
            if uv_bin is None:
                msg = 'uv executable not found'
                raise RuntimeError(msg) from None

        return uv_bin

Remove the toplevel import and make the dependency optional in pyproject.toml

Additional context

To me the difficult part here is the test / coverage. I don't really know how to do.

It will also require to add some documentation, e.g.

By default tox-uv doesn't ship with uv. If you don´t have uv installed on your system, you can either 1) Let tox-uv managed uv with pip install tox-uv[uv 2) Install uv

The drawback I'm seeing is that it might break systems where people didn't have uv install as a global tool and used only uv through tox-uv.

Alternative Solutions

Do not make the uv dependency optional but prioritize system uv over tox-uv's uv.

    @property
    def uv(self) -> str:
         uv_bin = shutil.which('uv')
         if uv_bin is None:
             return uv.find_uv_bin()

         return uv_bin        
RomainBrault commented 2 weeks ago

also a @cached_property instead of @property might be relevant.

gaborbernat commented 2 weeks ago

I am strongly against this. The sane behavior is to manage our version of UV; otherwise we have no idea whether what we get it compatible or not with the functionality we want to provide.