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

Fix pex3 lock export: re-use --lock resolver. #1831

Merged
merged 3 commits into from
Jul 9, 2022

Conversation

jsirois
Copy link
Member

@jsirois jsirois commented Jul 8, 2022

Previously the lock export utility was using out-dated logic from the
early days of the lock project. Extract a subset operation from the
lock resolver and have it and lock export share that code.

Also properly plumb lock export target selection and validation of
exactly one target with updated help and error messages to make it clear
that locks can only be exported for a single target.

Fixes #1826
Closes #1645

Previously the lock export utility was using out-dated logic from the
early days of the lock project. Extract a `subset` operation from the
lock resolver and have it and lock export share that code.

Also properly plumb lock export target selection and validation of
exactly one target with updated help and error messages to make it clear
that locks can only be exported for a single target.

Fixes pex-tool#1826
Closes pex-tool#1645
@jsirois jsirois requested review from stuhood and asherf July 8, 2022 03:34
@jsirois jsirois marked this pull request as ready for review July 8, 2022 03:34
):
# type: (...) -> Union[SubsetResult, Error]

with TRACER.timed("Parsing requirements"):
Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -123,39 +115,3 @@ def extract_requirement(req):
locked_resolves = attr.ib() # type: SortedTuple[LockedResolve]
local_project_requirement_mapping = attr.ib(eq=False) # type: Mapping[str, Requirement]
source = attr.ib(default=None, eq=False) # type: Optional[str]

def select(self, targets):
Copy link
Member Author

Choose a reason for hiding this comment

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

This was the entry-point to the out-dated lock selection logic export (and update) were using. Now all removed.

update_requests = [
ResolveUpdateRequest(target=target, locked_resolve=locked_resolve)
for target, locked_resolve in lock_file.select(targets.unique_targets())
ResolveUpdateRequest(
Copy link
Member Author

@jsirois jsirois Jul 8, 2022

Choose a reason for hiding this comment

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

<- And this was the flip-over from the out-dated lock selection logic for lock update which came along for the ride so I could nuke the outdated logic wholesale. The existing update test still pass, vetting this.

@jsirois jsirois requested a review from benjyw July 8, 2022 16:46
@jsirois
Copy link
Member Author

jsirois commented Jul 8, 2022

Heads up that I'll need 1 more review. Asher took a look, but I'll need maintainer sign off too.

@jsirois jsirois merged commit 4418ad0 into pex-tool:main Jul 9, 2022
@jsirois jsirois deleted the issues/1826 branch July 9, 2022 10:55
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.

pex3 lock export does't seem to respect the platform flag. Clarify pex3 lock export command.
3 participants