Repository: bazelbuild/bazel
Issue: --host_cxxopt and --cxxopt are passed to the compiler also for .c files on Windows
Labels: type: bug
P2
team-Rules-CPP
help wanted

<!--
ATTENTION! Please read and follow:
- if this is a _question_ about how to build / test / query / deploy using Bazel, or a _discussion starter_, send it to bazel-discuss@googlegroups.com
- if this is a _bug_ or _feature request_, fill the form below as best as you can.
-->

### Description of the problem / feature request:

Since Bazel 5.0.0 `--host_cxxopt` and `--cxxopt` are passed to compilers compiling .c files on Windows.

### Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Use the autoconfigure toolchain to build (with -s) a .c file in Windows. Set `--host_cxxopt`  and `--cxxopt` in the .bazelrc and see in the compiler call the passed opts from the .bazelrc. Doing the same on Linux show, that the opts here are not passed.

### What operating system are you running Bazel on?

Windows 10 Enterprise (Version 10.0.19042 Build 19042)

### What's the output of `bazel info release`?

release 5.0.0

###  Have you found anything relevant by searching the web?

https://github.com/bazelbuild/bazel/pull/14005
https://github.com/bazelbuild/bazel/pull/14131

<!--
Places to look:
 - StackOverflow: http://stackoverflow.com/questions/tagged/bazel
 - GitHub issues: https://github.com/bazelbuild/bazel/issues
 - email threads on https://groups.google.com/forum/#!forum/bazel-discuss
-->

### Any other information, logs, or outputs that you want to share?

This commit https://github.com/bazelbuild/bazel/commit/861584ca049b1b2f3474c3977c4a54d1901f9458 introduced case insensity for files types on Windows 



https://github.com/bazelbuild/bazel/blob/88499f615bc03d1b6a3e4e38f2890a4e987a954d/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java#L28

The change also leads to .c files under Windows match to C_SOURCE (.c extension) and CPP_SOURCE (.C extension)

https://github.com/bazelbuild/bazel/blob/88499f615bc03d1b6a3e4e38f2890a4e987a954d/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java#L33-L34

Which results in passing cxxopt for .c files

https://github.com/bazelbuild/bazel/blob/88499f615bc03d1b6a3e4e38f2890a4e987a954d/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java#L1605-L1610

<!-- If the files are large, upload as attachment or provide link. -->


Issue comments:
Maybe this is a similar bug that should be fixed like https://github.com/bazelbuild/bazel/commit/edfe2a17e3434cce660757f59b14f2e9d6ab944e?
/cc @buildbreaker2021
We also encounter this problem. Unfortunately this causes us to not be able to compile Android on windows. Which is a major block for our project. Is there a rough estimate of when this will be fixed? Or is there any way we can workaround this? Thanks.
The worst thing here is that this issue is not even particularly complicated, even with `WindowsOsPathPolicy` rework included (the right thing to do: support for case sensitive directories on Windows 10+ will solve a lot of problems). Yet, it breaks a whole lot of plain C libraries on Windows putting a big question mark on Bazel's viability in general.

Which is a pity.

> The worst thing here is that this issue is not even particularly complicated, even with `WindowsOsPathPolicy` rework included (the right thing to do: support for case sensitive directories on Windows 10+ will solve a lot of problems). Yet, it breaks a whole lot of plain C libraries on Windows putting a big question mark on Bazel's viability in general.
> 
> Which is a pity.

Currently, you can workaround this by replacing `--cxxopt` with `--per_file_copt=.*\.cpp\$@`.

But I agree that it's very confusing why this issue is a feature request and is P4.
It won't work for flags which are set in the actual BUILD files. The primary issue here, is that bazel offers no way to say "this file is plain C" explicitly. So, in a project with several dozens of third party dependencies a lot of patching and testing is needed to get anywhere with Bazel v5.

Bazel v4 sort of works, but then it can not be used with VS2022 due to another open issue.

> Bazel v4 sort of works, but then it can not be used with VS2022 due to another open issue.

I thought this is not a problem for a msbuild based toolchain because msbuild only gives a warning? For us, it's a block issue because we're doing android cross compile on Windows and pass C++ flag to C compile simply failed.

Another interesting workaround is that we found somehow the C++ flags defined in toolchain won't be passed to C targets. So we end up moving the C++ flags to toolchain. But yeah it's lucky for us to be able to workaround this bug. For some complex project this may not work.
VS2022 is broken pretty badly, we had to go back to VS2019/bazel v4 for now. We may figure something out going forward; possibly by making all sources C++ compatible (this appears to be the best option on the cost/benefit scale).

Still, not nice.

@oquenchil This issue seems to have wide user impact on Windows. Is it caused by starlarkfifying the cc rules? Do you know which exact change caused this and how hard is it to fix this?
@meteorcloudy Looks like is caused by #14005. The `.c` extension is matched with `.C` cause bazel treat it as a C++ source. I think this can be fixed like https://github.com/bazelbuild/bazel/commit/edfe2a17e3434cce660757f59b14f2e9d6ab944e . I understand bazel team has many things to do and may not have the capacity to fix this. If you have the bandwidth to review a PR I can give this a try.
Not sure why `.c` and `.C` should mean different file types on Windows because file names are already case insensitive. Is this a Linux convention? Can you just change files with `.C` to `.cpp`?  
But given we are already making an exception in https://github.com/bazelbuild/bazel/commit/edfe2a17e3434cce660757f59b14f2e9d6ab944e, a PR to fix this sounds plausible, @oquenchil @comius should decide eventually.
> Can you just change files with `.C` to `.cpp`?

We don't actually have any `.C` files. I think it's bazel that treated all `.c` files as `.C` files causing the problem.


@meteorcloudy Also see #15550 for some further info.
Any progress on this issue? Bazel cannot compile C targets under Windows anymore since version 5. I don't think this warrants classification as a "feature request" or "P4".
Bazel can compile things on Windows, it just requires jumping through ugly loops with `--per_file_copt=` and possibly some patching.

But yes, this issue is bad and the lack of proper support for case sensitive abilities of NTFS is also unfortunate - that is, bazel would have worked fine on Windows with some extra configs if not for the rather stupid case forcing inside the Windows path resolver.


From the [documentation](https://bazel.build/docs/user-manual): `--cxxopts: This is similar to --copt, but only applies to C++ compilation, not to C compilation or linking.` In obvious contradiction bazel 5.3.2 passes `--cxxopts` arguments to C files. This is clearly a bug and not a "feature request". @oquenchil

I find if concerning that something that elementary is broken in bazel and swiped under the rug.
This bug is still causing significant problems in a project, forcing us to use an old bazel version which causes further compatibility issues and surfaces other bugs without fixes. Will it ever be looked at?
It's almost like Windows is not an important OS for developers any more. :-D

Yet, Windows is still the most used developer OS by a huge margin.

@oquenchil I think this shouldn't be a P4 issue, can you take a look?

If anyone has a fix for this, a PR is also very welcome!
Hi, I've created a PR. It's basically learned from https://github.com/bazelbuild/bazel/commit/edfe2a17e3434cce660757f59b14f2e9d6ab944e . I'm not very familiar to java actually, so please tell me if I did anything wrong. I've tested it on my environment and added a test.
Created the referenced Cherrypick @kkpattern Any objections agains the attempt?
> Created the referenced Cherrypick @kkpattern Any objections agains the attempt?

Not at all😃.
Why is this closed? Observed in the latest 5.x and 6.x builds.

This is C code! `sign.c` is a C source file! `--host_cxxopt=-std=c++17` causes `-std=c++17` to be passed to C compilation!!!

```
/home/.../.cache/bazel/_bazel_build/install/a09dbb90c658248f08f9aa0eba11997d/process-wrapper '--timeout=0' '--kill_delay=15' '--stats=/home/.../.cache/bazel/_bazel_build/36f2a9693014a18078cf75940b43707b/sandbox/processwrapper-sandbox/114/stats.out' \

/usr/bin/gcc-9 -MD -MF bazel-out/k8-dbg-gcc/bin/external/boringssl/_objs/crypto/sign.pic.d \
   '-frandom-seed=bazel-out/k8-dbg-gcc/bin/external/boringssl/_objs/crypto/sign.pic.o' \
   '-DBAZEL_CURRENT_REPOSITORY="boringssl"' \
   -iquote external/boringssl -iquote bazel-out/k8-dbg-gcc/bin/external/boringssl \
   -isystem external/boringssl/src/include \
   -isystem bazel-out/k8-dbg-gcc/bin/external/boringssl/src/include \
   '-std=c++17' \ # WUHT?????
   -g -O0 -fno-omit-frame-pointer -fPIC -fno-canonical-system-headers \
   '-fdiagnostics-color=always' -Wa,--noexecstack -Wall -Werror '-Wformat=2' \
   -Wsign-compare -Wmissing-field-initializers -Wwrite-strings -Wshadow \
   -fno-common '-D_XOPEN_SOURCE=700' '-std=c11' -Wmissing-prototypes \
   -Wold-style-definition -Wstrict-prototypes -c external/boringssl/src/crypto/evp/sign.c \
   -o bazel-out/k8-dbg-gcc/bin/external/boringssl/_objs/crypto/sign.pic.o
```
6.3.0 should contain the fix? https://github.com/bazelbuild/bazel/pull/18552
I did a simple test. When compile with MSVC with ` --cxxopt=/std:c++17`, bazel 5.1.1 added the flag to a C source file:

```
C:\...\cl.exe ... -D__TIME__="redacted" /std:c++17 ... /c hello.c
                                         ^^^^^^^^^
```

while bazel 6.3.2 didn't:

```
C:\...\cl.exe ... -D__TIME__="redacted" ... /c hello.c
```
I'm using Ubuntu 22.04... so I'm seeing the problem on Linux. Given that we're not using Bzlmod (because of incompatibilities), it's possible that I have a dependency of a dependency that's getting an old version of cc_tools or something. It's frustrating because it my build is broken because of two rather trivial bugs that _look_ like they're in bazel. (Where does @local_config_cc come from, after all???)

Pull request review comments:

Patch that resolved the issue:
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
index d525d0be65d8e5..c55cf90028708f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java
@@ -30,9 +30,44 @@ public final class CppFileTypes {
   // .cu and .cl are CUDA and OpenCL source extensions, respectively. They are expected to only be
   // supported with clang. Bazel is not officially supporting these targets, and the extensions are
   // listed only as long as they work with the existing C++ actions.
+  // FileType is extended to use case-sensitive comparison also on Windows
   public static final FileType CPP_SOURCE =
-      FileType.of(".cc", ".cpp", ".cxx", ".c++", ".C", ".cu", ".cl");
-  public static final FileType C_SOURCE = FileType.of(".c");
+      new FileType() {
+        final ImmutableList<String> extensions =
+            ImmutableList.of(".cc", ".cpp", ".cxx", ".c++", ".C", ".cu", ".cl");
+
+        @Override
+        public boolean apply(String path) {
+          for (String ext : extensions) {
+            if (path.endsWith(ext)) {
+              return true;
+            }
+          }
+          return false;
+        }
+
+        @Override
+        public ImmutableList<String> getExtensions() {
+          return extensions;
+        }
+      };
+
+  // FileType is extended to use case-sensitive comparison also on Windows
+  public static final FileType C_SOURCE =
+      new FileType() {
+        final String ext = ".c";
+
+        @Override
+        public boolean apply(String path) {
+          return path.endsWith(ext);
+        }
+
+        @Override
+        public ImmutableList<String> getExtensions() {
+          return ImmutableList.of(ext);
+        }
+      };
+
   public static final FileType OBJC_SOURCE = FileType.of(".m");
   public static final FileType OBJCPP_SOURCE = FileType.of(".mm");
   public static final FileType CLIF_INPUT_PROTO = FileType.of(".ipb");


Test patch:
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppFileTypesTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppFileTypesTest.java
index 88aa7ba707292f..4487d6545b8096 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CppFileTypesTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CppFileTypesTest.java
@@ -77,4 +77,12 @@ public void testNoExtensionLibraries() {
     assertThat(Link.ARCHIVE_LIBRARY_FILETYPES.matches("someframework")).isTrue();
     assertThat(Link.ARCHIVE_FILETYPES.matches("someframework")).isTrue();
   }
+
+  @Test
+  public void testCaseSensitiveCFiles() {
+    assertThat(CppFileTypes.C_SOURCE.matches("foo.c")).isTrue();
+    assertThat(CppFileTypes.CPP_SOURCE.matches("foo.c")).isFalse();
+    assertThat(CppFileTypes.C_SOURCE.matches("foo.C")).isFalse();
+    assertThat(CppFileTypes.CPP_SOURCE.matches("foo.C")).isTrue();
+  }
 }