Skip to content

Commit 1c6fdb8

Browse files
committed
Fix tests
1 parent 8216e7d commit 1c6fdb8

File tree

2 files changed

+16
-19
lines changed

2 files changed

+16
-19
lines changed

src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ public void clearAdditionalInputs() {
386386
public boolean discoversInputs() {
387387
return shouldScanIncludes
388388
|| getDotdFile() != null
389-
|| featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES);
389+
|| shouldParseShowIncludes();
390390
}
391391

392392
@Override
@@ -1399,7 +1399,7 @@ public ActionContinuationOrResult beginExecution(ActionExecutionContext actionEx
13991399
ActionExecutionContext spawnContext;
14001400
ShowIncludesFilter showIncludesFilterForStdout;
14011401
ShowIncludesFilter showIncludesFilterForStderr;
1402-
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
1402+
if (shouldParseShowIncludes()) {
14031403
showIncludesFilterForStdout = new ShowIncludesFilter(getSourceFile().getFilename());
14041404
showIncludesFilterForStderr = new ShowIncludesFilter(getSourceFile().getFilename());
14051405
FileOutErr originalOutErr = actionExecutionContext.getFileOutErr();
@@ -1445,6 +1445,10 @@ protected byte[] getDotDContents(SpawnResult spawnResult) throws EnvironmentalEx
14451445
return null;
14461446
}
14471447

1448+
protected boolean shouldParseShowIncludes() {
1449+
return featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES);
1450+
}
1451+
14481452
protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutionException {
14491453
// Intentionally not adding {@link CppCompileAction#inputsForInvalidation}, those are not needed
14501454
// for execution.
@@ -1458,7 +1462,8 @@ protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutio
14581462
}
14591463
NestedSet<ActionInput> inputs = inputsBuilder.build();
14601464

1461-
ImmutableMap<String, String> executionInfo = getExecutionInfo();
1465+
ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.<String, String>builder()
1466+
.putAll(getExecutionInfo());
14621467
if (getDotdFile() != null && useInMemoryDotdFiles()) {
14631468
/*
14641469
* CppCompileAction does dotd file scanning locally inside the Bazel process and thus
@@ -1468,34 +1473,25 @@ protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutio
14681473
* in-memory. We can do that via
14691474
* {@link ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS}.
14701475
*/
1471-
executionInfo =
1472-
ImmutableMap.<String, String>builderWithExpectedSize(executionInfo.size() + 1)
1473-
.putAll(executionInfo)
1474-
.put(
1475-
ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS,
1476-
getDotdFile().getExecPathString())
1477-
.build();
1476+
executionInfo.put(ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS,
1477+
getDotdFile().getExecPathString());
14781478
}
14791479

1480-
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
1480+
if (shouldParseShowIncludes()) {
14811481
// Hack on Windows. The included headers dumped by cl.exe in stdout contain absolute paths.
14821482
// When compiling the file from different workspace, the shared cache will cause header
14831483
// dependency checking to fail. This was initially fixed by a hack (see
14841484
// https://github.com/bazelbuild/bazel/issues/9172 for more details), but is broken again due
14851485
// to cl/356735700. We require execution service to ignore caches from other workspace.
1486-
executionInfo =
1487-
ImmutableMap.<String, String>builderWithExpectedSize(executionInfo.size() + 1)
1488-
.putAll(executionInfo)
1489-
.put(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "")
1490-
.build();
1486+
executionInfo.put(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "");
14911487
}
14921488

14931489
try {
14941490
return new SimpleSpawn(
14951491
this,
14961492
ImmutableList.copyOf(getArguments()),
14971493
getEnvironment(clientEnv),
1498-
executionInfo,
1494+
executionInfo.build(),
14991495
inputs,
15001496
getOutputs(),
15011497
estimateResourceConsumptionLocal());
@@ -1848,7 +1844,7 @@ public ActionContinuationOrResult execute()
18481844
.getOptions(BuildLanguageOptions.class)
18491845
.experimentalSiblingRepositoryLayout;
18501846

1851-
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
1847+
if (shouldParseShowIncludes()) {
18521848
NestedSet<Artifact> discoveredInputs =
18531849
discoverInputsFromShowIncludesFilters(
18541850
execRoot,
@@ -1888,7 +1884,7 @@ public ActionContinuationOrResult execute()
18881884
private void copyTempOutErrToActionOutErr() throws ActionExecutionException {
18891885
// If parse_showincludes feature is enabled, instead of parsing dotD file we parse the
18901886
// output of cl.exe caused by /showIncludes option.
1891-
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
1887+
if (shouldParseShowIncludes()) {
18921888
try {
18931889
FileOutErr tempOutErr = spawnExecutionContext.getFileOutErr();
18941890
FileOutErr outErr = actionExecutionContext.getFileOutErr();

src/test/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategyTest.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ public void testInMemoryDotdFileAndExecutionRequirement() throws Exception {
7272
when(action.getDotdFile()).thenReturn(dotdFile);
7373
when(action.useInMemoryDotdFiles()).thenReturn(true);
7474
when(action.estimateResourceConsumptionLocal()).thenReturn(AbstractAction.DEFAULT_RESOURCE_SET);
75+
when(action.shouldParseShowIncludes()).thenReturn(false);
7576
when(action.createSpawn(any())).thenCallRealMethod();
7677

7778
// act

0 commit comments

Comments
 (0)