bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
22.48k stars 3.93k forks source link

cc_proto_library should support cc_library parameters (like copts, linkopts, etc) #4446

Open pmuetschard opened 6 years ago

pmuetschard commented 6 years ago

Description of the problem / feature request:

The cc_proto_library is, in essence a cc_library rule and should thus honor the cc_library parameters, such as copts.

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

Large protos may require the -Wa,-mbig-obj copt on Windows, but it is not always possible to use that flag globally (e.g. if also compiling go code).

There are other reason why people may want to control the underlaying cc_library rule, of course.

What operating system are you running Bazel on?

Linux, OSX, Windows

What's the output of bazel info release?

0.9.0

RNabel commented 5 years ago

Hey @jin (sorry to rope you into this directly): is there some progress or a known workaround? I'm on bazel 0.14.0 and protobuf 3.4.0, my CROSSTOOL compiles with "-Wall" - the generated code breaks with this flag due to unused-parameters and extended-offsetof.

I tried to wrap cc_proto_library with a cc_library with copts = ["-w"] but the compilation still fails, stating it cannot build the underlying proto_library. i.e.:

proto_library(
    name = "A", 
    srcs = "some.proto"
    # in my case also depends on googleapis.
)

cc_proto_library(
    name = "B",
    deps = ":A",
)

cc_library(
    name = "C",
    deps = [":B"],
    copts = ["-w"]
)

Error:

C++ compilation of rule '//:A' failed (Exit 1) etc. etc.

bazel-out/darwin-opt-clang/genfiles/external/com_github_googleapis_googleapis/google/api/http.pb.cc:79:3: error: using extended field designator is an extension [-Werror,-Wextended-offsetof]
  GOOGLE_PROTOBUF_GENERATED_MESSAGE_FIELD_OFFSET(HttpRule, _oneof_case_[0]),
  ^                                                                    ~~~
external/protobuf/src/google/protobuf/generated_message_util.h:92:3: note: expanded from macro 'GOOGLE_PROTOBUF_GENERATED_MESSAGE_FIELD_OFFSET'
  __builtin_offsetof(TYPE, FIELD)                                    \
  ^                        ~~~~~
bazel-out/darwin-opt-clang/genfiles/external/com_github_googleapis_googleapis/google/api/http.pb.cc:254:57: error: unused parameter 'arena' [-Werror,-Wunused-parameter]
void Http::RegisterArenaDtor(::google::protobuf::Arena* arena) {

See related protobuf ticket: https://github.com/google/protobuf/issues/4628

jin commented 5 years ago

@RNabel I have little contextual knowledge around cc_{proto_}_library, adding in @mhlopko who works on the cc_* rules.

RNabel commented 5 years ago

I got something hacky to work, maybe this helps someone with the same problem.

  1. extract the cc_proto_library's compile_flags manually by using a custom rule, I used:
def _cc_proto_lib_impl(ctx):
  p = ctx.attr.cc_proto_libraries[0].cc
  print("=" * 50)
  print("compile_flags", p.compile_flags) # <---- these flags.
  print("=" * 50)
  print("defines", p.defines)
  print("=" * 50)
  print("include_directories", p.include_directories)
  print("=" * 50)
  print("libs", p.libs)
  print("=" * 50)
  print("link_flags", p.link_flags)
  print("=" * 50)
  print("quote_include_directories", p.quote_include_directories)
  print("=" * 50)
  print("system_include_directories", p.system_include_directories)
  print("=" * 50)
  print("transitive_headers", p.transitive_headers)
  print("=" * 50)

wrapped_cc_proto_library = rule(
    implementation = _cc_proto_lib_impl,
    attrs = {
        "cc_proto_libraries": attr.label_list(mandatory = True, allow_empty = False),
    },
)

This rule is then imported and used within a BUILD file like so:

wrapped_cc_proto_library(name = "whatever", cc_proto_libraries = [":your_cc_proto_library_target"])

which will print the contents of all fields in the cc provider of the cc_proto_library.

  1. Create a cc_library in which you copy-paste the compile_flags into the copts + add additional copts. I added "-w" to suppress all warnings. It would looks roughly like this:
cc_library(
    name = "some_general_cc_target",
    srcs = [
        ":some_cc_proto_library", # The main cc_proto_library library you're compiling. Yes, passed to srcs. Ugh. 
        ":transitive_dep_cc_proto_library", # You need to include all transitive dependencies' cc_proto_libraries.
    ],
    copts = [
        # Compile flags extracted using the custom rule here.
        "-isystem external/com_google_protobuf/src", etc. etc.

        # Custom copts.
        "-w",
    ]
)
hlopko commented 5 years ago

@RNabel I'd advise you not to spend too much time on that, since https://github.com/bazelbuild/bazel/issues/4570 is being actively worked on and that will allow you to do exactly what you want - create the exactly same actions as C++ rules would.

RNabel commented 5 years ago

@mhlopko Nice, thanks for the heads up. Is there a rough timeline?

hlopko commented 5 years ago

The roadmap says it will be done in July, which will be a challenge but it's possible. So it will be in released Bazel in August or in September. If all goes well.

meisterT commented 4 years ago

@oquenchil what's the current state?

oquenchil commented 4 years ago

No plans to work on this in the short term. I updated priorities.

TheKaur commented 3 years ago

Any further update on this? This would be a pretty useful feature for us.

jonathan-enf commented 2 years ago

Sadly, cc_common does not seem to contain a cc_proto_library rule, requiring that the user build their own by reverse-engineering the old cc_proto_library rule.

yeswalrus commented 1 year ago

Ping, any word on updating cc_proto_library with this support?

oquenchil commented 1 year ago

No plans in the immediate future.

github-actions[bot] commented 8 months ago

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 90 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

github-actions[bot] commented 5 months ago

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!