bazelbuild / bazelbuild/rules_cc

Additional common compiler/target tuples config_settings

Open
#583 3 comments 0 reactions 0 assignees View on GitHub
P3
Dominant language
Starlark
Stars
247
Forks
196
PR merge metrics
No merged PRs in 30d

Description

When building for windows, most cc packages assume targeting `@platforms//os:windows` implies compiler is msvc-style (msvc-cl or clang-cl).

As part `toolchains_llvm_bootstrapped`, we use `clang` to build for windows using `mingw` and hit a number of build errors because of this assumption. We upstreamed a number of fixes already but sometimes, selecting on the compiler alone is enough.

For example, in the code below, the correct select statement would need to be:
* `@platforms//os:windows` AND `@rules_cc//cc/compiler=msvc-cl` => `bcrypt.lib`
* `@platforms//os:windows` AND `@rules_cc//cc/compiler=clang-cl` => `bcrypt.lib`
* `@platforms//os:windows` AND `@rules_cc//cc/compiler=clang` => `-lbcrypt`
* default => None

https://github.com/bazelbuild/bazel-central-registry/blob/e4af536fd0528f0bc212f285a17cae5e64b60a5d/modules/libxml2/2.15.1/overlay/BUILD.bazel#L76-L83
```
linkopts = select({
"@platforms//os:windows": [
"bcrypt.lib",
],
"//conditions:default": [
"-lm",
"-pthread",
"-ldl",
],
}),
```

LLVM uses an extra `config_setting` to handle this properly too:

https://github.com/llvm/llvm-project/pull/171761/changes#diff-2a89e8ab0e6871441adbd58fc757f5bb1d026cad5b15b57fa1c105f7a3de8e02R1556-R1558
```
linkopts = select({
"//llvm:is_windows_clang_mingw": ["-lversion"],
"//llvm:is_windows_clang_cl": ["version.lib"],
"//llvm:is_windows_msvc": ["version.lib"],
"//conditions:default": [],
}),
```

As we progress fixing packages to support building for windows using `clang`, we will encounter more and more of those issues.

Would it be reasonable to have those config_settings inside `rules_cc` instead of redefining them in every project where it's needed ?

cc @keith since you added those `config_setting` first in llvm.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.