Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Cherry-pick proto_lang_toolchain Starlarkfication and proto_common module #15854

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
d9b403e
Put protoc label to a constant.
comius Nov 11, 2021
16b62f5
Remove proto_lang_toolchain rule's $(PLUGIN_OUT) placeholder and simp…
comius Nov 25, 2021
4158e52
Add plugin_format_flag attribute to ProtoLangToolchainRule
comius Nov 25, 2021
4099a43
Proxy proto compiler and proto opts over proto_lang_toolchain rule.
comius Mar 28, 2022
d53efdd
Extend proto_lang_toolchain rule with progress_message and mnemonic a…
comius Mar 28, 2022
b0181e7
Starlarkify proto_lang_toolchain and ProtoLangToolchainInfo provider
kotlaja Apr 25, 2022
92b9518
Roll forward of https://github.com/bazelbuild/bazel/commit/ae349e9368…
kotlaja May 4, 2022
1682183
Use data from transitive_proto_sources instead of transitive_sources …
comius May 6, 2022
fa217b1
Remove allow_files from proto_lang_toolchain's attributes
kotlaja May 6, 2022
4c0640b
Remove native implementation of proto_lang_toolchain rule
kotlaja May 6, 2022
a14e613
Fix name in proto_lang_toolchain rule
kotlaja May 6, 2022
cc6c523
Cherry-pick missing things.
comius Jul 13, 2022
d2ded6c
Separate ExecException.java from main actions target.
Nov 30, 2021
d887fe4
ResourceSet in StarlarkAction API
Dec 9, 2021
193399d
Implement and expose proto_common.compile call.
comius Apr 7, 2022
0049701
Fix ProtoCommon tests.
comius Jul 14, 2022
46f0658
Add experimental_progress_message parameter to proto_common.compile.
comius Apr 7, 2022
c4b8b95
Implement proto_common.experimental_should_generate_code.
comius Apr 7, 2022
600bd5e
Implement proto_common.declare_generated_files.
comius Apr 12, 2022
8b8f4e8
Fix docstring in proto_common
comius Apr 28, 2022
80d9ae2
Add proto_info parameter to proto_common.compile
comius Jun 17, 2022
bc452c1
Migrate proto_library_target to proto_info in proto_common.declare_ge…
comius Jun 22, 2022
0b381ac
Migrate proto_library_target to proto_info in proto_common.experiment…
comius Jul 11, 2022
3b78ca9
Remove proto_library_target from proto_common
comius Jul 12, 2022
18ae08c
Merge branch 'release-5.3.0' into cherry-pick-proto_lang_toolchain
ckolli5 Jul 14, 2022
e5427b2
Merge branch 'release-5.3.0' into cherry-pick-proto_lang_toolchain
ckolli5 Jul 14, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -115,6 +116,47 @@ private static NestedSet<Cause> 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;
Expand Down
12 changes: 10 additions & 2 deletions src/main/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"],
)
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,15 +36,15 @@ public class BaseSpawn implements Spawn {
private final ImmutableMap<String, String> executionInfo;
private final RunfilesSupplier runfilesSupplier;
private final ActionExecutionMetadata action;
private final ResourceSet localResources;
private final ResourceSetOrBuilder localResources;

public BaseSpawn(
List<String> arguments,
Map<String, String> environment,
Map<String, String> executionInfo,
RunfilesSupplier runfilesSupplier,
ActionExecutionMetadata action,
ResourceSet localResources) {
ResourceSetOrBuilder localResources) {
this.arguments = ImmutableList.copyOf(arguments);
this.environment = ImmutableMap.copyOf(environment);
this.executionInfo = ImmutableMap.copyOf(executionInfo);
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public ActionExecutionMetadata getResourceOwner() {
}

@Override
public ResourceSet getLocalResources() {
public ResourceSet getLocalResources() throws ExecException {
return spawn.getLocalResources();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -175,7 +175,7 @@ public String getTypeDescription() {
}

@Override
public ResourceSet buildResourceSet(NestedSet<Artifact> inputs) {
public ResourceSet buildResourceSet(OS os, int inputsSize) throws ExecException {
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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<Artifact> inputs);
public ResourceSet buildResourceSet(OS os, int inputsSize) throws ExecException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -590,8 +590,7 @@ private ActionSpawn(
executionInfo,
SpawnAction.this.getRunfilesSupplier(),
SpawnAction.this,
SpawnAction.this.resourceSetOrBuilder.buildResourceSet(inputs));

SpawnAction.this.resourceSetOrBuilder);
NestedSetBuilder<ActionInput> inputsBuilder = NestedSetBuilder.stableOrder();
ImmutableList<Artifact> manifests = getRunfilesSupplier().getManifests();
for (Artifact input : inputs.toList()) {
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down
Loading