Replace --[no-]process-cleanup with `--keep-sandboxes={always,never…

…,on_failure}` (pantsbuild#16415)

For the purposes of debugging an intermittent issue (such as a crash which occurs only in certain conditions in CI), `--no-process-cleanup` is too big of a hammer. To support capturing the sandboxes for only failing processes, this change replaces the `--[no-]process-cleanup` option with `--keep-sandboxes={always,never,on_failure}`.

[ci skip-rust]
[ci skip-build-wheels]

docs/markdown/Using Pants/troubleshooting.md

@@ -27,20 +27,22 @@ Setting the option `--pex-verbosity=9` can help debug exceptions that occur when
 
 Once you have this stack trace, we recommend copying it into Pastebin or a GitHub Gist, then opening a GitHub issue or posting on Slack. Someone from the Pants team would be happy to help. See [Getting Help](doc:getting-help).
 
-Debug tip: inspect the sandbox with `--no-process-cleanup`
+Debug tip: inspect the sandbox with `--keep-sandboxes`
 ----------------------------------------------------------
 
 Pants runs most processes in a hermetic sandbox (temporary directory), which allows for safely caching and running multiple processes in parallel. 
 
-Use the option `--no-process-cleanup` for Pants to log the paths to these sandboxes, and to keep them around after the run. You can then inspect them to check if the files you are expecting are present.
+Use the option `--keep-sandboxes=always` for Pants to log the paths to these sandboxes, and to keep them around after the run. You can then inspect them to check if the files you are expecting are present.
 
 ```bash
-./pants --no-process-cleanup lint src/project/app.py
+./pants --keep-sandboxes=always lint src/project/app.py
 ...
 21:26:13.55 [INFO] preserving local process execution dir `"/private/var/folders/hm/qjjq4w3n0fsb07kp5bxbn8rw0000gn/T/process-executionQgIOjb"` for "Run isort on 1 file."
 ...
 ```
 
+You can also pass `--keep-sandboxes=on_failure`, to preserve only the sandboxes of failing processes.
+
 There is even a `__run.sh` script in the directory that will run the process using the same argv and environment that Pants would use.
 
 Cache or pantsd invalidation issues

docs/markdown/Writing Plugins/rules-api/rules-api-process.md

@@ -38,7 +38,7 @@ The process will run in a temporary directory and is hermetic, meaning that it c
 
 > 📘 Debugging a `Process`
 > 
-> Setting the [`--no-process-cleanup`](docs:rules-api-tips#debugging-look-inside-the-chroot) flag will cause the sandboxes of `Process`es to be preserved and logged to the console for inspection.
+> Setting the [`--keep-sandboxes=always`](docs:rules-api-tips#debugging-look-inside-the-chroot) flag will cause the sandboxes of `Process`es to be preserved and logged to the console for inspection.
 > 
 > It can be very helpful while editing `Process` definitions!
 

docs/markdown/Writing Plugins/rules-api/rules-api-tips.md

@@ -61,12 +61,12 @@ Pants will also memoize in-memory the evaluation of all `@rule`s. This means tha
 Debugging: Look inside the chroot
 ---------------------------------
 
-When Pants runs most processes, it runs in a `chroot` (temporary directory). Usually, this gets cleaned up after the `Process` finishes. You can instead run `./pants --no-process-cleanup`, which will keep around the folder.
+When Pants runs most processes, it runs in a `chroot` (temporary directory). Usually, this gets cleaned up after the `Process` finishes. You can instead pass `--keep-sandboxes=always` to keep those directories for all processes, or `--keep-sandboxes=on_failure` to keep those directories for only processes which have failed.
 
 Pants will log the path to the chroot, e.g.:
 
 ```
-▶ ./pants --no-process-cleanup test src/python/pants/util/strutil_test.py
+▶ ./pants --keep-sandboxes=always test src/python/pants/util/strutil_test.py
 ...
 12:29:45.08 [INFO] preserving local process execution dir `"/private/var/folders/sx/pdpbqz4x5cscn9hhfpbsbqvm0000gn/T/process-executionN9Kdk0"` for "Test binary /Users/pantsbuild/.pyenv/shims/python3."
 ...

src/python/pants/backend/docker/goals/package_image.py

@@ -40,7 +40,7 @@
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import Target, WrappedTarget, WrappedTargetRequest
 from pants.engine.unions import UnionRule
-from pants.option.global_options import GlobalOptions, ProcessCleanupOption
+from pants.option.global_options import GlobalOptions, KeepSandboxes
 from pants.util.strutil import bullet_list
 
 logger = logging.getLogger(__name__)
@@ -233,7 +233,7 @@ async def build_docker_image(
     options: DockerOptions,
     global_options: GlobalOptions,
     docker: DockerBinary,
-    process_cleanup: ProcessCleanupOption,
+    keep_sandboxes: KeepSandboxes,
 ) -> BuiltPackage:
     """Build a Docker image using `docker build`."""
     context, wrapped_target = await MultiGet(
@@ -290,7 +290,7 @@ async def build_docker_image(
             result.stdout,
             result.stderr,
             process.description,
-            process_cleanup=process_cleanup.val,
+            keep_sandboxes=keep_sandboxes,
         )
 
     image_id = parse_image_id_from_docker_build_output(result.stdout, result.stderr)

src/python/pants/backend/docker/goals/package_image_test.py

@@ -53,7 +53,7 @@
     ProcessResultMetadata,
 )
 from pants.engine.target import InvalidFieldException, WrappedTarget, WrappedTargetRequest
-from pants.option.global_options import GlobalOptions, ProcessCleanupOption
+from pants.option.global_options import GlobalOptions, KeepSandboxes
 from pants.testutil.option_util import create_subsystem
 from pants.testutil.pytest_util import assert_logged, no_exception
 from pants.testutil.rule_runner import MockGet, QueryRule, RuleRunner, run_rule_with_mocks
@@ -146,7 +146,7 @@ def run_process_mock(process: Process) -> FallibleProcessResult:
             docker_options,
             global_options,
             DockerBinary("/dummy/docker"),
-            ProcessCleanupOption(True),
+            KeepSandboxes.never,
         ],
         mock_gets=[
             MockGet(

src/python/pants/backend/java/dependency_inference/java_parser.py

@@ -21,7 +21,7 @@
 from pants.jvm.jdk_rules import InternalJdk, JvmProcess
 from pants.jvm.resolve.coursier_fetch import ToolClasspath, ToolClasspathRequest
 from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool
-from pants.option.global_options import ProcessCleanupOption
+from pants.option.global_options import KeepSandboxes
 from pants.util.logging import LogLevel
 from pants.util.ordered_set import FrozenOrderedSet
 
@@ -53,7 +53,7 @@ class JavaParserCompiledClassfiles:
 @rule(level=LogLevel.DEBUG)
 async def resolve_fallible_result_to_analysis(
     fallible_result: FallibleJavaSourceDependencyAnalysisResult,
-    process_cleanup: ProcessCleanupOption,
+    keep_sandboxes: KeepSandboxes,
 ) -> JavaSourceDependencyAnalysis:
     # TODO(#12725): Just convert directly to a ProcessResult like this:
     # result = await Get(ProcessResult, FallibleProcessResult, fallible_result.process_result)
@@ -68,7 +68,7 @@ async def resolve_fallible_result_to_analysis(
         fallible_result.process_result.stdout,
         fallible_result.process_result.stderr,
         "Java source dependency analysis failed.",
-        process_cleanup=process_cleanup.val,
+        keep_sandboxes=keep_sandboxes,
     )
 
 

src/python/pants/backend/kotlin/dependency_inference/kotlin_parser.py

@@ -20,7 +20,7 @@
 from pants.jvm.resolve.common import ArtifactRequirements, Coordinate
 from pants.jvm.resolve.coursier_fetch import ToolClasspath, ToolClasspathRequest
 from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool
-from pants.option.global_options import ProcessCleanupOption
+from pants.option.global_options import KeepSandboxes
 from pants.util.frozendict import FrozenDict
 from pants.util.logging import LogLevel
 from pants.util.ordered_set import FrozenOrderedSet
@@ -207,7 +207,7 @@ async def analyze_kotlin_source_dependencies(
 @rule(level=LogLevel.DEBUG)
 async def resolve_fallible_result_to_analysis(
     fallible_result: FallibleKotlinSourceDependencyAnalysisResult,
-    process_cleanup: ProcessCleanupOption,
+    keep_sandboxes: KeepSandboxes,
 ) -> KotlinSourceDependencyAnalysis:
     # TODO(#12725): Just convert directly to a ProcessResult like this:
     # result = await Get(ProcessResult, FallibleProcessResult, fallible_result.process_result)
@@ -222,7 +222,7 @@ async def resolve_fallible_result_to_analysis(
         fallible_result.process_result.stdout,
         fallible_result.process_result.stderr,
         "Kotlin source dependency analysis failed.",
-        process_cleanup=process_cleanup.val,
+        keep_sandboxes=keep_sandboxes,
     )
 
 

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

@@ -49,7 +49,7 @@
 from pants.engine.rules import Get, MultiGet, collect_rules, rule
 from pants.engine.target import TransitiveTargets, TransitiveTargetsRequest
 from pants.engine.unions import UnionRule
-from pants.option.global_options import ProcessCleanupOption
+from pants.option.global_options import KeepSandboxes
 from pants.option.option_types import (
     BoolOption,
     EnumListOption,
@@ -487,7 +487,7 @@ async def generate_coverage_reports(
     coverage_setup: CoverageSetup,
     coverage_config: CoverageConfig,
     coverage_subsystem: CoverageSubsystem,
-    process_cleanup: ProcessCleanupOption,
+    keep_sandboxes: KeepSandboxes,
     distdir: DistDir,
 ) -> CoverageReports:
     """Takes all Python test results and generates a single coverage report."""
@@ -566,7 +566,7 @@ async def generate_coverage_reports(
                 res.stdout,
                 res.stderr,
                 proc.description,
-                process_cleanup=process_cleanup.val,
+                keep_sandboxes=keep_sandboxes,
             )
 
     # In practice if one result triggers --fail-under, they all will, but no need to rely on that.

src/python/pants/backend/scala/dependency_inference/scala_parser.py

@@ -34,7 +34,7 @@
 from pants.jvm.resolve.jvm_tool import GenerateJvmLockfileFromTool
 from pants.jvm.subsystems import JvmSubsystem
 from pants.jvm.target_types import JvmResolveField
-from pants.option.global_options import ProcessCleanupOption
+from pants.option.global_options import KeepSandboxes
 from pants.util.frozendict import FrozenDict
 from pants.util.logging import LogLevel
 from pants.util.ordered_set import FrozenOrderedSet
@@ -292,7 +292,7 @@ async def analyze_scala_source_dependencies(
 @rule(level=LogLevel.DEBUG)
 async def resolve_fallible_result_to_analysis(
     fallible_result: FallibleScalaSourceDependencyAnalysisResult,
-    process_cleanup: ProcessCleanupOption,
+    keep_sandboxes: KeepSandboxes,
 ) -> ScalaSourceDependencyAnalysis:
     # TODO(#12725): Just convert directly to a ProcessResult like this:
     # result = await Get(ProcessResult, FallibleProcessResult, fallible_result.process_result)
@@ -307,7 +307,7 @@ async def resolve_fallible_result_to_analysis(
         fallible_result.process_result.stdout,
         fallible_result.process_result.stderr,
         "Scala source dependency analysis failed.",
-        process_cleanup=process_cleanup.val,
+        keep_sandboxes=keep_sandboxes,
     )
 
 

src/python/pants/core/goals/run.py

@@ -25,7 +25,7 @@
     WrappedTargetRequest,
 )
 from pants.engine.unions import UnionMembership, union
-from pants.option.global_options import GlobalOptions
+from pants.option.global_options import GlobalOptions, KeepSandboxes
 from pants.option.option_types import ArgsListOption, BoolOption
 from pants.util.frozendict import FrozenDict
 from pants.util.meta import frozen_after_init
@@ -110,14 +110,17 @@ def activated(cls, union_membership: UnionMembership) -> bool:
     )
     cleanup = BoolOption(
         default=True,
+        deprecation_start_version="2.15.0.dev1",
+        removal_version="2.16.0.dev1",
+        removal_hint="Use the global `keep_sandboxes` option instead.",
         help=softwrap(
             """
             Whether to clean up the temporary directory in which the binary is chrooted.
             Set this to false to retain the directory, e.g., for debugging.
 
-            Note that setting the global --process-cleanup option to false will also conserve
-            this directory, along with those of all other processes that Pants executes.
-            This option is more selective and controls just the target binary's directory.
+            Note that setting the global --keep-sandboxes option may also conserve this directory,
+            along with those of all other processes that Pants executes. This option is more
+            selective and controls just the target binary's directory.
             """
         ),
     )
@@ -168,8 +171,11 @@ async def run(
         WrappedTarget, WrappedTargetRequest(field_set.address, description_of_origin="<infallible>")
     )
     restartable = wrapped_target.target.get(RestartableField).value
-    # 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
+    keep_sandboxes = (
+        global_options.keep_sandboxes
+        if run_subsystem.options.is_default("cleanup")
+        else (KeepSandboxes.never if run_subsystem.cleanup else KeepSandboxes.always)
+    )
 
     if run_subsystem.debug_adapter:
         logger.info(
@@ -189,7 +195,7 @@ async def run(
             input_digest=request.digest,
             run_in_workspace=True,
             restartable=restartable,
-            cleanup=cleanup,
+            keep_sandboxes=keep_sandboxes,
             immutable_input_digests=request.immutable_input_digests,
             append_only_caches=request.append_only_caches,
         ),

src/python/pants/core/goals/run_test.py

@@ -27,7 +27,7 @@
     WrappedTarget,
     WrappedTargetRequest,
 )
-from pants.option.global_options import GlobalOptions
+from pants.option.global_options import GlobalOptions, KeepSandboxes
 from pants.testutil.option_util import create_goal_subsystem, create_subsystem
 from pants.testutil.rule_runner import (
     MockEffect,
@@ -86,7 +86,9 @@ class TestBinaryTarget(Target):
                     port="5678",
                 ),
                 create_subsystem(
-                    GlobalOptions, pants_workdir=rule_runner.pants_workdir, process_cleanup=True
+                    GlobalOptions,
+                    pants_workdir=rule_runner.pants_workdir,
+                    keep_sandboxes=KeepSandboxes.never,
                 ),
                 workspace,
                 BuildRoot(),

src/python/pants/engine/internals/options_parsing.py

@@ -3,11 +3,13 @@
 
 from dataclasses import dataclass
 
+from pants.base.deprecated import warn_or_error
 from pants.build_graph.build_configuration import BuildConfiguration
 from pants.engine.internals.session import SessionValues
 from pants.engine.rules import collect_rules, rule
 from pants.option.global_options import (
     GlobalOptions,
+    KeepSandboxes,
     NamedCachesDirOption,
     ProcessCleanupOption,
     UseDeprecatedPexBinaryRunSemanticsOption,
@@ -59,8 +61,18 @@ def log_level(global_options: GlobalOptions) -> LogLevel:
 
 
 @rule
-def extract_process_cleanup_option(global_options: GlobalOptions) -> ProcessCleanupOption:
-    return ProcessCleanupOption(global_options.process_cleanup)
+def extract_process_cleanup_option(keep_sandboxes: KeepSandboxes) -> ProcessCleanupOption:
+    warn_or_error(
+        removal_version="2.15.0.dev1",
+        entity="ProcessCleanupOption",
+        hint="Instead, use `KeepSandboxes`.",
+    )
+    return ProcessCleanupOption(keep_sandboxes == KeepSandboxes.never)
+
+
+@rule
+def extract_keep_sandboxes(global_options: GlobalOptions) -> KeepSandboxes:
+    return GlobalOptions.resolve_keep_sandboxes(global_options.options)
 
 
 @rule

src/python/pants/engine/internals/scheduler.py

@@ -203,7 +203,7 @@ def __init__(
             local_cache=execution_options.local_cache,
             remote_cache_read=execution_options.remote_cache_read,
             remote_cache_write=execution_options.remote_cache_write,
-            local_cleanup=execution_options.process_cleanup,
+            local_keep_sandboxes=execution_options.keep_sandboxes.value,
             local_parallelism=execution_options.process_execution_local_parallelism,
             local_enable_nailgun=execution_options.process_execution_local_enable_nailgun,
             remote_parallelism=execution_options.process_execution_remote_parallelism,