Skip to content

Commit

Permalink
Flip the flag --incompatible_static_name_resolution_in_build_files
Browse files Browse the repository at this point in the history
Fixes bazelbuild#8022

RELNOTES: --incompatible_static_name_resolution_in_build_files is now enabled by default
PiperOrigin-RevId: 249521083
  • Loading branch information
laurentlb authored and irengrig committed Jun 18, 2019
1 parent 458ff00 commit c0ec3c4
Show file tree
Hide file tree
Showing 9 changed files with 27 additions and 54 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl

@Option(
name = "incompatible_static_name_resolution_in_build_files",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
metadataTags = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ public static Builder builderWithDefaults() {
.incompatibleRemapMainRepo(false)
.incompatibleRemoveNativeMavenJar(false)
.incompatibleRestrictNamedParams(false)
.incompatibleStaticNameResolutionInBuildFiles(false)
.incompatibleStaticNameResolutionInBuildFiles(true)
.incompatibleStringJoinRequiresStrings(false)
.internalSkylarkFlagTestCanary(false)
.incompatibleDoNotSplitLinkingCmdline(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -638,15 +638,13 @@ public void testDepOnGoodTargetInBadPkgAndTransitivelyBadTarget() throws Excepti
scratch.file("parent/BUILD",
"sh_library(name = 'foo',",
" srcs = ['//badpkg1:okay-target', '//okaypkg:transitively-bad-target'])");
Path badpkg1BuildFile = scratch.file("badpkg1/BUILD",
"exports_files(['okay-target'])",
"invalidbuildsyntax");
Path badpkg1BuildFile =
scratch.file("badpkg1/BUILD", "exports_files(['okay-target'])", "fail()");
scratch.file("okaypkg/BUILD",
"sh_library(name = 'transitively-bad-target',",
" srcs = ['//badpkg2:bad-target'])");
Path badpkg2BuildFile = scratch.file("badpkg2/BUILD",
"sh_library(name = 'bad-target')",
"invalidbuildsyntax");
Path badpkg2BuildFile =
scratch.file("badpkg2/BUILD", "sh_library(name = 'bad-target')", "fail()");
update(defaultFlags().with(Flag.KEEP_GOING), "//parent:foo");
assertThat(getFrequencyOfErrorsWithLocation(badpkg1BuildFile.asFragment(), eventCollector))
.isEqualTo(1);
Expand Down Expand Up @@ -695,12 +693,9 @@ public void testTransitiveLoadingDoesntShortCircuitInKeepGoing() throws Exceptio
return;
}
reporter.removeHandler(failFastHandler);
scratch.file("parent/BUILD",
"sh_library(name = 'a', deps = ['//child:b'])",
"parentisbad");
scratch.file("child/BUILD",
"sh_library(name = 'b')",
"childisbad");
scratch.file(
"parent/BUILD", "sh_library(name = 'a', deps = ['//child:b'])", "fail('parentisbad')");
scratch.file("child/BUILD", "sh_library(name = 'b')", "fail('childisbad')");
update(defaultFlags().with(Flag.KEEP_GOING), "//parent:a");
assertContainsEventWithFrequency("parentisbad", 1);
assertContainsEventWithFrequency("childisbad", 1);
Expand Down Expand Up @@ -1042,12 +1037,10 @@ public void testNonTopLevelErrorsPrintedExactlyOnce() throws Exception {
}
scratch.file("parent/BUILD",
"sh_library(name = 'a', deps = ['//child:b'])");
scratch.file("child/BUILD",
"sh_library(name = 'b')",
"undefined_symbol");
scratch.file("child/BUILD", "sh_library(name = 'b')", "fail('some error')");
reporter.removeHandler(failFastHandler);
assertThrows(ViewCreationFailedException.class, () -> update("//parent:a"));
assertContainsEventWithFrequency("name 'undefined_symbol' is not defined", 1);
assertContainsEventWithFrequency("some error", 1);
assertContainsEventWithFrequency(
"Target '//child:b' contains an error and its package is in error and referenced "
+ "by '//parent:a'", 1);
Expand All @@ -1061,12 +1054,10 @@ public void testNonTopLevelErrorsPrintedExactlyOnce_KeepGoing() throws Exception
}
scratch.file("parent/BUILD",
"sh_library(name = 'a', deps = ['//child:b'])");
scratch.file("child/BUILD",
"sh_library(name = 'b')",
"undefined_symbol");
scratch.file("child/BUILD", "sh_library(name = 'b')", "fail('some error')");
reporter.removeHandler(failFastHandler);
update(defaultFlags().with(Flag.KEEP_GOING), "//parent:a");
assertContainsEventWithFrequency("name 'undefined_symbol' is not defined", 1);
assertContainsEventWithFrequency("some error", 1);
assertContainsEventWithFrequency(
"Target '//child:b' contains an error and its package is in error and referenced "
+ "by '//parent:a'", 1);
Expand All @@ -1080,15 +1071,13 @@ public void testNonTopLevelErrorsPrintedExactlyOnce_ActionListener() throws Exce
}
scratch.file("parent/BUILD",
"sh_library(name = 'a', deps = ['//child:b'])");
scratch.file("child/BUILD",
"sh_library(name = 'b')",
"undefined_symbol");
scratch.file("child/BUILD", "sh_library(name = 'b')", "fail('some error')");
scratch.file("okay/BUILD",
"sh_binary(name = 'okay', srcs = ['okay.sh'])");
useConfiguration("--experimental_action_listener=//parent:a");
reporter.removeHandler(failFastHandler);
assertThrows(ViewCreationFailedException.class, () -> update("//okay"));
assertContainsEventWithFrequency("name 'undefined_symbol' is not defined", 1);
assertContainsEventWithFrequency("some error", 1);
assertContainsEventWithFrequency(
"Target '//child:b' contains an error and its package is in error and referenced "
+ "by '//parent:a'", 1);
Expand All @@ -1102,15 +1091,13 @@ public void testNonTopLevelErrorsPrintedExactlyOnce_ActionListener_KeepGoing() t
}
scratch.file("parent/BUILD",
"sh_library(name = 'a', deps = ['//child:b'])");
scratch.file("child/BUILD",
"sh_library(name = 'b')",
"undefined_symbol");
scratch.file("child/BUILD", "sh_library(name = 'b')", "fail('some error')");
scratch.file("okay/BUILD",
"sh_binary(name = 'okay', srcs = ['okay.sh'])");
useConfiguration("--experimental_action_listener=//parent:a");
reporter.removeHandler(failFastHandler);
update(defaultFlags().with(Flag.KEEP_GOING), "//okay");
assertContainsEventWithFrequency("name 'undefined_symbol' is not defined", 1);
assertContainsEventWithFrequency("some error", 1);
assertContainsEventWithFrequency(
"Target '//child:b' contains an error and its package is in error and referenced "
+ "by '//parent:a'", 1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,7 @@ protected void runTestDepOnGoodTargetInBadPkgAndTransitiveCycle(boolean incremen
scratch.file("okaypkg/BUILD",
"sh_library(name = 'transitively-a-cycle',",
" srcs = ['//symlinkcycle:cycle'])");
Path badpkgBuildFile = scratch.file("badpkg/BUILD",
"exports_files(['okay-target'])",
"invalidbuildsyntax");
Path badpkgBuildFile = scratch.file("badpkg/BUILD", "exports_files(['okay-target'])", "fail()");
if (incremental) {
update(defaultFlags().with(Flag.KEEP_GOING), "//okaypkg:transitively-a-cycle");
assertContainsEvent("circular symlinks detected");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -714,9 +714,7 @@ public void testGlobNegativeTest() throws Exception {
/*includes=*/ ImmutableList.of("W*", "subdir"),
/*excludes=*/ ImmutableList.<String>of(),
/* excludeDirs= */ true));
assertThat(e)
.hasMessageThat()
.isEqualTo("ERROR /globs/BUILD:2:73: name 'this_will_fail' is not defined");
assertThat(e).hasMessageThat().isEqualTo("ERROR /globs/BUILD:2:73: incorrect glob result");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,16 +199,12 @@ protected void assertGlobMatches(
List<String> result, List<String> includes, List<String> excludes, boolean excludeDirs)
throws Exception {

// The BUILD language, unlike Skylark, doesn't have fail(), so instead,
// we rely on boolean short circuit logic to only try to evaluate
// the undefined identifier this_will_fail if the result isn't as expected,
// in which case an error occurs (which we test in testGlobNegativeTest).
Pair<Package, GlobCache> evaluated =
evaluateGlob(
includes,
excludes,
excludeDirs,
Printer.format("(result == sorted(%r)) or this_will_fail()", result));
Printer.format("(result == sorted(%r)) or fail('incorrect glob result')", result));

Package pkg = evaluated.first;
GlobCache globCache = evaluated.second;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -735,23 +735,19 @@ public void testSuiteInSuite() throws Exception {

@Test
public void testTopLevelTargetErrorsPrintedExactlyOnce_NoKeepGoing() throws Exception {
tester.addFile("bad/BUILD",
"sh_binary(name = 'bad', srcs = ['bad.sh'])",
"undefined_symbol");
tester.addFile("bad/BUILD", "sh_binary(name = 'bad', srcs = ['bad.sh'])", "fail('some error')");
assertThrows(TargetParsingException.class, () -> tester.load("//bad"));
tester.assertContainsEventWithFrequency("name 'undefined_symbol' is not defined", 1);
tester.assertContainsEventWithFrequency("some error", 1);
PatternExpandingError err = tester.findPostOnce(PatternExpandingError.class);
assertThat(err.getPattern()).containsExactly("//bad");
}

@Test
public void testTopLevelTargetErrorsPrintedExactlyOnce_KeepGoing() throws Exception {
tester.addFile("bad/BUILD",
"sh_binary(name = 'bad', srcs = ['bad.sh'])",
"undefined_symbol");
tester.addFile("bad/BUILD", "sh_binary(name = 'bad', srcs = ['bad.sh'])", "fail('some error')");
TargetPatternPhaseValue result = tester.loadKeepGoing("//bad");
assertThat(result.hasError()).isTrue();
tester.assertContainsEventWithFrequency("name 'undefined_symbol' is not defined", 1);
tester.assertContainsEventWithFrequency("some error", 1);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,6 @@ public void testRepositoryMappingInChunks() throws Exception {
scratch.file("BUILD", "");
RootedPath workspace =
createWorkspaceFile(
"WORKSPACE",
"workspace(name = 'good')",
"local_repository(name = 'a', path = '../a', repo_mapping = {'@x' : '@y'})",
"load('//:b.bzl', 'b')",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -703,20 +703,19 @@ public void testForStatementForbiddenInBuild() throws Exception {

@Test
public void testIfStatementForbiddenInBuild() throws Exception {
new BuildTest()
.testIfErrorContains("if statements are not allowed in BUILD files", "if False: pass");
new BuildTest().testIfErrorContains("if statements are not allowed", "if False: pass");
}

@Test
public void testKwargsForbiddenInBuild() throws Exception {
new BuildTest()
.testIfErrorContains("**kwargs arguments are not allowed in BUILD files", "func(**dict)");
.testIfErrorContains("**kwargs arguments are not allowed in BUILD files", "print(**dict)");
}

@Test
public void testArgsForbiddenInBuild() throws Exception {
new BuildTest()
.testIfErrorContains("*args arguments are not allowed in BUILD files", "func(*array)");
.testIfErrorContains("*args arguments are not allowed in BUILD files", "print(*['a'])");
}

@Test
Expand Down

0 comments on commit c0ec3c4

Please sign in to comment.