diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java index 0e32500cf9f657..613a15187a6d47 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionException.java @@ -24,6 +24,7 @@ import com.google.devtools.build.lib.skyframe.DetailedException; import com.google.devtools.build.lib.util.DetailedExitCode; import com.google.devtools.build.lib.util.ExitCode; +import javax.annotation.Nullable; import net.starlark.java.syntax.Location; /** @@ -115,6 +116,47 @@ private static NestedSet rootCausesFromAction( detailedExitCode)); } + public static ActionExecutionException fromExecException(ExecException exception, Action action) { + return fromExecException(exception, null, action); + } + + /** + * Returns a new ActionExecutionException given an optional action subtask describing which part + * of the action failed (should be null for standard action failures). When appropriate (we use + * some heuristics to decide), produces an abbreviated message incorporating just the termination + * status if available. + * + * @param exception initial ExecException + * @param actionSubtask additional information about the action + * @param action failed action + * @return ActionExecutionException object describing the action failure + */ + public static ActionExecutionException fromExecException( + ExecException exception, @Nullable String actionSubtask, Action action) { + // Message from ActionExecutionException will be prepended with action.describe() where + // necessary: because not all ActionExecutionExceptions come from this codepath, it is safer + // for consumers to manually prepend. We still put action.describe() in the failure detail + // message argument. + String message = + (actionSubtask == null ? "" : actionSubtask + ": ") + + exception.getMessageForActionExecutionException(); + + DetailedExitCode code = + DetailedExitCode.of(exception.getFailureDetail(action.describe() + " failed: " + message)); + + if (exception instanceof LostInputsExecException) { + return ((LostInputsExecException) exception).fromExecException(message, action, code); + } + + return fromExecException(exception, message, action, code); + } + + public static ActionExecutionException fromExecException( + ExecException exception, String message, Action action, DetailedExitCode code) { + return new ActionExecutionException( + message, exception, action, exception.isCatastrophic(), code); + } + /** Returns the action that failed. */ public ActionAnalysisMetadata getAction() { return action; diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index 8ae7bb4d2d2779..5f2f6a06bc180e 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -297,8 +297,7 @@ java_library( "ResourceSetOrBuilder.java", ], deps = [ - ":artifacts", - "//src/main/java/com/google/devtools/build/lib/collect/nestedset", + ":exec_exception", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/jni", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", @@ -344,3 +343,12 @@ java_library( "//third_party:jsr305", ], ) + +java_library( + name = "exec_exception", + srcs = [ + "ExecException.java", + "UserExecException.java", + ], + deps = ["//src/main/protobuf:failure_details_java_proto"], +) diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java index 36ab4aded7ad8b..89d55f1726c912 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java @@ -20,6 +20,7 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.analysis.platform.PlatformInfo; import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.List; import java.util.Map; @@ -35,7 +36,7 @@ public class BaseSpawn implements Spawn { private final ImmutableMap executionInfo; private final RunfilesSupplier runfilesSupplier; private final ActionExecutionMetadata action; - private final ResourceSet localResources; + private final ResourceSetOrBuilder localResources; public BaseSpawn( List arguments, @@ -43,7 +44,7 @@ public BaseSpawn( Map executionInfo, RunfilesSupplier runfilesSupplier, ActionExecutionMetadata action, - ResourceSet localResources) { + ResourceSetOrBuilder localResources) { this.arguments = ImmutableList.copyOf(arguments); this.environment = ImmutableMap.copyOf(environment); this.executionInfo = ImmutableMap.copyOf(executionInfo); @@ -123,8 +124,9 @@ public ActionExecutionMetadata getResourceOwner() { } @Override - public ResourceSet getLocalResources() { - return localResources; + public ResourceSet getLocalResources() throws ExecException { + return localResources.buildResourceSet( + OS.getCurrent(), action.getInputs().memoizedFlattenAndGetSize()); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java index 112452344d3d26..1cc002e666bd78 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java @@ -84,7 +84,7 @@ public ActionExecutionMetadata getResourceOwner() { } @Override - public ResourceSet getLocalResources() { + public ResourceSet getLocalResources() throws ExecException { return spawn.getLocalResources(); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ExecException.java b/src/main/java/com/google/devtools/build/lib/actions/ExecException.java index 43ba000e3632f3..c3544877031246 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ExecException.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ExecException.java @@ -15,9 +15,6 @@ package com.google.devtools.build.lib.actions; import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; -import com.google.devtools.build.lib.util.DetailedExitCode; -import com.google.errorprone.annotations.ForOverride; -import javax.annotation.Nullable; /** * An exception indication that the execution of an action has failed OR could not be attempted OR @@ -82,52 +79,9 @@ public boolean isCatastrophic() { return catastrophe; } - /** - * Returns a new ActionExecutionException without a message prefix. - * - * @param action failed action - * @return ActionExecutionException object describing the action failure - */ - public final ActionExecutionException toActionExecutionException(Action action) { - return toActionExecutionException(null, action); - } - - /** - * Returns a new ActionExecutionException given an optional action subtask describing which part - * of the action failed (should be null for standard action failures). When appropriate (we use - * some heuristics to decide), produces an abbreviated message incorporating just the termination - * status if available. - * - * @param actionSubtask additional information about the action - * @param action failed action - * @return ActionExecutionException object describing the action failure - */ - public final ActionExecutionException toActionExecutionException( - @Nullable String actionSubtask, Action action) { - // Message from ActionExecutionException will be prepended with action.describe() where - // necessary: because not all ActionExecutionExceptions come from this codepath, it is safer - // for consumers to manually prepend. We still put action.describe() in the failure detail - // message argument. - String message = - (actionSubtask == null ? "" : actionSubtask + ": ") - + getMessageForActionExecutionException(); - return toActionExecutionException( - message, - action, - DetailedExitCode.of(getFailureDetail(action.describe() + " failed: " + message))); - } - - @ForOverride - protected ActionExecutionException toActionExecutionException( - String message, Action action, DetailedExitCode code) { - return new ActionExecutionException(message, this, action, isCatastrophic(), code); - } - - @ForOverride protected String getMessageForActionExecutionException() { return getMessage(); } - @ForOverride protected abstract FailureDetail getFailureDetail(String message); } diff --git a/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java b/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java index 0be336183d5bef..f992d5d3543e7f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java +++ b/src/main/java/com/google/devtools/build/lib/actions/LostInputsExecException.java @@ -64,8 +64,7 @@ public ActionInputDepOwners getOwners() { return owners; } - @Override - protected ActionExecutionException toActionExecutionException( + protected ActionExecutionException fromExecException( String message, Action action, DetailedExitCode code) { return new LostInputsActionExecutionException( message, lostInputs, owners, action, /*cause=*/ this, code); diff --git a/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java b/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java index b62f74d416907a..9fe4870b8de063 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ResourceSet.java @@ -16,8 +16,8 @@ import com.google.common.base.Splitter; import com.google.common.primitives.Doubles; -import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.common.options.Converter; import com.google.devtools.common.options.OptionsParsingException; @@ -175,7 +175,7 @@ public String getTypeDescription() { } @Override - public ResourceSet buildResourceSet(NestedSet inputs) { + public ResourceSet buildResourceSet(OS os, int inputsSize) throws ExecException { return this; } } diff --git a/src/main/java/com/google/devtools/build/lib/actions/ResourceSetOrBuilder.java b/src/main/java/com/google/devtools/build/lib/actions/ResourceSetOrBuilder.java index fee458896a1e6d..71242c8d8ee7fc 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ResourceSetOrBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ResourceSetOrBuilder.java @@ -14,7 +14,7 @@ package com.google.devtools.build.lib.actions; -import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.util.OS; /** Common interface for ResourceSet and builder. */ @FunctionalInterface @@ -24,5 +24,5 @@ public interface ResourceSetOrBuilder { * will flatten NestedSet. This action could create a lot of garbagge, so use it as close as * possible to the execution phase, */ - public ResourceSet buildResourceSet(NestedSet inputs); + public ResourceSet buildResourceSet(OS os, int inputsSize) throws ExecException; } diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java index cf47e4203f63ff..3b55e26e1a34a0 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java +++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java @@ -130,10 +130,8 @@ default boolean isMandatoryOutput(ActionInput output) { /** Returns the resource owner for local fallback. */ ActionExecutionMetadata getResourceOwner(); - /** - * Returns the amount of resources needed for local fallback. - */ - ResourceSet getLocalResources(); + /** Returns the amount of resources needed for local fallback. */ + ResourceSet getLocalResources() throws ExecException; /** * Returns a mnemonic (string constant) for this kind of spawn. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/AbstractFileWriteAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/AbstractFileWriteAction.java index 170f97cafd2f49..a0974b7813d162 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/AbstractFileWriteAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/AbstractFileWriteAction.java @@ -89,16 +89,14 @@ public ActionContinuationOrResult execute() return this; } } catch (ExecException e) { - throw e.toActionExecutionException( - AbstractFileWriteAction.this); + throw ActionExecutionException.fromExecException(e, AbstractFileWriteAction.this); } afterWrite(actionExecutionContext); return ActionContinuationOrResult.of(ActionResult.create(nextContinuation.get())); } }; } catch (ExecException e) { - throw e.toActionExecutionException( - this); + throw ActionExecutionException.fromExecException(e, this); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java index ca3c553479c915..402b418a994266 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java @@ -318,7 +318,7 @@ public final ActionContinuationOrResult beginExecution( beforeExecute(actionExecutionContext); spawn = getSpawn(actionExecutionContext); } catch (ExecException e) { - throw e.toActionExecutionException(this); + throw ActionExecutionException.fromExecException(e, this); } catch (CommandLineExpansionException e) { throw createCommandLineException(e); } @@ -590,8 +590,7 @@ private ActionSpawn( executionInfo, SpawnAction.this.getRunfilesSupplier(), SpawnAction.this, - SpawnAction.this.resourceSetOrBuilder.buildResourceSet(inputs)); - + SpawnAction.this.resourceSetOrBuilder); NestedSetBuilder inputsBuilder = NestedSetBuilder.stableOrder(); ImmutableList manifests = getRunfilesSupplier().getManifests(); for (Artifact input : inputs.toList()) { @@ -1428,7 +1427,7 @@ public ActionContinuationOrResult execute() } return new SpawnActionContinuation(actionExecutionContext, nextContinuation); } catch (ExecException e) { - throw e.toActionExecutionException(SpawnAction.this); + throw ActionExecutionException.fromExecException(e, SpawnAction.this); } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java index b84902218fd17f..5ac3c526eb5b78 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/TemplateExpansionAction.java @@ -164,8 +164,7 @@ public ActionContinuationOrResult execute() return this; } } catch (ExecException e) { - throw e.toActionExecutionException( - TemplateExpansionAction.this); + throw ActionExecutionException.fromExecException(e, TemplateExpansionAction.this); } return ActionContinuationOrResult.of(ActionResult.create(nextContinuation.get())); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java index 5fb62d507dddde..de77b7e696a5d5 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkActionFactory.java @@ -15,8 +15,10 @@ import static com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions.EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.Action; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.ActionLookupKey; @@ -24,8 +26,12 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.ArtifactRoot; import com.google.devtools.build.lib.actions.CommandLine; +import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ParamFileInfo; +import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.actions.ResourceSetOrBuilder; import com.google.devtools.build.lib.actions.RunfilesSupplier; +import com.google.devtools.build.lib.actions.UserExecException; import com.google.devtools.build.lib.actions.extra.ExtraActionInfo; import com.google.devtools.build.lib.actions.extra.SpawnInfo; import com.google.devtools.build.lib.analysis.BashCommandConstructor; @@ -49,23 +55,35 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions; +import com.google.devtools.build.lib.server.FailureDetails; +import com.google.devtools.build.lib.server.FailureDetails.FailureDetail; +import com.google.devtools.build.lib.server.FailureDetails.Interrupted; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant; import com.google.devtools.build.lib.starlarkbuildapi.FileApi; import com.google.devtools.build.lib.starlarkbuildapi.StarlarkActionFactoryApi; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.protobuf.GeneratedMessage; import java.nio.charset.StandardCharsets; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.UUID; import net.starlark.java.eval.Dict; import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.Mutability; import net.starlark.java.eval.Printer; import net.starlark.java.eval.Sequence; import net.starlark.java.eval.Starlark; +import net.starlark.java.eval.StarlarkCallable; +import net.starlark.java.eval.StarlarkFloat; +import net.starlark.java.eval.StarlarkFunction; +import net.starlark.java.eval.StarlarkInt; import net.starlark.java.eval.StarlarkSemantics; import net.starlark.java.eval.StarlarkThread; @@ -75,6 +93,10 @@ public class StarlarkActionFactory implements StarlarkActionFactoryApi { /** Counter for actions.run_shell helper scripts. Every script must have a unique name. */ private int runShellOutputCounter = 0; + private static final ResourceSet DEFAULT_RESOURCE_SET = ResourceSet.createWithRamCpu(250, 1); + private static final Set validResources = + new HashSet<>(Arrays.asList("cpu", "memory", "local_test")); + public StarlarkActionFactory(StarlarkRuleContext context) { this.context = context; } @@ -339,7 +361,8 @@ public void run( Object executionRequirementsUnchecked, Object inputManifestsUnchecked, Object execGroupUnchecked, - Object shadowedActionUnchecked) + Object shadowedActionUnchecked, + Object resourceSetUnchecked) throws EvalException { context.checkMutable("actions.run"); @@ -377,6 +400,7 @@ public void run( inputManifestsUnchecked, execGroupUnchecked, shadowedActionUnchecked, + resourceSetUnchecked, builder); } @@ -430,7 +454,8 @@ public void runShell( Object executionRequirementsUnchecked, Object inputManifestsUnchecked, Object execGroupUnchecked, - Object shadowedActionUnchecked) + Object shadowedActionUnchecked, + Object resourceSetUnchecked) throws EvalException { context.checkMutable("actions.run_shell"); RuleContext ruleContext = getRuleContext(); @@ -497,6 +522,7 @@ public void runShell( inputManifestsUnchecked, execGroupUnchecked, shadowedActionUnchecked, + resourceSetUnchecked, builder); } @@ -543,6 +569,7 @@ private void registerStarlarkAction( Object inputManifestsUnchecked, Object execGroupUnchecked, Object shadowedActionUnchecked, + Object resourceSetUnchecked, StarlarkAction.Builder builder) throws EvalException { if (inputs instanceof Sequence) { @@ -648,10 +675,127 @@ private void registerStarlarkAction( builder.setShadowedAction(Optional.of((Action) shadowedActionUnchecked)); } + if (getSemantics().getBool(BuildLanguageOptions.EXPERIMENTAL_ACTION_RESOURCE_SET) + && resourceSetUnchecked != Starlark.NONE) { + validateResourceSetBuilder(resourceSetUnchecked); + builder.setResources( + new StarlarkActionResourceSetBuilder( + (StarlarkFunction) resourceSetUnchecked, mnemonic, getSemantics())); + } + // Always register the action registerAction(builder.build(ruleContext)); } + private static class StarlarkActionResourceSetBuilder implements ResourceSetOrBuilder { + private final StarlarkCallable fn; + private final String mnemonic; + private final StarlarkSemantics semantics; + + public StarlarkActionResourceSetBuilder( + StarlarkCallable fn, String mnemonic, StarlarkSemantics semantics) { + this.fn = fn; + this.mnemonic = mnemonic; + this.semantics = semantics; + } + + @Override + public ResourceSet buildResourceSet(OS os, int inputsSize) throws ExecException { + try (Mutability mu = Mutability.create("resource_set_builder_function")) { + StarlarkThread thread = new StarlarkThread(mu, semantics); + StarlarkInt inputInt = StarlarkInt.of(inputsSize); + Object response = + Starlark.call( + thread, + this.fn, + ImmutableList.of(os.getCanonicalName(), inputInt), + ImmutableMap.of()); + Map resourceSetMapRaw = + Dict.cast(response, String.class, Object.class, "resource_set"); + + if (!validResources.containsAll(resourceSetMapRaw.keySet())) { + String message = + String.format( + "Illegal resource keys: (%s)", + Joiner.on(",").join(Sets.difference(resourceSetMapRaw.keySet(), validResources))); + throw new EvalException(message); + } + + return ResourceSet.create( + getNumericOrDefault(resourceSetMapRaw, "memory", DEFAULT_RESOURCE_SET.getMemoryMb()), + getNumericOrDefault(resourceSetMapRaw, "cpu", DEFAULT_RESOURCE_SET.getCpuUsage()), + (int) + getNumericOrDefault( + resourceSetMapRaw, + "local_test", + (double) DEFAULT_RESOURCE_SET.getLocalTestCount())); + } catch (EvalException e) { + throw new UserExecException( + FailureDetail.newBuilder() + .setMessage( + String.format("Could not build resources for %s. %s", mnemonic, e.getMessage())) + .setStarlarkAction( + FailureDetails.StarlarkAction.newBuilder() + .setCode(FailureDetails.StarlarkAction.Code.STARLARK_ACTION_UNKNOWN) + .build()) + .build()); + } catch (InterruptedException e) { + throw new UserExecException( + FailureDetail.newBuilder() + .setMessage(e.getMessage()) + .setInterrupted( + Interrupted.newBuilder().setCode(Interrupted.Code.INTERRUPTED).build()) + .build()); + } + } + + private static double getNumericOrDefault( + Map resourceSetMap, String key, double defaultValue) throws EvalException { + if (!resourceSetMap.containsKey(key)) { + return defaultValue; + } + + Object value = resourceSetMap.get(key); + if (value instanceof StarlarkInt) { + return ((StarlarkInt) value).toDouble(); + } + + if (value instanceof StarlarkFloat) { + return ((StarlarkFloat) value).toDouble(); + } + throw new EvalException( + String.format( + "Illegal resource value type for key %s: got %s, want int or float", + key, Starlark.type(value))); + } + } + + private static StarlarkFunction validateResourceSetBuilder(Object fn) throws EvalException { + if (!(fn instanceof StarlarkFunction)) { + throw Starlark.errorf( + "resource_set should be a Starlark-defined function, but got %s instead", + Starlark.type(fn)); + } + + StarlarkFunction sfn = (StarlarkFunction) fn; + + // Reject non-global functions, because arbitrary closures may cause large + // analysis-phase data structures to remain live into the execution phase. + // We require that the function is "global" as opposed to "not a closure" + // because a global function may be closure if it refers to load bindings. + // This unfortunately disallows such trivially safe non-global + // functions as "lambda x: x". + // See https://github.com/bazelbuild/bazel/issues/12701. + if (sfn.getModule().getGlobal(sfn.getName()) != sfn) { + throw Starlark.errorf( + "to avoid unintended retention of analysis data structures, " + + "the resource_set function (declared at %s) must be declared " + + "by a top-level def statement", + sfn.getLocation()); + } + return (StarlarkFunction) fn; + } + private String getMnemonic(Object mnemonicUnchecked) { String mnemonic = mnemonicUnchecked == Starlark.NONE ? "Action" : (String) mnemonicUnchecked; if (getRuleContext().getConfiguration().getReservedActionMnemonics().contains(mnemonic)) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java index 01795eb8658a6b..2c037f6996f940 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java @@ -899,10 +899,10 @@ public ActionContinuationOrResult beginExecution( testActionContext.isTestKeepGoing(), cancelFuture); } catch (ExecException e) { - throw e.toActionExecutionException(this); + throw ActionExecutionException.fromExecException(e, this); } catch (IOException e) { - throw new EnvironmentalExecException(e, Code.TEST_RUNNER_IO_EXCEPTION) - .toActionExecutionException(this); + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException(e, Code.TEST_RUNNER_IO_EXCEPTION), this); } } @@ -1203,10 +1203,11 @@ public ActionContinuationOrResult execute() Preconditions.checkState(actualMaxAttempts != 0); return process(result, actualMaxAttempts); } catch (ExecException e) { - throw e.toActionExecutionException(this.testRunnerAction); + throw ActionExecutionException.fromExecException(e, this.testRunnerAction); } catch (IOException e) { - throw new EnvironmentalExecException(e, Code.TEST_RUNNER_IO_EXCEPTION) - .toActionExecutionException(this.testRunnerAction); + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException(e, Code.TEST_RUNNER_IO_EXCEPTION), + this.testRunnerAction); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java index 6e09f9743caeef..242e45a2eac57a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/coverage/CoverageReportActionBuilder.java @@ -65,29 +65,29 @@ /** * A class to create the coverage report generator action. - * - *

The coverage report action is created after every test shard action is created, at the - * very end of the analysis phase. There is only one coverage report action per coverage - * command invocation. It can also be viewed as a single sink node of the action graph. - * + * + *

The coverage report action is created after every test shard action is created, at the very + * end of the analysis phase. There is only one coverage report action per coverage command + * invocation. It can also be viewed as a single sink node of the action graph. + * *

Its inputs are the individual coverage.dat files from the test outputs (each shard produces - * one) and the baseline coverage artifacts. Note that each ConfiguredTarget among the - * transitive dependencies of the top level test targets may provide baseline coverage artifacts. - * + * one) and the baseline coverage artifacts. Note that each ConfiguredTarget among the transitive + * dependencies of the top level test targets may provide baseline coverage artifacts. + * *

The coverage report generation can have two phases, though they both run in the same action. * The source code of the coverage report tool {@code lcov_merger} is in the {@code - * testing/coverage/lcov_merger} directory. The deployed binaries used by Blaze are under - * {@code tools/coverage}. - * + * testing/coverage/lcov_merger} directory. The deployed binaries used by Blaze are under {@code + * tools/coverage}. + * *

The first phase is merging the individual coverage files into a single report file. The * location of this file is reported by Blaze. This phase always happens if the {@code * --combined_report=lcov} or {@code --combined_report=html}. - * + * *

The second phase is generating an html report. It only happens if {@code - * --combined_report=html}. The action generates an html output file potentially for every - * tested source file into the report. Since this set of files is unknown in the analysis - * phase (the tool figures it out from the contents of the merged coverage report file) - * the action always runs locally when {@code --combined_report=html}. + * --combined_report=html}. The action generates an html output file potentially for every tested + * source file into the report. Since this set of files is unknown in the analysis phase (the tool + * figures it out from the contents of the merged coverage report file) the action always runs + * locally when {@code --combined_report=html}. */ public final class CoverageReportActionBuilder { @@ -148,7 +148,7 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext) actionExecutionContext.getEventHandler().handle(Event.info(locationMessage)); return ActionResult.create(spawnResults); } catch (ExecException e) { - throw e.toActionExecutionException(this); + throw ActionExecutionException.fromExecException(e, this); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java index b5fe5a7b1d4566..3504120aad1edb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaLiteProtoLibraryRule.java @@ -58,9 +58,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi .aspect(javaProtoAspect)) .add( attr(JavaProtoAspectCommon.LITE_PROTO_TOOLCHAIN_ATTR, LABEL) - .mandatoryBuiltinProviders( - ImmutableList.>of( - ProtoLangToolchainProvider.class)) + .mandatoryProviders(ProtoLangToolchainProvider.PROVIDER_ID) .value(getProtoToolchainLabel(DEFAULT_PROTO_TOOLCHAIN_LABEL))) .advertiseStarlarkProvider(StarlarkProviderIdentifier.forKey(JavaInfo.PROVIDER.getKey())) .build(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoAspect.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoAspect.java index d74eb5f097ae80..4d82d27032a3b2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/proto/BazelJavaProtoAspect.java @@ -16,6 +16,7 @@ import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER; +import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; @@ -26,9 +27,11 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.packages.AspectDefinition; import com.google.devtools.build.lib.packages.AspectParameters; +import com.google.devtools.build.lib.packages.StarlarkInfo; import com.google.devtools.build.lib.rules.java.proto.JavaProtoAspect; import com.google.devtools.build.lib.rules.java.proto.RpcSupport; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder; +import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ToolchainInvocation; import java.util.List; /** An Aspect which BazelJavaProtoLibrary injects to build Java SPEED protos. */ @@ -45,7 +48,7 @@ public BazelJavaProtoAspect(RuleDefinitionEnvironment env) { private static class NoopRpcSupport implements RpcSupport { @Override - public List getToolchainInvocation( + public List getToolchainInvocation( RuleContext ruleContext, Artifact sourceJar) { return ImmutableList.of(); } @@ -56,8 +59,8 @@ public boolean allowServices(RuleContext ruleContext) { } @Override - public NestedSet getForbiddenProtos(RuleContext ruleContext) { - return NestedSetBuilder.emptySet(STABLE_ORDER); + public Optional getToolchain(RuleContext ruleContext) { + return Optional.absent(); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java index de90cc6e754810..fc5dc61676466f 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeStrategy.java @@ -116,8 +116,8 @@ public void createSymlinks( .createSymlinksDirectly( action.getOutputManifest().getPath().getParentDirectory(), runfiles); } catch (IOException e) { - throw new EnvironmentalExecException(e, Code.SYMLINK_TREE_CREATION_IO_EXCEPTION) - .toActionExecutionException(action); + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException(e, Code.SYMLINK_TREE_CREATION_IO_EXCEPTION), action); } Path inputManifest = @@ -136,7 +136,7 @@ public void createSymlinks( actionExecutionContext.getFileOutErr()); } } catch (ExecException e) { - throw e.toActionExecutionException(action); + throw ActionExecutionException.fromExecException(e, action); } } } 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 8373d4fe87fb20..dba3e3827b4fdc 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 @@ -283,6 +283,20 @@ public class BuildLanguageOptions extends OptionsBase implements Serializable { + " https://github.com/bazelbuild/bazel/issues/8830 for details.") public boolean experimentalAllowTagsPropagation; + @Option( + name = "experimental_action_resource_set", + defaultValue = "true", + documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS, + effectTags = {OptionEffectTag.EXECUTION, OptionEffectTag.BUILD_FILE_SEMANTICS}, + metadataTags = { + OptionMetadataTag.EXPERIMENTAL, + }, + help = + "If set to true, ctx.actions.run() and ctx.actions.run_shell() accept a resource_set" + + " parameter for local execution. Otherwise it will default to 250 MB for memory" + + " and 1 cpu.") + public boolean experimentalActionResourceSet; + @Option( name = "incompatible_struct_has_no_methods", defaultValue = "false", @@ -571,6 +585,7 @@ public StarlarkSemantics toStarlarkSemantics() { .setBool(INCOMPATIBLE_ENABLE_EXPORTS_PROVIDER, incompatibleEnableExportsProvider) .setBool( INCOMPATIBLE_EXISTING_RULES_IMMUTABLE_VIEW, incompatibleExistingRulesImmutableView) + .setBool(EXPERIMENTAL_ACTION_RESOURCE_SET, experimentalActionResourceSet) .setBool(EXPERIMENTAL_GOOGLE_LEGACY_API, experimentalGoogleLegacyApi) .setBool(EXPERIMENTAL_NINJA_ACTIONS, experimentalNinjaActions) .setBool(EXPERIMENTAL_PLATFORMS_API, experimentalPlatformsApi) @@ -646,6 +661,7 @@ public StarlarkSemantics toStarlarkSemantics() { public static final String EXPERIMENTAL_REPO_REMOTE_EXEC = "-experimental_repo_remote_exec"; public static final String EXPERIMENTAL_SIBLING_REPOSITORY_LAYOUT = "-experimental_sibling_repository_layout"; + public static final String EXPERIMENTAL_ACTION_RESOURCE_SET = "+experimental_action_resource_set"; public static final String INCOMPATIBLE_ALLOW_TAGS_PROPAGATION = "-incompatible_allow_tags_propagation"; public static final String INCOMPATIBLE_ALWAYS_CHECK_DEPSET_ELEMENTS = diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java index f81fbd534e3be1..ac9a96be504e6c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/DexArchiveAspect.java @@ -176,11 +176,11 @@ public AspectDefinition getDefinition(AspectParameters params) { // For android_sdk rules, where we just want to get at aidl runtime deps. .requireStarlarkProviders(forKey(AndroidSdkProvider.PROVIDER.getKey())) .requireStarlarkProviders(forKey(ProtoInfo.PROVIDER.getKey())) - .requireProviderSets( + .requireStarlarkProviderSets( ImmutableList.of( // For proto_lang_toolchain rules, where we just want to get at their runtime // deps. - ImmutableSet.of(ProtoLangToolchainProvider.class))) + ImmutableSet.of(ProtoLangToolchainProvider.PROVIDER_ID))) .addRequiredToolchains( Label.parseAbsoluteUnchecked(toolsRepository + sdkToolchainLabel)) // Parse labels since we don't have RuleDefinitionEnvironment.getLabel like in a rule diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index ed39beb9d8c965..e713a5eb1e96b6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -478,7 +478,7 @@ private NestedSet findUsedHeaders( createFailureDetail("Find used headers failure", Code.FIND_USED_HEADERS_IO_EXCEPTION)); } } catch (ExecException e) { - throw e.toActionExecutionException("include scanning", this); + throw ActionExecutionException.fromExecException(e, "include scanning", this); } } @@ -1853,7 +1853,7 @@ public ActionContinuationOrResult execute() dotDContents = getDotDContents(spawnResults.get(0)); } catch (ExecException e) { copyTempOutErrToActionOutErr(); - throw e.toActionExecutionException(CppCompileAction.this); + throw ActionExecutionException.fromExecException(e, CppCompileAction.this); } catch (InterruptedException e) { copyTempOutErrToActionOutErr(); throw e; @@ -1931,9 +1931,10 @@ private void copyTempOutErrToActionOutErr() throws ActionExecutionException { } } } catch (IOException e) { - throw new EnvironmentalExecException( - e, createFailureDetail("OutErr copy failure", Code.COPY_OUT_ERR_FAILURE)) - .toActionExecutionException(CppCompileAction.this); + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException( + e, createFailureDetail("OutErr copy failure", Code.COPY_OUT_ERR_FAILURE)), + CppCompileAction.this); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java index e5a43dd72974bb..d8e92b262b8b6b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkAction.java @@ -111,7 +111,7 @@ public Artifact create( }; private static final String LINK_GUID = "58ec78bd-1176-4e36-8143-439f656b181d"; - + @Nullable private final String mnemonic; private final LibraryToLink outputLibrary; private final Artifact linkOutput; @@ -453,8 +453,7 @@ public ActionContinuationOrResult execute() } return ActionContinuationOrResult.of(ActionResult.create(nextContinuation.get())); } catch (ExecException e) { - throw e.toActionExecutionException( - CppLinkAction.this); + throw ActionExecutionException.fromExecException(e, CppLinkAction.this); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java index 689d29f2623087..34333475528fd2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/proto/CcProtoAspect.java @@ -41,6 +41,7 @@ import com.google.devtools.build.lib.packages.Attribute.LabelLateBoundDefault; import com.google.devtools.build.lib.packages.NativeAspectClass; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; +import com.google.devtools.build.lib.packages.StarlarkInfo; import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.rules.cpp.AspectLegalCppSemantics; import com.google.devtools.build.lib.rules.cpp.CcCommon; @@ -127,7 +128,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParameters) { .useToolchainTransition(true) .add( attr(PROTO_TOOLCHAIN_ATTR, LABEL) - .mandatoryBuiltinProviders(ImmutableList.of(ProtoLangToolchainProvider.class)) + .mandatoryProviders(ProtoLangToolchainProvider.PROVIDER_ID) .value(PROTO_TOOLCHAIN_LABEL)) .add( attr(CcToolchain.CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME, LABEL) @@ -284,7 +285,7 @@ private static void checkProtoLibrariesInDeps( private boolean areSrcsExcluded() { return !new ProtoSourceFileExcludeList( - ruleContext, getProtoToolchainProvider().forbiddenProtos()) + ruleContext, getProtoToolchainProvider().providedProtoSources()) .checkSrcs(protoInfo.getDirectSources(), "cc_proto_library"); } @@ -449,7 +450,6 @@ private void createProtoCompileAction(Collection outputs) { } else { genfilesPath = genfilesFragment.getRelative(protoRootFragment).getPathString(); } - ImmutableList.Builder invocations = ImmutableList.builder(); invocations.add( new ToolchainInvocation("C++", checkNotNull(getProtoToolchainProvider()), genfilesPath)); @@ -465,7 +465,7 @@ private void createProtoCompileAction(Collection outputs) { } private ProtoLangToolchainProvider getProtoToolchainProvider() { - return ruleContext.getPrerequisite(PROTO_TOOLCHAIN_ATTR, ProtoLangToolchainProvider.class); + return ProtoLangToolchainProvider.get(ruleContext, PROTO_TOOLCHAIN_ATTR); } public void addProviders(ConfiguredAspect.Builder builder) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/BUILD b/src/main/java/com/google/devtools/build/lib/rules/java/BUILD index fc4c128971aa2b..caae66d357a896 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/java/BUILD @@ -105,6 +105,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/packages", "//src/main/java/com/google/devtools/build/lib/rules/proto", + "//src/main/java/net/starlark/java/eval", "//third_party:guava", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java index e34e51d01b738e..d8c80a99cd4a9f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java @@ -677,9 +677,10 @@ private Deps.Dependencies readFullOutputDeps( return createFullOutputDeps( Iterables.getOnlyElement(results), outputDepsProto, getInputs(), actionExecutionContext); } catch (IOException e) { - throw new EnvironmentalExecException( - e, createFailureDetail(".jdeps read IOException", Code.JDEPS_READ_IO_EXCEPTION)) - .toActionExecutionException(this); + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException( + e, createFailureDetail(".jdeps read IOException", Code.JDEPS_READ_IO_EXCEPTION)), + this); } } @@ -755,12 +756,13 @@ public ActionContinuationOrResult execute() // We don't create any tree artifacts anyway. /*cleanupArchivedArtifacts=*/ false); } catch (IOException e) { - throw new EnvironmentalExecException( + throw ActionExecutionException.fromExecException( + new EnvironmentalExecException( e, createFailureDetail( "Failed to delete reduced action outputs", - Code.REDUCED_CLASSPATH_FALLBACK_CLEANUP_FAILURE)) - .toActionExecutionException(JavaCompileAction.this); + Code.REDUCED_CLASSPATH_FALLBACK_CLEANUP_FAILURE)), + JavaCompileAction.this); } actionExecutionContext.getMetadataHandler().resetOutputs(getOutputs()); Spawn spawn; @@ -777,7 +779,7 @@ public ActionContinuationOrResult execute() return new JavaFallbackActionContinuation( actionExecutionContext, results, fallbackContinuation); } catch (ExecException e) { - throw e.toActionExecutionException(JavaCompileAction.this); + throw ActionExecutionException.fromExecException(e, JavaCompileAction.this); } } } @@ -821,7 +823,7 @@ public ActionContinuationOrResult execute() ActionResult.create( ImmutableList.copyOf(Iterables.concat(primaryResults, fallbackResults)))); } catch (ExecException e) { - throw e.toActionExecutionException(JavaCompileAction.this); + throw ActionExecutionException.fromExecException(e, JavaCompileAction.this); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java index f504e8c5b99d52..80a5411a7f95ef 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaLiteProtoAspect.java @@ -111,9 +111,7 @@ public AspectDefinition getDefinition(AspectParameters aspectParameters) { ImmutableList.of(StarlarkProviderIdentifier.forKey(JavaInfo.PROVIDER.getKey()))) .add( attr(JavaProtoAspectCommon.LITE_PROTO_TOOLCHAIN_ATTR, LABEL) - .mandatoryBuiltinProviders( - ImmutableList.>of( - ProtoLangToolchainProvider.class)) + .mandatoryProviders(ProtoLangToolchainProvider.PROVIDER_ID) .value(getProtoToolchainLabel(defaultProtoToolchainLabel))) .add( attr(JavaRuleClasses.JAVA_TOOLCHAIN_ATTRIBUTE_NAME, LABEL) diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspectCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspectCommon.java index dedfb0308ba994..2f9021e499c59a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspectCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoAspectCommon.java @@ -21,7 +21,6 @@ import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.StrictDepsMode; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts; import com.google.devtools.build.lib.rules.java.JavaInfo; @@ -158,15 +157,14 @@ public ImmutableList getProtoRuntimeDeps() { /** Returns the toolchain that specifies how to generate code from {@code .proto} files. */ public ProtoLangToolchainProvider getProtoToolchainProvider() { - return checkNotNull( - ruleContext.getPrerequisite(protoToolchainAttr, ProtoLangToolchainProvider.class)); + return checkNotNull(ProtoLangToolchainProvider.get(ruleContext, protoToolchainAttr)); } /** * Returns the toolchain that specifies how to generate Java-lite code from {@code .proto} files. */ static ProtoLangToolchainProvider getLiteProtoToolchainProvider(RuleContext ruleContext) { - return ruleContext.getPrerequisite(LITE_PROTO_TOOLCHAIN_ATTR, ProtoLangToolchainProvider.class); + return ProtoLangToolchainProvider.get(ruleContext, LITE_PROTO_TOOLCHAIN_ATTR); } /** @@ -205,14 +203,8 @@ boolean shouldGenerateCode(ProtoInfo protoInfo, String ruleName) { return false; } - NestedSetBuilder forbiddenProtos = NestedSetBuilder.stableOrder(); - forbiddenProtos.addTransitive(getProtoToolchainProvider().forbiddenProtos()); - if (rpcSupport != null) { - forbiddenProtos.addTransitive(rpcSupport.getForbiddenProtos(ruleContext)); - } - final ProtoSourceFileExcludeList protoExcludeList = - new ProtoSourceFileExcludeList(ruleContext, forbiddenProtos.build()); + new ProtoSourceFileExcludeList(ruleContext, getProtoToolchainProvider().providedProtoSources()); return protoExcludeList.checkSrcs(protoInfo.getDirectSources(), ruleName); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoStarlarkCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoStarlarkCommon.java index a9dcc65b17975d..2cc5d71174ab21 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoStarlarkCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/JavaProtoStarlarkCommon.java @@ -81,6 +81,6 @@ private static ProtoLangToolchainProvider getProtoToolchainProvider( StarlarkRuleContext starlarkRuleContext, String protoToolchainAttr) throws EvalException { ConfiguredTarget javaliteToolchain = (ConfiguredTarget) checkNotNull(starlarkRuleContext.getAttr().getValue(protoToolchainAttr)); - return checkNotNull(javaliteToolchain.getProvider(ProtoLangToolchainProvider.class)); + return checkNotNull(ProtoLangToolchainProvider.get(javaliteToolchain)); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/proto/RpcSupport.java b/src/main/java/com/google/devtools/build/lib/rules/java/proto/RpcSupport.java index 7f989464071010..b373e233694116 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/proto/RpcSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/proto/RpcSupport.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.rules.java.proto; +import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; @@ -21,6 +22,7 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.packages.AspectDefinition; import com.google.devtools.build.lib.packages.AspectParameters; +import com.google.devtools.build.lib.packages.StarlarkInfo; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder; import java.util.List; @@ -31,7 +33,7 @@ List getToolchainInvocation( boolean allowServices(RuleContext ruleContext); - NestedSet getForbiddenProtos(RuleContext ruleContext); + Optional getToolchain(RuleContext ruleContext); ImmutableList getRuntimes(RuleContext ruleContext); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java index de89d98f883f81..22d9bb7c454e16 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/J2ObjcAspect.java @@ -51,6 +51,7 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.NativeAspectClass; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; +import com.google.devtools.build.lib.packages.StarlarkInfo; import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier; import com.google.devtools.build.lib.rules.apple.AppleConfiguration; import com.google.devtools.build.lib.rules.apple.AppleToolchain; @@ -390,10 +391,10 @@ private ConfiguredAspect proto(ConfiguredTarget base, RuleContext ruleContext) ImmutableList protoSources = protoInfo.getDirectProtoSources(); ProtoLangToolchainProvider protoToolchain = - ruleContext.getPrerequisite(J2OBJC_PROTO_TOOLCHAIN_ATTR, ProtoLangToolchainProvider.class); + ProtoLangToolchainProvider.get(ruleContext, J2OBJC_PROTO_TOOLCHAIN_ATTR); // Avoid pulling in any generated files from forbidden protos. ProtoSourceFileExcludeList protoExcludeList = - new ProtoSourceFileExcludeList(ruleContext, protoToolchain.forbiddenProtos()); + new ProtoSourceFileExcludeList(ruleContext, protoToolchain.providedProtoSources()); ImmutableList filteredProtoSources = ImmutableList.copyOf(protoExcludeList.filter(protoSources)); diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD b/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD index 6c45232c43407d..598d632f1844a0 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/BUILD @@ -38,21 +38,25 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment", "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection", "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_provider", + "//src/main/java/com/google/devtools/build/lib/analysis/starlark/annotations", "//src/main/java/com/google/devtools/build/lib/analysis/stringtemplate", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/packages", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant-annotation", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi", "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/proto", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:filetype", + "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/common/options", "//src/main/java/net/starlark/java/annot", "//src/main/java/net/starlark/java/eval", + "//src/main/java/net/starlark/java/syntax", "//third_party:auto_value", "//third_party:guava", "//third_party:jsr305", diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java index ced81605610e6c..49a985f1ccba9b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCommon.java @@ -31,15 +31,19 @@ import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.concurrent.BlazeInterners; +import com.google.devtools.build.lib.packages.BazelModuleContext; import com.google.devtools.build.lib.packages.BuildType; -import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import javax.annotation.Nullable; - -/** - * Utility functions for proto_library and proto aspect implementations. - */ +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.Module; +import net.starlark.java.eval.Sequence; +import net.starlark.java.eval.Starlark; +import net.starlark.java.eval.StarlarkThread; +import net.starlark.java.eval.Tuple; + +/** Utility functions for proto_library and proto aspect implementations. */ public class ProtoCommon { private ProtoCommon() { throw new UnsupportedOperationException(); @@ -521,4 +525,14 @@ public static boolean areDepsStrict(RuleContext ruleContext) { StrictDepsMode getBool = ruleContext.getFragment(ProtoConfiguration.class).strictProtoDeps(); return getBool != StrictDepsMode.OFF && getBool != StrictDepsMode.DEFAULT; } + + public static void checkPrivateStarlarkificationAllowlist(StarlarkThread thread) + throws EvalException { + Label label = + ((BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData()) + .label(); + if (!label.getPackageIdentifier().getRepository().toString().equals("@_builtins")) { + throw Starlark.errorf("Rule in '%s' cannot use private API", label.getPackageName()); + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java index 7f4b2daa3ec174..4bf12845baf1b7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilder.java @@ -21,7 +21,6 @@ import com.google.common.base.Preconditions; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.AbstractAction; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.CommandLineItem; @@ -42,6 +41,8 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.OnDemandString; import java.util.HashSet; import java.util.List; @@ -53,7 +54,7 @@ public class ProtoCompileActionBuilder { @VisibleForTesting public static final String STRICT_DEPS_FLAG_TEMPLATE = - "--direct_dependencies_violation_msg=" + StrictProtoDepsViolationMessage.MESSAGE; + "--direct_dependencies_violation_msg=" + ProtoConstants.STRICT_PROTO_DEPS_VIOLATION_MESSAGE; private static final String MNEMONIC = "GenProto"; @@ -190,9 +191,9 @@ public String lookupFunction(String name, String param) /** Builds a ResourceSet based on the number of inputs. */ public static class ProtoCompileResourceSetBuilder implements ResourceSetOrBuilder { @Override - public ResourceSet buildResourceSet(NestedSet inputs) { + public ResourceSet buildResourceSet(OS os, int inputsSize) { return ResourceSet.createWithRamCpu( - /* memoryMb= */ 25 + 0.15 * inputs.memoizedFlattenAndGetSize(), /* cpuUsage= */ 1); + /* memoryMb= */ 25 + 0.15 * inputsSize, /* cpuUsage= */ 1); } } @@ -332,7 +333,7 @@ public static void writeDescriptorSet( protoToolchain, ImmutableList.of( createDescriptorSetToolchain( - ruleContext.getFragment(ProtoConfiguration.class), output.getExecPathString())), + ruleContext.getFragment(ProtoConfiguration.class), output.getExecPathString(), protoToolchain)), protoInfo, ruleContext.getLabel(), ImmutableList.of(output), @@ -349,7 +350,7 @@ public static void writeDescriptorSet( } private static ToolchainInvocation createDescriptorSetToolchain( - ProtoConfiguration configuration, CharSequence outReplacement) { + ProtoConfiguration configuration, CharSequence outReplacement, ProtoToolchainInfo protoToolchain) { ImmutableList.Builder protocOpts = ImmutableList.builder(); if (configuration.experimentalProtoDescriptorSetsIncludeSourceInfo()) { protocOpts.add("--include_source_info"); @@ -357,15 +358,21 @@ private static ToolchainInvocation createDescriptorSetToolchain( return new ToolchainInvocation( "dontcare", - ProtoLangToolchainProvider.create( + ProtoLangToolchainProvider.createNative( // Note: adding --include_imports here was requested multiple times, but it'll cause the // output size to become quadratic, so don't. // A rule that concatenates the artifacts from ctx.deps.proto.transitive_descriptor_sets // provides similar results. - "--descriptor_set_out=$(OUT)", + "--descriptor_set_out=%s", + /* pluginFormatFlag = */ "", /* pluginExecutable= */ null, /* runtime= */ null, - /* providedProtoSources= */ ImmutableList.of()), + /* providedProtoSources= */ ImmutableList.of(), + /* protoCompiler= */ protoToolchain.getCompiler(), + /* protoCopts=*/ configuration.protocOpts(), + /* progressMessage=*/ "", + /* mnemonic=*/ "GenProtoDescriptorSet"), + outReplacement, protocOpts.build()); } @@ -488,8 +495,6 @@ public static boolean arePublicImportsStrict(RuleContext ruleContext) { *

    *
  • Each toolchain contributes a command-line, formatted from its commandLine() method. *
  • $(OUT) is replaced with the outReplacement field of ToolchainInvocation. - *
  • $(PLUGIN_out) is replaced with PLUGIN__out where 'key' is the key of - * toolchainInvocations. The key thus allows multiple plugins in one command-line. *
  • If a toolchain's {@code plugin()} is non-null, we point at it by emitting * --plugin=protoc-gen-PLUGIN_=. *
@@ -533,14 +538,15 @@ static CustomCommandLine createCommandLineFromToolchains( ProtoLangToolchainProvider toolchain = invocation.toolchain; + final String formatString = toolchain.outReplacementFormatFlag(); + final CharSequence outReplacement = invocation.outReplacement; cmdLine.addLazyString( - new OnDemandCommandLineExpansion( - toolchain.commandLine(), - ImmutableMap.of( - "OUT", - invocation.outReplacement, - "PLUGIN_OUT", - String.format("PLUGIN_%s_out", invocation.name)))); + new OnDemandString() { + @Override + public String toString() { + return String.format(formatString, outReplacement); + } + }); if (toolchain.pluginExecutable() != null) { cmdLine.addFormatted( diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java index 722b6ab41b1692..e626684e8df74b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConfiguration.java @@ -21,6 +21,7 @@ import com.google.devtools.build.lib.analysis.config.Fragment; import com.google.devtools.build.lib.analysis.config.FragmentOptions; import com.google.devtools.build.lib.analysis.config.RequiresOptions; +import com.google.devtools.build.lib.analysis.starlark.annotations.StarlarkConfigurationField; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.starlarkbuildapi.ProtoConfigurationApi; @@ -38,6 +39,7 @@ // configuration fragment in aspect definitions. @RequiresOptions(options = {ProtoConfiguration.Options.class}) public class ProtoConfiguration extends Fragment implements ProtoConfigurationApi { + /** Command line options. */ public static class Options extends FragmentOptions { @Option( @@ -81,7 +83,7 @@ public static class Options extends FragmentOptions { @Option( name = "proto_compiler", - defaultValue = "@com_google_protobuf//:protoc", + defaultValue = ProtoConstants.DEFAULT_PROTOC_LABEL, converter = CoreOptionConverters.LabelConverter.class, documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, effectTags = {OptionEffectTag.AFFECTS_OUTPUTS, OptionEffectTag.LOADING_AND_ANALYSIS}, @@ -220,6 +222,10 @@ public boolean runExperimentalProtoExtraActions() { return options.experimentalProtoExtraActions; } + @StarlarkConfigurationField( + name = "proto_compiler", + doc = "Label for the proto compiler.", + defaultLabel = ProtoConstants.DEFAULT_PROTOC_LABEL) public Label protoCompiler() { return options.protoCompiler; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/StrictProtoDepsViolationMessage.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConstants.java similarity index 51% rename from src/main/java/com/google/devtools/build/lib/rules/proto/StrictProtoDepsViolationMessage.java rename to src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConstants.java index 6d6d03844d1b64..419756cae76a92 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/StrictProtoDepsViolationMessage.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoConstants.java @@ -14,16 +14,22 @@ package com.google.devtools.build.lib.rules.proto; -/** - * This class is used in ProtoCompileActionBuilder to generate an error message that's displayed - * when a strict proto deps violation occurs. - * - *

%1$s is replaced with the label of the proto_library rule that's currently being built. - * - *

%%s is replaced with the literal "%s", which is passed to the proto-compiler, which replaces - * it with the .proto file that violates strict proto deps. - */ -public class StrictProtoDepsViolationMessage { - static final String MESSAGE = +/** Constants used in Proto rules. */ +public final class ProtoConstants { + /** Default label for proto compiler. */ + static final String DEFAULT_PROTOC_LABEL = "@com_google_protobuf//:protoc"; + + /** + * This constant is used in ProtoCompileActionBuilder to generate an error message that's + * displayed when a strict proto deps violation occurs. + * + *

%1$s is replaced with the label of the proto_library rule that's currently being built. + * + *

%%s is replaced with the literal "%s", which is passed to the proto-compiler, which replaces + * it with the .proto file that violates strict proto deps. + */ + static final String STRICT_PROTO_DEPS_VIOLATION_MESSAGE = "%%s is imported, but %1$s doesn't directly depend on a proto_library that 'srcs' it."; + + private ProtoConstants() {} } diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoInfo.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoInfo.java index 15a44346c8aca9..376e28f64ee713 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoInfo.java @@ -25,6 +25,8 @@ import com.google.devtools.build.lib.starlarkbuildapi.ProtoInfoApi; import com.google.devtools.build.lib.starlarkbuildapi.proto.ProtoBootstrap; import com.google.devtools.build.lib.vfs.PathFragment; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.StarlarkThread; /** * Configured target classes that implement this class can contribute .proto files to the @@ -116,6 +118,13 @@ public ImmutableList getDirectProtoSources() { return directProtoSources; } + @Override + public ImmutableList getDirectProtoSourcesForStarlark(StarlarkThread thread) + throws EvalException { + ProtoCommon.checkPrivateStarlarkificationAllowlist(thread); + return directSources; + } + /** * The source root of the current library. * @@ -133,6 +142,12 @@ public String getDirectProtoSourceRoot() { return Depset.of(Artifact.TYPE, getTransitiveProtoSources()); } + @Override + public Depset getTransitiveSourcesForStarlark(StarlarkThread thread) throws EvalException { + ProtoCommon.checkPrivateStarlarkificationAllowlist(thread); + return Depset.of(ProtoSource.TYPE, transitiveSources); + } + /** * The {@code .proto} source files in this {@code proto_library}'s {@code srcs} and all of its * transitive dependencies. @@ -222,6 +237,12 @@ public NestedSet getStrictImportableSources() { return strictImportableSources; } + @Override + public Depset getStrictImportableSourcesForStarlark(StarlarkThread thread) throws EvalException { + ProtoCommon.checkPrivateStarlarkificationAllowlist(thread); + return Depset.of(ProtoSource.TYPE, strictImportableSources); + } + /** * Returns a set of {@code .proto} sources that may be re-exported by this {@code proto_library}'s * direct sources. diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchain.java deleted file mode 100644 index fbc331d35be35d..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchain.java +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright 2016 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.rules.proto; - -import static com.google.devtools.build.lib.collect.nestedset.Order.STABLE_ORDER; - -import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException; -import com.google.devtools.build.lib.analysis.ConfiguredTarget; -import com.google.devtools.build.lib.analysis.FilesToRunProvider; -import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; -import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory; -import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.analysis.Runfiles; -import com.google.devtools.build.lib.analysis.RunfilesProvider; -import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.packages.Type; - -/** Implements {code proto_lang_toolchain}. */ -public class ProtoLangToolchain implements RuleConfiguredTargetFactory { - @Override - public ConfiguredTarget create(RuleContext ruleContext) - throws InterruptedException, RuleErrorException, ActionConflictException { - NestedSetBuilder providedProtoSources = NestedSetBuilder.stableOrder(); - for (ProtoInfo protoInfo : - ruleContext.getPrerequisites("blacklisted_protos", ProtoInfo.PROVIDER)) { - providedProtoSources.addTransitive(protoInfo.getTransitiveSources()); - } - - return new RuleConfiguredTargetBuilder(ruleContext) - .addProvider( - ProtoLangToolchainProvider.create( - ruleContext.attributes().get("command_line", Type.STRING), - ruleContext.getPrerequisite("plugin", FilesToRunProvider.class), - ruleContext.getPrerequisite("runtime"), - // We intentionally flatten the NestedSet here. - // - // `providedProtoSources` are read during analysis, so flattening the set here once - // avoid flattening it in every `_proto_aspect` applied to `proto_library` - // targets. While this has the potential to use more memory than using a NestedSet, - // there are only a few `proto_lang_toolchain` targets in every build, so the impact - // on memory consumption should be neglectable. - providedProtoSources.build().toList())) - .setFilesToBuild(NestedSetBuilder.emptySet(STABLE_ORDER)) - .addProvider(RunfilesProvider.simple(Runfiles.EMPTY)) - .build(); - } -} diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainProvider.java index 0a6bd07462e738..4ce277d5af7e81 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainProvider.java @@ -16,14 +16,25 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.FilesToRunProvider; +import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; -import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; -import com.google.devtools.build.lib.collect.nestedset.NestedSet; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.collect.nestedset.Depset; +import com.google.devtools.build.lib.collect.nestedset.Depset.ElementType; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.lib.packages.StarlarkInfo; +import com.google.devtools.build.lib.packages.StarlarkProvider; +import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier; +import java.util.LinkedHashMap; +import java.util.Map; import javax.annotation.Nullable; +import net.starlark.java.annot.StarlarkMethod; +import net.starlark.java.eval.EvalException; +import net.starlark.java.eval.NoneType; +import net.starlark.java.eval.Starlark; +import net.starlark.java.eval.StarlarkList; +import net.starlark.java.syntax.Location; // Note: AutoValue v1.4-rc1 has AutoValue.CopyAnnotations which makes it work with Starlark. No need // to un-AutoValue this class to expose it to Starlark. @@ -32,10 +43,23 @@ * rules. */ @AutoValue -@AutoCodec -public abstract class ProtoLangToolchainProvider implements TransitiveInfoProvider { - public abstract String commandLine(); +public abstract class ProtoLangToolchainProvider { + public static final String PROVIDER_NAME = "ProtoLangToolchainInfo"; + public static final StarlarkProvider.Key starlarkProtoLangToolchainKey = + new StarlarkProvider.Key( + Label.parseAbsoluteUnchecked("@_builtins//:common/proto/proto_common.bzl"), + PROVIDER_NAME); + public static final StarlarkProviderIdentifier PROVIDER_ID = + StarlarkProviderIdentifier.forKey(starlarkProtoLangToolchainKey); + // Format string used when passing output to the plugin used by proto compiler. + public abstract String outReplacementFormatFlag(); + + // Format string used when passing plugin to proto compiler. + @Nullable + public abstract String pluginFormatFlag(); + + // Proto compiler plugin. @Nullable public abstract FilesToRunProvider pluginExecutable(); @@ -46,42 +70,141 @@ public abstract class ProtoLangToolchainProvider implements TransitiveInfoProvid * Returns a list of {@link ProtoSource}s that are already provided by the protobuf runtime (i.e. * for which {@code _proto_library} should not generate bindings. */ + @StarlarkMethod( + name = "provided_proto_sources", + doc = "Proto sources provided by the toolchain.", + structField = true) public abstract ImmutableList providedProtoSources(); - /** - * This makes the blacklisted_protos member available in the provider. It can be removed after - * users are migrated and a sufficient time for Bazel rules to migrate has elapsed. - */ - @Deprecated - public NestedSet blacklistedProtos() { - return forbiddenProtos(); + // Proto compiler. + public abstract FilesToRunProvider protoc(); + + // Options to pass to proto compiler. + public StarlarkList protocOptsForStarlark() { + return StarlarkList.immutableCopyOf(protocOpts()); } - // TODO(yannic): Remove after migrating all users to `providedProtoSources()`. - @Deprecated - public abstract NestedSet forbiddenProtos(); + public abstract ImmutableList protocOpts(); + + // Progress message to set on the proto compiler action. + public abstract String progressMessage(); + + // Mnemonic to set on the proto compiler action. + public abstract String mnemonic(); - @AutoCodec.Instantiator - public static ProtoLangToolchainProvider createForDeserialization( - String commandLine, + public static StarlarkInfo create( + String outReplacementFormatFlag, + String pluginFormatFlag, FilesToRunProvider pluginExecutable, TransitiveInfoCollection runtime, ImmutableList providedProtoSources, - NestedSet blacklistedProtos) { - return new AutoValue_ProtoLangToolchainProvider( - commandLine, pluginExecutable, runtime, providedProtoSources, blacklistedProtos); + FilesToRunProvider protoc, + ImmutableList protocOpts, + String progressMessage, + String mnemonic) { + + NestedSetBuilder providedProtoSourcesSet = NestedSetBuilder.stableOrder(); + providedProtoSources.forEach(providedProtoSourcesSet::add); + NestedSetBuilder protocOptsSet = NestedSetBuilder.stableOrder(); + protocOpts.forEach(protocOptsSet::add); + + Map m = new LinkedHashMap<>(); + m.put("plugin", pluginExecutable == null ? Starlark.NONE : pluginExecutable); + m.put("plugin_format_flag", pluginFormatFlag == null ? Starlark.NONE : pluginFormatFlag); + m.put("proto_compiler", protoc == null ? Starlark.NONE : protoc); + m.put( + "provided_proto_sources", + StarlarkList.immutableCopyOf(providedProtoSources)); + m.put("protoc_opts", StarlarkList.immutableCopyOf(protocOpts)); + m.put("out_replacement_format_flag", outReplacementFormatFlag); + m.put("progress_message", progressMessage); + m.put("mnemonic", mnemonic); + m.put("plugin", pluginExecutable == null ? Starlark.NONE : pluginExecutable); + m.put("runtime", runtime == null ? Starlark.NONE : runtime); + + StarlarkProvider provider = + StarlarkProvider.createExportedSchemaless(starlarkProtoLangToolchainKey, + Location.BUILTIN); + + return StarlarkInfo.create(provider, m, Location.BUILTIN); } - public static ProtoLangToolchainProvider create( - String commandLine, + private static ImmutableList getToolchains( + RuleContext ruleContext, String attributeName) { + ImmutableList.Builder result = ImmutableList.builder(); + for (TransitiveInfoCollection prerequisite : ruleContext.getPrerequisites(attributeName)) { + ProtoLangToolchainProvider toolchain = get(prerequisite); + if (toolchain != null) { + result.add(toolchain); + } + } + return result.build(); + } + + public static ProtoLangToolchainProvider get(RuleContext ruleContext, String attributeName) { + return getToolchains(ruleContext, attributeName).stream().findFirst().orElse(null); + } + + public static ProtoLangToolchainProvider get(TransitiveInfoCollection prerequisite) { + StarlarkInfo provider = (StarlarkInfo) prerequisite.get(starlarkProtoLangToolchainKey); + return wrapStarlarkProviderWithNativeProvider(provider); + } + + public static StarlarkInfo getStarlarkProvider(RuleContext ruleContext, String attributeName) { + for (TransitiveInfoCollection prerequisite : ruleContext.getPrerequisites(attributeName)) { + StarlarkInfo provider = (StarlarkInfo) prerequisite.get(starlarkProtoLangToolchainKey); + if (provider != null) { + return provider; + } + } + return null; + } + + public static StarlarkInfo getStarlarkProvider(TransitiveInfoCollection prerequisite) { + return (StarlarkInfo) prerequisite.get(starlarkProtoLangToolchainKey); + } + + @SuppressWarnings("unchecked") + private static ProtoLangToolchainProvider wrapStarlarkProviderWithNativeProvider( + StarlarkInfo provider) { + if (provider != null) { + try { + return new AutoValue_ProtoLangToolchainProvider( + provider.getValue("out_replacement_format_flag", String.class), + provider.getValue("plugin_format_flag") instanceof NoneType ? + null + : provider.getValue("plugin_format_flag", String.class), + provider.getValue("plugin") instanceof NoneType + ? null + : provider.getValue("plugin", FilesToRunProvider.class), + provider.getValue("runtime") instanceof NoneType + ? null + : provider.getValue("runtime", TransitiveInfoCollection.class), + ImmutableList.copyOf( + (StarlarkList) provider.getValue("provided_proto_sources")), + provider.getValue("proto_compiler", FilesToRunProvider.class), + ImmutableList.copyOf((StarlarkList) provider.getValue("protoc_opts")), + provider.getValue("progress_message", String.class), + provider.getValue("mnemonic", String.class)); + } catch (EvalException e) { + return null; + } + } + return null; + } + + + public static ProtoLangToolchainProvider createNative( + String outReplacementFormatFlag, + String pluginFormatFlag, FilesToRunProvider pluginExecutable, TransitiveInfoCollection runtime, - ImmutableList providedProtoSources) { - NestedSetBuilder blacklistedProtos = NestedSetBuilder.stableOrder(); - for (ProtoSource protoSource : providedProtoSources) { - blacklistedProtos.add(protoSource.getOriginalSourceFile()); - } - return new AutoValue_ProtoLangToolchainProvider( - commandLine, pluginExecutable, runtime, providedProtoSources, blacklistedProtos.build()); + ImmutableList providedProtoSources, + FilesToRunProvider protoc, + ImmutableList protocOpts, + String progressMessage, + String mnemonic) { + return wrapStarlarkProviderWithNativeProvider(create(outReplacementFormatFlag,pluginFormatFlag,pluginExecutable,runtime,providedProtoSources, protoc, protocOpts, progressMessage, mnemonic)); } + } diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainRule.java index b05895452a5716..20b91245c286bb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainRule.java @@ -22,15 +22,41 @@ import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.Attribute; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier; import com.google.devtools.build.lib.packages.Type; -/** Implements {code proto_lang_toolchain}. */ +/** + * Implements {code proto_lang_toolchain}. + * + *

This rule is implemented in Starlark. This class remains only for doc-gen purposes. + */ public class ProtoLangToolchainRule implements RuleDefinition { + private static final Label DEFAULT_PROTO_COMPILER = + Label.parseAbsoluteUnchecked(ProtoConstants.DEFAULT_PROTOC_LABEL); + private static final Attribute.LabelLateBoundDefault PROTO_COMPILER = + Attribute.LabelLateBoundDefault.fromTargetConfiguration( + ProtoConfiguration.class, + DEFAULT_PROTO_COMPILER, + (rule, attributes, protoConfig) -> + protoConfig.protoCompiler() != null + ? protoConfig.protoCompiler() + : DEFAULT_PROTO_COMPILER); + @Override public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment environment) { return builder + /* + This value will be set as the progress message on protoc action. + */ + .add(attr("progress_message", Type.STRING).value("Generating proto_library %{label}")) + + /* + This value will be set as the mnemonic on protoc action. + */ + .add(attr("mnemonic", Type.STRING).value("GenProto")) /* This value will be passed to proto-compiler to generate the code. Only include the parts @@ -39,12 +65,17 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi

  • $(OUT) is LANG_proto_library-specific. The rules are expected to define how they interpret this variable. For Java, for example, $(OUT) will be replaced with the src-jar filename to create.
  • -
  • $(PLUGIN_out) will be substituted to work with a - `--plugin=protoc-gen-PLUGIN` command line.
  • */ .add(attr("command_line", Type.STRING).mandatory()) + /* + If provided, this value will be passed to proto-compiler to use the plugin. The value must + contain a single %s which is replaced with plugin executable. + --plugin=protoc-gen-PLUGIN=. + */ + .add(attr("plugin_format_flag", Type.STRING)) + /* If provided, will be made available to the action that calls the proto-compiler, and will be passed to the proto-compiler: @@ -73,8 +104,12 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment envi attr("blacklisted_protos", LABEL_LIST) .allowedFileTypes() .mandatoryProviders(StarlarkProviderIdentifier.forKey(ProtoInfo.PROVIDER.getKey()))) + .add( + attr(":proto_compiler", LABEL) + .cfg(ExecutionTransitionFactory.create()) + .exec() + .value(PROTO_COMPILER)) .requiresConfigurationFragments(ProtoConfiguration.class) - .advertiseProvider(ProtoLangToolchainProvider.class) .removeAttribute("data") .removeAttribute("deps") .build(); @@ -85,7 +120,7 @@ public Metadata getMetadata() { return RuleDefinition.Metadata.builder() .name("proto_lang_toolchain") .ancestors(BaseRuleClasses.NativeActionCreatingRule.class) - .factoryClass(ProtoLangToolchain.class) + .factoryClass(BaseRuleClasses.EmptyRuleConfiguredTargetFactory.class) .build(); } } @@ -122,7 +157,7 @@ Specifies how a LANG_proto_library rule (e.g., java_proto_library)
     proto_lang_toolchain(
         name = "javalite_toolchain",
    -    command_line = "--$(PLUGIN_OUT)=shared,immutable:$(OUT)",
    +    command_line = "--javalite_out=shared,immutable:$(OUT)",
         plugin = ":javalite_plugin",
         runtime = ":protobuf_lite",
     )
    diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSource.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSource.java
    index 754c37521a6fc7..01bdee4cd0972a 100644
    --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSource.java
    +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSource.java
    @@ -15,14 +15,19 @@
     package com.google.devtools.build.lib.rules.proto;
     
     import com.google.devtools.build.lib.actions.Artifact;
    +import com.google.devtools.build.lib.collect.nestedset.Depset;
     import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
    -import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
     import com.google.devtools.build.lib.vfs.PathFragment;
    +import net.starlark.java.annot.StarlarkMethod;
    +import net.starlark.java.eval.EvalException;
    +import net.starlark.java.eval.StarlarkThread;
    +import net.starlark.java.eval.StarlarkValue;
     
     /** Represents a single {@code .proto} source file. */
     @Immutable
    -@AutoCodec
    -class ProtoSource {
    +final class ProtoSource implements StarlarkValue {
    +  public static final Depset.ElementType TYPE = Depset.ElementType.of(ProtoSource.class);
    +
       private final Artifact sourceFile;
       private final Artifact originalSourceFile;
       private final PathFragment sourceRoot;
    @@ -31,7 +36,6 @@ public ProtoSource(Artifact sourceFile, PathFragment sourceRoot) {
         this(sourceFile, sourceFile, sourceRoot);
       }
     
    -  @AutoCodec.Instantiator
       ProtoSource(Artifact sourceFile, Artifact originalSourceFile, PathFragment sourceRoot) {
         this.sourceFile = sourceFile;
         this.originalSourceFile = originalSourceFile;
    @@ -42,8 +46,19 @@ public Artifact getSourceFile() {
         return sourceFile;
       }
     
    +  @StarlarkMethod(name = "source_file", documented = false, useStarlarkThread = true)
    +  public Artifact getSourceFileForStarlark(StarlarkThread thread) throws EvalException {
    +    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
    +    return sourceFile;
    +  }
    +
       /** Returns the original source file. Only for forbidding protos! */
    -  @Deprecated
    +  @StarlarkMethod(name = "original_source_file", documented = false, useStarlarkThread = true)
    +  public Artifact getOriginalSourceFileForStarlark(StarlarkThread thread) throws EvalException {
    +    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
    +    return originalSourceFile;
    +  }
    +
       Artifact getOriginalSourceFile() {
         return originalSourceFile;
       }
    @@ -52,6 +67,12 @@ public PathFragment getSourceRoot() {
         return sourceRoot;
       }
     
    +  @StarlarkMethod(name = "import_path", documented = false, useStarlarkThread = true)
    +  public String getImportPathForStarlark(StarlarkThread thread) throws EvalException {
    +    ProtoCommon.checkPrivateStarlarkificationAllowlist(thread);
    +    return getImportPath().getPathString();
    +  }
    +
       public PathFragment getImportPath() {
         return sourceFile.getExecPath().relativeTo(sourceRoot);
       }
    diff --git a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourceFileExcludeList.java b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourceFileExcludeList.java
    index 98d006450fd0e9..f477825fff5a26 100644
    --- a/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourceFileExcludeList.java
    +++ b/src/main/java/com/google/devtools/build/lib/rules/proto/ProtoSourceFileExcludeList.java
    @@ -54,12 +54,12 @@ public class ProtoSourceFileExcludeList {
        * @param noGenerateProtoFiles a list of .proto files. The list will be iterated.
        */
       public ProtoSourceFileExcludeList(
    -      RuleContext ruleContext, NestedSet noGenerateProtoFiles) {
    +      RuleContext ruleContext, ImmutableList noGenerateProtoFiles) {
         this.ruleContext = ruleContext;
         ImmutableSet.Builder noGenerateProtoFilePathsBuilder =
             new ImmutableSet.Builder<>();
    -    for (Artifact noGenerateProtoFile : noGenerateProtoFiles.toList()) {
    -      PathFragment execPath = noGenerateProtoFile.getExecPath();
    +    for (ProtoSource noGenerateProtoFile : noGenerateProtoFiles) {
    +      PathFragment execPath = noGenerateProtoFile.getOriginalSourceFile().getExecPath();
           // For listed protos bundled with the Bazel tools repository, their exec paths start
           // with external/bazel_tools/. This prefix needs to be removed first, because the protos in
           // user repositories will not have that prefix.
    diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
    index acdb1b0ec53fd5..06edeb33977fbb 100644
    --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
    +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
    @@ -613,7 +613,7 @@ Token checkActionCache(
                   remoteDefaultProperties,
                   isRemoteCacheEnabled);
         } catch (UserExecException e) {
    -      throw e.toActionExecutionException(action);
    +      throw ActionExecutionException.fromExecException(e, action);
         }
         if (token == null) {
           boolean eventPosted = false;
    @@ -693,7 +693,7 @@ void updateActionCache(
                   ? remoteOptions.getRemoteDefaultExecProperties()
                   : ImmutableSortedMap.of();
         } catch (UserExecException e) {
    -      throw e.toActionExecutionException(action);
    +      throw ActionExecutionException.fromExecException(e, action);
         }
     
         try {
    diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/ProtoInfoApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/ProtoInfoApi.java
    index ce1fad68e96a5d..6289e8f688025d 100644
    --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/ProtoInfoApi.java
    +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/ProtoInfoApi.java
    @@ -21,6 +21,8 @@
     import com.google.devtools.build.lib.starlarkbuildapi.core.StructApi;
     import net.starlark.java.annot.StarlarkBuiltin;
     import net.starlark.java.annot.StarlarkMethod;
    +import net.starlark.java.eval.EvalException;
    +import net.starlark.java.eval.StarlarkThread;
     
     /** Info object propagating information about protocol buffer sources. */
     @StarlarkBuiltin(
    @@ -62,6 +64,9 @@ interface ProtoInfoProviderApi extends ProviderApi {
           structField = true)
       ImmutableList getDirectProtoSources();
     
    +  @StarlarkMethod(name = "direct_proto_sources", documented = false, useStarlarkThread = true)
    +  ImmutableList getDirectProtoSourcesForStarlark(StarlarkThread thread) throws EvalException;
    +
       @StarlarkMethod(
           name = "check_deps_sources",
           doc =
    @@ -105,4 +110,10 @@ interface ProtoInfoProviderApi extends ProviderApi {
                   + " as a source, that source file would be imported as 'import c/d.proto'",
           structField = true)
       String getDirectProtoSourceRoot();
    +
    +  @StarlarkMethod(name = "strict_importable_sources", documented = false, useStarlarkThread = true)
    +  Depset getStrictImportableSourcesForStarlark(StarlarkThread thread) throws EvalException;
    +
    +  @StarlarkMethod(name = "transitive_proto_sources", documented = false, useStarlarkThread = true)
    +  Depset getTransitiveSourcesForStarlark(StarlarkThread thread) throws EvalException;
     }
    diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java
    index f2205cca4f47e0..27fee5e251c79a 100644
    --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java
    +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkActionFactoryApi.java
    @@ -24,6 +24,7 @@
     import net.starlark.java.eval.EvalException;
     import net.starlark.java.eval.NoneType;
     import net.starlark.java.eval.Sequence;
    +import net.starlark.java.eval.StarlarkCallable;
     import net.starlark.java.eval.StarlarkThread;
     import net.starlark.java.eval.StarlarkValue;
     
    @@ -435,6 +436,27 @@ void symlink(
                         + " environment added to the action's inputs list and environment. The action"
                         + " environment can overwrite any of the shadowed action's environment"
                         + " variables. If none, uses only the action's inputs and given environment."),
    +        @Param(
    +            name = "resource_set",
    +            allowedTypes = {
    +              @ParamType(type = StarlarkCallable.class),
    +              @ParamType(type = NoneType.class),
    +            },
    +            defaultValue = "None",
    +            named = true,
    +            positional = false,
    +            doc =
    +                "A callback function that returns a resource set dictionary, used to estimate"
    +                    + " resource usage at execution time if this action is run locally.

    The" + + " function accepts two positional arguments: a string representing an OS name" + + " (e.g. \"osx\"), and an integer representing the number of inputs to the" + + " action. The returned dictionary may contain the following entries, each of" + + " which may be a float or an int:

    • \"cpu\": number of CPUs; default" + + " 1
    • \"memory\": in MB; default 250
    • \"local_test\": number of local" + + " tests; default 1

    If this parameter is set to None or if" + + " --experimental_action_resource_set is false, the default" + + " values are used.

    The callback must be top-level (lambda and nested" + + " functions aren't allowed)."), }) void run( Sequence outputs, @@ -450,7 +472,8 @@ void run( Object executionRequirementsUnchecked, Object inputManifestsUnchecked, Object execGroupUnchecked, - Object shadowedAction) + Object shadowedAction, + Object resourceSetUnchecked) throws EvalException; @StarlarkMethod( @@ -639,6 +662,18 @@ void run( "Runs the action using the given shadowed action's discovered inputs" + " added to the action's inputs list. If none, uses only the action's" + " inputs."), + @Param( + name = "resource_set", + allowedTypes = { + @ParamType(type = StarlarkCallable.class), + @ParamType(type = NoneType.class), + }, + defaultValue = "None", + named = true, + positional = false, + doc = + "A callback function for estimating resource usage if run locally. See" + + "ctx.actions.run()."), }) void runShell( Sequence outputs, @@ -653,7 +688,8 @@ void runShell( Object executionRequirementsUnchecked, Object inputManifestsUnchecked, Object execGroupUnchecked, - Object shadowedAction) + Object shadowedAction, + Object resourceSetUnchecked) throws EvalException; @StarlarkMethod( diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/proto/ProtoBootstrap.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/proto/ProtoBootstrap.java index fca7d5aad1b63c..774ff9d3c6c2ee 100644 --- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/proto/ProtoBootstrap.java +++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/proto/ProtoBootstrap.java @@ -33,6 +33,8 @@ public class ProtoBootstrap implements Bootstrap { /** The name of the proto namespace in Starlark. */ public static final String PROTO_COMMON_NAME = "proto_common"; + public static final String PROTO_COMMON_SECOND_NAME = "proto_common_do_not_use"; + private final ProtoInfoProviderApi protoInfoApiProvider; private final ProtoToolchainInfoApi.Provider> protoToolchainInfoApi; @@ -59,6 +61,7 @@ public void addBindingsToBuilder(ImmutableMap.Builder builder) { builder.put(PROTO_INFO_STARLARK_NAME, protoInfoApiProvider); builder.put(ProtoToolchainInfoApi.NAME, protoToolchainInfoApi); builder.put(PROTO_COMMON_NAME, protoCommon); + builder.put(PROTO_COMMON_SECOND_NAME, protoCommon); builder.put( "ProtoRegistryAspect", FlagGuardedValue.onlyWhenExperimentalFlagIsTrue( diff --git a/src/main/starlark/builtins_bzl/common/exports.bzl b/src/main/starlark/builtins_bzl/common/exports.bzl index b2690c98ba3764..249e554ff2ac5f 100644 --- a/src/main/starlark/builtins_bzl/common/exports.bzl +++ b/src/main/starlark/builtins_bzl/common/exports.bzl @@ -22,12 +22,15 @@ load("@_builtins//:common/objc/objc_import.bzl", "objc_import") load("@_builtins//:common/objc/objc_library.bzl", "objc_library") load("@_builtins//:common/objc/apple_static_library.bzl", "apple_static_library") load("@_builtins//:common/objc/compilation_support.bzl", "compilation_support") +load("@_builtins//:common/proto/proto_lang_toolchain_wrapper.bzl", "proto_lang_toolchain") +load("@_builtins//:common/proto/proto_common.bzl", "proto_common_do_not_use") exported_toplevels = { # This dummy symbol is not part of the public API; it is only used to test # that builtins injection is working properly. Its built-in value is # "original value". "_builtins_dummy": "overridden value", + "proto_common_do_not_use": proto_common_do_not_use, } exported_rules = { "+cc_import": cc_import, @@ -38,6 +41,7 @@ exported_rules = { "+apple_static_library": apple_static_library, "+cc_shared_library": cc_shared_library, "+cc_shared_library_permissions": cc_shared_library_permissions, + "proto_lang_toolchain": proto_lang_toolchain, } exported_to_java = { "register_compile_and_archive_actions_for_j2objc": compilation_support.register_compile_and_archive_actions_for_j2objc, diff --git a/src/main/starlark/builtins_bzl/common/proto/proto_common.bzl b/src/main/starlark/builtins_bzl/common/proto/proto_common.bzl new file mode 100644 index 00000000000000..a3fa72e010fafc --- /dev/null +++ b/src/main/starlark/builtins_bzl/common/proto/proto_common.bzl @@ -0,0 +1,230 @@ +# Copyright 2021 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. + +"""Definition of proto_common module, together with bazel providers for proto rules.""" + +ProtoLangToolchainInfo = provider( + doc = "Specifies how to generate language-specific code from .proto files. Used by LANG_proto_library rules.", + fields = dict( + out_replacement_format_flag = "(str) Format string used when passing output to the plugin used by proto compiler.", + plugin_format_flag = "(str) Format string used when passing plugin to proto compiler.", + plugin = "(FilesToRunProvider) Proto compiler plugin.", + runtime = "(Target) Runtime.", + provided_proto_sources = "(list[ProtoSource]) Proto sources provided by the toolchain.", + proto_compiler = "(FilesToRunProvider) Proto compiler.", + protoc_opts = "(list[str]) Options to pass to proto compiler.", + progress_message = "(str) Progress message to set on the proto compiler action.", + mnemonic = "(str) Mnemonic to set on the proto compiler action.", + ), +) + +def _proto_path_flag(path): + if path == ".": + return None + return "--proto_path=%s" % path + +def _Iimport_path_equals_fullpath(proto_source): + return "-I%s=%s" % (proto_source.import_path(), proto_source.source_file().path) + +def _compile( + actions, + proto_info, + proto_lang_toolchain_info, + generated_files, + plugin_output = None, + additional_args = None, + additional_tools = [], + additional_inputs = depset(), + resource_set = None, + experimental_progress_message = None): + """Creates proto compile action for compiling *.proto files to language specific sources. + + Args: + actions: (ActionFactory) Obtained by ctx.actions, used to register the actions. + proto_info: (ProtoInfo) The ProtoInfo from proto_library to generate the sources for. + proto_lang_toolchain_info: (ProtoLangToolchainInfo) The proto lang toolchain info. + Obtained from a `proto_lang_toolchain` target or constructed ad-hoc.. + generated_files: (list[File]) The output files generated by the proto compiler. + Callee needs to declare files using `ctx.actions.declare_file`. + See also: `proto_common.declare_generated_files`. + plugin_output: (File|str) The file or directory passed to the plugin. + Formatted with `proto_lang_toolchain.out_replacement_format_flag` + additional_args: (Args) Additional arguments to add to the action. + Accepts an ctx.actions.args() object that is added at the beginning + of the command line. + additional_tools: (list[File]) Additional tools to add to the action. + additional_inputs: (Depset[File]) Additional input files to add to the action. + resource_set: (func) A callback function that is passed to the created action. + See `ctx.actions.run`, `resource_set` parameter for full definition of + the callback. + experimental_progress_message: Overrides progres_message from the toolchain. + Don't use this parameter. It's only intended for the transition. + """ + args = actions.args() + args.use_param_file(param_file_arg = "@%s") + args.set_param_file_format("multiline") + tools = list(additional_tools) + + if plugin_output: + args.add(plugin_output, format = proto_lang_toolchain_info.out_replacement_format_flag) + if proto_lang_toolchain_info.plugin: + tools.append(proto_lang_toolchain_info.plugin) + args.add(proto_lang_toolchain_info.plugin.executable, format = proto_lang_toolchain_info.plugin_format_flag) + + args.add_all(proto_info.transitive_proto_path, map_each = _proto_path_flag) + # Example: `--proto_path=--proto_path=bazel-bin/target/third_party/pkg/_virtual_imports/subpkg` + + args.add_all(proto_lang_toolchain_info.protoc_opts) + + # Include maps + # For each import, include both the import as well as the import relativized against its + # protoSourceRoot. This ensures that protos can reference either the full path or the short + # path when including other protos. + args.add_all(proto_info.transitive_proto_sources(), map_each = _Iimport_path_equals_fullpath) + # Example: `-Ia.proto=bazel-bin/target/third_party/pkg/_virtual_imports/subpkg/a.proto` + + args.add_all(proto_info.direct_sources) + + if additional_args: + additional_args.use_param_file(param_file_arg = "@%s") + additional_args.set_param_file_format("multiline") + + actions.run( + mnemonic = proto_lang_toolchain_info.mnemonic, + progress_message = experimental_progress_message if experimental_progress_message else proto_lang_toolchain_info.progress_message, + executable = proto_lang_toolchain_info.proto_compiler, + arguments = [additional_args, args] if additional_args else [args], + inputs = depset(transitive = [proto_info.transitive_sources, additional_inputs]), + outputs = generated_files, + tools = tools, + use_default_shell_env = True, + resource_set = resource_set, + ) + +_BAZEL_TOOLS_PREFIX = "external/bazel_tools/" + +def _experimental_filter_sources(proto_info, proto_lang_toolchain_info): + if not proto_info.direct_sources: + return [], [] + + # Collect a set of provided protos + provided_proto_sources = proto_lang_toolchain_info.provided_proto_sources + provided_paths = {} + for src in provided_proto_sources: + path = src.original_source_file().path + + # For listed protos bundled with the Bazel tools repository, their exec paths start + # with external/bazel_tools/. This prefix needs to be removed first, because the protos in + # user repositories will not have that prefix. + if path.startswith(_BAZEL_TOOLS_PREFIX): + provided_paths[path[len(_BAZEL_TOOLS_PREFIX):]] = None + else: + provided_paths[path] = None + + # Filter proto files + proto_files = [src.original_source_file() for src in proto_info.direct_proto_sources()] + excluded = [] + included = [] + for proto_file in proto_files: + if proto_file.path in provided_paths: + excluded.append(proto_file) + else: + included.append(proto_file) + return included, excluded + +def _experimental_should_generate_code( + proto_info, + proto_lang_toolchain_info, + rule_name, + target_label): + """Checks if the code should be generated for the given proto_library. + + The code shouldn't be generated only when the toolchain already provides it + to the language through its runtime dependency. + + It fails when the proto_library contains mixed proto files, that should and + shouldn't generate code. + + Args: + proto_info: (ProtoInfo) The ProtoInfo from proto_library to check the generation for. + proto_lang_toolchain_info: (ProtoLangToolchainInfo) The proto lang toolchain info. + Obtained from a `proto_lang_toolchain` target or constructed ad-hoc. + rule_name: (str) Name of the rule used in the failure message. + target_label: (Label) The label of the target used in the failure message. + + Returns: + (bool) True when the code should be generated. + """ + included, excluded = _experimental_filter_sources(proto_info, proto_lang_toolchain_info) + + if included and excluded: + fail(("The 'srcs' attribute of '%s' contains protos for which '%s' " + + "shouldn't generate code (%s), in addition to protos for which it should (%s).\n" + + "Separate '%s' into 2 proto_library rules.") % ( + target_label, + rule_name, + ", ".join([f.short_path for f in excluded]), + ", ".join([f.short_path for f in included]), + target_label, + )) + + return bool(included) + +def _declare_generated_files( + actions, + proto_info, + extension, + name_mapper = None): + """Declares generated files with a specific extension. + + Use this in lang_proto_library-es when protocol compiler generates files + that correspond to .proto file names. + + The function removes ".proto" extension with given one (e.g. ".pb.cc") and + declares new output files. + + Args: + actions: (ActionFactory) Obtained by ctx.actions, used to declare the files. + proto_info: (ProtoInfo) The ProtoInfo to declare the files for. + extension: (str) The extension to use for generated files. + name_mapper: (str->str) A function mapped over the base filename without + the extension. Used it to replace characters in the name that + cause problems in a specific programming language. + + Returns: + (list[File]) The list of declared files. + """ + proto_sources = proto_info.direct_sources + outputs = [] + + for src in proto_sources: + basename_no_ext = src.basename[:-(len(src.extension) + 1)] + + if name_mapper: + basename_no_ext = name_mapper(basename_no_ext) + + # Note that two proto_library rules can have the same source file, so this is actually a + # shared action. NB: This can probably result in action conflicts if the proto_library rules + # are not the same. + outputs.append(actions.declare_file(basename_no_ext + extension, sibling = src)) + + return outputs + +proto_common_do_not_use = struct( + compile = _compile, + declare_generated_files = _declare_generated_files, + experimental_should_generate_code = _experimental_should_generate_code, + experimental_filter_sources = _experimental_filter_sources, + ProtoLangToolchainInfo = ProtoLangToolchainInfo, +) diff --git a/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain.bzl b/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain.bzl new file mode 100644 index 00000000000000..1fd63f7632298e --- /dev/null +++ b/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain.bzl @@ -0,0 +1,89 @@ +# Copyright 2021 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. + +"""A Starlark implementation of the proto_lang_toolchain rule.""" + +load(":common/proto/proto_common.bzl", "ProtoLangToolchainInfo") +load(":common/proto/proto_semantics.bzl", "semantics") + +ProtoInfo = _builtins.toplevel.ProtoInfo + +def _rule_impl(ctx): + provided_proto_sources = depset(transitive = [bp[ProtoInfo].transitive_proto_sources() for bp in ctx.attr.blacklisted_protos]).to_list() + + flag = ctx.attr.command_line + if flag.find("$(PLUGIN_OUT)") > -1: + fail("in attribute 'command_line': Placeholder '$(PLUGIN_OUT)' is not supported.") + flag = flag.replace("$(OUT)", "%s") + + plugin = None + if ctx.attr.plugin != None: + plugin = ctx.attr.plugin[DefaultInfo].files_to_run + + proto_compiler = getattr(ctx.attr, "proto_compiler", None) + proto_compiler = getattr(ctx.attr, "_proto_compiler", proto_compiler) + + return [ + DefaultInfo( + files = depset(), + runfiles = ctx.runfiles(), + ), + ProtoLangToolchainInfo( + out_replacement_format_flag = flag, + plugin_format_flag = ctx.attr.plugin_format_flag, + plugin = plugin, + runtime = ctx.attr.runtime, + provided_proto_sources = provided_proto_sources, + proto_compiler = proto_compiler.files_to_run, + protoc_opts = ctx.fragments.proto.experimental_protoc_opts, + progress_message = ctx.attr.progress_message, + mnemonic = ctx.attr.mnemonic, + ), + ] + +def make_proto_lang_toolchain(custom_proto_compiler): + return rule( + _rule_impl, + attrs = dict( + { + "progress_message": attr.string(default = "Generating proto_library %{label}"), + "mnemonic": attr.string(default = "GenProto"), + "command_line": attr.string(mandatory = True), + "plugin_format_flag": attr.string(), + "plugin": attr.label( + executable = True, + cfg = "exec", + ), + "runtime": attr.label(), + "blacklisted_protos": attr.label_list( + providers = [ProtoInfo], + ), + }, + **({ + "proto_compiler": attr.label( + cfg = "exec", + executable = True, + ), + } if custom_proto_compiler else { + "_proto_compiler": attr.label( + cfg = "exec", + executable = True, + allow_files = True, + default = configuration_field("proto", "proto_compiler"), + ), + }) + ), + provides = [ProtoLangToolchainInfo], + fragments = ["proto"] + semantics.EXTRA_FRAGMENTS, + ) diff --git a/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain_custom_protoc.bzl b/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain_custom_protoc.bzl new file mode 100644 index 00000000000000..b4157f6aef66ad --- /dev/null +++ b/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain_custom_protoc.bzl @@ -0,0 +1,23 @@ +# Copyright 2021 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. + +"""Defines a proto_lang_toolchain rule class with custom proto compiler. + +There are two physical rule classes for proto_lang_toolchain and we want both of them +to have a name string of "proto_lang_toolchain". +""" + +load(":common/proto/proto_lang_toolchain.bzl", "make_proto_lang_toolchain") + +proto_lang_toolchain = make_proto_lang_toolchain(custom_proto_compiler = True) diff --git a/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain_default_protoc.bzl b/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain_default_protoc.bzl new file mode 100644 index 00000000000000..7345bc37fc0013 --- /dev/null +++ b/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain_default_protoc.bzl @@ -0,0 +1,23 @@ +# Copyright 2021 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. + +"""Defines a proto_lang_toolchain rule class with default proto compiler. + +There are two physical rule classes for proto_lang_toolchain and we want both of them +to have a name string of "proto_lang_toolchain". +""" + +load(":common/proto/proto_lang_toolchain.bzl", "make_proto_lang_toolchain") + +proto_lang_toolchain = make_proto_lang_toolchain(custom_proto_compiler = False) diff --git a/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain_wrapper.bzl b/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain_wrapper.bzl new file mode 100644 index 00000000000000..075e6d73c1c2c0 --- /dev/null +++ b/src/main/starlark/builtins_bzl/common/proto/proto_lang_toolchain_wrapper.bzl @@ -0,0 +1,35 @@ +# Copyright 2021 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. + +"""Macro encapsulating the proto_lang_toolchain implementation. + +This is needed since proto compiler can be defined, or used as a default one. +There are two implementations of proto_lang_toolchain - one with public proto_compiler attribute, and the other one with private compiler. +""" + +load(":common/proto/proto_lang_toolchain_default_protoc.bzl", toolchain_default_protoc = "proto_lang_toolchain") +load(":common/proto/proto_lang_toolchain_custom_protoc.bzl", toolchain_custom_protoc = "proto_lang_toolchain") + +def proto_lang_toolchain( + proto_compiler = None, + **kwargs): + if proto_compiler != None: + toolchain_custom_protoc( + proto_compiler = proto_compiler, + **kwargs + ) + else: + toolchain_default_protoc( + **kwargs + ) diff --git a/src/main/starlark/builtins_bzl/common/proto/proto_semantics.bzl b/src/main/starlark/builtins_bzl/common/proto/proto_semantics.bzl new file mode 100644 index 00000000000000..2d96efab970963 --- /dev/null +++ b/src/main/starlark/builtins_bzl/common/proto/proto_semantics.bzl @@ -0,0 +1,21 @@ +# Copyright 2021 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. + +""" +Proto Semantics +""" + +semantics = struct( + EXTRA_FRAGMENTS = [], +) diff --git a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java index 110667fe880406..10e62bf50d5994 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/semantics/ConsistencyTest.java @@ -130,6 +130,7 @@ private static BuildLanguageOptions buildRandomOptions(Random rand) throws Excep "--incompatible_allow_tags_propagation=" + rand.nextBoolean(), // flag, Java names differ "--experimental_cc_shared_library=" + rand.nextBoolean(), "--experimental_repo_remote_exec=" + rand.nextBoolean(), + "--experimental_action_resource_set=" + rand.nextBoolean(), "--incompatible_always_check_depset_elements=" + rand.nextBoolean(), "--incompatible_depset_for_libraries_to_link_getter=" + rand.nextBoolean(), "--incompatible_disable_target_provider_fields=" + rand.nextBoolean(), @@ -171,6 +172,7 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) { .setBool(BuildLanguageOptions.EXPERIMENTAL_ALLOW_TAGS_PROPAGATION, rand.nextBoolean()) .setBool(BuildLanguageOptions.EXPERIMENTAL_CC_SHARED_LIBRARY, rand.nextBoolean()) .setBool(BuildLanguageOptions.EXPERIMENTAL_REPO_REMOTE_EXEC, rand.nextBoolean()) + .setBool(BuildLanguageOptions.EXPERIMENTAL_ACTION_RESOURCE_SET, rand.nextBoolean()) .setBool(BuildLanguageOptions.INCOMPATIBLE_ALWAYS_CHECK_DEPSET_ELEMENTS, rand.nextBoolean()) .setBool( BuildLanguageOptions.INCOMPATIBLE_DEPSET_FOR_LIBRARIES_TO_LINK_GETTER, diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BUILD b/src/test/java/com/google/devtools/build/lib/rules/proto/BUILD index a3adb78806ba3a..52cab96a1ff8bf 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/proto/BUILD +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/BUILD @@ -12,6 +12,27 @@ filegroup( visibility = ["//src:__subpackages__"], ) +java_test( + name = "BazelProtoCommonTest", + srcs = ["BazelProtoCommonTest.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", + "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", + "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", + "//src/main/java/com/google/devtools/build/lib/analysis:file_provider", + "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/packages", + "//src/main/java/com/google/devtools/build/lib/util:os", + "//src/test/java/com/google/devtools/build/lib/actions/util", + "//src/test/java/com/google/devtools/build/lib/analysis/util", + "//src/test/java/com/google/devtools/build/lib/packages:testutil", + "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants", + "//third_party:junit4", + "//third_party:truth", + "@com_google_testparameterinjector//:testparameterinjector", + ], +) + java_test( name = "ProtoCompileActionBuilderTest", srcs = ["ProtoCompileActionBuilderTest.java"], @@ -25,6 +46,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/collect/nestedset", "//src/main/java/com/google/devtools/build/lib/rules/proto", "//src/main/java/com/google/devtools/build/lib/util", + "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", @@ -43,8 +65,8 @@ java_test( deps = [ "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection", "//src/main/java/com/google/devtools/build/lib/cmdline", + "//src/main/java/com/google/devtools/build/lib/packages", "//src/main/java/com/google/devtools/build/lib/rules/proto", - "//src/test/java/com/google/devtools/build/lib/actions/util", "//src/test/java/com/google/devtools/build/lib/analysis/util", "//src/test/java/com/google/devtools/build/lib/packages:testutil", "//src/test/java/com/google/devtools/build/lib/testutil:TestConstants", diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoCommonTest.java new file mode 100644 index 00000000000000..161d5e27b5fa7e --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/BazelProtoCommonTest.java @@ -0,0 +1,613 @@ +// Copyright 2022 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.rules.proto; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.prettyArtifactNames; + +import com.google.common.truth.Correspondence; +import com.google.devtools.build.lib.actions.ResourceSet; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; +import com.google.devtools.build.lib.analysis.FileProvider; +import com.google.devtools.build.lib.analysis.actions.SpawnAction; +import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.StarlarkInfo; +import com.google.devtools.build.lib.packages.StarlarkProvider; +import com.google.devtools.build.lib.packages.StarlarkProviderIdentifier; +import com.google.devtools.build.lib.packages.util.MockProtoSupport; +import com.google.devtools.build.lib.testutil.TestConstants; +import com.google.devtools.build.lib.util.OS; +import com.google.testing.junit.testparameterinjector.TestParameterInjector; +import com.google.testing.junit.testparameterinjector.TestParameters; +import java.util.List; +import java.util.regex.Pattern; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** Unit test for proto_common module. */ +@RunWith(TestParameterInjector.class) +public class BazelProtoCommonTest extends BuildViewTestCase { + private static final Correspondence MATCHES_REGEX = + Correspondence.from((a, b) -> Pattern.matches(b, a), "matches"); + + private static final StarlarkProviderIdentifier boolProviderId = + StarlarkProviderIdentifier.forKey( + new StarlarkProvider.Key( + Label.parseAbsoluteUnchecked("//foo:should_generate.bzl"), "BoolProvider")); + + @Before + public final void setup() throws Exception { + MockProtoSupport.setupWorkspace(scratch); + invalidatePackages(); + + MockProtoSupport.setup(mockToolsConfig); + + scratch.file( + "third_party/x/BUILD", + "licenses(['unencumbered'])", + "cc_binary(name = 'plugin', srcs = ['plugin.cc'])", + "cc_library(name = 'runtime', srcs = ['runtime.cc'])", + "filegroup(name = 'descriptors', srcs = ['metadata.proto', 'descriptor.proto'])", + "filegroup(name = 'any', srcs = ['any.proto'])", + "filegroup(name = 'something', srcs = ['something.proto'])", + "proto_library(name = 'mixed', srcs = [':descriptors', ':something'])", + "proto_library(name = 'denied', srcs = [':descriptors', ':any'])"); + scratch.file( + "foo/BUILD", + TestConstants.LOAD_PROTO_LANG_TOOLCHAIN, + "proto_lang_toolchain(", + " name = 'toolchain',", + " command_line = '--java_out=param1,param2:$(OUT)',", + " plugin_format_flag = '--plugin=%s',", + " plugin = '//third_party/x:plugin',", + " runtime = '//third_party/x:runtime',", + " blacklisted_protos = ['//third_party/x:denied'],", + " progress_message = 'Progress Message %{label}',", + " mnemonic = 'MyMnemonic',", + ")", + "proto_lang_toolchain(", + " name = 'toolchain_noplugin',", + " command_line = '--java_out=param1,param2:$(OUT)',", + " runtime = '//third_party/x:runtime',", + " blacklisted_protos = ['//third_party/x:denied'],", + " progress_message = 'Progress Message %{label}',", + " mnemonic = 'MyMnemonic',", + ")"); + + scratch.file( + "foo/generate.bzl", + "def _resource_set_callback(os, inputs_size):", + " return {'memory': 25 + 0.15 * inputs_size, 'cpu': 1}", + "def _impl(ctx):", + " outfile = ctx.actions.declare_file('out')", + " kwargs = {}", + " if ctx.attr.plugin_output:", + " kwargs['plugin_output'] = ctx.attr.plugin_output", + " if ctx.attr.additional_args:", + " additional_args = ctx.actions.args()", + " additional_args.add_all(ctx.attr.additional_args)", + " kwargs['additional_args'] = additional_args", + " if ctx.files.additional_tools:", + " kwargs['additional_tools'] = [f[DefaultInfo].files_to_run for f in ctx.attr.additional_tools]", + " if ctx.files.additional_inputs:", + " kwargs['additional_inputs'] = depset(ctx.files.additional_inputs)", + " if ctx.attr.use_resource_set:", + " kwargs['resource_set'] = _resource_set_callback", + " if ctx.attr.progress_message:", + " kwargs['experimental_progress_message'] = ctx.attr.progress_message", + " proto_common_do_not_use.compile(", + " ctx.actions,", + " ctx.attr.proto_dep[ProtoInfo],", + " ctx.attr.toolchain[proto_common_do_not_use.ProtoLangToolchainInfo],", + " [outfile],", + " **kwargs)", + " return [DefaultInfo(files = depset([outfile]))]", + "generate_rule = rule(_impl,", + " attrs = {", + " 'proto_dep': attr.label(),", + " 'plugin_output': attr.string(),", + " 'toolchain': attr.label(default = '//foo:toolchain'),", + " 'additional_args': attr.string_list(),", + " 'additional_tools': attr.label_list(cfg = 'exec'),", + " 'additional_inputs': attr.label_list(allow_files = True),", + " 'use_resource_set': attr.bool(),", + " 'progress_message': attr.string(),", + " })"); + + scratch.file( + "foo/should_generate.bzl", + "BoolProvider = provider()", + "def _impl(ctx):", + " result = proto_common_do_not_use.experimental_should_generate_code(", + " ctx.attr.proto_dep[ProtoInfo],", + " ctx.attr.toolchain[proto_common_do_not_use.ProtoLangToolchainInfo],", + " 'MyRule',", + " ctx.attr.proto_dep.label)", + " return [BoolProvider(value = result)]", + "should_generate_rule = rule(_impl,", + " attrs = {", + " 'proto_dep': attr.label(),", + " 'toolchain': attr.label(default = '//foo:toolchain'),", + " })"); + + scratch.file( + "foo/declare_generated_files.bzl", + "def _impl(ctx):", + " files = proto_common_do_not_use.declare_generated_files(", + " ctx.actions,", + " ctx.attr.proto_dep[ProtoInfo],", + " ctx.attr.extension,", + " (lambda s: s.replace('-','_').replace('.','/')) if ctx.attr.python_names else None)", + " for f in files:", + " ctx.actions.write(f, '')", + " return [DefaultInfo(files = depset(files))]", + "declare_generated_files = rule(_impl,", + " attrs = {", + " 'proto_dep': attr.label(),", + " 'extension': attr.string(),", + " 'python_names': attr.bool(default = False),", + " })"); + } + + /** Verifies basic usage of proto_common.generate_code. */ + @Test + public void generateCode_basic() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "generate_rule(name = 'simple', proto_dep = ':proto')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + SpawnAction spawnAction = getGeneratingSpawnAction(getBinArtifact("out", target)); + List cmdLine = spawnAction.getRemainingArguments(); + assertThat(cmdLine) + .comparingElementsUsing(MATCHES_REGEX) + .containsExactly( + "--plugin=bl?azel?-out/[^/]*-exec-[^/]*/bin/third_party/x/plugin", + "-Ibar/A.proto=bar/A.proto", + "bar/A.proto") + .inOrder(); + assertThat(spawnAction.getMnemonic()).isEqualTo("MyMnemonic"); + assertThat(spawnAction.getProgressMessage()).isEqualTo("Progress Message //bar:simple"); + } + + /** Verifies usage of proto_common.generate_code with no plugin specified by toolchain. */ + @Test + public void generateCode_noPlugin() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "generate_rule(name = 'simple', proto_dep = ':proto',", + " toolchain = '//foo:toolchain_noplugin')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + List cmdLine = + getGeneratingSpawnAction(getBinArtifact("out", target)).getRemainingArguments(); + assertThat(cmdLine) + .comparingElementsUsing(MATCHES_REGEX) + .containsExactly("-Ibar/A.proto=bar/A.proto", "bar/A.proto") + .inOrder(); + } + + /** + * Verifies usage of proto_common.generate_code with plugin_output + * parameter. + */ + @Test + public void generateCode_withPluginOutput() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "generate_rule(name = 'simple', proto_dep = ':proto', plugin_output = 'foo.srcjar')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + List cmdLine = + getGeneratingSpawnAction(getBinArtifact("out", target)).getRemainingArguments(); + assertThat(cmdLine) + .comparingElementsUsing(MATCHES_REGEX) + .containsExactly( + "--java_out=param1,param2:foo.srcjar", + "--plugin=bl?azel?-out/[^/]*-exec-[^/]*/bin/third_party/x/plugin", + "-Ibar/A.proto=bar/A.proto", + "bar/A.proto") + .inOrder(); + } + + /** + * Verifies usage of proto_common.generate_code with additional_args + * parameter. + */ + @Test + public void generateCode_additionalArgs() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "generate_rule(name = 'simple', proto_dep = ':proto', additional_args = ['--a', '--b'])"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + List cmdLine = + getGeneratingSpawnAction(getBinArtifact("out", target)).getRemainingArguments(); + assertThat(cmdLine) + .comparingElementsUsing(MATCHES_REGEX) + .containsExactly( + "--a", + "--b", + "--plugin=bl?azel?-out/[^/]*-exec-[^/]*/bin/third_party/x/plugin", + "-Ibar/A.proto=bar/A.proto", + "bar/A.proto") + .inOrder(); + } + + /** + * Verifies usage of proto_common.generate_code with additional_tools + * parameter. + */ + @Test + public void generateCode_additionalTools() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "cc_binary(name = 'tool1', srcs = ['tool1.cc'])", + "cc_binary(name = 'tool2', srcs = ['tool2.cc'])", + "generate_rule(name = 'simple', proto_dep = ':proto',", + " additional_tools = [':tool1', ':tool2'])"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + SpawnAction spawnAction = getGeneratingSpawnAction(getBinArtifact("out", target)); + assertThat(prettyArtifactNames(spawnAction.getTools())) + .containsAtLeast("bar/tool1", "bar/tool2", "third_party/x/plugin"); + } + + /** + * Verifies usage of proto_common.generate_code with additional_tools + * parameter and no plugin on the toolchain. + */ + @Test + public void generateCode_additionalToolsNoPlugin() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "cc_binary(name = 'tool1', srcs = ['tool1.cc'])", + "cc_binary(name = 'tool2', srcs = ['tool2.cc'])", + "generate_rule(name = 'simple',", + " proto_dep = ':proto',", + " additional_tools = [':tool1', ':tool2'],", + " toolchain = '//foo:toolchain_noplugin',", + ")"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + SpawnAction spawnAction = getGeneratingSpawnAction(getBinArtifact("out", target)); + assertThat(prettyArtifactNames(spawnAction.getTools())) + .containsAtLeast("bar/tool1", "bar/tool2"); + } + + /** + * Verifies usage of proto_common.generate_code with additional_inputs + * parameter. + */ + @Test + public void generateCode_additionalInputs() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "generate_rule(name = 'simple', proto_dep = ':proto',", + " additional_inputs = [':input1.txt', ':input2.txt'])"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + SpawnAction spawnAction = getGeneratingSpawnAction(getBinArtifact("out", target)); + assertThat(prettyArtifactNames(spawnAction.getInputs())) + .containsAtLeast("bar/input1.txt", "bar/input2.txt"); + } + + /** + * Verifies usage of proto_common.generate_code with resource_set + * parameter. + */ + @Test + public void generateCode_resourceSet() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "generate_rule(name = 'simple', proto_dep = ':proto', use_resource_set = True)"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + SpawnAction spawnAction = getGeneratingSpawnAction(getBinArtifact("out", target)); + assertThat(spawnAction.getResourceSetOrBuilder().buildResourceSet(OS.DARWIN, 0)) + .isEqualTo(ResourceSet.createWithRamCpu(25, 1)); + assertThat(spawnAction.getResourceSetOrBuilder().buildResourceSet(OS.LINUX, 2)) + .isEqualTo(ResourceSet.createWithRamCpu(25.3, 1)); + } + + /** Verifies --protocopts are passed to command line. */ + @Test + public void generateCode_protocOpts() throws Exception { + useConfiguration("--protocopt=--foo", "--protocopt=--bar"); + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "generate_rule(name = 'simple', proto_dep = ':proto')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + SpawnAction spawnAction = getGeneratingSpawnAction(getBinArtifact("out", target)); + List cmdLine = spawnAction.getRemainingArguments(); + assertThat(cmdLine) + .comparingElementsUsing(MATCHES_REGEX) + .containsExactly( + "--plugin=bl?azel?-out/[^/]*-exec-[^/]*/bin/third_party/x/plugin", + "--foo", + "--bar", + "-Ibar/A.proto=bar/A.proto", + "bar/A.proto") + .inOrder(); + } + + /** + * Verifies proto_common.generate_code correctly handles direct generated + * .proto files. + */ + @Test + public void generateCode_directGeneratedProtos() throws Exception { + useConfiguration("--noincompatible_generated_protos_in_virtual_imports"); + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "genrule(name = 'generate', srcs = ['A.txt'], cmd = '', outs = ['G.proto'])", + "proto_library(name = 'proto', srcs = ['A.proto', 'G.proto'])", + "generate_rule(name = 'simple', proto_dep = ':proto')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + SpawnAction spawnAction = getGeneratingSpawnAction(getBinArtifact("out", target)); + List cmdLine = spawnAction.getRemainingArguments(); + assertThat(cmdLine) + .comparingElementsUsing(MATCHES_REGEX) + .containsExactly( + "--plugin=bl?azel?-out/[^/]*-exec-[^/]*/bin/third_party/x/plugin", + "-Ibar/A.proto=bar/A.proto", + "-Ibar/G.proto=bl?azel?-out/k8-fastbuild/bin/bar/G.proto", + "bar/A.proto", + "bl?azel?-out/k8-fastbuild/bin/bar/G.proto") + .inOrder(); + } + + /** + * Verifies proto_common.generate_code correctly handles in-direct generated + * .proto files. + */ + @Test + public void generateCode_inDirectGeneratedProtos() throws Exception { + useConfiguration("--noincompatible_generated_protos_in_virtual_imports"); + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "genrule(name = 'generate', srcs = ['A.txt'], cmd = '', outs = ['G.proto'])", + "proto_library(name = 'generated', srcs = ['G.proto'])", + "proto_library(name = 'proto', srcs = ['A.proto'], deps = [':generated'])", + "generate_rule(name = 'simple', proto_dep = ':proto')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + SpawnAction spawnAction = getGeneratingSpawnAction(getBinArtifact("out", target)); + List cmdLine = spawnAction.getRemainingArguments(); + assertThat(cmdLine) + .comparingElementsUsing(MATCHES_REGEX) + .containsExactly( + "--plugin=bl?azel?-out/[^/]*-exec-[^/]*/bin/third_party/x/plugin", + "-Ibar/A.proto=bar/A.proto", + "-Ibar/G.proto=bl?azel?-out/k8-fastbuild/bin/bar/G.proto", + "bar/A.proto") + .inOrder(); + } + + /** + * Verifies proto_common.generate_code correctly handles external proto_library + * -es. + */ + @Test + @TestParameters({ + "{virtual: false, sibling: false, generated: false, expectedFlags:" + + " ['--proto_path=external/foo','-Ie/E.proto=external/foo/e/E.proto']}", + "{virtual: true, sibling: false, generated: false,expectedFlags:" + + " ['--proto_path=external/foo','-Ie/E.proto=external/foo/e/E.proto']}", + "{virtual: true, sibling: false, generated: true, expectedFlags:" + + " ['--proto_path=bl?azel?-out/k8-fastbuild/bin/external/foo/e/_virtual_imports/e'," + + " '-Ie/E.proto=bl?azel?-out/k8-fastbuild/bin/external/foo/e/_virtual_imports/e/e/E.proto']}", + "{virtual: true, sibling: true, generated: true, expectedFlags:" + + " ['--proto_path=bl?azel?-out/foo/k8-fastbuild/bin/e/_virtual_imports/e'," + + " '-Ie/E.proto=bl?azel?-out/foo/k8-fastbuild/bin/e/_virtual_imports/e/e/E.proto']}", + }) + public void generateCode_externalProtoLibrary( + boolean virtual, boolean sibling, boolean generated, List expectedFlags) + throws Exception { + if (virtual) { + useConfiguration("--incompatible_generated_protos_in_virtual_imports"); + } else { + useConfiguration("--noincompatible_generated_protos_in_virtual_imports"); + } + if (sibling) { + setBuildLanguageOptions("--experimental_sibling_repository_layout"); + } + scratch.appendFile("WORKSPACE", "local_repository(name = 'foo', path = '/foo')"); + invalidatePackages(); + scratch.file("/foo/WORKSPACE"); + scratch.file( + "/foo/e/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "proto_library(name='e', srcs=['E.proto'])", + generated + ? "genrule(name = 'generate', srcs = ['A.txt'], cmd = '', outs = ['E.proto'])" + : ""); + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'], deps = ['@foo//e:e'])", + "generate_rule(name = 'simple', proto_dep = ':proto')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + SpawnAction spawnAction = getGeneratingSpawnAction(getBinArtifact("out", target)); + List cmdLine = spawnAction.getRemainingArguments(); + assertThat(cmdLine) + .comparingElementsUsing(MATCHES_REGEX) + .containsExactly( + "--plugin=bl?azel?-out/[^/]*-exec-[^/]*/bin/third_party/x/plugin", + expectedFlags.get(0), + "-Ibar/A.proto=bar/A.proto", + expectedFlags.get(1), + "bar/A.proto") + .inOrder(); + } + + /** Verifies experimental_progress_message parameters. */ + @Test + public void generateCode_overrideProgressMessage() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:generate.bzl', 'generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "generate_rule(name = 'simple', proto_dep = ':proto', progress_message = 'My %{label}')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + SpawnAction spawnAction = getGeneratingSpawnAction(getBinArtifact("out", target)); + List cmdLine = spawnAction.getRemainingArguments(); + assertThat(cmdLine) + .comparingElementsUsing(MATCHES_REGEX) + .containsExactly( + "--plugin=bl?azel?-out/[^/]*-exec-[^/]*/bin/third_party/x/plugin", + "-Ibar/A.proto=bar/A.proto", + "bar/A.proto") + .inOrder(); + assertThat(spawnAction.getMnemonic()).isEqualTo("MyMnemonic"); + assertThat(spawnAction.getProgressMessage()).isEqualTo("My //bar:simple"); + } + + /** Verifies proto_common.should_generate_code call. */ + @Test + public void shouldGenerateCode_basic() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:should_generate.bzl', 'should_generate_rule')", + "proto_library(name = 'proto', srcs = ['A.proto'])", + "should_generate_rule(name = 'simple', proto_dep = ':proto')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + StarlarkInfo boolProvider = (StarlarkInfo) target.get(boolProviderId); + assertThat(boolProvider.getValue("value", Boolean.class)).isTrue(); + } + + /** Verifies proto_common.should_generate_code call. */ + @Test + public void shouldGenerateCode_dontGenerate() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:should_generate.bzl', 'should_generate_rule')", + "should_generate_rule(name = 'simple', proto_dep = '//third_party/x:denied')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + StarlarkInfo boolProvider = (StarlarkInfo) target.get(boolProviderId); + assertThat(boolProvider.getValue("value", Boolean.class)).isFalse(); + } + + /** Verifies proto_common.should_generate_code call. */ + @Test + public void shouldGenerateCode_mixed() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:should_generate.bzl', 'should_generate_rule')", + "should_generate_rule(name = 'simple', proto_dep = '//third_party/x:mixed')"); + + reporter.removeHandler(failFastHandler); + getConfiguredTarget("//bar:simple"); + + assertContainsEvent( + "The 'srcs' attribute of '//third_party/x:mixed' contains protos for which 'MyRule'" + + " shouldn't generate code (third_party/x/metadata.proto," + + " third_party/x/descriptor.proto), in addition to protos for which it should" + + " (third_party/x/something.proto).\n" + + "Separate '//third_party/x:mixed' into 2 proto_library rules."); + } + + /** Verifies proto_common.declare_generated_files call. */ + @Test + public void declareGenerateFiles_basic() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:declare_generated_files.bzl', 'declare_generated_files')", + "proto_library(name = 'proto', srcs = ['A.proto', 'b/B.proto'])", + "declare_generated_files(name = 'simple', proto_dep = ':proto', extension = '.cc')"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + assertThat(prettyArtifactNames(target.getProvider(FileProvider.class).getFilesToBuild())) + .containsExactly("bar/A.cc", "bar/b/B.cc"); + } + + /** Verifies proto_common.declare_generated_files call for Python. */ + @Test + public void declareGenerateFiles_pythonc() throws Exception { + scratch.file( + "bar/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "load('//foo:declare_generated_files.bzl', 'declare_generated_files')", + "proto_library(name = 'proto', srcs = ['my-proto.gen.proto'])", + "declare_generated_files(name = 'simple', proto_dep = ':proto', extension = '_pb2.py',", + " python_names = True)"); + + ConfiguredTarget target = getConfiguredTarget("//bar:simple"); + + assertThat(prettyArtifactNames(target.getProvider(FileProvider.class).getFilesToBuild())) + .containsExactly("bar/my_proto/gen_pb2.py"); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java index ce9edd7450bf05..f9b0168c8cdc3f 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoCompileActionBuilderTest.java @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.Exports; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.Services; import com.google.devtools.build.lib.rules.proto.ProtoCompileActionBuilder.ToolchainInvocation; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.util.OnDemandString; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.PathFragment; @@ -103,18 +104,27 @@ public void commandLine_basic() throws Exception { artifact("//:dont-care", "protoc-gen-javalite.exe")); ProtoLangToolchainProvider toolchainNoPlugin = - ProtoLangToolchainProvider.create( - "--java_out=param1,param2:$(OUT)", + ProtoLangToolchainProvider.createNative( + "--java_out=param1,param2:%s", + /* pluginFormatFlag= */ null, /* pluginExecutable= */ null, /* runtime= */ mock(TransitiveInfoCollection.class), - /* providedProtoSources= */ ImmutableList.of()); - + /* providedProtoSources= */ ImmutableList.of(), + /* protoc= */ FilesToRunProvider.EMPTY, + /* protocOpts= */ ImmutableList.of(), + /* progressMessage = */ "", + /* mnemonic= */ ""); ProtoLangToolchainProvider toolchainWithPlugin = - ProtoLangToolchainProvider.create( - "--$(PLUGIN_OUT)=param3,param4:$(OUT)", + ProtoLangToolchainProvider.createNative( + "--PLUGIN_pluginName_out=param3,param4:%s", + /* pluginFormatFlag= */ "--plugin=protoc-gen-PLUGIN_pluginName=%s", plugin, /* runtime= */ mock(TransitiveInfoCollection.class), - /* providedProtoSources= */ ImmutableList.of()); + /* providedProtoSources= */ ImmutableList.of(), + /* protoc= */ FilesToRunProvider.EMPTY, + /* protocOpts= */ ImmutableList.of(), + /* progressMessage = */ "", + /* mnemonic= */ ""); CustomCommandLine cmdLine = createCommandLineFromToolchains( @@ -173,11 +183,16 @@ public void commandline_derivedArtifact() throws Exception { @Test public void commandLine_strictDeps() throws Exception { ProtoLangToolchainProvider toolchain = - ProtoLangToolchainProvider.create( - "--java_out=param1,param2:$(OUT)", + ProtoLangToolchainProvider.createNative( + "--java_out=param1,param2:%s", + /* pluginFormatFlag= */ null, /* pluginExecutable= */ null, /* runtime= */ mock(TransitiveInfoCollection.class), - /* providedProtoSources= */ ImmutableList.of()); + /* providedProtoSources= */ ImmutableList.of(), + /* protoc= */ FilesToRunProvider.EMPTY, + /* protocOpts= */ ImmutableList.of(), + /* progressMessage = */ "", + /* mnemonic= */ ""); CustomCommandLine cmdLine = createCommandLineFromToolchains( @@ -211,11 +226,16 @@ public void commandLine_strictDeps() throws Exception { @Test public void commandLine_exports() throws Exception { ProtoLangToolchainProvider toolchain = - ProtoLangToolchainProvider.create( - "--java_out=param1,param2:$(OUT)", + ProtoLangToolchainProvider.createNative( + "--java_out=param1,param2:%s", + /* pluginFormatFlag= */ null, /* pluginExecutable= */ null, /* runtime= */ mock(TransitiveInfoCollection.class), - /* providedProtoSources= */ ImmutableList.of()); + /* providedProtoSources= */ ImmutableList.of(), + /* protoc= */ FilesToRunProvider.EMPTY, + /* protocOpts= */ ImmutableList.of(), + /* progressMessage = */ "", + /* mnemonic= */ ""); CustomCommandLine cmdLine = createCommandLineFromToolchains( @@ -281,11 +301,16 @@ public String toString() { }; ProtoLangToolchainProvider toolchain = - ProtoLangToolchainProvider.create( - "--java_out=param1,param2:$(OUT)", + ProtoLangToolchainProvider.createNative( + "--java_out=param1,param2:%s", + /* pluginFormatFlag= */ null, /* pluginExecutable= */ null, /* runtime= */ mock(TransitiveInfoCollection.class), - /* providedProtoSources= */ ImmutableList.of()); + /* providedProtoSources= */ ImmutableList.of(), + /* protoc= */ FilesToRunProvider.EMPTY, + /* protocOpts= */ ImmutableList.of(), + /* progressMessage = */ "", + /* mnemonic= */ ""); CustomCommandLine cmdLine = createCommandLineFromToolchains( @@ -315,18 +340,27 @@ public String toString() { @Test public void exceptionIfSameName() throws Exception { ProtoLangToolchainProvider toolchain1 = - ProtoLangToolchainProvider.create( - "dontcare", + ProtoLangToolchainProvider.createNative( + "dontcare=%s", + /* pluginFormatFlag= */ null, /* pluginExecutable= */ null, /* runtime= */ mock(TransitiveInfoCollection.class), - /* providedProtoSources= */ ImmutableList.of()); - + /* providedProtoSources= */ ImmutableList.of(), + /* protoc= */ FilesToRunProvider.EMPTY, + /* protocOpts= */ ImmutableList.of(), + /* progressMessage = */ "", + /* mnemonic= */ ""); ProtoLangToolchainProvider toolchain2 = - ProtoLangToolchainProvider.create( - "dontcare", + ProtoLangToolchainProvider.createNative( + "dontcare=%s", + /* pluginFormatFlag= */ null, /* pluginExecutable= */ null, /* runtime= */ mock(TransitiveInfoCollection.class), - /* providedProtoSources= */ ImmutableList.of()); + /* providedProtoSources= */ ImmutableList.of(), + /* protoc= */ FilesToRunProvider.EMPTY, + /* protocOpts= */ ImmutableList.of(), + /* progressMessage = */ "", + /* mnemonic= */ ""); IllegalStateException e = assertThrows( @@ -460,17 +494,12 @@ public void testEstimateResourceConsumptionLocal() throws Exception { assertThat( new ProtoCompileActionBuilder.ProtoCompileResourceSetBuilder() - .buildResourceSet(NestedSetBuilder.emptySet(STABLE_ORDER))) + .buildResourceSet(OS.DARWIN, 0)) .isEqualTo(ResourceSet.createWithRamCpu(25, 1)); assertThat( new ProtoCompileActionBuilder.ProtoCompileResourceSetBuilder() - .buildResourceSet( - NestedSetBuilder.wrap( - STABLE_ORDER, - ImmutableList.of( - artifact("//:dont-care", "protoc-gen-javalite.exe"), - artifact("//:dont-care-2", "protoc-gen-javalite-2.exe"))))) + .buildResourceSet(OS.LINUX, 2)) .isEqualTo(ResourceSet.createWithRamCpu(25.3, 1)); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java index 55c49ea1d8689f..2a0d47fe52f792 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/ProtoLangToolchainTest.java @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.rules.proto; import static com.google.common.truth.Truth.assertThat; -import static com.google.devtools.build.lib.actions.util.ActionsTestUtil.prettyArtifactNames; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -23,6 +22,9 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.packages.Provider; +import com.google.devtools.build.lib.packages.StarlarkProvider; import com.google.devtools.build.lib.packages.util.MockProtoSupport; import com.google.devtools.build.lib.testutil.TestConstants; import org.junit.Before; @@ -30,17 +32,26 @@ import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +/** Unit tests for {@code proto_lang_toolchain}. */ @RunWith(JUnit4.class) public class ProtoLangToolchainTest extends BuildViewTestCase { @Before public void setUp() throws Exception { MockProtoSupport.setupWorkspace(scratch); MockProtoSupport.setup(mockToolsConfig); + useConfiguration("--protocopt=--myflag"); invalidatePackages(); } + Provider.Key getStarlarkProtoLangToolchainInfoKey() throws LabelSyntaxException { + return new StarlarkProvider.Key( + Label.parseAbsolute("@_builtins//:common/proto/proto_common.bzl", ImmutableMap.of()), + "ProtoLangToolchainInfo"); + } + private void validateProtoLangToolchain(ProtoLangToolchainProvider toolchain) throws Exception { - assertThat(toolchain.commandLine()).isEqualTo("cmd-line"); + assertThat(toolchain.outReplacementFormatFlag()).isEqualTo("cmd-line:%s"); + assertThat(toolchain.pluginFormatFlag()).isEqualTo("--plugin=%s"); assertThat(toolchain.pluginExecutable().getExecutable().getRootRelativePathString()) .isEqualTo("third_party/x/plugin"); @@ -48,11 +59,15 @@ private void validateProtoLangToolchain(ProtoLangToolchainProvider toolchain) th assertThat(runtimes.getLabel()) .isEqualTo(Label.parseAbsolute("//third_party/x:runtime", ImmutableMap.of())); - assertThat(prettyArtifactNames(toolchain.forbiddenProtos())) - .containsExactly( - "third_party/x/metadata.proto", - "third_party/x/descriptor.proto", - "third_party/x/any.proto"); + assertThat(toolchain.protocOpts()).containsExactly("--myflag"); + + assertThat(toolchain.progressMessage()).isEqualTo("Progress Message %{label}"); + assertThat(toolchain.mnemonic()).isEqualTo("MyMnemonic"); + } + + private void validateProtoCompiler(ProtoLangToolchainProvider toolchain, Label protoCompiler) { + assertThat(toolchain.protoc().getExecutable().prettyPrint()) + .isEqualTo(protoCompiler.toPathFragment().getPathString()); } @Test @@ -72,16 +87,56 @@ public void protoToolchain() throws Exception { "licenses(['unencumbered'])", "proto_lang_toolchain(", " name = 'toolchain',", - " command_line = 'cmd-line',", + " command_line = 'cmd-line:$(OUT)',", + " plugin_format_flag = '--plugin=%s',", " plugin = '//third_party/x:plugin',", " runtime = '//third_party/x:runtime',", - " blacklisted_protos = ['//third_party/x:denied']", + " progress_message = 'Progress Message %{label}',", + " mnemonic = 'MyMnemonic',", ")"); update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus()); + ProtoLangToolchainProvider toolchain = + ProtoLangToolchainProvider.get(getConfiguredTarget("//foo:toolchain")); + Label protoc = Label.parseAbsoluteUnchecked(ProtoConstants.DEFAULT_PROTOC_LABEL); - validateProtoLangToolchain( - getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class)); + validateProtoLangToolchain(toolchain); + validateProtoCompiler(toolchain, protoc); + } + + @Test + public void protoToolchain_setProtoCompiler() throws Exception { + scratch.file( + "third_party/x/BUILD", + "licenses(['unencumbered'])", + "cc_binary(name = 'plugin', srcs = ['plugin.cc'])", + "cc_library(name = 'runtime', srcs = ['runtime.cc'])", + "filegroup(name = 'descriptors', srcs = ['metadata.proto', 'descriptor.proto'])", + "filegroup(name = 'any', srcs = ['any.proto'])", + "proto_library(name = 'denied', srcs = [':descriptors', ':any'])", + "cc_binary(name = 'compiler')"); + + scratch.file( + "foo/BUILD", + TestConstants.LOAD_PROTO_LANG_TOOLCHAIN, + "licenses(['unencumbered'])", + "proto_lang_toolchain(", + " name = 'toolchain',", + " command_line = 'cmd-line:$(OUT)',", + " plugin_format_flag = '--plugin=%s',", + " plugin = '//third_party/x:plugin',", + " runtime = '//third_party/x:runtime',", + " progress_message = 'Progress Message %{label}',", + " mnemonic = 'MyMnemonic',", + " proto_compiler = '//third_party/x:compiler',", + ")"); + + ProtoLangToolchainProvider toolchain = + ProtoLangToolchainProvider.get(getConfiguredTarget("//foo:toolchain")); + Label protoc = Label.parseAbsoluteUnchecked("//third_party/x:compiler"); + + validateProtoLangToolchain(toolchain); + validateProtoCompiler(toolchain, protoc); } @Test @@ -100,16 +155,21 @@ public void protoToolchainBlacklistProtoLibraries() throws Exception { TestConstants.LOAD_PROTO_LANG_TOOLCHAIN, "proto_lang_toolchain(", " name = 'toolchain',", - " command_line = 'cmd-line',", + " command_line = 'cmd-line:$(OUT)',", + " plugin_format_flag = '--plugin=%s',", " plugin = '//third_party/x:plugin',", " runtime = '//third_party/x:runtime',", - " blacklisted_protos = ['//third_party/x:descriptors', '//third_party/x:any']", + " progress_message = 'Progress Message %{label}',", + " mnemonic = 'MyMnemonic',", ")"); update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus()); + ProtoLangToolchainProvider toolchain = + ProtoLangToolchainProvider.get(getConfiguredTarget("//foo:toolchain")); + Label protoc = Label.parseAbsoluteUnchecked(ProtoConstants.DEFAULT_PROTOC_LABEL); - validateProtoLangToolchain( - getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class)); + validateProtoLangToolchain(toolchain); + validateProtoCompiler(toolchain, protoc); } @Test @@ -128,16 +188,21 @@ public void protoToolchainBlacklistTransitiveProtos() throws Exception { TestConstants.LOAD_PROTO_LANG_TOOLCHAIN, "proto_lang_toolchain(", " name = 'toolchain',", - " command_line = 'cmd-line',", + " command_line = 'cmd-line:$(OUT)',", + " plugin_format_flag = '--plugin=%s',", " plugin = '//third_party/x:plugin',", " runtime = '//third_party/x:runtime',", - " blacklisted_protos = ['//third_party/x:any']", + " progress_message = 'Progress Message %{label}',", + " mnemonic = 'MyMnemonic',", ")"); update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus()); + ProtoLangToolchainProvider toolchain = + ProtoLangToolchainProvider.get(getConfiguredTarget("//foo:toolchain")); + Label protoc = Label.parseAbsoluteUnchecked(ProtoConstants.DEFAULT_PROTOC_LABEL); - validateProtoLangToolchain( - getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class)); + validateProtoLangToolchain(toolchain); + validateProtoCompiler(toolchain, protoc); } @Test @@ -151,13 +216,11 @@ public void optionalFieldsAreEmpty() throws Exception { ")"); update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus()); - ProtoLangToolchainProvider toolchain = - getConfiguredTarget("//foo:toolchain").getProvider(ProtoLangToolchainProvider.class); + ProtoLangToolchainProvider.get(getConfiguredTarget("//foo:toolchain")); assertThat(toolchain.pluginExecutable()).isNull(); assertThat(toolchain.runtime()).isNull(); - assertThat(toolchain.blacklistedProtos().toList()).isEmpty(); - assertThat(toolchain.forbiddenProtos().toList()).isEmpty(); + assertThat(toolchain.mnemonic()).isEqualTo("GenProto"); } } diff --git a/src/test/java/com/google/devtools/build/lib/rules/proto/StarlarkProtoLangToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/proto/StarlarkProtoLangToolchainTest.java new file mode 100644 index 00000000000000..de33c3881fde17 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/rules/proto/StarlarkProtoLangToolchainTest.java @@ -0,0 +1,197 @@ +// Copyright 2016 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.rules.proto; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.eventbus.EventBus; +import com.google.devtools.build.lib.analysis.FilesToRunProvider; +import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; +import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.cmdline.LabelSyntaxException; +import com.google.devtools.build.lib.packages.Provider; +import com.google.devtools.build.lib.packages.StarlarkInfo; +import com.google.devtools.build.lib.packages.StarlarkProvider; +import com.google.devtools.build.lib.testutil.TestConstants; +import net.starlark.java.eval.Starlark; +import net.starlark.java.eval.StarlarkList; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Unit tests for {@code proto_lang_toolchain}. */ +@RunWith(JUnit4.class) +public class StarlarkProtoLangToolchainTest extends ProtoLangToolchainTest { + + @Override + @Before + public void setupStarlarkRule() throws Exception { + setBuildLanguageOptions("--experimental_builtins_injection_override=+proto_lang_toolchain"); + } + + Provider.Key getStarlarkProtoLangToolchainInfoKey() throws LabelSyntaxException { + return new StarlarkProvider.Key( + Label.parseAbsolute("@_builtins//:common/proto/providers.bzl", ImmutableMap.of()), + "ProtoLangToolchainInfo"); + } + + @SuppressWarnings("unchecked") + private void validateStarlarkProtoLangToolchain(StarlarkInfo toolchain) throws Exception { + assertThat(toolchain.getValue("out_replacement_format_flag")).isEqualTo("cmd-line:%s"); + assertThat(toolchain.getValue("plugin_format_flag")).isEqualTo("--plugin=%s"); + assertThat(toolchain.getValue("progress_message")).isEqualTo("Progress Message %{label}"); + assertThat(toolchain.getValue("mnemonic")).isEqualTo("MyMnemonic"); + assertThat(ImmutableList.copyOf((StarlarkList) toolchain.getValue("protoc_opts"))) + .containsExactly("--myflag"); + assertThat( + ((FilesToRunProvider) toolchain.getValue("plugin")) + .getExecutable() + .getRootRelativePathString()) + .isEqualTo("third_party/x/plugin"); + + TransitiveInfoCollection runtimes = (TransitiveInfoCollection) toolchain.getValue("runtime"); + assertThat(runtimes.getLabel()) + .isEqualTo(Label.parseAbsolute("//third_party/x:runtime", ImmutableMap.of())); + + Label protoc = Label.parseAbsoluteUnchecked(ProtoConstants.DEFAULT_PROTOC_LABEL); + assertThat( + ((FilesToRunProvider) toolchain.getValue("proto_compiler")) + .getExecutable() + .prettyPrint()) + .isEqualTo(protoc.toPathFragment().getPathString()); + } + + @Override + @Test + public void protoToolchain() throws Exception { + scratch.file( + "third_party/x/BUILD", + "licenses(['unencumbered'])", + "cc_binary(name = 'plugin', srcs = ['plugin.cc'])", + "cc_library(name = 'runtime', srcs = ['runtime.cc'])", + "filegroup(name = 'descriptors', srcs = ['metadata.proto', 'descriptor.proto'])", + "filegroup(name = 'any', srcs = ['any.proto'])", + "proto_library(name = 'denied', srcs = [':descriptors', ':any'])"); + + scratch.file( + "foo/BUILD", + TestConstants.LOAD_PROTO_LANG_TOOLCHAIN, + "licenses(['unencumbered'])", + "proto_lang_toolchain(", + " name = 'toolchain',", + " command_line = 'cmd-line:$(OUT)',", + " plugin_format_flag = '--plugin=%s',", + " plugin = '//third_party/x:plugin',", + " runtime = '//third_party/x:runtime',", + " progress_message = 'Progress Message %{label}',", + " mnemonic = 'MyMnemonic',", + ")"); + + update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus()); + + validateStarlarkProtoLangToolchain( + (StarlarkInfo) + getConfiguredTarget("//foo:toolchain").get(getStarlarkProtoLangToolchainInfoKey())); + } + + @Override + @Test + public void protoToolchainBlacklistProtoLibraries() throws Exception { + scratch.file( + "third_party/x/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "licenses(['unencumbered'])", + "cc_binary(name = 'plugin', srcs = ['plugin.cc'])", + "cc_library(name = 'runtime', srcs = ['runtime.cc'])", + "proto_library(name = 'descriptors', srcs = ['metadata.proto', 'descriptor.proto'])", + "proto_library(name = 'any', srcs = ['any.proto'], strip_import_prefix = '/third_party')"); + + scratch.file( + "foo/BUILD", + TestConstants.LOAD_PROTO_LANG_TOOLCHAIN, + "proto_lang_toolchain(", + " name = 'toolchain',", + " command_line = 'cmd-line:$(OUT)',", + " plugin_format_flag = '--plugin=%s',", + " plugin = '//third_party/x:plugin',", + " runtime = '//third_party/x:runtime',", + " progress_message = 'Progress Message %{label}',", + " mnemonic = 'MyMnemonic',", + ")"); + + update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus()); + + validateStarlarkProtoLangToolchain( + (StarlarkInfo) + getConfiguredTarget("//foo:toolchain").get(getStarlarkProtoLangToolchainInfoKey())); + } + + @Override + @Test + public void protoToolchainBlacklistTransitiveProtos() throws Exception { + scratch.file( + "third_party/x/BUILD", + TestConstants.LOAD_PROTO_LIBRARY, + "licenses(['unencumbered'])", + "cc_binary(name = 'plugin', srcs = ['plugin.cc'])", + "cc_library(name = 'runtime', srcs = ['runtime.cc'])", + "proto_library(name = 'descriptors', srcs = ['metadata.proto', 'descriptor.proto'])", + "proto_library(name = 'any', srcs = ['any.proto'], deps = [':descriptors'])"); + + scratch.file( + "foo/BUILD", + TestConstants.LOAD_PROTO_LANG_TOOLCHAIN, + "proto_lang_toolchain(", + " name = 'toolchain',", + " command_line = 'cmd-line:$(OUT)',", + " plugin_format_flag = '--plugin=%s',", + " plugin = '//third_party/x:plugin',", + " runtime = '//third_party/x:runtime',", + " progress_message = 'Progress Message %{label}',", + " mnemonic = 'MyMnemonic',", + ")"); + + update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus()); + + validateStarlarkProtoLangToolchain( + (StarlarkInfo) + getConfiguredTarget("//foo:toolchain").get(getStarlarkProtoLangToolchainInfoKey())); + } + + @Override + @Test + public void optionalFieldsAreEmpty() throws Exception { + scratch.file( + "foo/BUILD", + TestConstants.LOAD_PROTO_LANG_TOOLCHAIN, + "proto_lang_toolchain(", + " name = 'toolchain',", + " command_line = 'cmd-line:$(OUT)',", + ")"); + + update(ImmutableList.of("//foo:toolchain"), false, 1, true, new EventBus()); + + StarlarkInfo toolchain = + (StarlarkInfo) + getConfiguredTarget("//foo:toolchain").get(getStarlarkProtoLangToolchainInfoKey()); + + assertThat(toolchain.getValue("plugin")).isEqualTo(Starlark.NONE); + assertThat(toolchain.getValue("runtime")).isEqualTo(Starlark.NONE); + assertThat(toolchain.getValue("mnemonic")).isEqualTo("GenProto"); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java index a94299c3b43296..2bdfb5a52f978d 100644 --- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java @@ -321,7 +321,7 @@ public void testStandardError() throws Exception { public void testVerboseFailures() { ExecException e = assertThrows(ExecException.class, () -> run(createSpawn(getFalseCommand()))); ActionExecutionException actionExecutionException = - e.toActionExecutionException(new NullAction()); + ActionExecutionException.fromExecException(e, new NullAction()); assertWithMessage("got: " + actionExecutionException.getMessage()) .that(actionExecutionException.getMessage().contains("failed: error executing command")) .isTrue(); diff --git a/src/test/java/com/google/devtools/build/lib/starlark/BUILD b/src/test/java/com/google/devtools/build/lib/starlark/BUILD index 0f8a78aecbf428..f86649cd7c21ff 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/BUILD +++ b/src/test/java/com/google/devtools/build/lib/starlark/BUILD @@ -29,6 +29,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", + "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/actions:thread_state_receiver", "//src/main/java/com/google/devtools/build/lib/analysis:actions/parameter_file_write_action", "//src/main/java/com/google/devtools/build/lib/analysis:actions/substitution", @@ -67,6 +68,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:classpath", "//src/main/java/com/google/devtools/build/lib/util:filetype", + "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/common/options", diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java index dda0eb9c83ccfc..94964936fa1ae1 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleContextTest.java @@ -29,6 +29,8 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionAnalysisMetadata; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.actions.ResourceSet; import com.google.devtools.build.lib.analysis.ActionsProvider; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.ConfiguredTarget; @@ -54,6 +56,7 @@ import com.google.devtools.build.lib.starlark.util.BazelEvaluationTestCase; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.util.FileTypeSet; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; @@ -652,6 +655,232 @@ public void testCreateStarlarkActionArgumentsWithUnusedInputsList() throws Excep assertThat(action.isShareable()).isFalse(); } + @Test + public void testCreateStarlarkActionArgumentsWithResourceSet_success() throws Exception { + setBuildLanguageOptions("--experimental_action_resource_set"); + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + + ev.exec( + "def get_resources(os, inputs_size):", + " if os == \"osx\":", + " return {\"cpu\": 2., \"memory\": 350. + inputs_size * 20, \"local_test\": 2.}", + " return {\"cpu\": 1., \"memory\": 350. + inputs_size * 10, \"local_test\": 0.}", + "ruleContext.actions.run(", + " inputs = ruleContext.files.srcs,", + " outputs = ruleContext.files.srcs,", + " resource_set = get_resources,", + " executable = 'executable')"); + StarlarkAction action = + (StarlarkAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + + assertThat(action.getResourceSetOrBuilder().buildResourceSet(OS.LINUX, 2)) + .isEqualTo(ResourceSet.create(370, 1, 0)); + assertThat(action.getResourceSetOrBuilder().buildResourceSet(OS.DARWIN, 2)) + .isEqualTo(ResourceSet.create(390, 2, 2)); + } + + @Test + public void testCreateStarlarkActionArgumentsWithResourceSet_flagDisabled() throws Exception { + setBuildLanguageOptions("--noexperimental_action_resource_set"); + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + + ev.exec( + "def get_resources(os, inputs_size):", + " if os == \"osx\":", + " return {\"cpu\": 2., \"memory\": 350. + inputs_size * 20, \"local_test\": 2.}", + " return {\"cpu\": 1., \"memory\": 350. + inputs_size * 10, \"local_test\": 0.}", + "ruleContext.actions.run(", + " inputs = ruleContext.files.srcs,", + " outputs = ruleContext.files.srcs,", + " resource_set = get_resources,", + " executable = 'executable')"); + StarlarkAction action = + (StarlarkAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + + assertThat(action.getResourceSetOrBuilder().buildResourceSet(OS.LINUX, 2)) + .isEqualTo(ResourceSet.create(250, 1, 0)); + } + + @Test + public void testCreateStarlarkActionArgumentsWithResourceSet_lambdaForbidden() throws Exception { + setBuildLanguageOptions("--experimental_action_resource_set"); + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + + Exception thrown = + assertThrows( + EvalException.class, + () -> + ev.exec( + "ruleContext.actions.run(", + " inputs = ruleContext.files.srcs,", + " outputs = ruleContext.files.srcs,", + " resource_set = lambda os, inputs_size : {\"cpu\": 1., \"memory\": 1.," + + " \"local_test\": 1.} ,", + " executable = 'executable')")); + + assertThat(thrown).hasMessageThat().contains("must be declared by a top-level def statement"); + } + + @Test + public void testCreateStarlarkActionArgumentsWithResourceSet_illegalResource() throws Exception { + setBuildLanguageOptions("--experimental_action_resource_set"); + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + + ev.exec( + "def get_resources(os, inputs_size):", + " return {\"cpu\": 2., \"memory\": 350., \"local_test\": 2., \"gpu\": 1.}", + "ruleContext.actions.run(", + " inputs = ruleContext.files.srcs,", + " outputs = ruleContext.files.srcs,", + " resource_set = get_resources,", + " executable = 'executable')"); + StarlarkAction action = + (StarlarkAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + + Exception thrown = + assertThrows( + ExecException.class, + () -> action.getResourceSetOrBuilder().buildResourceSet(OS.LINUX, 2)); + assertThat(thrown).hasMessageThat().contains("Illegal resource keys: (gpu)"); + } + + @Test + public void testCreateStarlarkActionArgumentsWithResourceSet_defaultValue() throws Exception { + setBuildLanguageOptions("--experimental_action_resource_set"); + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + + ev.exec( + "def get_resources(os, inputs_size):", + " return {\"cpu\": 2., \"local_test\": 2.}", + "ruleContext.actions.run(", + " inputs = ruleContext.files.srcs,", + " outputs = ruleContext.files.srcs,", + " resource_set = get_resources,", + " executable = 'executable')"); + StarlarkAction action = + (StarlarkAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + + assertThat(action.getResourceSetOrBuilder().buildResourceSet(OS.LINUX, 2)) + .isEqualTo(ResourceSet.create(250, 2, 2)); + } + + @Test + public void testCreateStarlarkActionArgumentsWithResourceSet_intDict() throws Exception { + setBuildLanguageOptions("--experimental_action_resource_set"); + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + + ev.exec( + "def get_resources(os, inputs_size):", + " return {\"cpu\": 1, \"memory\": 2, \"local_test\": 3}", + "ruleContext.actions.run(", + " inputs = ruleContext.files.srcs,", + " outputs = ruleContext.files.srcs,", + " resource_set = get_resources,", + " executable = 'executable')"); + StarlarkAction action = + (StarlarkAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + + assertThat(action.getResourceSetOrBuilder().buildResourceSet(OS.LINUX, 0)) + .isEqualTo(ResourceSet.create(2, 1, 3)); + } + + @Test + public void testCreateStarlarkActionArgumentsWithResourceSet_notDict() throws Exception { + setBuildLanguageOptions("--experimental_action_resource_set"); + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + + ev.exec( + "def get_resources(os, inputs_size):", + " return \"keks\"", + "ruleContext.actions.run(", + " inputs = ruleContext.files.srcs,", + " outputs = ruleContext.files.srcs,", + " resource_set = get_resources,", + " executable = 'executable')"); + StarlarkAction action = + (StarlarkAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + + Exception thrown = + assertThrows( + ExecException.class, + () -> action.getResourceSetOrBuilder().buildResourceSet(OS.LINUX, 2)); + assertThat(thrown).hasMessageThat().contains("got string for 'resource_set', want dict"); + } + + @Test + public void testCreateStarlarkActionArgumentsWithResourceSet_wrongDict() throws Exception { + setBuildLanguageOptions("--experimental_action_resource_set"); + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + + ev.exec( + "def get_resources(os, inputs_size):", + " return {\"cpu\": 1, \"memory\": 2, \"local_test\": \"hi\"}", + "ruleContext.actions.run(", + " inputs = ruleContext.files.srcs,", + " outputs = ruleContext.files.srcs,", + " resource_set = get_resources,", + " executable = 'executable')"); + StarlarkAction action = + (StarlarkAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + + Exception thrown = + assertThrows( + ExecException.class, + () -> action.getResourceSetOrBuilder().buildResourceSet(OS.LINUX, 2)); + assertThat(thrown).hasMessageThat().contains("Illegal resource value type for key local_test"); + } + + @Test + public void testCreateStarlarkActionArgumentsWithResourceSet_incorrectSignature() + throws Exception { + setBuildLanguageOptions("--experimental_action_resource_set"); + StarlarkRuleContext ruleContext = createRuleContext("//foo:foo"); + setRuleContext(ruleContext); + + ev.exec( + "def get_resources(os):", + " return {\"cpu\": 1, \"memory\": 2, \"local_test\": \"hi\"}", + "ruleContext.actions.run(", + " inputs = ruleContext.files.srcs,", + " outputs = ruleContext.files.srcs,", + " resource_set = get_resources,", + " executable = 'executable')"); + StarlarkAction action = + (StarlarkAction) + Iterables.getOnlyElement( + ruleContext.getRuleContext().getAnalysisEnvironment().getRegisteredActions()); + + Exception thrown = + assertThrows( + ExecException.class, + () -> action.getResourceSetOrBuilder().buildResourceSet(OS.LINUX, 2)); + assertThat(thrown) + .hasMessageThat() + .contains("get_resources() accepts no more than 1 positional argument but got 2"); + } + @Test public void testCreateStarlarkActionArgumentsWithoutUnusedInputsList() throws Exception { StarlarkRuleContext ruleContext = createRuleContext("//foo:foo");