juftin / hatch-pip-compile

hatch plugin to use pip-compile (or uv) to manage project dependencies and lockfiles
http://juftin.com/hatch-pip-compile/
MIT License
71 stars 3 forks source link

Recursive optional dependencies lead to incorrect requirements files #78

Open dhdaines opened 3 months ago

dhdaines commented 3 months ago

A not entirely documented (https://github.com/pypa/pip/issues/11296) but extremely useful feature of pip since version 21.2 is that optional-dependencies groups can depend on each other: https://hynek.me/articles/python-recursive-optional-dependencies/

For example if your package is mypkg and it has an optional REST API which you could run with uvicorn or some other server, which you run from the uvicorn environment in hatch, and for which you'd like to lock dependencies, you could do this:

[project.optional-dependencies]
api = [
    "fastapi"
]
uvicorn = [
     "mypkg[api]",
     "uvicorn",
]
[tool.hatch.envs.uvicorn]
features = [ "uvicorn" ]
type = "pip-compile"

Unfortunately this doesn't really work with pip-compile or hatch-pip-compile, because you will end up with this in requirements/requirements-uvicorn.txt:

mypkg==1.0.0
    # via hatch.envs.uvicorn

Oh noes! It went off to PyPI and found that yes, indeed, there is a package there already called mypkg and proceeded to add it as a dependency to the requirements file. That's definitely not what you want!

Note that if you run pip-compile --extra uvicorn in the environment you'll get this instead:

mypkg[api] @ file:///my/local/path/to/mypkg
    # via file:///my/local/path/to/mypkg

Which is not what you want either, but definitely better than pulling in some possibly unrelated or out-of-date package from PyPI. In this case it would be easy to post-process the output of pip-compile to remove editable installs of recursive optional dependencies, for instance.

Responsibility for fixing this might be partly in pip-tools if there isn't a way for hatch-pip-compile to get it to do the right thing...

dhdaines commented 3 months ago

At first glance, adding --unsafe-package mypkg --no-allow-unsafe to the pip-compile arguments will do the right thing, based on https://github.com/jazzband/pip-tools/issues/2002#issuecomment-1786936141

juftin commented 3 months ago

This is an interesting one. I didn't know about recursive optional dependencies.

hatch-pip-compile is an EnvironmentPlugin - so at "pip-compile time" it references an environment property, dependencies, and sends that over to pip-compile as a temporary requirements.in file.

So in a perfect world for your example, the dependencies property should return ['fastapi', 'uvicorn'] - but instead it's returning ['mypkg[api]', 'uvicorn'].

I can try to add some logic here that detects recursive optional dependencies, gathers all of the ultimate dependencies, and injects them into pip-compile - but IMO an ideal fix would be upstream on the dependencies / dependencies_complex properties. @ofek I'm curious what you're doing to handle this scenario where optional dependency groups refer to other groups. Is it this a reasonable request for hatch to change the behavior of dependencies / dependencies_complex for these cases?

ofek commented 3 months ago

Yes this is an oversight https://github.com/pypa/hatch/issues/1347

dhdaines commented 3 months ago

At first glance, adding --unsafe-package mypkg --no-allow-unsafe to the pip-compile arguments will do the right thing, based on jazzband/pip-tools#2002 (comment)

Some further information, this workaround unfortunately doesn't work correctly with hatch-pip-compile (by adding --unsafe-package to pip-compile-args), because (perhaps for the reasons in previous comments) it ends up referring to to the upstream mypkg[api] rather than the local one. So you still get the wrong transitive dependencies (or lack thereof) even if you don't get the incorrect self-dependency.

I think this is what you're talking about in https://github.com/juftin/hatch-pip-compile/issues/78#issuecomment-2028151851?

juftin commented 3 months ago

This example helped to clarify this issue for me since pypackage1234 doesn't exist on PyPI. This works when it uses the default virtualenv environment type by hatch, but breaks on hatch env create uvicorn when you un-comment the last line and use the pip-compile environment type from hatch-pip-compile:

pip._internal.exceptions.DistributionNotFound: No matching distribution found for mypkg1234[api]
[project]
name = "mypkg1234"
version = "1.0.0"

[project.optional-dependencies]
api = [
    "fastapi"
]
uvicorn = [
     "mypkg1234[api]",
     "uvicorn",
]

[tool.hatch.envs.uvicorn]
features = [ "uvicorn" ]
# type = "pip-compile"

Inside of the environment plugin, the dependencies property returns ['mypkg[api]', 'uvicorn'], although I believe that ['fastapi', 'uvicorn'] would be more intuitive - since that's the actual intention behind the recursive optional dependencies.


I can try to see how I would handle this in hatch-pip-compile - but implementing it upstream would work everywhere and benefit other plugins. If this is something I can figure out I can submit a PR upstream too.

In the meantime, until this is resolved if you'd like to use hatch-pip-compile the best way might be to stay away from recursive dependencies 😞

[project]
name = "mypkg"
version = "1.0.0"

[project.optional-dependencies]
api = [
    "fastapi"
]
uvicorn = [
     "fastapi",
     "uvicorn",
]

[tool.hatch.envs.uvicorn]
features = [ "uvicorn" ]
type = "pip-compile"
dhdaines commented 3 months ago

Thanks for looking into this and all the examples and explanation! For the moment we can just workaround by not using recursive dependencies for the particular environment that uses pip-compile.

juftin commented 3 months ago

Will be resolved upstream by https://github.com/pypa/hatch/pull/1387

ofek commented 3 months ago

Can you please confirm that this works on the master branch of Hatch?

juftin commented 3 months ago

Can you please confirm that this works on the master branch of Hatch?

Yep, confirmed working. Really nice work @ofek 🙇

pyproject.toml

```toml [project] name = "mypkg1234" version = "1.0.0" [project.optional-dependencies] api = [ "fastapi" ] uvicorn = [ "mypkg1234[api]", "uvicorn", ] [tool.hatch.envs.uvicorn] features = [ "uvicorn" ] type = "pip-compile" ```

requirements/requirements-uvicorn.txt

```text # # This file is autogenerated by hatch-pip-compile with Python 3.12 # # - fastapi # - uvicorn # annotated-types==0.6.0 # via pydantic anyio==4.3.0 # via starlette click==8.1.7 # via uvicorn fastapi==0.110.2 # via hatch.envs.uvicorn h11==0.14.0 # via uvicorn idna==3.7 # via anyio pydantic==2.7.0 # via fastapi pydantic-core==2.18.1 # via pydantic sniffio==1.3.1 # via anyio starlette==0.37.2 # via fastapi typing-extensions==4.11.0 # via # fastapi # pydantic # pydantic-core uvicorn==0.29.0 # via hatch.envs.uvicorn ```

ofek commented 3 months ago

Awesome, thank you! Just FYI while I have you here, the next release will begin shipping UV directly as a dependency.

juftin commented 3 months ago

Awesome, thank you! Just FYI while I have you here, the next release will begin shipping UV directly as a dependency.

I saw uv fly by while I was installing from master, that is awesome!