Fix lock updates for locks with sdist bystanders. #2325
Conversation
Previously, a `pex3 lock update` would fail whenever bystander projects (those projects in the lock but not targeted for update via `-p`) had an sdist primary artifact. Fixes pex-tool#2324
pex/resolve/lockfile/updater.py
Outdated
| original=original_pin.as_requirement(), updated=updated_pin.as_requirement() | ||
| ) | ||
| ) | ||
| assert updated_requirement.artifact == locked_requirement.artifact, ( |
There was a problem hiding this comment.
@cburroughs, although these asserts should never happen and thus not need good error messages, this one did trip which made your life harder debugging; so ... perhaps if there is a regression the new IT does not catch this helps some day.
pex/resolve/locked_resolve.py
Outdated
| @@ -148,7 +148,7 @@ def from_url( | |||
|
|
|||
| url = attr.ib() # type: str | |||
| fingerprint = attr.ib() # type: Fingerprint | |||
| verified = attr.ib() # type: bool | |||
| verified = attr.ib(eq=False) # type: bool | |||
There was a problem hiding this comment.
A comment here explaining why this is necessary would be good (and you mentioned adding one in your response to #2324).
There was a problem hiding this comment.
Hrm, yeah. The single worst idea perpetuated by most languages is attaching eq to a type / not making it easy to detach it from a type. I think the explanation will make ~no sense there since it needs to refer to usages it should know nothing about. I'll re-work so the caller ~supplies their own eq.
There was a problem hiding this comment.
Ok, fixed. Now doing the right thing. Thanks for keeping me honest.
| @@ -1,4 +1,4 @@ | |||
| # Copyright 2015 Pants project contributors (see CONTRIBUTORS.md). | |||
| # Licensed under the Apache License, Version 2.0 (see LICENSE). | |||
|
|
|||
| __version__ = "2.1.156" | |||
| __version__ = "2.1.157" | |||
There was a problem hiding this comment.
Wouldn't you normally bump this (and edit the CHANGES) in a separate release prep PR? Is this a rebasing error, or deliberate?
There was a problem hiding this comment.
This is deliberate. I paused this time and asked myself if there was a good reason to keep these separate, and in the case of a single change release, I couldn't think of a good enough reason.
Previously, a
pex3 lock updatewould fail whenever bystander projects(those projects in the lock but not targeted for update via
-p) had ansdist primary artifact.
Fixes #2324