Fix tests

bazelbuild · Apr 14, 2021 · 1a3609f · 1a3609f
1 parent 149889b
commit 1a3609f
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 19 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java
@@ -386,7 +386,7 @@ public void clearAdditionalInputs() {
   public boolean discoversInputs() {
     return shouldScanIncludes
         || getDotdFile() != null
-        || featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES);
+        || shouldParseShowIncludes();
   }
 
   @Override
@@ -1407,7 +1407,7 @@ public ActionContinuationOrResult beginExecution(ActionExecutionContext actionEx
     ActionExecutionContext spawnContext;
     ShowIncludesFilter showIncludesFilterForStdout;
     ShowIncludesFilter showIncludesFilterForStderr;
-    if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
+    if (shouldParseShowIncludes()) {
       showIncludesFilterForStdout = new ShowIncludesFilter(getSourceFile().getFilename());
       showIncludesFilterForStderr = new ShowIncludesFilter(getSourceFile().getFilename());
       FileOutErr originalOutErr = actionExecutionContext.getFileOutErr();
@@ -1453,6 +1453,10 @@ protected byte[] getDotDContents(SpawnResult spawnResult) throws EnvironmentalEx
     return null;
   }
 
+  protected boolean shouldParseShowIncludes() {
+    return featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES);
+  }
+
   protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutionException {
     // Intentionally not adding {@link CppCompileAction#inputsForInvalidation}, those are not needed
     // for execution.
@@ -1466,7 +1470,8 @@ protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutio
     }
     NestedSet<ActionInput> inputs = inputsBuilder.build();
 
-    ImmutableMap<String, String> executionInfo = getExecutionInfo();
+    ImmutableMap.Builder<String, String> executionInfo = ImmutableMap.<String, String>builder()
+        .putAll(getExecutionInfo());
     if (getDotdFile() != null && useInMemoryDotdFiles()) {
       /*
        * 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
        * in-memory. We can do that via
        * {@link ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS}.
        */
-      executionInfo =
-          ImmutableMap.<String, String>builderWithExpectedSize(executionInfo.size() + 1)
-              .putAll(executionInfo)
-              .put(
-                  ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS,
-                  getDotdFile().getExecPathString())
-              .build();
+      executionInfo.put(ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS,
+          getDotdFile().getExecPathString());
     }
 
-    if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
+    if (shouldParseShowIncludes()) {
       // Hack on Windows. The included headers dumped by cl.exe in stdout contain absolute paths.
       // When compiling the file from different workspace, the shared cache will cause header
       // dependency checking to fail. This was initially fixed by a hack (see
       // https://github.com/bazelbuild/bazel/issues/9172 for more details), but is broken again due
       // to cl/356735700. We require execution service to ignore caches from other workspace.
-      executionInfo =
-          ImmutableMap.<String, String>builderWithExpectedSize(executionInfo.size() + 1)
-              .putAll(executionInfo)
-              .put(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "")
-              .build();
+      executionInfo.put(ExecutionRequirements.DIFFERENTIATE_WORKSPACE_CACHE, "");
     }
 
     try {
       return new SimpleSpawn(
           this,
           ImmutableList.copyOf(getArguments()),
           getEffectiveEnvironment(clientEnv),
-          executionInfo,
+          executionInfo.build(),
           inputs,
           getOutputs(),
           estimateResourceConsumptionLocal());
@@ -1861,7 +1857,7 @@ public ActionContinuationOrResult execute()
               .getOptions(BuildLanguageOptions.class)
               .experimentalSiblingRepositoryLayout;
 
-      if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
+      if (shouldParseShowIncludes()) {
         NestedSet<Artifact> discoveredInputs =
             discoverInputsFromShowIncludesFilters(
                 execRoot,
@@ -1901,7 +1897,7 @@ public ActionContinuationOrResult execute()
     private void copyTempOutErrToActionOutErr() throws ActionExecutionException {
       // If parse_showincludes feature is enabled, instead of parsing dotD file we parse the
       // output of cl.exe caused by /showIncludes option.
-      if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
+      if (shouldParseShowIncludes()) {
         try {
           FileOutErr tempOutErr = spawnExecutionContext.getFileOutErr();
           FileOutErr outErr = actionExecutionContext.getFileOutErr();

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategyTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/SpawnGccStrategyTest.java
@@ -72,6 +72,7 @@ public void testInMemoryDotdFileAndExecutionRequirement() throws Exception {
     when(action.getDotdFile()).thenReturn(dotdFile);
     when(action.useInMemoryDotdFiles()).thenReturn(true);
     when(action.estimateResourceConsumptionLocal()).thenReturn(AbstractAction.DEFAULT_RESOURCE_SET);
+    when(action.shouldParseShowIncludes()).thenReturn(false);
     when(action.createSpawn(any())).thenCallRealMethod();
 
     // act