Skip to content

Commit

Permalink
Remove the restriction on InteractiveProcess inputs, and use native c…
Browse files Browse the repository at this point in the history
…hroots.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
stuhood committed Jul 8, 2022
1 parent 088fffb commit 530bd42
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 110 deletions.
15 changes: 5 additions & 10 deletions src/python/pants/backend/scala/goals/repl.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# Copyright 2021 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

from pants.backend.scala.subsystems.scala import ScalaSubsystem
Expand Down Expand Up @@ -66,32 +67,26 @@ async def create_scala_repl_request(
Get(Digest, AddPrefix(d, user_classpath_prefix)) for d in user_classpath.digests()
)

# TODO: Manually merging the `immutable_input_digests` since InteractiveProcess doesn't
# support them yet. See https://github.com/pantsbuild/pants/issues/13852.
jdk_digests = await MultiGet(
Get(Digest, AddPrefix(digest, relpath))
for relpath, digest in jdk.immutable_input_digests.items()
)

repl_digest = await Get(
Digest,
MergeDigests([*prefixed_user_classpath, tool_classpath.content.digest, *jdk_digests]),
MergeDigests([*prefixed_user_classpath, tool_classpath.content.digest]),
)

return ReplRequest(
digest=repl_digest,
args=[
*jdk.args(bash, tool_classpath.classpath_entries()),
*jdk.args(bash, tool_classpath.classpath_entries(), chroot="{chroot}"),
"-Dscala.usejavacp=true",
"scala.tools.nsc.MainGenericRunner",
"-classpath",
":".join(user_classpath.args(prefix=user_classpath_prefix)),
],
run_in_workspace=False,
extra_env={
**jdk.env,
"PANTS_INTERNAL_ABSOLUTE_PREFIX": "",
},
run_in_workspace=False,
immutable_input_digests=jdk.immutable_input_digests,
append_only_caches=jdk.append_only_caches,
)

Expand Down
52 changes: 20 additions & 32 deletions src/python/pants/core/goals/repl.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import os
from abc import ABC
from dataclasses import dataclass
from pathlib import PurePath
from typing import ClassVar, Iterable, Mapping, Optional, Sequence, Tuple

from pants.base.build_root import BuildRoot
Expand All @@ -14,14 +13,12 @@
from pants.engine.environment import CompleteEnvironment
from pants.engine.fs import Digest, Workspace
from pants.engine.goal import Goal, GoalSubsystem
from pants.engine.internals.native_engine import EMPTY_DIGEST
from pants.engine.process import InteractiveProcess, InteractiveProcessResult
from pants.engine.rules import Effect, Get, collect_rules, goal_rule
from pants.engine.target import FilteredTargets, Target
from pants.engine.unions import UnionMembership, union
from pants.option.global_options import GlobalOptions
from pants.option.option_types import BoolOption, StrOption
from pants.util.contextutil import temporary_dir
from pants.util.frozendict import FrozenDict
from pants.util.memo import memoized_property
from pants.util.meta import frozen_after_init
Expand All @@ -38,10 +35,9 @@ class ReplImplementation(ABC):
name: ClassVar[str]

targets: Sequence[Target]
chroot: str # Absolute path of the chroot the sources will be materialized to.

def in_chroot(self, relpath: str) -> str:
return os.path.join(self.chroot, relpath)
return os.path.join("{chroot}", relpath)

@memoized_property
def addresses(self) -> Addresses:
Expand Down Expand Up @@ -76,6 +72,7 @@ class ReplRequest:
digest: Digest
args: Tuple[str, ...]
extra_env: FrozenDict[str, str]
immutable_input_digests: FrozenDict[str, Digest]
append_only_caches: FrozenDict[str, str]
run_in_workspace: bool

Expand All @@ -85,12 +82,14 @@ def __init__(
digest: Digest,
args: Iterable[str],
extra_env: Optional[Mapping[str, str]] = None,
immutable_input_digests: Mapping[str, Digest] | None = None,
append_only_caches: Mapping[str, str] | None = None,
run_in_workspace: bool = True,
) -> None:
self.digest = digest
self.args = tuple(args)
self.extra_env = FrozenDict(extra_env or {})
self.immutable_input_digests = FrozenDict(immutable_input_digests or {})
self.append_only_caches = FrozenDict(append_only_caches or {})
self.run_in_workspace = run_in_workspace

Expand Down Expand Up @@ -119,33 +118,22 @@ async def run_repl(
)
return Repl(-1)

with temporary_dir(root_dir=global_options.pants_workdir, cleanup=False) as tmpdir:
repl_impl = repl_implementation_cls(targets=specified_targets, chroot=tmpdir)
request = await Get(ReplRequest, ReplImplementation, repl_impl)

input_digest = request.digest
if request.run_in_workspace:
workspace.write_digest(
request.digest,
path_prefix=PurePath(tmpdir).relative_to(build_root.path).as_posix(),
# We don't want to influence whether the InteractiveProcess is able to restart. Because
# we're writing into a temp directory, we can safely mark this side_effecting=False.
side_effecting=False,
)
input_digest = EMPTY_DIGEST

env = {**complete_env, **request.extra_env}
result = await Effect(
InteractiveProcessResult,
InteractiveProcess(
argv=request.args,
env=env,
input_digest=input_digest,
run_in_workspace=request.run_in_workspace,
restartable=repl_subsystem.restartable,
append_only_caches=request.append_only_caches,
),
)
repl_impl = repl_implementation_cls(targets=specified_targets)
request = await Get(ReplRequest, ReplImplementation, repl_impl)

env = {**complete_env, **request.extra_env}
result = await Effect(
InteractiveProcessResult,
InteractiveProcess(
argv=request.args,
env=env,
input_digest=request.digest,
run_in_workspace=request.run_in_workspace,
restartable=repl_subsystem.restartable,
immutable_input_digests=request.immutable_input_digests,
append_only_caches=request.append_only_caches,
),
)
return Repl(result.exit_code)


Expand Down
54 changes: 20 additions & 34 deletions src/python/pants/core/goals/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import logging
from abc import ABCMeta
from dataclasses import dataclass
from pathlib import PurePath
from typing import Iterable, Mapping, Optional, Tuple

from pants.base.build_root import BuildRoot
Expand All @@ -25,7 +24,6 @@
from pants.engine.unions import UnionMembership, union
from pants.option.global_options import GlobalOptions
from pants.option.option_types import ArgsListOption, BoolOption
from pants.util.contextutil import temporary_dir
from pants.util.frozendict import FrozenDict
from pants.util.meta import frozen_after_init
from pants.util.strutil import softwrap
Expand Down Expand Up @@ -164,41 +162,29 @@ async def run(
# Cleanup is the default, so we want to preserve the chroot if either option is off.
cleanup = run_subsystem.cleanup and global_options.process_cleanup

with temporary_dir(root_dir=global_options.pants_workdir, cleanup=cleanup) as tmpdir:
if not cleanup:
logger.info(f"Preserving running binary chroot {tmpdir}")
workspace.write_digest(
request.digest,
path_prefix=PurePath(tmpdir).relative_to(build_root.path).as_posix(),
# We don't want to influence whether the InteractiveProcess is able to restart. Because
# we're writing into a temp directory, we can safely mark this side_effecting=False.
side_effecting=False,
)

args = (arg.format(chroot=tmpdir) for arg in request.args)
env = {**complete_env, **{k: v.format(chroot=tmpdir) for k, v in request.extra_env.items()}}
if run_subsystem.debug_adapter:
logger.info(
softwrap(
f"""
Launching debug adapter at '{debug_adapter.host}:{debug_adapter.port}',
which will wait for a client connection...
"""
)
if run_subsystem.debug_adapter:
logger.info(
softwrap(
f"""
Launching debug adapter at '{debug_adapter.host}:{debug_adapter.port}',
which will wait for a client connection...
"""
)

result = await Effect(
InteractiveProcessResult,
InteractiveProcess(
argv=(*args, *run_subsystem.args),
env=env,
run_in_workspace=True,
restartable=restartable,
),
)
exit_code = result.exit_code

return Run(exit_code)
result = await Effect(
InteractiveProcessResult,
InteractiveProcess(
argv=(*request.args, *run_subsystem.args),
env={**complete_env, **request.extra_env},
input_digest=request.digest,
run_in_workspace=True,
restartable=restartable,
cleanup=cleanup,
),
)

return Run(result.exit_code)


def rules():
Expand Down
15 changes: 0 additions & 15 deletions src/python/pants/engine/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,21 +331,6 @@ def __init__(
self.restartable = restartable
self.cleanup = cleanup

self.__post_init__()

def __post_init__(self):
if self.process.input_digest != EMPTY_DIGEST and self.run_in_workspace:
raise ValueError(
"InteractiveProcess should use the Workspace API to materialize any needed "
"files when it runs in the workspace"
)
if self.process.append_only_caches and self.run_in_workspace:
raise ValueError(
"InteractiveProcess requested setup of append-only caches and also requested to run"
" in the workspace. These options are incompatible since setting up append-only"
" caches would modify the workspace."
)

@classmethod
def from_process(
cls,
Expand Down
27 changes: 11 additions & 16 deletions src/python/pants/engine/process_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,32 +252,27 @@ def test_create_files(rule_runner: RuleRunner) -> None:
assert result.stdout == b"hellogoodbye"


def test_interactive_process_cannot_have_input_files_and_workspace() -> None:
mock_digest = Digest(EMPTY_DIGEST.fingerprint, 1)
with pytest.raises(ValueError):
InteractiveProcess(argv=["/bin/echo"], input_digest=mock_digest, run_in_workspace=True)


def test_interactive_process_cannot_have_append_only_caches_and_workspace() -> None:
with pytest.raises(ValueError):
InteractiveProcess(
argv=["/bin/echo"], append_only_caches={"foo": "bar"}, run_in_workspace=True
)


def test_interactive_process_immutable_input_digests(rule_runner: RuleRunner) -> None:
@pytest.mark.parametrize("run_in_workspace", [True, False])
def test_interactive_process_inputs(rule_runner: RuleRunner, run_in_workspace: bool) -> None:
digest0 = rule_runner.request(Digest, [CreateDigest([FileContent("file0", b"")])])
digest1 = rule_runner.request(Digest, [CreateDigest([FileContent("file1", b"")])])
digest2 = rule_runner.request(
Digest, [CreateDigest([FileContent("file2", b""), FileContent("file3", b"")])]
)
process = InteractiveProcess(
argv=["/bin/bash", "-c", "ls -1"],
argv=["/bin/bash", "-c", "ls -1 '{chroot}'"],
env={"BAZ": "QUX"},
input_digest=digest0,
immutable_input_digests={"prefix1": digest1, "prefix2": digest2},
append_only_caches={"cache_name": "append_only0"},
run_in_workspace=run_in_workspace,
)
with mock_console(rule_runner.options_bootstrapper) as (_, stdio_reader):
result = rule_runner.run_interactive_process(process)
assert result.exit_code == 0
assert set(stdio_reader.get_stdout().splitlines()) == {"file0", "prefix1", "prefix2"}
assert set(stdio_reader.get_stdout().splitlines()) == {
"append_only0",
"file0",
"prefix1",
"prefix2",
}
13 changes: 10 additions & 3 deletions src/python/pants/jvm/jdk_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,20 @@ class JdkEnvironment:
jdk_preparation_script: ClassVar[str] = f"{bin_dir}/jdk.sh"
java_home: ClassVar[str] = "__java_home"

def args(self, bash: BashBinary, classpath_entries: Iterable[str]) -> tuple[str, ...]:
def args(
self, bash: BashBinary, classpath_entries: Iterable[str], chroot: str | None = None
) -> tuple[str, ...]:
def in_chroot(path: str) -> str:
if not chroot:
return path
return os.path.join(chroot, path)

return (
bash.path,
self.jdk_preparation_script,
in_chroot(self.jdk_preparation_script),
f"{self.java_home}/bin/java",
"-cp",
":".join([self.nailgun_jar, *classpath_entries]),
":".join([in_chroot(self.nailgun_jar), *classpath_entries]),
)

@property
Expand Down

0 comments on commit 530bd42

Please sign in to comment.