Materialize "large" files in a new store location and hardlink them in sandboxes

conftest.py

@@ -1,6 +1,9 @@
 # Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
+import glob
+import os
+import shutil
 
 import pytest
 
@@ -32,3 +35,19 @@ def pytest_addoption(parser):
     parser.addoption(
         "--noskip", action="store_true", default=False, help="Treat skipped tests as errors"
     )
+
+
+def pytest_runtestloop():
+    # These files are so big they exceed our "immutable symlink" hueristic. However, our integration
+    # tests run Pants in the sandbox, and Pants doesn't allow globbing absolute symlinks.
+    # Therefore we re-materialize them from their symlink.
+    for fileglob in [
+        "*.pex/.bootstrap",
+        "pantsbuild.pants*.whl",
+        "src/python/pants/engine/internals/native_engine.so",
+    ]:
+        for filepath in glob.iglob(fileglob):
+            if os.path.islink(filepath):
+                actual = os.path.realpath(filepath)
+                os.unlink(filepath)
+                shutil.copyfile(actual, filepath)

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

@@ -100,6 +100,10 @@ async def create_archive(request: CreateArchive) -> Digest:
             output_dir_digest = await Get(Digest, CreateDigest([Directory(output_dir)]))
             input_digests.append(output_dir_digest)
 
+    # @TODO: Excludelist from immutable input symlinking everything, as `tar` is symlink-aware
+    # See https://github.com/pantsbuild/pants/issues/17712.
+    # (Currently we ask tar to collect using -h)
+
     input_digest = await Get(Digest, MergeDigests([*files_digests, *input_digests]))
 
     result = await Get(

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

@@ -327,7 +327,8 @@ def create_archive_argv(
         )
 
         files_from = ("--files-from", input_file_list_filename) if input_file_list_filename else ()
-        return (self.path, f"c{compression}f", output_filename, *input_files) + files_from
+        # NB: -h follows symlinks, which ensures we don't capture a symlink to an immutable input, for instance
+        return (self.path, f"c{compression}fh", output_filename, *input_files) + files_from
 
     def extract_archive_argv(
         self, archive_path: str, extract_path: str, *, archive_suffix: str

src/rust/engine/fs/fs_util/src/main.rs

@@ -523,8 +523,8 @@ async fn execute(top_match: &clap::ArgMatches) -> Result<(), ExitError> {
             .materialize_directory(
               destination,
               output_digest,
+              false,
               &BTreeSet::new(),
-              None,
               Permissions::Writable,
             )
             .await?,
@@ -547,8 +547,8 @@ async fn execute(top_match: &clap::ArgMatches) -> Result<(), ExitError> {
             .materialize_directory(
               destination,
               digest,
+              false,
               &BTreeSet::new(),
-              None,
               Permissions::Writable,
             )
             .await?,

src/rust/engine/fs/store/benches/store.rs

@@ -43,7 +43,7 @@ use protos::gen::build::bazel::remote::execution::v2 as remexec;
 use task_executor::Executor;
 use tempfile::TempDir;
 
-use store::{ImmutableInputs, OneOffStoreFileByDigest, Snapshot, SnapshotOps, Store, SubsetParams};
+use store::{OneOffStoreFileByDigest, Snapshot, SnapshotOps, Store, SubsetParams};
 
 fn executor() -> Executor {
   Executor::new_owned(num_cpus::get(), num_cpus::get() * 4, || ()).unwrap()
@@ -62,9 +62,6 @@ pub fn criterion_benchmark_materialize(c: &mut Criterion) {
       let (store, _tempdir, digest) = snapshot(&executor, count, size);
       let parent_dest = TempDir::new().unwrap();
       let parent_dest_path = parent_dest.path();
-      let immutable_inputs_dest = TempDir::new().unwrap();
-      let immutable_inputs_path = immutable_inputs_dest.path();
-      let immutable_inputs = ImmutableInputs::new(store.clone(), immutable_inputs_path).unwrap();
 
       cgroup
         .sample_size(10)
@@ -81,8 +78,8 @@ pub fn criterion_benchmark_materialize(c: &mut Criterion) {
                 .block_on(store.materialize_directory(
                   dest,
                   digest.clone(),
+                  false,
                   &BTreeSet::new(),
-                  Some(&immutable_inputs),
                   perms,
                 ))
                 .unwrap();

src/rust/engine/fs/store/src/immutable_inputs.rs

@@ -51,69 +51,6 @@ impl ImmutableInputs {
     self.0.workdir.path()
   }
 
-  /// Returns an absolute Path to immutably consume the given Digest from.
-  pub(crate) async fn path_for_file(
-    &self,
-    digest: Digest,
-    is_executable: bool,
-  ) -> Result<PathBuf, StoreError> {
-    let cell = self.0.contents.lock().entry(digest).or_default().clone();
-
-    // We (might) need to initialize the value.
-    //
-    // Because this code executes a side-effect which could be observed elsewhere within this
-    // process (other threads can observe the contents of the temporary directory), we need to
-    // ensure that if this method is cancelled (via async Drop), whether the cell has been
-    // initialized or not stays in sync with whether the side-effect is visible.
-    //
-    // Making the initialization "cancellation safe", involves either:
-    //
-    //   1. Adding a Drop guard to "undo" the side-effect if we're dropped before we fully
-    //      initialize the cell.
-    //       * This is challenging to do correctly in this case, because the `Drop` guard cannot
-    //         be created until after initialization begins, but cannot be cleared until after the
-    //         cell has been initialized (i.e., after `get_or_try_init` returns).
-    //   2. Shielding ourselves from cancellation by `spawn`ing a new Task to guarantee that the
-    //      cell initialization always runs to completion.
-    //       * This would work, but would mean that we would finish initializing cells even when
-    //         work was cancelled. Cancellation usually means that the work is no longer necessary,
-    //         and so that could result in a lot of spurious IO (in e.g. warm cache cases which
-    //         never end up actually needing any inputs).
-    //       * An advanced variant of this approach would be to _pause_ work on materializing a
-    //         Digest when demand for it disappeared, and resume the work if another caller
-    //         requested that Digest.
-    //   3. Using anonymous destination paths, such that multiple attempts to initialize cannot
-    //      collide.
-    //       * This means that although the side-effect is visible, it can never collide.
-    //
-    // We take the final approach here currently (for simplicity's sake), but the advanced variant
-    // of approach 2 might eventually be worthwhile.
-    cell
-      .get_or_try_init(async {
-        let chroot = TempDir::new_in(self.0.workdir.path()).map_err(|e| {
-          format!(
-            "Failed to create a temporary directory for materialization of immutable input \
-          digest {digest:?}: {e}"
-          )
-        })?;
-
-        let dest = chroot.path().join(digest.hash.to_hex());
-        self
-          .0
-          .store
-          .materialize_file(dest.clone(), digest, Permissions::ReadOnly, is_executable)
-          .await?;
-
-        // Now that we've successfully initialized the destination, forget the TempDir so that it
-        // is not cleaned up.
-        let _ = chroot.into_path();
-
-        Ok(dest)
-      })
-      .await
-      .cloned()
-  }
-
   /// Returns an absolute Path to immutably consume the given Digest from.
   pub(crate) async fn path_for_dir(
     &self,
@@ -167,8 +104,8 @@ impl ImmutableInputs {
           .materialize_directory(
             dest.clone(),
             directory_digest,
+            false,
             &BTreeSet::new(),
-            Some(self),
             Permissions::ReadOnly,
           )
           .await?;