Skip to content

Commit 9c0bbba

Browse files
committed
set find-links per resolve, not globally
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 d250c80 commit 9c0bbba

File tree

3 files changed

+62
-4
lines changed

3 files changed

+62
-4
lines changed

docs/notes/2.31.x.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ pantsd now preserves web proxy environment variables (`HTTP_PROXY`, `HTTPS_PROXY
2727

2828
### Goals
2929

30+
#### `generate-lockfiles`
31+
32+
Previously if any Python resolve set `find-links`, then *all* Python resolves used `find-links` during lockfile generation. Notably this included the `find-links` automatically injected by `pants.backend.plugin_development`. In Pants 2.31, `find-links` are correctly used per resolve.
33+
34+
Having extraneous un-scoped `find-links` can materially affect dependency resolution time. In some real world user report >30% improvements in `generate-lockfiles` time.
35+
3036
### Backends
3137

3238
#### Helm

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

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

336336
resolve_to_requirements_fields = defaultdict(set)
337-
find_links: set[str] = set()
337+
resolve_to_find_links: dict[str, set[str]] = defaultdict(set)
338338
for tgt in all_targets:
339339
if not tgt.has_fields((PythonRequirementResolveField, PythonRequirementsField)):
340340
continue
341341
resolve = tgt[PythonRequirementResolveField].normalized_value(python_setup)
342342
resolve_to_requirements_fields[resolve].add(tgt[PythonRequirementsField])
343-
find_links.update(tgt[PythonRequirementFindLinksField].value or ())
343+
resolve_to_find_links[resolve].update(tgt[PythonRequirementFindLinksField].value or ())
344344

345345
tools = ExportableTool.filter_for_subclasses(union_membership, PythonToolBase)
346346

@@ -352,7 +352,7 @@ async def setup_user_lockfile_requests(
352352
requirements=PexRequirements.req_strings_from_requirement_fields(
353353
resolve_to_requirements_fields[resolve]
354354
),
355-
find_links=FrozenOrderedSet(find_links),
355+
find_links=FrozenOrderedSet(resolve_to_find_links[resolve]),
356356
interpreter_constraints=InterpreterConstraints(
357357
python_setup.resolves_to_interpreter_constraints.get(
358358
resolve, python_setup.interpreter_constraints
@@ -381,7 +381,7 @@ async def setup_user_lockfile_requests(
381381
out.add(
382382
GeneratePythonLockfile(
383383
requirements=FrozenOrderedSet(sorted(tool.requirements)),
384-
find_links=FrozenOrderedSet(find_links),
384+
find_links=FrozenOrderedSet(),
385385
interpreter_constraints=ic,
386386
resolve_name=resolve,
387387
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)