Add --[no]check_bzl_visibility

This is a break-glass flag analogous to --[no]check_visibility, designed to unblock developers who are prototyping changes locally.

Work toward bazelbuild#11261.

PiperOrigin-RevId: 479387465
Change-Id: I4a10a5e2f9616065ccd525d5f627d21773cd5032

src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java

@@ -467,12 +467,13 @@ public ExecConfigurationDistinguisherSchemeConverter() {
  + " '//package:target --options'.")
  public RunUnder runUnder;
 
+ // TODO(b/248763226): Consider moving this to a non-FragmentOptions.
  @Option(
  name = "check_visibility",
  defaultValue = "true",
  documentationCategory = OptionDocumentationCategory.INPUT_STRICTNESS,
  effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
- help = "If disabled, visibility errors are demoted to warnings.")
+ help = "If disabled, visibility errors in target dependencies are demoted to warnings.")
  public boolean checkVisibility;
 
  @Option(

src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java

@@ -176,6 +176,14 @@ public final class BuildLanguageOptions extends OptionsBase {
  + " special item \"everyone\", all packages are permitted.")
  public List<String> experimentalBzlVisibilityAllowlist;
 
+ @Option(
+ name = "check_bzl_visibility",
+ defaultValue = "true",
+ documentationCategory = OptionDocumentationCategory.INPUT_STRICTNESS,
+ effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
+ help = "If disabled, bzl-visibility errors in load() statements are demoted to warnings.")
+ public boolean checkBzlVisibility;
+
  @Option(
  name = "experimental_cc_skylark_api_enabled_packages",
  converter = CommaSeparatedOptionListConverter.class,
@@ -674,6 +682,7 @@ public StarlarkSemantics toStarlarkSemantics() {
  .set(EXPERIMENTAL_BUILTINS_INJECTION_OVERRIDE, experimentalBuiltinsInjectionOverride)
  .setBool(EXPERIMENTAL_BZL_VISIBILITY, experimentalBzlVisibility)
  .set(EXPERIMENTAL_BZL_VISIBILITY_ALLOWLIST, experimentalBzlVisibilityAllowlist)
+ .setBool(CHECK_BZL_VISIBILITY, checkBzlVisibility)
  .setBool(
  EXPERIMENTAL_ENABLE_ANDROID_MIGRATION_APIS, experimentalEnableAndroidMigrationApis)
  .setBool(ENABLE_BZLMOD, enableBzlmod)
@@ -762,6 +771,7 @@ public StarlarkSemantics toStarlarkSemantics() {
  "-experimental_allow_tags_propagation";
  public static final String EXPERIMENTAL_BUILTINS_DUMMY = "-experimental_builtins_dummy";
  public static final String EXPERIMENTAL_BZL_VISIBILITY = "-experimental_bzl_visibility";
+ public static final String CHECK_BZL_VISIBILITY = "+check_bzl_visibility";
  public static final String EXPERIMENTAL_CC_SHARED_LIBRARY = "-experimental_cc_shared_library";
  public static final String EXPERIMENTAL_DISABLE_EXTERNAL_PACKAGE =
  "-experimental_disable_external_package";

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

@@ -776,6 +776,8 @@ private BzlLoadValue computeInternalWithCompiledBzl(
  loadValues,
  loadKeys,
  programLoads,
+ /*demoteErrorsToWarnings=*/ !builtins.starlarkSemantics.getBool(
+ BuildLanguageOptions.CHECK_BZL_VISIBILITY),
  env.getListener());
 
  // Accumulate a transitive digest of the bzl file, the digests of its direct loads, and the
@@ -1104,29 +1106,35 @@ static void checkLoadVisibilities(
  List<BzlLoadValue> loadValues,
  List<BzlLoadValue.Key> loadKeys,
  List<Pair<String, Location>> programLoads,
+ boolean demoteErrorsToWarnings,
  EventHandler handler)
  throws BzlLoadFailedException {
- boolean ok = true;
+ boolean foundViolation = false;
  for (int i = 0; i < loadValues.size(); i++) {
  BzlVisibility loadVisibility = loadValues.get(i).getBzlVisibility();
  Label loadLabel = loadKeys.get(i).getLabel();
  PackageIdentifier loadPackage = loadLabel.getPackageIdentifier();
  if (!(requestingPackage.equals(loadPackage)
  || loadVisibility.allowsPackage(requestingPackage))) {
- handler.handle(
- Event.error(
- programLoads.get(i).second,
- String.format(
- // TODO(brandjon): Consider whether we should try to report error messages (here
- // and elsewhere) using the literal text of the load() rather than the (already
- // repo-remapped) label.
- "Starlark file %s is not visible for loading from package %s. Check the"
- + " file's `visibility()` declaration.",
- loadLabel, requestingPackage.getCanonicalForm())));
- ok = false;
+ Location loc = programLoads.get(i).second;
+ String msg =
+ String.format(
+ // TODO(brandjon): Consider whether we should try to report error messages (here
+ // and elsewhere) using the literal text of the load() rather than the (already
+ // repo-remapped) label.
+ "Starlark file %s is not visible for loading from package %s. Check the"
+ + " file's `visibility()` declaration.",
+ loadLabel, requestingPackage.getCanonicalForm());
+ if (demoteErrorsToWarnings) {
+ msg += " Continuing because --nocheck_bzl_visibility is active";
+ handler.handle(Event.warn(loc, msg));
+ } else {
+ handler.handle(Event.error(loc, msg));
+ }
+ foundViolation = true;
  }
  }
- if (!ok) {
+ if (foundViolation && !demoteErrorsToWarnings) {
  throw BzlLoadFailedException.visibilityViolation(requestingFileDescription);
  }
  }

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

@@ -154,7 +154,8 @@ private BzlmodRepoRuleValue createRuleFromSpec(
  }
  ruleClass = ruleClassProvider.getRuleClassMap().get(repoSpec.ruleClassName());
  } else {
- ImmutableMap<String, Module> loadedModules = loadBzlModules(env, repoSpec.bzlFile().get());
+ ImmutableMap<String, Module> loadedModules =
+ loadBzlModules(env, repoSpec.bzlFile().get(), starlarkSemantics);
  if (env.valuesMissing()) {
  return null;
  }
@@ -182,7 +183,8 @@ private BzlmodRepoRuleValue createRuleFromSpec(
  }
 
  /** Loads modules from the given bzl file. */
- private ImmutableMap<String, Module> loadBzlModules(Environment env, String bzlFile)
+ private ImmutableMap<String, Module> loadBzlModules(
+ Environment env, String bzlFile, StarlarkSemantics starlarkSemantics)
  throws InterruptedException, BzlmodRepoRuleFunctionException {
  ImmutableList<Pair<String, Location>> programLoads =
  ImmutableList.of(Pair.of(bzlFile, Location.BUILTIN));
@@ -213,7 +215,13 @@ private ImmutableMap<String, Module> loadBzlModules(Environment env, String bzlF
  // TODO(b/22193153, wyv): Determine whether bzl-visibility should apply at all to this type of
  // bzl load. As it stands, this call checks that bzlFile is visible to package @//.
  return PackageFunction.loadBzlModules(
- env, PackageIdentifier.EMPTY_PACKAGE_ID, "Bzlmod system", programLoads, keys, null);
+ env,
+ PackageIdentifier.EMPTY_PACKAGE_ID,
+ "Bzlmod system",
+ programLoads,
+ keys,
+ starlarkSemantics,
+ null);
  } catch (NoSuchPackageException e) {
  throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
  }

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

@@ -642,6 +642,7 @@ static ImmutableMap<String, Module> loadBzlModules(
  String requestingFileDescription,
  List<Pair<String, Location>> programLoads,
  List<BzlLoadValue.Key> keys,
+ StarlarkSemantics semantics,
  @Nullable BzlLoadFunction bzlLoadFunctionForInlining)
  throws NoSuchPackageException, InterruptedException {
  List<BzlLoadValue> bzlLoads;
@@ -656,7 +657,13 @@ static ImmutableMap<String, Module> loadBzlModules(
  // Validate that the current BUILD/WORKSPACE file satisfies each loaded dependency's
  // bzl-visibility.
  BzlLoadFunction.checkLoadVisibilities(
- packageId, requestingFileDescription, bzlLoads, keys, programLoads, env.getListener());
+ packageId,
+ requestingFileDescription,
+ bzlLoads,
+ keys,
+ programLoads,
+ /*demoteErrorsToWarnings=*/ !semantics.getBool(BuildLanguageOptions.CHECK_BZL_VISIBILITY),
+ env.getListener());
  } catch (BzlLoadFailedException e) {
  Throwable rootCause = Throwables.getRootCause(e);
  throw PackageFunctionException.builder()
@@ -1314,6 +1321,7 @@ private LoadedPackage loadPackage(
  "file " + buildFileLabel.getCanonicalForm(),
  programLoads,
  keys.build(),
+ starlarkBuiltinsValue.starlarkSemantics,
  bzlLoadFunctionForInlining);
  } catch (NoSuchPackageException e) {
  throw new PackageFunctionException(e, Transience.PERSISTENT);

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

@@ -323,6 +323,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
  "WORKSPACE content",
  programLoads,
  keys.build(),
+ starlarkSemantics,
  bzlLoadFunctionForInlining);
  } catch (NoSuchPackageException e) {
  throw new WorkspaceFileFunctionException(e, Transience.PERSISTENT);

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkBuildApiGlobals.java

@@ -43,7 +43,9 @@ public interface StarlarkBuildApiGlobals {
  + "<p><code>visibility()</code> may only be called once per .bzl file, and only at"
  + " the top level, not inside a function. The preferred style is to put this call"
  + " immediately below the <code>load()</code> statements and any brief logic needed"
- + " to determine the argument.",
+ + " to determine the argument."
+ + "<p>If the flag <code>--check_bzl_visibility</code> is set to false, bzl-visibility"
+ + " violations will emit warnings but not fail the build.",
  parameters = {
  @Param(
  name = "value",

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

@@ -1000,6 +1000,28 @@ public void testBzlVisibility_invalid_negationNotSupported() throws Exception {
  assertContainsEvent("Cannot use negative package patterns here");
  }
 
+ @Test
+ public void testBzlVisibility_errorsDemotedToWarningWhenBreakGlassFlagIsSet() throws Exception {
+ setBuildLanguageOptions(
+ "--experimental_bzl_visibility=true",
+ "--experimental_bzl_visibility_allowlist=everyone",
+ "--check_bzl_visibility=false");
+
+ scratch.file("a/BUILD");
+ scratch.file(
+ "a/foo.bzl", //
+ "load(\"//b:bar.bzl\", \"x\")");
+ scratch.file("b/BUILD");
+ scratch.file(
+ "b/bar.bzl", //
+ "visibility(\"private\")",
+ "x = 1");
+
+ checkSuccessfulLookup("//a:foo.bzl");
+ assertContainsEvent("Starlark file //b:bar.bzl is not visible for loading from package //a.");
+ assertContainsEvent("Continuing because --nocheck_bzl_visibility is active");
+ }
+
  @Test
  public void testLoadFromNonExistentRepository_producesMeaningfulError() throws Exception {
  scratch.file("BUILD", "load(\"@repository//dir:file.bzl\", \"foo\")");

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

@@ -791,6 +791,27 @@ public void testBzlVisibilityViolation() throws Exception {
  assertContainsEvent("Starlark file //b:foo.bzl is not visible for loading from package //a.");
  }
 
+ @Test
+ public void testBzlVisibilityViolationDemotedToWarningWhenBreakGlassFlagIsSet() throws Exception {
+ setBuildLanguageOptions(
+ "--experimental_bzl_visibility=true",
+ "--experimental_bzl_visibility_allowlist=b",
+ "--check_bzl_visibility=false");
+
+ scratch.file(
+ "a/BUILD", //
+ "load(\"//b:foo.bzl\", \"x\")");
+ scratch.file("b/BUILD");
+ scratch.file(
+ "b/foo.bzl", //
+ "visibility(\"private\")",
+ "x = 1");
+
+ validPackageWithoutErrors(PackageValue.key(PackageIdentifier.createInMainRepo("a")));
+ assertContainsEvent("Starlark file //b:foo.bzl is not visible for loading from package //a.");
+ assertContainsEvent("Continuing because --nocheck_bzl_visibility is active");
+ }
+
  @Test
  public void testVisibilityCallableNotAvailableInBUILD() throws Exception {
  setBuildLanguageOptions(