From e336e38a95f81c6a5b4b8a41244d8e1fb0daff4c Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Fri, 21 Aug 2020 12:08:43 -0700 Subject: [PATCH 1/3] Fix `run`, `repl`, and `test --debug` to have hermetic environments # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] --- .../backend/python/rules/pytest_runner.py | 146 ++++++++---------- src/python/pants/backend/python/rules/repl.py | 35 +++-- .../backend/python/rules/run_python_binary.py | 22 +-- src/python/pants/engine/process.py | 6 + src/rust/engine/src/externs/interface.rs | 7 +- 5 files changed, 108 insertions(+), 108 deletions(-) diff --git a/src/python/pants/backend/python/rules/pytest_runner.py b/src/python/pants/backend/python/rules/pytest_runner.py index 71e968850679..8a2b8b8dfb43 100644 --- a/src/python/pants/backend/python/rules/pytest_runner.py +++ b/src/python/pants/backend/python/rules/pytest_runner.py @@ -5,7 +5,7 @@ import logging from dataclasses import dataclass from pathlib import PurePath -from typing import Optional, Tuple +from typing import Optional from uuid import UUID from pants.backend.python.rules.coverage import ( @@ -33,7 +33,7 @@ from pants.engine.addresses import Addresses from pants.engine.fs import AddPrefix, Digest, DigestSubset, MergeDigests, PathGlobs, Snapshot from pants.engine.internals.uuid import UUIDRequest -from pants.engine.process import FallibleProcessResult, InteractiveProcess +from pants.engine.process import FallibleProcessResult, InteractiveProcess, Process from pants.engine.rules import Get, MultiGet, collect_rules, rule from pants.engine.target import TransitiveTargets from pants.engine.unions import UnionRule @@ -61,15 +61,15 @@ def is_conftest(self) -> bool: @dataclass(frozen=True) -class TestTargetSetup: - test_runner_pex: Pex - args: Tuple[str, ...] - input_digest: Digest - source_roots: Tuple[str, ...] - timeout_seconds: Optional[int] - xml_dir: Optional[str] - junit_family: str - execution_slot_variable: str +class TestSetupRequest: + field_set: PythonTestFieldSet + is_debug: bool + + +@dataclass(frozen=True) +class TestSetup: + process: Process + results_file_name: Optional[str] # Prevent this class from being detected by pytest as a test class. __test__ = False @@ -77,14 +77,15 @@ class TestTargetSetup: @rule(level=LogLevel.DEBUG) async def setup_pytest_for_target( - field_set: PythonTestFieldSet, + request: TestSetupRequest, pytest: PyTest, test_subsystem: TestSubsystem, python_setup: PythonSetup, coverage_config: CoverageConfig, coverage_subsystem: CoverageSubsystem, -) -> TestTargetSetup: - test_addresses = Addresses((field_set.address,)) + global_options: GlobalOptions, +) -> TestSetup: + test_addresses = Addresses((request.field_set.address,)) transitive_targets = await Get(TransitiveTargets, Addresses, test_addresses) all_targets = transitive_targets.closure @@ -154,7 +155,9 @@ async def setup_pytest_for_target( # Get the file names for the test_target so that we can specify to Pytest precisely which files # to test, rather than using auto-discovery. - field_set_source_files_request = Get(SourceFiles, SourceFilesRequest([field_set.sources])) + field_set_source_files_request = Get( + SourceFiles, SourceFilesRequest([request.field_set.sources]) + ) ( pytest_pex, @@ -183,56 +186,32 @@ async def setup_pytest_for_target( ), ) - coverage_args = [] - if test_subsystem.use_coverage: - cov_paths = coverage_subsystem.filter if coverage_subsystem.filter else (".",) - coverage_args = [ - "--cov-report=", # Turn off output. - *itertools.chain.from_iterable(["--cov", cov_path] for cov_path in cov_paths), - ] - return TestTargetSetup( - test_runner_pex=test_runner_pex, - args=(*pytest.options.args, *coverage_args, *field_set_source_files.files), - input_digest=input_digest, - source_roots=prepared_sources.source_roots, - timeout_seconds=field_set.timeout.calculate_from_global_options(pytest), - xml_dir=pytest.options.junit_xml_dir, - junit_family=pytest.options.junit_family, - execution_slot_variable=pytest.options.execution_slot_var, - ) - - -# TODO(#10618): Once this is fixed, move `TestTargetSetup` into an `await Get` so that we only set -# up the test if it isn't skipped. -@rule(desc="Run Pytest", level=LogLevel.DEBUG) -async def run_python_test( - field_set: PythonTestFieldSet, - setup: TestTargetSetup, - global_options: GlobalOptions, - test_subsystem: TestSubsystem, -) -> TestResult: - if field_set.is_conftest(): - return TestResult.skip(field_set.address) - add_opts = [f"--color={'yes' if global_options.options.colors else 'no'}"] - output_files = [] - # Configure generation of JUnit-compatible test report. - test_results_file = None - if setup.xml_dir: - test_results_file = f"{field_set.address.path_safe_spec}.xml" + + results_file_name = None + if pytest.options.junit_xml_dir and not request.is_debug: + results_file_name = f"{request.field_set.address.path_safe_spec}.xml" add_opts.extend( - (f"--junitxml={test_results_file}", "-o", f"junit_family={setup.junit_family}") + (f"--junitxml={results_file_name}", "-o", f"junit_family={pytest.options.junit_family}") ) - output_files.append(test_results_file) + output_files.append(results_file_name) - # Configure generation of a coverage report. - if test_subsystem.use_coverage: + coverage_args = [] + if test_subsystem.use_coverage and not request.is_debug: output_files.append(".coverage") + cov_paths = coverage_subsystem.filter if coverage_subsystem.filter else (".",) + coverage_args = [ + "--cov-report=", # Turn off output. + *itertools.chain.from_iterable(["--cov", cov_path] for cov_path in cov_paths), + ] - env = {"PYTEST_ADDOPTS": " ".join(add_opts), "PEX_EXTRA_SYS_PATH": ":".join(setup.source_roots)} + extra_env = { + "PYTEST_ADDOPTS": " ".join(add_opts), + "PEX_EXTRA_SYS_PATH": ":".join(prepared_sources.source_roots), + } - if test_subsystem.force: + if test_subsystem.force and not request.is_debug: # This is a slightly hacky way to force the process to run: since the env var # value is unique, this input combination will never have been seen before, # and therefore never cached. The two downsides are: @@ -241,22 +220,34 @@ async def run_python_test( # 2. This run will be cached even though it can never be re-used. # TODO: A more principled way of forcing rules to run? uuid = await Get(UUID, UUIDRequest()) - env["__PANTS_FORCE_TEST_RUN__"] = str(uuid) + extra_env["__PANTS_FORCE_TEST_RUN__"] = str(uuid) - result = await Get( - FallibleProcessResult, + process = await Get( + Process, PexProcess( - setup.test_runner_pex, - argv=setup.args, - input_digest=setup.input_digest, - output_files=tuple(output_files) if output_files else None, - description=f"Run Pytest for {field_set.address}", - timeout_seconds=setup.timeout_seconds, - extra_env=env, - execution_slot_variable=setup.execution_slot_variable, + test_runner_pex, + argv=(*pytest.options.args, *coverage_args, *field_set_source_files.files), + extra_env=extra_env, + input_digest=input_digest, + output_files=output_files, + timeout_seconds=request.field_set.timeout.calculate_from_global_options(pytest), + execution_slot_variable=pytest.options.execution_slot_var, + description=f"Run Pytest for {request.field_set.address}", level=LogLevel.DEBUG, ), ) + return TestSetup(process, results_file_name=results_file_name) + + +@rule(desc="Run Pytest", level=LogLevel.DEBUG) +async def run_python_test( + field_set: PythonTestFieldSet, test_subsystem: TestSubsystem, pytest: PyTest +) -> TestResult: + if field_set.is_conftest(): + return TestResult.skip(field_set.address) + + setup = await Get(TestSetup, TestSetupRequest(field_set, is_debug=False)) + result = await Get(FallibleProcessResult, Process, setup.process) coverage_data = None if test_subsystem.use_coverage: @@ -269,34 +260,31 @@ async def run_python_test( logger.warning(f"Failed to generate coverage data for {field_set.address}.") xml_results_digest = None - if test_results_file: + if setup.results_file_name: xml_results_snapshot = await Get( - Snapshot, DigestSubset(result.output_digest, PathGlobs([test_results_file])) + Snapshot, DigestSubset(result.output_digest, PathGlobs([setup.results_file_name])) ) - if xml_results_snapshot.files == (test_results_file,): + if xml_results_snapshot.files == (setup.results_file_name,): xml_results_digest = await Get( - Digest, - AddPrefix(xml_results_snapshot.digest, setup.xml_dir), # type: ignore[arg-type] + Digest, AddPrefix(xml_results_snapshot.digest, pytest.options.junit_xml_dir), ) else: logger.warning(f"Failed to generate JUnit XML data for {field_set.address}.") return TestResult.from_fallible_process_result( result, + address=field_set.address, coverage_data=coverage_data, xml_results=xml_results_digest, - address=field_set.address, ) @rule(desc="Set up Pytest to run interactively", level=LogLevel.DEBUG) -def debug_python_test(field_set: PythonTestFieldSet, setup: TestTargetSetup) -> TestDebugRequest: +async def debug_python_test(field_set: PythonTestFieldSet) -> TestDebugRequest: if field_set.is_conftest(): return TestDebugRequest(None) - process = InteractiveProcess( - argv=(setup.test_runner_pex.name, *setup.args), input_digest=setup.input_digest, - ) - return TestDebugRequest(process) + setup = await Get(TestSetup, TestSetupRequest(field_set, is_debug=True)) + return TestDebugRequest(InteractiveProcess.from_process(setup.process)) def rules(): diff --git a/src/python/pants/backend/python/rules/repl.py b/src/python/pants/backend/python/rules/repl.py index 5e1257a83327..d6172e0358f0 100644 --- a/src/python/pants/backend/python/rules/repl.py +++ b/src/python/pants/backend/python/rules/repl.py @@ -4,6 +4,7 @@ from typing import Tuple from pants.backend.python.rules.pex import Pex, PexRequest, PexRequirements +from pants.backend.python.rules.pex_environment import PexEnvironment from pants.backend.python.rules.pex_from_targets import PexFromTargetsRequest from pants.backend.python.rules.python_sources import PythonSourceFiles, PythonSourceFilesRequest from pants.backend.python.subsystems.ipython import IPython @@ -20,7 +21,7 @@ class PythonRepl(ReplImplementation): @rule(level=LogLevel.DEBUG) -async def create_python_repl_request(repl: PythonRepl) -> ReplRequest: +async def create_python_repl_request(repl: PythonRepl, pex_env: PexEnvironment) -> ReplRequest: requirements_request = Get( Pex, PexFromTargetsRequest, @@ -35,12 +36,11 @@ async def create_python_repl_request(repl: PythonRepl) -> ReplRequest: merged_digest = await Get( Digest, MergeDigests((requirements_pex.digest, sources.source_files.snapshot.digest)) ) + chrooted_source_roots = [repl.in_chroot(sr) for sr in sources.source_roots] - return ReplRequest( - digest=merged_digest, - args=(repl.in_chroot(requirements_pex.name),), - env={"PEX_EXTRA_SYS_PATH": ":".join(chrooted_source_roots)}, - ) + env = {**pex_env.environment_dict, "PEX_EXTRA_SYS_PATH": ":".join(chrooted_source_roots)} + + return ReplRequest(digest=merged_digest, args=(repl.in_chroot(requirements_pex.name),), env=env) class IPythonRepl(ReplImplementation): @@ -48,7 +48,9 @@ class IPythonRepl(ReplImplementation): @rule(level=LogLevel.DEBUG) -async def create_ipython_repl_request(repl: IPythonRepl, ipython: IPython) -> ReplRequest: +async def create_ipython_repl_request( + repl: IPythonRepl, ipython: IPython, pex_env: PexEnvironment +) -> ReplRequest: # Note that we get an intermediate PexRequest here (instead of going straight to a Pex) # so that we can get the interpreter constraints for use in ipython_request. requirements_pex_request = await Get( @@ -85,18 +87,19 @@ async def create_ipython_repl_request(repl: IPythonRepl, ipython: IPython) -> Re (requirements_pex.digest, sources.source_files.snapshot.digest, ipython_pex.digest) ), ) - chrooted_source_roots = [repl.in_chroot(sr) for sr in sources.source_roots] + args: Tuple[str, ...] = (repl.in_chroot(ipython_pex.name),) if ipython.options.ignore_cwd: args = args + ("--ignore-cwd",) - return ReplRequest( - digest=merged_digest, - args=args, - env={ - "PEX_PATH": repl.in_chroot(requirements_pex_request.output_filename), - "PEX_EXTRA_SYS_PATH": ":".join(chrooted_source_roots), - }, - ) + + chrooted_source_roots = [repl.in_chroot(sr) for sr in sources.source_roots] + env = { + **pex_env.environment_dict, + "PEX_PATH": repl.in_chroot(requirements_pex_request.output_filename), + "PEX_EXTRA_SYS_PATH": ":".join(chrooted_source_roots), + } + + return ReplRequest(digest=merged_digest, args=args, env=env) def rules(): diff --git a/src/python/pants/backend/python/rules/run_python_binary.py b/src/python/pants/backend/python/rules/run_python_binary.py index 9fc39c2e23df..b3ef55f1da59 100644 --- a/src/python/pants/backend/python/rules/run_python_binary.py +++ b/src/python/pants/backend/python/rules/run_python_binary.py @@ -5,6 +5,7 @@ from pants.backend.python.rules.create_python_binary import PythonBinaryFieldSet from pants.backend.python.rules.pex import Pex, PexRequest +from pants.backend.python.rules.pex_environment import PexEnvironment from pants.backend.python.rules.pex_from_targets import PexFromTargetsRequest from pants.backend.python.rules.python_sources import PythonSourceFiles, PythonSourceFilesRequest from pants.backend.python.target_types import PythonBinaryDefaults, PythonBinarySources @@ -26,7 +27,9 @@ @rule(level=LogLevel.DEBUG) async def create_python_binary_run_request( - field_set: PythonBinaryFieldSet, python_binary_defaults: PythonBinaryDefaults + field_set: PythonBinaryFieldSet, + python_binary_defaults: PythonBinaryDefaults, + pex_env: PexEnvironment, ) -> RunRequest: entry_point = field_set.entry_point.value if entry_point is None: @@ -86,17 +89,18 @@ async def create_python_binary_run_request( def in_chroot(relpath: str) -> str: return os.path.join("{chroot}", relpath) - chrooted_source_roots = [in_chroot(sr) for sr in sources.source_roots] pex_path = in_chroot(requirements_pex_request.output_filename) + chrooted_source_roots = [in_chroot(sr) for sr in sources.source_roots] + env = { + **pex_env.environment_dict, + "PEX_PATH": pex_path, + "PEX_EXTRA_SYS_PATH": ":".join(chrooted_source_roots), + } + return RunRequest( - digest=merged_digest, - args=(in_chroot(runner_pex.name), "-m", entry_point), - env={"PEX_PATH": pex_path, "PEX_EXTRA_SYS_PATH": ":".join(chrooted_source_roots)}, + digest=merged_digest, args=(in_chroot(runner_pex.name), "-m", entry_point), env=env ) def rules(): - return [ - *collect_rules(), - UnionRule(RunFieldSet, PythonBinaryFieldSet), - ] + return [*collect_rules(), UnionRule(RunFieldSet, PythonBinaryFieldSet)] diff --git a/src/python/pants/engine/process.py b/src/python/pants/engine/process.py index ef409ea71087..5e7e151c1e42 100644 --- a/src/python/pants/engine/process.py +++ b/src/python/pants/engine/process.py @@ -283,6 +283,12 @@ def __post_init__(self): "files when it runs in the workspace" ) + @classmethod + def from_process(cls, process: Process) -> "InteractiveProcess": + return InteractiveProcess( + argv=process.argv, env=process.env, input_digest=process.input_digest + ) + @side_effecting @dataclass(frozen=True) diff --git a/src/rust/engine/src/externs/interface.rs b/src/rust/engine/src/externs/interface.rs index 853c6619fa04..472bf3dafa23 100644 --- a/src/rust/engine/src/externs/interface.rs +++ b/src/rust/engine/src/externs/interface.rs @@ -1561,10 +1561,9 @@ fn run_local_interactive_process( command.current_dir(tempdir.path()); } - let env = externs::project_frozendict(&value, "env"); - for (key, value) in env.iter() { - command.env(key, value); - } + // We use a hermetic environment to mirror normal Processes. + command.env_clear(); + command.envs(externs::project_frozendict(&value, "env")); let mut subprocess = command.spawn().map_err(|e| format!("Error executing interactive process: {}", e.to_string()))?; let exit_status = subprocess.wait().map_err(|e| e.to_string())?; From 197800377a091d6f81e547d266530afd7240b635 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Fri, 21 Aug 2020 13:01:24 -0700 Subject: [PATCH 2/3] Fix `test --open-coverage` This rewrites it to use our new `BinaryPaths` abstraction to find the `open` program. It also fixes a cheat where we were using `Console` and `InteractiveRunner` outside of a goal rule. # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- .../backend/python/rules/run_python_binary.py | 3 +- src/python/pants/core/goals/test.py | 8 +- src/python/pants/core/goals/test_test.py | 6 + src/python/pants/engine/desktop.py | 124 ++++++++---------- src/python/pants/init/engine_initializer.py | 3 +- 5 files changed, 69 insertions(+), 75 deletions(-) diff --git a/src/python/pants/backend/python/rules/run_python_binary.py b/src/python/pants/backend/python/rules/run_python_binary.py index b3ef55f1da59..7aa7bec87394 100644 --- a/src/python/pants/backend/python/rules/run_python_binary.py +++ b/src/python/pants/backend/python/rules/run_python_binary.py @@ -89,11 +89,10 @@ async def create_python_binary_run_request( def in_chroot(relpath: str) -> str: return os.path.join("{chroot}", relpath) - pex_path = in_chroot(requirements_pex_request.output_filename) chrooted_source_roots = [in_chroot(sr) for sr in sources.source_roots] env = { **pex_env.environment_dict, - "PEX_PATH": pex_path, + "PEX_PATH": in_chroot(requirements_pex_request.output_filename), "PEX_EXTRA_SYS_PATH": ":".join(chrooted_source_roots), } diff --git a/src/python/pants/core/goals/test.py b/src/python/pants/core/goals/test.py index 4f12211b52d9..32b918ef12b6 100644 --- a/src/python/pants/core/goals/test.py +++ b/src/python/pants/core/goals/test.py @@ -13,10 +13,10 @@ FieldSetsWithSources, FieldSetsWithSourcesRequest, ) -from pants.engine import desktop from pants.engine.addresses import Address from pants.engine.collection import Collection from pants.engine.console import Console +from pants.engine.desktop import OpenFiles, OpenFilesRequest from pants.engine.engine_aware import EngineAware from pants.engine.fs import Digest, MergeDigests, Workspace from pants.engine.goal import Goal, GoalSubsystem @@ -432,7 +432,11 @@ async def run_tests( coverage_report_files.extend(report_files) if coverage_report_files and test_subsystem.open_coverage: - desktop.ui_open(console, interactive_runner, coverage_report_files) + open_files = await Get( + OpenFiles, OpenFilesRequest(coverage_report_files, error_if_open_not_found=False) + ) + for process in open_files.processes: + interactive_runner.run(process) return Test(exit_code) diff --git a/src/python/pants/core/goals/test_test.py b/src/python/pants/core/goals/test_test.py index 4b56dfbc417c..c8bcb62f79db 100644 --- a/src/python/pants/core/goals/test_test.py +++ b/src/python/pants/core/goals/test_test.py @@ -26,6 +26,7 @@ FieldSetsWithSourcesRequest, ) from pants.engine.addresses import Address +from pants.engine.desktop import OpenFiles, OpenFilesRequest from pants.engine.fs import EMPTY_DIGEST, CreateDigest, Digest, FileContent, MergeDigests, Workspace from pants.engine.process import InteractiveProcess, InteractiveRunner from pants.engine.target import ( @@ -180,6 +181,11 @@ def mock_coverage_report_generation( subject_type=CoverageDataCollection, mock=mock_coverage_report_generation, ), + MockGet( + product_type=OpenFiles, + subject_type=OpenFilesRequest, + mock=lambda _: OpenFiles(()), + ), ], union_membership=union_membership, ) diff --git a/src/python/pants/engine/desktop.py b/src/python/pants/engine/desktop.py index 1d9c6f4a6c16..f8cf35354519 100644 --- a/src/python/pants/engine/desktop.py +++ b/src/python/pants/engine/desktop.py @@ -1,83 +1,67 @@ # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). -from abc import ABC +import logging from dataclasses import dataclass from pathlib import PurePath -from typing import ClassVar, Iterable, Iterator +from typing import Iterable, Tuple -from pants.engine.console import Console -from pants.engine.process import InteractiveProcess, InteractiveRunner -from pants.util.osutil import get_os_name -from pants.util.strutil import safe_shlex_join +from pants.engine.platform import Platform +from pants.engine.process import BinaryPathRequest, BinaryPaths, InteractiveProcess +from pants.engine.rules import Get, RootRule, collect_rules, rule +from pants.util.meta import frozen_after_init +logger = logging.getLogger(__name__) -@dataclass(frozen=True) -class _Opener(ABC): - console: Console - runner: InteractiveRunner - - program: ClassVar[str] - - def _iter_openers(self, files: Iterable[PurePath]) -> Iterator[InteractiveProcess]: - # N.B.: We cannot mark this method @abc.abstractmethod due to: - # https://github.com/python/mypy/issues/5374 - raise NotImplementedError() - - def open(self, files: Iterable[PurePath]) -> None: - for request in self._iter_openers(files): - open_command = safe_shlex_join(request.argv) - try: - result = self.runner.run(request) - if result.exit_code != 0: - self.console.print_stderr( - f"Failed to open files for viewing using `{open_command}` - received exit " - f"code {result.exit_code}." - ) - except Exception as e: - self.console.print_stderr( - f"Failed to open files for viewing using " f"`{open_command}`: {e}" - ) - self.console.print_stderr( - f"Ensure {self.program} is installed on your `PATH` and " f"re-run this goal." - ) - - -class _DarwinOpener(_Opener): - program = "open" - - def _iter_openers(self, files: Iterable[PurePath]) -> Iterator[InteractiveProcess]: - yield InteractiveProcess( - argv=(self.program, *(str(f) for f in files)), run_in_workspace=True - ) - - -class _LinuxOpener(_Opener): - program = "xdg-open" - - def _iter_openers(self, files: Iterable[PurePath]) -> Iterator[InteractiveProcess]: - for f in files: - yield InteractiveProcess(argv=(self.program, str(f)), run_in_workspace=True) +@frozen_after_init +@dataclass(unsafe_hash=True) +class OpenFilesRequest: + files: Tuple[PurePath, ...] + error_if_open_not_found: bool -_OPENERS_BY_OSNAME = {"darwin": _DarwinOpener, "linux": _LinuxOpener} + def __init__(self, files: Iterable[PurePath], *, error_if_open_not_found: bool = True) -> None: + self.files = tuple(files) + self.error_if_open_not_found = error_if_open_not_found -def ui_open(console: Console, runner: InteractiveRunner, files: Iterable[PurePath]) -> None: - """Opens the given files with the appropriate application for the current operating system. - - Any failures to either locate an appropriate application to open the files with or else execute - that program are reported to the console stderr. - """ - osname = get_os_name() - opener_type = _OPENERS_BY_OSNAME.get(osname) - if opener_type is None: - console.print_stderr(f"Could not open {' '.join(map(str, files))} for viewing.") - console.print_stderr( - f"Opening files for viewing is currently not supported for the " - f"{osname} operating system." +@dataclass(frozen=True) +class OpenFiles: + processes: Tuple[InteractiveProcess, ...] + + +@rule +async def find_open_program(request: OpenFilesRequest, plat: Platform) -> OpenFiles: + open_program_name = "open" if plat == Platform.darwin else "xdg-open" + open_program_paths = await Get( + BinaryPaths, + BinaryPathRequest(binary_name=open_program_name, search_path=("/bin", "/usr/bin")), + ) + if not open_program_paths.first_path: + error = ( + f"Could not find the program '{open_program_name}' on `/bin` or `/usr/bin`, so cannot " + f"open the files {sorted(request.files)}." ) - return - - opener = opener_type(console, runner) - opener.open(files) + if request.error_if_open_not_found: + raise OSError(error) + logger.error(error) + return OpenFiles(()) + + if plat == Platform.darwin: + processes = [ + InteractiveProcess( + argv=(open_program_paths.first_path, *(str(f) for f in request.files)), + run_in_workspace=True, + ) + ] + else: + processes = [ + InteractiveProcess(argv=(open_program_paths.first_path, str(f)), run_in_workspace=True) + for f in request.files + ] + + return OpenFiles(tuple(processes)) + + +def rules(): + return (*collect_rules(), RootRule(OpenFilesRequest)) diff --git a/src/python/pants/init/engine_initializer.py b/src/python/pants/init/engine_initializer.py index 50e30b49ded2..18dacf8ef80e 100644 --- a/src/python/pants/init/engine_initializer.py +++ b/src/python/pants/init/engine_initializer.py @@ -11,7 +11,7 @@ from pants.base.exiter import PANTS_SUCCEEDED_EXIT_CODE from pants.base.specs import Specs from pants.build_graph.build_configuration import BuildConfiguration -from pants.engine import fs, process +from pants.engine import desktop, fs, process from pants.engine.console import Console from pants.engine.fs import Workspace from pants.engine.goal import Goal @@ -227,6 +227,7 @@ def build_root_singleton() -> BuildRoot: *collect_rules(locals()), RootRule(Console), *build_files.rules(), + *desktop.rules(), *fs.rules(), *graph.rules(), *uuid.rules(), From 5a116b1e445f181f6d9d4e57f370b86df50793d7 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Fri, 21 Aug 2020 14:10:37 -0700 Subject: [PATCH 3/3] xfail Pantsd test that relies on the env var leaking # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- tests/python/pants_test/pantsd/pantsd_integration_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/python/pants_test/pantsd/pantsd_integration_test.py b/tests/python/pants_test/pantsd/pantsd_integration_test.py index 22aaaca89225..3ead2b74cd9f 100644 --- a/tests/python/pants_test/pantsd/pantsd_integration_test.py +++ b/tests/python/pants_test/pantsd/pantsd_integration_test.py @@ -175,6 +175,7 @@ def test_pantsd_filesystem_invalidation(self): join() + @pytest.mark.xfail(reason="TODO(#10644): re-enable once we can set arbitrary env vars.") def test_pantsd_client_env_var_is_inherited_by_pantsd_runner_children(self): EXPECTED_KEY = "TEST_ENV_VAR_FOR_PANTSD_INTEGRATION_TEST" EXPECTED_VALUE = "333"