Simplify ExecuteProcessRequest construction (#6345)

Remove create_from_snapshot and create_with_empty_snapshot - they don't
pull their weight.

Allow default values for all defaultable values.

src/python/pants/backend/graph_info/tasks/cloc.py

@@ -75,13 +75,10 @@ def console_output(self, targets):
 
     # The cloc script reaches into $PATH to look up perl. Let's assume it's in /usr/bin.
     req = ExecuteProcessRequest(
-      cmd,
-      (),
-      directory_digest,
-      ('ignored', 'report'),
-      (),
-      15 * 60,
-      'cloc'
+      argv=cmd,
+      input_files=directory_digest,
+      output_files=('ignored', 'report'),
+      description='cloc',
     )
     exec_result = self.context.execute_process_synchronously(req, 'cloc', (WorkUnitLabel.TOOL,))
 

src/python/pants/backend/jvm/tasks/jvm_compile/javac/javac_compile.py

@@ -216,9 +216,9 @@ def _execute_hermetic_compile(self, cmd, ctx):
       os.path.relpath(f.path.replace('.java', '.class'), ctx.target.target_base)
       for f in input_snapshot.files if f.path.endswith('.java')
     )
-    exec_process_request = ExecuteProcessRequest.create_from_snapshot(
+    exec_process_request = ExecuteProcessRequest(
       argv=tuple(cmd),
-      snapshot=input_snapshot,
+      input_files=input_snapshot.directory_digest,
       output_files=output_files,
       description='Compiling {} with javac'.format(ctx.target.address.spec),
     )

src/python/pants/engine/isolated_process.py

@@ -8,7 +8,7 @@
 
 import six
 
-from pants.engine.fs import EMPTY_SNAPSHOT, DirectoryDigest
+from pants.engine.fs import DirectoryDigest
 from pants.engine.rules import RootRule, rule
 from pants.engine.selectors import Select
 from pants.util.objects import Exactly, SubclassesOf, TypeCheckError, datatype
@@ -21,21 +21,20 @@
 
 class ExecuteProcessRequest(datatype([
   ('argv', tuple),
-  ('env', tuple),
   ('input_files', DirectoryDigest),
+  ('description', SubclassesOf(*six.string_types)),
+  ('env', tuple),
   ('output_files', tuple),
   ('output_directories', tuple),
   # NB: timeout_seconds covers the whole remote operation including queuing and setup.
   ('timeout_seconds', Exactly(float, int)),
-  ('description', SubclassesOf(*six.string_types)),
 ])):
   """Request for execution with args and snapshots to extract."""
 
-  @classmethod
-  def create_from_snapshot(
+  def __new__(
     cls,
     argv,
-    snapshot,
+    input_files,
     description,
     env=None,
     output_files=(),
@@ -45,50 +44,27 @@ def create_from_snapshot(
     if env is None:
       env = ()
     else:
-      cls._verify_env_is_dict(env)
+      if not isinstance(env, dict):
+        raise TypeCheckError(
+          cls.__name__,
+          "arg 'env' was invalid: value {} (with type {}) must be a dict".format(
+            env,
+            type(env)
+          )
+        )
       env = tuple(item for pair in env.items() for item in pair)
 
-    return ExecuteProcessRequest(
+    return super(ExecuteProcessRequest, cls).__new__(
+      cls,
       argv=argv,
       env=env,
-      input_files=snapshot.directory_digest,
+      input_files=input_files,
+      description=description,
       output_files=output_files,
       output_directories=output_directories,
       timeout_seconds=timeout_seconds,
-      description=description,
     )
 
-  @classmethod
-  def create_with_empty_snapshot(
-    cls,
-    argv,
-    description,
-    env=None,
-    output_files=(),
-    output_directories=(),
-    timeout_seconds=_default_timeout_seconds,
-  ):
-    return cls.create_from_snapshot(
-      argv,
-      EMPTY_SNAPSHOT,
-      description,
-      env,
-      output_files,
-      output_directories,
-      timeout_seconds,
-    )
-
-  @classmethod
-  def _verify_env_is_dict(cls, env):
-    if not isinstance(env, dict):
-      raise TypeCheckError(
-        cls.__name__,
-        "arg 'env' was invalid: value {} (with type {}) must be a dict".format(
-          env,
-          type(env)
-        )
-      )
-
 
 class ExecuteProcessResult(datatype(['stdout', 'stderr', 'output_directory_digest'])):
   """Result of successfully executing a process.

tests/python/pants_test/engine/test_isolated_process.py

@@ -72,9 +72,9 @@ class CatExecutionRequest(datatype([('shell_cat', ShellCat), ('path_globs', Path
 def cat_files_process_result_concatted(cat_exe_req):
   cat_bin = cat_exe_req.shell_cat
   cat_files_snapshot = yield Get(Snapshot, PathGlobs, cat_exe_req.path_globs)
-  process_request = ExecuteProcessRequest.create_from_snapshot(
+  process_request = ExecuteProcessRequest(
     argv=cat_bin.argv_from_snapshot(cat_files_snapshot),
-    snapshot=cat_files_snapshot,
+    input_files=cat_files_snapshot.directory_digest,
     description='cat some files',
   )
   cat_process_result = yield Get(ExecuteProcessResult, ExecuteProcessRequest, process_request)
@@ -102,9 +102,10 @@ def gen_argv(self):
 
 @rule(ExecuteProcessRequest, [Select(JavacVersionExecutionRequest)])
 def process_request_from_javac_version(javac_version_exe_req):
-  yield ExecuteProcessRequest.create_with_empty_snapshot(
+  yield ExecuteProcessRequest(
     argv=javac_version_exe_req.gen_argv(),
     description=javac_version_exe_req.description,
+    input_files=EMPTY_DIRECTORY_DIGEST,
   )
 
 
@@ -168,9 +169,9 @@ def javac_compile_process_result(javac_compile_req):
       raise ValueError("Can only compile .java files but got {}".format(java_file))
   sources_snapshot = yield Get(Snapshot, PathGlobs, PathGlobs(java_files, ()))
   output_dirs = tuple({os.path.dirname(java_file) for java_file in java_files})
-  process_request = ExecuteProcessRequest.create_from_snapshot(
+  process_request = ExecuteProcessRequest(
     argv=javac_compile_req.argv_from_source_snapshot(sources_snapshot),
-    snapshot=sources_snapshot,
+    input_files=sources_snapshot.directory_digest,
     output_directories=output_dirs,
     description='javac compilation'
   )
@@ -196,10 +197,11 @@ def create_javac_compile_rules():
 class ExecuteProcessRequestTest(unittest.TestCase):
   def _default_args_execute_process_request(self, argv=tuple(), env=None):
     env = env or dict()
-    return ExecuteProcessRequest.create_with_empty_snapshot(
+    return ExecuteProcessRequest(
       argv=argv,
       description='',
       env=env,
+      input_files=EMPTY_DIRECTORY_DIGEST,
       output_files=(),
     )
 
@@ -219,7 +221,7 @@ def test_blows_up_on_invalid_args(self):
     with self.assertRaisesRegexp(TypeCheckError, "env"):
       ExecuteProcessRequest(
         argv=('1',),
-        env=dict(),
+        env=(),
         input_files='',
         output_files=(),
         output_directories=(),
@@ -238,7 +240,7 @@ def test_blows_up_on_invalid_args(self):
     with self.assertRaisesRegexp(TypeCheckError, "output_files"):
       ExecuteProcessRequest(
         argv=('1',),
-        env=tuple(),
+        env=dict(),
         input_files=EMPTY_DIRECTORY_DIGEST,
         output_files=("blah"),
         output_directories=(),
@@ -248,7 +250,7 @@ def test_blows_up_on_invalid_args(self):
     with self.assertRaisesRegexp(TypeCheckError, "timeout"):
       ExecuteProcessRequest(
         argv=('1',),
-        env=tuple(),
+        env=dict(),
         input_files=EMPTY_DIRECTORY_DIGEST,
         output_files=("blah"),
         output_directories=(),
@@ -257,10 +259,11 @@ def test_blows_up_on_invalid_args(self):
       )
 
   def test_create_from_snapshot_with_env(self):
-    req = ExecuteProcessRequest.create_with_empty_snapshot(
+    req = ExecuteProcessRequest(
       argv=('foo',),
       description="Some process",
       env={'VAR': 'VAL'},
+      input_files=EMPTY_DIRECTORY_DIGEST,
     )
     self.assertEqual(req.env, ('VAR', 'VAL'))
 
@@ -294,10 +297,11 @@ def test_javac_version_example(self):
     self.assertIn('javac', result.value)
 
   def test_write_file(self):
-    request = ExecuteProcessRequest.create_with_empty_snapshot(
+    request = ExecuteProcessRequest(
       argv=("/bin/bash", "-c", "echo -n 'European Burmese' > roland"),
       description="echo roland",
-      output_files=("roland",)
+      output_files=("roland",),
+      input_files=EMPTY_DIRECTORY_DIGEST,
     )
 
     execute_process_result = self.scheduler.product_request(
@@ -328,10 +332,11 @@ def test_exercise_python_side_of_timeout_implementation(self):
     # but this allows us to ensure that all of the setup
     # on the python side does not blow up.
 
-    request = ExecuteProcessRequest.create_with_empty_snapshot(
+    request = ExecuteProcessRequest(
       argv=("/bin/bash", "-c", "/bin/sleep 1; echo -n 'European Burmese'"),
       timeout_seconds=0.1,
       description='sleepy-cat',
+      input_files=EMPTY_DIRECTORY_DIGEST,
     )
 
     self.scheduler.product_request(ExecuteProcessResult, [request])[0]
@@ -383,19 +388,21 @@ class Broken {
     self.assertIn("NOT VALID JAVA", e.stderr)
 
   def test_fallible_failing_command_returns_exited_result(self):
-    request = ExecuteProcessRequest.create_with_empty_snapshot(
+    request = ExecuteProcessRequest(
       argv=("/bin/bash", "-c", "exit 1"),
       description='one-cat',
+      input_files=EMPTY_DIRECTORY_DIGEST,
     )
 
     result = self.scheduler.product_request(FallibleExecuteProcessResult, [request])[0]
 
     self.assertEqual(result.exit_code, 1)
 
   def test_non_fallible_failing_command_raises(self):
-    request = ExecuteProcessRequest.create_with_empty_snapshot(
+    request = ExecuteProcessRequest(
       argv=("/bin/bash", "-c", "exit 1"),
       description='one-cat',
+      input_files=EMPTY_DIRECTORY_DIGEST,
     )
 
     with self.assertRaises(ExecutionError) as cm: