cc_proto_library unusable with -Werror in CROSSTOOL

[RNabel]

Description of the problem / feature request:

Suppress all C++ compilation warnings when compiling the generated C++ code from .proto files or allow passing of copts to the cc_proto_library (related: #4446).

Feature requests: what underlying problem are you trying to solve with this feature?

The cc_proto_library compiles C++ code without allowing copts to be set, the generated C++ code is known to produce C++ compilation warnings.

If a custom CROSSTOOL sets strict error flags it makes cc_proto_library completely unusable.

Our CROSSTOOL for example sets:

  compiler_flag: "-Wall"
  compiler_flag: "-Wextra"
  compiler_flag: "-Wpedantic"
  compiler_flag: "-Werror"

and we are not prepared to ignore, for example, unused-parameters warnings.

The recommendation made by a protobuf maintainer is to make sure that "the protos are organized in a separate lib and compile them without warnings" (source). This is currently impossible.

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

Clone: https://github.com/cgrushko/proto_library

Run bazel build src:person_cc_proto --copt="-Werror -Weverything" to simulate a strict CROSSTOOL.

What operating system are you running Bazel on?

MacOS.

What's the output of bazel info release?

bazel 0.14.0

Have you found anything relevant by searching the web?

Comment mentioning a series of protobuf issues re generated code: protocolbuffers/protobuf#3128 (comment)

Bazel cc_proto_library should support cc_library parameters: #4446

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

Replace these lines with your answer.

If the files are large, upload as attachment or provide link.

[rongjiecomputer]

I want to ask for defines in cc_proto_library too in addition to copts.

One Tensorflow's proto file defines enum values as NEAR and FAR, but it will conflict with files that include the generated header and windows.h because windows.h defines macros named NEAR and FAR too if NOGDI is not defined.

Adding -DNOGDI to all other targets depending on this cc_proto_library will work, but it will be easier if I can just add defines = ["NOGDI"] in this cc_proto_library.

[RNabel]

You may want to add a comment about adding defines to #4446 as well.

[RNabel]

@cgrushko (since you wrote parts of the cc_proto_library) would you mind TAL and commenting on a) current plans to improve the cc_proto_library, b) what your preferred approach to this issue is, and c) whether #5467 has any chance of being accepted?

Sorry for directly pinging you, but it would be really awesome if this could be clarified :)

[cgrushko]

Hey, I unfortunately no longer work on Bazel. java_proto_library has a similar situation, and perhaps @cushon remembers what we do there. @lberki, can you route?

…

On Thu, Jun 28, 2018 at 11:49 AM Robin Nabel ***@***.***> wrote: @cgrushko <https://github.com/cgrushko> (since you wrote parts of the cc_proto_library) would you mind TAL and commenting on a) current plans to improve the cc_proto_library, b) what your preferred approach to this issue is, and c) whether #5467 <#5467> has any chance of being accepted? Sorry for directly pinging you, but it would be really awesome if this could be clarified :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5466 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB5_YXzcDhWRsC3aNMfCgq-I0ne6eMWvks5uBJiGgaJpZM4U4Awo> .

[cushon]

java_proto_library has a similar situation

We have some logic to pass additional javac flags to all proto compilations.

I'm not necessarily recommending this approach for c++, but it might be an option.

bazel/tools/jdk/default_java_toolchain.bzl

Line 47 in 178e6cc

PROTO_JAVACOPTS = [

bazel/src/main/java/com/google/devtools/build/lib/rules/java/proto/ProtoJavacOpts.java

Line 45 in 1d49184

.addAll(toolchain.getCompatibleJavacOptions(JavaSemantics.PROTO_JAVACOPTS_KEY))

[hlopko]

Current status: protoc should not generate warning triggerring code (protocolbuffers/protobuf#4628 (comment)). Lowering the priority of this issue to p4.

[hlopko]

Actually, there's nothing actionable for us here AFAIU. Therefore I'll close this issue. Please ping protocolbuffers/protobuf#4628 to make sure it will be paid attention to.