Skip to content

Commit bebf4a0

Browse files
authored
set plugin development find-links only on the needed resolve (#23024)
As described at <#21223 (comment)>, if you set find-links (and that includes everyone with in-repo plugins by way of `pants.backend.plugin_development`) on one resolve, we were erroneously setting it for *all* resolves during lockfile generation. This materially impacts resolution time.
1 parent af698f0 commit bebf4a0

File tree

3 files changed

+58
-4
lines changed

3 files changed

+58
-4
lines changed

docs/notes/2.32.x.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ For those language ecosystems that use separate type checkers from compilers (So
6363

6464
For `generate-lockfiles`, typos in the name of a resolve now give "Did you mean?" style suggestions.
6565

66+
The `find-links` automatically injected by `pants.backend.plugin_development` is no longer injected into *all* Python resolves, only the resolve associated with `pants_requirements(name="pants")`. Having extraneous un-scoped `find-links` can materially affect dependency resolution time. In some real world user reports, this results in a >30% improvements in `generate-lockfiles` wall time.
67+
6668
#### Publish
6769

6870
`PublishFieldSet` now has a `check_skip_request` hook to enable preemptive skips of packaging requests for targets where the publishing will be skipped (such as when a `skip_push=True` field exists).

src/python/pants/backend/python/goals/lockfile.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,13 +361,13 @@ async def setup_user_lockfile_requests(
361361
return UserGenerateLockfiles()
362362

363363
resolve_to_requirements_fields = defaultdict(set)
364-
find_links: set[str] = set()
364+
resolve_to_find_links: dict[str, set[str]] = defaultdict(set)
365365
for tgt in all_targets:
366366
if not tgt.has_fields((PythonRequirementResolveField, PythonRequirementsField)):
367367
continue
368368
resolve = tgt[PythonRequirementResolveField].normalized_value(python_setup)
369369
resolve_to_requirements_fields[resolve].add(tgt[PythonRequirementsField])
370-
find_links.update(tgt[PythonRequirementFindLinksField].value or ())
370+
resolve_to_find_links[resolve].update(tgt[PythonRequirementFindLinksField].value or ())
371371

372372
tools = ExportableTool.filter_for_subclasses(union_membership, PythonToolBase)
373373

@@ -379,7 +379,7 @@ async def setup_user_lockfile_requests(
379379
requirements=PexRequirements.req_strings_from_requirement_fields(
380380
resolve_to_requirements_fields[resolve]
381381
),
382-
find_links=FrozenOrderedSet(find_links),
382+
find_links=FrozenOrderedSet(resolve_to_find_links[resolve]),
383383
interpreter_constraints=InterpreterConstraints(
384384
python_setup.resolves_to_interpreter_constraints.get(
385385
resolve, python_setup.interpreter_constraints
@@ -408,7 +408,7 @@ async def setup_user_lockfile_requests(
408408
out.add(
409409
GeneratePythonLockfile(
410410
requirements=FrozenOrderedSet(sorted(tool.requirements)),
411-
find_links=FrozenOrderedSet(find_links),
411+
find_links=FrozenOrderedSet(),
412412
interpreter_constraints=ic,
413413
resolve_name=resolve,
414414
lockfile_dest=DEFAULT_TOOL_LOCKFILE,

src/python/pants/backend/python/goals/lockfile_test.py

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,58 @@ def test_multiple_resolves() -> None:
341341
}
342342

343343

344+
def test_find_links_scoped_to_resolve() -> None:
345+
rule_runner = PythonRuleRunner(
346+
rules=[
347+
setup_user_lockfile_requests,
348+
*PythonSetup.rules(),
349+
QueryRule(UserGenerateLockfiles, [RequestedPythonUserResolveNames]),
350+
],
351+
target_types=[PythonRequirementTarget],
352+
)
353+
rule_runner.write_files(
354+
{
355+
"BUILD": dedent(
356+
"""\
357+
# Using the underscore field directly instead of pulling all of
358+
# pants.backend.plugin_development into the test
359+
python_requirement(
360+
name='a',
361+
requirements=['a'],
362+
resolve='a',
363+
_find_links=['https://example.com/wheels'],
364+
)
365+
python_requirement(
366+
name='b',
367+
requirements=['b'],
368+
resolve='b',
369+
)
370+
"""
371+
),
372+
}
373+
)
374+
rule_runner.set_options(
375+
[
376+
"--python-resolves={'a': 'a.lock', 'b': 'b.lock'}",
377+
"--python-enable-resolves",
378+
],
379+
env_inherit=PYTHON_BOOTSTRAP_ENV,
380+
)
381+
result = rule_runner.request(
382+
UserGenerateLockfiles, [RequestedPythonUserResolveNames(["a", "b"])]
383+
)
384+
assert all(isinstance(r, GeneratePythonLockfile) for r in result)
385+
result_by_resolve = {r.resolve_name: r for r in result}
386+
assert isinstance(result_by_resolve["a"], GeneratePythonLockfile)
387+
assert isinstance(result_by_resolve["b"], GeneratePythonLockfile)
388+
389+
assert result_by_resolve["a"].requirements == FrozenOrderedSet(["a"])
390+
assert result_by_resolve["b"].requirements == FrozenOrderedSet(["b"])
391+
392+
assert result_by_resolve["a"].find_links == FrozenOrderedSet(["https://example.com/wheels"])
393+
assert result_by_resolve["b"].find_links == FrozenOrderedSet([])
394+
395+
344396
def test_empty_requirements(rule_runner: PythonRuleRunner) -> None:
345397
with pytest.raises(ExecutionError) as excinfo:
346398
json.loads(

0 commit comments

Comments
 (0)