Enable incompatible_disallow_load_labels_to_cross_package_boundaries …

…by default

Fixes #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

src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java

@@ -313,7 +313,7 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
 
   @Option(
       name = "incompatible_disallow_load_labels_to_cross_package_boundaries",
-      defaultValue = "false",
+      defaultValue = "true",
       documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
       effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
       metadataTags = {

src/main/java/com/google/devtools/build/lib/skyframe/SkylarkModuleCycleReporter.java

@@ -127,11 +127,10 @@ public String apply(SkyKey input) {
                     + "' 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;
       }
     }

src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -219,7 +219,7 @@ public static Builder builderWithDefaults() {
           .incompatibleDisallowFileType(true)
           .incompatibleDisallowLegacyJavaProvider(false)
           .incompatibleDisallowLegacyJavaInfo(false)
-          .incompatibleDisallowLoadLabelsToCrossPackageBoundaries(false)
+          .incompatibleDisallowLoadLabelsToCrossPackageBoundaries(true)
           .incompatibleDisallowNativeInBuildFile(false)
           .incompatibleDisallowOldStyleArgsAdd(true)
           .incompatibleDisallowStructProviderSyntax(false)

src/test/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunctionTest.java

@@ -195,8 +195,8 @@ public void testSkylarkImportLookupNoBuildFile() throws Exception {
     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 @@ public void testSkylarkImportLookupNoBuildFileForLoad() throws Exception {
     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 @@ public void testLoadUsingLabelThatDoesntCrossBoundaryOfPackage() throws Exceptio
 
   @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 @@ public void testLoadUsingLabelThatCrossesBoundaryOfPackage_Disallow_OfSamePkg()
   @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 @@ public void testLoadUsingLabelThatCrossesBoundaryOfPackage_Disallow_OfDifferentP
   @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");

src/test/java/com/google/devtools/build/lib/skylark/SkylarkDefinedAspectsTest.java

@@ -1252,7 +1252,7 @@ public void topLevelAspectDoesNotExistNoBuildFile() throws Exception {
     } 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

src/test/shell/bazel/skylark_repository_test.sh

@@ -293,8 +293,7 @@ EOF
   [ $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,17 +311,15 @@ EOF
   [ $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
   bazel query //... >& $TEST_log || exitCode=$?
   [ $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() {

src/test/shell/integration/loading_phase_test.sh

@@ -365,15 +365,19 @@ function test_incompatible_disallow_load_labels_to_cross_package_boundaries() {
   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 \
     --incompatible_disallow_load_labels_to_cross_package_boundaries=true \
     "$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"
 }