Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[rules_cc]: Bazel 5 is not matching case sensitive file extensions on Windows correctly #15550

Closed
oakad opened this issue May 23, 2022 · 7 comments
Labels
area-Windows Windows-specific issues and feature requests P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: bug

Comments

@oakad
Copy link

oakad commented May 23, 2022

Description of the bug:

Bazel considers ".c" (lowercase c) files to be plain c files and would not apply cxxopts when compiling files with this extension. However, Bazel considers ".C" (uppercase C) files to be C++ files and all the cxxopts apply.

It so happens, that on Windows Bazel will match all the "*.c" files as "C++ sources" because of case insensitive matching. This sort of works for normal compiles, because MSVC compiler tends to silently ignore C++ specific flags passed to compiler when compiling C files.

However, when cross compiling to something like Android, clang will refuse to compile the C sources, because "c++ flags are "not allowed with 'C'".

Some sort of mitigation is necessary here.

It is rather unfortunate, that to this day Bazel has no way to designate plain C files explicitly.

What's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Cross compile any C library on Windows with Android clang compiler.

Compile any C library on Windows and observe C++ flags added to the compiler command line (even though the compilation may work).

Which operating system are you running Bazel on?

Windows 10

What is the output of bazel info release?

release 5.1.1

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse master; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

No response

Any other information, logs, or outputs that you want to share?

No response

@oakad
Copy link
Author

oakad commented May 23, 2022

I would like to further add that Windows now supports case sensitive file name matching which can be enabled on per-directory level. Alas, implementation in com.google.devtools.build.lib.vfs.WindowsOsPathPolicy is totally not friendly to this sort of development - it forces case conversion blindly, which is rather wrong. I'd say, there should be a command line flag to control VFS behaviour on windows, allowing Unix-style behaviour where enabled.

@sgowroji
Copy link
Member

Hello @oakad, Thank you for reaching us regarding the above query. Could you please provide a repo with complete steps to reproduce the above issue. Thanks!

@oakad
Copy link
Author

oakad commented May 24, 2022

Here: https://github.com/oakad/bazel_issue_15550

The issue triggers always on most basic of setups. :-)

On Linux: bazel run :t1 compiles and runs, no issues, no warnings.

On Windows, with default MSVC toolchain, we get the following warning:

cl : Command line warning D9002 : ignoring unknown option '-bogus_c++_flag'

The issue here is that t1.c gets classified as C++ source and cxxopt gets applied, incorrectly.

Of course, MSVC goes on to compile the program anyway, because it is lenient like that.

But any cross-compiling, non MSVC based toolchain will break.

@sgowroji sgowroji added type: bug area-Windows Windows-specific issues and feature requests untriaged and removed more data needed labels May 25, 2022
@foxandi
Copy link
Contributor

foxandi commented May 30, 2022

Might be related to: #15073

@oakad
Copy link
Author

oakad commented May 31, 2022

@foxandi It does look like the same issue.

@oquenchil oquenchil added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) and removed untriaged labels Jul 7, 2022
@github-actions
Copy link

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

@github-actions github-actions bot added the stale Issues or PRs that are stale (no activity for 30 days) label Sep 11, 2023
@fmeum
Copy link
Collaborator

fmeum commented Sep 11, 2023

Appears to be a duplicate of #15073, which has been fixed.

@fmeum fmeum closed this as completed Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Windows Windows-specific issues and feature requests P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Rules-CPP Issues for C++ rules type: bug
Projects
None yet
Development

No branches or pull requests

5 participants