poetry icon indicating copy to clipboard operation
poetry copied to clipboard

no-update with poetry add --lock

Open jvenant opened this issue 3 years ago • 9 comments

  • [x] I have searched the issues of this repo and believe that this is not a duplicate.
  • [x] I have searched the FAQ and general documentation and believe that my question is not already covered.

Feature Request

Hi, In my current workflow, I install my python module in a docker image. I copy the project sources (with pyproject.toml poetry.lock) in the image and do a poetry install. it's simple and it works perfectly But sometime, I'd like to test just one module upgrade So In a job preceding the build, I'd like to update the pyproject and lock files with just this module to a specific version. And eventually its own dependencies if the current constraints doesn't match. I didn't find any commands and/or options to do that :

  • poetry update <module> only get the latest version
  • poetry add <module@version> also do the install
  • poetry add <module@version> --lock do a full lock update

What I'm looking for is something like a poetry add <module@version> --lock --no-update But this add command option doesn't currently exist.

I made my own plugin duplicating the add command and adding a --no-update option to add. It's very simple

I could do a PR with the change. But I'm actually not sure if it is a feature you want to have Maybe it is some kind of "anti-pattern" or maybe there is an other approach to achieve what I try to do

jvenant avatar Jan 06 '23 12:01 jvenant

Hey @jvenant,

poetry add already only updates existing dependencies if it's necessary, because the current version would be in conflict with the requirements of the new dependencies.

If you believe this is not the case, please provide a full reproducible example.

fin swimmer

finswimmer avatar Jan 06 '23 13:01 finswimmer

Sorry, I'm not sure I'm using the good terms then. To be very clear, what I'm talking about is this : https://github.com/python-poetry/poetry/blob/ef89e90fc1305e07a62e9e715c91a66c8d7425f7/src/poetry/console/commands/add.py#L256 I sometime need to set self.installer.update to False

My version change is :

if self.option("lock"):
    self.installer.lock()
    self.installer.update(not self.option("no-update"))

jvenant avatar Jan 06 '23 13:01 jvenant

It's not a full update due to the whitelist:

https://github.com/python-poetry/poetry/blob/ef89e90fc1305e07a62e9e715c91a66c8d7425f7/src/poetry/console/commands/add.py#L260

An example (pyproject.toml/poetry.lock) where your change matters would be really helpful.

radoering avatar Jan 06 '23 16:01 radoering

sorry for the answer delay

I was writing my answer and the walk-through to reproduce the behavior when I realized that I was in 1.2.x and the code involve to resolve the version has change a lot in 1.3.x

in 1.2.0 the behavior was related to unsatisfied dependencies https://github.com/python-poetry/poetry/blob/df9d3c91bf6ebf95bf951bea320c8723383f9ca1/src/poetry/mixology/version_solver.py#L413 and the fact that in some cases the resolver pick the first element in the list :

if package is None:
    with suppress(IndexError):
        package = packages[0]

this code does not exist anymore in 1.3.0 So I'll retry once I migrate to 1.3.0 and see if I can reproduce my case

meanwhile I think we can close this issue

jvenant avatar Jan 12 '23 13:01 jvenant

that code was refactored rather than removed: that fragment didn't "sometimes" pick the first element in the list, it always did.

However if you're happy to close the issue - then please do it

dimbleby avatar Jan 12 '23 15:01 dimbleby

So here is my usecase

with this pyproject.toml

[build-system]
requires = ["poetry-core>=1.0.0"]
build-backend = "poetry.core-mansonry.api"

[tool.poetry]
authors = ["nobody"]
name = "test"
version = "0.0.1"
description = "test"

[tool.poetry.dependencies]
python = [ ">=3.8,<3.11" ]
requests = "2.24.0"
pyopenssl = "22.1.0"

do a poetry lock to generate the lock poetry show idna : 2.10

then run poetry add [email protected] --lock poetry show idna : 3.4 In the lock file the idna dependency has been update to a higher version Through _dependency_cache and packages[0] cf https://github.com/python-poetry/poetry/issues/7309#issuecomment-1380339046 From my understanding it shouldn't as the current lock version is still valid for the new requests version poetry show requests : 2.28.1 (idna requirements : >=2.5,<4)

Now revert the changes in poetry.lock and pyproject.toml patch the add command (self.installer.update(True)) and set self.installer.update(False) AFTER the self.installer.lock()

do a poetry add [email protected] --lock again the idna dependency is not updated poetry show idna : 2.10 requests is updated poetry show requests : 2.28.1 (idna requirements : >=2.5,<4)

Once again this use case has been done on a poetry 1.2.2. Not sure about how it behave on 1.3.x

jvenant avatar Jan 12 '23 16:01 jvenant

Hi, I am also experiencing with Poetry 1.4.0 that poetry add package@version just updates (and installs) the specified package(s) in poetry.lock, but poetry add --lock package@version performs a full poetry.lock update, which is not what I want. I see two potential solutions.

Either self.installer.lock() inside if self.option("lock"): can be modified to self.installer.lock(update=not self.option("no-update")), so it behaves same as the lock command (basically a slight modification of @jvenant suggestion). edit: I believe I see how this would prevent the version solver from running at all, based on _do_install would take the other branch.

Or if self.installer.whitelist([r["name"] for r in requirements]) is also meant to control what is eligible to be updated when self.installer.lock() runs (as @radoering may have suggested, sorry it was not entirely clear), it can be moved before if self.option("lock"):. edit: I see the order here is not important.

I will debug and check if the full update with add --lock occurs because self.installer.whitelist runs after self.installer.lock. The current behaviour is inconsistent and undesirable from my perspective. Hope we can merge a resolution. edit: no this is not the reason. See next comment.

hwmq avatar Feb 28 '23 18:02 hwmq

The order of self.installer.whitelist does not appear to matter. However one difference I see is locked_repository = self._locker.locked_repository() is called both for add and lock --no-update, resulting in locked_repository being a poetry.repositories.lockfile_repository.LockfileRepository. But for add --lock, because both self._update and self._lock are True, locked_repository remains the result of locked_repository = Repository("poetry-locked"), which is a poetry.repositories.repository.Repository. I will try to investigate if this is the cause. @radoering how does this sound?

https://github.com/python-poetry/poetry/blob/1.4.0/src/poetry/installation/installer.py#L235

hwmq avatar Mar 01 '23 00:03 hwmq

@finswimmer @radoering @dimbleby The folllowing achieves the behaviour I desire and expect. Thoughts? Specifically does this cause any issue for poetry lock? poetry lock --no-update is not affected since it takes the other branch.

diff --git a/installation/installer.py b/installation/installer.py
index b5aa54f..c28ca79 100644
--- a/installation/installer.py
+++ b/installation/installer.py
@@ -234,7 +234,7 @@ class Installer:
 
         locked_repository = Repository("poetry-locked")
         if self._update:
-            if not self._lock and self._locker.is_locked():
+            if self._locker.is_locked():
                 locked_repository = self._locker.locked_repository()
 
                 # If no packages have been whitelisted (The ones we want to update),

https://github.com/python-poetry/poetry/blob/1.4.0/src/poetry/installation/installer.py#L237

hwmq avatar Mar 01 '23 02:03 hwmq

This change causes two tests to fail.

test_lock_with_invalid_lockfile[False]
test_lock_with_incompatible_lockfile[False]

Both tests employ the same reasoning. https://github.com/python-poetry/poetry/blob/0e72a55c43a993ec0258facec23416c9212964ba/tests/console/commands/test_lock.py#L281-L292 https://github.com/python-poetry/poetry/blob/0e72a55c43a993ec0258facec23416c9212964ba/tests/console/commands/test_lock.py#L311-L318

A lock file is not required if self._lock is True? But the presence of a lock file is being ignored which causes this bug. The test logic is confusing to me.

hwmq avatar Mar 09 '23 21:03 hwmq

Yes, the logic is confusing and could require a refactoring but maybe a small change is enough to fix this issue:

After taking a quick look, I believe you have to replace https://github.com/python-poetry/poetry/blob/40061f9d1351a9f76d6bdc1db8b3d903864c1373/src/poetry/console/commands/add.py#L257-L258 with https://github.com/python-poetry/poetry/blob/40061f9d1351a9f76d6bdc1db8b3d903864c1373/src/poetry/console/commands/update.py#L54

radoering avatar Mar 10 '23 17:03 radoering

Interesting suggestion. Keep in mind your proposed fix will cause poetry add --lock to have the same printing behaviour that poetry update --lock has. See https://github.com/python-poetry/poetry/issues/7588

It also causes two different tests to fail

test_add_with_lock_old_installer
test_add_with_lock

But it does seem safe. Some false prints is an improvement over unwanted updates, at the very least.

hwmq avatar Mar 11 '23 01:03 hwmq

A possible solution to fix the printing might be to replace the condition in https://github.com/python-poetry/poetry/blob/860c83872b7d93a25acfb331ccf1d00ebf2e302a/src/poetry/installation/installer.py#L294

-         if self._lock and self._update:
+         if not (self._execute_operations or self._dry_run):

radoering avatar Apr 06 '23 16:04 radoering

Hi everyone, I should be able to contribute to a fix. What I'm not sure is the desired outcome, which seemed to drift from wanting a no-update option to changing the behaviour of add --lock without adding any additional option. @radoering can you confirm? If so, could you update the title accordingly?

ralbertazzi avatar May 14 '23 07:05 ralbertazzi

Updated the title. We don't need a no-update flag because that's what poetry add should do no matter if you run it with or without --lock.

radoering avatar May 14 '23 08:05 radoering

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 Feb 29 '24 13:02 github-actions[bot]