generate-lockfiles writes tool lockfiles twice

[kaos]

Describe the bug

❯ pants generate-lockfiles
11:18:36.09 [INFO] Initializing scheduler...
11:18:36.16 [INFO] Initializing Nailgun pool for 24 processes...
11:18:37.16 [INFO] Scheduler initialized.
11:18:48.46 [INFO] Completed: Generate lockfile for black
11:18:48.49 [INFO] Completed: Generate lockfile for python-default
11:18:48.49 [INFO] Wrote lockfile for the resolve `python-default` to default.lock
11:18:48.49 [INFO] Wrote lockfile for the resolve `black` to black.lock
11:18:48.49 [INFO] Wrote lockfile for the resolve `black` to black.lock

Lockfile diff: default.lock [python-default]

==                      Added dependencies                      ==

  certifi                        2024.8.30
  charset-normalizer             3.4.0
  idna                           3.10
  requests                       2.32.3
  urllib3                        2.2.3

Lockfile diff: black.lock [black]

==                      Added dependencies                      ==

  black                          24.10.0
  click                          8.1.7
  mypy-extensions                1.0.0
  packaging                      24.1
  pathspec                       0.12.1
  platformdirs                   4.3.6

Lockfile diff: black.lock [black]

==                      Added dependencies                      ==

  black                          24.10.0
  click                          8.1.7
  mypy-extensions                1.0.0
  packaging                      24.1
  pathspec                       0.12.1
  platformdirs                   4.3.6

Pants version
2.23.0rc2

OS
MacOS

Additional info
Repro, using txtar format.

-- BUILD --
python_requirements(name="default", source="requirements.txt")
python_requirements(name="black", source="black-requirements.txt", resolve="black")
-- black-requirements.txt --
black
-- pants.toml --
[GLOBAL]
pants_version = "2.23.0rc2"
backend_packages = [
  "pants.backend.python",
  "pants.backend.python.lint.black",
  
]

[python]
interpreter_constraints = ["==3.12.*"]
enable_resolves = true

[python.resolves]
python-default = "default.lock"
black = "black.lock"

[black]
install_from_resolve = "black"

-- requirements.txt --
requests

[huonw]

I can reproduce this.

Notes:

• it appears to be a regression in 2.22.x (reproduces in 2.22.0, but not 2.21.0)
• requires:
  • running with pants generate-lockfiles (i.e. all resolves), running with pants generate-lockfiles --resolve=black just writes it once
  • doesn't need to actually be used by the tool (deleting the [black].install_from_resolve option completely still reproduces)
  • requires the resolve to exactly match the tool: either commenting out the "pants.backend.python.lint.black" subsystem or renaming the resolve (e.g. black2) stops it reproducing

Minimal (executable) reproducer:

cd $(mktemp -d)

cat > pants.toml <<EOF
[GLOBAL]
pants_version = "2.22.0"
backend_packages = [
  "pants.backend.python",
  "pants.backend.python.lint.black",
]

[python]
interpreter_constraints = ["==3.12.*"]
enable_resolves = true

[python.resolves]
python-default = "default.lock"
black = "black.lock"
EOF

cat > BUILD <<EOF
python_requirement(name="black", requirements=["black"], resolve="black")
python_requirement(name="requests", requirements=["requests"])
EOF

echo "*** Okay: using --resolve=black => only written once"
pants generate-lockfiles --resolve=black

echo "*** BUG: written twice"
pants generate-lockfiles


echo "*** Okay: doesn't match a backend => only written once"
cat > pants.toml <<EOF
[GLOBAL]
pants_version = "2.22.0"
backend_packages = [
  "pants.backend.python",
]

[python]
interpreter_constraints = ["==3.12.*"]
enable_resolves = true

[python.resolves]
python-default = "default.lock"
black = "black.lock"
EOF
pants generate-lockfiles

Output (with my trimming):

*** Okay: using --resolve=black => only written once
...
09:40:03.64 [INFO] Wrote lockfile for the resolve `black` to black.lock                                                                  
...


*** BUG: written twice
...
09:40:06.06 [INFO] Wrote lockfile for the resolve `python-default` to default.lock
09:40:06.06 [INFO] Wrote lockfile for the resolve `black` to black.lock
09:40:06.06 [INFO] Wrote lockfile for the resolve `black` to black.lock
                                                                  
...


*** Okay: doesn't match a backend => only written once
...
09:40:10.99 [INFO] Wrote lockfile for the resolve `python-default` to default.lock
09:40:10.99 [INFO] Wrote lockfile for the resolve `black` to black.lock

[huonw]

Bisecting on the release versions suggests this regressed between 2.22.0.dev2 (writes once) and 2.22.0.dev3 (writes twice).

Diff: release_2.22.0.dev2...release_2.22.0.dev3

Skimming that list makes me wonder about 20f3924 / #20787 being the cause, but I have not confirmed this.

[kaos]

Skimming that list makes me wonder about 20f3924 / #20787 being the cause, but I have not confirmed this.

I can confirm this suspicion:

# first bad commit: [20f39242575e561a3b7a3be96e5d5268846b3055] Mark all Python tools as exportable (#20787)

[lilatomic]

There's a codepath that's supposed to implement the shadowing

pants/src/python/pants/core/goals/generate_lockfiles.py

Lines 407 to 409 in 6afd222

	# When a user selects no resolves, we try generating lockfiles for all resolves.
	# The intention is clearly to not generate lockfiles for internal tools, so we skip them here.
	continue

[lilatomic]

The shadowing codepath isn't working because both the user resolve and the tool resolve have the same lockfiles path, even though [black].install_from_resolve isn't set to the user resolve. The automatic assignment to the user lockfile (which I think is the root of the bug) is done in setup_user_lockfile_requests. This function operates on names only, and since the black subsystem is expecting a resolve named "black" it uses the user resolve.

Doing some more thinking, there are actually a few bugs here:

1. RequestedUserResolveNames is not deduplicated. If a resolve name is provided by multiple sources (eg Python and tools), it will be exported multiple times.
2. subsystems do not consider install_from_resolve when exposing their tool lockfile. If they're using their default lockfile, they shouldn't expose it.
3. Python user and tool resolves are not properly deduplicated