Turn on unambiguous label stringification by default

Fixes #15916 Fixes #16196 PiperOrigin-RevId: 470939938 Change-Id: I38caf7879fb82c72e07ef11a96be20ade5ac5401
bazelbuild · Sep 2, 2022 · bf8ec1e · bf8ec1e
1 parent 0aa750b
commit bf8ec1e
Show file tree

Hide file tree

Showing 21 changed files with 302 additions and 299 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java
@@ -80,7 +80,7 @@ public class CoreOptions extends FragmentOptions implements Cloneable {
 
   @Option(
       name = "incompatible_use_platforms_repo_for_constraints",
-      defaultValue = "true",
+      defaultValue = "false",
       documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
       effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
       metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},

diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
@@ -523,7 +523,7 @@ public final class BuildLanguageOptions extends OptionsBase {
 
   @Option(
       name = "incompatible_unambiguous_label_stringification",
-      defaultValue = "false",
+      defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
       metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
@@ -759,7 +759,7 @@ public StarlarkSemantics toStarlarkSemantics() {
   public static final String INCOMPATIBLE_USE_CC_CONFIGURE_FROM_RULES_CC =
       "-incompatible_use_cc_configure_from_rules";
   public static final String INCOMPATIBLE_UNAMBIGUOUS_LABEL_STRINGIFICATION =
-      "-incompatible_unambiguous_label_stringification";
+      "+incompatible_unambiguous_label_stringification";
   public static final String INCOMPATIBLE_VISIBILITY_PRIVATE_ATTRIBUTES_AT_DEFINITION =
       "-incompatible_visibility_private_attributes_at_definition";
   public static final String INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS =

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -184,7 +184,7 @@ public void testGetArtifactOwnerInStarlark() throws Exception {
     scratch.file("foo/BUILD", "load(':rule.bzl', 'gen')", "gen(name = 'a')");
 
     update("//foo:a");
-    assertContainsEvent("DEBUG /workspace/foo/rule.bzl:3:8: f owner is //foo:a");
+    assertContainsEvent("DEBUG /workspace/foo/rule.bzl:3:8: f owner is @//foo:a");
   }
 
   @Test

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkAttrTransitionProviderTest.java
@@ -562,7 +562,7 @@ private void writeReadAndPassthroughOptionsTestFiles() throws Exception {
         "settings_under_test = {",
         "  '//command_line_option:cpu': 'armeabi-v7a',",
         "  '//command_line_option:compilation_mode': 'dbg',",
-        "  '//command_line_option:crosstool_top': '//android/crosstool:everything',",
+        "  '//command_line_option:crosstool_top': '@//android/crosstool:everything',",
         "  '//command_line_option:platform_suffix': 'my-platform-suffix',",
         "}",
         "def set_options_transition_func(settings, attr):",

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/test/TestActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/test/TestActionBuilderTest.java
@@ -219,7 +219,7 @@ public void testAspectOverNonExpandingTestSuitesVisitsImplicitTests() throws Exc
         new StarlarkProvider.Key(Label.parseAbsoluteUnchecked("//:aspect.bzl"), "StructImpl");
     StructImpl info = (StructImpl) aspectValue.get(key);
     assertThat(((Depset) info.getValue("labels")).getSet(String.class).toList())
-        .containsExactly("//:suite", "//:test_a", "//:test_b");
+        .containsExactly("@//:suite", "@//:test_a", "@//:test_b");
   }
 
   @Test
@@ -252,7 +252,7 @@ public void testAspectOverNonExpandingTestSuitesVisitsExplicitTests() throws Exc
         new StarlarkProvider.Key(Label.parseAbsoluteUnchecked("//:aspect.bzl"), "StructImpl");
     StructImpl info = (StructImpl) aspectValue.get(key);
     assertThat(((Depset) info.getValue("labels")).getSet(String.class).toList())
-        .containsExactly("//:suite", "//:test_b");
+        .containsExactly("@//:suite", "@//:test_b");
   }
 
   @Test
@@ -285,7 +285,7 @@ public void testAspectOverExpandingTestSuitesDoesNotVisitSuite() throws Exceptio
       StructImpl info = (StructImpl) a.get(key);
       labels.addAll(((Depset) info.getValue("labels")).getSet(String.class).toList());
     }
-    assertThat(labels).containsExactly("//:test_a", "//:test_b");
+    assertThat(labels).containsExactly("@//:test_a", "@//:test_b");
   }
 
   private void writeLabelCollectionAspect() throws IOException {

diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/KeepGoingTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/KeepGoingTest.java
@@ -259,7 +259,7 @@ public void testConfigurationErrorsAreToleratedWithKeepGoing() throws Exception
     assertBuildFailedExceptionFromBuilding(
         "command succeeded, but not all targets were analyzed", "//a", "//b");
     events.assertContainsError(
-        "in srcs attribute of cc_library rule //a:a: source file '//a:missing.foo' is misplaced"
+        "in srcs attribute of cc_library rule @//a:a: source file '@//a:missing.foo' is misplaced"
             + " here");
     events.assertContainsInfo("Analysis succeeded for only 1 of 2 top-level targets");
 

diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/LabelTest.java
@@ -515,7 +515,7 @@ public void testWorkspaceName() throws Exception {
   @Test
   public void starlarkStrAndRepr() throws Exception {
     Label label = Label.parseCanonical("//x");
-    assertThat(Starlark.str(label, StarlarkSemantics.DEFAULT)).isEqualTo("//x:x");
+    assertThat(Starlark.str(label, StarlarkSemantics.DEFAULT)).isEqualTo("@//x:x");
     assertThat(Starlark.repr(label)).isEqualTo("Label(\"//x:x\")");
 
     label = Label.parseCanonical("@hello//x");
@@ -524,31 +524,31 @@ public void starlarkStrAndRepr() throws Exception {
   }
 
   @Test
-  public void starlarkStr_unambiguous() throws Exception {
+  public void starlarkStr_ambiguous() throws Exception {
     StarlarkSemantics semantics =
         StarlarkSemantics.builder()
-            .setBool(BuildLanguageOptions.INCOMPATIBLE_UNAMBIGUOUS_LABEL_STRINGIFICATION, true)
+            .setBool(BuildLanguageOptions.INCOMPATIBLE_UNAMBIGUOUS_LABEL_STRINGIFICATION, false)
             .build();
-    assertThat(Starlark.str(Label.parseCanonical("//x"), semantics)).isEqualTo("@//x:x");
+    assertThat(Starlark.str(Label.parseCanonical("//x"), semantics)).isEqualTo("//x:x");
     assertThat(Starlark.str(Label.parseCanonical("@x//y"), semantics)).isEqualTo("@x//y:y");
   }
 
   @Test
   public void starlarkStr_canonicalLabelLiteral() throws Exception {
     StarlarkSemantics semantics =
         StarlarkSemantics.builder().setBool(BuildLanguageOptions.ENABLE_BZLMOD, true).build();
-    assertThat(Starlark.str(Label.parseCanonical("//x"), semantics)).isEqualTo("//x:x");
+    assertThat(Starlark.str(Label.parseCanonical("//x"), semantics)).isEqualTo("@@//x:x");
     assertThat(Starlark.str(Label.parseCanonical("@x//y"), semantics)).isEqualTo("@@x//y:y");
   }
 
   @Test
-  public void starlarkStr_unambiguousAndCanonicalLabelLiteral() throws Exception {
+  public void starlarkStr_ambiguousAndCanonicalLabelLiteral() throws Exception {
     StarlarkSemantics semantics =
         StarlarkSemantics.builder()
-            .setBool(BuildLanguageOptions.INCOMPATIBLE_UNAMBIGUOUS_LABEL_STRINGIFICATION, true)
+            .setBool(BuildLanguageOptions.INCOMPATIBLE_UNAMBIGUOUS_LABEL_STRINGIFICATION, false)
             .setBool(BuildLanguageOptions.ENABLE_BZLMOD, true)
             .build();
-    assertThat(Starlark.str(Label.parseCanonical("//x"), semantics)).isEqualTo("@@//x:x");
+    assertThat(Starlark.str(Label.parseCanonical("//x"), semantics)).isEqualTo("//x:x");
     assertThat(Starlark.str(Label.parseCanonical("@x//y"), semantics)).isEqualTo("@@x//y:y");
   }
 }
diff --git a/...st/java/com/google/devtools/build/lib/rules/android/AbstractAndroidLocalTestTestBase.java b/...st/java/com/google/devtools/build/lib/rules/android/AbstractAndroidLocalTestTestBase.java
@@ -318,7 +318,9 @@ public void testFeatureFlagsAttributeSetsFeatureFlagProviderValues() throws Exce
             "/FooFlags.java");
     FileWriteAction action = (FileWriteAction) getGeneratingAction(flagList);
     assertThat(action.getFileContents())
-        .isEqualTo("//java/com/foo:flag1: on\n//java/com/foo:flag2: off");
+        .isAnyOf(
+            "//java/com/foo:flag1: on\n//java/com/foo:flag2: off",
+            "@//java/com/foo:flag1: on\n@//java/com/foo:flag2: off");
   }
 
   @Test

diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/android/AndroidBinaryTest.java
@@ -3777,7 +3777,7 @@ public void testFeatureFlagsAttributeSetsFeatureFlagProviderValues() throws Exce
             "/FooFlags.java");
     FileWriteAction action = (FileWriteAction) getGeneratingAction(flagList);
     assertThat(action.getFileContents())
-        .isEqualTo("//java/com/foo:flag1: on\n//java/com/foo:flag2: off");
+        .isEqualTo("@//java/com/foo:flag1: on\n@//java/com/foo:flag2: off");
   }
 
   @Test

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcBadDependenciesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcBadDependenciesTest.java
@@ -30,12 +30,11 @@ private ConfiguredTarget configure(String targetLabel) throws Exception {
   @Test
   public void testRejectsSingleUnknownSourceFile() throws Exception {
     reporter.removeHandler(failFastHandler);
-    scratch.file("foo/BUILD",
-                "cc_library(name = 'foo', srcs = ['unknown.oops'])");
+    scratch.file("foo/BUILD", "cc_library(name = 'foo', srcs = ['unknown.oops'])");
     scratch.file("foo/unknown.oops", "foo");
     configure("//foo:foo");
-    assertContainsEvent(getErrorMsgMisplacedFiles(
-        "srcs", "cc_library", "//foo:foo", "//foo:unknown.oops"));
+    assertContainsEvent(
+        getErrorMsgMisplacedFiles("srcs", "cc_library", "@//foo:foo", "@//foo:unknown.oops"));
   }
 
   @Test
@@ -65,6 +64,6 @@ public void testRejectsBadGeneratedFile() throws Exception {
     assertContainsEvent(
         String.format(
             "attribute srcs: '%s' does not produce any cc_library srcs files",
-            "//dependency:generated"));
+            "@//dependency:generated"));
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoCommonTest.java
@@ -580,11 +580,11 @@ public void shouldGenerateCode_mixed() throws Exception {
     getConfiguredTarget("//bar:simple");
 
     assertContainsEvent(
-        "The 'srcs' attribute of '//third_party/x:mixed' contains protos for which 'MyRule'"
+        "The 'srcs' attribute of '@//third_party/x:mixed' contains protos for which 'MyRule'"
             + " shouldn't generate code (third_party/x/metadata.proto,"
             + " third_party/x/descriptor.proto), in addition to protos for which it should"
             + " (third_party/x/something.proto).\n"
-            + "Separate '//third_party/x:mixed' into 2 proto_library rules.");
+            + "Separate '@//third_party/x:mixed' into 2 proto_library rules.");
   }
 
   /** Verifies <code>proto_common.declare_generated_files</code> call. */

diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PythonSrcsVersionAspectTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PythonSrcsVersionAspectTest.java
@@ -78,10 +78,10 @@ public void simpleCase() throws Exception {
             "<None>",
             "",
             "Python 3-only deps:",
-            "//pkg:lib",
+            "@//pkg:lib",
             "",
             "Paths to these deps:",
-            "//pkg:bin -> //pkg:lib",
+            "@//pkg:bin -> @//pkg:lib",
             "");
     assertThat(result).isEqualTo(golden);
   }
@@ -138,14 +138,14 @@ public void twoContradictoryRequirements() throws Exception {
     String golden =
         join(
             "Python 2-only deps:",
-            "//pkg:lib2",
+            "@//pkg:lib2",
             "",
             "Python 3-only deps:",
-            "//pkg:lib3",
+            "@//pkg:lib3",
             "",
             "Paths to these deps:",
-            "//pkg:bin -> //pkg:lib2",
-            "//pkg:bin -> //pkg:lib3",
+            "@//pkg:bin -> @//pkg:lib2",
+            "@//pkg:bin -> @//pkg:lib3",
             "");
     assertThat(result).isEqualTo(golden);
   }
@@ -168,10 +168,10 @@ public void toplevelSelfContradictory() throws Exception {
             "<None>",
             "",
             "Python 3-only deps:",
-            "//pkg:bin",
+            "@//pkg:bin",
             "",
             "Paths to these deps:",
-            "//pkg:bin",
+            "@//pkg:bin",
             "");
     assertThat(result).isEqualTo(golden);
   }
@@ -209,10 +209,10 @@ public void indirectDependencies() throws Exception {
             "<None>",
             "",
             "Python 3-only deps:",
-            "//pkg:libB",
+            "@//pkg:libB",
             "",
             "Paths to these deps:",
-            "//pkg:bin -> //pkg:libC -> //pkg:libB",
+            "@//pkg:bin -> @//pkg:libC -> @//pkg:libB",
             "");
     assertThat(result).isEqualTo(golden);
   }
@@ -251,10 +251,10 @@ public void onlyReportTopmost() throws Exception {
             "<None>",
             "",
             "Python 3-only deps:",
-            "//pkg:libC",
+            "@//pkg:libC",
             "",
             "Paths to these deps:",
-            "//pkg:bin -> //pkg:libC",
+            "@//pkg:bin -> @//pkg:libC",
             "");
     assertThat(result).isEqualTo(golden);
   }
@@ -294,12 +294,12 @@ public void oneTopmostReachesAnother() throws Exception {
             "<None>",
             "",
             "Python 3-only deps:",
-            "//pkg:libA",
-            "//pkg:libC",
+            "@//pkg:libA",
+            "@//pkg:libC",
             "",
             "Paths to these deps:",
-            "//pkg:bin -> //pkg:libA",
-            "//pkg:bin -> //pkg:libC",
+            "@//pkg:bin -> @//pkg:libA",
+            "@//pkg:bin -> @//pkg:libC",
             "");
     assertThat(result).isEqualTo(golden);
   }
@@ -338,10 +338,10 @@ public void multiplePathsToRequirement() throws Exception {
             "<None>",
             "",
             "Python 3-only deps:",
-            "//pkg:libA",
+            "@//pkg:libA",
             "",
             "Paths to these deps:",
-            "//pkg:bin -> //pkg:libB -> //pkg:libA",
+            "@//pkg:bin -> @//pkg:libB -> @//pkg:libA",
             "");
     assertThat(result).isEqualTo(golden);
   }
@@ -391,10 +391,10 @@ public void noSrcsVersionButIntroducesRequirement() throws Exception {
             "<None>",
             "",
             "Python 3-only deps:",
-            "//pkg:libB",
+            "@//pkg:libB",
             "",
             "Paths to these deps:",
-            "//pkg:bin -> //pkg:libC -> //pkg:libB",
+            "@//pkg:bin -> @//pkg:libC -> @//pkg:libB",
             "");
     assertThat(result).isEqualTo(golden);
   }