Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression with Git transitive dependencies #4051

Closed
3 tasks done
mtraynham opened this issue May 8, 2021 · 7 comments · Fixed by #4202
Closed
3 tasks done

Regression with Git transitive dependencies #4051

mtraynham opened this issue May 8, 2021 · 7 comments · Fixed by #4202
Labels
kind/bug Something isn't working as expected

Comments

@mtraynham
Copy link

mtraynham commented May 8, 2021

  • I am on the latest Poetry version.

  • I have searched the issues of this repo and believe that this is not a duplicate.

  • If an exception occurs when executing a command, I executed it again in debug mode (-vvv option).

  • OS version and name: Linux/Mac

  • Poetry version: 1.1.6

  • Link of a Gist with the contents of your pyproject.toml file: https://github.com/mtraynham/poetry_4051

Issue

There seems to be a regression with transitive dependencies of a Git dependency. I haven't exactly figured out why, but the
change for the whitelist on the installer is what causes the transitive dependency to be flagged for removal rather than keeping it. The dependency is in the lock file, but when it goes to install the project, it skips it.

This particular pull causes the issue, #3947. Reverting the whitelist changes seems to correct the issue:
https://github.com/python-poetry/poetry/pull/3947/files#diff-95c2e34175a95676a55830f8442a68da156ae6a31dfec68317a2fd746be2243fL292-L297
https://github.com/python-poetry/poetry/pull/3947/files#diff-95c2e34175a95676a55830f8442a68da156ae6a31dfec68317a2fd746be2243fL308

diff --git a/poetry/installation/installer.py b/poetry/installation/installer.py
index 8cef65d5..ebfe05db 100644
--- a/poetry/installation/installer.py
+++ b/poetry/installation/installer.py
@@ -289,6 +289,12 @@ class Installer:

         pool.add_repository(repo)

+        # We whitelist all packages to be sure
+        # that the latest ones are picked up
+        whitelist = []
+        for pkg in locked_repository.packages:
+            whitelist.append(pkg.name)
+
         solver = Solver(
             root,
             pool,
@@ -302,7 +308,7 @@ class Installer:
         solver.provider.load_deferred(False)

         with solver.use_environment(self._env):
-            ops = solver.solve(use_latest=self._whitelist)
+            ops = solver.solve(use_latest=whitelist)

         # We need to filter operations so that packages
         # not compatible with the current system,

This may actually be caused because I have 1 git dependency, which depends on another git dependency. E.g. the dependency tree looks like:

Project A -> Project B (git; poetry) -> Project C (git; setup.py) -> PyPi dep (not installed, but in the lock file)

@mtraynham mtraynham added kind/bug Something isn't working as expected status/triage This issue needs to be triaged labels May 8, 2021
@mtraynham
Copy link
Author

mtraynham commented May 8, 2021

I have recreated the issue with a minimal example, https://github.com/mtraynham/poetry_4051

Tests

poetry==1.1.5

Creating virtualenv dep-a in /home/foobar/poetry_4051/.venv
Using virtualenv: /home/foobar/poetry_4051/.venv
Installing dependencies from lock file

Finding the necessary packages for the current system

Package operations: 4 installs, 0 updates, 0 removals

  • Installing six (1.16.0): Pending...
  • Installing six (1.16.0): Installing...
  • Installing six (1.16.0)
  • Installing grpcio (1.28.1): Pending...
  • Installing grpcio (1.28.1): Installing...
  • Installing grpcio (1.28.1)
  • Installing dep-c (0.1.0 f886f69): Pending...
  • Installing dep-c (0.1.0 f886f69): Cloning...
  • Installing dep-c (0.1.0 f886f69): Building...
  • Installing dep-c (0.1.0 f886f69)
  • Installing dep-b (0.1.0 1a00be6): Pending...
  • Installing dep-b (0.1.0 1a00be6): Cloning...
  • Installing dep-b (0.1.0 1a00be6): Building...
  • Installing dep-b (0.1.0 1a00be6)

Installing the current project: dep_a (0.1.0)
  - Building package dep-a in editable mode
  - Adding dep_a.pth to /home/foobar/poetry_4051/.venv/lib/python3.8/site-packages for /home/foobar/poetry_4051
  - Adding the dep_a-0.1.0.dist-info directory to /home/foobar/poetry_4051/.venv/lib/python3.8/site-packages

poetry==1.1.6

Creating virtualenv dep-a in /home/foobar/poetry_4051/.venv
Using virtualenv: /home/foobar/poetry_4051/.venv
Installing dependencies from lock file

Finding the necessary packages for the current system

Package operations: 2 installs, 0 updates, 0 removals, 2 skipped

  • Removing grpcio (1.28.1): Pending...
  • Removing grpcio (1.28.1): Skipped for the following reason: Not currently installed
  • Removing six (1.16.0): Pending...
  • Removing six (1.16.0): Skipped for the following reason: Not currently installed
  • Installing dep-c (0.1.0 f886f69): Pending...
  • Installing dep-c (0.1.0 f886f69): Cloning...
  • Installing dep-c (0.1.0 f886f69): Building...
  • Installing dep-c (0.1.0 f886f69)
  • Installing dep-b (0.1.0 1a00be6): Pending...
  • Installing dep-b (0.1.0 1a00be6): Cloning...
  • Installing dep-b (0.1.0 1a00be6): Building...
  • Installing dep-b (0.1.0 1a00be6)

Installing the current project: dep_a (0.1.0)
  - Building package dep-a in editable mode
  - Adding dep_a.pth to /home/foobar/poetry_4051/.venv/lib/python3.8/site-packages for /home/foobar/poetry_4051
  - Adding the dep_a-0.1.0.dist-info directory to /home/foobar/poetry_4051/.venv/lib/python3.8/site-packages

1.1.6 with diff patch

Installing dependencies from lock file

Finding the necessary packages for the current system

Package operations: 2 installs, 0 updates, 0 removals, 2 skipped

  • Installing six (1.16.0): Pending...
  • Installing six (1.16.0): Installing...
  • Installing six (1.16.0)
  • Installing grpcio (1.28.1): Pending...
  • Installing grpcio (1.28.1): Installing...
  • Installing grpcio (1.28.1)
  • Installing dep-c (0.1.0 f886f69): Pending...
  • Installing dep-c (0.1.0 f886f69): Skipped for the following reason: Already installed
  • Installing dep-b (0.1.0 1a00be6): Pending...
  • Installing dep-b (0.1.0 1a00be6): Skipped for the following reason: Already installed

Installing the current project: dep_a (0.1.0)
  - Building package dep-a in editable mode
  - Adding dep_a.pth to /home/foobar/poetry_4051/.venv/lib/python3.8/site-packages for /home/foobar/poetry_4051
  - Removing existing dep_a-0.1.0.dist-info directory from /home/foobar/poetry_4051/.venv/lib/python3.8/site-packages
  - Adding the dep_a-0.1.0.dist-info directory to /home/foobar/poetry_4051/.venv/lib/python3.8/site-packages

@babyhuey
Copy link

We are seeing the same issue. Can confirm adding the whitelist back fixes the issue. We have downgraded to 1.1.5 in the meantime

@dobbymoodge
Copy link

This looks like the same issue as #3368

It looks like the whitelist fix referenced in the initial report is indeed the cause; it seems to have been added to prevent the solver from adding newer versions of packages than what are specified in the lockfile. It's not clear to me, but it seems like the bug this was supposed to fix is where a package that pulls in a path or VCS dependency ignores the dependency lock file, resolves potentially newer versions of the transitive dependencies, and adds those to its own lockfile. The whitelist change prevents the solver from resolving the transitive dependencies, but instead of installing the "locked" packages, it skips them.

In 1.1.5, the solver does resolve the transitive dependencies, but it ignores the versions from the VCS/path deps lockfiles, and installs the latest version that satisfies the dependency specification from the pyproject.toml file. This is arguably incorrect, but the behavior in 1.1.6 is definitely incorrect.

Not having a firm grasp on the code, and too little time right now to debug in depth, this snippet in particular looks like it might have significance. The deferred dependencies might not need to be resolved, but they should still be loaded?

@sdispater, as the contributor of the breaking PR, what do you think? Surely the transitive package dependencies should all be installed by poetry install whether the top-level pyproject.toml specifies the dependencies as path, VCS or otherwise? This is a blocking bug for me and my team, as we rely on path dependencies for organizing our internal tooling.

@mtraynham
Copy link
Author

mtraynham commented Jun 12, 2021

Likely related issues for 1.1.6:
#3956
#4000
#4111

@sdispater I believe #3947 has introduced this issue, but I am not well versed in how or why the lockfile is no longer being used as a whitelist during install, nor why the lockfile generation is somehow different from what an install performs. I wouldn't mind adding a PR to fix the issue, but some guidance would be appreciated; I'm not sure any of us understands the code well enough to suggest a correct fix.

@dobbymoodge
Copy link

Thanks @mtraynham, I had seen those, I have no idea why I didn't include those issues in my last comment

Also, a correction:

In 1.1.5, the solver does resolve the transitive dependencies, but it ignores the versions from the VCS/path deps lockfiles, and installs the latest version that satisfies the dependency specification from the pyproject.toml file. This is arguably incorrect, but the behavior in 1.1.6 is definitely incorrect.

Thinking about this a bit more, if you specify 2 path deps in pyproject.toml, and those deps have overlapping package dependencies, it's likely that some of the deps in their lock file will have conflicting versions. There's no clear way to prioritize versions from poetry.lock in path dependencies, so I think the 1.1.5 strategy of ignoring those lock files is actually correct. If someone needs their environment to adhere closely to what's specified in the path dep lockfiles, they should use a manually created venv and install the path deps by hand, in the order they want to prioritize the poetry.lock preferences for each dep.

@sdispater
Copy link
Member

The root cause has been identified (see #4202) and this will be fixed in the next bugfix release.

Note that you will to regenerate the lock file for the fix to take effect.

@abn abn removed the status/triage This issue needs to be triaged label Mar 3, 2022
Copy link

github-actions bot commented Mar 2, 2024

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 github-actions bot locked as resolved and limited conversation to collaborators Mar 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working as expected
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants