Every Process is now platform-specific (impacts remote caching)

docs/markdown/Writing Plugins/common-plugin-tasks/plugin-upgrade-guide.md

@@ -9,6 +9,21 @@ updatedAt: "2022-07-25T20:02:17.695Z"
 2.15
 ----
 
+### `platform` kwarg for `Process` deprecated
+
+Previously, we assumed processes were platform-agnostic, i.e. they had identical output on all
+platforms (OS x CPU architecture). You had to opt into platform awareness by setting the kwarg
+`platform` on the `Process`; otherwise, remote caching could incorrectly use results from a
+different platform.
+
+This was not a safe default, and this behavior also breaks the new Docker support. So, now all
+processes automatically are marked as platform-specific.
+
+https://github.com/pantsbuild/pants/issues/16873 proposes how you will eventually be able to mark
+a `Process` as platform-agnostic.
+
+To fix this deprecation, simply delete the `platform` kwarg.
+
 ### `Environment`, `EnvironmentRequest`, and `CompleteEnvironment` renamed and moved
 
 The types were moved from `pants.engine.environment` to `pants.engine.env_vars`, and now have

src/python/pants/backend/codegen/protobuf/go/rules.py

@@ -557,7 +557,7 @@ async def generate_go_from_protobuf(
 
 
 @rule
-async def setup_go_protoc_plugin(platform: Platform) -> _SetupGoProtocPlugin:
+async def setup_go_protoc_plugin() -> _SetupGoProtocPlugin:
     go_mod_digest = await Get(
         Digest,
         CreateDigest(
@@ -587,7 +587,6 @@ async def setup_go_protoc_plugin(platform: Platform) -> _SetupGoProtocPlugin:
                 input_digest=download_sources_result.output_digest,
                 output_files=["gopath/bin/protoc-gen-go"],
                 description="Build Go protobuf plugin for `protoc`.",
-                platform=platform,
             ),
         ),
         Get(
@@ -600,7 +599,6 @@ async def setup_go_protoc_plugin(platform: Platform) -> _SetupGoProtocPlugin:
                 input_digest=download_sources_result.output_digest,
                 output_files=["gopath/bin/protoc-gen-go-grpc"],
                 description="Build Go gRPC protobuf plugin for `protoc`.",
-                platform=platform,
             ),
         ),
     )

src/python/pants/backend/go/util_rules/sdk.py

@@ -14,7 +14,6 @@
 from pants.engine.env_vars import EnvironmentVars, EnvironmentVarsRequest
 from pants.engine.fs import EMPTY_DIGEST, CreateDigest, Digest, FileContent, MergeDigests
 from pants.engine.internals.selectors import Get, MultiGet
-from pants.engine.platform import Platform
 from pants.engine.process import Process, ProcessResult
 from pants.engine.rules import collect_rules, rule
 from pants.util.frozendict import FrozenDict
@@ -32,7 +31,6 @@ class GoSdkProcess:
     working_dir: str | None
     output_files: tuple[str, ...]
     output_directories: tuple[str, ...]
-    platform: Platform | None
     replace_sandbox_root_in_args: bool
 
     def __init__(
@@ -46,7 +44,6 @@ def __init__(
         output_files: Iterable[str] = (),
         output_directories: Iterable[str] = (),
         allow_downloads: bool = False,
-        platform: Platform | None = None,
         replace_sandbox_root_in_args: bool = False,
     ) -> None:
         self.command = tuple(command)
@@ -60,7 +57,6 @@ def __init__(
         self.working_dir = working_dir
         self.output_files = tuple(output_files)
         self.output_directories = tuple(output_directories)
-        self.platform = platform
         self.replace_sandbox_root_in_args = replace_sandbox_root_in_args
 
 
@@ -136,7 +132,6 @@ async def setup_go_sdk_process(
         output_files=request.output_files,
         output_directories=request.output_directories,
         level=LogLevel.DEBUG,
-        platform=request.platform,
     )
 
 

src/python/pants/backend/python/util_rules/pex.py

@@ -555,8 +555,8 @@ async def build_pex(
     else:
         output_directories = [request.output_filename]
 
-    process = await Get(
-        Process,
+    result = await Get(
+        ProcessResult,
         PexCliProcess(
             python=pex_python_setup.python,
             subcommand=(),
@@ -569,13 +569,6 @@ async def build_pex(
         ),
     )
 
-    process = dataclasses.replace(process, platform=platform)
-
-    # NB: Building a Pex is platform dependent, so in order to get a PEX that we can use locally
-    # without cross-building, we specify that our PEX command should be run on the current local
-    # platform.
-    result = await Get(ProcessResult, Process, process)
-
     if pex_subsystem.verbosity > 0:
         log_output = result.stderr.decode()
         if log_output:

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

@@ -268,7 +268,7 @@ def test_from_fallible_process_result_output_prepping(
             stderr=b"stderr \033[0;31m/var/pants-sandbox-123/red/path.py\033[0m \033[1mbold\033[0m",
             stderr_digest=EMPTY_FILE_DIGEST,
             output_digest=EMPTY_DIGEST,
-            platform=Platform.current,
+            platform=Platform.create_for_localhost(),
             metadata=ProcessResultMetadata(0, "ran_locally", 0),
         ),
         strip_chroot_path=strip_chroot_path,

src/python/pants/core/util_rules/environments.py

@@ -356,11 +356,13 @@ async def get_target_for_environment_name(
 
 
 @rule
-def extract_process_config_from_environment(tgt: EnvironmentTarget) -> ProcessConfigFromEnvironment:
+def extract_process_config_from_environment(
+    tgt: EnvironmentTarget, platform: Platform
+) -> ProcessConfigFromEnvironment:
     docker_image = (
         tgt.val[DockerImageField].value if tgt.val and tgt.val.has_field(DockerImageField) else None
     )
-    return ProcessConfigFromEnvironment(docker_image=docker_image)
+    return ProcessConfigFromEnvironment(platform=platform.value, docker_image=docker_image)
 
 
 class EnvironmentSensitiveOptionFieldMixin:

src/python/pants/engine/internals/native_engine.pyi

@@ -172,7 +172,7 @@ class ProcessConfigFromEnvironment:
     `env` in the `Process` constructor.
     """
 
-    def __init__(self, *, docker_image: str | None) -> None: ...
+    def __init__(self, *, platform: str, docker_image: str | None) -> None: ...
     def __eq__(self, other: ProcessConfigFromEnvironment | Any) -> bool: ...
     def __hash__(self) -> int: ...
     def __repr__(self) -> str: ...

src/python/pants/engine/process.py

@@ -9,6 +9,7 @@
 from enum import Enum
 from typing import Iterable, Mapping
 
+from pants.base.deprecated import warn_or_error
 from pants.engine.engine_aware import SideEffecting
 from pants.engine.fs import EMPTY_DIGEST, Digest, FileDigest
 from pants.engine.internals.native_engine import (  # noqa: F401
@@ -21,6 +22,7 @@
 from pants.util.frozendict import FrozenDict
 from pants.util.logging import LogLevel
 from pants.util.meta import frozen_after_init
+from pants.util.strutil import softwrap
 
 logger = logging.getLogger(__name__)
 
@@ -65,7 +67,6 @@ class Process:
     execution_slot_variable: str | None
     concurrency_available: int
     cache_scope: ProcessCacheScope
-    platform: str | None
 
     def __init__(
         self,
@@ -133,7 +134,18 @@ def __init__(
         self.execution_slot_variable = execution_slot_variable
         self.concurrency_available = concurrency_available
         self.cache_scope = cache_scope
-        self.platform = platform.value if platform is not None else None
+
+        if platform is not None:
+            warn_or_error(
+                "2.16.0.dev0",
+                "the `platform` kwarg for `Process`",
+                softwrap(
+                    """
+                    The `platform` kwarg no longer does anything because the `platform` is always
+                    automatically set. To fix this deprecation, delete the kwarg.
+                    """
+                ),
+            )
 
 
 @dataclass(frozen=True)

src/python/pants/jvm/jdk_rules.py

@@ -16,7 +16,6 @@
 from pants.core.util_rules.system_binaries import BashBinary
 from pants.engine.fs import CreateDigest, Digest, FileContent, FileDigest, MergeDigests
 from pants.engine.internals.selectors import Get
-from pants.engine.platform import Platform
 from pants.engine.process import FallibleProcessResult, Process, ProcessCacheScope
 from pants.engine.rules import collect_rules, rule
 from pants.engine.target import CoarsenedTarget
@@ -320,7 +319,6 @@ class JvmProcess:
     output_files: tuple[str, ...]
     output_directories: tuple[str, ...]
     timeout_seconds: int | float | None
-    platform: Platform | None
     extra_immutable_input_digests: FrozenDict[str, Digest]
     extra_env: FrozenDict[str, str]
     cache_scope: ProcessCacheScope | None
@@ -341,7 +339,6 @@ def __init__(
         extra_immutable_input_digests: Mapping[str, Digest] | None = None,
         extra_env: Mapping[str, str] | None = None,
         timeout_seconds: int | float | None = None,
-        platform: Platform | None = None,
         cache_scope: ProcessCacheScope | None = None,
         use_nailgun: bool = True,
     ):
@@ -356,7 +353,6 @@ def __init__(
         self.output_files = tuple(output_files or ())
         self.output_directories = tuple(output_directories or ())
         self.timeout_seconds = timeout_seconds
-        self.platform = platform
         self.cache_scope = cache_scope
         self.extra_immutable_input_digests = FrozenDict(extra_immutable_input_digests or {})
         self.extra_env = FrozenDict(extra_env or {})
@@ -427,7 +423,6 @@ def valid_jvm_opt(opt: str) -> str:
         level=request.level,
         output_directories=request.output_directories,
         env=env,
-        platform=request.platform,
         timeout_seconds=request.timeout_seconds,
         append_only_caches=jdk.append_only_caches,
         output_files=request.output_files,

src/python/pants/testutil/rule_runner.py

@@ -42,6 +42,7 @@
 from pants.engine.internals.scheduler import ExecutionError, Scheduler, SchedulerSession
 from pants.engine.internals.selectors import Effect, Get, Params
 from pants.engine.internals.session import SessionValues
+from pants.engine.platform import Platform
 from pants.engine.process import InteractiveProcess, InteractiveProcessResult
 from pants.engine.rules import QueryRule as QueryRule
 from pants.engine.rules import rule
@@ -517,7 +518,11 @@ def write_digest(self, digest: Digest, *, path_prefix: str | None = None) -> Non
 
     def run_interactive_process(self, request: InteractiveProcess) -> InteractiveProcessResult:
         return native_engine.session_run_interactive_process(
-            self.scheduler.py_session, request, ProcessConfigFromEnvironment(docker_image=None)
+            self.scheduler.py_session,
+            request,
+            ProcessConfigFromEnvironment(
+                platform=Platform.create_for_localhost().value, docker_image=None
+            ),
         )
 
 

src/rust/engine/src/externs/process.rs

@@ -5,8 +5,11 @@ use std::collections::hash_map::DefaultHasher;
 use std::hash::{Hash, Hasher};
 
 use pyo3::basic::CompareOp;
+use pyo3::exceptions::PyValueError;
 use pyo3::prelude::*;
 
+use process_execution::Platform;
+
 pub(crate) fn register(m: &PyModule) -> PyResult<()> {
   m.add_class::<PyProcessConfigFromEnvironment>()?;
 
@@ -16,25 +19,32 @@ pub(crate) fn register(m: &PyModule) -> PyResult<()> {
 #[pyclass(name = "ProcessConfigFromEnvironment")]
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct PyProcessConfigFromEnvironment {
+  pub platform: Platform,
   pub docker_image: Option<String>,
 }
 
 #[pymethods]
 impl PyProcessConfigFromEnvironment {
   #[new]
-  fn __new__(docker_image: Option<String>) -> Self {
-    Self { docker_image }
+  fn __new__(platform: String, docker_image: Option<String>) -> PyResult<Self> {
+    let platform = Platform::try_from(platform).map_err(PyValueError::new_err)?;
+    Ok(Self {
+      platform,
+      docker_image,
+    })
   }
 
   fn __hash__(&self) -> u64 {
     let mut s = DefaultHasher::new();
+    self.platform.hash(&mut s);
     self.docker_image.hash(&mut s);
     s.finish()
   }
 
   fn __repr__(&self) -> String {
     format!(
-      "ProcessConfigFromEnvironment(docker_image={})",
+      "ProcessConfigFromEnvironment(platform={}, docker_image={})",
+      String::from(self.platform),
       self.docker_image.as_ref().unwrap_or(&"None".to_owned())
     )
   }

src/rust/engine/src/nodes.rs

@@ -32,7 +32,7 @@ use fs::{
   RelativePath, StrictGlobMatching, Vfs,
 };
 use process_execution::{
-  self, CacheName, InputDigests, Platform, Process, ProcessCacheScope, ProcessResultSource,
+  self, CacheName, InputDigests, Process, ProcessCacheScope, ProcessResultSource,
 };
 
 use crate::externs::engine_aware::{EngineAwareParameter, EngineAwareReturnType};
@@ -386,13 +386,6 @@ impl ExecuteProcess {
         .try_into()?
     };
 
-    let platform_constraint =
-      if let Some(p) = externs::getattr_as_optional_string(value, "platform") {
-        Some(Platform::try_from(p)?)
-      } else {
-        None
-      };
-
     Ok(process_execution::Process {
       argv: externs::getattr(value, "argv").unwrap(),
       env,
@@ -405,7 +398,7 @@ impl ExecuteProcess {
       level,
       append_only_caches,
       jdk_home,
-      platform_constraint,
+      platform_constraint: Some(process_config.platform),
       execution_slot_variable,
       concurrency_available,
       cache_scope,