commit 8c11405339c2ca395bf30e04658b0bacae7d4fee
author salma-samy <salmasamy@google.com> Tue May 16 13:32:46 2023 -0700
committer Copybara-Service <copybara-worker@google.com> Tue May 16 13:34:42 2023 -0700
tree 3424b42f8c1db69308e41570d9d89ea3a2ffe1a7
parent 280de20949b1b852f789a16acf6899fc41a20d52

Stop checking .bzl files visibility in RepoRuleFunction
Fixes https://github.com/bazelbuild/bazel/issues/18346

At this point we are creating a rule from an extension repospec and that is always public

PiperOrigin-RevId: 532558852
Change-Id: I245de0a23f92ba6283be4bf3be6af4863b7a8c68

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

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java
index d097df2..8b36f67 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java
@@ -240,8 +240,7 @@
 
     // Load the .bzl module.
     try {
-      // TODO(b/22193153, wyv): Determine whether .bzl load visibility should apply at all to this
-      // type of .bzl load. As it stands, this call checks that bzlFile is visible to package @//.
+      // No need to check visibility for an extension repospec that is always public
       return PackageFunction.loadBzlModules(
           env,
           PackageIdentifier.EMPTY_PACKAGE_ID,
@@ -249,7 +248,8 @@
           programLoads,
           keys,
           starlarkSemantics,
-          null);
+          null,
+          /* checkVisibility= */ false);
     } catch (NoSuchPackageException e) {
       throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
     }

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

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index 26a223f..ae8ba7b 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -634,6 +634,7 @@
    * construction to the caller, so that loadPrelude can become just a call to the factored-out
    * code.
    */
+  // TODO(18422): Cleanup/refactor this method's signature.
   @Nullable
   static ImmutableMap<String, Module> loadBzlModules(
       Environment env,
@@ -642,7 +643,8 @@
       List<Pair<String, Location>> programLoads,
       List<BzlLoadValue.Key> keys,
       StarlarkSemantics semantics,
-      @Nullable BzlLoadFunction bzlLoadFunctionForInlining)
+      @Nullable BzlLoadFunction bzlLoadFunctionForInlining,
+      boolean checkVisibility)
       throws NoSuchPackageException, InterruptedException {
     List<BzlLoadValue> bzlLoads;
     try {
@@ -655,14 +657,17 @@
       }
       // Validate that the current BUILD/WORKSPACE file satisfies each loaded dependency's
       // load visibility.
-      BzlLoadFunction.checkLoadVisibilities(
-          packageId,
-          requestingFileDescription,
-          bzlLoads,
-          keys,
-          programLoads,
-          /*demoteErrorsToWarnings=*/ !semantics.getBool(BuildLanguageOptions.CHECK_BZL_VISIBILITY),
-          env.getListener());
+      if (checkVisibility) {
+        BzlLoadFunction.checkLoadVisibilities(
+            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()
@@ -1335,7 +1340,8 @@
                   programLoads,
                   keys.build(),
                   starlarkBuiltinsValue.starlarkSemantics,
-                  bzlLoadFunctionForInlining);
+                  bzlLoadFunctionForInlining,
+                  /* checkVisibility= */ true);
         } catch (NoSuchPackageException e) {
           throw new PackageFunctionException(e, Transience.PERSISTENT);
         }

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

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
index 7eabc01..26ddf3d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/WorkspaceFileFunction.java
@@ -334,7 +334,8 @@
               programLoads,
               keys.build(),
               starlarkSemantics,
-              bzlLoadFunctionForInlining);
+              bzlLoadFunctionForInlining,
+              /* checkVisibility= */ true);
     } catch (NoSuchPackageException e) {
       throw new WorkspaceFileFunctionException(e, Transience.PERSISTENT);
     }