Add implicit mapping from "@mainrepo" to "@". This fixes the issue where referencing the main repository using its name caused bazel to treat it as a separate external repository.

Closes #5586.
Fixes #3115.

RELNOTES: None
PiperOrigin-RevId: 204752150

4 files changed, 138 insertions, 42 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index 0837b823f4..6cc7ddfffe 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -18,6 +18,7 @@ import static com.google.devtools.build.lib.syntax.Runtime.NONE;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -301,18 +302,17 @@ public class WorkspaceFactory {
   }
 
   @SkylarkSignature(
-    name = "workspace",
-    objectType = Object.class,
-    returnType = SkylarkList.class,
-    doc =
-        "Sets the name for this workspace. Workspace names should be a Java-package-style "
-            + "description of the project, using underscores as separators, e.g., "
-            + "github.com/bazelbuild/bazel should use com_github_bazelbuild_bazel. Names must "
-            + "start with a letter and can only contain letters, numbers, and underscores.",
-    parameters = {@Param(name = "name", type = String.class, doc = "the name of the workspace.")},
-    useAst = true,
-    useEnvironment = true
-  )
+      name = "workspace",
+      objectType = Object.class,
+      returnType = SkylarkList.class,
+      doc =
+          "Sets the name for this workspace. Workspace names should be a Java-package-style "
+              + "description of the project, using underscores as separators, e.g., "
+              + "github.com/bazelbuild/bazel should use com_github_bazelbuild_bazel. Names must "
+              + "start with a letter and can only contain letters, numbers, and underscores.",
+      parameters = {@Param(name = "name", type = String.class, doc = "the name of the workspace.")},
+      useAst = true,
+      useEnvironment = true)
   private static final BuiltinFunction.Factory newWorkspaceFunction =
       new BuiltinFunction.Factory("workspace") {
         public BuiltinFunction create(boolean allowOverride, final RuleFactory ruleFactory) {
@@ -344,6 +344,12 @@ public class WorkspaceFactory {
                 } catch (InvalidRuleException | NameConflictException | LabelSyntaxException e) {
                   throw new EvalException(ast.getLocation(), e.getMessage());
                 }
+                // Add entry in repository map from "@name" --> "@" to avoid issue where bazel
+                // treats references to @name as a separate external repo
+                builder.addRepositoryMappingEntry(
+                    RepositoryName.MAIN,
+                    RepositoryName.createFromValidStrippedName(name),
+                    RepositoryName.MAIN);
                 return NONE;
               }
             };
@@ -488,17 +494,27 @@ public class WorkspaceFactory {
                     + kwargs.get("name")
                     + "')");
           }
+          String externalRepoName = (String) kwargs.get("name");
+          // Add an entry in every repository from @<mainRepoName> to "@" to avoid treating
+          // @<mainRepoName> as a separate repository. This will be overridden if the main
+          // repository has a repo_mapping entry from <mainRepoName> to something.
+          if (!Strings.isNullOrEmpty(builder.pkg.getWorkspaceName())) {
+            builder.addRepositoryMappingEntry(
+                RepositoryName.createFromValidStrippedName(externalRepoName),
+                RepositoryName.createFromValidStrippedName(builder.pkg.getWorkspaceName()),
+                RepositoryName.MAIN);
+          }
           if (env.getSemantics().experimentalEnableRepoMapping()) {
             if (kwargs.containsKey("repo_mapping")) {
               if (!(kwargs.get("repo_mapping") instanceof Map)) {
                 throw new EvalException(
                     ast.getLocation(),
-                    "Invalid value for 'repo_mapping': '" + kwargs.get("repo_mapping")
+                    "Invalid value for 'repo_mapping': '"
+                        + kwargs.get("repo_mapping")
                         + "'. Value must be a map.");
               }
               @SuppressWarnings("unchecked")
               Map<String, String> map = (Map<String, String>) kwargs.get("repo_mapping");
-              String externalRepoName = (String) kwargs.get("name");
               for (Map.Entry<String, String> e : map.entrySet()) {
                 builder.addRepositoryMappingEntry(
                     RepositoryName.createFromValidStrippedName(externalRepoName),
diff --git a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
index eb23ff4572..de9b48e825 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/WorkspaceFactoryTest.java
@@ -122,12 +122,14 @@ public class WorkspaceFactoryTest {
   public void testWorkspaceMappings() throws Exception {
     helper.setSkylarkSemantics("--experimental_enable_repo_mapping");
     helper.parse(
+        "workspace(name = 'bar')",
         "local_repository(",
         "    name = 'foo',",
         "    path = '/foo',",
         "    repo_mapping = {'@x' : '@y'},",
         ")");
     assertMapping(helper, "@foo", "@x", "@y");
+    assertMapping(helper, "@foo", "@bar", "@");
   }
   
   @Test
@@ -196,6 +198,32 @@ public class WorkspaceFactoryTest {
         .contains("Invalid value for 'repo_mapping': 'hello'. Value must be a map.");
   }
 
+  @Test
+  public void testImplicitMainRepoRename() throws Exception {
+    helper.parse("workspace(name = 'foo')");
+    assertMapping(helper, "@", "@foo", "@");
+  }
+
+  @Test
+  public void testEmptyRepositoryHasEmptyMap() throws Exception {
+    helper.parse("");
+    assertThat(helper.getPackage().getRepositoryMapping("@")).isEmpty();
+  }
+
+  @Test
+  public void testOverrideImplicitMainRepoRename() throws Exception {
+    helper.setSkylarkSemantics("--experimental_enable_repo_mapping");
+    helper.parse(
+        "workspace(name = 'bar')",
+        "local_repository(",
+        "    name = 'foo',",
+        "    path = '/foo',",
+        "    repo_mapping = {'@x' : '@y', '@bar' : '@newname'},",
+        ")");
+    assertMapping(helper, "@foo", "@x", "@y");
+    assertMapping(helper, "@foo", "@bar", "@newname");
+  }
+
   private void assertMapping(
       WorkspaceFactoryTestHelper helper, String repo, String local, String global)
       throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java
index 8a1aa0b20e..737fa5b51c 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RepositoryMappingFunctionTest.java
@@ -23,6 +23,7 @@ import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
+import com.google.devtools.build.lib.testutil.TestConstants;
 import com.google.devtools.build.lib.vfs.ModifiedFileSet;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Root;
@@ -66,7 +67,9 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase {
         .hasEntryThat(skyKey)
         .isEqualTo(
             RepositoryMappingValue.withMapping(
-                ImmutableMap.of(RepositoryName.create("@a"), RepositoryName.create("@b"))));
+                ImmutableMap.of(
+                    RepositoryName.create("@good"), RepositoryName.MAIN,
+                    RepositoryName.create("@a"), RepositoryName.create("@b"))));
   }
 
   @Test
@@ -94,12 +97,16 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase {
         .hasEntryThat(skyKey1)
         .isEqualTo(
             RepositoryMappingValue.withMapping(
-                ImmutableMap.of(RepositoryName.create("@a"), RepositoryName.create("@b"))));
+                ImmutableMap.of(
+                    RepositoryName.create("@good"), RepositoryName.MAIN,
+                    RepositoryName.create("@a"), RepositoryName.create("@b"))));
     assertThatEvaluationResult(eval(skyKey2))
         .hasEntryThat(skyKey2)
         .isEqualTo(
             RepositoryMappingValue.withMapping(
-                ImmutableMap.of(RepositoryName.create("@x"), RepositoryName.create("@y"))));
+                ImmutableMap.of(
+                    RepositoryName.create("@good"), RepositoryName.MAIN,
+                    RepositoryName.create("@x"), RepositoryName.create("@y"))));
   }
 
   @Test
@@ -121,6 +128,7 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase {
         .isEqualTo(
             RepositoryMappingValue.withMapping(
                 ImmutableMap.of(
+                    RepositoryName.create("@good"), RepositoryName.MAIN,
                     RepositoryName.create("@a"), RepositoryName.create("@b"),
                     RepositoryName.create("@x"), RepositoryName.create("@y"))));
   }
@@ -148,11 +156,10 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase {
   }
 
   @Test
-  public void testEmptyMapping() throws Exception {
+  public void testDefaultMainRepoNameInMapping() throws Exception {
     setSkylarkSemanticsOptions("--experimental_enable_repo_mapping");
     scratch.overwriteFile(
         "WORKSPACE",
-        "workspace(name = 'good')",
         "local_repository(",
         "    name = 'a_remote_repo',",
         "    path = '/a_remote_repo',",
@@ -165,31 +172,13 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase {
         .hasEntryThat(skyKey)
         .isEqualTo(
             RepositoryMappingValue.withMapping(
-                ImmutableMap.of()));
-  }
-
-  @Test
-  public void testNoMappings_noFlag() throws Exception {
-    setSkylarkSemanticsOptions("--noexperimental_enable_repo_mapping");
-    scratch.overwriteFile(
-        "WORKSPACE",
-        "workspace(name = 'good')",
-        "local_repository(",
-        "    name = 'a_remote_repo',",
-        "    path = '/a_remote_repo',",
-        ")");
-    RepositoryName name = RepositoryName.create("@a_remote_repo");
-    SkyKey skyKey = RepositoryMappingValue.key(name);
-
-    assertThatEvaluationResult(eval(skyKey))
-        .hasEntryThat(skyKey)
-        .isEqualTo(
-            RepositoryMappingValue.withMapping(ImmutableMap.of()));
+                ImmutableMap.of(
+                    RepositoryName.createFromValidStrippedName(TestConstants.WORKSPACE_NAME),
+                    RepositoryName.MAIN)));
   }
 
   @Test
-  public void testNoMappings_withFlag() throws Exception {
-    setSkylarkSemanticsOptions("--experimental_enable_repo_mapping");
+  public void testExplicitMainRepoNameInMapping() throws Exception {
     scratch.overwriteFile(
         "WORKSPACE",
         "workspace(name = 'good')",
@@ -203,7 +192,8 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase {
     assertThatEvaluationResult(eval(skyKey))
         .hasEntryThat(skyKey)
         .isEqualTo(
-            RepositoryMappingValue.withMapping(ImmutableMap.of()));
+            RepositoryMappingValue.withMapping(
+                ImmutableMap.of(RepositoryName.create("@good"), RepositoryName.MAIN)));
   }
 
   @Test
diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh
index 22b7a4741b..72915a8b27 100755
--- a/src/test/shell/bazel/workspace_test.sh
+++ b/src/test/shell/bazel/workspace_test.sh
@@ -560,4 +560,66 @@ EOF
       || fail "Expected build to succeed"
 }
 
+function test_mainrepo_name_is_not_different_repo() {
+  # Repository a refers to @x
+  mkdir -p mainrepo
+  echo "workspace(name = 'mainrepo')" > mainrepo/WORKSPACE
+  cat > mainrepo/BUILD<<EOF
+load("//:def.bzl", "a")
+load("@mainrepo//:def.bzl", "a")
+EOF
+  cat > mainrepo/def.bzl<<EOF
+print("def.bzl loaded")
+a = 1
+EOF
+
+  cd mainrepo
+  bazel query //... &>"$TEST_log" || fail "Expected query to succeed"
+  expect_log "def.bzl loaded."
+  expect_not_log "external"
+}
+
+function test_mainrepo_name_remapped_properly() {
+  mkdir -p mainrepo
+  touch mainrepo/BUILD
+  cat > mainrepo/WORKSPACE<<EOF
+workspace(name = "mainrepo")
+local_repository(
+  name = "a",
+  path = "../a"
+)
+EOF
+  cat > mainrepo/def.bzl<<EOF
+print ("def.bzl loaded")
+x = 10
+EOF
+
+  mkdir -p a
+  touch a/WORKSPACE
+  echo "load('@mainrepo//:def.bzl', 'x')"> a/BUILD
+
+  # the bzl file should be loaded from the main workspace and
+  # not as an external repository
+  cd mainrepo
+  bazel query @a//... &>"$TEST_log" || fail "Expected query to succeed"
+  expect_log "def.bzl loaded"
+  expect_not_log "external"
+
+  cd ..
+  cat > mainrepo/WORKSPACE<<EOF
+workspace(name = "mainrepo")
+local_repository(
+  name = "a",
+  path = "../a",
+  repo_mapping = {"@mainrepo" : "@newname"}
+)
+EOF
+
+  # now that @mainrepo doesn't exist within workspace "a",
+  # the query should fail
+  cd mainrepo
+  bazel query --experimental_enable_repo_mapping @a//... &>"$TEST_log" \
+      && fail "Failure expected" || true
+}
+
 run_suite "workspace tests"