python-poetry / poetry

Python packaging and dependency management made easy
https://python-poetry.org
MIT License
31.05k stars 2.25k forks source link

`poetry check` does not take into a account path dependencies #8633

Open adriangb opened 10 months ago

adriangb commented 10 months ago

If you have a path dependency and add a dependency to its pyproject.toml file then run poetry check --lock it will say everything is OK when it shouldn't be. It seems to me that in general the mechanism to check if the lockfile is up to date with pyproject.toml should take into account path dependencies.

I can think of two solutions:

The first approach suffers from the problem that these path dependencies might not even use a pyproject.toml file / PEP 517. They could use setup.py or something super dynamic. The second approach would be slow for the common case of path dependencies that do use PEP-517 (and likely even use poetry).

A compromise is best: if all of the path dependencies are PEP-517 dependencies then assume that the build backend is deterministic (I think this is part of the PEP / a fair assumption) and just check the content-hash (which would include a hash of all of the pyproject.toml files). If any path dependency is not PEP-517 compliant fall back to never assuming the lockfile is up to date based on the hash.

Does this seem reasonable? If so I can work on an implementation.

adriangb commented 10 months ago

cc @radoering since we worked together previously on #6843 and related issues

radoering commented 10 months ago

if all of the path dependencies are PEP-517 dependencies then assume that the build backend is deterministic

I suppose with "PEP-517 dependencies" you are referring to projects that declare their dependencies according to PEP 621 in a dependencies section in the pyproject.toml (or in case of poetry in tool.poetry.dependencies).

the content-hash (which would include a hash of all of the pyproject.toml files)

I'd probably keep hashes of path dependencies' pyproject.toml content separately. That way we can still check the content-hash of the project itself even if some path dependencies, which might not be relevant for the current environment, are not available.

If any path dependency is not PEP-517 compliant fall back to never assuming the lockfile is up to date based on the hash.

I don't like the idea that poetry check --lock will always fail and you can't do anything about it. That should probably be opt-in.

In general, I tend to think that we should make such a behavior opt-in. There are several options for making it optional (not sure which would be best):

adriangb commented 10 months ago

I suppose with "PEP-517 dependencies" you are referring to projects that declare their dependencies according to PEP 621 in a dependencies section in the pyproject.toml (or in case of poetry in tool.poetry.dependencies).

Not really. I meant dependencies for which their sub-dependencies can be statically determined without executing code. That is, not a setup.py.

I'd probably keep hashes of path dependencies' pyproject.toml content separately. That way we can still check the content-hash of the project itself even if some path dependencies, which might not be relevant for the current environment, are not available.

Yup that makes sense!

I don't like the idea that poetry check --lock will always fail and you can't do anything about it. That should probably be opt-in.

I'd rather make it opt-out, the only users that would be impacted are those that are using path dependencies and those path dependencies are using setup.py / we can't statically determine the sub-dependencies and need to hash them. I'm guessing that's a small subset of users, I'd rather make the behavior safer / more correct for the rest of users even if we annoy that small fraction a bit. A compromise could be to still resolve the dependencies by running setup.py (or whatever Poetry would do to install the project) and emit a warning saying poetry check --lock will be slow because .... But I'm not sure how technically complex that might be vs. a --check-lock-on-path-dependencies-using-setup-py (or whatever a boolean flag would be called).

sfarina commented 5 months ago

Facing this at the moment with editable dependencies that have changed. I expect the project that relies on the editable dependencies to fail a check.

schematically: HighLevelCode uses an editable install of ProjectLibrary ProjectLibrary depends on version 1.0 of SomeLibrary lock HighLevelCode, it depends on SomeLibrary v1.0 ProjectLibrary updates the dependency of SomeLibrary to 2.0 HighLevelCode is still locked to SomeLibrary v1.0, it needs to be re-locked, so poetry check --lock should fail

 ~/r/d/s/api $ poetry check
All set!
 ~/r/d/s/api $ poetry lock --check
poetry lock --check is deprecated, use `poetry check --lock` instead.
poetry.lock is consistent with pyproject.toml.
 ~/r/d/s/api $ poetry check --lock 
All set!

edit: sorry if this is missing the point of this ticket.