Allow WORKSPACE file to be a symlink if no managed directories is used.

Fixes #8475.
Introduced in 0.26 with managed directories support changes.

Closes #8480.

PiperOrigin-RevId: 250490084

src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java

@@ -901,37 +901,35 @@ private boolean refreshWorkspaceHeader(ExtendedEventHandler eventHandler)
 
     WorkspaceFileValue oldValue =
         (WorkspaceFileValue) memoizingEvaluator.getExistingValue(workspaceFileKey);
-    maybeInvalidateWorkspaceFileStateValue(workspacePath);
+    FileStateValue newFileStateValue = maybeInvalidateWorkspaceFileStateValue(workspacePath);
     WorkspaceFileValue newValue =
         (WorkspaceFileValue) evaluateSingleValue(workspaceFileKey, eventHandler);
+    if (newValue != null
+        && !newValue.getManagedDirectories().isEmpty()
+        && FileStateType.SYMLINK.equals(newFileStateValue.getType())) {
+      throw new AbruptExitException(
+          "WORKSPACE file can not be a symlink if incrementally updated directories are used.",
+          ExitCode.PARSING_FAILURE);
+    }
     return managedDirectoriesKnowledge.workspaceHeaderReloaded(oldValue, newValue);
   }
 
   // We only check the FileStateValue of the WORKSPACE file; we do not support the case
   // when the WORKSPACE file is a symlink.
-  private void maybeInvalidateWorkspaceFileStateValue(RootedPath workspacePath)
+  private FileStateValue maybeInvalidateWorkspaceFileStateValue(RootedPath workspacePath)
       throws InterruptedException, AbruptExitException {
     SkyKey workspaceFileStateKey = FileStateValue.key(workspacePath);
     SkyValue oldWorkspaceFileState = memoizingEvaluator.getExistingValue(workspaceFileStateKey);
-    if (oldWorkspaceFileState == null) {
-      // no need to invalidate if not cached
-      return;
-    }
     FileStateValue newWorkspaceFileState;
     try {
       newWorkspaceFileState = FileStateValue.create(workspacePath, tsgm.get());
-      if (FileStateType.SYMLINK.equals(newWorkspaceFileState.getType())) {
-        throw new AbruptExitException(
-            "WORKSPACE file can not be a symlink if incrementally"
-                + " updated directories feature is enabled.",
-            ExitCode.PARSING_FAILURE);
-      }
     } catch (IOException e) {
       throw new AbruptExitException("Can not read WORKSPACE file.", ExitCode.PARSING_FAILURE, e);
     }
-    if (!oldWorkspaceFileState.equals(newWorkspaceFileState)) {
+    if (oldWorkspaceFileState != null && !oldWorkspaceFileState.equals(newWorkspaceFileState)) {
       recordingDiffer.invalidate(ImmutableSet.of(workspaceFileStateKey));
     }
+    return newWorkspaceFileState;
   }
 
   private SkyValue evaluateSingleValue(SkyKey key, ExtendedEventHandler eventHandler)

src/test/java/com/google/devtools/build/lib/blackbox/tests/manageddirs/ManagedDirectoriesBlackBoxTest.java

@@ -22,6 +22,7 @@
 import com.google.devtools.build.lib.blackbox.junit.AbstractBlackBoxTest;
 import com.google.devtools.build.lib.util.ResourceFileLoader;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.List;
@@ -331,6 +332,42 @@ public void testRepositoryOverrideChangeToConflictWithManagedDirectories() throw
                 + " have managed directories: @generated_node_modules");
   }
 
+  /**
+   * The test to verify that WORKSPACE file can not be a symlink when managed directories are used.
+   *
+   * <p>The test of the case, when WORKSPACE file is a symlink, but not managed directories are
+   * used, is in {@link WorkspaceBlackBoxTest#testWorkspaceFileIsSymlink()}
+   */
+  @Test
+  public void testWorkspaceSymlinkThrowsWithManagedDirectories() throws Exception {
+    generateProject();
+
+    Path workspaceFile = context().getWorkDir().resolve(WORKSPACE);
+    assertThat(workspaceFile.toFile().delete()).isTrue();
+
+    Path tempWorkspace = Files.createTempFile(context().getTmpDir(), WORKSPACE, "");
+    PathUtils.writeFile(
+        tempWorkspace,
+        "workspace(name = \"fine_grained_user_modules\",",
+        "managed_directories = {'@generated_node_modules': ['node_modules']})",
+        "",
+        "load(\":use_node_modules.bzl\", \"generate_fine_grained_node_modules\")",
+        "",
+        "generate_fine_grained_node_modules(",
+        "    name = \"generated_node_modules\",",
+        "    package_json = \"//:package.json\",",
+        ")");
+    Files.createSymbolicLink(workspaceFile, tempWorkspace);
+
+    ProcessResult result = bazel().shouldFail().build("//...");
+    assertThat(
+            findPattern(
+                result,
+                "WORKSPACE file can not be a symlink if incrementally updated directories are"
+                    + " used."))
+        .isTrue();
+  }
+
   private void generateProject() throws IOException {
     writeProjectFile("BUILD.test", "BUILD");
     writeProjectFile("WORKSPACE.test", "WORKSPACE");

src/test/java/com/google/devtools/build/lib/blackbox/tests/workspace/WorkspaceBlackBoxTest.java

@@ -225,6 +225,33 @@ public void testPathWithSpace() throws Exception {
     bazel.help();
   }
 
+  @Test
+  public void testWorkspaceFileIsSymlink() throws Exception {
+    if (isWindows()) {
+      // Do not test file symlinks on Windows.
+      return;
+    }
+    Path repo = context().getTmpDir().resolve(testName.getMethodName());
+    new RepoWithRuleWritingTextGenerator(repo).withOutputText("hi").setupRepository();
+
+    Path workspaceFile = context().getWorkDir().resolve(WORKSPACE);
+    assertThat(workspaceFile.toFile().delete()).isTrue();
+
+    Path tempWorkspace = Files.createTempFile(context().getTmpDir(), WORKSPACE, "");
+    PathUtils.writeFile(
+        tempWorkspace,
+        "workspace(name = 'abc')",
+        String.format(
+            "local_repository(name = 'ext', path = '%s',)", PathUtils.pathForStarlarkFile(repo)));
+    Files.createSymbolicLink(workspaceFile, tempWorkspace);
+
+    BuilderRunner bazel = WorkspaceTestUtils.bazel(context());
+    bazel.build("@ext//:all");
+    PathUtils.append(workspaceFile, "# comment");
+    // At this point, there is already some cache workspace file/file state value.
+    bazel.build("@ext//:all");
+  }
+
   private boolean isWindows() {
     return OS.WINDOWS.equals(OS.getCurrent());
   }