[7.2.0] Handle file type changes with in-process symlink creation (#2…

…2116)

`SymlinkTreeUpdater` now correctly replaces symlinks with directories
and vice versa in runfiles trees.

Fixes #20266

Closes #22087.

PiperOrigin-RevId: 628028591
Change-Id: Id94cea42f9cdfab001e56242a329b8d6d253ba29

Commit
4b18dbe

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>

src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java

@@ -233,7 +233,7 @@ void syncTreeRecursively(Path at) throws IOException {
       // if (!stat.isExecutable() || !stat.isReadable()) {
       //   at.chmod(stat.getMods() | 0700);
       // }
-      for (Dirent dirent : at.readdir(Symlinks.FOLLOW)) {
+      for (Dirent dirent : at.readdir(Symlinks.NOFOLLOW)) {
         String basename = dirent.getName();
         Path next = at.getChild(basename);
         if (symlinks.containsKey(basename)) {
@@ -245,6 +245,11 @@ void syncTreeRecursively(Path at) throws IOException {
             }
             // For consistency with build-runfiles.cc, we don't truncate the file if one exists.
           } else {
+            // ensureSymbolicLink will replace a symlink that doesn't have the correct target, but
+            // everything else needs to be deleted first.
+            if (dirent.getType() != Dirent.Type.SYMLINK) {
+              next.deleteTree();
+            }
             // TODO(ulfjack): On Windows, this call makes a copy rather than creating a symlink.
             FileSystemUtils.ensureSymbolicLink(next, value.getPath().asFragment());
           }

src/test/shell/bazel/runfiles_test.sh

@@ -261,4 +261,151 @@ EOF
     bazel build --spawn_strategy=local --nobuild_runfile_links --enable_runfiles=false //:out
 }
 
+function setup_runfiles_tree_file_type_changes {
+  mkdir -p rules
+  touch rules/BUILD
+  cat > rules/defs.bzl <<'EOF'
+def _make_fake_executable(ctx):
+    fake_executable = ctx.actions.declare_file(ctx.label.name)
+    ctx.actions.write(
+        output = fake_executable,
+        content = "echo 'i do nothing'",
+        is_executable = True,
+    )
+    return fake_executable
+
+def _tree_artifact(ctx):
+    d = ctx.actions.declare_directory("lib")
+    ctx.actions.run_shell(
+        outputs = [d],
+        arguments = [d.path],
+        command = """
+touch $1/sample1.txt
+touch $1/sample2.txt
+""",
+    )
+
+    return DefaultInfo(
+        runfiles = ctx.runfiles(symlinks = {"lib": d}),
+    )
+
+tree_artifact = rule(implementation = _tree_artifact)
+
+def _individual_files(ctx):
+    symlinks = {}
+    for file in ctx.files.srcs:
+        _, relative_path = file.path.split("/", 1)
+        symlinks[relative_path] = file
+    return DefaultInfo(
+        runfiles = ctx.runfiles(symlinks = symlinks),
+    )
+
+individual_files = rule(
+    implementation = _individual_files,
+    attrs = {
+        "srcs": attr.label_list(allow_files = True),
+    },
+)
+
+def _output_impl(ctx):
+    return DefaultInfo(
+        runfiles = ctx.attr.src[DefaultInfo].default_runfiles,
+        executable = _make_fake_executable(ctx),
+    )
+
+output = rule(
+    implementation = _output_impl,
+    executable = True,
+    attrs = {
+        "src": attr.label(),
+    },
+)
+EOF
+
+  mkdir -p pkg/lib
+  touch pkg/lib/sample1.txt
+  touch pkg/lib/sample2.txt
+  cat > pkg/BUILD <<'EOF'
+load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
+load("//rules:defs.bzl", "tree_artifact", "individual_files", "output")
+
+bool_flag(
+    name = "use_tree",
+    build_setting_default = False,
+)
+
+config_setting(
+    name = "should_use_tree",
+    flag_values = {"//pkg:use_tree": "True"},
+)
+
+tree_artifact(name = "tree_artifact")
+
+individual_files(
+    name = "individual_files",
+    srcs = glob(["lib/*"]),
+)
+
+output(
+    name = "output",
+    src = select({
+        "//pkg:should_use_tree": ":tree_artifact",
+        "//conditions:default": ":individual_files",
+    }),
+)
+EOF
+}
+
+function test_runfiles_tree_file_type_changes_tree_to_individual {
+  setup_runfiles_tree_file_type_changes
+
+  bazel build --//pkg:use_tree=True //pkg:output || fail "Build failed"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample1.txt ]] || fail "sample1.txt not found"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample2.txt ]] || fail "sample2.txt not found"
+
+  bazel build --//pkg:use_tree=False //pkg:output || fail "Build failed"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample1.txt ]] || fail "sample1.txt not found"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample2.txt ]] || fail "sample2.txt not found"
+}
+
+function test_runfiles_tree_file_type_changes_individual_to_tree {
+  setup_runfiles_tree_file_type_changes
+
+  bazel build --//pkg:use_tree=False //pkg:output || fail "Build failed"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample1.txt ]] || fail "sample1.txt not found"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample2.txt ]] || fail "sample2.txt not found"
+
+  bazel build --//pkg:use_tree=True //pkg:output || fail "Build failed"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample1.txt ]] || fail "sample1.txt not found"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample2.txt ]] || fail "sample2.txt not found"
+}
+
+function test_runfiles_tree_file_type_changes_tree_to_individual_inprocess {
+  setup_runfiles_tree_file_type_changes
+
+  bazel build --experimental_inprocess_symlink_creation \
+    --//pkg:use_tree=True //pkg:output || fail "Build failed"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample1.txt ]] || fail "sample1.txt not found"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample2.txt ]] || fail "sample2.txt not found"
+
+  bazel build --experimental_inprocess_symlink_creation \
+    --//pkg:use_tree=False //pkg:output || fail "Build failed"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample1.txt ]] || fail "sample1.txt not found"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample2.txt ]] || fail "sample2.txt not found"
+}
+
+function test_runfiles_tree_file_type_changes_individual_to_tree_inprocess {
+  setup_runfiles_tree_file_type_changes
+
+  bazel build --experimental_inprocess_symlink_creation \
+    --//pkg:use_tree=False //pkg:output || fail "Build failed"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample1.txt ]] || fail "sample1.txt not found"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample2.txt ]] || fail "sample2.txt not found"
+
+  bazel build --experimental_inprocess_symlink_creation \
+    --//pkg:use_tree=True //pkg:output || fail "Build failed"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample1.txt ]] || fail "sample1.txt not found"
+  [[ -f bazel-bin/pkg/output.runfiles/_main/lib/sample2.txt ]] || fail "sample2.txt not found"
+}
+
 run_suite "runfiles tests"