Skip to content

Commit

Permalink
Respect Starlark builtins when dumping rules via info build-language
Browse files Browse the repository at this point in the history
Note that we can still trigger the legacy behavior of dumping only old native
rules by calling `info build-language --experimental_builtins_bzl_path=`

PiperOrigin-RevId: 606332328
Change-Id: I3cca300574bac01775603dd908445e297bfa7948
  • Loading branch information
tetromino authored and copybara-github committed Feb 12, 2024
1 parent 30b95e3 commit add245c
Show file tree
Hide file tree
Showing 6 changed files with 144 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,8 @@ public void throwPendingException() throws AbruptExitException {

/**
* Initializes and syncs the graph with the given options, readying it for the next evaluation.
*
* @throws IllegalStateException if the method has already been called in this environment.
*/
public void syncPackageLoading(OptionsProvider options)
throws InterruptedException, AbruptExitException {
Expand All @@ -760,6 +762,11 @@ public void syncPackageLoading(OptionsProvider options)
options);
}

/** Returns true if {@link #syncPackageLoading} has already been called. */
public boolean hasSyncedPackageLoading() {
return hasSyncedPackageLoading;
}

public void recordLastExecutionTime() {
workspace.recordLastExecutionTime(getCommandStartTime());
}
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/com/google/devtools/build/lib/runtime/InfoItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ public String getDescription() {
return description;
}

/**
* Returns true if this info item requires CommandEnvironment.syncPackageLoading to be called,
* e.g. in order to initialize the skyframe executor.
*
* <p>Virtually all info items do not need it.
*/
public boolean needsSyncPackageLoading() {
return false;
}

/**
* Whether the key is printed when "blaze info" is invoked without arguments.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@
import com.google.devtools.common.options.OptionEffectTag;
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParsingResult;
import com.google.devtools.common.options.OptionsProvider;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.List;
Expand Down Expand Up @@ -151,12 +152,12 @@ public BlazeCommandResult exec(
// In order to be able to answer configuration-specific queries, we need to set up
// the package path. Since info inherits all the build options, all the necessary
// information is available here.
env.syncPackageLoading(optionsParsingResult);
ensureSyncPackageLoading(env, optionsParsingResult);
// TODO(bazel-team): What if there are multiple configurations? [multi-config]
BuildOptions buildOptions = runtime.createBuildOptions(optionsParsingResult);
env.getSkyframeExecutor().setBaselineConfiguration(buildOptions);
return env.getSkyframeExecutor()
.getConfiguration(env.getReporter(), buildOptions, /*keepGoing=*/ true);
.getConfiguration(env.getReporter(), buildOptions, /* keepGoing= */ true);
} catch (InvalidConfigurationException e) {
env.getReporter().handle(Event.error(e.getMessage()));
throw new AbruptExitRuntimeException(e.getDetailedExitCode());
Expand Down Expand Up @@ -189,7 +190,11 @@ public BlazeCommandResult exec(
byte[] value;
if (items.containsKey(key)) {
try (SilentCloseable c = Profiler.instance().profile(key + ".infoItem")) {
value = items.get(key).get(configurationSupplier, env);
InfoItem infoItem = items.get(key);
if (infoItem.needsSyncPackageLoading()) {
ensureSyncPackageLoading(env, optionsParsingResult);
}
value = infoItem.get(configurationSupplier, env);
if (residue.size() > 1) {
outErr.getOutputStream().write((key + ": ").getBytes(StandardCharsets.UTF_8));
}
Expand Down Expand Up @@ -218,6 +223,9 @@ public BlazeCommandResult exec(
if (infoItem.isHidden()) {
continue;
}
if (infoItem.needsSyncPackageLoading()) {
ensureSyncPackageLoading(env, optionsParsingResult);
}
outErr.getOutputStream().write(
(infoItem.getName() + ": ").getBytes(StandardCharsets.UTF_8));
try (SilentCloseable c = Profiler.instance().profile(infoItem.getName() + ".infoItem")) {
Expand All @@ -242,6 +250,13 @@ public BlazeCommandResult exec(
return BlazeCommandResult.success();
}

private static void ensureSyncPackageLoading(CommandEnvironment env, OptionsProvider options)
throws InterruptedException, AbruptExitException {
if (!env.hasSyncedPackageLoading()) {
env.syncPackageLoading(options);
}
}

private static BlazeCommandResult createFailureResult(
String message, ExitCode exitCode, FailureDetails.InfoCommand.Code detailedCode) {
return BlazeCommandResult.detailedExitCode(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,20 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
"//src/main/java/com/google/devtools/build/lib/skyframe:starlark_builtins_value",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:debug-logger-configurator",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:heap_offset_helper",
"//src/main/java/com/google/devtools/build/lib/util:string",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/worker:worker_process_metrics",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/com/google/devtools/common/options",
"//src/main/java/net/starlark/java/eval",
"//src/main/protobuf:build_java_proto",
"//src/main/protobuf:failure_details_java_proto",
"//third_party:flogger",
"//third_party:guava",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,27 +20,36 @@
import com.google.common.base.Predicates;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.Attribute.StarlarkComputedDefaultTemplate;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.ProtoUtils;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.RuleFunction;
import com.google.devtools.build.lib.packages.TriState;
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.query2.proto.proto2api.Build.AllowedRuleClassInfo;
import com.google.devtools.build.lib.query2.proto.proto2api.Build.AttributeDefinition;
import com.google.devtools.build.lib.query2.proto.proto2api.Build.AttributeValue;
import com.google.devtools.build.lib.query2.proto.proto2api.Build.BuildLanguage;
import com.google.devtools.build.lib.query2.proto.proto2api.Build.RuleDefinition;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.InfoItem;
import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsValue;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Collection;
import java.util.Comparator;
import java.util.List;
import java.util.Map;
import net.starlark.java.eval.StarlarkFunction;
import net.starlark.java.eval.StarlarkInt;

/**
Expand All @@ -50,24 +59,88 @@
*/
@Deprecated
public final class BuildLanguageInfoItem extends InfoItem {

public BuildLanguageInfoItem() {
super("build-language", "A protobuffer with the build language structure", true);
}

@Override
public byte[] get(
Supplier<BuildConfigurationValue> configurationSupplier, CommandEnvironment env) {
public boolean needsSyncPackageLoading() {
// Requires CommandEnvironment.syncPackageLoading to be called in order to initialize the
// skyframe executor.
return true;
}

@Override
public byte[] get(Supplier<BuildConfigurationValue> configurationSupplier, CommandEnvironment env)
throws AbruptExitException {
checkNotNull(env);
return print(getBuildLanguageDefinition(env.getRuntime().getRuleClassProvider()));
StarlarkBuiltinsValue builtins = loadStarlarkBuiltins(env);
return print(getBuildLanguageDefinition(getRuleClasses(builtins, env)));
}

private StarlarkBuiltinsValue loadStarlarkBuiltins(CommandEnvironment env)
throws AbruptExitException {
EvaluationResult<SkyValue> result =
env.getSkyframeExecutor()
.evaluateSkyKeys(
env.getReporter(),
ImmutableList.of(StarlarkBuiltinsValue.key()),
/* keepGoing= */ false);
if (result.hasError()) {
throw new AbruptExitException(
DetailedExitCode.of(
FailureDetails.FailureDetail.newBuilder()
.setMessage("Failed to load Starlark builtins")
.setInfoCommand(FailureDetails.InfoCommand.getDefaultInstance())
.build()));
}
return (StarlarkBuiltinsValue) result.get(StarlarkBuiltinsValue.key());
}

private ImmutableList<RuleClass> getRuleClasses(
StarlarkBuiltinsValue builtins, CommandEnvironment env) {
ImmutableMap<String, RuleClass> nativeRuleClasses =
env.getRuntime().getRuleClassProvider().getRuleClassMap();

// The conditional for selecting whether or not to load symbols from @_builtins is the same as
// in PackageFunction.compileBuildFile
if (builtins
.starlarkSemantics
.get(BuildLanguageOptions.EXPERIMENTAL_BUILTINS_BZL_PATH)
.isEmpty()) {
return ImmutableList.sortedCopyOf(
Comparator.comparing(RuleClass::getName), nativeRuleClasses.values());
} else {
ImmutableList.Builder<RuleClass> ruleClasses = ImmutableList.builder();
for (Map.Entry<String, Object> entry : builtins.predeclaredForBuild.entrySet()) {
if (entry.getValue() instanceof RuleFunction) {
ruleClasses.add(((RuleFunction) entry.getValue()).getRuleClass());
} else if (entry.getValue() instanceof StarlarkFunction) {
if (nativeRuleClasses.containsKey(entry.getKey())) {
// entry.getValue() is a Starlark macro in @_builtins overriding a native rule. We
// cannot extract the macro's metadata (other than by, perhaps, parsing its Starlark
// docstring via starlark_doc_extract, but that does not have sufficient fidelity to
// get rule attribute metadata), so we extract it from the legacy rule instead.
// Note that we *cannot* rely on StarlarkFunction.getName() because under which the
// macro is defined may not match the name under which @_builtins exports it.
ruleClasses.add(nativeRuleClasses.get(entry.getKey()));
}
}
}
return ImmutableList.sortedCopyOf(
Comparator.comparing(RuleClass::getName), ruleClasses.build());
}
}

/** Returns a byte array containing a proto-buffer describing the build language. */
private static byte[] getBuildLanguageDefinition(RuleClassProvider provider) {
/**
* Returns a byte array containing a proto-buffer describing the build language.
*
* @param ruleClasses a sorted list of rule classes
*/
private static byte[] getBuildLanguageDefinition(ImmutableList<RuleClass> ruleClasses) {
BuildLanguage.Builder resultPb = BuildLanguage.newBuilder();
ImmutableList<RuleClass> sortedRuleClasses =
ImmutableList.sortedCopyOf(
Comparator.comparing(RuleClass::getName), provider.getRuleClassMap().values());
for (RuleClass ruleClass : sortedRuleClasses) {
for (RuleClass ruleClass : ruleClasses) {
if (isAbstractRule(ruleClass)) {
continue;
}
Expand Down Expand Up @@ -99,7 +172,7 @@ private static byte[] getBuildLanguageDefinition(RuleClassProvider provider) {
}
attrPb.setExecutable(attr.isExecutable());
if (BuildType.isLabelType(t)) {
attrPb.setAllowedRuleClasses(getAllowedRuleClasses(sortedRuleClasses, attr));
attrPb.setAllowedRuleClasses(getAllowedRuleClasses(ruleClasses, attr));
attrPb.setNodep(t.getLabelClass() == Type.LabelClass.NONDEP_REFERENCE);
}
rulePb.addAttribute(attrPb);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1928,30 +1928,11 @@ private EvaluationResult<SkyValue> evaluateSkyKeys(
return evaluateSkyKeys(eventHandler, skyKeys, false);
}

/** Evaluates sky keys that require action execution and returns their evaluation results. */
public EvaluationResult<SkyValue> evaluateSkyKeysWithExecution(
final Reporter reporter,
final Executor executor,
final Iterable<? extends SkyKey> skyKeys,
final OptionsProvider options,
final ActionCacheChecker actionCacheChecker,
final ActionOutputDirectoryHelper outputDirectoryHelper) {

prepareSkyframeActionExecutorForExecution(
reporter, executor, options, actionCacheChecker, outputDirectoryHelper);
try {
return evaluateSkyKeys(
reporter, skyKeys, options.getOptions(KeepGoingOption.class).keepGoing);
} finally {
cleanUpAfterSingleEvaluationWithActionExecution(reporter);
}
}

/**
* Evaluates the given sky keys, blocks, and returns their evaluation results. Enables/disables
* "keep going" on evaluation errors as specified.
*/
EvaluationResult<SkyValue> evaluateSkyKeys(
public EvaluationResult<SkyValue> evaluateSkyKeys(
final ExtendedEventHandler eventHandler,
final Iterable<? extends SkyKey> skyKeys,
final boolean keepGoing) {
Expand All @@ -1973,6 +1954,25 @@ EvaluationResult<SkyValue> evaluateSkyKeys(
return result;
}

/** Evaluates sky keys that require action execution and returns their evaluation results. */
public EvaluationResult<SkyValue> evaluateSkyKeysWithExecution(
final Reporter reporter,
final Executor executor,
final Iterable<? extends SkyKey> skyKeys,
final OptionsProvider options,
final ActionCacheChecker actionCacheChecker,
final ActionOutputDirectoryHelper outputDirectoryHelper) {

prepareSkyframeActionExecutorForExecution(
reporter, executor, options, actionCacheChecker, outputDirectoryHelper);
try {
return evaluateSkyKeys(
reporter, skyKeys, options.getOptions(KeepGoingOption.class).keepGoing);
} finally {
cleanUpAfterSingleEvaluationWithActionExecution(reporter);
}
}

private class EnableAnalysisScope implements AutoCloseable {
private EnableAnalysisScope() {
skyframeBuildView.enableAnalysis(true);
Expand Down

0 comments on commit add245c

Please sign in to comment.