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 lockfile generation for shadowed resolves #21642

Merged
merged 4 commits into from
Nov 20, 2024

Conversation

lilatomic
Copy link
Contributor

When a user resolve shadows a tool resolve, pants generate-lockfiles will generate the lockfile twice.
This is because only the --request arg is deduplicated. Without one specified, all known resolves are added to a non-deduplicated collection. The resolves are added separately for each backend and ExportableTool. This can introduce duplicates into this list. And, although the shadowing is correct, the resolve is exported twice. For example, the tool Black and a user resolve named "black" will both use the same resolve name, resulting in a list of resolves to export like RequestedPythonUserResolveNames(['black', 'python-default', 'coverage-py', 'pytest', 'ipython', 'black']).

This MR makes 3 adjustments to this process:

  1. RequestedUserResolveNames is now a DeduplicatedCollection. This mitigates the most proximate issue of specifying the same resolve.
  2. Python deduplicates GeneratePythonLockfile. This may already be covered by 1, but can't hurt
  3. Python does not present tool resolves for tools that are using a user resolve. For example, if [black].install_from_resolve="linters", its default resolve "black" will not be exposed.

Note that in the case of shadowing without install_from_resolve, although the shadowed resolve is not surfaced (for export), Pants will still correctly use the default lockfile.

closes #21625

@lilatomic lilatomic added category:bugfix Bug fixes for released features backend: Python Python backend-related issues labels Nov 13, 2024
@lilatomic lilatomic added this to the 2.22.x milestone Nov 13, 2024
@lilatomic lilatomic requested review from kaos and huonw November 13, 2024 23:32
Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

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

Thank you!

(Let's merge this one but maybe wait for 2.23.0 to be released before merging the 2.23.x cherrypick PR.)

@@ -21,8 +21,16 @@ The "legacy" options system is removed in this release. All options parsing is n

### Goals

#### `generate-lockfiles`
Copy link
Contributor

Choose a reason for hiding this comment

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

oh, minor thing, but since we're cherrypicking this to 2.24, 2.24.0 will eb the first major release with this change so these notes could go in 2.24.x... but also it doesn't matter that much, I think.

for example, `black` exposes the resolve "black" so that `pants export --resolve=black` works.
 But if `[black].install_from_resolve` is set to "my_resolve",
we shouldn't expose the resolve "black"
the request is selected twice, but they're identical because they select by name.
We can just use a set to deduplicate them
A resolve name could be provided by both users and tools,
so it was possible to have the same requested name twice.

This fixes that by deduplicating them in the request.
@lilatomic lilatomic merged commit bc2aae3 into pantsbuild:main Nov 20, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Python Python backend-related issues category:bugfix Bug fixes for released features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

generate-lockfiles writes tool lockfiles twice
2 participants