From 62394fbb68edb3844ca25e1c84aabaa0dcef0b2c Mon Sep 17 00:00:00 2001
From: Matt Mackay <matt@aspect.dev>
Date: Thu, 21 Sep 2023 16:27:08 -0400
Subject: [PATCH] fix: lookup source files from execroot when running diff_test
 on RBE (#531)

---
 e2e/copy_to_directory/BUILD.bazel             | 12 -----
 e2e/smoke/BUILD.bazel                         |  4 --
 lib/private/diff_test.bzl                     |  1 +
 lib/private/diff_test_tmpl.sh                 | 49 ++++++++++++++-----
 .../copy_directory_bin_action/BUILD.bazel     |  4 --
 .../copy_to_directory_bin_action/BUILD.bazel  |  2 -
 lib/tests/write_source_files/BUILD.bazel      | 14 ------
 7 files changed, 37 insertions(+), 49 deletions(-)

diff --git a/e2e/copy_to_directory/BUILD.bazel b/e2e/copy_to_directory/BUILD.bazel
index 95eb919..b5cc209 100644
--- a/e2e/copy_to_directory/BUILD.bazel
+++ b/e2e/copy_to_directory/BUILD.bazel
@@ -26,16 +26,12 @@ diff_test(
     name = "test1",
     file1 = ":dir1",
     file2 = ":expected1",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
 
 diff_test(
     name = "test1b",
     file1 = ":dir1b",
     file2 = ":expected1",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
 
 copy_to_directory(
@@ -64,16 +60,12 @@ diff_test(
     name = "test2",
     file1 = ":dir2",
     file2 = ":expected2",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
 
 diff_test(
     name = "test2b",
     file1 = ":dir2b",
     file2 = ":expected2",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
 
 copy_to_directory(
@@ -102,14 +94,10 @@ diff_test(
     name = "test3",
     file1 = ":dir3",
     file2 = ":expected3",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
 
 diff_test(
     name = "test3b",
     file1 = ":dir3b",
     file2 = ":expected3",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
diff --git a/e2e/smoke/BUILD.bazel b/e2e/smoke/BUILD.bazel
index 173eb88..d98e6e9 100644
--- a/e2e/smoke/BUILD.bazel
+++ b/e2e/smoke/BUILD.bazel
@@ -43,8 +43,6 @@ diff_test(
     name = "copy_directory_test",
     file1 = "d",
     file2 = "copy_directory_case",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
 
 # Validate that copy_to_directory works and resolves its toolchain
@@ -59,8 +57,6 @@ diff_test(
     name = "copy_to_directory_test",
     file1 = "d",
     file2 = "copy_to_directory_case",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
 
 # Validate that expand_template works and resolves its toolchain
diff --git a/lib/private/diff_test.bzl b/lib/private/diff_test.bzl
index 6e10e44..0d94ad4 100644
--- a/lib/private/diff_test.bzl
+++ b/lib/private/diff_test.bzl
@@ -71,6 +71,7 @@ def _diff_test_impl(ctx):
             "{fail_msg}": ctx.attr.failure_message,
             "{file1}": file1_path,
             "{file2}": file2_path,
+            "{build_file_path}": ctx.build_file_path,
         },
         is_executable = True,
     )
diff --git a/lib/private/diff_test_tmpl.sh b/lib/private/diff_test_tmpl.sh
index 7660b59..60f6f5b 100644
--- a/lib/private/diff_test_tmpl.sh
+++ b/lib/private/diff_test_tmpl.sh
@@ -19,23 +19,46 @@ EOF
   echo >&2 "FAIL: $1"
   exit 1
 }
+resolve_exec_root() {
+  local RUNFILES_PARENT
+  RUNFILES_PARENT=$(dirname "$RUNFILES_DIR")
+  local BIN_DIR
+  BIN_DIR="${RUNFILES_PARENT%$BUILD_FILE_DIR}"
+  local EXEC_ROOT
+  EXEC_ROOT=$(dirname $(dirname $(dirname "${BIN_DIR}")))
+
+  echo -n "$EXEC_ROOT"
+}
+find_file() {
+  local F_RAW="$1"
+  local F="$2"
+  local RF=
+
+  if [[ -f "$TEST_SRCDIR/$F1" || -d "$TEST_SRCDIR/$F" ]]; then
+    RF="$TEST_SRCDIR/$F"
+  elif [[ -d "${RUNFILES_DIR:-/dev/null}" && "${RUNFILES_MANIFEST_ONLY:-}" != 1 ]]; then
+    EXEC_ROOT=$(resolve_exec_root)
+    if [[ -e "$EXEC_ROOT/$F_RAW" ]]; then
+      RF="$EXEC_ROOT/$F_RAW"
+    else
+      RF="$RUNFILES_DIR/$F1"
+    fi
+  elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+    RF="$(grep -F -m1 "$F " "$RUNFILES_MANIFEST_FILE" | sed 's/^[^ ]* //')"
+  else
+    echo >&2 "ERROR: could not find \"${F_RAW}\""
+    exit 1
+  fi
+
+  echo -n "$RF"
+}
+BUILD_FILE_DIR="$(dirname "{build_file_path}")"
 F1="{file1}"
 F2="{file2}"
 [[ "$F1" =~ ^external/ ]] && F1="${F1#external/}" || F1="$TEST_WORKSPACE/$F1"
 [[ "$F2" =~ ^external/ ]] && F2="${F2#external/}" || F2="$TEST_WORKSPACE/$F2"
-if [[ -d "${RUNFILES_DIR:-/dev/null}" && "${RUNFILES_MANIFEST_ONLY:-}" != 1 ]]; then
-  RF1="$RUNFILES_DIR/$F1"
-  RF2="$RUNFILES_DIR/$F2"
-elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
-  RF1="$(grep -F -m1 "$F1 " "$RUNFILES_MANIFEST_FILE" | sed 's/^[^ ]* //')"
-  RF2="$(grep -F -m1 "$F2 " "$RUNFILES_MANIFEST_FILE" | sed 's/^[^ ]* //')"
-elif [[ -f "$TEST_SRCDIR/$F1" && -f "$TEST_SRCDIR/$F2" ]]; then
-  RF1="$TEST_SRCDIR/$F1"
-  RF2="$TEST_SRCDIR/$F2"
-else
-  echo >&2 "ERROR: could not find \"{file1}\" and \"{file2}\""
-  exit 1
-fi
+RF1="$(find_file {file1} "$F1")"
+RF2="$(find_file {file2} "$F2")"
 DF1=
 DF2=
 [[ ! -d "$RF1" ]] || DF1=1
diff --git a/lib/tests/copy_directory_bin_action/BUILD.bazel b/lib/tests/copy_directory_bin_action/BUILD.bazel
index be2ec9b..1d82064 100644
--- a/lib/tests/copy_directory_bin_action/BUILD.bazel
+++ b/lib/tests/copy_directory_bin_action/BUILD.bazel
@@ -17,16 +17,12 @@ diff_test(
     name = "copy_test",
     file1 = ":d",
     file2 = ":pkg",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
 
 diff_test(
     name = "hardlink_test",
     file1 = ":d",
     file2 = ":pkg2",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
 
 diff_test(
diff --git a/lib/tests/copy_to_directory_bin_action/BUILD.bazel b/lib/tests/copy_to_directory_bin_action/BUILD.bazel
index cad0043..b9c79c4 100644
--- a/lib/tests/copy_to_directory_bin_action/BUILD.bazel
+++ b/lib/tests/copy_to_directory_bin_action/BUILD.bazel
@@ -53,8 +53,6 @@ diff_test(
     name = "test",
     file1 = ":pkg",
     file2 = ":expected_pkg",
-    # Source directories are not support on remote execution.
-    tags = ["no-remote-exec"],
 )
 
 bzl_library(
diff --git a/lib/tests/write_source_files/BUILD.bazel b/lib/tests/write_source_files/BUILD.bazel
index 410768a..7bc92d7 100644
--- a/lib/tests/write_source_files/BUILD.bazel
+++ b/lib/tests/write_source_files/BUILD.bazel
@@ -117,22 +117,12 @@ write_source_file(
     name = "e_dir_test",
     in_file = ":e_dir-desired",
     out_file = "e_dir",
-    # The diff_test that is contained within this macro takes `out_file` which is a source directory
-    # as an input but source directories are not supported on remote execution so we must tag that
-    # rule with "no-remote-exec".
-    # ERROR: cannot compare a directory "lib/tests/write_source_files/e_dir-desired" against a file "lib/tests/write_source_files/e_dir"
-    tags = ["no-remote-exec"],
 )
 
 write_source_file(
     name = "es_dir_test",
     in_file = ":es_dir-desired",
     out_file = "es_dir",
-    # The diff_test that is contained within this macro takes `out_file` which is a source directory
-    # as an input but source directories are not supported on remote execution so we must tag that
-    # rule with "no-remote-exec".
-    # ERROR: cannot compare a directory "lib/tests/write_source_files/es_dir-desired" against a file "lib/tests/write_source_files/es_dir"
-    tags = ["no-remote-exec"],
 )
 
 write_source_file_test(
@@ -160,10 +150,6 @@ write_source_files(
         "f2.js": ":f-desired",
         "g2.js": ":g-desired",
     },
-    # The diff_test that is contained within this macro takes output directories `es_dir` and
-    # `es2dir` which are source directories as inputs but source directories are not supported on
-    # remote execution so we must tag that rule with "no-remote-exec".
-    tags = ["no-remote-exec"],
 )
 
 genrule(