diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java index 2de6c5541a9b51..edcfbf31dbcf9d 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java @@ -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 = { diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java index bc576d639ba613..3d1d3e667518d9 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java @@ -250,7 +250,7 @@ public static Builder builderWithDefaults() { .incompatibleRemapMainRepo(false) .incompatibleRemoveNativeMavenJar(false) .incompatibleRestrictNamedParams(false) - .incompatibleStaticNameResolutionInBuildFiles(false) + .incompatibleStaticNameResolutionInBuildFiles(true) .incompatibleStringJoinRequiresStrings(false) .internalSkylarkFlagTestCanary(false) .incompatibleDoNotSplitLinkingCmdline(false) 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 index f221cabeb8b872..9379951222badf 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); @@ -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); diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestBase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestBase.java index 071c486b1c5fda..6678bf45722669 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestBase.java @@ -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"); diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java index 4caa8525ddeda5..bdd7d35490e0e2 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java @@ -714,9 +714,7 @@ public void testGlobNegativeTest() throws Exception { /*includes=*/ ImmutableList.of("W*", "subdir"), /*excludes=*/ ImmutableList.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 diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java index 7c3adb022f0834..ec9425036eef54 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageFactoryTestBase.java @@ -199,16 +199,12 @@ protected void assertGlobMatches( List result, List includes, List 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 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; diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java index 7a08d8db74e340..ad872a660aec42 100644 --- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java @@ -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 diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java index 8dcdf8e4da93cf..8bf0d8d6a9be34 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunctionTest.java @@ -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')", diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java index 2d25e1a984143e..60bbb78fc8d3ac 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java @@ -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