Support output directory saving for local process execution. (#5944)

Closes #5860

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

@@ -77,7 +77,15 @@ 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')
+    req = ExecuteProcessRequest(
+      cmd,
+      (),
+      directory_digest,
+      ('ignored', 'report'),
+      (),
+      15 * 60,
+      'cloc'
+    )
     exec_result = self.context.execute_process_synchronously(req, 'cloc', (WorkUnitLabel.TOOL,))
 
     # TODO: Remove this check when https://github.com/pantsbuild/pants/issues/5719 is resolved.

src/python/pants/engine/isolated_process.py

@@ -24,27 +24,54 @@ class ExecuteProcessRequest(datatype([
   ('env', tuple),
   ('input_files', DirectoryDigest),
   ('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(cls, argv, env, snapshot, output_files, timeout_seconds=_default_timeout_seconds, description='process'):
+  def create_from_snapshot(
+    cls,
+    argv,
+    env,
+    snapshot,
+    output_files=(),
+    output_directories=(),
+    timeout_seconds=_default_timeout_seconds,
+    description='process'
+  ):
     cls._verify_env_is_dict(env)
     return ExecuteProcessRequest(
       argv=argv,
       env=tuple(env.items()),
       input_files=snapshot.directory_digest,
       output_files=output_files,
+      output_directories=output_directories,
       timeout_seconds=timeout_seconds,
       description=description,
     )
 
   @classmethod
-  def create_with_empty_snapshot(cls, argv, env, output_files, timeout_seconds=_default_timeout_seconds, description='process'):
-    return cls.create_from_snapshot(argv, env, EMPTY_SNAPSHOT, output_files, timeout_seconds, description)
+  def create_with_empty_snapshot(
+    cls,
+    argv,
+    env,
+    output_files=(),
+    output_directories=(),
+    timeout_seconds=_default_timeout_seconds,
+    description='process'
+  ):
+    return cls.create_from_snapshot(
+      argv,
+      env,
+      EMPTY_SNAPSHOT,
+      output_files,
+      output_directories,
+      timeout_seconds,
+      description
+    )
 
   @classmethod
   def _verify_env_is_dict(cls, env):

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

@@ -59,6 +59,8 @@ pub struct ExecuteProcessRequest {
 
   pub output_files: BTreeSet<PathBuf>,
 
+  pub output_directories: BTreeSet<PathBuf>,
+
   pub timeout: std::time::Duration,
 
   pub description: String,

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

@@ -1,8 +1,10 @@
 extern crate tempfile;
 
 use boxfuture::{BoxFuture, Boxable};
-use fs::{self, PathStatGetter};
+use fs::{self, GlobMatching, PathGlobs, PathStatGetter, Snapshot, Store, StrictGlobMatching};
 use futures::{future, Future};
+use std::collections::BTreeSet;
+use std::path::PathBuf;
 use std::process::Command;
 use std::sync::Arc;
 
@@ -21,6 +23,51 @@ impl CommandRunner {
   pub fn new(store: fs::Store, fs_pool: Arc<fs::ResettablePool>) -> CommandRunner {
     CommandRunner { store, fs_pool }
   }
+
+  fn construct_output_snapshot(
+    store: Store,
+    posix_fs: Arc<fs::PosixFS>,
+    output_file_paths: BTreeSet<PathBuf>,
+    output_dir_paths: BTreeSet<PathBuf>,
+  ) -> BoxFuture<Snapshot, String> {
+    let output_dirs_glob_strings: Result<Vec<String>, String> = output_dir_paths
+      .into_iter()
+      .map(|p| {
+        p.into_os_string()
+          .into_string()
+          .map_err(|e| format!("Error stringifying output_directories: {:?}", e))
+          .map(|s| format!("{}/**", s))
+      })
+      .collect();
+
+    let output_dirs_future = posix_fs
+      .expand(try_future!(PathGlobs::create(
+        &try_future!(output_dirs_glob_strings),
+        &[],
+        StrictGlobMatching::Ignore,
+      )))
+      .map_err(|e| format!("Error stating output dirs: {}", e));
+
+    let output_files_future = posix_fs
+      .path_stats(output_file_paths.into_iter().collect())
+      .map_err(|e| format!("Error stating output files: {}", e));
+
+    output_files_future
+      .join(output_dirs_future)
+      .and_then(|(output_files_stats, output_dirs_stats)| {
+        let paths: Vec<_> = output_files_stats
+          .into_iter()
+          .chain(output_dirs_stats.into_iter().map(|p| Some(p)))
+          .collect();
+
+        fs::Snapshot::from_path_stats(
+          store.clone(),
+          fs::OneOffStoreFileByDigest::new(store, posix_fs),
+          paths.into_iter().filter_map(|v| v).collect(),
+        )
+      })
+      .to_boxed()
+  }
 }
 
 impl super::CommandRunner for CommandRunner {
@@ -42,6 +89,7 @@ impl super::CommandRunner for CommandRunner {
     let fs_pool = self.fs_pool.clone();
     let env = req.env;
     let output_file_paths = req.output_files;
+    let output_dir_paths = req.output_directories;
     let argv = req.argv;
     self
       .store
@@ -60,37 +108,36 @@ impl super::CommandRunner for CommandRunner {
                   .map(|output| (output, workdir))
       })
       .and_then(|(output, workdir)| {
-        let output_snapshot = if output_file_paths.is_empty() {
+        let output_snapshot = if output_file_paths.is_empty() && output_dir_paths.is_empty() {
           future::ok(fs::Snapshot::empty()).to_boxed()
         } else {
           // Use no ignore patterns, because we are looking for explicitly listed paths.
-          future::done(fs::PosixFS::new(
-                  workdir.path(),
-                  fs_pool,
-                  vec![],
-              )).map_err(|err| {
-                  format!(
-                      "Error making posix_fs to fetch local process execution output files: {}",
-                      err
-                  )
-              })
-                  .map(|posix_fs| Arc::new(posix_fs))
-                  .and_then(|posix_fs| {
-                      posix_fs
-                          .path_stats(output_file_paths.into_iter().collect())
-                          .map_err(|e| format!("Error stating output files: {}", e))
-                          .and_then(move |paths| {
-                              fs::Snapshot::from_path_stats(
-                                  store.clone(),
-                                  fs::OneOffStoreFileByDigest::new(store, posix_fs),
-                                  paths.into_iter().filter_map(|v| v).collect(),
-                              )
-                          })
-                  })
-                  // Force workdir not to get dropped until after we've ingested the outputs
-                  .map(|result| (result, workdir))
-                  .map(|(result, _workdir)| result)
-                  .to_boxed()
+          future::done(
+            fs::PosixFS::new(
+              workdir.path(),
+              fs_pool,
+              vec![],
+            )
+          )
+          .map_err(|err| {
+            format!(
+              "Error making posix_fs to fetch local process execution output files: {}",
+              err
+            )
+          })
+          .map(|posix_fs| Arc::new(posix_fs))
+          .and_then(|posix_fs| {
+            CommandRunner::construct_output_snapshot(
+              store,
+              posix_fs,
+              output_file_paths,
+              output_dir_paths
+            )
+          })
+          // Force workdir not to get dropped until after we've ingested the outputs
+          .map(|result| (result, workdir))
+          .map(|(result, _workdir)| result)
+          .to_boxed()
         };
 
         output_snapshot
@@ -139,6 +186,7 @@ mod tests {
       env: BTreeMap::new(),
       input_files: fs::EMPTY_DIGEST,
       output_files: BTreeSet::new(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "echo foo".to_string(),
     });
@@ -162,6 +210,7 @@ mod tests {
       env: BTreeMap::new(),
       input_files: fs::EMPTY_DIGEST,
       output_files: BTreeSet::new(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "echo foo and fail".to_string(),
     });
@@ -189,6 +238,7 @@ mod tests {
       env: env.clone(),
       input_files: fs::EMPTY_DIGEST,
       output_files: BTreeSet::new(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "run env".to_string(),
     });
@@ -223,6 +273,7 @@ mod tests {
         env: env,
         input_files: fs::EMPTY_DIGEST,
         output_files: BTreeSet::new(),
+        output_directories: BTreeSet::new(),
         timeout: Duration::from_millis(1000),
         description: "run env".to_string(),
       }
@@ -241,6 +292,7 @@ mod tests {
       env: BTreeMap::new(),
       input_files: fs::EMPTY_DIGEST,
       output_files: BTreeSet::new(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "echo foo".to_string(),
     }).expect_err("Want Err");
@@ -257,6 +309,7 @@ mod tests {
       env: BTreeMap::new(),
       input_files: fs::EMPTY_DIGEST,
       output_files: BTreeSet::new(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "bash".to_string(),
     });
@@ -282,6 +335,7 @@ mod tests {
       env: BTreeMap::new(),
       input_files: fs::EMPTY_DIGEST,
       output_files: vec![PathBuf::from("roland")].into_iter().collect(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "bash".to_string(),
     });
@@ -297,6 +351,38 @@ mod tests {
     )
   }
 
+  #[test]
+  fn output_dirs() {
+    let result = run_command_locally_in_dir(ExecuteProcessRequest {
+      argv: vec![
+        find_bash(),
+        "-c".to_owned(),
+        format!(
+          "/bin/mkdir cats && echo -n {} > {} ; echo -n {} > treats",
+          TestData::roland().string(),
+          "cats/roland",
+          TestData::catnip().string()
+        ),
+      ],
+      env: BTreeMap::new(),
+      input_files: fs::EMPTY_DIGEST,
+      output_files: vec![PathBuf::from("treats")].into_iter().collect(),
+      output_directories: vec![PathBuf::from("cats")].into_iter().collect(),
+      timeout: Duration::from_millis(1000),
+      description: "bash".to_string(),
+    });
+
+    assert_eq!(
+      result.unwrap(),
+      ExecuteProcessResult {
+        stdout: as_bytes(""),
+        stderr: as_bytes(""),
+        exit_code: 0,
+        output_directory: TestDirectory::recursive().digest(),
+      }
+    )
+  }
+
   #[test]
   fn output_files_many() {
     let result = run_command_locally_in_dir(ExecuteProcessRequest {
@@ -314,6 +400,7 @@ mod tests {
       output_files: vec![PathBuf::from("cats/roland"), PathBuf::from("treats")]
         .into_iter()
         .collect(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "treats-roland".to_string(),
     });
@@ -344,6 +431,7 @@ mod tests {
       env: BTreeMap::new(),
       input_files: fs::EMPTY_DIGEST,
       output_files: vec![PathBuf::from("roland")].into_iter().collect(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "echo foo".to_string(),
     });
@@ -372,6 +460,7 @@ mod tests {
       output_files: vec![PathBuf::from("roland"), PathBuf::from("susannah")]
         .into_iter()
         .collect(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "echo-roland".to_string(),
     });

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

@@ -661,6 +661,7 @@ mod tests {
         .into_iter()
         .map(|p| PathBuf::from(p))
         .collect(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "some description".to_owned(),
     };
@@ -707,6 +708,7 @@ mod tests {
           env: BTreeMap::new(),
           input_files: fs::EMPTY_DIGEST,
           output_files: BTreeSet::new(),
+          output_directories: BTreeSet::new(),
           timeout: Duration::from_millis(1000),
           description: "wrong command".to_string(),
         }).unwrap()
@@ -920,6 +922,7 @@ mod tests {
       env: BTreeMap::new(),
       input_files: fs::EMPTY_DIGEST,
       output_files: BTreeSet::new(),
+      output_directories: BTreeSet::new(),
       timeout: request_timeout,
       description: "echo-a-foo".to_string(),
     };
@@ -1539,6 +1542,7 @@ mod tests {
       env: BTreeMap::new(),
       input_files: fs::EMPTY_DIGEST,
       output_files: BTreeSet::new(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(5000),
       description: "echo a foo".to_string(),
     }
@@ -1715,6 +1719,7 @@ mod tests {
       env: BTreeMap::new(),
       input_files: TestDirectory::containing_roland().digest(),
       output_files: BTreeSet::new(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "cat a roland".to_string(),
     }
@@ -1726,6 +1731,7 @@ mod tests {
       env: BTreeMap::new(),
       input_files: fs::EMPTY_DIGEST,
       output_files: BTreeSet::new(),
+      output_directories: BTreeSet::new(),
       timeout: Duration::from_millis(1000),
       description: "unleash a roaring meow".to_string(),
     }

src/rust/engine/process_executor/src/main.rs

@@ -127,6 +127,7 @@ fn main() {
     env,
     input_files,
     output_files: BTreeSet::new(),
+    output_directories: BTreeSet::new(),
     timeout: Duration::new(15 * 60, 0),
     description: "process_executor".to_string(),
   };