Skip to content

Optimize pure delete lock updates. #2569

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

Merged
merged 1 commit into from
Oct 24, 2024
Merged

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Oct 23, 2024

When a lock update only asks for deletes, we now process the "resolve"
offline without consulting Pip.

Fixes #2566

When a lock update only asks for deletes, we now process the "resolve"
offline without consulting Pip.

Fixes pex-tool#2566
@jsirois jsirois requested a review from benjyw October 23, 2024 20:58
@jsirois
Copy link
Member Author

jsirois commented Oct 23, 2024

N.B.: Existing tests cover all the interesting delete cases, including transitive dependencies of deleted node being partially retained when something else depends on those, and they pass.

Here proj-b 1 depends on proj-a and proj-c:

def test_lock_update_mixed(
run_lock_update, # type: RunLockUpdate
lock, # type: str
path_mappings, # type: PathMappings
):
# type: (...) -> None
run_lock_update("-p", "proj-a", "-d", "proj-b").assert_success(
expected_error_re=re_exact(
dedent(
"""\
Updates for lock generated by universal:
Deleted proj-b 1
Deleted proj-c 1
Updated proj-a from 1.1 to 2
Updates to lock input requirements:
Deleted 'proj-b==1.*'
"""
)
)
)
lockfile = json_codec.load(lock, path_mappings)
locked_requirements = locked_requirement_pins(lockfile)
assert {pin("proj-a", "2"), pin("ansicolors", "1.1.8")} == locked_requirements

Copy link
Collaborator

@benjyw benjyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@jsirois jsirois merged commit 386af2e into pex-tool:main Oct 24, 2024
23 checks passed
@jsirois jsirois deleted the issues/2566 branch October 24, 2024 01:10
jsirois added a commit to jsirois/pex that referenced this pull request Oct 25, 2024
I claimed there was already one of these in pex-tool#2569 but I was wrong
because the existing complex case test include an update; so is not a
pure delete.

Add a new test of the pure delete case where the deleted node's graph
is partially retained.
@jsirois
Copy link
Member Author

jsirois commented Oct 25, 2024

My claim of an existing complex delete case in #2569 (comment) was a lie, only simple deletes are tested through the new code path. That case has a delete and an update; so does not use the new code path and uses Pip instead. I've added a complex pure delete test case in #2576. It passes with no changes; so I'll process with the release before landing that change.

jsirois added a commit that referenced this pull request Oct 25, 2024
I claimed there was already one of these in #2569 but I was wrong
because the existing complex case test include an update; so is not a
pure delete.

Add a new test of the pure delete case where the deleted node's graph
is partially retained.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Special case lock updates that are pure delete.
2 participants