Skip to content

Commit

Permalink
[internal] Better error message for ./pants repl when using multipl…
Browse files Browse the repository at this point in the history
…e resolves (#14327)

First part of #14295. Finishes Python - JVM still needs it.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Feb 5, 2022
1 parent 7e31f4b commit c2f6404
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 31 deletions.
46 changes: 43 additions & 3 deletions src/python/pants/backend/python/goals/repl.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

import os
from typing import Iterable

from pants.backend.python.subsystems.ipython import IPython
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import PythonResolveField
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.local_dists import LocalDistsPex, LocalDistsPexRequest
from pants.backend.python.util_rules.pex import Pex, PexRequest
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 (
Expand All @@ -18,17 +25,48 @@
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
from pants.engine.target import TransitiveTargets, TransitiveTargetsRequest
from pants.engine.target import Target, TransitiveTargets, TransitiveTargetsRequest
from pants.engine.unions import UnionRule
from pants.util.logging import LogLevel


def validate_compatible_resolve(root_targets: Iterable[Target], python_setup: PythonSetup) -> None:
"""Eagerly validate that all roots are compatible.
We already end up checking this in pex_from_targets.py, but this is a more eager check so that
we have a better error message.
"""
root_resolves = {
root[PythonResolveField].normalized_value(python_setup)
for root in root_targets
if root.has_field(PythonResolveField)
}
if len(root_resolves) > 1:
raise NoCompatibleResolveException(
python_setup,
"The input targets did not have a resolve in common",
root_targets,
(
"To work around this, choose which resolve you want to use from above. "
'Then, run `./pants peek :: | jq -r \'.[] | select(.resolve == "example") | '
'.["address"]\' | xargs ./pants repl`, where you replace "example" with the '
"resolve name, and possibly replace the specs `::` with what you were using "
"before. This will result in opening a REPL with only targets using the desired "
"resolve."
),
)


class PythonRepl(ReplImplementation):
name = "python"


@rule(level=LogLevel.DEBUG)
async def create_python_repl_request(request: PythonRepl, pex_env: PexEnvironment) -> ReplRequest:
async def create_python_repl_request(
request: PythonRepl, pex_env: PexEnvironment, python_setup: PythonSetup
) -> ReplRequest:
validate_compatible_resolve(request.targets, python_setup)

interpreter_constraints, transitive_targets = await MultiGet(
Get(InterpreterConstraints, InterpreterConstraintsRequest(request.addresses)),
Get(TransitiveTargets, TransitiveTargetsRequest(request.addresses)),
Expand Down Expand Up @@ -79,8 +117,10 @@ class IPythonRepl(ReplImplementation):

@rule(level=LogLevel.DEBUG)
async def create_ipython_repl_request(
request: IPythonRepl, ipython: IPython, pex_env: PexEnvironment
request: IPythonRepl, ipython: IPython, pex_env: PexEnvironment, python_setup: PythonSetup
) -> ReplRequest:
validate_compatible_resolve(request.targets, python_setup)

interpreter_constraints, transitive_targets = await MultiGet(
Get(InterpreterConstraints, InterpreterConstraintsRequest(request.addresses)),
Get(TransitiveTargets, TransitiveTargetsRequest(request.addresses)),
Expand Down
45 changes: 37 additions & 8 deletions src/python/pants/backend/python/goals/repl_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,29 @@

from __future__ import annotations

from textwrap import dedent

import pytest

from pants.backend.codegen.protobuf.target_types import ProtobufSourceTarget
from pants.backend.python.goals import repl as python_repl
from pants.backend.python.subsystems.ipython import rules as ipython_subsystem_rules
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import PythonSourcesGeneratorTarget
from pants.backend.python.target_types import PythonSourcesGeneratorTarget, PythonSourceTarget
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.repl import Repl
from pants.core.goals.repl import rules as repl_rules
from pants.engine.process import Process
from pants.testutil.python_interpreter_selection import all_major_minor_python_versions
from pants.testutil.rule_runner import GoalRuleResult, QueryRule, RuleRunner, mock_console
from pants.testutil.rule_runner import (
GoalRuleResult,
QueryRule,
RuleRunner,
engine_error,
mock_console,
)


@pytest.fixture
Expand All @@ -30,7 +39,7 @@ def rule_runner() -> RuleRunner:
*local_dists.rules(),
QueryRule(Process, (PexProcess,)),
],
target_types=[PythonSourcesGeneratorTarget, ProtobufSourceTarget],
target_types=[PythonSourcesGeneratorTarget, ProtobufSourceTarget, PythonSourceTarget],
)
rule_runner.write_files(
{
Expand All @@ -45,21 +54,23 @@ def rule_runner() -> RuleRunner:
return rule_runner


def run_repl(rule_runner: RuleRunner, *, extra_args: list[str] | None = None) -> GoalRuleResult:
def run_repl(
rule_runner: RuleRunner, args: list[str], *, global_args: list[str] | None = None
) -> GoalRuleResult:
# TODO(#9108): Expand `mock_console` to allow for providing input for the repl to verify
# that, e.g., the generated protobuf code is available. Right now this test prepares for
# that by including generated code, but cannot actually verify it.
with mock_console(rule_runner.options_bootstrapper):
return rule_runner.run_goal_rule(
Repl,
global_args=extra_args or (),
args=["src/python/lib.py"],
global_args=global_args or (),
args=args,
env_inherit={"PATH", "PYENV_ROOT", "HOME"},
)


def test_default_repl(rule_runner: RuleRunner) -> None:
assert run_repl(rule_runner).exit_code == 0
assert run_repl(rule_runner, ["src/python/lib.py"]).exit_code == 0


@pytest.mark.platform_specific_behavior
Expand All @@ -71,10 +82,28 @@ def test_ipython(rule_runner: RuleRunner, major_minor_interpreter: str) -> None:
assert (
run_repl(
rule_runner,
extra_args=[
["src/python/lib.py"],
global_args=[
"--repl-shell=ipython",
f"--python-interpreter-constraints=['=={major_minor_interpreter}.*']",
],
).exit_code
== 0
)


def test_eagerly_validate_roots_have_common_resolve(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"BUILD": dedent(
"""\
python_source(name='t1', source='f.py', resolve='a')
python_source(name='t2', source='f.py', resolve='b')
"""
)
}
)
with engine_error(NoCompatibleResolveException, contains="./pants peek"):
run_repl(
rule_runner, ["//:t1", "//:t2"], global_args=["--python-resolves={'a': '', 'b': ''}"]
)
33 changes: 13 additions & 20 deletions src/python/pants/backend/python/util_rules/pex_from_targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,11 @@ 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]
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:
Expand All @@ -224,22 +228,14 @@ def __init__(
"Targets which will be used together must all have the same resolve (from the "
f"[resolve]({doc_url('reference-python_test#coderesolvecode')}) or "
f"[compatible_resolves]({doc_url('reference-python_requirement#codecompatible_resolvescode')}) "
"fields) in common."
"fields) in common." + (f"\n\n{msg_suffix}" if msg_suffix else "")
)


@rule
async def choose_python_resolve(
request: ChosenPythonResolveRequest, python_setup: PythonSetup
) -> ChosenPythonResolve:
# If there are no targets, we fall back to the default resolve. This is relevant,
# for example, when running `./pants repl` with no specs.
if not request.addresses:
return ChosenPythonResolve(
name=python_setup.default_resolve,
lockfile_path=python_setup.resolves[python_setup.default_resolve],
)

transitive_targets = await Get(TransitiveTargets, TransitiveTargetsRequest(request.addresses))

# First, choose the resolve by inspecting the root targets.
Expand All @@ -249,17 +245,14 @@ async def choose_python_resolve(
if root.has_field(PythonResolveField)
}
if not root_resolves:
root_targets = bullet_list(
f"{tgt.address.spec} ({tgt.alias})" for tgt in transitive_targets.roots
)
raise AssertionError(
"Used `ChosenPythonResolveRequest` with input addresses that don't have the "
f"`PythonResolveField` field registered:\n\n{root_targets}\n\n"
"If you encountered this bug while using core Pants functionality, please open a "
"bug at https://github.com/pantsbuild/pants/issues/new with this error message when "
"`--print-stacktrace` is enabled. If this is from your own plugin, register "
"`PythonResolveField` on the relevant target types."
# If there are no targets, we fall back to the default resolve. This is relevant,
# for example, when running `./pants repl` with no specs or directly on
# python_requirement targets.
return ChosenPythonResolve(
name=python_setup.default_resolve,
lockfile_path=python_setup.resolves[python_setup.default_resolve],
)

if len(root_resolves) > 1:
raise NoCompatibleResolveException(
python_setup,
Expand Down

0 comments on commit c2f6404

Please sign in to comment.