Skip to content

Commit 1a3609f

Browse files
committed
Fix tests
1 parent 149889b commit 1a3609f

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
@@ -1407,7 +1407,7 @@ public ActionContinuationOrResult beginExecution(ActionExecutionContext actionEx
14071407
ActionExecutionContext spawnContext;
14081408
ShowIncludesFilter showIncludesFilterForStdout;
14091409
ShowIncludesFilter showIncludesFilterForStderr;
1410-
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
1410+
if (shouldParseShowIncludes()) {
14111411
showIncludesFilterForStdout = new ShowIncludesFilter(getSourceFile().getFilename());
14121412
showIncludesFilterForStderr = new ShowIncludesFilter(getSourceFile().getFilename());
14131413
FileOutErr originalOutErr = actionExecutionContext.getFileOutErr();
@@ -1453,6 +1453,10 @@ protected byte[] getDotDContents(SpawnResult spawnResult) throws EnvironmentalEx
14531453
return null;
14541454
}
14551455

1456+
protected boolean shouldParseShowIncludes() {
1457+
return featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES);
1458+
}
1459+
14561460
protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutionException {
14571461
// Intentionally not adding {@link CppCompileAction#inputsForInvalidation}, those are not needed
14581462
// for execution.
@@ -1466,7 +1470,8 @@ protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutio
14661470
}
14671471
NestedSet<ActionInput> inputs = inputsBuilder.build();
14681472

1469-
ImmutableMap<String, String> executionInfo = getExecutionInfo();
1473+
ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.<String, String>builder()
1474+
.putAll(getExecutionInfo());
14701475
if (getDotdFile() != null && useInMemoryDotdFiles()) {
14711476
/*
14721477
* CppCompileAction does dotd file scanning locally inside the Bazel process and thus
@@ -1476,34 +1481,25 @@ protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutio
14761481
* in-memory. We can do that via
14771482
* {@link ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS}.
14781483
*/
1479-
executionInfo =
1480-
ImmutableMap.<String, String>builderWithExpectedSize(executionInfo.size() + 1)
1481-
.putAll(executionInfo)
1482-
.put(
1483-
ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS,
1484-
getDotdFile().getExecPathString())
1485-
.build();
1484+
executionInfo.put(ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS,
1485+
getDotdFile().getExecPathString());
14861486
}
14871487

1488-
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
1488+
if (shouldParseShowIncludes()) {
14891489
// Hack on Windows. The included headers dumped by cl.exe in stdout contain absolute paths.
14901490
// When compiling the file from different workspace, the shared cache will cause header
14911491
// dependency checking to fail. This was initially fixed by a hack (see
14921492
// https://github.com/bazelbuild/bazel/issues/9172 for more details), but is broken again due
14931493
// to cl/356735700. We require execution service to ignore caches from other workspace.
1494-
executionInfo =
1495-
ImmutableMap.<String, String>builderWithExpectedSize(executionInfo.size() + 1)
1496-
.putAll(executionInfo)
1497-
.put(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "")
1498-
.build();
1494+
executionInfo.put(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "");
14991495
}
15001496

15011497
try {
15021498
return new SimpleSpawn(
15031499
this,
15041500
ImmutableList.copyOf(getArguments()),
15051501
getEffectiveEnvironment(clientEnv),
1506-
executionInfo,
1502+
executionInfo.build(),
15071503
inputs,
15081504
getOutputs(),
15091505
estimateResourceConsumptionLocal());
@@ -1861,7 +1857,7 @@ public ActionContinuationOrResult execute()
18611857
.getOptions(BuildLanguageOptions.class)
18621858
.experimentalSiblingRepositoryLayout;
18631859

1864-
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
1860+
if (shouldParseShowIncludes()) {
18651861
NestedSet<Artifact> discoveredInputs =
18661862
discoverInputsFromShowIncludesFilters(
18671863
execRoot,
@@ -1901,7 +1897,7 @@ public ActionContinuationOrResult execute()
19011897
private void copyTempOutErrToActionOutErr() throws ActionExecutionException {
19021898
// If parse_showincludes feature is enabled, instead of parsing dotD file we parse the
19031899
// output of cl.exe caused by /showIncludes option.
1904-
if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
1900+
if (shouldParseShowIncludes()) {
19051901
try {
19061902
FileOutErr tempOutErr = spawnExecutionContext.getFileOutErr();
19071903
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)