Deps pruning with MSVC may result in an incorrect build

[kkpattern]

Hi Bazel Community!

Recently, we found that deps pruning with MSVC may result in incorrect build. When built with MSVC, Bazel uses the /showIncludes flag to get the actual included header files from a source file. Then Bazel will prune the dependencies based on the output. This behavior is enabled in this commit. The problem is that parsing the /showIncludes output can be tricky sometimes.

The main problem is the /showIncludes output may come with different languages and encodings. Bazel already supports 14 languages. But all with UTF-8 encoding. If the encoding of the MSVC output is not UTF-8(for example, the default encoding for zh-CN locale is cp936), then Bazel will fail to detect the /showIncludes output. But this will not result in any error. Bazel will simply think this source file includes no header files. If any of these header files is modified without updating the source file, this source file will not be re-compiled, resulting in an incorrect build. We noticed this when we upgraded Bazel from 5.4.1 to 6.3.2 recently.

One way to fix this is by adding the VSLANG=1033 environment variable to force MSVC to generate output in English. @meteorcloudy suggested we can add this env var as a default to the parse_showincludes feature. We tested it, and it works great. Sadly, if we didn't install the English language pack, then MSVC would fall back to one of the languages installed, so VSLANG=1033 may cause a non-reproducible build and the problem is not entirely solved.

We can fix this problem by installing the English language or changing our build environment to UTF-8 encoding. But I think it's important to avoid Bazel causing incorrect builds without the user's knowledge. Here are some options I came up with:

1. Try to detect the encoding of the /showIncludes output. If it's not UTF-8, fail the build. But I don't know if we can find a reliable way to detect the encoding.
2. Make deps pruning with MSVC opt-in. We can disable the parse_showincludes feature by default. In order to do this, we need to add another feature to tell Bazel that MSVC won't produce a dotD file. Currently, Bazel uses the parse_showincludes feature to determine whether the compiler will generate dotD files. If we turn off the parse_showincludes feature, the build will fail because no dotD file is generated.
3. Add a message about this on the Using Bazel on Windows page

This problem should only affect a few users at present, but it invalidates the most important feature of Bazel: always correct. So I think it is worth solving, and I look forward to your opinions.

Thanks

[meteorcloudy]

Thanks for the nice summary! (Context #17928 (comment))

If it's not UTF-8, fail the build. But I don't know if we can find a reliable way to detect the encoding.

Yeah, this seems to be hard.

We can disable the parse_showincludes feature by default.

I believe header detection is also an important feature to ensure correctness for C++ builds, so disabling it by default doesn't sound ideal.

Add a message about this on the Using Bazel on Windows page

Yeah, personally I prefer setting VSLANG=1033 for parse_showincludes feature + documentation. Is the English language pack installed by default or it depends on the user's host config?

[fmeum]

@meteorcloudy Do you know whether there is a way to force a subprocess to run with cp 65001?

[kkpattern]

Do you know which encoding cl uses for the paths it emits?

I'm not sure what encoding is used for the paths because none of our source file paths contain non-ASCII characters. Generally speaking, it should be quite rare to use non-ASCII characters in C/C++ header filename I think? CMake seems utilized this, they match the /showIncludes output mainly relies on the two : and the following C/C++ header file path. Maybe we can use a similar regex pattern in Bazel?

[fmeum]

Having non-ASCII chars in C/C++ header filesnames would probably be rare, but the /showIncludes paths are absolute, which probably make them more likely to contain special characters. But I could see that being difficult already with Bazel or really any build system.

Maybe we can use a similar regex pattern in Bazel?

While we don't have regexes in Starlark, we could do something similar. However, we would need a way to store this information in the cc_toolchain for ShowIncludeFilter to pick it up.

[meteorcloudy]

Do you know whether there is a way to force a subprocess to run with cp 65001?

I have no idea...

[kkpattern]

Do you know whether there is a way to force a subprocess to run with cp 65001?

There is a Windows API named GetACP seems can get the code page of the current process. Maybe we can use it to get the code page then decode and re-encode the /showIncludes output into UTF-8. But I'm not familiar with this API, not sure if it's reliable. I only know that CPython is using it.