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

Disabling warnings from external headers #46

Open
tpudlik opened this issue Feb 2, 2023 · 2 comments
Open

Disabling warnings from external headers #46

tpudlik opened this issue Feb 2, 2023 · 2 comments

Comments

@tpudlik
Copy link
Contributor

tpudlik commented Feb 2, 2023

The default Bazel C++ toolchain has a cool feature that allows disabling warnings from external headers: bazelbuild/bazel#13107. It would be nice to add this to rules_cc_toolchain, too. It would make clang version upgrades less painful for downstream users like Pigweed (since third-party deps may pick up lots of new warnings on a version upgrade).

The implementation in the default toolchain is actually very simple, but it's not possible to express as a cc_feature, so it's not immediately clear to me how to add it.

Perhaps this should wait until the modular cc toolchains is finalized, since that may involve big changes to rules_cc_toolchain.

tpudlik added a commit to tpudlik/rules_cc_toolchain that referenced this issue Feb 3, 2023
This useful feature was added to the default Bazel C++ toolchain
in Bazel 5.0; see bazelbuild/bazel#13107.

Fixes bazelembedded#46
tpudlik added a commit to tpudlik/rules_cc_toolchain that referenced this issue Feb 3, 2023
This useful feature was added to the default Bazel C++ toolchain
in Bazel 5.0; see bazelbuild/bazel#13107.

Fixes bazelembedded#46
tpudlik added a commit to tpudlik/rules_cc_toolchain that referenced this issue Feb 3, 2023
This useful feature was added to the default Bazel C++ toolchain
in Bazel 5.0; see bazelbuild/bazel#13107.

Fixes bazelembedded#46
@tpudlik
Copy link
Contributor Author

tpudlik commented Feb 3, 2023

I used to think this would be quite easy: tpudlik@80677f3

But to my surprise, this leads to errors when I try to compile Pigweed. Typical error:

In file included from external/boringssl/src/crypto/x509/x_val.c:62:
In file included from external/boringssl/src/crypto/x509/internal.h:62:
external/boringssl/src/include/openssl/base.h:340:13: error: typedef redefinition with different types ('int' vs 'struct crypto_threadid_st')
typedef int CRYPTO_THREADID;
            ^
external/debian_stretch_amd64_sysroot/usr/include/openssl/crypto.h:237:3: note: previous definition is here
} CRYPTO_THREADID;
  ^

I think what's going on here is that the -I vs -isystem distinction was used in rules_cc_toolchain to override headers from the sysroot with headers from other external repos (boringssl in this case). But external_include_paths is implemented in upstream Bazel by converting -Is from external repos to -isystem.

This can probably be fixed by ensuring the include order for sysroot libraries is correct (so that they're overriden anyway), but the details are hazy to me.

@nathaniel-brough
Copy link
Collaborator

Yeah, I don't really have a good understanding of this either. I'm more than happy to change the usage of include flags in rules_cc_toolchain. I'll admit I'm not super familiar with the differences between; -I, -isystem, -iquote. I've read the docs for these flags, but I didn't feel like I fully understood the specific differences in enough detail to offer any guidance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants