Repository: bazelbuild/bazel
Issue: Tree artifacts don't seem to work on Windows
Labels: type: bug
P2
team-Rules-CPP

### Description of the problem / feature request:

Tree artifacts don't seem to work on Windows.

### Feature requests: what underlying problem are you trying to solve with this feature?

Generate a `cc_library` from an unknown number of source files, e.g. because they are produced by a tool.

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Same as in the accepted answer here: https://stackoverflow.com/questions/48417712/how-to-build-static-library-from-the-generated-source-files-using-bazel-build/48524539#48524539

But with a small modification to make it compatible with Windows:

genccs.bzl: verbatim.

BUILD: verbatim.

genccs.cpp:
```cpp
#include <fstream>
#include <Windows.h>// BEFORE: <sys/stat.h>
using namespace std;

int main (int argc, char *argv[]) {
  CreateDirectory(argv[1], NULL); // BEFORE: mkdir(argv[1], S_IRWXU);
  ofstream myfile;
  myfile.open(string(argv[1]) + string("/foo.cpp"));
  myfile << "int main() { return 42; }";
  return 0;
}
```

WORKSPACE: empty file.

### What operating system are you running Bazel on?

Windows 10

### What's the output of `bazel info release`?

release 0.29.0

### If `bazel info release` returns "development version" or "(@non-git)", tell us how you built Bazel.

N/A

### What's the output of `git remote get-url origin ; git rev-parse master ; git rev-parse HEAD` ?

N/A

###  Have you found anything relevant by searching the web?

Relevant question and answer: https://stackoverflow.com/questions/48417712/how-to-build-static-library-from-the-generated-source-files-using-bazel-build/48524539#48524539

Other relevant links:
https://github.com/bazelbuild/bazel/issues/5093

### Any other information, logs, or outputs that you want to share?

When running `bazel build main` with the above example, I'm getting:
```
INFO: Analyzed target //:main (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
ERROR: C:/src/treeartifact/BUILD:7:1: output '_dotd/main/gen_tree/foo.obj.d' was not created
ERROR: C:/src/treeartifact/BUILD:7:1: not all outputs were created or valid
Target //:main failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 3.087s, Critical Path: 2.23s
INFO: 4 processes: 4 local.
FAILED: Build did NOT complete successfully
```


Issue comments:
cc @laszlocsomor 

Chatted about this with @meteorcloudy this week. We don't expand them in the manifest as we do in the runfiles tree. Will send a fix.
@buchgr This looks like a different issue, `_dotd/main/gen_tree/foo.obj.d` shouldn't be required on Windows.
Maybe using tree artifact triggered some code path that should not run on Windows.
@buchgr : thanks for the info! Do you have a repro?
Is there a workaround I can use in the meanwhile?
@ale64bit : I don't think so.

/cc @hlopko , @oquenchil , @scentini:
Meanwhile I found it's CppCompileActionTemplate unconditionally creating a `.d` output: https://github.com/bazelbuild/bazel/blob/ed68a81f7938067d05fd7525f4aece6db59326b3/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java#L130-L132

If I comment that out and make `dotdFileArtifact = null`, and comment out these lines: https://github.com/bazelbuild/bazel/blob/ed68a81f7938067d05fd7525f4aece6db59326b3/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java#L195-L197

then the bug vanishes.
Yup that looks like our problem :)
I just sent a fix for this, https://github.com/bazelbuild/bazel/pull/9454 ;)

Pull request review comments:
Can you add a comment here what this is for?
Can you please also mark dotdTreeArtifact as @Nullable?
Done
Thanks, done!

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
index 4b8a101c3229e6..d7261db061fca0 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java
@@ -1398,12 +1398,18 @@ private Artifact createCompileActionTemplate(
             /* additionalBuildVariables= */ ImmutableMap.of()));
     semantics.finalizeCompileActionBuilder(configuration, featureConfiguration, builder);
     // Make sure this builder doesn't reference ruleContext outside of analysis phase.
+    SpecialArtifact dotdTreeArtifact = null;
+    // The MSVC compiler won't generate .d file, instead we parse the output of /showIncludes flag.
+    // Therefore, dotdTreeArtifact should be null in this case.
+    if (!featureConfiguration.isEnabled(CppRuleClasses.PARSE_SHOWINCLUDES)) {
+      dotdTreeArtifact = CppHelper.getDotdOutputTreeArtifact(
+          actionConstructionContext, label, sourceArtifact, outputName, usePic);
+    }
     CppCompileActionTemplate actionTemplate =
         new CppCompileActionTemplate(
             sourceArtifact,
             outputFiles,
-            CppHelper.getDotdOutputTreeArtifact(
-                actionConstructionContext, label, sourceArtifact, outputName, usePic),
+            dotdTreeArtifact,
             builder,
             ccToolchain,
             outputCategories,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
index 2609d32a6de6dd..96b8ac27a117ff 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionTemplate.java
@@ -38,6 +38,8 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import javax.annotation.Nullable;
+
 /** An {@link ActionTemplate} that expands into {@link CppCompileAction}s at execution time. */
 public final class CppCompileActionTemplate extends ActionKeyCacher
     implements ActionTemplate<CppCompileAction> {
@@ -127,9 +129,12 @@ public Iterable<CppCompileAction> generateActionForInputArtifacts(
         TreeFileArtifact outputTreeFileArtifact =
             ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
                 outputTreeArtifact, PathFragment.create(outputName), artifactOwner);
-        TreeFileArtifact dotdFileArtifact =
-            ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
-                dotdTreeArtifact, PathFragment.create(outputName + ".d"), artifactOwner);
+        TreeFileArtifact dotdFileArtifact = null;
+        if (dotdTreeArtifact != null) {
+          dotdFileArtifact =
+              ActionInputHelper.treeFileArtifactWithNoGeneratingActionSet(
+                  dotdTreeArtifact, PathFragment.create(outputName + ".d"), artifactOwner);
+        }
         expandedActions.add(
             createAction(
                 inputTreeFileArtifact, outputTreeFileArtifact, dotdFileArtifact, privateHeaders));
@@ -176,7 +181,7 @@ private boolean shouldCompileHeaders() {
   private CppCompileAction createAction(
       Artifact sourceTreeFileArtifact,
       Artifact outputTreeFileArtifact,
-      Artifact dotdFileArtifact,
+      @Nullable Artifact dotdFileArtifact,
       ImmutableList<Artifact> privateHeaders)
       throws ActionTemplateExpansionException {
     CppCompileActionBuilder builder = new CppCompileActionBuilder(cppCompileActionBuilder);
@@ -192,9 +197,11 @@ private CppCompileAction createAction(
     buildVariables.overrideStringVariable(
         CompileBuildVariables.OUTPUT_FILE.getVariableName(),
         outputTreeFileArtifact.getExecPathString());
-    buildVariables.overrideStringVariable(
-        CompileBuildVariables.DEPENDENCY_FILE.getVariableName(),
-        dotdFileArtifact.getExecPathString());
+    if (dotdFileArtifact != null) {
+      buildVariables.overrideStringVariable(
+          CompileBuildVariables.DEPENDENCY_FILE.getVariableName(),
+          dotdFileArtifact.getExecPathString());
+    }
 
     builder.setVariables(buildVariables.build());
 
@@ -278,6 +285,9 @@ public Iterable<Artifact> getInputs() {
 
   @Override
   public ImmutableSet<Artifact> getOutputs() {
+    if (dotdTreeArtifact == null) {
+      return ImmutableSet.of(outputTreeArtifact);
+    }
     return ImmutableSet.of(outputTreeArtifact, dotdTreeArtifact);
   }
 


Test patch:
diff --git a/src/test/py/bazel/bazel_windows_cpp_test.py b/src/test/py/bazel/bazel_windows_cpp_test.py
index 3b20bec3537259..6299439cb2396d 100644
--- a/src/test/py/bazel/bazel_windows_cpp_test.py
+++ b/src/test/py/bazel/bazel_windows_cpp_test.py
@@ -701,6 +701,66 @@ def testCacheBetweenWorkspaceWithDifferentNames(self):
         ['build', '--disk_cache=' + cache_dir, ':lib'], cwd=dir_b)
     self.AssertExitCode(exit_code, 0, stderr)
 
+  # Regression test for https://github.com/bazelbuild/bazel/issues/9321
+  def testCcCompileWithTreeArtifactAsSource(self):
+    self.CreateWorkspaceWithDefaultRepos('WORKSPACE')
+    self.ScratchFile('BUILD', [
+      'load(":genccs.bzl", "genccs")',
+      '',
+      'genccs(',
+      '    name = "gen_tree",',
+      ')',
+      '',
+      'cc_library(',
+      '    name = "main",',
+      '    srcs = [ "gen_tree" ]',
+      ')',
+      '',
+      'cc_binary(',
+      '    name = "genccs",',
+      '    srcs = [ "genccs.cpp" ],',
+      ')',
+    ])
+    self.ScratchFile('genccs.bzl', [
+      'def _impl(ctx):',
+      '  tree = ctx.actions.declare_directory(ctx.attr.name + ".cc")',
+      '  ctx.actions.run(',
+      '    inputs = [],',
+      '    outputs = [ tree ],',
+      '    arguments = [ tree.path ],',
+      '    progress_message = "Generating cc files into \'%s\'" % tree.path,',
+      '    executable = ctx.executable._tool,',
+      '  )',
+      '',
+      '  return [ DefaultInfo(files = depset([ tree ])) ]',
+      '',
+      'genccs = rule(',
+      '  implementation = _impl,',
+      '  attrs = {',
+      '    "_tool": attr.label(',
+      '      executable = True,',
+      '      cfg = "host",',
+      '      allow_files = True,',
+      '      default = Label("//:genccs"),',
+      '    )',
+      '  }',
+      ')',
+    ])
+    self.ScratchFile('genccs.cpp', [
+      '#include <fstream>',
+      '#include <Windows.h>',
+      'using namespace std;',
+      '',
+      'int main (int argc, char *argv[]) {',
+      '  CreateDirectory(argv[1], NULL);',
+      '  ofstream myfile;',
+      '  myfile.open(string(argv[1]) + string("/foo.cpp"));',
+      '  myfile << "int main() { return 42; }";',
+      '  return 0;',
+      '}',
+    ])
+    exit_code, _, stderr = self.RunBazel(['build', '//:main'])
+    self.AssertExitCode(exit_code, 0, stderr)
 
 if __name__ == '__main__':
   unittest.main()