Allow reducing transitive includes arguments for many generated C++ headers

[jemeador]

Description

I'm trying to get the most out of Bazel for our C++ builds, and the first step is breaking up some monolithic cpp_proto_library rules. I've created some automation in my project to split up a small set of giant protobuf files into a large set of fine-grained proto_library and cpp_proto_library build rules. The automation produces build rules that work just fine when using clang. Unfortunately, this does not work on our target platform. On ARM, gcc8 complains about argument lists being too long.

ERROR: /path/to/workspace/src/somelib/BUILD:14:11: Compiling src/somelib/somecode.cpp failed: (Exit 1): arm-linux-gnueabihf-gcc failed: error executing command (from target //src/somelib:lib) external/local_arm_cc_toolchains/wrappers/arm-linux-gnueabihf-gcc '--sysroot=external/arm_gcc_8_3_0/arm-linux-gnueabihf/libc' -Bexternal/arm_gcc_8_3_0/bin ... (remaining 3991 arguments skipped)
Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging arm-linux-gnueabihf-gcc: error trying to exec 'external/arm_gcc_8_3_0/libexec/gcc/arm-linux-gnueabihf/8.3.0/cc1plus': execv: Argument list too long

I get this error because I now have hundreds of compiler flags for every cc_* rule that indirectly depends on the full set of generated protos and C++ headers. Running the build with -s shows the problem:

...
-isystem external/my_defs_repo/my_defs/foo_message_cc_pb
-isystem bazel-out/armeabi-v7a-opt/bin/external/my_defs_repo/my_defs/foo_message_cc_pb
-isystem external/my_defs_repo/my_defs/bar_message_cc_pb
-isystem bazel-out/armeabi-v7a-opt/bin/external/my_defs_repo/my_defs/bar_message_cc_pb
...

There is a pair of -isystem flags for every one of the nearly 1000 cpp_*_library rules.

I can use output_mode = "NO_PREFIX" in cpp_*_library to get all headers in one output directory, but Bazel still tries to include some rule-specific paths instead of the flattened-out directory. This is what my output directory looks like under bazel-out/armeabi-v7a-opt/bin/external/my_defs_repo/my_defs:

├── foo_message_cc_pb 
│ └── my_defs 
│  ├── foo_message.pb.cc
│  └── foo_message.pb.h 
├── foo_message.pb.cc
├── foo_message.pb.h 
├── foo_message_proto-descriptor-set.proto.bin 
├── bar_message_cc_pb 
│ └── my_defs 
│  ├── bar_message.pb.cc
│  └── bar_message.pb.h 
├── bar_message.pb.cc
├── bar_message.pb.h 
├── bar_message_proto-descriptor-set.proto.bin

Changing the cpp_*_library rules to omit the line includes = [name_pb], from the underlying cc_library rule fixes the problem. The redundant foo_message_cc_pb and bar_message_cc_pb directories are omitted, and only one include path is necessary. Can we allow the configuration of includes to make the best use of output_mode = "NO_PREFEIX" in this situation? Ideally, the rule would continue to transitively include these headers as system headers as well (with my change, they are only available under an -iquote path).

[aaliddell]

Hmm, in theory we could just disable setting includes when output mode is set non-default, since the current include points to a directory that I don't think exists. If you run bazel clean and relist your bazel-out/armeabi-v7a-opt/bin/external/my_defs_repo/my_defs directory after a build with includes = [name_pb] removed, do you still see the foo_message_cc_pb directory?

[jemeador]

If I recall correctly, the foo_message_cc_pb is absent if includes = [name_pb] is removed

[aaliddell]

Could you try using the branch cpp-output-mode?

http_archive(
    name = "rules_proto_grpc",
    sha256 = "90a2c4516891484841259239b83ae050a109cabb8466f2410e7b387b71bc89c9",
    strip_prefix = "rules_proto_grpc-6a5dd8156e29c151fa4babdbd1780dfd6a00271c",
    urls = ["https://github.com/rules-proto-grpc/rules_proto_grpc/archive/6a5dd8156e29c151fa4babdbd1780dfd6a00271c.tar.gz"],
)

[jemeador]

This works! I'll play with it a bit and see if I can get the headers pulled in as -isystem instead of -iquote

[jemeador]

I was able to get this hammered into shape with something like this I changed the cpp_proto_library (and grpc) rule to use includes = '.'. Ideally, includes would be configurable, but default to something reasonable.

protos are in //mydefs/package_name

//my_defs/BUILD.bazel:

cpp_proto_library(
    name = "foo_message_cc",
    protos = [":foo_message_proto"],
    copts = ["-I", "$(GENDIR)/my_defs/"],
    deps = [
        ":bar_message_cc",
    ],
    output_mode = "NO_PREFIX",
)
proto_library(
    name = "foo_message_proto",
    strip_import_prefix = "/mydefs",
    srcs = ["package_name/foo_message.proto"],
    deps = [
        ":bar_message_proto",
    ],
)

This gives me the -isystem path I need without redundancy.

[aaliddell]

Closed by #276