From 5072f6f2eb5e76ccb39e1e1b9298d5d519fb1434 Mon Sep 17 00:00:00 2001
From: aiuto <aiuto@google.com>
Date: Mon, 17 Aug 2020 21:01:33 -0700
Subject: [PATCH] Stop needlessly parsing WORKSPACE files from external
 repositories.

Fixes: https://github.com/bazelbuild/bazel/issues/11936

I have an additional test to prove the spurious error message is gone and that I still can refer to a workspace by name, in https://github.com/aiuto/repro_11936/tree/more_tests
I have not added it yet because it is not really clear what the correct behavior should be for external repos - should the WORKSPACE be valid and used, or ignored. This change brings the code in line with the documentation (ignored), but the better behavior might be different.

RELNOTES: Stop needlessly parsing WORKSPACE files from external repositories.
PiperOrigin-RevId: 327157715
---
 .../repository/RepositoryLoaderFunction.java  | 35 -------------------
 .../skyframe/WorkspaceFileFunctionTest.java   | 18 ++++++++++
 2 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryLoaderFunction.java b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryLoaderFunction.java
index 75f2c719b525f5..e2ea4f19c1cb05 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryLoaderFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryLoaderFunction.java
@@ -14,12 +14,8 @@
 
 package com.google.devtools.build.lib.rules.repository;
 
-import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
-import com.google.devtools.build.lib.packages.WorkspaceFileValue;
 import com.google.devtools.build.lib.skyframe.RepositoryValue;
-import com.google.devtools.build.lib.vfs.Root;
-import com.google.devtools.build.lib.vfs.RootedPath;
 import com.google.devtools.build.skyframe.SkyFunction;
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
@@ -47,37 +43,6 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     if (!repository.repositoryExists()) {
       return RepositoryValue.notFound(nameFromRule);
     }
-    RootedPath workspaceFilePath;
-    try {
-      workspaceFilePath =
-          WorkspaceFileHelper.getWorkspaceRootedFile(Root.fromPath(repository.getPath()), env);
-      if (workspaceFilePath == null) {
-        return null;
-      }
-    } catch (IOException e) {
-      throw new RepositoryLoaderFunctionException(
-          new IOException(
-              "Could not determine workspace file (\"WORKSPACE.bazel\" or \"WORKSPACE\"): "
-                  + e.getMessage()),
-          Transience.PERSISTENT);
-    }
-    SkyKey workspaceKey = WorkspaceFileValue.key(workspaceFilePath);
-    WorkspaceFileValue workspacePackage = (WorkspaceFileValue) env.getValue(workspaceKey);
-    if (workspacePackage == null) {
-      return null;
-    }
-
-    try {
-      String workspaceNameStr = workspacePackage.getPackage().getWorkspaceName();
-      if (workspaceNameStr.isEmpty()) {
-        RepositoryName.create("");
-      } else {
-        RepositoryName.create("@" + workspaceNameStr);
-      }
-    } catch (LabelSyntaxException e) {
-      throw new IllegalStateException(e);
-    }
-
     return RepositoryValue.success(nameFromRule, repository);
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
index 9f700631688196..a64c5c6d87a4d7 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java
@@ -518,6 +518,24 @@ public void testDoNotSymlinkInExecroot() throws Exception {
     }
   }
 
+  @Test
+  public void testMangledExternalWorkspaceFileIsIgnored() throws Exception {
+    scratch.file("secondary/WORKSPACE", "garbage");
+    RootedPath workspace =
+        createWorkspaceFile(
+            "workspace(name = 'good')",
+            "local_repository(name = \"secondary\", path = \"./secondary/\")");
+
+    SkyKey key1 = WorkspaceFileValue.key(workspace, 1);
+    EvaluationResult<WorkspaceFileValue> result1 = eval(key1);
+    WorkspaceFileValue value1 = result1.get(key1);
+    RepositoryName good = RepositoryName.create("@good");
+    RepositoryName main = RepositoryName.create("@");
+    RepositoryName secondary = RepositoryName.create("@secondary");
+    assertThat(value1.getRepositoryMapping()).containsEntry(secondary, ImmutableMap.of(good, main));
+    assertNoEvents();
+  }
+
   private static class TestManagedDirectoriesKnowledge implements ManagedDirectoriesKnowledge {
     private String lastWorkspaceName;
     private int cnt = 0;