-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Add poetry lock --diff
flag to show which packages were changed when locking
#9259
Comments
I think you have misunderstood something, though I cannot tell what.
I expect your problem is simply that you have allowed You might want to add a Then I think that both this feature request and most of the work you have done in that other repository are unnecessary |
that is what expand/collapse
The problem is that we can't install the package because of #8737 -- so we have to run
so, yes. what i'm talking about is how we can't see what was updated in a
well that's pretty rude! would love to hear how you would do it better. we are trying to consolidate lockfile updates into specific, coordinated PRs from a known user to keep our PRs and git history uncluttered, so if the answer is to relock in every PR that's pretty user hostile behavior when the alternative is add an optional print command for information that seems pretty natural to want when one relocks. |
The problem is that you have allowed your The fix is to make a one-time correction - after which the whole premise for this feature request and that other pull request collapses. There is no "conundrum", future updates will work exactly as they always have. Pointing this out is intended to be helpful rather than rude. You can take a horse to water... |
the weird thing is that they didn't, and the divergence was caused by a combination of a problem that is being discussed elsewhere and other extremely normal things that happen in the course of collaborative development. The merge and lock happened out of order, and it was masked in CI by the cache. We got that. Already resolved. Fully 100% crystal clear on how manually locking resolves that problem this time, as I was when i opened the issue. That's actually not really relevant at all to the content of this issue though -
The premise for this feature request is extremely simple: "when poetry does something, can it tell me what it did?" Even in the normal situation, we still want to be able to see what packages were changed when we automatically refresh the lockfile as the docs suggest doing. It would be nice if we didn't have to install the packages every time in order to be able to use the output from
are there any other devs that aren't going to talk down to me for asking and offering to write something both simple and expected? I'd even happily take a "no, we don't want to display changes when updating a lockfile because..." |
yes they did, as you can confirm by running |
please read the rest of that sentence. i'm just about done with poetry unless there are any other devs that aren't intent on missing the purpose of the issue, this isn't the first issue where someone is being pointlessly combative. look allow me to recenter the purpose of the issue: "When I run the |
I really am trying to help you by getting to the root of your use case What you wrote when you opened this was that you saw a problem running This is wrong, and a far simpler way to address the problem that you saw would be to keep your You seem now to be shifting to "this is a good idea anyway". I'm not sure whether I agree or not, if you prefer to fix your pipelines by this more complicated approach then so be it. |
I'm not in charge of that repo. I don't approve the pull requests. I don't ensure that everyone did everything in the right order. I am just trying to help out. I wanted to write something that would be robust to potential future errors like this. I will pull a check like this. I will also keep the PR that I made because i a) didn't want to have to install the packages every time in the first place because it would waste cycles or cache space, and b) need it any way to get a presentable PR from that action. In order to not have to do that workaround, i proposed something that I thought would be an obvious, near-zero labor, improvement to the lock command. When a tool requires people to do awkward things to work around its limitations, it could be 100% my fault, user error, I should have obviously known the Correct thing to do before the issue ever happened and it's fundamentally bad for poetry to offer informative feedback on what it's doing. it also could be that there is some shortcoming in the docs or featureset of the tool. it's usually both, and that's why people raise issues - 'hey this isn't working for me can it do something different.' So yes - I may have written the initial issue imprecisely when I said "it will throw an error every time" when i meant "it will throw an error every time this particular situation happens." mea culpa. Please take the note that there is definitely something else wrong here too. If it's the position of the devs that there being no way to tell what changed during a lock aside from diffing it is intended behavior, great. that's about all i came here to ask and offer help if wanted about. |
I represent only myself fwiw I think that the change you propose to make might not be so easy as you expect - try it and prove me wrong! So if I were you I would absolutely be looking for an easier way to solve the problem that I had in front of me. Another simple approach, if linkml are unwilling or unable to keep the files in their repository consistent, would be to preface the existing action with |
--- a/poetry/console/commands/lock.py
+++ b/poetry/console/commands/lock.py
@@ -20,6 +20,10 @@ class LockCommand(InstallerCommand):
" version of <comment>pyproject.toml</>. (<warning>Deprecated</>) Use"
" <comment>poetry check --lock</> instead.",
),
+ option(
+ "show-updates",
+ None,
+ "Show which packages were updated during lockfile generation.")
]
help = """
@@ -50,6 +54,6 @@ file.
)
return 1
- self.installer.lock(update=not self.option("no-update"))
+ self.installer.lock(update=not self.option("no-update"), show_updates=self.option('show-updates'))
return self.installer.run()
--- a/poetry/installation/installer.py
+++ b/poetry/installation/installer.py
@@ -58,6 +58,7 @@ class Installer:
self._groups: Iterable[str] | None = None
self._skip_directory = False
self._lock = False
+ self._show_updates = False
self._whitelist: list[NormalizedName] = []
@@ -143,16 +144,24 @@ class Installer:
return self
- def lock(self, update: bool = True) -> Installer:
+ def lock(self, update: bool = True, show_updates: bool = False) -> Installer:
"""
Prepare the installer for locking only.
"""
self.update(update=update)
+ self.show_updates(show_updates=show_updates)
self.execute_operations(False)
self._lock = True
return self
+ def show_updates(self, show_updates: bool = False):
+ """
+ When locking, show which packages were updated
+ """
+ self._show_updates = show_updates
+ return self
+
def is_updating(self) -> bool:
return self._update
@@ -267,8 +276,16 @@ class Installer:
uninstalls = self._populate_lockfile_repo(lockfile_repo, ops)
if not self.executor.enabled:
+ if self._show_updates:
+ pre_ops = self._get_operations_from_lock(self._locker.locked_repository())
+
# If we are only in lock mode, no need to go any further
self._write_lock_file(lockfile_repo)
+
+ if self._show_updates:
+ post_ops = self._get_operations_from_lock(self._locker.locked_repository())
+ self._do_show_updates(pre_ops, post_ops)
+
return 0
if self._groups is not None:
@@ -353,6 +370,28 @@ class Installer:
self._io.write_line("")
self._io.write_line("<info>Writing lock file</>")
+ def _do_show_updates(self, pre_ops: list[Operation], post_ops: list[Operation]):
+ pre_ops = {op.package.name: op.package for op in pre_ops}
+ post_ops = {op.package.name: op.package for op in post_ops}
+ changes = []
+ for pre_pkg_name, pre_pkg in pre_ops.items():
+ if pre_pkg.name not in post_ops:
+ changes.append(Uninstall(pre_pkg))
+ else:
+ post_pkg = post_ops.pop(pre_pkg.name)
+ if pre_pkg.version != post_pkg.version:
+ changes.append(Update(initial=pre_pkg, target=post_pkg))
+ for post_pkg in post_ops.values():
+ changes.append(Install(post_pkg))
+
+
+ self._io.write_line("")
+ self.executor._display_summary(changes)
+ for op in changes:
+ message = f" <fg=blue;options=bold>-</> {self.executor.get_operation_message(op, done=True)}"
+ self._io.write_line(message)
+
def _execute(self, operations: list[Operation]) -> int:
return self._executor.execute(operations) |
aiui, So you are not showing changes to the lockfile - you are showing changes that would be made to the environment, if you were to install from the updated lockfile. I think that is not what you want. (But if it is what you want, good news! we already have |
I think I understand what you want and I think it makes sense to me. However, there are some pitfalls:
|
If you'll read the rest of the code, thats fine here - calculating operations before and after updating the lockfile and taking the diff gives you the changes to the lockfile. I tested it without anything installed, with an installation, and with a partial installation. There is probably a better way to do it, but I would rather delete this issue and stop using poetry altogether than have more input from you, thanks.
@radoering THANK YOU for your input.
I noticed! And thats totally reasonable. I only poked around a bit, but I think I was able to capture the diff by getting the operations from the lockfile before and after locking - its not very sophisticated and it feels a little expensive, but it was ofc much shorter than the solving step, so maybe thats ok for an optional flag? The operation system is super nice btw, nice work there, good to find a package where things work as expected :)
Good point!! I had missed that. Id be happy to update my example and take a closer look at the tests to see what other cases I have (certainly) missed and make a more organized PR if this is something yall would consider. It seems like it might be a good thing in general to take a diff of operations to be performed - it looks like maybe the Thanks for your courteous reply, happy to draft this so its as little work to yall as I can make it. |
I have decided that following the discussion on this issue I will not try and PR this and am instead transitioning my active projects away from poetry. Thanks to the maintainers who have not been extremely rude to me and others over the years for their work. It might be time to re-read the code of conduct and see if theres anything to be done to improve how these discussions go - as someone who I showed this issue to said: "communities fall to the level of horrible behavior they tolerate." Thanks again for the work, maybe we'll link up in some kinder place ♥ |
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. |
Issue Kind
Brand new capability
Description
first of all THANKS to all y'all, really appreciate all the work that has gone into poetry, we use it all the time and love it.
We have made an action to open a pull request every week to keep our lockfile up to date in a more organized way, and as part of that we would like to include in the body of the PR the packages that were updated.
As far as I am aware there is no way to show that as the output of the
poetry lock
command. Previously we were usingpoetry install && poetry update
which WOULD show the version diff, but after 1.8.0 thepoetry install
command throws an error anytime the action whose purpose is to update the lockfile would be needed because the lockfile needs to be updated! conundrum!It would be pretty great to be able to do this and replicate the output of
poetry update
, which would also save us the CI time of needing to actually install the packages when we don't really need toIf the devs think this is an OK idea, i would be happy to write the PR with some guidance on what would be acceptable here.
Impact
Described a bit above, but it seems like a pretty intuitive thing to be able to expect from
poetry lock
- "when i relock, show me what changed."poetry update
already behaves this way. This I think would be most relevant when using poetry in CI, but it also would be useful in general interactive usage, i frequently will lock and then go and check out the git diff to see what changed, which is pretty opaque behavior even if it's not that big of a deal.Workarounds
We are currently doing this as a workaround:
to get
The text was updated successfully, but these errors were encountered: