Put main repo remapping behind a flag.

RELNOTES: None
PiperOrigin-RevId: 207801155

7 files changed, 58 insertions, 33 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
index 3277c38440..5ef70d319b 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkSemanticsOptions.java
@@ -59,15 +59,6 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   // <== Add new options here in alphabetic order ==>
 
   @Option(
-      name = "experimental_enable_repo_mapping",
-      defaultValue = "false",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
-      effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS,
-      help = "If set to true, enables the use of the `repo_mapping` attribute in WORKSPACE files."
-  )
-  public boolean experimentalEnableRepoMapping;
-
-  @Option(
       name = "experimental_cc_skylark_api_enabled_packages",
       converter = CommaSeparatedOptionListConverter.class,
       defaultValue = "",
@@ -80,6 +71,22 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
   public List<String> experimentalCcSkylarkApiEnabledPackages;
 
   @Option(
+      name = "experimental_enable_repo_mapping",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS,
+      help = "If set to true, enables the use of the `repo_mapping` attribute in WORKSPACE files.")
+  public boolean experimentalEnableRepoMapping;
+
+  @Option(
+      name = "experimental_remap_main_repo",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = OptionEffectTag.LOADING_AND_ANALYSIS,
+      help = "If set to true, will treat references to '@<main repo name>' the same as '@'.")
+  public boolean experimentalRemapMainRepo;
+
+  @Option(
     name = "incompatible_bzl_disallow_load_after_statement",
     defaultValue = "false",
     documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
@@ -363,6 +370,7 @@ public class SkylarkSemanticsOptions extends OptionsBase implements Serializable
         // <== Add new options here in alphabetic order ==>
         .experimentalCcSkylarkApiEnabledPackages(experimentalCcSkylarkApiEnabledPackages)
         .experimentalEnableRepoMapping(experimentalEnableRepoMapping)
+        .experimentalRemapMainRepo(experimentalRemapMainRepo)
         .incompatibleBzlDisallowLoadAfterStatement(incompatibleBzlDisallowLoadAfterStatement)
         .incompatibleDepsetIsNotIterable(incompatibleDepsetIsNotIterable)
         .incompatibleDepsetUnion(incompatibleDepsetUnion)
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 6cc7ddfffe..c412181506 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
@@ -346,10 +346,12 @@ public class WorkspaceFactory {
                 }
                 // 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);
+                if (env.getSemantics().experimentalRemapMainRepo()) {
+                  builder.addRepositoryMappingEntry(
+                      RepositoryName.MAIN,
+                      RepositoryName.createFromValidStrippedName(name),
+                      RepositoryName.MAIN);
+                }
                 return NONE;
               }
             };
@@ -498,11 +500,13 @@ public class WorkspaceFactory {
           // 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().experimentalRemapMainRepo()) {
+            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")) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
index 5893ac49ab..85f6d4d212 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSemantics.java
@@ -45,6 +45,8 @@ public abstract class SkylarkSemantics {
 
   public abstract boolean experimentalEnableRepoMapping();
 
+  public abstract boolean experimentalRemapMainRepo();
+
   public abstract boolean incompatibleBzlDisallowLoadAfterStatement();
 
   public abstract boolean incompatibleDepsetIsNotIterable();
@@ -102,6 +104,7 @@ public abstract class SkylarkSemantics {
           // <== Add new options here in alphabetic order ==>
           .experimentalCcSkylarkApiEnabledPackages(ImmutableList.of())
           .experimentalEnableRepoMapping(false)
+          .experimentalRemapMainRepo(false)
           .incompatibleBzlDisallowLoadAfterStatement(false)
           .incompatibleDepsetIsNotIterable(false)
           .incompatibleDepsetUnion(false)
@@ -133,6 +136,8 @@ public abstract class SkylarkSemantics {
 
     public abstract Builder experimentalEnableRepoMapping(boolean value);
 
+    public abstract Builder experimentalRemapMainRepo(boolean value);
+
     public abstract Builder incompatibleBzlDisallowLoadAfterStatement(boolean value);
 
     public abstract Builder incompatibleDepsetIsNotIterable(boolean value);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index 21328c8ff1..5bc110986c 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -123,6 +123,7 @@ public class SkylarkSemanticsConsistencyTest {
             + ","
             + rand.nextDouble(),
         "--experimental_enable_repo_mapping=" + rand.nextBoolean(),
+        "--experimental_remap_main_repo=" + rand.nextBoolean(),
         "--incompatible_bzl_disallow_load_after_statement=" + rand.nextBoolean(),
         "--incompatible_depset_is_not_iterable=" + rand.nextBoolean(),
         "--incompatible_depset_union=" + rand.nextBoolean(),
@@ -155,6 +156,7 @@ public class SkylarkSemanticsConsistencyTest {
         .experimentalCcSkylarkApiEnabledPackages(
             ImmutableList.of(String.valueOf(rand.nextDouble()), String.valueOf(rand.nextDouble())))
         .experimentalEnableRepoMapping(rand.nextBoolean())
+        .experimentalRemapMainRepo(rand.nextBoolean())
         .incompatibleBzlDisallowLoadAfterStatement(rand.nextBoolean())
         .incompatibleDepsetIsNotIterable(rand.nextBoolean())
         .incompatibleDepsetUnion(rand.nextBoolean())
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 de9b48e825..c964539e7a 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
@@ -129,7 +129,6 @@ public class WorkspaceFactoryTest {
         "    repo_mapping = {'@x' : '@y'},",
         ")");
     assertMapping(helper, "@foo", "@x", "@y");
-    assertMapping(helper, "@foo", "@bar", "@");
   }
   
   @Test
@@ -200,11 +199,20 @@ public class WorkspaceFactoryTest {
 
   @Test
   public void testImplicitMainRepoRename() throws Exception {
+    helper.setSkylarkSemantics("--experimental_remap_main_repo");
     helper.parse("workspace(name = 'foo')");
     assertMapping(helper, "@", "@foo", "@");
   }
 
   @Test
+  public void testNoImplicitMainRepoRenameWithoutFlag() throws Exception {
+    helper.parse("workspace(name = 'foo')");
+    RepositoryName foo = RepositoryName.create("@foo");
+    assertThat(helper.getPackage().getRepositoryMapping("@"))
+        .doesNotContainEntry(foo, RepositoryName.MAIN);
+  }
+
+  @Test
   public void testEmptyRepositoryHasEmptyMap() throws Exception {
     helper.parse("");
     assertThat(helper.getPackage().getRepositoryMapping("@")).isEmpty();
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 737fa5b51c..78a95305f6 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
@@ -67,9 +67,7 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase {
         .hasEntryThat(skyKey)
         .isEqualTo(
             RepositoryMappingValue.withMapping(
-                ImmutableMap.of(
-                    RepositoryName.create("@good"), RepositoryName.MAIN,
-                    RepositoryName.create("@a"), RepositoryName.create("@b"))));
+                ImmutableMap.of(RepositoryName.create("@a"), RepositoryName.create("@b"))));
   }
 
   @Test
@@ -97,16 +95,12 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase {
         .hasEntryThat(skyKey1)
         .isEqualTo(
             RepositoryMappingValue.withMapping(
-                ImmutableMap.of(
-                    RepositoryName.create("@good"), RepositoryName.MAIN,
-                    RepositoryName.create("@a"), RepositoryName.create("@b"))));
+                ImmutableMap.of(RepositoryName.create("@a"), RepositoryName.create("@b"))));
     assertThatEvaluationResult(eval(skyKey2))
         .hasEntryThat(skyKey2)
         .isEqualTo(
             RepositoryMappingValue.withMapping(
-                ImmutableMap.of(
-                    RepositoryName.create("@good"), RepositoryName.MAIN,
-                    RepositoryName.create("@x"), RepositoryName.create("@y"))));
+                ImmutableMap.of(RepositoryName.create("@x"), RepositoryName.create("@y"))));
   }
 
   @Test
@@ -128,7 +122,6 @@ 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"))));
   }
@@ -157,7 +150,8 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase {
 
   @Test
   public void testDefaultMainRepoNameInMapping() throws Exception {
-    setSkylarkSemanticsOptions("--experimental_enable_repo_mapping");
+    setSkylarkSemanticsOptions(
+        "--experimental_remap_main_repo", "--experimental_enable_repo_mapping");
     scratch.overwriteFile(
         "WORKSPACE",
         "local_repository(",
@@ -179,6 +173,7 @@ public class RepositoryMappingFunctionTest extends BuildViewTestCase {
 
   @Test
   public void testExplicitMainRepoNameInMapping() throws Exception {
+    setSkylarkSemanticsOptions("--experimental_remap_main_repo");
     scratch.overwriteFile(
         "WORKSPACE",
         "workspace(name = 'good')",
diff --git a/src/test/shell/bazel/workspace_test.sh b/src/test/shell/bazel/workspace_test.sh
index 680d71153e..daed1375c4 100755
--- a/src/test/shell/bazel/workspace_test.sh
+++ b/src/test/shell/bazel/workspace_test.sh
@@ -574,7 +574,8 @@ a = 1
 EOF
 
   cd mainrepo
-  bazel query //... &>"$TEST_log" || fail "Expected query to succeed"
+  bazel query --experimental_remap_main_repo //... &>"$TEST_log" \
+      || fail "Expected query to succeed"
   expect_log "def.bzl loaded"
   expect_not_log "external"
 }
@@ -601,7 +602,8 @@ EOF
   # 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"
+  bazel query --experimental_remap_main_repo @a//... &>"$TEST_log" \
+      || fail "Expected query to succeed"
   expect_log "def.bzl loaded"
   expect_not_log "external"
 
@@ -618,7 +620,8 @@ 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" \
+  bazel query --experimental_remap_main_repo --experimental_enable_repo_mapping \
+      @a//... &>"$TEST_log" \
       && fail "Failure expected" || true
 }