Skip to content

Commit

Permalink
Turn on unambiguous label stringification by default
Browse files Browse the repository at this point in the history
Fixes #15916

Fixes #16196

PiperOrigin-RevId: 474520825
Change-Id: I38caf7879fb82c72e07ef11a96be20ade5ac5401
  • Loading branch information
Wyverald authored and copybara-github committed Sep 15, 2022
1 parent 5d1856e commit b1113f8
Show file tree
Hide file tree
Showing 20 changed files with 301 additions and 298 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
Loading

0 comments on commit b1113f8

Please sign in to comment.