From 1c6fdb8e5e45989091ab7b74d381e6f13b97db25 Mon Sep 17 00:00:00 2001
From: Chi Wang <chiwang@google.com>
Date: Wed, 14 Apr 2021 15:45:15 +0800
Subject: [PATCH] Fix tests

---
 .../build/lib/rules/cpp/CppCompileAction.java | 34 ++++++++-----------
 .../lib/rules/cpp/SpawnGccStrategyTest.java   |  1 +
 2 files changed, 16 insertions(+), 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
index 55fde109880651..e4721e6fcb9eef 100644
--- 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
@@ -1399,7 +1399,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();
@@ -1445,6 +1445,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.
@@ -1458,7 +1462,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
@@ -1468,26 +1473,17 @@ 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 {
@@ -1495,7 +1491,7 @@ protected Spawn createSpawn(Map<String, String> clientEnv) throws ActionExecutio
           this,
           ImmutableList.copyOf(getArguments()),
           getEnvironment(clientEnv),
-          executionInfo,
+          executionInfo.build(),
           inputs,
           getOutputs(),
           estimateResourceConsumptionLocal());
@@ -1848,7 +1844,7 @@ public ActionContinuationOrResult execute()
               .getOptions(BuildLanguageOptions.class)
               .experimentalSiblingRepositoryLayout;
 
-      if (featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
+      if (shouldParseShowIncludes()) {
         NestedSet<Artifact> discoveredInputs =
             discoverInputsFromShowIncludesFilters(
                 execRoot,
@@ -1888,7 +1884,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
index 28cb32d7c108c4..753f9e101694bf 100644
--- 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