poetry icon indicating copy to clipboard operation
poetry copied to clipboard

poetry won't check for pre-releases during dependency resolving of non-direct dependencies

Open struegamer opened this issue 3 years ago • 3 comments

  • [X] I am on the latest Poetry version.
  • [X] I have searched the issues of this repo and believe that this is not a duplicate.
  • [X] If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).
  • MacOS Catalina and also on Ubuntu Linux 18.04/20.04:
  • 1.1.7:

Issue

So, I have three python libs all of them are managed by poetry.

  • lib-a
  • lib-b
  • lib-c

All libs are building fine and can be uploaded to a repository.

lib-b and lib-c are dependencies of lib-a, so far so good.

I changed the python version dependency of all three libs to

[tool.poetry.dependencies]
python = "~3.6 || ~3.8"

And all lib versions are pre-release semver version (i.e. 0.1.2-alpha.0, etc.) so, there are no major.minor.patch versions for now. The dep lines for lib-a to add lib-b and lib-c as deps are looking like this:

lib-b = {version = "^0.1.0", allow-prereleases = true}
lib-c = {version = "^0.1.0", allow-prereleases = true}

doing a poetry install with this, it all works smooth and nicely.

Now I created a tool which is using lib-a as direct dependency,

pyproject.toml looks like this:

[tool.poetry]
name = "awesome-project"
version = "0.1.5-alpha.0"
description = ""
authors = ["me <[email protected]>"]
packages = [
  {include="awesomeproject", from="src"}
]

[tool.poetry.dependencies]
python = "~3.6 || ~3.8"
lib-a = {version = "^1.0.0", allow-prereleases = true}
....

Now lib-a has a pre-release version number, lib-b and lib-c too. lib-a pulls in lib-b and lib-c and it also references allow-prereleases=true.

Expectation would be that poetry install would check all python version deps from lib-a and it would also check the python version deps from lib-b and lib-c and also takes allow-prerelease=true setting for these two libs in lib-a into account.

Sadly, it breaks, telling me that lib-a is not compatible with the awesome-project python version setting, because lib-a checks for lib-b and lib-c not for the pre-release versions, only for major.minor.patch versions. Which is obviously not available.

Now, to test that this assumption is true, I updated lib-b and lib-c version to a major.minor.patch version, uploaded them to my local pypi repo, poetry updated lib-a (still using pre-release version number) and uploaded this to my local pypi repo.

Now for awesome-project doing a poetry install and it magically works now. all the same settings, just the version numbers of lib-b and lib-c changed.

Real bug is now: poetry won't check for pre-releases during dependency resolving of non-direct dependencies. Even when the version requirements are correctly set in the generated setup.py of the uploaded tarball.

I know dependency resolving is not easy, especially in python :) But during development you don't want to rely on the "latest stable" version of a library (direct or indirect), I guess there needs to be a fix for this issue.

struegamer avatar Aug 19 '21 13:08 struegamer

I think I just hit this in Poetry 1.2.0b2, in a trivial project (created with poetry-1.2 init -n; poetry-1.2 lock).

> poetry-1.2 --version
Poetry (version 1.2.0b2)
> poetry-1.2 add --lock --dry-run opentelemetry-distro
Using version ^0.31b0 for opentelemetry-distro

Updating dependencies
Resolving dependencies... (0.0s)

Because no versions of opentelemetry-distro match >0.31b0,<0.31
 and opentelemetry-distro (0.31b0) depends on opentelemetry-sdk (1.12.0rc1), opentelemetry-distro (>=0.31b0,<0.31) requires opentelemetry-sdk (1.12.0rc1).
And because opentelemetry-sdk (1.12.0rc1) depends on opentelemetry-api (1.12.0rc1), opentelemetry-distro (>=0.31b0,<0.31) requires opentelemetry-api (1.12.0rc1).
So, because no versions of opentelemetry-api match 1.12.0rc1
 and scratch depends on opentelemetry-distro (^0.31b0), version solving failed.

This is surprising because opentelemetry-distro's requirement is opentelemetry-sdk == 1.12.0rc1, i.e. Poetry has no discretion on which version is matched. As discussed in #3538, --allow-prereleases has no effect here because that only affects the generaion of constraints for entries on the command-line, and it's already selected the correct version of opentelemetry-distro.

Edit: I'm no longer sure that my reproduction above is the same as the problem described in this issue, so I opened #5825 for mine.

Edit: Seems like this was the right problem after all, so #5825 is closed as a duplicate of this one again.

TBBle avatar Jun 07 '22 11:06 TBBle

I've been poking at this for a while, and there seems to be two issues at-play in my repro-case (detailed in #5825):


DependencyCache (src/poetry/mixology/version_solver.py) caches a depdency lookup by a key that doesn't include constraint.

However, the data being cached is affected by the constraint, specifically whether pre-releases are allowed. So in my case, this code sees opentelemetry-api ~=1.3, and caches a list without pre-releases (because all the repositories pre-filter for the given constraint and apply the pre-release filtering logic). Later it sees opentelemetry-api==1.12.0rc1, and filters its existing list against that, producing [].

I worked around this by just ignoring this cache, and always passing the constraint down to the provider. However, I suspect this is wrong if the code above is relying on this cache's behaviour to deliver the intersection of constraints. That would be reasonable except that implies that Poetry considers ~=1.3 and ==1.12.0rc to have a null intersection because the first one is being treated as "Never allow a pre-release if a non-pre release exists", when the intent as I understand is supposed to be "Never select a pre-release if a non-pre release exists".

So the fix might actually be more complex if we need the DependencyCache to be delivering intersections of constraints, as it would involve taking away the Repository support for excluding pre-releases and making that code work in the DependencyCache so that it can cache results with pre-releases but not include them in the returned values when not appropriate.

If the intersecting behaviour of DependencyCache was just a nice-to-have, then my fix is probably fine. There's already a functools.lru_cache on this API to cover identical calls, which doesn't have this issue.

Edit: I noticed that the intersection behaviour is actually documented as DependencyCache's "key observation", so this is almost certainly the wrong path. I'm not sure how "backtracking" is handled (as mentioned there) but I guess it involves clearing the DependencyCache internal cache and rebuilding itm, as there's no apparent mechanism for undoing a particular intersection. As such, all this "first approach" is probably wrong, per the final two blocks of this comment.


During install, after writing the lockfile, the solver is rerun with a repo containing only the selected packages in order to build the install step. This isn't done if --lock is passed to the command, so this didn't reproduce for me in practical tests, but my proposed test-case in #5825 did trigger this.

The repo gets the same set of version queries again, and this time because the only version of opentelemetry-api it knows about is a pre-release, it returns an empty list for opentelemetry-api ~=1.3 which fails the install.

My initial attempt at a fix for this is to introduce a new Repository subclass for this case, that ignores the pre-release question when resolving. Since at this point we have already resolved the choice of package, I don't see why we'd want to filter the choice of package at all.


However, this latter fix makes me suspicious that I'm actually working around a higher-level issue:

  • The same call to PyPiRepository during dependency resolution returned non-empty only because there are existing releases as well. We didn't choose any of those results, suggesting that we're not applying those results correctly. (This is what the DependencyCache intersection behaviour was guaranteeing)
  • The logic in Repository.find_packages that implements "If there are no releases, use a pre-release" is predicated on constraint.is_any(), so it wouldn't kick-in for a ~= check.

I confirmed this with the following test-case:

def test_add_can_select_prereleases_subdependencies_when_really_necessary(
    app: PoetryTestApplication, repo: TestRepository, tester: CommandTester
):
    package_distro = get_package("opentelemetry-distro", "0.31b0")
    package_distro.add_dependency(
        Factory.create_dependency("opentelemetry-sdk", "1.12.0rc1")
    )
    package_distro.add_dependency(
        Factory.create_dependency("opentelemetry-api", ">=1.3,<2.0")
    )

    package_sdk = get_package("opentelemetry-sdk", "1.12.0rc1")
    package_sdk.add_dependency(
        Factory.create_dependency("opentelemetry-api", "1.12.0rc1")
    )
    package_sdk_stable = get_package("opentelemetry-sdk", "1.11.1")

    package_api = get_package("opentelemetry-api", "1.12.0rc1")

    repo.add_package(package_distro)
    repo.add_package(package_sdk_stable)
    repo.add_package(package_sdk)
    repo.add_package(package_api)

    tester.execute("opentelemetry-distro")

    expected = """\
Using version ^0.31b0 for opentelemetry-distro

Updating dependencies
Resolving dependencies...

Writing lock file

Package operations: 3 installs, 0 updates, 0 removals

  • Installing opentelemetry-api (1.12.0rc1)
  • Installing opentelemetry-sdk (1.12.0rc1)
  • Installing opentelemetry-distro (0.31b0)
"""

    assert tester.io.fetch_output() == expected
    assert tester.command.installer.executor.installations_count == 3

    content = app.poetry.file.read()["tool"]["poetry"]

    assert "opentelemetry-distro" in content["dependencies"]
    assert content["dependencies"]["opentelemetry-distro"] == "^0.31b0"

This is the same test-case as the one I created in #5825, but without package_api_stable. It fails even with my changes.

So I'm thinking that a wider change is needed, that DependencyCache implementing an 'intersection' behaviour is probably more-correct for its call-patterns, but the logic of "Don't choose pre-releases if it can be avoided" should be handled there (or in the calling code), rather than in Repository and its various sub-classes.


I have pushed the first fix attempt (that fails test_add_can_select_prereleases_subdependencies_when_really_necessary) to https://github.com/TBBle/poetry/commit/5dc3dfc49e224622a4a8ad88398ebfad03e4af96, and an outline of the second fix idea that passes it to https://github.com/TBBle/poetry/commit/eaa4174ba5f1aa6c562a3431cd4a5ca5ef27c131. As I noted in the comment on the latter commit, I think that "allow pre-releases" probably needs to be handled later/higher though, as we are still in the situation that DependencyCache.search_for can have packages re-appear that were previously not chosen if we suddenly require pre-releases because a new constraint ruled out all existing non-pre release packages.

TBBle avatar Jun 10 '22 15:06 TBBle

I am running into this issue also. Specifically in a similar situation to @TBBle with Opentelemetry. Interally, we have a data generation package that uses opentelemetry-sdk directly (which has its own internal dependency on opentelemetry-api). To test some of the upcoming changes and how they would affect us, I swapped our dependency to 1.12.0rc2. Testing the data generation package does fine (it uses pip). But some of its downstream dependencies use poetry and barf on opentelemetry-api:

Sum up:

  • Package A depends on opentelemetry-sdk 1.12.0rc2
  • Package B depends on Package A

Results: Package B fails to resolve opentelemetry-api dependency from opentelemetry-sdk, even though that version does exist: Because no versions of otlp-data-generator match >0.0.1,<0.0.2 and otlp-data-generator (0.0.1) depends on opentelemetry-sdk (1.12.0rc2), otlp-data-generator (>=0.0.1,<0.0.2) requires opentelemetry-sdk (1.12.0rc2). And because opentelemetry-sdk (1.12.0rc2) depends on opentelemetry-api (1.12.0rc2), otlp-data-generator (>=0.0.1,<0.0.2) requires opentelemetry-api (1.12.0rc2). So, because no versions of opentelemetry-api match 1.12.0rc2 and storage-etl depends on otlp-data-generator (^0.0.1), version solving failed.

To further test this, I pushed out a version of Package A that explicitly lists a dependency on opentelemetry-api 1.12.0rc2. The dependency failure in Package B has gone away.

I would expect these to resolve just fine, especially if --allow-prereleases is used...but I am not sure where the line would be drawn for resolving this.

Seefooo avatar Aug 03 '22 16:08 Seefooo

I only see one reproducer in this thread, and it works fine today

$ poetry init -n
$ poetry add --lock opentelemetry-distro==0.31b0

Updating dependencies
Resolving dependencies... (0.2s)

Writing lock file
$ poetry export --without-hashes
Warning: poetry-plugin-export will not be installed by default in a future version of Poetry.
In order to avoid a breaking change and make your automation forward-compatible, please install poetry-plugin-export explicitly. See https://python-poetry.org/docs/plugins/#using-plugins for details on how to install a plugin.
To disable this warning run 'poetry config warnings.export false'.
deprecated==1.2.14 ; python_version >= "3.10" and python_version < "4.0"
opentelemetry-api==1.12.0rc1 ; python_version >= "3.10" and python_version < "4.0"
opentelemetry-distro==0.31b0 ; python_version >= "3.10" and python_version < "4.0"
opentelemetry-instrumentation==0.31b0 ; python_version >= "3.10" and python_version < "4.0"
opentelemetry-sdk==1.12.0rc1 ; python_version >= "3.10" and python_version < "4.0"
opentelemetry-semantic-conventions==0.31b0 ; python_version >= "3.10" and python_version < "4.0"
setuptools==69.1.0 ; python_version >= "3.10" and python_version < "4.0"
typing-extensions==4.9.0 ; python_version >= "3.10" and python_version < "4.0"
wrapt==1.16.0 ; python_version >= "3.10" and python_version < "4.0"

NB various pre-releases resolved as expected

I guess this should be closed

dimbleby avatar Feb 19 '24 14:02 dimbleby

A quick poke at the history of the relevant file suggests #7978 was the fix, appearing in Poetry 1.5.1 and later.

Note for self: Before cleaning up https://github.com/TBBle/poetry/tree/allow-exact-pre_release-child-dependencies, check that both tests now pass. #7978 only tests one of the two cases I had in those patches. (The might correctly resolve both cases, but I'm too far from this code these days to be sure, and didn't have a public test-case to reproduce the second case, i.e. if no package_api_stable existed.)

TBBle avatar Feb 19 '24 17:02 TBBle

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

github-actions[bot] avatar Mar 22 '24 00:03 github-actions[bot]