From f9ed0b03269ee5364f01e1d177129e4597ba0a48 Mon Sep 17 00:00:00 2001 From: Ivo List Date: Fri, 23 Aug 2024 02:37:47 -0700 Subject: [PATCH] Implement incompatible_autoload_externally Issue: https://github.com/bazelbuild/bazel/issues/22928 Incompatible flag issue: https://github.com/bazelbuild/bazel/issues/23043 The flag supports Bazel users in migrating the rules from being embedded in Bazel to external repositories. Listing a symbol or a rule in the flag automatically adds a load to the respective repository. Listing it with a '+' keeps the symbol available in the rules_repository. Listing a symbol with "-" forcefully removes it from Bazel. Cycles are prevented with a list of repositories where autoloads should not be used. The new environments are injected into BzlCompile, BzlLoad and Package function. StarlarkBuiltinsFunction with autoloads true key is used to load the external symbols. Closes #23016. Downstream test: https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/4003 PiperOrigin-RevId: 666710259 Change-Id: Idaa9b6b13c9a474f700e69e22b6162ed59b7fab0 --- src/MODULE.tools | 13 +- .../build/lib/packages/AutoloadSymbols.java | 701 ++++++++++++++++++ .../google/devtools/build/lib/packages/BUILD | 1 + .../semantics/BuildLanguageOptions.java | 59 ++ .../google/devtools/build/lib/skyframe/BUILD | 1 + .../lib/skyframe/BzlCompileFunction.java | 10 +- .../build/lib/skyframe/BzlLoadFunction.java | 28 +- .../lib/skyframe/BzlmodRepoCycleReporter.java | 25 +- .../build/lib/skyframe/PackageFunction.java | 12 +- .../build/lib/skyframe/PrecomputedValue.java | 5 + .../build/lib/skyframe/SkyframeExecutor.java | 6 + .../skyframe/StarlarkBuiltinsFunction.java | 119 ++- .../lib/skyframe/StarlarkBuiltinsValue.java | 30 +- .../packages/AbstractPackageLoader.java | 6 + .../bzlmod/ModuleExtensionResolutionTest.java | 9 +- .../repository/RepositoryDelegatorTest.java | 9 +- src/test/shell/integration/BUILD | 8 + .../integration/load_removed_symbols_test.sh | 405 ++++++++++ 18 files changed, 1413 insertions(+), 34 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/packages/AutoloadSymbols.java create mode 100755 src/test/shell/integration/load_removed_symbols_test.sh diff --git a/src/MODULE.tools b/src/MODULE.tools index e6e564d5f75ea5..fda903cb21881d 100644 --- a/src/MODULE.tools +++ b/src/MODULE.tools @@ -4,15 +4,11 @@ module(name = "bazel_tools") -bazel_dep(name = "rules_cc", version = "0.0.9") -bazel_dep(name = "rules_java", version = "7.9.0") bazel_dep(name = "rules_license", version = "0.0.3") bazel_dep(name = "rules_proto", version = "4.0.0") -bazel_dep(name = "rules_python", version = "0.22.1") bazel_dep(name = "buildozer", version = "7.1.2") bazel_dep(name = "platforms", version = "0.0.9") -bazel_dep(name = "protobuf", version = "3.19.6", repo_name = "com_google_protobuf") bazel_dep(name = "zlib", version = "1.3.1.bcr.3") cc_configure = use_extension("//tools/cpp:cc_configure.bzl", "cc_configure_extension") @@ -49,3 +45,12 @@ use_repo(android_sdk_proxy_extensions, "android_external") # Used by bazel mod tidy (see BazelModTidyFunction). buildozer_binary = use_extension("@buildozer//:buildozer_binary.bzl", "buildozer_binary") use_repo(buildozer_binary, "buildozer_binary") + +# Dependencies used to auto-load removed symbols and rules from Bazel (due to Starlarkification) +# See also: --incompatible_autoload_externally, AutoloadSymbols +bazel_dep(name = "protobuf", version = "3.19.6", repo_name = "com_google_protobuf") +bazel_dep(name = "rules_java", version = "7.9.0") +bazel_dep(name = "rules_cc", version = "0.0.9") +bazel_dep(name = "rules_python", version = "0.22.1") +# add rules_android +# add apple_support diff --git a/src/main/java/com/google/devtools/build/lib/packages/AutoloadSymbols.java b/src/main/java/com/google/devtools/build/lib/packages/AutoloadSymbols.java new file mode 100644 index 00000000000000..f15af962f86ec4 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/packages/AutoloadSymbols.java @@ -0,0 +1,701 @@ +// Copyright 2024 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +package com.google.devtools.build.lib.packages; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableMap.toImmutableMap; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +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.RepositoryName; +import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; +import com.google.devtools.build.lib.skyframe.BzlLoadValue; +import com.google.devtools.build.lib.skyframe.RepositoryMappingValue; +import com.google.devtools.build.skyframe.SkyFunction; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.function.Predicate; +import javax.annotation.Nullable; +import net.starlark.java.eval.Starlark; +import net.starlark.java.eval.StarlarkSemantics; + +/** + * Implementation of --incompatible_autoload_externally. + * + *

The flag adds loads to external repository for rules and top-level symbols, or removes them. + * This class prepares new environment for BzlCompileFunction, BzlLoadFunction and PackageFuntions. + * + *

Environment for BzlCompileFunction is prepared immediately during construction. Only names of + * the symbols are needed and provided. Values of the symbols are set to None. + * + *

Environments for BzlLoadFunction and PackageFunctions are prepared in StarlarkBuilinsFunction. + * + *

Cycles are prevented by disallowing autoloads in repos that we autoload from and the repos + * they depend on. + */ +public class AutoloadSymbols { + // Following fields autoloadedSymbols, removedSymbols, partiallyRemovedSymbols, + // reposDisallowingAutoloads are empty if autoloads aren't used + // Symbols that aren't prefixed with '-', that get loaded from external repositories (in allowed + // repositories). + private final ImmutableList autoloadedSymbols; + // Symbols that are prefixed with '-', that are removed everywhere (from each repository) + private final ImmutableList removedSymbols; + // Symbols that aren't prefixed with '+', that are removed from everywhere, + // except in repositories where autoloaded symbols are defined + private final ImmutableList partiallyRemovedSymbols; + + // Repositories where autoloads shouldn't be used + private final ImmutableSet reposDisallowingAutoloads; + + // The environment formed by taking BazelStarlarkEnvironment's bzl environment and adding/removing + // autoloaded symbols. The values of any added symbols are set to None (i.e. not actually loaded). + // This is intended for BzlCompileFunction only. + private final ImmutableMap uninjectedBuildBzlEnvWithAutoloads; + + // bzl environment where autoloads aren't used, uninjected (not loaded yet) + private final ImmutableMap uninjectedBuildBzlEnvWithoutAutoloads; + + // Used for nicer error messages + private final boolean bzlmodEnabled; + private final boolean autoloadsEnabled; + + public AutoloadSymbols(RuleClassProvider ruleClassProvider, StarlarkSemantics semantics) { + ImmutableList symbolConfiguration = + ImmutableList.copyOf(semantics.get(BuildLanguageOptions.INCOMPATIBLE_AUTOLOAD_EXTERNALLY)); + this.bzlmodEnabled = semantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD); + this.autoloadsEnabled = !symbolConfiguration.isEmpty(); + + if (!autoloadsEnabled) { + ImmutableMap originalBuildBzlEnv = + ruleClassProvider.getBazelStarlarkEnvironment().getUninjectedBuildBzlEnv(); + this.uninjectedBuildBzlEnvWithAutoloads = originalBuildBzlEnv; + this.uninjectedBuildBzlEnvWithoutAutoloads = originalBuildBzlEnv; + this.reposDisallowingAutoloads = ImmutableSet.of(); + this.autoloadedSymbols = ImmutableList.of(); + this.removedSymbols = ImmutableList.of(); + this.partiallyRemovedSymbols = ImmutableList.of(); + return; + } + + // Validates the inputs + Set uniqueSymbols = new HashSet<>(); + for (String symbol : symbolConfiguration) { + String symbolWithoutPrefix = + symbol.startsWith("+") || symbol.startsWith("-") ? symbol.substring(1) : symbol; + if (!uniqueSymbols.add(symbolWithoutPrefix)) { + throw new IllegalStateException( + String.format( + "Duplicated symbol '%s' in --incompatible_autoload_externally", + symbolWithoutPrefix)); + } + if (!AUTOLOAD_CONFIG.containsKey(symbolWithoutPrefix)) { + throw new IllegalStateException("Undefined symbol in --incompatible_autoload_externally"); + } + } + + this.autoloadedSymbols = filterSymbols(symbolConfiguration, symbol -> !symbol.startsWith("-")); + this.removedSymbols = filterSymbols(symbolConfiguration, symbol -> symbol.startsWith("-")); + this.partiallyRemovedSymbols = + filterSymbols(symbolConfiguration, symbol -> !symbol.startsWith("+")); + + this.reposDisallowingAutoloads = + ImmutableSet.builder() + .addAll(PREDECLARED_REPOS_DISALLOWING_AUTOLOADS) + .addAll(semantics.get(BuildLanguageOptions.REPOSITORIES_WITHOUT_AUTOLOAD)) + .build(); + + ImmutableMap originalBuildBzlEnv = + ruleClassProvider.getBazelStarlarkEnvironment().getUninjectedBuildBzlEnv(); + + // Sets up environments for BzlCompile function + this.uninjectedBuildBzlEnvWithAutoloads = + modifyBuildBzlEnv( + /* isWithAutoloads= */ true, + originalBuildBzlEnv, + /* newSymbols= */ autoloadedSymbols.stream() + .collect(toImmutableMap(key -> key, key -> Starlark.NONE))); + this.uninjectedBuildBzlEnvWithoutAutoloads = + modifyBuildBzlEnv( + /* isWithAutoloads= */ false, originalBuildBzlEnv, /* newSymbols= */ ImmutableMap.of()); + + // Validate rdeps - this ensures that all the rules using a provider are also removed + // Check what's still available in Bazel (some symbols might already be deleted) + ImmutableSet allAvailableSymbols = + ImmutableSet.builder() + .addAll(uninjectedBuildBzlEnvWithoutAutoloads.keySet()) + .addAll( + convertNativeStructToMap( + (StarlarkInfo) uninjectedBuildBzlEnvWithoutAutoloads.get("native")) + .keySet()) + .build(); + for (String symbol : partiallyRemovedSymbols) { + ImmutableList unsatisfiedRdeps = + AUTOLOAD_CONFIG.get(symbol).getRdeps().stream() + .filter(allAvailableSymbols::contains) + .collect(toImmutableList()); + if (!unsatisfiedRdeps.isEmpty()) { + throw new IllegalStateException( + String.format( + "Symbol in '%s' can't be removed, because it's still used by: %s", + symbol, String.join(", ", unsatisfiedRdeps))); + } + } + } + + /** An optimisation, checking is autoloads are used at all. */ + public boolean isEnabled() { + return autoloadsEnabled; + } + + /** Returns the environment for BzlCompile function */ + public ImmutableMap getUninjectedBuildBzlEnv(@Nullable Label key) { + return autoloadsDisabledForRepo(key) + ? uninjectedBuildBzlEnvWithoutAutoloads + : uninjectedBuildBzlEnvWithAutoloads; + } + + /** Check if autoloads shouldn't be used. */ + public boolean autoloadsDisabledForRepo(@Nullable Label key) { + if (!autoloadsEnabled) { + return true; + } + return key == null || autoloadsDisabledForRepo(key.getRepository().getName()); + } + + /** + * Check if autoloads shouldn't be used in the given repository. + * + *

Autoloads aren't used for repos in {@link #PREDECLARED_REPOS_DISALLOWING_AUTOLOADS}, + * specified by the --repositories_without_autoloads flag or any of their immediate descendants + * (parsing the cannonical repository name to check this). + */ + public boolean autoloadsDisabledForRepo(String repo) { + if (!autoloadsEnabled) { + return true; + } + int separatorIndex = repo.contains("~") ? repo.indexOf("~") : repo.indexOf("+"); + return reposDisallowingAutoloads.contains( + separatorIndex >= 0 ? repo.substring(0, separatorIndex) : repo); + } + + /** + * Modifies the environment for BzlLoad function (returned from StarlarkBuiltinsFunction). + * + *

{@code originalEnv} contains original environment and {@code newSymbols} is a map from new + * symbol name to symbol's value. {@code isWithAutoloads} chooses the semantics, described in + * details on --incompatible_autoload_externally flag. + */ + public ImmutableMap modifyBuildBzlEnv( + boolean isWithAutoloads, + ImmutableMap originalEnv, + ImmutableMap newSymbols) { + if (isWithAutoloads) { + return modifyBuildBzlEnv(originalEnv, /* add= */ newSymbols, /* remove= */ removedSymbols); + } else { + return modifyBuildBzlEnv( + originalEnv, /* add= */ ImmutableMap.of(), /* remove= */ partiallyRemovedSymbols); + } + } + + /** + * Creates modified environment that's used in BzlCompileFunction and StarlarkBuiltinsFunction. + * + *

It starts with the original environment. Adds the symbols to it or removes them. + */ + private ImmutableMap modifyBuildBzlEnv( + ImmutableMap originalEnv, + ImmutableMap add, + ImmutableList remove) { + Map envBuilder = new LinkedHashMap<>(originalEnv); + Map nativeBindings = + convertNativeStructToMap((StarlarkInfo) envBuilder.remove("native")); + + for (Map.Entry symbol : add.entrySet()) { + if (AUTOLOAD_CONFIG.get(symbol.getKey()).isRule()) { + nativeBindings.put(symbol.getKey(), symbol.getValue()); + } else { + envBuilder.put(symbol.getKey(), symbol.getValue()); + } + } + for (String symbol : remove) { + if (AUTOLOAD_CONFIG.get(symbol).isRule()) { + nativeBindings.remove(symbol); + } else { + envBuilder.remove(symbol); + } + } + envBuilder.put( + "native", StructProvider.STRUCT.create(nativeBindings, "no native function or rule '%s'")); + return ImmutableMap.copyOf(envBuilder); + } + + /** Modifies the environment for Package function (returned from StarlarkBuiltinsFunction). */ + public ImmutableMap modifyBuildEnv( + boolean isWithAutoloads, + ImmutableMap originalEnv, + ImmutableMap newSymbols) { + final ImmutableMap add; + if (isWithAutoloads) { + add = newSymbols; + } else { + add = ImmutableMap.of(); + } + Map envBuilder = new LinkedHashMap<>(originalEnv); + for (Map.Entry symbol : add.entrySet()) { + if (AUTOLOAD_CONFIG.get(symbol.getKey()).isRule()) { + envBuilder.put(symbol.getKey(), symbol.getValue()); + } + } + for (String symbol : removedSymbols) { + if (AUTOLOAD_CONFIG.get(symbol).isRule()) { + envBuilder.remove(symbol); + } + } + return ImmutableMap.copyOf(envBuilder); + } + + private static ImmutableList filterSymbols( + ImmutableList symbols, Predicate when) { + return symbols.stream() + .filter(when) + .map( + symbol -> + symbol.startsWith("+") || symbol.startsWith("-") ? symbol.substring(1) : symbol) + .collect(toImmutableList()); + } + + private static Map convertNativeStructToMap(StarlarkInfo struct) { + LinkedHashMap destr = new LinkedHashMap<>(); + for (String field : struct.getFieldNames()) { + destr.put(field, struct.getValue(field)); + } + return destr; + } + + /** + * Returns a list of all the extra .bzl files that need to be loaded + * + *

Keys are coming from {@link AUTOLOAD_CONFIG} table. + * + *

Actual loading is done in {@link StarlarkBuiltinsValue} and then passed to {@link + * #processLoads} for final processing. The parameter {@code autoloadValues} must correspond to + * the map returned by * {@link #getLoadKeys}. + */ + @Nullable + public ImmutableMap getLoadKeys(SkyFunction.Environment env) + throws InterruptedException { + RepositoryMappingValue repositoryMappingValue = + (RepositoryMappingValue) + env.getValue(RepositoryMappingValue.key(RepositoryName.BAZEL_TOOLS)); + + if (repositoryMappingValue == null) { + return null; + } + + RepoContext repoContext = + Label.RepoContext.of( + RepositoryName.BAZEL_TOOLS, repositoryMappingValue.getRepositoryMapping()); + + // Inject loads for rules and symbols removed from Bazel + ImmutableMap.Builder loadKeysBuilder = + ImmutableMap.builderWithExpectedSize(autoloadedSymbols.size()); + for (String symbol : autoloadedSymbols) { + loadKeysBuilder.put(symbol, AUTOLOAD_CONFIG.get(symbol).getKey(repoContext)); + } + return loadKeysBuilder.buildOrThrow(); + } + + /** + * Processes LoadedValues into a map of symbols + * + *

The parameter {@code autoloadValues} must correspond to the map returned by {@link + * #getLoadKeys}. Actual loading is done in {@link StarlarkBuiltinsValue}. + * + *

Keys are coming from {@link AUTOLOAD_CONFIG} table. + */ + public ImmutableMap processLoads( + ImmutableMap autoloadValues) throws AutoloadException { + if (autoloadValues.isEmpty()) { + return ImmutableMap.of(); + } + + ImmutableMap.Builder newSymbols = + ImmutableMap.builderWithExpectedSize(autoloadValues.size()); + String workspaceWarning = + bzlmodEnabled + ? "" + : " Most likely you need to upgrade the version of rules repository in the" + + " WORKSPACE file."; + for (Map.Entry autoload : autoloadValues.entrySet()) { + String symbol = autoload.getKey(); + // Check if the symbol is named differently in the bzl file than natively. Renames are rare: + // Example is renaming native.ProguardSpecProvider to ProguardSpecInfo. + String newName = AUTOLOAD_CONFIG.get(symbol).getNewName(); + if (newName == null) { + newName = symbol; + } + BzlLoadValue v = autoload.getValue(); + Object symbolValue = v.getModule().getGlobal(newName); + if (symbolValue == null) { + throw new AutoloadException( + String.format( + "The toplevel symbol '%s' set by --incompatible_load_symbols_externally couldn't" + + " be loaded. '%s' not found in auto loaded '%s'.%s", + symbol, newName, AUTOLOAD_CONFIG.get(symbol).getLoadLabel(), workspaceWarning)); + } + newSymbols.put(symbol, symbolValue); // Exposed as old name + } + return newSymbols.buildOrThrow(); + } + + @Override + public final int hashCode() { + // These fields are used to generate all other private fields. + // Thus, other fields don't need to be included in hash code. + return Objects.hash( + autoloadedSymbols, removedSymbols, partiallyRemovedSymbols, reposDisallowingAutoloads); + } + + @Override + public final boolean equals(Object that) { + if (this == that) { + return true; + } + if (that instanceof AutoloadSymbols) { + AutoloadSymbols other = (AutoloadSymbols) that; + // These fields are used to generate all other private fields. + // Thus, other fields don't need to be included in comparison. + return this.autoloadedSymbols.equals(other.autoloadedSymbols) + && this.removedSymbols.equals(other.removedSymbols) + && this.partiallyRemovedSymbols.equals(other.partiallyRemovedSymbols) + && this.reposDisallowingAutoloads.equals(other.reposDisallowingAutoloads); + } + return false; + } + + /** Configuration of a symbol */ + @AutoValue + public abstract static class SymbolRedirect { + + public abstract String getLoadLabel(); + + public abstract boolean isRule(); + + @Nullable + public abstract String getNewName(); + + public abstract ImmutableSet getRdeps(); + + public BzlLoadValue.Key getKey(RepoContext bazelToolsRepoContext) throws InterruptedException { + try { + return BzlLoadValue.keyForBuild( + Label.parseWithRepoContext(getLoadLabel(), bazelToolsRepoContext)); + } catch (LabelSyntaxException e) { + throw new IllegalStateException(e); + } + } + } + + /** Indicates a problem performing automatic loads. */ + public static final class AutoloadException extends Exception { + + AutoloadException(String message) { + super(message); + } + } + + private static SymbolRedirect ruleRedirect(String label) { + return new AutoValue_AutoloadSymbols_SymbolRedirect(label, true, null, ImmutableSet.of()); + } + + private static SymbolRedirect symbolRedirect(String label, String... rdeps) { + return new AutoValue_AutoloadSymbols_SymbolRedirect( + label, false, null, ImmutableSet.copyOf(rdeps)); + } + + private static SymbolRedirect renamedSymbolRedirect( + String label, String newName, String... rdeps) { + return new AutoValue_AutoloadSymbols_SymbolRedirect( + label, false, newName, ImmutableSet.copyOf(rdeps)); + } + + private static final String[] androidRules = { + "aar_import", "android_binary", "android_library", "android_local_test", "android_sdk" + }; + + private static final ImmutableSet PREDECLARED_REPOS_DISALLOWING_AUTOLOADS = + ImmutableSet.of( + "protobuf", + "com_google_protobuf", + "rules_android", + "rules_cc", + "rules_java", + "rules_java_builtin", + "rules_python", + "rules_python_internal", + "rules_sh", + "apple_common", + "bazel_skylib", + "bazel_tools"); + + private static final ImmutableMap AUTOLOAD_CONFIG = + ImmutableMap.builder() + .put( + "CcSharedLibraryInfo", + symbolRedirect( + "@rules_cc//cc/common:cc_shared_library_info.bzl", "cc_shared_library")) + .put( + "CcSharedLibraryHintInfo", + symbolRedirect("@rules_cc//cc/common:cc_shared_library_hint_info.bzl", "cc_common")) + .put( + "cc_proto_aspect", + symbolRedirect("@protobuf//bazel/common:cc_proto_library.bzl", "cc_proto_library")) + .put( + "ProtoInfo", + symbolRedirect( + "@protobuf//bazel/common:proto_info.bzl", + "proto_library", + "cc_proto_library", + "cc_shared_library", + "java_lite_proto_library", + "java_proto_library", + "proto_lang_toolchain", + "java_binary", + "py_extension", + "proto_common_do_not_use")) + .put( + "proto_common_do_not_use", symbolRedirect("@protobuf//bazel/common:proto_common.bzl")) + .put("cc_common", symbolRedirect("@rules_cc//cc/common:cc_common.bzl")) + .put( + "CcInfo", + symbolRedirect( + "@rules_cc//cc/common:cc_info.bzl", + "cc_binary", + "cc_library", + "cc_test", + "cc_shared_library", + "cc_common", + "java_library", + "cc_proto_library", + "java_import", + "java_runtime", + "java_binary", + "objc_library", + "java_common", + "JavaInfo", + "py_extension", + "cc_import", + "objc_import", + "objc_library", + "cc_toolchain", + "PyCcLinkParamsProvider", + "py_library")) + .put( + "DebugPackageInfo", + symbolRedirect("@rules_cc//cc/common:debug_package_info.bzl", "cc_binary", "cc_test")) + .put( + "CcToolchainConfigInfo", + symbolRedirect( + "@rules_cc//cc/toolchains:cc_toolchain_config_info.bzl", "cc_toolchain")) + .put("java_common", symbolRedirect("@rules_java//java/common:java_common.bzl")) + .put( + "JavaInfo", + symbolRedirect( + "@rules_java//java/common:java_info.bzl", + "java_binary", + "java_library", + "java_test", + "java_proto_library", + "java_lite_proto_library", + "java_plugin", + "java_import", + "java_common")) + .put( + "JavaPluginInfo", + symbolRedirect( + "@rules_java//java/common:java_plugin_info.bzl", + "java_plugin", + "java_library", + "java_binary", + "java_test")) + .put( + "ProguardSpecProvider", + renamedSymbolRedirect( + "@rules_java//java/common:proguard_spec_info.bzl", + "ProguardSpecInfo", + "java_lite_proto_library", + "java_import", + "android_binary", + "android_library")) + .put("android_common", symbolRedirect("@rules_android//rules:common.bzl")) + .put( + "AndroidIdeInfo", symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put("ApkInfo", symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidInstrumentationInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidResourcesInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidNativeLibsInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidApplicationResourceInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidBinaryNativeLibsInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidSdkInfo", symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidManifestInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidAssetsInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidLibraryAarInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidProguardInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidIdlInfo", symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidPreDexJarInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidCcLinkParamsInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "DataBindingV2Info", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidLibraryResourceClassJarProvider", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidFeatureFlagSet", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "ProguardMappingInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidBinaryData", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "BaselineProfileProvider", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidNeverLinkLibrariesProvider", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidOptimizedJarInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidDexInfo", symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "AndroidOptimizationInfo", + symbolRedirect("@rules_android//rules:providers.bzl", androidRules)) + .put( + "PyInfo", + symbolRedirect( + "@rules_python//python:py_info.bzl", "py_binary", "py_test", "py_library")) + .put( + "PyRuntimeInfo", + symbolRedirect( + "@rules_python//python:py_runtime_info.bzl", + "py_binary", + "py_test", + "py_library")) + .put( + "PyCcLinkParamsProvider", + renamedSymbolRedirect( + "@rules_python//python:py_cc_link_params_info.bzl", + "PyCcLinkParamsInfo", + "py_binary", + "py_test", + "py_library")) + .put("aar_import", ruleRedirect("@rules_android//rules:rules.bzl")) + .put("android_binary", ruleRedirect("@rules_android//rules:rules.bzl")) + .put("android_device_script_fixture", ruleRedirect("@rules_android//rules:rules.bzl")) + .put("android_host_service_fixture", ruleRedirect("@rules_android//rules:rules.bzl")) + .put("android_library", ruleRedirect("@rules_android//rules:rules.bzl")) + .put("android_local_test", ruleRedirect("@rules_android//rules:rules.bzl")) + .put("android_sdk", ruleRedirect("@rules_android//rules:rules.bzl")) + .put("android_tools_defaults_jar", ruleRedirect("@rules_android//rules:rules.bzl")) + .put("cc_binary", ruleRedirect("@rules_cc//cc:cc_binary.bzl")) + .put("cc_import", ruleRedirect("@rules_cc//cc:cc_import.bzl")) + .put("cc_library", ruleRedirect("@rules_cc//cc:cc_library.bzl")) + .put("cc_proto_library", ruleRedirect("@protobuf//bazel:cc_proto_library.bzl")) + .put("cc_shared_library", ruleRedirect("@rules_cc//cc:cc_shared_library.bzl")) + .put("cc_test", ruleRedirect("@rules_cc//cc:cc_test.bzl")) + .put("cc_toolchain", ruleRedirect("@rules_cc//cc/toolchains:cc_toolchain.bzl")) + .put( + "cc_toolchain_suite", ruleRedirect("@rules_cc//cc/toolchains:cc_toolchain_suite.bzl")) + .put( + "fdo_prefetch_hints", ruleRedirect("@rules_cc//cc/toolchains:fdo_prefetch_hints.bzl")) + .put("fdo_profile", ruleRedirect("@rules_cc//cc/toolchains:fdo_profile.bzl")) + .put("java_binary", ruleRedirect("@rules_java//java:java_binary.bzl")) + .put("java_import", ruleRedirect("@rules_java//java:java_import.bzl")) + .put("java_library", ruleRedirect("@rules_java//java:java_library.bzl")) + .put( + "java_lite_proto_library", + ruleRedirect("@protobuf//bazel:java_lite_proto_library.bzl")) + .put( + "java_package_configuration", + ruleRedirect("@rules_java//java/toolchains:java_package_configuration.bzl")) + .put("java_plugin", ruleRedirect("@rules_java//java:java_plugin.bzl")) + .put("java_proto_library", ruleRedirect("@protobuf//bazel:java_proto_library.bzl")) + .put("java_runtime", ruleRedirect("@rules_java//java/toolchains:java_runtime.bzl")) + .put("java_test", ruleRedirect("@rules_java//java:java_test.bzl")) + .put("java_toolchain", ruleRedirect("@rules_java//java/toolchains:java_toolchain.bzl")) + .put("memprof_profile", ruleRedirect("@rules_cc//cc/toolchains:memprof_profile.bzl")) + .put("objc_import", ruleRedirect("@rules_cc//cc:objc_import.bzl")) + .put("objc_library", ruleRedirect("@rules_cc//cc:objc_library.bzl")) + .put( + "propeller_optimize", ruleRedirect("@rules_cc//cc/toolchains:propeller_optimize.bzl")) + .put( + "proto_lang_toolchain", + ruleRedirect("@protobuf//bazel/toolchain:proto_lang_toolchain.bzl")) + .put("proto_library", ruleRedirect("@protobuf//bazel:proto_library.bzl")) + .put("py_binary", ruleRedirect("@rules_python//python:py_binary.bzl")) + .put("py_library", ruleRedirect("@rules_python//python:py_library.bzl")) + .put("py_runtime", ruleRedirect("@rules_python//python:py_runtime.bzl")) + .put("py_test", ruleRedirect("@rules_python//python:py_test.bzl")) + .put("sh_binary", ruleRedirect("@rules_sh//sh:sh_binary.bzl")) + .put("sh_library", ruleRedirect("@rules_sh//sh:sh_library.bzl")) + .put("sh_test", ruleRedirect("@rules_sh//sh:sh_test.bzl")) + .put("available_xcodes", ruleRedirect("@apple_support//xcode:available_xcodes.bzl")) + .put("xcode_config", ruleRedirect("@apple_support//xcode:xcode_config.bzl")) + .put("xcode_config_alias", ruleRedirect("@apple_support//xcode:xcode_config_alias.bzl")) + .put("xcode_version", ruleRedirect("@apple_support//xcode:xcode_version.bzl")) + // this redirect doesn't exists and probably never will, we still need a configuration for + // it, so that it can be removed from Bazels <= 7 if needed + .put( + "apple_common", + symbolRedirect("@apple_support//lib:apple_common.bzl", "objc_import", "objc_library")) + .buildOrThrow(); +} diff --git a/src/main/java/com/google/devtools/build/lib/packages/BUILD b/src/main/java/com/google/devtools/build/lib/packages/BUILD index 014f6c96710324..c41443a94b618f 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BUILD +++ b/src/main/java/com/google/devtools/build/lib/packages/BUILD @@ -87,6 +87,7 @@ 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", diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java index 54c27c8ae368dd..34a2c1f5d51d39 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java +++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.concurrent.BlazeInterners; import com.google.devtools.common.options.Converters.CommaSeparatedNonEmptyOptionListConverter; import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter; +import com.google.devtools.common.options.Converters.CommaSeparatedOptionSetConverter; import com.google.devtools.common.options.Option; import com.google.devtools.common.options.OptionDocumentationCategory; import com.google.devtools.common.options.OptionEffectTag; @@ -97,6 +98,58 @@ public final class BuildLanguageOptions extends OptionsBase { + "the builtins injection mechanism entirely.") public String experimentalBuiltinsBzlPath; + @Option( + name = "incompatible_autoload_externally", + converter = CommaSeparatedOptionSetConverter.class, + defaultValue = "", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE, OptionEffectTag.BUILD_FILE_SEMANTICS}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = + "A comma-separated list of rules (or other symbols) that were previously part of Bazel" + + " and which are now to be retrieved from their respective external repositories." + + " This flag is intended to be used to facilitate migration of rules out of Bazel." + + " See also https://github.com/bazelbuild/bazel/issues/23043.\n" + + "A symbol that is autoloaded within a file behaves as if its built-into-Bazel" + + " definition were replaced by its canonical new definition in an external" + + " repository. For a BUILD file, this essentially means implicitly adding a load()" + + " statement. For a .bzl file, it's either a load() statement or a change to a field" + + " of the `native` object, depending on whether the autoloaded symbol is a rule.\n" + + "Bazel maintains a hardcoded list of all symbols that may be autoloaded; only those" + + " symbols may appear in this flag. For each symbol, Bazel knows the new definition" + + " location in an external repository, as well as a set of special-cased" + + " repositories that must not autoload it to avoid creating cycles.\n" + + "A list item of \"+foo\" in this flag causes symbol foo to be autoloaded, except in" + + " foo's exempt repositories, within which the Bazel-defined version of foo is still" + + " available.\n" + + "A list item of \"foo\" triggers autoloading as above, but the Bazel-defined" + + " version of foo is not made available to the excluded repositories. This ensures" + + " that foo's external repository does not depend on the old Bazel implementation of" + + " foo\n" + + "A list item of \"-foo\" does not trigger any autoloading, but makes the" + + " Bazel-defined version of foo inaccessible throughout the workspace. This is used" + + " to validate that the workspace is ready for foo's definition to be deleted from" + + " Bazel.\n" + + "If a symbol is not named in this flag then it continues to work as normal -- no" + + " autoloading is done, nor is the Bazel-defined version suppressed. For" + + " configuration see" + + " https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/packages/AutoloadSymbols.java") + public List incompatibleAutoloadExternally; + + @Option( + name = "repositories_without_autoloads", + converter = CommaSeparatedOptionSetConverter.class, + defaultValue = "", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.LOSES_INCREMENTAL_STATE, OptionEffectTag.BUILD_FILE_SEMANTICS}, + metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE}, + help = + "A list of additional repositories (beyond the hardcoded ones Bazel knows about) where " + + "autoloads are not to be added. This should typically contain repositories that are" + + " transitively depended on by a repository that may be loaded automatically " + + "(and which can therefore potentially create a cycle).") + public List repositoriesWithoutAutoloads; + @Option( name = "experimental_builtins_dummy", defaultValue = "false", @@ -737,6 +790,8 @@ public StarlarkSemantics toStarlarkSemantics() { incompatibleStopExportingLanguageModules) .setBool(INCOMPATIBLE_ALLOW_TAGS_PROPAGATION, experimentalAllowTagsPropagation) .set(EXPERIMENTAL_BUILTINS_BZL_PATH, experimentalBuiltinsBzlPath) + .set(INCOMPATIBLE_AUTOLOAD_EXTERNALLY, incompatibleAutoloadExternally) + .set(REPOSITORIES_WITHOUT_AUTOLOAD, repositoriesWithoutAutoloads) .setBool(EXPERIMENTAL_BUILTINS_DUMMY, experimentalBuiltinsDummy) .set(EXPERIMENTAL_BUILTINS_INJECTION_OVERRIDE, experimentalBuiltinsInjectionOverride) .setBool(EXPERIMENTAL_BZL_VISIBILITY, experimentalBzlVisibility) @@ -927,6 +982,10 @@ public StarlarkSemantics toStarlarkSemantics() { // non-booleans public static final StarlarkSemantics.Key EXPERIMENTAL_BUILTINS_BZL_PATH = new StarlarkSemantics.Key<>("experimental_builtins_bzl_path", "%bundled%"); + public static final StarlarkSemantics.Key> INCOMPATIBLE_AUTOLOAD_EXTERNALLY = + new StarlarkSemantics.Key<>("incompatible_autoload_externally", ImmutableList.of()); + public static final StarlarkSemantics.Key> REPOSITORIES_WITHOUT_AUTOLOAD = + new StarlarkSemantics.Key<>("repositories_without_autoloads", ImmutableList.of()); public static final StarlarkSemantics.Key> EXPERIMENTAL_BUILTINS_INJECTION_OVERRIDE = new StarlarkSemantics.Key<>("experimental_builtins_injection_override", ImmutableList.of()); public static final StarlarkSemantics.Key MAX_COMPUTATION_STEPS = diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index f8da79ac642b68..45814af22a7278 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -2621,6 +2621,7 @@ java_library( ":bzl_load_value", ":repository_mapping_value", ":sky_functions", + ":starlark_builtins_value", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileFunction.java index d18bfe2ca3e699..38c691fce1a27e 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileFunction.java @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.cmdline.BazelCompileContext; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.packages.AutoloadSymbols; import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -157,7 +158,14 @@ static BzlCompileValue computeInline( // For WORKSPACE-loaded bzl files, the env isn't quite right not because of injection but // because the "native" object is different. But A) that will be fixed with #11954, and B) we // don't care for the same reason as above. - predeclared = bazelStarlarkEnvironment.getUninjectedBuildBzlEnv(); + + // 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); + if (autoloadSymbols == null) { + return null; + } + predeclared = autoloadSymbols.getUninjectedBuildBzlEnv(key.getLabel()); } // We have all deps. Parse, resolve, and return. diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java index 9cefa2cf5147d2..1e16e34f904eb9 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.io.InconsistentFilesystemException; +import com.google.devtools.build.lib.packages.AutoloadSymbols; import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment; import com.google.devtools.build.lib.packages.BuildFileNotFoundException; import com.google.devtools.build.lib.packages.BzlInitThreadContext; @@ -609,13 +610,19 @@ private StarlarkBuiltinsValue getBuiltins( } return StarlarkBuiltinsValue.createEmpty(starlarkSemantics); } + AutoloadSymbols autoloadSymbols = PrecomputedValue.AUTOLOAD_SYMBOLS.get(env); + if (autoloadSymbols == null) { + return null; + } try { + boolean withAutoloads = requiresAutoloads(key, autoloadSymbols); if (inliningState == null) { return (StarlarkBuiltinsValue) - env.getValueOrThrow(StarlarkBuiltinsValue.key(), BuiltinsFailedException.class); + env.getValueOrThrow( + StarlarkBuiltinsValue.key(withAutoloads), BuiltinsFailedException.class); } else { return StarlarkBuiltinsFunction.computeInline( - StarlarkBuiltinsValue.key(), + StarlarkBuiltinsValue.key(withAutoloads), inliningState, ruleClassProvider.getBazelStarlarkEnvironment(), /* bzlLoadFunction= */ this); @@ -635,6 +642,23 @@ private static boolean requiresBuiltinsInjection(BzlLoadValue.Key key) { && !(key instanceof BzlLoadValue.KeyForBzlmodBootstrap)); } + private static boolean requiresAutoloads(BzlLoadValue.Key key, AutoloadSymbols autoloadSymbols) { + // We do autoloads for all BUILD files and BUILD-loaded .bzl files, except for files in + // certain rule repos (see AutoloadSymbols#reposDisallowingAutoloads). + // + // We don't do autoloads for the WORKSPACE file, Bzlmod files, or .bzls loaded by them, + // because in general the rules repositories that we would load are not yet available. + // + // We never do autoloads for builtins bzls. + // + // We don't do autoloads for the prelude file, but that's a single file so users can migrate it + // easily. (We do autoloads in .bzl files that are loaded by the prelude file.) + return autoloadSymbols.isEnabled() + && key instanceof BzlLoadValue.KeyForBuild + && !key.isBuildPrelude() + && !autoloadSymbols.autoloadsDisabledForRepo(key.getLabel()); + } + /** * Given a bzl key, validates that the corresponding package exists (if required) and returns the * associated compile key based on the package's root. Returns null for a missing Skyframe dep or diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoCycleReporter.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoCycleReporter.java index 823e615d9de62e..78a9f556552d8f 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoCycleReporter.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoCycleReporter.java @@ -186,12 +186,25 @@ public boolean maybeReportCycle( } else if (Iterables.any(cycle, IS_BZL_LOAD)) { Label fileLabel = ((BzlLoadValue.Key) Iterables.getLast(Iterables.filter(cycle, IS_BZL_LOAD))).getLabel(); - eventHandler.handle( - Event.error( - null, - String.format( - "Failed to load .bzl file '%s': possible dependency cycle detected.\n", - fileLabel))); + final String errorMessage; + if (cycle.get(0).equals(StarlarkBuiltinsValue.key(true))) { + // We know `fileLabel` is the last .bzl visited in the cycle. We also know that + // BzlLoadFunction triggered the cycle by requesting StarlarkBuiltinsValue w/autoloads. + // We know that we're not in builtins .bzls, because they don't request w/autoloads. + // Thus, `fileLabel` is a .bzl transitively depended on by an autoload. + errorMessage = + String.format( + "Cycle caused by autoloads, failed to load .bzl file '%s'.\n" + + "Add '%s' to --repositories_without_autoloads or disable autoloads by setting" + + " '--incompatible_autoload_externally='\n" + + "More information on https://github.com/bazelbuild/bazel/issues/23043.\n", + fileLabel, fileLabel.getRepository().getName()); + } else { + errorMessage = + String.format( + "Failed to load .bzl file '%s': possible dependency cycle detected.\n", fileLabel); + } + eventHandler.handle(Event.error(null, errorMessage)); return true; } else if (Iterables.any(cycle, IS_PACKAGE_LOOKUP)) { PackageIdentifier pkg = 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 2bd811cd7fd7d8..f59765321cefb2 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 @@ -34,6 +34,7 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.io.FileSymlinkException; import com.google.devtools.build.lib.io.InconsistentFilesystemException; +import com.google.devtools.build.lib.packages.AutoloadSymbols; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.BuildFileNotFoundException; import com.google.devtools.build.lib.packages.CachingPackageLocator; @@ -457,14 +458,21 @@ 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); + if (autoloadSymbols == null) { + return null; + } if (bzlLoadFunctionForInlining == null) { starlarkBuiltinsValue = (StarlarkBuiltinsValue) - env.getValueOrThrow(StarlarkBuiltinsValue.key(), BuiltinsFailedException.class); + env.getValueOrThrow( + StarlarkBuiltinsValue.key(/* withAutoloads= */ autoloadSymbols.isEnabled()), + BuiltinsFailedException.class); } else { starlarkBuiltinsValue = StarlarkBuiltinsFunction.computeInline( - StarlarkBuiltinsValue.key(), + StarlarkBuiltinsValue.key(/* withAutoloads= */ autoloadSymbols.isEnabled()), BzlLoadFunction.InliningState.create(env), packageFactory.getRuleClassProvider().getBazelStarlarkEnvironment(), bzlLoadFunctionForInlining); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java index 21082d2472b142..734bbd35e16f87 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrecomputedValue.java @@ -19,6 +19,7 @@ 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; @@ -84,6 +85,10 @@ public static Injected injected(Precomputed precomputed, T value) { public static final Precomputed STARLARK_SEMANTICS = new Precomputed<>("starlark_semantics"); + // Configuration of --incompatible_load_externally + public static final Precomputed AUTOLOAD_SYMBOLS = + new Precomputed<>("autoload_symbols"); + public static final Precomputed BUILD_ID = new Precomputed<>("build_id", /* shareable= */ false); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index fed05dbca128d8..a7b330ce57ec63 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java @@ -142,6 +142,7 @@ import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.io.FileSymlinkCycleUniquenessFunction; import com.google.devtools.build.lib.io.FileSymlinkInfiniteExpansionUniquenessFunction; +import com.google.devtools.build.lib.packages.AutoloadSymbols; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.BuildFileName; import com.google.devtools.build.lib.packages.NoSuchPackageException; @@ -1334,6 +1335,10 @@ private void setStarlarkSemantics(StarlarkSemantics starlarkSemantics) { PrecomputedValue.STARLARK_SEMANTICS.set(injectable(), starlarkSemantics); } + private void setAutoloadsConfiguration(AutoloadSymbols autoloadSymbols) { + PrecomputedValue.AUTOLOAD_SYMBOLS.set(injectable(), autoloadSymbols); + } + public void setBaselineConfiguration(BuildOptions buildOptions) { PrecomputedValue.BASELINE_CONFIGURATION.set(injectable(), buildOptions); } @@ -1476,6 +1481,7 @@ public void preparePackageLoading( StarlarkSemantics starlarkSemantics = getEffectiveStarlarkSemantics(buildLanguageOptions); setStarlarkSemantics(starlarkSemantics); + setAutoloadsConfiguration(new AutoloadSymbols(ruleClassProvider, starlarkSemantics)); setSiblingDirectoryLayout( starlarkSemantics.getBool(BuildLanguageOptions.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT)); setPackageLocator(pkgLocator); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java index 9fbcd35899d134..f32df8ad4f0ef3 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java @@ -17,17 +17,22 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.AutoloadSymbols; +import com.google.devtools.build.lib.packages.AutoloadSymbols.AutoloadException; import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment; import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment.InjectionException; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; import com.google.devtools.build.lib.skyframe.BzlLoadFunction.InlineCacheManager; +import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.skyframe.RecordingSkyFunctionEnvironment; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.build.skyframe.SkyFunctionException; import com.google.devtools.build.skyframe.SkyFunctionException.Transience; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyValue; +import com.google.devtools.build.skyframe.SkyframeLookupResult; import javax.annotation.Nullable; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; @@ -61,8 +66,11 @@ * href="https://docs.google.com/document/d/1GW7UVo1s9X0cti9OMgT3ga5ozKYUWLPk9k8c4-34rC4">design * doc: * - *

This function has a trivial key, so there can only be one value in the build at a time. It has - * a single dependency, on the result of evaluating the exports.bzl file to a {@link BzlLoadValue}. + *

This function has two trivial keys, so there can only be two values in the build at a time. + * First key/value (without autoloads) has a single dependency, on the result of evaluating the + * exports.bzl file to a {@link BzlLoadValue}. Second key/value has dependencies on the results of + * extenally loaded symbols and rules. For more information on the second value see {@link + * AutoloadSymbols}. * *

This function supports a special "inlining" mode, similar to {@link BzlLoadFunction} (see that * class's javadoc and code comments). Whenever we inline {@link BzlLoadFunction} we also inline @@ -105,10 +113,13 @@ public StarlarkBuiltinsFunction(BazelStarlarkEnvironment bazelStarlarkEnvironmen @Nullable public SkyValue compute(SkyKey skyKey, Environment env) throws StarlarkBuiltinsFunctionException, InterruptedException { - // skyKey is a singleton, unused. try { return computeInternal( - env, bazelStarlarkEnvironment, /* inliningState= */ null, /* bzlLoadFunction= */ null); + env, + bazelStarlarkEnvironment, + ((StarlarkBuiltinsValue.Key) skyKey.argument()).isWithAutoloads(), + /* inliningState= */ null, + /* bzlLoadFunction= */ null); } catch (BuiltinsFailedException e) { throw new StarlarkBuiltinsFunctionException(e); } @@ -125,7 +136,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) */ @Nullable public static StarlarkBuiltinsValue computeInline( - StarlarkBuiltinsValue.Key key, // singleton value, unused + StarlarkBuiltinsValue.Key key, BzlLoadFunction.InliningState inliningState, BazelStarlarkEnvironment bazelStarlarkEnvironment, BzlLoadFunction bzlLoadFunction) @@ -145,6 +156,7 @@ public static StarlarkBuiltinsValue computeInline( computeInternal( inliningState.getEnvironment(), bazelStarlarkEnvironment, + key.isWithAutoloads(), inliningState, bzlLoadFunction); if (computedBuiltins == null) { @@ -161,6 +173,7 @@ public static StarlarkBuiltinsValue computeInline( private static StarlarkBuiltinsValue computeInternal( Environment env, BazelStarlarkEnvironment bazelStarlarkEnvironment, + boolean isWithAutoloads, @Nullable BzlLoadFunction.InliningState inliningState, @Nullable BzlLoadFunction bzlLoadFunction) throws BuiltinsFailedException, InterruptedException { @@ -172,9 +185,27 @@ private static StarlarkBuiltinsValue computeInternal( if (starlarkSemantics.get(BuildLanguageOptions.EXPERIMENTAL_BUILTINS_BZL_PATH).isEmpty()) { return StarlarkBuiltinsValue.createEmpty(starlarkSemantics); } + AutoloadSymbols autoloadSymbols = PrecomputedValue.AUTOLOAD_SYMBOLS.get(env); + if (autoloadSymbols == null) { + return null; + } + + if (autoloadSymbols.isEnabled()) { + // We can't do autoloads where the rules are implemented (disabling them when running in + // main repository named rules_python) + ModuleFileValue mainModule = + (ModuleFileValue) env.getValue(ModuleFileValue.KEY_FOR_ROOT_MODULE); + if (mainModule == null) { + return null; + } + if (autoloadSymbols.autoloadsDisabledForRepo(mainModule.getModule().getName())) { + isWithAutoloads = false; + } + } // Load exports.bzl. If we were requested using inlining, make sure to inline the call back into // BzlLoadFunction. + // If requested also loads "autoloads": rules and providers from external repositories. BzlLoadValue exportsValue; try { if (inliningState == null) { @@ -187,12 +218,59 @@ private static StarlarkBuiltinsValue computeInternal( } catch (BzlLoadFailedException ex) { throw BuiltinsFailedException.errorEvaluatingBuiltinsBzls(ex); } - if (exportsValue == null) { + + ImmutableMap autoBzlLoadValues; + try { + ImmutableMap autoBzlLoadKeys = + isWithAutoloads ? autoloadSymbols.getLoadKeys(env) : ImmutableMap.of(); + if (autoBzlLoadKeys == null) { + return null; + } + ImmutableMap.Builder autoBzlLoadValuesBuilder = + ImmutableMap.builderWithExpectedSize(autoBzlLoadKeys.size()); + if (inliningState == null) { + SkyframeLookupResult values = env.getValuesAndExceptions(autoBzlLoadKeys.values()); + for (var symbolKeyEntry : autoBzlLoadKeys.entrySet()) { + String symbol = symbolKeyEntry.getKey(); + BzlLoadValue value = + (BzlLoadValue) + values.getOrThrow(symbolKeyEntry.getValue(), BzlLoadFailedException.class); + if (value != null) { + autoBzlLoadValuesBuilder.put(symbol, value); + } + } + } else { + for (var symbolKeyEntry : autoBzlLoadKeys.entrySet()) { + String symbol = symbolKeyEntry.getKey(); + BzlLoadValue value = + bzlLoadFunction.computeInline(symbolKeyEntry.getValue(), inliningState); + if (value != null) { + autoBzlLoadValuesBuilder.put(symbol, value); + } + } + } + autoBzlLoadValues = autoBzlLoadValuesBuilder.buildOrThrow(); + } catch (BzlLoadFailedException ex) { + + throw BuiltinsFailedException.errorEvaluatingAutoloadedBzls( + ex, starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD)); + } + if (env.valuesMissing()) { return null; } - // Apply declarations of exports.bzl to the native predeclared symbols. + // Compute digest of exports.bzl and possibly externally loaded symbols (Bazel) byte[] transitiveDigest = exportsValue.getTransitiveDigest(); + if (!autoBzlLoadValues.isEmpty()) { + Fingerprint fp = new Fingerprint(); + fp.addBytes(transitiveDigest); + for (BzlLoadValue value : autoBzlLoadValues.values()) { + fp.addBytes(value.getTransitiveDigest()); + } + transitiveDigest = fp.digestAndReset(); + } + + // Apply declarations of exports.bzl to the native predeclared symbols. Module module = exportsValue.getModule(); try { ImmutableMap exportedToplevels = getDict(module, "exported_toplevels"); @@ -212,6 +290,14 @@ private static StarlarkBuiltinsValue computeInternal( bazelStarlarkEnvironment.createBuildEnvUsingInjection( exportedRules, starlarkSemantics.get(BuildLanguageOptions.EXPERIMENTAL_BUILTINS_INJECTION_OVERRIDE)); + + // Apply declarations of externally loaded symbols to the native predeclared symbols. + ImmutableMap newSymbols = autoloadSymbols.processLoads(autoBzlLoadValues); + predeclaredForBuild = + autoloadSymbols.modifyBuildEnv(isWithAutoloads, predeclaredForBuild, newSymbols); + predeclaredForBuildBzl = + autoloadSymbols.modifyBuildBzlEnv(isWithAutoloads, predeclaredForBuildBzl, newSymbols); + return StarlarkBuiltinsValue.create( predeclaredForBuildBzl, predeclaredForWorkspaceBzl, @@ -221,6 +307,11 @@ private static StarlarkBuiltinsValue computeInternal( starlarkSemantics); } catch (EvalException | InjectionException ex) { throw BuiltinsFailedException.errorApplyingExports(ex); + } catch (AutoloadException ex) { + throw new BuiltinsFailedException( + String.format("Failed to apply symbols loaded externally: %s", ex.getMessage()), + ex, + Transience.PERSISTENT); } } @@ -272,6 +363,20 @@ static BuiltinsFailedException errorEvaluatingBuiltinsBzls( transience); } + static BuiltinsFailedException errorEvaluatingAutoloadedBzls( + BzlLoadFailedException cause, boolean bzlmodEnabled) { + String additionalMessage = + bzlmodEnabled + ? "" + : ". Most likely you need to upgrade the version of rules repository in the" + + " WORKSPACE file."; + return new BuiltinsFailedException( + String.format( + "Failed to autoload external symbols: %s%s", cause.getMessage(), additionalMessage), + cause, + cause.getTransience()); + } + static BuiltinsFailedException errorApplyingExports(Exception cause) { return new BuiltinsFailedException( String.format("Failed to apply declared builtins: %s", cause.getMessage()), diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsValue.java index a262bd63980d79..a77aa2a1351316 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsValue.java @@ -23,7 +23,8 @@ import net.starlark.java.eval.StarlarkSemantics; /** - * A Skyframe value representing the result of evaluating the {@code @_builtins} pseudo-repository. + * A Skyframe value representing the result of evaluating the {@code @_builtins} pseudo-repository, + * and in Bazel where applicable, applying autoloads. * *

To avoid unnecessary Skyframe edges, the {@code StarlarkSemantics} are included in this value, * so that a caller who obtains a StarlarkBuiltinsValue can also access the StarlarkSemantics @@ -135,11 +136,19 @@ public static StarlarkBuiltinsValue createEmpty(StarlarkSemantics starlarkSemant starlarkSemantics); } - /** Returns the singleton SkyKey for this type of value. */ + /** Returns the SkyKey for BuiltinsValue containing only additional builtin symbols and rules. */ public static Key key() { return Key.INSTANCE; } + /** + * Returns the SkyKey for BuiltinsValue optionally amended with externally loaded symbols and + * rules. + */ + public static Key key(boolean withAutoloads) { + return withAutoloads ? Key.INSTANCE_WITH_AUTOLOADS : Key.INSTANCE; + } + /** * Skyframe key for retrieving the {@code @_builtins} definitions. * @@ -147,9 +156,18 @@ public static Key key() { */ static final class Key implements SkyKey { - private static final Key INSTANCE = new Key(); + private final boolean withAutoloads; - private Key() {} + private static final Key INSTANCE = new Key(false); + private static final Key INSTANCE_WITH_AUTOLOADS = new Key(true); + + private Key(boolean withAutoloads) { + this.withAutoloads = withAutoloads; + } + + public boolean isWithAutoloads() { + return withAutoloads; + } @Override public SkyFunctionName functionName() { @@ -163,12 +181,12 @@ public String toString() { @Override public boolean equals(Object other) { - return other instanceof Key; + return other instanceof Key key && this.withAutoloads == key.withAutoloads; } @Override public int hashCode() { - return 7727; // more or less xkcd/221 + return withAutoloads ? 7727 : 7277; // more or less xkcd/221 } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java index 2eaf03dc05bb47..08a40352346975 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java @@ -40,6 +40,7 @@ import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.io.FileSymlinkCycleUniquenessFunction; import com.google.devtools.build.lib.io.FileSymlinkInfiniteExpansionUniquenessFunction; +import com.google.devtools.build.lib.packages.AutoloadSymbols; import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException; import com.google.devtools.build.lib.packages.BuildFileName; import com.google.devtools.build.lib.packages.CachingPackageLocator; @@ -51,6 +52,7 @@ import com.google.devtools.build.lib.packages.PackageLoadingListener; import com.google.devtools.build.lib.packages.PackageOverheadEstimator; import com.google.devtools.build.lib.packages.PackageValidator; +import com.google.devtools.build.lib.packages.RuleClassProvider; import com.google.devtools.build.lib.packages.RuleVisibility; import com.google.devtools.build.lib.packages.WorkspaceFileValue; import com.google.devtools.build.lib.pkgcache.PathPackageLocator; @@ -311,6 +313,7 @@ public final PackageLoader build() { makePreinjectedDiff( starlarkSemantics, builder.pkgLocator, + ruleClassProvider, ImmutableList.copyOf(builder.extraPrecomputedValues)); pkgFactory = new PackageFactory( @@ -325,6 +328,7 @@ public final PackageLoader build() { private static ImmutableDiff makePreinjectedDiff( StarlarkSemantics starlarkSemantics, PathPackageLocator pkgLocator, + RuleClassProvider ruleClassProvider, ImmutableList extraPrecomputedValues) { final Map valuesToInject = new HashMap<>(); Injectable injectable = @@ -347,6 +351,8 @@ 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( + injectable, new AutoloadSymbols(ruleClassProvider, starlarkSemantics)); return new ImmutableDiff(ImmutableList.of(), valuesToInject); } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index 5004c7d8d59350..70eeaaea0a3979 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.cmdline.PackageIdentifier; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.EventKind; +import com.google.devtools.build.lib.packages.AutoloadSymbols; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.WorkspaceFileValue; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; @@ -259,12 +260,14 @@ public void setup() throws Exception { .build(), differencer); - PrecomputedValue.STARLARK_SEMANTICS.set( - differencer, + StarlarkSemantics semantics = StarlarkSemantics.builder() .setBool(BuildLanguageOptions.ENABLE_BZLMOD, true) .setBool(BuildLanguageOptions.EXPERIMENTAL_ISOLATED_EXTENSION_USAGES, true) - .build()); + .build(); + PrecomputedValue.STARLARK_SEMANTICS.set(differencer, semantics); + PrecomputedValue.AUTOLOAD_SYMBOLS.set( + differencer, new AutoloadSymbols(ruleClassProvider, semantics)); RepositoryDelegatorFunction.REPOSITORY_OVERRIDES.set(differencer, ImmutableMap.of()); RepositoryDelegatorFunction.FORCE_FETCH.set( differencer, RepositoryDelegatorFunction.FORCE_FETCH_DISABLED); diff --git a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java index 4ccb9bac94dbfe..b232bc18509b11 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorTest.java @@ -48,6 +48,7 @@ import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.events.StoredEventHandler; +import com.google.devtools.build.lib.packages.AutoloadSymbols; import com.google.devtools.build.lib.packages.PackageFactory; import com.google.devtools.build.lib.packages.WorkspaceFileValue; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; @@ -266,9 +267,11 @@ public void setupDelegator() throws Exception { RepositoryDelegatorFunction.FORCE_FETCH.set( differencer, RepositoryDelegatorFunction.FORCE_FETCH_DISABLED); PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get()); - PrecomputedValue.STARLARK_SEMANTICS.set( - differencer, - StarlarkSemantics.builder().setBool(BuildLanguageOptions.ENABLE_BZLMOD, true).build()); + StarlarkSemantics semantics = + StarlarkSemantics.builder().setBool(BuildLanguageOptions.ENABLE_BZLMOD, true).build(); + PrecomputedValue.STARLARK_SEMANTICS.set(differencer, semantics); + PrecomputedValue.AUTOLOAD_SYMBOLS.set( + differencer, new AutoloadSymbols(ruleClassProvider, semantics)); RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE.set( differencer, Optional.empty()); PrecomputedValue.REPO_ENV.set(differencer, ImmutableMap.of()); diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD index 805da61158abe9..fa0951242130c9 100644 --- a/src/test/shell/integration/BUILD +++ b/src/test/shell/integration/BUILD @@ -761,6 +761,14 @@ sh_test( tags = ["no_windows"], ) +sh_test( + name = "load_removed_symbols_test", + size = "medium", + srcs = ["load_removed_symbols_test.sh"], + data = [":test-deps"], + tags = ["no_windows"], +) + sh_test( name = "bazel_java_test", size = "medium", diff --git a/src/test/shell/integration/load_removed_symbols_test.sh b/src/test/shell/integration/load_removed_symbols_test.sh new file mode 100755 index 00000000000000..230a6ca2951165 --- /dev/null +++ b/src/test/shell/integration/load_removed_symbols_test.sh @@ -0,0 +1,405 @@ +#!/bin/bash +# +# Copyright 2024 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Tests the behaviour of --incompatible_autoload_externally flag. + +# Load the test setup defined in the parent directory +CURRENT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +source "${CURRENT_DIR}/../integration_test_setup.sh" \ + || { echo "integration_test_setup.sh not found!" >&2; exit 1; } + +#### SETUP ############################################################# + +set -e + +#### TESTS ############################################################# + +function mock_rules_android() { + rules_android_workspace="${TEST_TMPDIR}/rules_android_workspace" + mkdir -p "${rules_android_workspace}/rules" + touch "${rules_android_workspace}/rules/BUILD" + touch "${rules_android_workspace}/WORKSPACE" + cat > "${rules_android_workspace}/MODULE.bazel" << EOF +module(name = "rules_android") +EOF + cat > "${rules_android_workspace}/rules/rules.bzl" << EOF +def _impl(ctx): + pass + +aar_import = rule( + implementation = _impl, + attrs = { + "aar": attr.label(allow_files = True), + "deps": attr.label_list(), + } +) +EOF + + cat >> MODULE.bazel << EOF +bazel_dep( + name = "rules_android", +) +local_path_override( + module_name = "rules_android", + path = "${rules_android_workspace}", +) +EOF +} + +function disabled_test_removed_rule_loaded() { + setup_module_dot_bazel + mock_rules_android + + 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 failed" + # TODO(b/355260271): add test with workspace enabled +} + +function disabled_test_removed_rule_loaded_from_bzl() { + setup_module_dot_bazel + mock_rules_android + + cat > macro.bzl << EOF +def macro(): + native.aar_import( + name = 'aar', + aar = 'aar.file', + deps = [], + ) +EOF + + cat > BUILD << EOF +load(":macro.bzl", "macro") +macro() +EOF + bazel build --incompatible_autoload_externally=aar_import :aar >&$TEST_log 2>&1 || fail "build failed" + # TODO(b/355260271): add test with workspace enabled +} + +# TODO(b/355260271): enable this once we have a removed symbol +function disabled_test_removed_symbol_loaded() { + setup_module_dot_bazel + + cat > symbol.bzl << EOF +def symbol(): + a = ProtoInfo +EOF + + cat > BUILD << EOF +load(":symbol.bzl", "symbol") +symbol() +EOF + bazel build --incompatible_autoload_externally=ProtoInfo :all >&$TEST_log 2>&1 || fail "build failed" +} + +function test_existing_rule_is_redirected() { + setup_module_dot_bazel + + cat > BUILD << EOF +py_library( + name = 'py_library', +) +EOF + bazel query --incompatible_autoload_externally=+py_library ':py_library' --output=build >&$TEST_log 2>&1 || fail "build failed" + expect_log "rules_python./python/py_library.bzl" +} + +function test_existing_rule_is_redirected_in_bzl() { + setup_module_dot_bazel + + cat > macro.bzl << EOF +def macro(): + native.py_library( + name = 'py_library', + ) +EOF + + cat > BUILD << EOF +load(":macro.bzl", "macro") +macro() +EOF + bazel query --incompatible_autoload_externally=+py_library ':py_library' --output=build >&$TEST_log 2>&1 || fail "build failed" + expect_log "rules_python./python/py_library.bzl" +} + + +function test_removed_rule_not_loaded() { + setup_module_dot_bazel + + cat > BUILD << EOF +aar_import( + name = 'aar', + aar = 'aar.file', + deps = [], + visibility = ['//visibility:public'], +) +EOF + + bazel build --incompatible_autoload_externally= :aar >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "name 'aar_import' is not defined" +} + +function test_removed_rule_not_loaded_in_bzl() { + setup_module_dot_bazel + + cat > macro.bzl << EOF +def macro(): + native.aar_import( + name = 'aar', + aar = 'aar.file', + deps = [], + visibility = ['//visibility:public'], + ) +EOF + + cat > BUILD << EOF +load(":macro.bzl", "macro") +macro() +EOF + + bazel build --incompatible_autoload_externally= :aar >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "no native function or rule 'aar_import'" +} + +# TODO: enable once we have a removed symbol +function disabled_test_removed_symbol_not_loaded_in_bzl() { + setup_module_dot_bazel + + cat > symbol.bzl << EOF +def symbol(): + a = ProtoInfo +EOF + + cat > BUILD << EOF +load(":symbol.bzl", "symbol") +symbol() +EOF + + bazel build --incompatible_autoload_externally= :all >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "name 'ProtoInfo' is not defined" +} + + +function test_removing_existing_rule() { + setup_module_dot_bazel + + cat > BUILD << EOF +android_binary( + name = "bin", + srcs = [ + "MainActivity.java", + "Jni.java", + ], + manifest = "AndroidManifest.xml", + deps = [ + ":lib", + ":jni" + ], +) +EOF + + bazel build --incompatible_autoload_externally=-android_binary :bin >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "name 'android_binary' is not defined" +} + +function test_removing_existing_rule_in_bzl() { + setup_module_dot_bazel + + cat > macro.bzl << EOF +def macro(): + native.android_binary( + name = "bin", + srcs = [ + "MainActivity.java", + "Jni.java", + ], + manifest = "AndroidManifest.xml", + deps = [ + ":lib", + ":jni" + ], + ) +EOF + + cat > BUILD << EOF +load(":macro.bzl", "macro") +macro() +EOF + + bazel build --incompatible_autoload_externally=-android_binary :bin >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "no native function or rule 'android_binary'" +} + +function test_removing_symbol_incompletely() { + setup_module_dot_bazel + + cat > symbol.bzl << EOF +def symbol(): + a = ProtoInfo +EOF + + cat > BUILD << EOF +load(":symbol.bzl", "symbol") +symbol() +EOF + + bazel build --incompatible_autoload_externally=-ProtoInfo :all >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "Symbol in 'ProtoInfo' can't be removed, because it's still used by: proto_library, cc_proto_library, cc_shared_library, java_lite_proto_library, java_proto_library, proto_lang_toolchain, java_binary, proto_common_do_not_use" +} + +function test_removing_existing_symbol() { + setup_module_dot_bazel + + cat > symbol.bzl << EOF +def symbol(): + a = DebugPackageInfo +EOF + + cat > BUILD << EOF +load(":symbol.bzl", "symbol") +symbol() +EOF + + bazel build --incompatible_autoload_externally=-DebugPackageInfo,-cc_binary,-cc_test :all >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "name 'DebugPackageInfo' is not defined" +} + +function test_removing_symbol_typo() { + setup_module_dot_bazel + + cat > bzl_file.bzl << EOF +def bzl_file(): + pass +EOF + + cat > BUILD << EOF +load(":bzl_file.bzl", "bzl_file") +EOF + + bazel build --incompatible_autoload_externally=-ProtozzzInfo :all >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "Undefined symbol in --incompatible_autoload_externally" +} + +function test_removing_rule_typo() { + setup_module_dot_bazel + + touch BUILD + + bazel build --incompatible_autoload_externally=-androidzzz_binary :all >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "Undefined symbol in --incompatible_autoload_externally" +} + +function test_redirecting_rule_with_bzl_typo() { + setup_module_dot_bazel + + # Bzl file is evaluated first, so this should cover bzl file support + cat > bzl_file.bzl << EOF +def bzl_file(): + pass +EOF + + cat > BUILD << EOF +load(":bzl_file.bzl", "bzl_file") +EOF + + bazel build --incompatible_autoload_externally=pyzzz_library :all >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "Undefined symbol in --incompatible_autoload_externally" +} + +function test_redirecting_rule_typo() { + setup_module_dot_bazel + + cat > BUILD << EOF +EOF + + + bazel build --incompatible_autoload_externally=pyzzz_library :all >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "Undefined symbol in --incompatible_autoload_externally" +} + +function test_redirecting_symbols_typo() { + setup_module_dot_bazel + + # Bzl file is evaluated first, so this should cover bzl file support + cat > bzl_file.bzl << EOF +def bzl_file(): + pass +EOF + + cat > BUILD << EOF +load(":bzl_file.bzl", "bzl_file") +EOF + + bazel build --incompatible_autoload_externally=ProotoInfo :all >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "Undefined symbol in --incompatible_autoload_externally" +} + +function test_bad_flag_value() { + setup_module_dot_bazel + + cat > BUILD << EOF +py_library( + name = 'py_library', +) +EOF + bazel query --incompatible_autoload_externally=py_library,-py_library ':py_library' --output=build >&$TEST_log 2>&1 && fail "build unexpectedly succeeded" + expect_log "Duplicated symbol 'py_library' in --incompatible_autoload_externally" +} + +function disabled_test_missing_symbol_error() { + setup_module_dot_bazel + mock_rules_android + rules_android_workspace="${TEST_TMPDIR}/rules_android_workspace" + # emptying the file simulates a missing symbol + cat > "${rules_android_workspace}/rules/rules.bzl" << EOF +EOF + + 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 "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() { + setup_module_dot_bazel + mock_rules_android + rules_android_workspace="${TEST_TMPDIR}/rules_android_workspace" + rm "${rules_android_workspace}/rules/rules.bzl" + + 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 "Failed to autoload external symbols: cannot load '@@rules_android+//rules:rules.bzl': no such file" +} + + +run_suite "load_removed_symbols"