Enable incompatible_disallow_load_labels_to_cross_package_boundaries by default
Fixes https://github.com/bazelbuild/bazel/issues/6408
Tested: https://buildkite.com/bazel/bazelisk-plus-incompatible-flags/builds/52
RELNOTES: `--incompatible_disallow_load_labels_to_cross_package_boundaries` is enabled by default
PiperOrigin-RevId: 240347889
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 095e3e83..59b6376 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
@@ -313,7 +313,7 @@
@Option(
name = "incompatible_disallow_load_labels_to_cross_package_boundaries",
- 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/skyframe/SkylarkModuleCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
index 71035b4..12ab5e0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java
@@ -127,11 +127,10 @@
+ "' was defined too late in your WORKSPACE file."));
return true;
} else if (Iterables.any(cycle, IS_PACKAGE_LOOKUP)) {
- eventHandler.handle(
- Event.error(null, "cycle detected loading "
- + String.join(
- " ", lastPathElement.functionName().toString().toLowerCase().split("_"))
- + " '" + lastPathElement.argument().toString() + "'"));
+ PackageIdentifier pkg =
+ (PackageIdentifier)
+ Iterables.getLast(Iterables.filter(cycle, IS_PACKAGE_LOOKUP)).argument();
+ eventHandler.handle(Event.error(null, "cannot load package '" + pkg + "'"));
return true;
}
}
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 9e54d4c..c60216d 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
@@ -219,7 +219,7 @@
.incompatibleDisallowFileType(true)
.incompatibleDisallowLegacyJavaProvider(false)
.incompatibleDisallowLegacyJavaInfo(false)
- .incompatibleDisallowLoadLabelsToCrossPackageBoundaries(false)
+ .incompatibleDisallowLoadLabelsToCrossPackageBoundaries(true)
.incompatibleDisallowNativeInBuildFile(false)
.incompatibleDisallowOldStyleArgsAdd(true)
.incompatibleDisallowStructProviderSyntax(false)
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java
index 5dad405..7d412d8 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java
@@ -195,8 +195,8 @@
ErrorInfo errorInfo = result.getError(skylarkImportLookupKey);
String errorMessage = errorInfo.getException().getMessage();
assertThat(errorMessage)
- .isEqualTo(
- "Unable to load package for '//pkg:ext.bzl': BUILD file not found on package path");
+ .contains(
+ "Every .bzl file must have a corresponding package, but '//pkg:ext.bzl' does not");
}
@Test
@@ -211,9 +211,7 @@
assertThat(result.hasError()).isTrue();
ErrorInfo errorInfo = result.getError(skylarkImportLookupKey);
String errorMessage = errorInfo.getException().getMessage();
- assertThat(errorMessage)
- .isEqualTo(
- "Unable to load package for '//pkg:ext.bzl': BUILD file not found on package path");
+ assertThat(errorMessage).contains("Every .bzl file must have a corresponding package");
}
@Test
@@ -276,6 +274,8 @@
@Test
public void testLoadUsingLabelThatCrossesBoundaryOfPackage_Allow_OfSamePkg() throws Exception {
+ setSkylarkSemanticsOptions("--noincompatible_disallow_load_labels_to_cross_package_boundaries");
+
scratch.file("a/BUILD");
scratch.file("a/a.bzl", "load('//a:b/b.bzl', 'b')");
scratch.file("a/b/BUILD", "");
@@ -314,6 +314,7 @@
@Test
public void testLoadUsingLabelThatCrossesBoundaryOfPackage_Allow_OfDifferentPkgUnder()
throws Exception {
+ setSkylarkSemanticsOptions("--noincompatible_disallow_load_labels_to_cross_package_boundaries");
scratch.file("a/BUILD");
scratch.file("a/a.bzl", "load('//a/b:c/c.bzl', 'c')");
scratch.file("a/b/BUILD", "");
@@ -355,6 +356,7 @@
@Test
public void testLoadUsingLabelThatCrossesBoundaryOfPackage_Allow_OfDifferentPkgAbove()
throws Exception {
+ setSkylarkSemanticsOptions("--noincompatible_disallow_load_labels_to_cross_package_boundaries");
scratch.file("a/b/BUILD");
scratch.file("a/b/b.bzl", "load('//a/c:c/c.bzl', 'c')");
scratch.file("a/BUILD");
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
index e578b03..e471c19 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java
@@ -1252,7 +1252,7 @@
} catch (ViewCreationFailedException e) {
// expect to fail.
}
- assertContainsEvent("Unable to load package for '//foo:aspect.bzl'");
+ assertContainsEvent("Every .bzl file must have a corresponding package");
}
@Test
diff --git a/src/test/shell/bazel/skylark_repository_test.sh b/src/test/shell/bazel/skylark_repository_test.sh
index f1e9c9b..d5e090b 100755
--- a/src/test/shell/bazel/skylark_repository_test.sh
+++ b/src/test/shell/bazel/skylark_repository_test.sh
@@ -293,8 +293,7 @@
[ $exitCode != 0 ] || fail "building @foo//:data.txt succeed while expected failure"
expect_not_log "PACKAGE"
- expect_log "Failed to load Starlark extension '@foo//:ext.bzl'"
- expect_log "repository 'foo' was defined too late in your WORKSPACE file"
+ expect_log "cannot load package '@foo//'"
}
function test_load_nonexistent_with_subworkspace() {
@@ -312,8 +311,7 @@
[ $exitCode != 0 ] || fail "building //... succeed while expected failure"
expect_not_log "PACKAGE"
- expect_log "Failed to load Starlark extension '@does_not_exist//:random.bzl'"
- expect_log "repository 'does_not_exist' was defined too late in your WORKSPACE file"
+ expect_log "cannot load package '@does_not_exist//'"
# Retest with query //...
bazel clean --expunge
@@ -321,8 +319,7 @@
[ $exitCode != 0 ] || fail "querying //... succeed while expected failure"
expect_not_log "PACKAGE"
- expect_log "Failed to load Starlark extension '@does_not_exist//:random.bzl'"
- expect_log "repository 'does_not_exist' was defined too late in your WORKSPACE file"
+ expect_log "cannot load package '@does_not_exist//'"
}
function test_skylark_local_repository() {
diff --git a/src/test/shell/integration/loading_phase_test.sh b/src/test/shell/integration/loading_phase_test.sh
index 2fc74af..c208628 100755
--- a/src/test/shell/integration/loading_phase_test.sh
+++ b/src/test/shell/integration/loading_phase_test.sh
@@ -365,7 +365,9 @@
touch "$pkg"/foo/a/b/BUILD
echo "b = 42" > "$pkg"/foo/a/b/b.bzl
- bazel query "$pkg/foo:BUILD" >& "$TEST_log" || fail "Expected success"
+ bazel query \
+ --incompatible_disallow_load_labels_to_cross_package_boundaries=false \
+ "$pkg/foo:BUILD" >& "$TEST_log" || fail "Expected success"
expect_log "//$pkg/foo:BUILD"
bazel query \
@@ -373,7 +375,9 @@
"$pkg/foo:BUILD" >& "$TEST_log" && fail "Expected failure"
expect_log "Label '//$pkg/foo/a:b/b.bzl' crosses boundary of subpackage '$pkg/foo/a/b'"
- bazel query "$pkg/foo:BUILD" >& "$TEST_log" || fail "Expected success"
+ bazel query \
+ --incompatible_disallow_load_labels_to_cross_package_boundaries=false \
+ "$pkg/foo:BUILD" >& "$TEST_log" || fail "Expected success"
expect_log "//$pkg/foo:BUILD"
}
