Windows!

[cpsauer]

Update: Windows support landed! Thanks all. The latest commits should have it. You can go ahead and start using this tool on Windows.

We haven't yet tested on Windows. Windows might need some patching parallel to that for macOS (in refresh.template.py), but it should be a relatively easy adaptation compared to writing things from scratch. If you're trying to use it on Windows, let us know here. We'd love to work together to get things working smoothly.

[lukasoyen]

Just to leave a quick note while I am researching this:

• it seems we should not just do command = ' '.join(args) as the path to cl.exe contains whitespaces. I guess we could use https://stackoverflow.com/a/12130261
• /showIncludes and /P are the flags we are interested here
  • https://docs.microsoft.com/en-us/cpp/build/reference/showincludes-list-include-files?view=msvc-160
  • https://docs.microsoft.com/en-us/cpp/build/reference/p-preprocess-to-a-file?view=msvc-170
• I get a lot of warnings about missing header files from the C++ stdlib. https://docs.microsoft.com/en-us/cpp/error-messages/compiler-errors-1/fatal-error-c1083?view=msvc-170#the-command-line-include-or-lib-environment-is-not-set is a good pointe and indeed the bazel default toolchain sets these environment variables https://github.com/bazelbuild/bazel/blob/6dc941e58dfc1d4a9714a76b921fbe11fce658ed/tools/cpp/windows_cc_toolchain_config.bzl#L627. Currently I am not sure if the bazel aquery exposes that correctly. For some other action I see environmentVariables but not for the compile actions.

[cpsauer]

Hi, @lummax! Thanks for diving in here. Looks like good stuff to me. Tapping back some thoughts, in case they're helpful.

• Yep, I definitely should have considered shell escaping. Good call.
  • Alternatively, we could use the arguments array option in the compile_commands.json spec?
• 👍🏻
• Aha! Good find. Yeah, environment variables & clangd are tricky, especially since clangd doesn't know how to parse them from the specified command. You've probably already seen this in refresh.template.py, but I had a similar issue with the wrapper script for Apple platforms. I'd definitely suggest getting the environment variables from aquery if you can, and converting them into flags--sounds like you're already onto that. On Apple they weren't provided, so I extracted them from the system.
  • Just to double check: The search paths needed to find standard lib headers are indeed missing from the command, right? I just want to make sure that the issue isn't that clangd is failing to parse msvc flags.

Couple other notes from my mental cache that might be handy:

• There's this older project that I remember from the Implementation Readme. My memory is that it's windows/clang-cl specific, hasn't been updated in a while, and not, say, code we could really use, but it might be worth a quick look to see if it sparks good thinking about some additional edge case.

If there are other things where I could save you a bundle of time by answering questions or otherwise helping orient you in the code, please let me know. Communication lines are open, and I really appreciate your efforts.

[lukasoyen]

On Apple they weren't provided, so I extracted them from the system.

I would like to avoid that. We are experimenting with using a vendored compiler toolchain where the complete configuration is already statically known in our bazel toolchain config. I guess it would make sense for the average user though :)

The search paths needed to find standard lib headers are indeed missing from the command, right?

It seems the search paths should be passed via environment variables. If these environment variables are not set header-extraction in extract.py fails as soon as the first stdlib header is included. But some included (project specific headers) are printed so there is at least some additional information.

clangd seems to inspect the environment or infers the correct stdlib paths from the cl.exe path. The problem I am having is localized to extract.py.

[lukasoyen]

A completely different finding: https://github.com/bazelbuild/bazel/blob/09c621e4cf5b968f4c6cdf905ab142d5961f9ddc/src/main/java/com/google/devtools/build/lib/rules/cpp/ShowIncludesFilter.java#L58

When extracting the included headers from the cl.exe output, the extraction should be able to handle different languages it seems.

[cpsauer]

Gross. At least they all seem to end with a colon (58)? Still tough especially if it makes you recognize them in stderr.

(Ah! Didn't realize it was specific to refresh.template.py header finding. Clangd does indeed do some detection from the structure of known compilers. Hoping you find that the aquery output has the environment variables you need!)

[LoSealL]

Hi, I am happy to help make it work on Windows, and I init a PR to do so #25 .

And I'd like your help to figure out what else needs to be added there.

For now there are some known issues while using Bazel on Windows:

1. The bazel configuration seems not passed to the script template and it use the default config to do aquery and run. Hence the generated commands are all directed to x64-windows-fastbuild even the actual build happens in x64-windows-dbg.
2. The external bazel libraries' header paths (i.e. gtest) can't be get corrrectly from aquery command and because they are not installed, I don't know where to find them.

[cpsauer]

Wow--should I be so lucky as to have not one, but two efforts to add Windows support!

@LoSealL, thanks for your efforts and contribution. @lummax, though, is already quite far along with his work to add Windows support, including solving issues you raise. We're working on merging in the first part of his fix in #16, the other open PR, which moves everything into python so windows doesn't need a separate bat file!

If I may, and you'd still like to help, could I ask that you join forces? Perhaps there's a part where he'd like some help.

[LoSealL]

Oh, I'm not aware @lummax is working on Win support, just seeing a refactor PR opened. Thanks you for your reply.
I'd like to make my contribution if there are things to be done.

[lukasoyen]

Sorry, I was slow to respond on the refactoring PR.

We are running https://github.com/axivion/bazel-compile-commands-extractor/commits/axivion (ignore the ugly git history) on Windows. Feel free to give that a try.

[LoSealL]

Hi @lummax, thanks for your great effort!
A little issue found:

• if system language (or more specific, VS language) is not utf-8 encoded (such as Chinese/Japanese), the subprocess.run commands will raise an exception about decoding error.
  Change code page to utf-8 and VS language to English could mitigate this issue, but can encoding='utf-8' be determined by system locale?

• Second one is the generated arguments are not quoted, and clangd may faile to execute the arguments.

     [
       {
         "file": "src/foo.cc",
         "arguments": [
           "C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.29.30133/bin/HostX64/x64/cl.exe",
           "--language=c++",
          ...
         ]
       }
     ]

  What expected is "\"C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.29.30133/bin/HostX64/x64/cl.exe\""

[LoSealL]

The include arguments for external packages are not correct

{
    "file": "src/foo.cc",
    "arguments": [
      "\"C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.29.30133/bin/HostX64/x64/cl.exe\"",
      "--language=c++",
      "/nologo",
      "/DCOMPILER_MSVC",
      "/DNOMINMAX",
      "/D_WIN32_WINNT=0x0601",
      "/D_CRT_SECURE_NO_DEPRECATE",
      "/D_CRT_SECURE_NO_WARNINGS",
      "/bigobj",
      "/Zm500",
      "/EHsc",
      "/wd4351",
      "/wd4291",
      "/wd4250",
      "/wd4996",
      "/I.",
      "/Ibazel-out/x64_windows-fastbuild/bin",
      "/Iexternal/gtest",
      "/Ibazel-out/x64_windows-fastbuild/bin/external/gtest",
      "/Iexternal/bazel_tools",
      "/Ibazel-out/x64_windows-fastbuild/bin/external/bazel_tools",
      "/Ibazel-out/x64_windows-fastbuild/bin/spdlog/include",
      "/Ibazel-out/x64_windows-fastbuild/bin/fmt/include",
      "/Iexternal/gtest/googlemock",
      "/Ibazel-out/x64_windows-fastbuild/bin/external/gtest/googlemock",
      "/Iexternal/gtest/googlemock/include",
      "/Ibazel-out/x64_windows-fastbuild/bin/external/gtest/googlemock/include",
      "/Iexternal/gtest/googletest",
      "/Ibazel-out/x64_windows-fastbuild/bin/external/gtest/googletest",
      "/Iexternal/gtest/googletest/include",
      "/Ibazel-out/x64_windows-fastbuild/bin/external/gtest/googletest/include",
      "/DSPDLOG_FMT_EXTERNAL",
      "/DSPDLOG_WCHAR_TO_UTF8_SUPPORT",
      "/DSPDLOG_WCHAR_FILENAMES",
      "/showIncludes",
      "/MD",
      "/Od",
      "/Z7",
      "/wd4117",
      "-D__DATE__=\"redacted\"",
      "-D__TIMESTAMP__=\"redacted\"",
      "-D__TIME__=\"redacted\"",
      "/MT",
      "/Fobazel-out/x64_windows-fastbuild/bin/src/_objs/foo/foo.obj",
      "/c",
      "src/foo.cc"
    ]

For example the /Iexternal/gtest should be /Ibazel-<workspace_name>/external/gtest

[cpsauer]

Just merged @lummax's first PR! Thank you so much, Lukas!
I'll comment more in the PR in a second, but I'm excited to work with you on the ones this unblocked.
Let's go timezone tag-team.

@LoSealL, to respond to your three:

1. Encoding: This would be an awesome place for you to help, if you're willing! I know next to nothing about non-UTF-8 encodings, and it sounds like you know what's up. FWIW, I tried removing the encoding line, which should fall back to the system locale according to the docs, but it doesn't seem to work. Instead, it doesn't try to encode at all.
2. Quoting paths with spaces: This one I should have just fixed with a call to shlex.join. Credit to Lukas for showing me that package! (Sorry, @lummax: I think that raises the min python to 3.8, and you said you were currently running 3.7. Okay to upgrade?)
   Interesting that this works on Windows! Bazel categorically refuses to run on paths with spaces on other platforms, in my experience.
3. External symlink: Gah! No. this is a trap, and the output is right. bazel-<workspace_name>/external points into the execroot, which is super unstable and gets torn down and reconfigured on each build. You want to go through the output base.
   1. For now, please make sure you've got an external symlink, equivalent to the one described in the readme. [If that doesn't work or can't be done on Windows, please say something! My Windows knowledge is currently weak.]
   2. If you'd like to know more, please read https://github.com/hedronvision/bazel-compile-commands-extractor/blob/main/ImplementationReadme.md
   3. Lukas is working on auto-linking/rewiring options, I believe.

[LoSealL]

@cpsauer
For 1, I'm looking for how to deal with it, and once done, I'll make my PR again.
For 3, I missed the guide in Readme to explictily create a link for external folder. now I got it.