Introduce an --incompatible_disallow_symlink_file_to_dir flag so we can eventually disallow calling ctx.actions.symlink on an input directory and an output file.

PiperOrigin-RevId: 466002367
Change-Id: I5a9275deb458ae6388e10c5b941e2ccef2ddaa81

Author: tjgq

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java

@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.analysis.starlark;
 
 import static com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT;
+import static com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions.INCOMPATIBLE_DISALLOW_SYMLINK_FILE_TO_DIR;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableList;
@@ -282,11 +283,14 @@ public void symlink(
                 + "file or directory, not a symlink (did you mean to use declare_file() or "
                 + "declare_directory() instead of declare_symlink()?)");
       }
-      // TODO(tjgq): Enforce that input and output are either both files or both directories.
-      // This requires fixing callers passing a directory as the input and a file as the output.
-      if (!inputArtifact.isTreeArtifact() && outputArtifact.isTreeArtifact()) {
-        String inputType = inputArtifact.isTreeArtifact() ? "directory" : "file";
-        String outputType = outputArtifact.isTreeArtifact() ? "directory" : "file";
+
+      boolean inputOutputMismatch =
+          getSemantics().getBool(INCOMPATIBLE_DISALLOW_SYMLINK_FILE_TO_DIR)
+              ? inputArtifact.isDirectory() != outputArtifact.isDirectory()
+              : !inputArtifact.isDirectory() && outputArtifact.isDirectory();
+      if (inputOutputMismatch) {
+        String inputType = inputArtifact.isDirectory() ? "directory" : "file";
+        String outputType = outputArtifact.isDirectory() ? "directory" : "file";
         throw Starlark.errorf(
             "symlink() with \"target_file\" %s param requires that \"output\" be declared as a %s "
                 + "(did you mean to use declare_%s() instead of declare_%s()?)",

src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java

@@ -63,6 +63,16 @@ public final class BuildLanguageOptions extends OptionsBase {
 
   // <== Add new options here in alphabetic order ==>
 
+  @Option(
+      name = "incompatible_disallow_symlink_file_to_dir",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+      help =
+          "If set to true, `ctx.actions.symlink` will disallow symlinking a file into a directory.")
+  public boolean incompatibleAllowSymlinkFileToDir;
+
   @Option(
       name = "experimental_build_setting_api",
       defaultValue = "true",
@@ -609,6 +619,7 @@ public StarlarkSemantics toStarlarkSemantics() {
     StarlarkSemantics semantics =
         StarlarkSemantics.builder()
             // <== Add new options here in alphabetic order ==>
+            .setBool(INCOMPATIBLE_DISALLOW_SYMLINK_FILE_TO_DIR, incompatibleAllowSymlinkFileToDir)
             .setBool(EXPERIMENTAL_ALLOW_TAGS_PROPAGATION, experimentalAllowTagsPropagation)
             .set(EXPERIMENTAL_BUILTINS_BZL_PATH, experimentalBuiltinsBzlPath)
             .setBool(EXPERIMENTAL_BUILTINS_DUMMY, experimentalBuiltinsDummy)
@@ -684,6 +695,8 @@ public StarlarkSemantics toStarlarkSemantics() {
   // (In principle, a key not associated with a command-line flag may be declared anywhere.)
 
   // booleans: the +/- prefix indicates the default value (true/false).
+  public static final String INCOMPATIBLE_DISALLOW_SYMLINK_FILE_TO_DIR =
+      "-incompatible_disallow_symlink_file_to_dir";
   public static final String EXPERIMENTAL_ALLOW_TAGS_PROPAGATION =
       "-experimental_allow_tags_propagation";
   public static final String EXPERIMENTAL_BUILTINS_DUMMY = "-experimental_builtins_dummy";

src/test/shell/bazel/bazel_symlink_test.sh

@@ -492,6 +492,13 @@ EOF
   bazel build //a:a || fail "build failed"
   assert_contains "Hello, World!" bazel-bin/a/a.link/inside.txt
   expect_symlink bazel-bin/a/a.link
+
+  bazel build --noincompatible_disallow_symlink_file_to_dir || fail "build failed"
+  assert_contains "Hello, World!" bazel-bin/a/a.link/inside.txt
+  expect_symlink bazel-bin/a/a.link
+
+  bazel build --incompatible_disallow_symlink_file_to_dir //a:a >& $TEST_log && fail "build succeeded"
+  expect_log "symlink() with \"target_file\" directory param requires that \"output\" be declared as a directory"
 }
 
 function test_symlink_directory_to_file_created_from_symlink_action() {