Skip to content

Commit

Permalink
Add an explicit bit to actions that tells whether they are shareable …
Browse files Browse the repository at this point in the history
…or not.

This bit is set to "true" by default everywhere except on CppCompileActions that are not used for linkstamp data compilation.

Note that CppCompileAction#discoversInputs() will still almost always return true because it's still set to true when .d file pruning is enabled.

This is an encore of e03a119, which got rolled back due to an issue with Apple rules, which then got fixed in unknown commit.

RELNOTES: None.
PiperOrigin-RevId: 213231143
  • Loading branch information
lberki authored and Copybara-Service committed Sep 17, 2018
1 parent efdb3f6 commit 772fdfe
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,11 @@ public boolean isVolatile() {
return false;
}

@Override
public boolean isShareable() {
return true;
}

@Override
public boolean showsOutputUnconditionally() {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ public interface ActionAnalysisMetadata {
*/
ActionOwner getOwner();

/**
* Returns true if the action can be shared, i.e. multiple configured targets can create the same
* action.
*
* <p>In theory, these should not exist, but in practice, they do.
*/
boolean isShareable();

/**
* Returns a mnemonic (string constant) for this kind of action; written into
* the master log so that the appropriate parser can be invoked for the output
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ public final class Actions {
*/
public static boolean canBeShared(
ActionKeyContext actionKeyContext, ActionAnalysisMetadata a, ActionAnalysisMetadata b) {
if (!a.isShareable() || !b.isShareable()) {
return false;
}

if (!a.getMnemonic().equals(b.getMnemonic())) {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ public ActionOwner getOwner() {
return actionOwner;
}

@Override
public boolean isShareable() {
return true;
}

@Override
public final String getMnemonic() {
return mnemonic;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public class CppCompileAction extends AbstractAction
private final NestedSet<Artifact> additionalPrunableHeaders;

@Nullable private final Artifact grepIncludes;
private final boolean shareable;
private final boolean shouldScanIncludes;
private final boolean shouldPruneModules;
private final boolean usePic;
Expand Down Expand Up @@ -200,6 +201,7 @@ public class CppCompileAction extends AbstractAction
CcToolchainVariables variables,
Artifact sourceFile,
CppConfiguration cppConfiguration,
boolean shareable,
boolean shouldScanIncludes,
boolean shouldPruneModules,
boolean usePic,
Expand Down Expand Up @@ -236,6 +238,7 @@ public class CppCompileAction extends AbstractAction
Preconditions.checkArgument(!shouldPruneModules || shouldScanIncludes);
this.outputFile = Preconditions.checkNotNull(outputFile);
this.sourceFile = sourceFile;
this.shareable = shareable;
this.cppConfiguration = cppConfiguration;
// We do not need to include the middleman artifact since it is a generated artifact and will
// definitely exist prior to this action execution.
Expand Down Expand Up @@ -983,6 +986,11 @@ public ResourceSet estimateResourceConsumptionLocal() {
/* memoryMb= */ 200, /* cpuUsage= */ 0.5, /* ioUsage= */ 0.0);
}

@Override
public boolean isShareable() {
return shareable;
}

@Override
public void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) {
fp.addUUID(actionClassId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public class CppCompileActionBuilder {
public static final UUID GUID = UUID.fromString("97493805-894f-493a-be66-9a698f45c31d");

private final ActionOwner owner;
private boolean shareable;
private final BuildConfiguration configuration;
private CcToolchainFeatures.FeatureConfiguration featureConfiguration;
private CcToolchainVariables variables = CcToolchainVariables.EMPTY;
Expand Down Expand Up @@ -112,6 +113,7 @@ private CppCompileActionBuilder(
CcToolchainProvider ccToolchain,
@Nullable Artifact grepIncludes) {
this.owner = actionOwner;
this.shareable = false;
this.configuration = configuration;
this.cppConfiguration = configuration.getFragment(CppConfiguration.class);
this.mandatoryInputsBuilder = NestedSetBuilder.stableOrder();
Expand All @@ -128,6 +130,7 @@ private CppCompileActionBuilder(
*/
public CppCompileActionBuilder(CppCompileActionBuilder other) {
this.owner = other.owner;
this.shareable = other.shareable;
this.featureConfiguration = other.featureConfiguration;
this.sourceFile = other.sourceFile;
this.mandatoryInputsBuilder = NestedSetBuilder.<Artifact>stableOrder()
Expand Down Expand Up @@ -304,6 +307,7 @@ public CppCompileAction buildAndVerify(Consumer<String> errorCollector) {
variables,
sourceFile,
cppConfiguration,
shareable,
shouldScanIncludes,
shouldPruneModules(),
usePic,
Expand Down Expand Up @@ -331,6 +335,7 @@ public CppCompileAction buildAndVerify(Consumer<String> errorCollector) {
variables,
sourceFile,
cppConfiguration,
shareable,
shouldScanIncludes,
shouldPruneModules(),
usePic,
Expand Down Expand Up @@ -597,6 +602,11 @@ public CppCompileActionBuilder setSemantics(CppSemantics semantics) {
return this;
}

public CppCompileActionBuilder setShareable(boolean shareable) {
this.shareable = shareable;
return this;
}

public CppCompileActionBuilder setShouldScanIncludes(boolean shouldScanIncludes) {
this.shouldScanIncludes = shouldScanIncludes;
return this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,11 @@ public ActionOwner getOwner() {
return actionOwner;
}

@Override
public boolean isShareable() {
return false;
}

@Override
public final String getMnemonic() {
return "CppCompileActionTemplate";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ public static CppCompileAction createLinkstampCompileAction(
.setBuiltinIncludeFiles(buildInfoHeaderArtifacts)
.addMandatoryInputs(nonCodeInputs)
.setCppConfiguration(cppConfiguration)
.setShareable(true)
.setShouldScanIncludes(false)
.setActionName(CppActionNames.LINKSTAMP_COMPILE);
semantics.finalizeCompileActionBuilder(ruleContext, builder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public class FakeCppCompileAction extends CppCompileAction {
CcToolchainVariables variables,
Artifact sourceFile,
CppConfiguration cppConfiguration,
boolean shareable,
boolean shouldScanIncludes,
boolean shouldPruneModules,
boolean usePic,
Expand All @@ -88,6 +89,7 @@ public class FakeCppCompileAction extends CppCompileAction {
variables,
sourceFile,
cppConfiguration,
shareable,
shouldScanIncludes,
shouldPruneModules,
usePic,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,11 @@ public ActionOwner getOwner() {
throw new IllegalStateException();
}

@Override
public boolean isShareable() {
throw new IllegalStateException();
}

@Override
public String prettyPrint() {
throw new IllegalStateException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ public ActionOwner getOwner() {
null);
}

@Override
public boolean isShareable() {
return false;
}

@Override
public String getMnemonic() {
return mnemonic;
Expand Down

0 comments on commit 772fdfe

Please sign in to comment.