Use any dependency while autoloading symbols externally

This changes from loading symbols as a dependency of bazel_tools to loading them as any dependency of any of the resolved modules.

In case the dependency is missing a warning is reported and Bazel only fails when the actual symbol is needed but not available.

PiperOrigin-RevId: 681347025
Change-Id: I4847a6a0420d2ae9e22d347dcd898ae9cdbdb0d6

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

@@ -22,12 +22,17 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue;
+import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.Label.RepoContext;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.skyframe.BzlLoadValue;
+import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
 import com.google.devtools.build.lib.skyframe.RepositoryMappingValue;
 import com.google.devtools.build.skyframe.SkyFunction;
 import java.util.HashSet;
@@ -83,6 +88,10 @@ public class AutoloadSymbols {
   private final boolean bzlmodEnabled;
   private final boolean autoloadsEnabled;
 
+  // Configuration of  --incompatible_load_externally
+  public static final Precomputed<AutoloadSymbols> AUTOLOAD_SYMBOLS =
+      new Precomputed<>("autoload_symbols");
+
   public AutoloadSymbols(RuleClassProvider ruleClassProvider, StarlarkSemantics semantics) {
     ImmutableList<String> symbolConfiguration =
         ImmutableList.copyOf(semantics.get(BuildLanguageOptions.INCOMPATIBLE_AUTOLOAD_EXTERNALLY));
@@ -378,23 +387,71 @@ private static Map<String, Object> convertNativeStructToMap(StarlarkInfo struct)
   @Nullable
   public ImmutableMap<String, BzlLoadValue.Key> getLoadKeys(SkyFunction.Environment env)
       throws InterruptedException {
-    RepositoryMappingValue repositoryMappingValue =
-        (RepositoryMappingValue)
-            env.getValue(RepositoryMappingValue.key(RepositoryName.BAZEL_TOOLS));
 
-    if (repositoryMappingValue == null) {
-      return null;
-    }
+    final RepoContext repoContext;
+    if (bzlmodEnabled) {
+      BazelDepGraphValue bazelDepGraphValue =
+          (BazelDepGraphValue) env.getValue(BazelDepGraphValue.KEY);
+      if (bazelDepGraphValue == null) {
+        return null;
+      }
 
-    RepoContext repoContext =
-        Label.RepoContext.of(
-            RepositoryName.BAZEL_TOOLS, repositoryMappingValue.getRepositoryMapping());
+      ImmutableMap<String, ModuleKey> highestVersions =
+          bazelDepGraphValue.getCanonicalRepoNameLookup().values().stream()
+              .collect(
+                  toImmutableMap(
+                      ModuleKey::name,
+                      moduleKey -> moduleKey,
+                      (m1, m2) -> m1.version().compareTo(m2.version()) >= 0 ? m1 : m1));
+      RepositoryMapping repositoryMapping =
+          RepositoryMapping.create(
+              highestVersions.entrySet().stream()
+                  .collect(
+                      toImmutableMap(
+                          Map.Entry::getKey,
+                          entry ->
+                              bazelDepGraphValue
+                                  .getCanonicalRepoNameLookup()
+                                  .inverse()
+                                  .get(entry.getValue()))),
+              RepositoryName.MAIN);
+      repoContext = Label.RepoContext.of(RepositoryName.MAIN, repositoryMapping);
+    } else {
+      RepositoryMappingValue repositoryMappingValue =
+          (RepositoryMappingValue) env.getValue(RepositoryMappingValue.key(RepositoryName.MAIN));
+      if (repositoryMappingValue == null) {
+        return null;
+      }
+      // Create with owner, so that we can report missing references (isVisible is false if missing)
+      repoContext =
+          Label.RepoContext.of(
+              RepositoryName.MAIN,
+              RepositoryMapping.create(
+                  repositoryMappingValue.getRepositoryMapping().entries(), RepositoryName.MAIN));
+    }
 
     // Inject loads for rules and symbols removed from Bazel
     ImmutableMap.Builder<String, BzlLoadValue.Key> loadKeysBuilder =
         ImmutableMap.builderWithExpectedSize(autoloadedSymbols.size());
+    ImmutableSet.Builder<String> missingRepositories = ImmutableSet.builder();
     for (String symbol : autoloadedSymbols) {
-      loadKeysBuilder.put(symbol, AUTOLOAD_CONFIG.get(symbol).getKey(repoContext));
+      Label label = AUTOLOAD_CONFIG.get(symbol).getLabel(repoContext);
+      // Only load if the dependency is present
+      if (label.getRepository().isVisible()) {
+        loadKeysBuilder.put(symbol, BzlLoadValue.keyForBuild(label));
+      } else {
+        missingRepositories.add(label.getRepository().getName());
+      }
+    }
+    for (String missingRepository : missingRepositories.build()) {
+      env.getListener()
+          .handle(
+              Event.warn(
+                  String.format(
+                      "Couldn't auto load rules or symbols, because no dependency on"
+                          + " module/repository '%s' found. This will result in a failure if"
+                          + " there's a reference to those rules or symbols.",
+                      missingRepository)));
     }
     return loadKeysBuilder.buildOrThrow();
   }
@@ -480,10 +537,9 @@ public abstract static class SymbolRedirect {
 
     public abstract ImmutableSet<String> getRdeps();
 
-    public BzlLoadValue.Key getKey(RepoContext bazelToolsRepoContext) throws InterruptedException {
+    Label getLabel(RepoContext repoContext) throws InterruptedException {
       try {
-        return BzlLoadValue.keyForBuild(
-            Label.parseWithRepoContext(getLoadLabel(), bazelToolsRepoContext));
+        return Label.parseWithRepoContext(getLoadLabel(), repoContext);
       } catch (LabelSyntaxException e) {
         throw new IllegalStateException(e);
       }

src/main/java/com/google/devtools/build/lib/packages/BUILD

@@ -54,6 +54,7 @@ java_library(
             "ConfiguredAttributeMapper.java",
             "LabelPrinter.java",
             "PackageSpecification.java",
+            "AutoloadSymbols.java",
         ],
     ),
     deps = [
@@ -87,7 +88,6 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/profiler",
         "//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
-        "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:starlark_builtins_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:abstract-exported-starlark-symbol-codec",
@@ -121,6 +121,27 @@ java_library(
     ],
 )
 
+java_library(
+    name = "autoload_symbols",
+    srcs = ["AutoloadSymbols.java"],
+    deps = [
+        "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common",
+        "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
+        "//src/main/java/com/google/devtools/build/lib/events",
+        "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/packages/semantics",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:bzl_load_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value",
+        "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
+        "//src/main/java/net/starlark/java/eval",
+        "//third_party:auto_value",
+        "//third_party:guava",
+        "//third_party:jsr305",
+    ],
+)
+
 java_library(
     name = "bzl_visibility",
     srcs = ["BzlVisibility.java"],

src/main/java/com/google/devtools/build/lib/skyframe/BUILD

@@ -317,6 +317,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception",
         "//src/main/java/com/google/devtools/build/lib/io:process_package_directory_exception",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/packages:autoload_symbols",
         "//src/main/java/com/google/devtools/build/lib/packages:bzl_visibility",
         "//src/main/java/com/google/devtools/build/lib/packages:globber",
         "//src/main/java/com/google/devtools/build/lib/packages:globber_utils",
@@ -796,6 +797,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/packages:autoload_symbols",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
         "//src/main/java/net/starlark/java/eval",

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

@@ -161,7 +161,7 @@ static BzlCompileValue computeInline(
 
       // Takes into account --incompatible_autoload_externally, similarly to the comment above, this
       // only defines the correct set of symbols, but does not load them yet.
-      AutoloadSymbols autoloadSymbols = PrecomputedValue.AUTOLOAD_SYMBOLS.get(env);
+      AutoloadSymbols autoloadSymbols = AutoloadSymbols.AUTOLOAD_SYMBOLS.get(env);
       if (autoloadSymbols == null) {
         return null;
       }

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

@@ -610,7 +610,7 @@ private StarlarkBuiltinsValue getBuiltins(
       }
       return StarlarkBuiltinsValue.createEmpty(starlarkSemantics);
     }
-    AutoloadSymbols autoloadSymbols = PrecomputedValue.AUTOLOAD_SYMBOLS.get(env);
+    AutoloadSymbols autoloadSymbols = AutoloadSymbols.AUTOLOAD_SYMBOLS.get(env);
     if (autoloadSymbols == null) {
       return null;
     }

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

@@ -462,7 +462,7 @@ public SkyValue compute(SkyKey key, Environment env)
     StarlarkBuiltinsValue starlarkBuiltinsValue;
     try {
       // Bazel: we do autoloads for all BUILD files if enabled
-      AutoloadSymbols autoloadSymbols = PrecomputedValue.AUTOLOAD_SYMBOLS.get(env);
+      AutoloadSymbols autoloadSymbols = AutoloadSymbols.AUTOLOAD_SYMBOLS.get(env);
       if (autoloadSymbols == null) {
         return null;
       }

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

@@ -19,7 +19,6 @@
 import com.google.common.base.Supplier;
 import com.google.common.base.Suppliers;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
-import com.google.devtools.build.lib.packages.AutoloadSymbols;
 import com.google.devtools.build.lib.packages.Package.ConfigSettingVisibilityPolicy;
 import com.google.devtools.build.lib.packages.RuleVisibility;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
@@ -85,10 +84,6 @@ public static <T> Injected injected(Precomputed<T> precomputed, T value) {
   public static final Precomputed<StarlarkSemantics> STARLARK_SEMANTICS =
       new Precomputed<>("starlark_semantics");
 
-  // Configuration of  --incompatible_load_externally
-  public static final Precomputed<AutoloadSymbols> AUTOLOAD_SYMBOLS =
-      new Precomputed<>("autoload_symbols");
-
   public static final Precomputed<UUID> BUILD_ID =
       new Precomputed<>("build_id", /* shareable= */ false);
 

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

@@ -1351,7 +1351,7 @@ private void setStarlarkSemantics(StarlarkSemantics starlarkSemantics) {
   }
 
   private void setAutoloadsConfiguration(AutoloadSymbols autoloadSymbols) {
-    PrecomputedValue.AUTOLOAD_SYMBOLS.set(injectable(), autoloadSymbols);
+    AutoloadSymbols.AUTOLOAD_SYMBOLS.set(injectable(), autoloadSymbols);
   }
 
   public void setBaselineConfiguration(BuildOptions buildOptions) {

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

@@ -185,7 +185,7 @@ private static StarlarkBuiltinsValue computeInternal(
     if (starlarkSemantics.get(BuildLanguageOptions.EXPERIMENTAL_BUILTINS_BZL_PATH).isEmpty()) {
       return StarlarkBuiltinsValue.createEmpty(starlarkSemantics);
     }
-    AutoloadSymbols autoloadSymbols = PrecomputedValue.AUTOLOAD_SYMBOLS.get(env);
+    AutoloadSymbols autoloadSymbols = AutoloadSymbols.AUTOLOAD_SYMBOLS.get(env);
     if (autoloadSymbols == null) {
       return null;
     }
@@ -251,7 +251,6 @@ private static StarlarkBuiltinsValue computeInternal(
       }
       autoBzlLoadValues = autoBzlLoadValuesBuilder.buildOrThrow();
     } catch (BzlLoadFailedException ex) {
-
       throw BuiltinsFailedException.errorEvaluatingAutoloadedBzls(
           ex, starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD));
     }
@@ -368,7 +367,7 @@ static BuiltinsFailedException errorEvaluatingAutoloadedBzls(
       String additionalMessage =
           bzlmodEnabled
               ? ""
-              : ". Most likely you need to upgrade the version of rules repository in the"
+              : " Most likely you need to upgrade the version of rules repository in the"
                   + " WORKSPACE file.";
       return new BuiltinsFailedException(
           String.format(

src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java

@@ -351,7 +351,7 @@ public void inject(SkyKey key, Delta delta) {
     PrecomputedValue.CONFIG_SETTING_VISIBILITY_POLICY.set(
         injectable, ConfigSettingVisibilityPolicy.LEGACY_OFF);
     PrecomputedValue.STARLARK_SEMANTICS.set(injectable, starlarkSemantics);
-    PrecomputedValue.AUTOLOAD_SYMBOLS.set(
+    AutoloadSymbols.AUTOLOAD_SYMBOLS.set(
         injectable, new AutoloadSymbols(ruleClassProvider, starlarkSemantics));
     return new ImmutableDiff(ImmutableList.of(), valuesToInject);
   }

src/main/java/com/google/devtools/build/lib/skyframe/packages/BUILD

@@ -48,6 +48,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/io:file_symlink_cycle_uniqueness_function",
         "//src/main/java/com/google/devtools/build/lib/io:file_symlink_infinite_expansion_uniqueness_function",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/packages:autoload_symbols",
         "//src/main/java/com/google/devtools/build/lib/pkgcache",
         "//src/main/java/com/google/devtools/build/lib/repository:external_package_helper",
         "//src/main/java/com/google/devtools/build/lib/skyframe:bzl_compile",

src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD

@@ -51,6 +51,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/packages:autoload_symbols",
         "//src/main/java/com/google/devtools/build/lib/packages/semantics",
         "//src/main/java/com/google/devtools/build/lib/pkgcache",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/local_repository_rule",

src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java

@@ -268,7 +268,7 @@ public void setup() throws Exception {
             .set(BuildLanguageOptions.INCOMPATIBLE_AUTOLOAD_EXTERNALLY, ImmutableList.of())
             .build();
     PrecomputedValue.STARLARK_SEMANTICS.set(differencer, semantics);
-    PrecomputedValue.AUTOLOAD_SYMBOLS.set(
+    AutoloadSymbols.AUTOLOAD_SYMBOLS.set(
         differencer, new AutoloadSymbols(ruleClassProvider, semantics));
     RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(differencer, ImmutableMap.of());
     RepositoryDelegatorFunction.FORCE_FETCH.set(

src/test/java/com/google/devtools/build/lib/rules/repository/BUILD

@@ -30,6 +30,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
+        "//src/main/java/com/google/devtools/build/lib/packages:autoload_symbols",
         "//src/main/java/com/google/devtools/build/lib/packages/semantics",
         "//src/main/java/com/google/devtools/build/lib/pkgcache",
         "//src/main/java/com/google/devtools/build/lib/repository:external_package_helper",

src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java

@@ -279,7 +279,7 @@ public void setupDelegator() throws Exception {
     StarlarkSemantics semantics =
         StarlarkSemantics.builder().setBool(BuildLanguageOptions.ENABLE_BZLMOD, true).build();
     PrecomputedValue.STARLARK_SEMANTICS.set(differencer, semantics);
-    PrecomputedValue.AUTOLOAD_SYMBOLS.set(
+    AutoloadSymbols.AUTOLOAD_SYMBOLS.set(
         differencer, new AutoloadSymbols(ruleClassProvider, semantics));
     RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE.set(
         differencer, Optional.empty());

src/test/shell/integration/load_removed_symbols_test.sh

@@ -79,7 +79,33 @@ local_path_override(
 EOF
 }
 
-function disabled_test_removed_rule_loaded() {
+function test_missing_necessary_bzlmod_dep() {
+  # Intentionally not adding rules_android to MODULE.bazel
+  cat > BUILD << EOF
+aar_import(
+    name = 'aar',
+    aar = 'aar.file',
+    deps = [],
+)
+EOF
+  bazel build --incompatible_autoload_externally=aar_import :aar >&$TEST_log 2>&1 && fail "build unexpectedly succeeded"
+  expect_log "name 'aar_import' is not defined"
+  expect_log "WARNING: Couldn't auto load rules or symbols, because no dependency on module/repository 'rules_android' found. This will result in a failure if there's a reference to those rules or symbols."
+}
+
+function test_missing_unnecessary_bzmod_dep() {
+  # Intentionally not adding rules_android to MODULE.bazel
+  cat > BUILD << EOF
+filegroup(
+    name = 'filegroup',
+    srcs = [],
+)
+EOF
+  bazel build --incompatible_autoload_externally=aar_import :filegroup >&$TEST_log 2>&1 || fail "build failed"
+  expect_log "WARNING: Couldn't auto load rules or symbols, because no dependency on module/repository 'rules_android' found. This will result in a failure if there's a reference to those rules or symbols."
+}
+
+function test_removed_rule_loaded() {
   setup_module_dot_bazel
   mock_rules_android
 
@@ -94,7 +120,7 @@ EOF
   # TODO(b/355260271): add test with workspace enabled
 }
 
-function disabled_test_removed_rule_loaded_from_bzl() {
+function test_removed_rule_loaded_from_bzl() {
   setup_module_dot_bazel
   mock_rules_android
 
@@ -385,7 +411,7 @@ EOF
   expect_log "Duplicated symbol 'py_library' in --incompatible_autoload_externally"
 }
 
-function disabled_test_missing_symbol_error() {
+function test_missing_symbol_error() {
   setup_module_dot_bazel
   mock_rules_android
   rules_android_workspace="${TEST_TMPDIR}/rules_android_workspace"
@@ -404,7 +430,7 @@ EOF
   expect_log "Failed to apply symbols loaded externally: The toplevel symbol 'aar_import' set by --incompatible_load_symbols_externally couldn't be loaded. 'aar_import' not found in auto loaded '@rules_android//rules:rules.bzl'."
 }
 
-function disabled_test_missing_bzlfile_error() {
+function test_missing_bzlfile_error() {
   setup_module_dot_bazel
   mock_rules_android
   rules_android_workspace="${TEST_TMPDIR}/rules_android_workspace"