Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement incompatible_load_externally #23016

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/MODULE.tools
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,11 @@
module(name = "bazel_tools")

bazel_dep(name = "rules_cc", version = "0.0.9")
bazel_dep(name = "rules_java", version = "7.6.5")
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")

cc_configure = use_extension("//tools/cpp:cc_configure.bzl", "cc_configure_extension")
Expand Down Expand Up @@ -49,3 +46,9 @@ 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: tools/redirects_do_not_use, --incompatible_load_rules_externally, --incompatible_load_symbols_externally
bazel_dep(name = "protobuf", version = "3.19.6", repo_name = "com_google_protobuf")
bazel_dep(name = "rules_java", version = "7.6.5")
bazel_dep(name = "rules_python", version = "0.22.1")

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -97,6 +98,37 @@ 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 list of rules or symbols (previously part of Bazel) that are automatically loaded"
+ " from external repositories. For configuration see "
+ "https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/packages/AttributeValueSource.java "
+ "Prefixing a symbol with '+', loads the symbol from rules_repository, but keeps "
+ " the native implementation available in rules_repositories (redirects work). "
+ "Symbol without a prefix is loaded and the native implementation is not available. "
+ "Symbol prefixed with a '-' is not loaded and the native implementation is not "
+ "available (same as if the symbol was deleted from Bazel)")
public List<String> 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 where autoloads aren't be used. Add repositories that "
+ "are needed by rules_repositories. Adding loads to such repositories would create a"
+ " cycle back to rules_repositories.")
public List<String> repositoriesWithoutAutoloads;

@Option(
name = "experimental_builtins_dummy",
defaultValue = "false",
Expand Down Expand Up @@ -735,6 +767,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)
Expand Down Expand Up @@ -927,6 +961,10 @@ public StarlarkSemantics toStarlarkSemantics() {
// non-booleans
public static final StarlarkSemantics.Key<String> EXPERIMENTAL_BUILTINS_BZL_PATH =
new StarlarkSemantics.Key<>("experimental_builtins_bzl_path", "%bundled%");
public static final StarlarkSemantics.Key<List<String>> INCOMPATIBLE_AUTOLOAD_EXTERNALLY =
new StarlarkSemantics.Key<>("incompatible_autoload_externally", ImmutableList.of());
public static final StarlarkSemantics.Key<List<String>> REPOSITORIES_WITHOUT_AUTOLOAD =
new StarlarkSemantics.Key<>("repositories_without_autoloads", ImmutableList.of());
public static final StarlarkSemantics.Key<List<String>> EXPERIMENTAL_BUILTINS_INJECTION_OVERRIDE =
new StarlarkSemantics.Key<>("experimental_builtins_injection_override", ImmutableList.of());
public static final StarlarkSemantics.Key<Long> MAX_COMPUTATION_STEPS =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,11 @@ 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.
predeclared = PrecomputedValue.AUTOLOADS_CONFIGURATION.get(env)
.getUninjectedBuildBzlEnv(key.getLabel());
}

// We have all deps. Parse, resolve, and return.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.AutoloadsConfiguration;
import com.google.devtools.build.lib.packages.BazelStarlarkEnvironment;
import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
import com.google.devtools.build.lib.packages.BzlInitThreadContext;
Expand Down Expand Up @@ -610,12 +611,16 @@ private StarlarkBuiltinsValue getBuiltins(
return StarlarkBuiltinsValue.createEmpty(starlarkSemantics);
}
try {
AutoloadsConfiguration autoloadsConfiguration = PrecomputedValue.AUTOLOADS_CONFIGURATION.get(
env);
boolean withAutoloads = requiresAutoloads(key, autoloadsConfiguration);
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);
Expand All @@ -632,7 +637,18 @@ private static boolean requiresBuiltinsInjection(BzlLoadValue.Key key) {
// `@_builtins` depends on `@bazel_tools` for repo mapping, so we ignore some bzl files
// to avoid a cyclic dependency
|| (key instanceof BzlLoadValue.KeyForBzlmod
&& !(key instanceof BzlLoadValue.KeyForBzlmodBootstrap));
&& !(key instanceof BzlLoadValue.KeyForBzlmodBootstrap));
}

private static boolean requiresAutoloads(BzlLoadValue.Key key,
AutoloadsConfiguration autoloadsConfiguration) {
// We do autoloads for all BUILD files and bzl files outside of rules_repositories.
// We don't do them for prelude, WORKSPACE or Bzlmod files.
// Prelude is a single file, so users should be able to add loads to it easily.
// In WORKSPACE and bzlmod preloads are not possible, because rules_repositories are not
// available yet.
return key instanceof BzlLoadValue.KeyForBuild && !key.isBuildPrelude()
&& !autoloadsConfiguration.checkPristine(key.getLabel());
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,14 +457,17 @@ public SkyValue compute(SkyKey key, Environment env)

StarlarkBuiltinsValue starlarkBuiltinsValue;
try {
// Bazel: we do autoloads for all BUILD files if enabled
if (bzlLoadFunctionForInlining == null) {
starlarkBuiltinsValue =
(StarlarkBuiltinsValue)
env.getValueOrThrow(StarlarkBuiltinsValue.key(), BuiltinsFailedException.class);
env.getValueOrThrow(
StarlarkBuiltinsValue.key(/* withAutoloads= */ true),
BuiltinsFailedException.class);
} else {
starlarkBuiltinsValue =
StarlarkBuiltinsFunction.computeInline(
StarlarkBuiltinsValue.key(),
StarlarkBuiltinsValue.key(/* withAutoloads= */ true),
BzlLoadFunction.InliningState.create(env),
packageFactory.getRuleClassProvider().getBazelStarlarkEnvironment(),
bzlLoadFunctionForInlining);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.AutoloadsConfiguration;
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;
Expand Down Expand Up @@ -84,6 +85,10 @@ 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<AutoloadsConfiguration> AUTOLOADS_CONFIGURATION =
new Precomputed<>("uninjected_build_bzl_env");

public static final Precomputed<UUID> BUILD_ID =
new Precomputed<>("build_id", /* shareable= */ false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,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.AutoloadsConfiguration;
import com.google.devtools.build.lib.packages.BuildFileContainsErrorsException;
import com.google.devtools.build.lib.packages.BuildFileName;
import com.google.devtools.build.lib.packages.NoSuchPackageException;
Expand Down Expand Up @@ -1313,6 +1314,10 @@ private void setStarlarkSemantics(StarlarkSemantics starlarkSemantics) {
PrecomputedValue.STARLARK_SEMANTICS.set(injectable(), starlarkSemantics);
}

private void setAutoloadsConfiguration(AutoloadsConfiguration autoloadsConfiguration) {
PrecomputedValue.AUTOLOADS_CONFIGURATION.set(injectable(), autoloadsConfiguration);
}

public void setBaselineConfiguration(BuildOptions buildOptions) {
PrecomputedValue.BASELINE_CONFIGURATION.set(injectable(), buildOptions);
}
Expand Down Expand Up @@ -1455,6 +1460,8 @@ public void preparePackageLoading(

StarlarkSemantics starlarkSemantics = getEffectiveStarlarkSemantics(buildLanguageOptions);
setStarlarkSemantics(starlarkSemantics);
setAutoloadsConfiguration(
new AutoloadsConfiguration(ruleClassProvider, starlarkSemantics));
setSiblingDirectoryLayout(
starlarkSemantics.getBool(BuildLanguageOptions.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT));
setPackageLocator(pkgLocator);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,23 @@

import static com.google.common.base.Preconditions.checkNotNull;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.packages.AutoloadsConfiguration;
import com.google.devtools.build.lib.packages.AutoloadsConfiguration.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;
Expand Down Expand Up @@ -105,10 +110,14 @@ public StarlarkBuiltinsFunction(BazelStarlarkEnvironment bazelStarlarkEnvironmen
@Nullable
public SkyValue compute(SkyKey skyKey, Environment env)
throws StarlarkBuiltinsFunctionException, InterruptedException {
// skyKey is a singleton, unused.
// skyKey is either with or without autoloads.
try {
return computeInternal(
env, bazelStarlarkEnvironment, /* inliningState= */ null, /* bzlLoadFunction= */ null);
env,
bazelStarlarkEnvironment,
/* inliningState= */ null,
/* bzlLoadFunction= */ null,
((StarlarkBuiltinsValue.Key) skyKey.argument()).isWithAutoloads());
} catch (BuiltinsFailedException e) {
throw new StarlarkBuiltinsFunctionException(e);
}
Expand Down Expand Up @@ -146,7 +155,8 @@ public static StarlarkBuiltinsValue computeInline(
inliningState.getEnvironment(),
bazelStarlarkEnvironment,
inliningState,
bzlLoadFunction);
bzlLoadFunction,
key.isWithAutoloads());
if (computedBuiltins == null) {
return null;
}
Expand All @@ -162,7 +172,8 @@ private static StarlarkBuiltinsValue computeInternal(
Environment env,
BazelStarlarkEnvironment bazelStarlarkEnvironment,
@Nullable BzlLoadFunction.InliningState inliningState,
@Nullable BzlLoadFunction bzlLoadFunction)
@Nullable BzlLoadFunction bzlLoadFunction,
boolean isWithAutoloads)
throws BuiltinsFailedException, InterruptedException {
StarlarkSemantics starlarkSemantics = PrecomputedValue.STARLARK_SEMANTICS.get(env);
if (starlarkSemantics == null) {
Expand All @@ -172,27 +183,57 @@ private static StarlarkBuiltinsValue computeInternal(
if (starlarkSemantics.get(BuildLanguageOptions.EXPERIMENTAL_BUILTINS_BZL_PATH).isEmpty()) {
return StarlarkBuiltinsValue.createEmpty(starlarkSemantics);
}
AutoloadsConfiguration autoloadsConfiguration = PrecomputedValue.AUTOLOADS_CONFIGURATION.get(
env);
if (autoloadsConfiguration == null) {
return null;
}

// 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;
BzlLoadValue[] autoloadValues;
try {
ImmutableList<BzlLoadValue.Key> bzlAutoloadKeys =
isWithAutoloads ? autoloadsConfiguration.getAutoloads()
: ImmutableList.of();
autoloadValues = new BzlLoadValue[bzlAutoloadKeys.size()];
if (inliningState == null) {
exportsValue =
(BzlLoadValue)
env.getValueOrThrow(EXPORTS_ENTRYPOINT_KEY, BzlLoadFailedException.class);
SkyframeLookupResult values = env.getValuesAndExceptions(bzlAutoloadKeys);
for (int i = 0; i < bzlAutoloadKeys.size(); i++) {
autoloadValues[i] = (BzlLoadValue) values.getOrThrow(bzlAutoloadKeys.get(i),
BzlLoadFailedException.class);
}
} else {
exportsValue = bzlLoadFunction.computeInline(EXPORTS_ENTRYPOINT_KEY, inliningState);
for (int i = 0; i < bzlAutoloadKeys.size(); i++) {
autoloadValues[i] = bzlLoadFunction.computeInline(bzlAutoloadKeys.get(i),
inliningState);
}
}
} catch (BzlLoadFailedException ex) {
throw BuiltinsFailedException.errorEvaluatingBuiltinsBzls(ex);
}
if (exportsValue == null) {
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 (autoloadValues.length > 0) {
Fingerprint fp = new Fingerprint();
fp.addBytes(transitiveDigest);
for (BzlLoadValue value : autoloadValues) {
fp.addBytes(value.getTransitiveDigest());
}
transitiveDigest = fp.digestAndReset();
}

// Apply declarations of exports.bzl to the native predeclared symbols.
Module module = exportsValue.getModule();
try {
ImmutableMap<String, Object> exportedToplevels = getDict(module, "exported_toplevels");
Expand All @@ -212,6 +253,16 @@ 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<String, Object> newSymbols = autoloadsConfiguration.processLoads(autoloadValues);
predeclaredForBuild = autoloadsConfiguration.modifyBuildEnv(
isWithAutoloads,
predeclaredForBuild, newSymbols);
predeclaredForBuildBzl = autoloadsConfiguration.modifyBuildBzlEnv(
isWithAutoloads,
predeclaredForBuildBzl, newSymbols);

return StarlarkBuiltinsValue.create(
predeclaredForBuildBzl,
predeclaredForWorkspaceBzl,
Expand All @@ -221,6 +272,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);
}
}

Expand Down
Loading
Loading