From 2a300002f00d3099feca6805f48600cfbb2a6e12 Mon Sep 17 00:00:00 2001 From: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com> Date: Fri, 13 May 2022 12:45:41 -0500 Subject: [PATCH] Improve error message for `dependencies` on incompatible Python resolves (Cherry-pick of #15416) (#15440) Closes https://github.com/pantsbuild/pants/issues/14864. This code is written generically so that we can hook it up to JVM easily. [ci skip-rust] --- src/python/pants/backend/python/goals/repl.py | 33 +++-- .../python/goals/repl_integration_test.py | 2 +- .../python/util_rules/pex_from_targets.py | 60 +++------ .../util_rules/pex_from_targets_test.py | 39 +----- .../pants/core/goals/generate_lockfiles.py | 102 ++++++++++++++- .../core/goals/generate_lockfiles_test.py | 118 ++++++++++++++++++ 6 files changed, 258 insertions(+), 96 deletions(-) diff --git a/src/python/pants/backend/python/goals/repl.py b/src/python/pants/backend/python/goals/repl.py index 75debc14043..f85f44339f0 100644 --- a/src/python/pants/backend/python/goals/repl.py +++ b/src/python/pants/backend/python/goals/repl.py @@ -15,13 +15,13 @@ from pants.backend.python.util_rules.pex_environment import PexEnvironment from pants.backend.python.util_rules.pex_from_targets import ( InterpreterConstraintsRequest, - NoCompatibleResolveException, RequirementsPexRequest, ) from pants.backend.python.util_rules.python_sources import ( PythonSourceFiles, PythonSourceFilesRequest, ) +from pants.core.goals.generate_lockfiles import NoCompatibleResolveException from pants.core.goals.repl import ReplImplementation, ReplRequest from pants.engine.fs import Digest, MergeDigests from pants.engine.rules import Get, MultiGet, collect_rules, rule @@ -29,6 +29,7 @@ from pants.engine.unions import UnionRule from pants.util.docutil import bin_name from pants.util.logging import LogLevel +from pants.util.strutil import softwrap def validate_compatible_resolve(root_targets: Iterable[Target], python_setup: PythonSetup) -> None: @@ -42,19 +43,27 @@ def validate_compatible_resolve(root_targets: Iterable[Target], python_setup: Py for root in root_targets if root.has_field(PythonResolveField) } + + def maybe_get_resolve(t: Target) -> str | None: + if not t.has_field(PythonResolveField): + return None + return t[PythonResolveField].normalized_value(python_setup) + if len(root_resolves) > 1: - raise NoCompatibleResolveException( - python_setup, - "The input targets did not have a resolve in common", + raise NoCompatibleResolveException.bad_input_roots( root_targets, - ( - "To work around this, choose which resolve you want to use from above. " - f'Then, run `{bin_name()} peek :: | jq -r \'.[] | select(.resolve == "example") | ' - f'.["address"]\' | xargs {bin_name()} repl`, where you replace "example" with the ' - "resolve name, and possibly replace the specs `::` with what you were using " - "before. If the resolve is the `[python].default_resolve`, use " - '`select(.resolve == "example" or .resolve == null)`. These queries will result in ' - "opening a REPL with only targets using the desired resolve." + maybe_get_resolve=maybe_get_resolve, + doc_url_slug="python-third-party-dependencies#multiple-lockfiles", + workaround=softwrap( + f""" + To work around this, choose which resolve you want to use from above. Then, run + `{bin_name()} peek :: | jq -r \'.[] | select(.resolve == "example") | + .["address"]\' | xargs {bin_name()} repl`, where you replace "example" with the + resolve name, and possibly replace the specs `::` with what you were using + before. If the resolve is the `[python].default_resolve`, use + `select(.resolve == "example" or .resolve == null)`. These queries will result in + opening a REPL with only targets using the desired resolve. + """ ), ) diff --git a/src/python/pants/backend/python/goals/repl_integration_test.py b/src/python/pants/backend/python/goals/repl_integration_test.py index 55ac85b031e..8cf4101f874 100644 --- a/src/python/pants/backend/python/goals/repl_integration_test.py +++ b/src/python/pants/backend/python/goals/repl_integration_test.py @@ -19,7 +19,7 @@ from pants.backend.python.target_types_rules import rules as target_types_rules from pants.backend.python.util_rules import local_dists, pex_from_targets from pants.backend.python.util_rules.pex import PexProcess -from pants.backend.python.util_rules.pex_from_targets import NoCompatibleResolveException +from pants.core.goals.generate_lockfiles import NoCompatibleResolveException from pants.core.goals.repl import Repl from pants.core.goals.repl import rules as repl_rules from pants.engine.process import Process diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets.py b/src/python/pants/backend/python/util_rules/pex_from_targets.py index 69d08c91e6f..bc793631127 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets.py @@ -5,7 +5,6 @@ import dataclasses import logging -from collections import defaultdict from dataclasses import dataclass from typing import Iterable @@ -44,6 +43,7 @@ StrippedPythonSourceFiles, ) from pants.backend.python.util_rules.python_sources import rules as python_sources_rules +from pants.core.goals.generate_lockfiles import NoCompatibleResolveException from pants.engine.addresses import Address, Addresses from pants.engine.collection import DeduplicatedCollection from pants.engine.fs import Digest, DigestContents, GlobMatchErrorBehavior, MergeDigests, PathGlobs @@ -52,7 +52,7 @@ from pants.util.docutil import doc_url from pants.util.logging import LogLevel from pants.util.meta import frozen_after_init -from pants.util.strutil import bullet_list, path_safe +from pants.util.strutil import path_safe logger = logging.getLogger(__name__) @@ -205,42 +205,17 @@ class ChosenPythonResolveRequest: addresses: Addresses -# Note: Inspired by `coursier_fetch.py`. -class NoCompatibleResolveException(Exception): - """No compatible resolve could be found for a set of targets.""" - - def __init__( - self, - python_setup: PythonSetup, - msg_prefix: str, - relevant_targets: Iterable[Target], - msg_suffix: str = "", - ) -> None: - resolves_to_addresses = defaultdict(list) - for tgt in relevant_targets: - if tgt.has_field(PythonResolveField): - resolve = tgt[PythonResolveField].normalized_value(python_setup) - resolves_to_addresses[resolve].append(tgt.address.spec) - - formatted_resolve_lists = "\n\n".join( - f"{resolve}:\n{bullet_list(sorted(addresses))}" - for resolve, addresses in sorted(resolves_to_addresses.items()) - ) - super().__init__( - f"{msg_prefix}:\n\n" - f"{formatted_resolve_lists}\n\n" - "Targets which will be used together must all have the same resolve (from the " - f"[resolve]({doc_url('reference-python_test#coderesolvecode')}) " - "field) in common." + (f"\n\n{msg_suffix}" if msg_suffix else "") - ) - - @rule async def choose_python_resolve( request: ChosenPythonResolveRequest, python_setup: PythonSetup ) -> ChosenPythonResolve: transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(request.addresses)) + def maybe_get_resolve(t: Target) -> str | None: + if not t.has_field(PythonResolveField): + return None + return t[PythonResolveField].normalized_value(python_setup) + # First, choose the resolve by inspecting the root targets. root_resolves = { root[PythonResolveField].normalized_value(python_setup) @@ -249,10 +224,11 @@ async def choose_python_resolve( } if root_resolves: if len(root_resolves) > 1: - raise NoCompatibleResolveException( - python_setup, - "The input targets did not have a resolve in common", + raise NoCompatibleResolveException.bad_input_roots( transitive_targets.roots, + maybe_get_resolve=maybe_get_resolve, + doc_url_slug="python-third-party-dependencies#multiple-lockfiles", + workaround=None, ) chosen_resolve = next(iter(root_resolves)) @@ -263,14 +239,12 @@ async def choose_python_resolve( tgt.has_field(PythonResolveField) and tgt[PythonResolveField].normalized_value(python_setup) != chosen_resolve ): - plural = ("s", "their") if len(transitive_targets.roots) > 1 else ("", "its") - raise NoCompatibleResolveException( - python_setup, - ( - f"The resolve chosen for the root target{plural[0]} was {chosen_resolve}, but " - f"some of {plural[1]} dependencies are not compatible with that resolve" - ), - transitive_targets.closure, + raise NoCompatibleResolveException.bad_dependencies( + maybe_get_resolve=maybe_get_resolve, + doc_url_slug="python-third-party-dependencies#multiple-lockfiles", + root_resolve=chosen_resolve, + root_targets=transitive_targets.roots, + dependencies=transitive_targets.dependencies, ) else: diff --git a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py index 8b02de25aa2..a61144986b8 100644 --- a/src/python/pants/backend/python/util_rules/pex_from_targets_test.py +++ b/src/python/pants/backend/python/util_rules/pex_from_targets_test.py @@ -15,14 +15,10 @@ from pants.backend.python import target_types_rules from pants.backend.python.subsystems import setuptools -from pants.backend.python.subsystems.setup import PythonSetup from pants.backend.python.subsystems.setuptools import Setuptools from pants.backend.python.target_types import ( PexLayout, - PythonRequirementsField, PythonRequirementTarget, - PythonResolveField, - PythonSourceField, PythonSourcesGeneratorTarget, PythonSourceTarget, PythonTestTarget, @@ -33,7 +29,6 @@ ChosenPythonResolve, ChosenPythonResolveRequest, GlobalRequirementConstraints, - NoCompatibleResolveException, PexFromTargetsRequest, ) from pants.backend.python.util_rules.pex_requirements import ( @@ -44,8 +39,8 @@ ) from pants.backend.python.util_rules.pex_test_utils import get_all_data from pants.build_graph.address import Address +from pants.core.goals.generate_lockfiles import NoCompatibleResolveException from pants.engine.addresses import Addresses -from pants.testutil.option_util import create_subsystem from pants.testutil.rule_runner import QueryRule, RuleRunner, engine_error from pants.util.contextutil import pushd from pants.util.ordered_set import OrderedSet @@ -73,38 +68,6 @@ def rule_runner() -> RuleRunner: ) -def test_no_compatible_resolve_error() -> None: - python_setup = create_subsystem(PythonSetup, resolves={"a": "", "b": ""}, enable_resolves=True) - targets = [ - PythonRequirementTarget( - {PythonRequirementsField.alias: [], PythonResolveField.alias: "a"}, - Address("", target_name="t1"), - ), - PythonSourceTarget( - {PythonSourceField.alias: "f.py", PythonResolveField.alias: "a"}, - Address("", target_name="t2"), - ), - PythonSourceTarget( - {PythonSourceField.alias: "f.py", PythonResolveField.alias: "b"}, - Address("", target_name="t3"), - ), - ] - assert str(NoCompatibleResolveException(python_setup, "Prefix", targets)).startswith( - dedent( - """\ - Prefix: - - a: - * //:t1 - * //:t2 - - b: - * //:t3 - """ - ) - ) - - def test_choose_compatible_resolve(rule_runner: RuleRunner) -> None: def create_target_files( directory: str, *, req_resolve: str, source_resolve: str, test_resolve: str diff --git a/src/python/pants/core/goals/generate_lockfiles.py b/src/python/pants/core/goals/generate_lockfiles.py index a2a024c1c99..76c6c682569 100644 --- a/src/python/pants/core/goals/generate_lockfiles.py +++ b/src/python/pants/core/goals/generate_lockfiles.py @@ -8,16 +8,18 @@ from collections import defaultdict from dataclasses import dataclass from enum import Enum -from typing import ClassVar, Iterable, Sequence +from typing import Callable, ClassVar, Iterable, Sequence from pants.engine.collection import Collection from pants.engine.fs import Digest, MergeDigests, Workspace from pants.engine.goal import Goal, GoalSubsystem from pants.engine.internals.selectors import Get, MultiGet from pants.engine.rules import collect_rules, goal_rule +from pants.engine.target import Target from pants.engine.unions import UnionMembership, union from pants.option.option_types import StrListOption, StrOption -from pants.util.docutil import bin_name +from pants.util.docutil import bin_name, doc_url +from pants.util.strutil import bullet_list, softwrap logger = logging.getLogger(__name__) @@ -373,5 +375,101 @@ async def generate_lockfiles_goal( return GenerateLockfilesGoal(exit_code=0) +# ----------------------------------------------------------------------------------------------- +# Helpers for determining the resolve +# ----------------------------------------------------------------------------------------------- + + +class NoCompatibleResolveException(Exception): + """No compatible resolve could be found for a set of targets.""" + + @classmethod + def bad_input_roots( + cls, + targets: Iterable[Target], + *, + maybe_get_resolve: Callable[[Target], str | None], + doc_url_slug: str, + workaround: str | None, + ) -> NoCompatibleResolveException: + resolves_to_addresses = defaultdict(list) + for tgt in targets: + maybe_resolve = maybe_get_resolve(tgt) + if maybe_resolve is not None: + resolves_to_addresses[maybe_resolve].append(tgt.address.spec) + + formatted_resolve_lists = "\n\n".join( + f"{resolve}:\n{bullet_list(sorted(addresses))}" + for resolve, addresses in sorted(resolves_to_addresses.items()) + ) + return NoCompatibleResolveException( + f"The input targets did not have a resolve in common.\n\n" + f"{formatted_resolve_lists}\n\n" + "Targets used together must use the same resolve, set by the `resolve` field. For more " + f"information on 'resolves' (lockfiles), see {doc_url(doc_url_slug)}." + + (f"\n\n{workaround}" if workaround else "") + ) + + @classmethod + def bad_dependencies( + cls, + *, + maybe_get_resolve: Callable[[Target], str | None], + doc_url_slug: str, + root_resolve: str, + root_targets: Sequence[Target], + dependencies: Iterable[Target], + ) -> NoCompatibleResolveException: + if len(root_targets) == 1: + addr = root_targets[0].address + prefix = softwrap( + f""" + The target {addr} uses the `resolve` `{root_resolve}`, but some of its + dependencies are not compatible with that resolve: + """ + ) + change_input_targets_instructions = f"of the target {addr}" + else: + assert root_targets + prefix = softwrap( + f""" + The input targets use the `resolve` `{root_resolve}`, but some of their + dependencies are not compatible with that resolve. + + Input targets: + + {bullet_list(sorted(t.address.spec for t in root_targets))} + + Bad dependencies: + """ + ) + change_input_targets_instructions = "of the input targets" + + deps_strings = [] + for dep in dependencies: + maybe_resolve = maybe_get_resolve(dep) + if maybe_resolve is None or maybe_resolve == root_resolve: + continue + deps_strings.append(f"{dep.address} ({maybe_resolve})") + + return NoCompatibleResolveException( + softwrap( + f""" + {prefix} + + {bullet_list(deps_strings)} + + All dependencies must work with the same `resolve`. To fix this, either change + the `resolve=` field on those dependencies to `{root_resolve}`, or change + the `resolve=` {change_input_targets_instructions}. If those dependencies should + work with multiple resolves, use the `parametrize` mechanism with the `resolve=` + field or manually create multiple targets for the same entity. + + For more information, see {doc_url(doc_url_slug)}. + """ + ) + ) + + def rules(): return collect_rules() diff --git a/src/python/pants/core/goals/generate_lockfiles_test.py b/src/python/pants/core/goals/generate_lockfiles_test.py index 3ea6cb58bb9..35b421d6ab0 100644 --- a/src/python/pants/core/goals/generate_lockfiles_test.py +++ b/src/python/pants/core/goals/generate_lockfiles_test.py @@ -5,6 +5,15 @@ import pytest +from pants.backend.python.subsystems.setup import PythonSetup +from pants.backend.python.target_types import ( + PythonRequirementsField, + PythonRequirementTarget, + PythonResolveField, + PythonSourceField, + PythonSourceTarget, +) +from pants.build_graph.address import Address from pants.core.goals.generate_lockfiles import ( DEFAULT_TOOL_LOCKFILE, NO_TOOL_LOCKFILE, @@ -12,12 +21,16 @@ GenerateLockfile, GenerateToolLockfileSentinel, KnownUserResolveNames, + NoCompatibleResolveException, RequestedUserResolveNames, UnrecognizedResolveNamesError, WrappedGenerateLockfile, determine_resolves_to_generate, filter_tool_lockfile_requests, ) +from pants.engine.target import Dependencies, Target +from pants.testutil.option_util import create_subsystem +from pants.util.strutil import softwrap def test_determine_tool_sentinels_to_generate() -> None: @@ -148,3 +161,108 @@ def assert_filtered( assert f"`[{default_tool.resolve_name}].lockfile` is set to `{DEFAULT_TOOL_LOCKFILE}`" in str( exc.value ) + + +def test_no_compatible_resolve_error() -> None: + python_setup = create_subsystem(PythonSetup, resolves={"a": "", "b": ""}, enable_resolves=True) + t1 = PythonRequirementTarget( + { + PythonRequirementsField.alias: [], + PythonResolveField.alias: "a", + Dependencies.alias: ["//:t3"], + }, + Address("", target_name="t1"), + ) + t2 = PythonSourceTarget( + { + PythonSourceField.alias: "f.py", + PythonResolveField.alias: "a", + Dependencies.alias: ["//:t3"], + }, + Address("", target_name="t2"), + ) + t3 = PythonSourceTarget( + {PythonSourceField.alias: "f.py", PythonResolveField.alias: "b"}, + Address("", target_name="t3"), + ) + + def maybe_get_resolve(t: Target) -> str | None: + if not t.has_field(PythonResolveField): + return None + return t[PythonResolveField].normalized_value(python_setup) + + bad_roots_err = str( + NoCompatibleResolveException.bad_input_roots( + [t2, t3], maybe_get_resolve=maybe_get_resolve, doc_url_slug="", workaround=None + ) + ) + assert bad_roots_err.startswith( + softwrap( + """ + The input targets did not have a resolve in common. + + a: + * //:t2 + + b: + * //:t3 + + Targets used together must use the same resolve, set by the `resolve` field. + """ + ) + ) + + bad_single_dep_err = str( + NoCompatibleResolveException.bad_dependencies( + maybe_get_resolve=maybe_get_resolve, + doc_url_slug="", + root_targets=[t1], + root_resolve="a", + dependencies=[t3], + ) + ) + assert bad_single_dep_err.startswith( + softwrap( + """ + The target //:t1 uses the `resolve` `a`, but some of its + dependencies are not compatible with that resolve: + + * //:t3 (b) + + All dependencies must work with the same `resolve`. To fix this, either change + the `resolve=` field on those dependencies to `a`, or change + the `resolve=` of the target //:t1. + """ + ) + ) + + bad_multiple_deps_err = str( + NoCompatibleResolveException.bad_dependencies( + maybe_get_resolve=maybe_get_resolve, + doc_url_slug="", + root_targets=[t1, t2], + root_resolve="a", + dependencies=[t3], + ) + ) + assert bad_multiple_deps_err.startswith( + softwrap( + """ + The input targets use the `resolve` `a`, but some of their + dependencies are not compatible with that resolve. + + Input targets: + + * //:t1 + * //:t2 + + Bad dependencies: + + * //:t3 (b) + + All dependencies must work with the same `resolve`. To fix this, either change + the `resolve=` field on those dependencies to `a`, or change + the `resolve=` of the input targets. + """ + ) + )