Skip to content

Commit

Permalink
Rollback of commit a9b8457.
Browse files Browse the repository at this point in the history
*** Reason for rollback ***

Mutability violates the Action contract: this change breaks incremental builds.

*** Original change description ***

Propogate BAZEL_VERBOSE_FAILURES and BAZEL_SUBCOMMANDS to the execution environments of runtime tools.

--
MOS_MIGRATED_REVID=113958481
  • Loading branch information
calpeyser authored and davidzchen committed Feb 5, 2016
1 parent fbb8a0a commit 8378cd8
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 61 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,6 @@ void extend(ExtraActionInfo.Builder builder) {

private static final String GUID = "ebd6fce3-093e-45ee-adb6-bf513b602f0d";

private static final String VERBOSE_FAILURES_KEY = "BAZEL_VERBOSE_FAILURES";
private static final String SUBCOMMANDS_KEY = "BAZEL_SUBCOMMANDS";

private final CommandLine argv;

private final boolean executeUnconditionally;
Expand All @@ -95,13 +92,11 @@ void extend(ExtraActionInfo.Builder builder) {
private final ImmutableMap<PathFragment, Artifact> inputManifests;

private final ResourceSet resourceSet;
private ImmutableMap<String, String> environment;
private final ImmutableMap<String, String> environment;
private final ImmutableMap<String, String> executionInfo;

private final ExtraActionInfoSupplier<?> extraActionInfoSupplier;

private final boolean verboseFailuresAndSubcommandsInEnv;

/**
* Constructs a SpawnAction using direct initialization arguments.
* <p>
Expand Down Expand Up @@ -145,8 +140,7 @@ public SpawnAction(
ImmutableMap.<PathFragment, Artifact>of(),
mnemonic,
false,
null,
false);
null);
}

/**
Expand All @@ -171,9 +165,6 @@ public SpawnAction(
* @param inputManifests entries in inputs that are symlink manifest files.
* These are passed to remote execution in the environment rather than as inputs.
* @param mnemonic the mnemonic that is reported in the master log.
* @param verboseFailuresAndSubcommandsInEnv if the presense of "--verbose_failures" and/or
* "--subcommands" in the execution should be propogated to the environment of the
* action.
*/
public SpawnAction(
ActionOwner owner,
Expand All @@ -188,8 +179,7 @@ public SpawnAction(
ImmutableMap<PathFragment, Artifact> inputManifests,
String mnemonic,
boolean executeUnconditionally,
ExtraActionInfoSupplier<?> extraActionInfoSupplier,
boolean verboseFailuresAndSubcommandsInEnv) {
ExtraActionInfoSupplier<?> extraActionInfoSupplier) {
super(owner, tools, inputs, outputs);
this.resourceSet = resourceSet;
this.executionInfo = executionInfo;
Expand All @@ -200,7 +190,6 @@ public SpawnAction(
this.mnemonic = mnemonic;
this.executeUnconditionally = executeUnconditionally;
this.extraActionInfoSupplier = extraActionInfoSupplier;
this.verboseFailuresAndSubcommandsInEnv = verboseFailuresAndSubcommandsInEnv;
}

/**
Expand Down Expand Up @@ -257,22 +246,6 @@ protected void internalExecute(
public void execute(ActionExecutionContext actionExecutionContext)
throws ActionExecutionException, InterruptedException {
Executor executor = actionExecutionContext.getExecutor();

// Reconstruct environment to communicate if verbose_failures and/or subcommands is set.
if (verboseFailuresAndSubcommandsInEnv) {
ImmutableMap.Builder<String, String> environmentBuilder =
new ImmutableMap.Builder<String, String>().putAll(environment);

if (executor.getVerboseFailures()) {
environmentBuilder.put(VERBOSE_FAILURES_KEY, "true");
}
if (executor.reportsSubcommands()) {
environmentBuilder.put(SUBCOMMANDS_KEY, "true");
}

this.environment = environmentBuilder.build();
}

try {
internalExecute(actionExecutionContext);
} catch (ExecException e) {
Expand Down Expand Up @@ -383,11 +356,6 @@ public ExtraActionInfo.Builder getExtraActionInfo() {

/**
* Returns the environment in which to run this action.
*
* <p>Note that if setVerboseFailuresAndSubcommandsInEnv() is called on the builder, and either
* verbose_failures or subcommands is specified in the execution context, corresponding variables
* will be added to the environment. These variables will not be known until execution time,
* however, and so are not returned by getEnvironment().
*/
public Map<String, String> getEnvironment() {
return environment;
Expand Down Expand Up @@ -491,8 +459,6 @@ public static class Builder {
private String mnemonic = "Unknown";
private ExtraActionInfoSupplier<?> extraActionInfoSupplier = null;

private boolean verboseFailuresAndSubcommandsInEnv = false;

/**
* Creates a SpawnAction builder.
*/
Expand Down Expand Up @@ -521,7 +487,6 @@ public Builder(Builder other) {
this.progressMessage = other.progressMessage;
this.paramFileInfo = other.paramFileInfo;
this.mnemonic = other.mnemonic;
this.verboseFailuresAndSubcommandsInEnv = other.verboseFailuresAndSubcommandsInEnv;
}

/**
Expand Down Expand Up @@ -608,8 +573,7 @@ public Action[] build(ActionOwner owner, AnalysisEnvironment analysisEnvironment
ImmutableMap.copyOf(inputAndToolManifests),
mnemonic,
executeUnconditionally,
extraActionInfoSupplier,
verboseFailuresAndSubcommandsInEnv));
extraActionInfoSupplier));
return actions.toArray(new Action[actions.size()]);
}

Expand Down Expand Up @@ -708,17 +672,6 @@ public Builder useDefaultShellEnvironment() {
return this;
}

/**
* Sets the environment variable "BAZEL_VERBOSE_FAILURES" to "true" if --verbose_failures is
* set in the execution context. Sets the environment variable "BAZEL_SUBCOMMANDS" to "true"
* if --subcommands is set in the execution context.
*
*/
public Builder setVerboseFailuresAndSubcommandsInEnv() {
this.verboseFailuresAndSubcommandsInEnv = true;
return this;
}

/**
* Makes the action always execute, even if none of its inputs have changed.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ public GenRuleAction(
runfilesManifests,
"Genrule",
false,
null,
false);
null);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,7 @@ public ExtraAction(
getManifests(shadowedAction),
mnemonic,
false,
null,
false);
null);
this.shadowedAction = shadowedAction;
this.runfilesManifests = ImmutableMap.copyOf(runfilesManifests);
this.createDummyOutput = createDummyOutput;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ private void registerInterfaceBuilderActions(ObjcProvider objcProvider) {
.setCommandLine(ibActionsCommandLine(archiveRoot, zipOutput, storyboardInput))
.addOutput(zipOutput)
.addInput(storyboardInput)
.setVerboseFailuresAndSubcommandsInEnv()
.build(ruleContext));
}
}
Expand Down Expand Up @@ -278,7 +277,6 @@ private void registerMomczipActions(ObjcProvider objcProvider) {
.setExecutable(attributes.momcWrapper())
.addOutput(outputZip)
.addInputs(datamodel.getInputs())
.setVerboseFailuresAndSubcommandsInEnv()
.setCommandLine(CustomCommandLine.builder()
.addPath(outputZip.getExecPath())
.add(datamodel.archiveRootForMomczip())
Expand Down Expand Up @@ -308,7 +306,6 @@ private void registerConvertXibsActions(ObjcProvider objcProvider) {
.setCommandLine(ibActionsCommandLine(archiveRoot, zipOutput, original))
.addOutput(zipOutput)
.addInput(original)
.setVerboseFailuresAndSubcommandsInEnv()
.build(ruleContext));
}
}
Expand Down Expand Up @@ -363,7 +360,6 @@ private void registerMergeInfoplistAction(
.addInputArgument(plMergeControlArtifact)
.addTransitiveInputs(mergingContentArtifacts)
.addOutput(ObjcRuleClasses.intermediateArtifacts(ruleContext).mergedInfoplist())
.setVerboseFailuresAndSubcommandsInEnv()
.build(ruleContext));
}

Expand All @@ -388,7 +384,6 @@ private void registerActoolActionIfNecessary(ObjcProvider objcProvider) {
.addTransitiveInputs(objcProvider.get(ASSET_CATALOG))
.addOutput(zipOutput)
.addOutput(actoolPartialInfoplist)
.setVerboseFailuresAndSubcommandsInEnv()
.setCommandLine(actoolzipCommandLine(
objcProvider,
zipOutput,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,6 @@ private void registerBundleMergeActions(Artifact ipaUnsigned,
.addInputArgument(bundleMergeControlArtifact)
.addTransitiveInputs(bundleContentArtifacts)
.addOutput(ipaUnsigned)
.setVerboseFailuresAndSubcommandsInEnv()
.build(ruleContext));
}

Expand Down

0 comments on commit 8378cd8

Please sign in to comment.