Deduplicate Platform and PlatformConstraint

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

@@ -636,9 +636,7 @@ async def create_pex(
     # 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, MultiPlatformProcess({PlatformConstraint(platform.value): process})
-    )
+    result = await Get(ProcessResult, MultiPlatformProcess({PlatformConstraint(platform): process}))
 
     if pex_runtime_env.verbosity > 0:
         log_output = result.stderr.decode()

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

@@ -24,7 +24,8 @@
 )
 from pants.engine.internals.scheduler import ExecutionError
 from pants.engine.internals.scheduler_test_base import SchedulerTestBase
-from pants.engine.platform import PlatformConstraint, create_platform_rules
+from pants.engine.platform import PlatformConstraint
+from pants.engine.platform import rules as platform_rules
 from pants.engine.process import MultiPlatformProcess, Process, ProcessResult
 from pants.engine.process import rules as process_rules
 from pants.engine.rules import Get, MultiGet, rule
@@ -657,10 +658,10 @@ async def a_rule() -> TrueResult:
                 ["/bin/sh", "-c", "true"],
                 description="always true",
             )
-            _ = await Get(ProcessResult, MultiPlatformProcess({PlatformConstraint.none: proc}))
+            _ = await Get(ProcessResult, MultiPlatformProcess({PlatformConstraint(None): proc}))
             return TrueResult()
 
-        rules = [a_rule, QueryRule(TrueResult, tuple()), *process_rules(), *create_platform_rules()]
+        rules = [a_rule, QueryRule(TrueResult, tuple()), *process_rules(), *platform_rules()]
 
         scheduler = self.mk_scheduler(
             rules, include_trace_on_error=False, should_report_workunits=True

src/python/pants/engine/platform.py

@@ -1,8 +1,9 @@
 # Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+from dataclasses import dataclass
 from enum import Enum
-from typing import Iterable
+from typing import Iterable, Optional
 
 from pants.engine.rules import Rule, collect_rules, rule
 from pants.util.memo import memoized_classproperty
@@ -19,22 +20,17 @@ def current(cls) -> "Platform":
         return Platform(get_normalized_os_name())
 
 
-class PlatformConstraint(Enum):
-    darwin = "darwin"
-    linux = "linux"
-    none = "none"
-
-    @memoized_classproperty
-    def local_platform(cls) -> "PlatformConstraint":
-        return PlatformConstraint(Platform.current.value)
+@dataclass(frozen=True)
+class PlatformConstraint:
+    platform: Optional[Platform]
 
 
 # TODO We will want to allow users to specify the execution platform for rules,
 # which means replacing this singleton rule with a RootRule populated by an option.
 @rule
-def materialize_platform() -> Platform:
+def current_platform() -> Platform:
     return Platform.current
 
 
-def create_platform_rules() -> Iterable[Rule]:
+def rules() -> Iterable[Rule]:
     return collect_rules()

src/python/pants/engine/process.py

@@ -117,20 +117,23 @@ def __init__(
 @frozen_after_init
 @dataclass(unsafe_hash=True)
 class MultiPlatformProcess:
-    platform_constraints: Tuple[str, ...]
+    platform_constraints: Tuple[Optional[str], ...]
     processes: Tuple[Process, ...]
 
     def __init__(self, request_dict: Dict[PlatformConstraint, Process]) -> None:
         if len(request_dict) == 0:
             raise ValueError("At least one platform constrained Process must be passed.")
-        validated_constraints = tuple(constraint.value for constraint in request_dict)
+        serialized_constraints = tuple(
+            constraint.platform.value if constraint.platform else None
+            for constraint in request_dict
+        )
         if len([req.description for req in request_dict.values()]) != 1:
             raise ValueError(
                 f"The `description` of all processes in a {MultiPlatformProcess.__name__} must "
                 f"be identical, but got: {list(request_dict.values())}."
             )
 
-        self.platform_constraints = validated_constraints
+        self.platform_constraints = serialized_constraints
         self.processes = tuple(request_dict.values())
 
     @property
@@ -211,7 +214,7 @@ def get_multi_platform_request_description(req: MultiPlatformProcess) -> Product
 @rule
 def upcast_process(req: Process) -> MultiPlatformProcess:
     """This rule allows an Process to be run as a platform compatible MultiPlatformProcess."""
-    return MultiPlatformProcess({PlatformConstraint.none: req})
+    return MultiPlatformProcess({PlatformConstraint(None): req})
 
 
 @rule

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 desktop, fs, process
+from pants.engine import desktop, fs, platform, process
 from pants.engine.console import Console
 from pants.engine.fs import PathGlobs, Snapshot, Workspace
 from pants.engine.goal import Goal
@@ -21,7 +21,6 @@
 from pants.engine.internals.scheduler import Scheduler, SchedulerSession
 from pants.engine.internals.selectors import Params
 from pants.engine.internals.session import SessionValues
-from pants.engine.platform import create_platform_rules
 from pants.engine.process import InteractiveRunner
 from pants.engine.rules import QueryRule, collect_rules, rule
 from pants.engine.target import RegisteredTargetTypes
@@ -240,7 +239,7 @@ def build_root_singleton() -> BuildRoot:
                 *uuid.rules(),
                 *options_parsing.rules(),
                 *process.rules(),
-                *create_platform_rules(),
+                *platform.rules(),
                 *changed_rules(),
                 *specs_calculator.rules(),
                 *rules,

src/rust/engine/process_execution/src/lib.rs

@@ -84,7 +84,6 @@ impl Platform {
   pub fn current() -> Result<Platform, String> {
     let platform_info =
       uname::uname().map_err(|_| "Failed to get local platform info!".to_string())?;
-
     match platform_info {
       uname::Info { ref sysname, .. } if sysname.to_lowercase() == "darwin" => Ok(Platform::Darwin),
       uname::Info { ref sysname, .. } if sysname.to_lowercase() == "linux" => Ok(Platform::Linux),
@@ -93,62 +92,63 @@ impl Platform {
   }
 }
 
-#[derive(PartialOrd, Ord, Clone, Copy, Debug, Eq, PartialEq, Hash)]
-pub enum PlatformConstraint {
-  Darwin,
-  Linux,
-  None,
-}
-
-impl PlatformConstraint {
-  pub fn current_platform_constraint() -> Result<PlatformConstraint, String> {
-    Platform::current().map(|p: Platform| p.into())
-  }
-}
-
-impl From<Platform> for PlatformConstraint {
-  fn from(platform: Platform) -> PlatformConstraint {
+impl From<Platform> for String {
+  fn from(platform: Platform) -> String {
     match platform {
-      Platform::Linux => PlatformConstraint::Linux,
-      Platform::Darwin => PlatformConstraint::Darwin,
+      Platform::Linux => "linux".to_string(),
+      Platform::Darwin => "darwin".to_string(),
     }
   }
 }
 
-impl TryFrom<&String> for PlatformConstraint {
+impl TryFrom<&String> for Platform {
   type Error = String;
-  ///
-  /// This is a helper method to convert values from the python/engine/platform.py::PlatformConstraint enum,
-  /// which have been serialized, into the rust PlatformConstraint enum.
-  ///
   fn try_from(variant_candidate: &String) -> Result<Self, Self::Error> {
     match variant_candidate.as_ref() {
-      "darwin" => Ok(PlatformConstraint::Darwin),
-      "linux" => Ok(PlatformConstraint::Linux),
-      "none" => Ok(PlatformConstraint::None),
+      "darwin" => Ok(Platform::Darwin),
+      "linux" => Ok(Platform::Linux),
       other => Err(format!(
-        "Unknown, platform {:?} encountered in parsing",
+        "Unknown platform {:?} encountered in parsing",
         other
       )),
     }
   }
 }
 
-impl From<Platform> for String {
-  fn from(platform: Platform) -> String {
-    match platform {
-      Platform::Linux => "linux".to_string(),
-      Platform::Darwin => "darwin".to_string(),
-    }
+#[derive(PartialOrd, Ord, Clone, Copy, Debug, Eq, PartialEq, Hash)]
+pub struct PlatformConstraint {
+  platform: Option<Platform>,
+}
+
+impl PlatformConstraint {
+  pub fn new(platform: Option<Platform>) -> PlatformConstraint {
+    PlatformConstraint { platform }
+  }
+}
+
+impl From<Platform> for PlatformConstraint {
+  fn from(platform: Platform) -> PlatformConstraint {
+    PlatformConstraint::new(Some(platform))
   }
 }
 
 impl From<PlatformConstraint> for String {
-  fn from(platform: PlatformConstraint) -> String {
-    match platform {
-      PlatformConstraint::Linux => "linux".to_string(),
-      PlatformConstraint::Darwin => "darwin".to_string(),
-      PlatformConstraint::None => "none".to_string(),
+  fn from(constraint: PlatformConstraint) -> String {
+    match constraint.platform {
+      Some(plat) => plat.into(),
+      None => "none".to_string(),
+    }
+  }
+}
+
+impl TryFrom<Option<String>> for PlatformConstraint {
+  type Error = String;
+  fn try_from(maybe_constraint: Option<String>) -> Result<Self, Self::Error> {
+    match maybe_constraint {
+      Some(constraint) => {
+        Platform::try_from(&constraint).map(|plat| PlatformConstraint::new(Some(plat)))
+      }
+      None => Ok(PlatformConstraint::new(None)),
     }
   }
 }
@@ -222,7 +222,8 @@ pub struct Process {
   /// see https://github.com/pantsbuild/pants/issues/6416.
   ///
   pub jdk_home: Option<PathBuf>,
-  pub target_platform: PlatformConstraint,
+
+  pub platform_constraint: PlatformConstraint,
 
   pub is_nailgunnable: bool,
 
@@ -253,7 +254,7 @@ impl Process {
       level: log::Level::Info,
       append_only_caches: BTreeMap::new(),
       jdk_home: None,
-      target_platform: PlatformConstraint::None,
+      platform_constraint: PlatformConstraint::new(None),
       is_nailgunnable: false,
       execution_slot_variable: None,
       cache_failures: false,
@@ -300,7 +301,7 @@ impl TryFrom<MultiPlatformProcess> for Process {
   type Error = String;
 
   fn try_from(req: MultiPlatformProcess) -> Result<Self, Self::Error> {
-    match req.0.get(&PlatformConstraint::None) {
+    match req.0.get(&PlatformConstraint::new(None)) {
       Some(crossplatform_req) => Ok(crossplatform_req.clone()),
       None => Err(String::from(
         "Cannot coerce to a simple Process, no cross platform request exists.",
@@ -340,8 +341,12 @@ impl MultiPlatformProcess {
 }
 
 impl From<Process> for MultiPlatformProcess {
-  fn from(req: Process) -> Self {
-    MultiPlatformProcess(vec![(PlatformConstraint::None, req)].into_iter().collect())
+  fn from(proc: Process) -> Self {
+    MultiPlatformProcess(
+      vec![(PlatformConstraint::new(None), proc)]
+        .into_iter()
+        .collect(),
+    )
   }
 }
 

src/rust/engine/process_execution/src/local.rs

@@ -220,7 +220,7 @@ impl ChildResults {
 #[async_trait]
 impl super::CommandRunner for CommandRunner {
   fn extract_compatible_request(&self, req: &MultiPlatformProcess) -> Option<Process> {
-    for compatible_constraint in vec![PlatformConstraint::None, self.platform.into()].iter() {
+    for compatible_constraint in vec![PlatformConstraint::new(None), self.platform.into()].iter() {
       if let Some(compatible_req) = req.0.get(compatible_constraint) {
         return Some(compatible_req.clone());
       }

src/rust/engine/process_execution/src/nailgun/mod.rs

@@ -62,7 +62,7 @@ fn construct_nailgun_server_request(
     level: log::Level::Info,
     append_only_caches: BTreeMap::new(),
     jdk_home: Some(jdk),
-    target_platform: platform_constraint,
+    platform_constraint,
     is_nailgunnable: true,
     execution_slot_variable: None,
     cache_failures: false,
@@ -213,8 +213,12 @@ impl CapturedWorkdir for CommandRunner {
       .jdk_home
       .clone()
       .ok_or("JDK home must be specified for all nailgunnable requests.")?;
-    let nailgun_req =
-      construct_nailgun_server_request(&nailgun_name, nailgun_args, jdk_home, req.target_platform);
+    let nailgun_req = construct_nailgun_server_request(
+      &nailgun_name,
+      nailgun_args,
+      jdk_home,
+      req.platform_constraint,
+    );
     trace!("Extracted nailgun request:\n {:#?}", &nailgun_req);
 
     let nailgun_req_digest = crate::digest(

src/rust/engine/process_execution/src/nailgun/tests.rs

@@ -41,7 +41,7 @@ fn mock_nailgunnable_request(jdk_home: Option<PathBuf>) -> Process {
   let mut process = Process::new(vec![]);
   process.jdk_home = jdk_home;
   process.is_nailgunnable = true;
-  process.target_platform = PlatformConstraint::Darwin;
+  process.platform_constraint = PlatformConstraint::Darwin;
   process
 }
 

src/rust/engine/process_execution/src/remote.rs

@@ -790,7 +790,7 @@ impl crate::CommandRunner for CommandRunner {
 
   // TODO: This is a copy of the same method on crate::remote::CommandRunner.
   fn extract_compatible_request(&self, req: &MultiPlatformProcess) -> Option<Process> {
-    for compatible_constraint in vec![PlatformConstraint::None, self.platform.into()].iter() {
+    for compatible_constraint in vec![PlatformConstraint::new(None), self.platform.into()].iter() {
       if let Some(compatible_req) = req.0.get(compatible_constraint) {
         return Some(compatible_req.clone());
       }
@@ -873,7 +873,7 @@ pub fn make_execute_request(
   {
     let mut env = bazel_protos::remote_execution::Command_EnvironmentVariable::new();
     env.set_name(CACHE_KEY_TARGET_PLATFORM_ENV_VAR_NAME.to_string());
-    env.set_value(req.target_platform.into());
+    env.set_value(req.platform_constraint.into());
     command.mut_environment_variables().push(env);
   }
 

src/rust/engine/process_execution/src/remote_tests.rs

@@ -79,7 +79,7 @@ async fn make_execute_request() {
     level: log::Level::Info,
     append_only_caches: BTreeMap::new(),
     jdk_home: None,
-    target_platform: PlatformConstraint::None,
+    platform_constraint: PlatformConstraint::new(None),
     is_nailgunnable: false,
     execution_slot_variable: None,
     cache_failures: false,
@@ -160,7 +160,7 @@ async fn make_execute_request_with_instance_name() {
     level: log::Level::Info,
     append_only_caches: BTreeMap::new(),
     jdk_home: None,
-    target_platform: PlatformConstraint::None,
+    platform_constraint: PlatformConstraint::new(None),
     is_nailgunnable: false,
     execution_slot_variable: None,
     cache_failures: false,
@@ -254,7 +254,7 @@ async fn make_execute_request_with_cache_key_gen_version() {
     level: log::Level::Info,
     append_only_caches: BTreeMap::new(),
     jdk_home: None,
-    target_platform: PlatformConstraint::None,
+    platform_constraint: PlatformConstraint::new(None),
     is_nailgunnable: false,
     execution_slot_variable: None,
     cache_failures: false,
@@ -497,7 +497,7 @@ async fn make_execute_request_with_timeout() {
     level: log::Level::Info,
     append_only_caches: BTreeMap::new(),
     jdk_home: None,
-    target_platform: PlatformConstraint::None,
+    platform_constraint: PlatformConstraint::new(None),
     is_nailgunnable: false,
     execution_slot_variable: None,
     cache_failures: false,

src/rust/engine/process_execution/src/speculate_tests.rs

@@ -258,7 +258,7 @@ impl CommandRunner for DelayedCommandRunner {
 
   fn extract_compatible_request(&self, req: &MultiPlatformProcess) -> Option<Process> {
     if self.is_compatible {
-      Some(req.0.get(&PlatformConstraint::None).unwrap().clone())
+      Some(req.0.get(&PlatformConstraint::new(None)).unwrap().clone())
     } else {
       None
     }