windows cxx: support for depfiles

Summary: On windows includes are provided in stderr when /showIncludes option is passed to a compiler, an include is reported as `Note: including file: include.h` (see https://docs.microsoft.com/en-us/cpp/build/reference/showincludes-list-include-files for details) Test Plan: CI: CxxDependencyFileIntegrationTest is run on windows now fbshipit-source-id: 9b35297
jasonnam · Aug 2, 2017 · ddfeca0 · ddfeca0
1 parent c7cd1be
commit ddfeca0
Show file tree

Hide file tree

Showing 13 changed files with 155 additions and 49 deletions.
diff --git a/src/com/facebook/buck/cxx/CompilerDelegate.java b/src/com/facebook/buck/cxx/CompilerDelegate.java
@@ -18,6 +18,7 @@
 
 import com.facebook.buck.cxx.platform.Compiler;
 import com.facebook.buck.cxx.platform.DebugPathSanitizer;
+import com.facebook.buck.cxx.platform.DependencyTrackingMode;
 import com.facebook.buck.rules.BuildRule;
 import com.facebook.buck.rules.RuleKeyAppendable;
 import com.facebook.buck.rules.RuleKeyObjectSink;
@@ -100,8 +101,8 @@ public boolean isArgFileSupported() {
     return compiler.isArgFileSupported();
   }
 
-  public boolean isDependencyFileSupported() {
-    return compiler.isDependencyFileSupported();
+  public DependencyTrackingMode getDependencyTrackingMode() {
+    return compiler.getDependencyTrackingMode();
   }
 
   public Compiler getCompiler() {

diff --git a/src/com/facebook/buck/cxx/CxxInferCapture.java b/src/com/facebook/buck/cxx/CxxInferCapture.java
@@ -16,6 +16,7 @@
 
 package com.facebook.buck.cxx;
 
+import com.facebook.buck.cxx.platform.DependencyTrackingMode;
 import com.facebook.buck.io.BuildCellRelativePath;
 import com.facebook.buck.io.ProjectFilesystem;
 import com.facebook.buck.model.BuildTarget;
@@ -166,7 +167,8 @@ public ImmutableList<SourcePath> getInputsAfterBuildingLocally(
               preprocessorDelegate.getHeaderVerification(),
               getDepFilePath(),
               context.getSourcePathResolver().getRelativePath(input),
-              output);
+              output,
+              DependencyTrackingMode.MAKEFILE);
     } catch (Depfiles.HeaderVerificationException e) {
       throw new HumanReadableException(e);
     }

diff --git a/src/com/facebook/buck/cxx/CxxPrecompiledHeader.java b/src/com/facebook/buck/cxx/CxxPrecompiledHeader.java
@@ -231,7 +231,8 @@ private ImmutableList<Path> getDependencies(BuildContext context)
                   getDepFilePath(context.getSourcePathResolver()),
                   // TODO(10194465): This uses relative path so as to get relative paths in the dep file
                   getRelativeInputPath(context.getSourcePathResolver()),
-                  output));
+                  output,
+                  compilerDelegate.getDependencyTrackingMode()));
     } catch (ExecutionException e) {
       // Unwrap and re-throw the loader's Exception.
       Throwables.throwIfInstanceOf(e.getCause(), IOException.class);

diff --git a/src/com/facebook/buck/cxx/CxxPreprocessAndCompile.java b/src/com/facebook/buck/cxx/CxxPreprocessAndCompile.java
@@ -303,7 +303,7 @@ public SourcePath getInput() {
 
   @Override
   public boolean useDependencyFileRuleKeys() {
-    return compilerDelegate.isDependencyFileSupported();
+    return true;
   }
 
   @Override
@@ -355,7 +355,8 @@ public ImmutableList<SourcePath> getInputsAfterBuildingLocally(
                 preprocessDelegate.get().getHeaderVerification(),
                 getDepFilePath(),
                 getRelativeInputPath(context.getSourcePathResolver()),
-                output);
+                output,
+                compilerDelegate.getDependencyTrackingMode());
       } catch (Depfiles.HeaderVerificationException e) {
         throw new HumanReadableException(e);
       }

diff --git a/src/com/facebook/buck/cxx/CxxPreprocessAndCompileStep.java b/src/com/facebook/buck/cxx/CxxPreprocessAndCompileStep.java
@@ -18,6 +18,7 @@
 
 import com.facebook.buck.cxx.platform.Compiler;
 import com.facebook.buck.cxx.platform.DebugPathSanitizer;
+import com.facebook.buck.cxx.platform.DependencyTrackingMode;
 import com.facebook.buck.event.ConsoleEvent;
 import com.facebook.buck.io.ProjectFilesystem;
 import com.facebook.buck.log.Logger;
@@ -38,7 +39,9 @@
 import java.io.InputStreamReader;
 import java.io.UncheckedIOException;
 import java.nio.file.Path;
+import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.logging.Level;
@@ -70,6 +73,8 @@ public class CxxPreprocessAndCompileStep implements Step {
   private static final FileLastModifiedDateContentsScrubber FILE_LAST_MODIFIED_DATE_SCRUBBER =
       new FileLastModifiedDateContentsScrubber();
 
+  private static final String DEPENDENCY_OUTPUT_PREFIX = "Note: including file:";
+
   public CxxPreprocessAndCompileStep(
       ProjectFilesystem filesystem,
       Operation operation,
@@ -200,13 +205,38 @@ private int executeCompilation(ExecutionContext context)
     // We buffer error messages in memory, as these are typically small.
     String err;
     int exitCode;
+
+    List<String> includeLines = Collections.emptyList();
     try (BufferedReader reader =
         new BufferedReader(new InputStreamReader(compiler.getErrorStream(process)))) {
       CxxErrorTransformer cxxErrorTransformer =
           new CxxErrorTransformer(
               filesystem, context.shouldReportAbsolutePaths(), headerPathNormalizer);
-      err =
-          reader.lines().map(cxxErrorTransformer::transformLine).collect(Collectors.joining("\n"));
+
+      if (compiler.getDependencyTrackingMode() == DependencyTrackingMode.SHOW_INCLUDES) {
+        Map<Boolean, List<String>> includesAndErrors =
+            reader
+                .lines()
+                .collect(Collectors.partitioningBy(CxxPreprocessAndCompileStep::isShowIncludeLine));
+        includeLines =
+            includesAndErrors
+                .getOrDefault(true, Collections.emptyList())
+                .stream()
+                .map(CxxPreprocessAndCompileStep::parseShowIncludeLine)
+                .collect(Collectors.toList());
+        List<String> errorLines = includesAndErrors.getOrDefault(false, Collections.emptyList());
+        err =
+            errorLines
+                .stream()
+                .map(cxxErrorTransformer::transformLine)
+                .collect(Collectors.joining("\n"));
+      } else {
+        err =
+            reader
+                .lines()
+                .map(cxxErrorTransformer::transformLine)
+                .collect(Collectors.joining("\n"));
+      }
       exitCode = executor.waitForLaunchedProcess(process).getExitCode();
     } catch (UncheckedIOException e) {
       throw e.getCause();
@@ -215,6 +245,10 @@ private int executeCompilation(ExecutionContext context)
       executor.waitForLaunchedProcess(process);
     }
 
+    if (compiler.getDependencyTrackingMode() == DependencyTrackingMode.SHOW_INCLUDES) {
+      filesystem.writeLinesToPath(includeLines, depFile.get());
+    }
+
     // If we generated any error output, print that to the console.
     if (!err.isEmpty()) {
       context
@@ -230,6 +264,14 @@ private int executeCompilation(ExecutionContext context)
     return exitCode;
   }
 
+  private static boolean isShowIncludeLine(String line) {
+    return line.startsWith(DEPENDENCY_OUTPUT_PREFIX);
+  }
+
+  private static String parseShowIncludeLine(String line) {
+    return line.substring(DEPENDENCY_OUTPUT_PREFIX.length()).trim();
+  }
+
   private ConsoleEvent createConsoleEvent(
       ExecutionContext context, boolean commandOutputsColor, Level level, String message) {
     if (context.getAnsi().isAnsiTerminal() && commandOutputsColor) {

diff --git a/src/com/facebook/buck/cxx/Depfiles.java b/src/com/facebook/buck/cxx/Depfiles.java
@@ -16,6 +16,7 @@
 
 package com.facebook.buck.cxx;
 
+import com.facebook.buck.cxx.platform.DependencyTrackingMode;
 import com.facebook.buck.cxx.platform.HeaderVerification;
 import com.facebook.buck.event.BuckEventBus;
 import com.facebook.buck.event.ConsoleEvent;
@@ -34,6 +35,7 @@
 import java.io.InputStreamReader;
 import java.nio.CharBuffer;
 import java.nio.file.Path;
+import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.logging.Level;
@@ -169,27 +171,38 @@ public static Depfile parseDepfile(Readable readable) throws IOException {
     }
   }
 
-  public static ImmutableList<String> getRawUsedHeadersFromDepfile(
-      ProjectFilesystem filesystem, Path sourceDepFile, Path inputPath) throws IOException {
-
-    try (InputStream input = filesystem.newFileInputStream(sourceDepFile);
-        BufferedReader reader = new BufferedReader(new InputStreamReader(input))) {
-      ImmutableList<String> prereqs = Depfiles.parseDepfile(reader).getPrereqs();
-      // Additional files passed in via command-line flags (e.g. `-fsanitize-blacklist=<file>`)
-      // appear first in the dep file, followed by the input source file.  So, just skip over
-      // everything until just after the input source which should position us at the headers.
-      //
-      // TODO(#11303454): This means we're not including the content of these special files into the
-      // rule key.  The correct way to handle this is likely to support macros in preprocessor/
-      // compiler flags at which point we can use the entries for these files in the depfile to
-      // verify that the user properly references these files via the macros.
-      int inputIndex = prereqs.indexOf(inputPath.toString());
-      Preconditions.checkState(
-          inputIndex != -1,
-          "Could not find input source (%s) in dep file prereqs (%s)",
-          inputPath,
-          prereqs);
-      return prereqs.subList(inputIndex + 1, prereqs.size());
+  private static List<String> getRawUsedHeadersFromDepfile(
+      ProjectFilesystem filesystem,
+      Path sourceDepFile,
+      Path inputPath,
+      DependencyTrackingMode dependencyTrackingMode)
+      throws IOException {
+    switch (dependencyTrackingMode) {
+      case MAKEFILE:
+        try (InputStream input = filesystem.newFileInputStream(sourceDepFile);
+            BufferedReader reader = new BufferedReader(new InputStreamReader(input))) {
+          ImmutableList<String> prereqs = Depfiles.parseDepfile(reader).getPrereqs();
+          // Additional files passed in via command-line flags (e.g. `-fsanitize-blacklist=<file>`)
+          // appear first in the dep file, followed by the input source file.  So, just skip over
+          // everything until just after the input source which should position us at the headers.
+          //
+          // TODO(#11303454): This means we're not including the content of these special files into the
+          // rule key.  The correct way to handle this is likely to support macros in preprocessor/
+          // compiler flags at which point we can use the entries for these files in the depfile to
+          // verify that the user properly references these files via the macros.
+          int inputIndex = prereqs.indexOf(inputPath.toString());
+          Preconditions.checkState(
+              inputIndex != -1,
+              "Could not find input source (%s) in dep file prereqs (%s)",
+              inputPath,
+              prereqs);
+          return prereqs.subList(inputIndex + 1, prereqs.size());
+        }
+      case SHOW_INCLUDES:
+        return filesystem.readLines(sourceDepFile);
+      default:
+        // never happens
+        throw new IllegalStateException();
     }
   }
 
@@ -205,6 +218,8 @@ public static ImmutableList<String> getRawUsedHeadersFromDepfile(
    * @param inputPath Path to source file input, used to skip any leading entries from {@code
    *     -fsanitize-blacklist}.
    * @param outputPath Path to object file output, used for stat tracking.
+   * @param dependencyTrackingMode Setting for how a compiler works with dependencies, used to parse
+   *     depfile
    * @return Normalized path objects suitable for use as arguments to {@link
    *     HeaderPathNormalizer#getSourcePathForAbsolutePath(Path)}.
    * @throws IOException if an IO error occurs.
@@ -218,7 +233,8 @@ public static ImmutableList<Path> parseAndVerifyDependencies(
       HeaderVerification headerVerification,
       Path sourceDepFile,
       Path inputPath,
-      Path outputPath)
+      Path outputPath,
+      DependencyTrackingMode dependencyTrackingMode)
       throws IOException, HeaderVerificationException {
     // Process the dependency file, fixing up the paths, and write it out to it's final location.
     // The paths of the headers written out to the depfile are the paths to the symlinks from the
@@ -233,8 +249,9 @@ public static ImmutableList<Path> parseAndVerifyDependencies(
             PerfEventId.of("depfile-parse"),
             ImmutableMap.of("input", inputPath, "output", outputPath))) {
 
-      ImmutableList<String> headers =
-          getRawUsedHeadersFromDepfile(filesystem, sourceDepFile, inputPath);
+      List<String> headers =
+          getRawUsedHeadersFromDepfile(
+              filesystem, sourceDepFile, inputPath, dependencyTrackingMode);
 
       return normalizeAndVerifyHeaders(
           eventBus, filesystem, headerPathNormalizer, headerVerification, inputPath, headers);
@@ -247,7 +264,7 @@ private static ImmutableList<Path> normalizeAndVerifyHeaders(
       HeaderPathNormalizer headerPathNormalizer,
       HeaderVerification headerVerification,
       Path inputPath,
-      ImmutableList<String> headers)
+      List<String> headers)
       throws IOException, HeaderVerificationException {
     ImmutableList.Builder<Path> resultBuilder = ImmutableList.builder();
     for (String rawHeader : headers) {

diff --git a/src/com/facebook/buck/cxx/platform/Compiler.java b/src/com/facebook/buck/cxx/platform/Compiler.java
@@ -34,8 +34,6 @@ ImmutableList<String> getFlagsForReproducibleBuild(
 
   boolean isArgFileSupported();
 
-  boolean isDependencyFileSupported();
-
   ImmutableList<String> outputArgs(String outputPath);
 
   ImmutableList<String> outputDependenciesArgs(String outputPath);
@@ -44,6 +42,8 @@ ImmutableList<String> getFlagsForReproducibleBuild(
 
   ImmutableList<String> getPdcFlags();
 
+  DependencyTrackingMode getDependencyTrackingMode();
+
   boolean shouldSanitizeOutputBinary();
 
   InputStream getErrorStream(ProcessExecutor.LaunchedProcess compilerProcess);

diff --git a/src/com/facebook/buck/cxx/platform/DefaultCompiler.java b/src/com/facebook/buck/cxx/platform/DefaultCompiler.java
@@ -79,11 +79,6 @@ public boolean isArgFileSupported() {
     return true;
   }
 
-  @Override
-  public boolean isDependencyFileSupported() {
-    return true;
-  }
-
   @Override
   public ImmutableList<String> outputArgs(String outputPath) {
     return ImmutableList.of("-o", outputPath);
@@ -109,6 +104,11 @@ public ImmutableList<String> getPicFlags() {
     return ImmutableList.of("-fPIC");
   }
 
+  @Override
+  public DependencyTrackingMode getDependencyTrackingMode() {
+    return DependencyTrackingMode.MAKEFILE;
+  }
+
   @Override
   public boolean shouldSanitizeOutputBinary() {
     return true;

diff --git a/src/com/facebook/buck/cxx/platform/DependencyTrackingMode.java b/src/com/facebook/buck/cxx/platform/DependencyTrackingMode.java
@@ -0,0 +1,25 @@
+/*
+ * Copyright 2017-present Facebook, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License. You may obtain
+ * a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package com.facebook.buck.cxx.platform;
+
+/** Describes the underlying mechanism to track dependencies for a cxx compilation unit. */
+public enum DependencyTrackingMode {
+  /** MAKEFILE corresponds to `gcc -MD -MF depfile ...` on *nix */
+  MAKEFILE,
+  /** SHOW_INCLUDES corresponds to `cl.exe /showIncludes ...` on windows */
+  SHOW_INCLUDES,
+}
diff --git a/src/com/facebook/buck/cxx/platform/WindowsCompiler.java b/src/com/facebook/buck/cxx/platform/WindowsCompiler.java
@@ -27,11 +27,6 @@ public WindowsCompiler(Tool tool) {
   }
 
 
-  @Override
-  public boolean isDependencyFileSupported() {
-    return false;
-  }
-
   @Override
   public ImmutableList<String> outputArgs(String outputPath) {
     return ImmutableList.of("/Fo" + outputPath);
@@ -44,7 +39,7 @@ public boolean isArgFileSupported() {
 
   @Override
   public ImmutableList<String> outputDependenciesArgs(String outputPath) {
-    return ImmutableList.of();
+    return ImmutableList.of("/showIncludes");
   }
 
   @Override
@@ -62,6 +57,11 @@ public ImmutableList<String> getPicFlags() {
     return ImmutableList.of();
   }
 
+  @Override
+  public DependencyTrackingMode getDependencyTrackingMode() {
+    return DependencyTrackingMode.SHOW_INCLUDES;
+  }
+
   @Override
   public boolean shouldSanitizeOutputBinary() {
     return false;