FR: Add option to disable warnings from external C++ headers

[devjgm]

I work on https://github.com/googleapis/google-cloud-cpp and we use CMake and bazel. Our project has several external dependencies defined in our WORKSPACE file. This is all working, but we'd like to be able to compile our own code with a high-level of compiler warnings, like -Wconversion, and we don't want to be bothered by errors in our external dependencies (because we cannot fix those errors).

In our CMake builds, our deps get "installed", and we include them with angle-brackets and they're found by some -isystem dir flags. This works fine with CMake.

However, this doesn't work with Bazel, because bazel encourages all code to be compiled with the same flags, so there's no distinction between my project's code and the code of our external dependencies. The issue is that if I want to compile with -Wconversion, I'll be warned about all my deps' problems that I cannot fix.

WANT: I'd like bazel to provide some supported mechanism to let users compile their own code different warnings flags than their dependencies.

Can bazel already do this? If so, what's the right way to disable warnings from external deps with Bazel?

We have some ideas that seem to work (maybe) that rely on the includes = [ "." ] attribute. We think this might be working, but it also seems like maybe the wrong thing. Is there some other recommended good solution that we should be considering?

[devjgm]

Tagging @hicksjoseph

[ulfjack]

How do you identify which code is first-party and which is third-party?

[devjgm]

I'm not sure who @ulfjack 's question is intended for, but I'll say that any dep that bazel thinks is an external dep should likely be considered third-party.

[devjgm]

Any update on this?

[oquenchil]

@ulfjack would Greg's suggestion work?

Make anything starting with external be included as /external? Would that mean flattening the headers nested set in analysis?

Regarding when to tackle this Greg, it would help a lot if you can provide a Windows repro. Also if you could try out replacing /I with /external yourself to verify that that it would work, I could take it from there and check it in the change.

[ulfjack]

I think the proposal makes sense. I'm not sure how it would be implemented though. We did something like this in google3, but I'm not sure how it currently works.

[derekmauro]

google3 basically says

--system-header-prefix=third_party/
# Projects in third_party to opt back in:
--no-system-header-prefix=third_party/absl/
...

--system-header-prefix is unfortunately a Clang-only thing. I believe an equivalent exists for MSVC, but I don't know of any good way to do it for GCC.

I agree with @devjgm that external deps are a good start as to what should be considered third party, though I guess it would also be nice if we could control that in some way.

[erenon]

Would you take a PR that makes headers of external libraries referenced using -isystem instead of -I?

[brentleyjones]

Should this be closed now that 08936ae landed?

[aminya]

Confirming that adding the following to .bazelrc fixes the issue:

build --features=external_include_paths

[ananta-code]

i am using -Wconditional-uninitialized flag and then i am getting some errrors from the library.

external/com_google_protobuf/src/google/protobuf/arena.cc:398:12: error: variable 'arena' may be uninitialized when used here [-Werror,-Wconditional-uninitialized]
return arena->AllocateAlignedWithCleanup(n, alloc_policy_.get());
^~~~~
external/com_google_protobuf/src/google/protobuf/arena.cc:395:21: note: initialize the variable 'arena' to silence this warning
SerialArena* arena;
^
= nullptr
1 error generated.

i want to filter out this error since it is coming from the library and i am using Bazel 5.0 and feature : build --features=external_include_paths but still i am seeing the above error from the library.

Could you please help me on this?

[ananta-code]

-Wconversion
i am using -Wconditional-uninitialized flag and then i am getting some errrors from the library.

external/com_google_protobuf/src/google/protobuf/arena.cc:398:12: error: variable 'arena' may be uninitialized when used here [-Werror,-Wconditional-uninitialized]
return arena->AllocateAlignedWithCleanup(n, alloc_policy_.get());
^~~~~
external/com_google_protobuf/src/google/protobuf/arena.cc:395:21: note: initialize the variable 'arena' to silence this warning
SerialArena* arena;
^
= nullptr
1 error generated.

i want to filter out this error since it is coming from the library and i am using Bazel 5.0 and feature : build --features=external_include_paths but still i am seeing the above error from the library.

Could you please help me on this?

[xumykargo]

Does external_include_paths only cover what's specified in includes in external cc_library rules?

I have multiple external repos in my WORKSPACE and with --verbose_failures, I can check which directories are using iquote / isystem. Not all the external headers are covered by isystem; some are only covered by iquote, and trigger warnings. By spotchecks, it seems like hdrs are not covered by isystem, and only includes are.