-
Notifications
You must be signed in to change notification settings - Fork 157
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
GRPC shouldn't be required if you don't use it #48
Comments
Apparently I needed I would have thought 'False' should be the default value for that field. But then: Same thing -- this should not be needed when I set 'with_grpc' to False. |
I agree this is suboptimal. The architectural decision to include both grpc and non-grpc metadata in the proto_language rule entangles these concepts. I will need to think about how to address this and don't have a satisfactory answer at the moment. |
I like when all I have to do is 'use_grpc = True' or similar.
Somehow there needs to be a clean check in the macro layer and make sure
that no gRPC labels are passed to the rule layer, otherwise resolution is
required.
…On Tue, Dec 13, 2016 at 10:50 PM, Paul Cody Johnston < ***@***.***> wrote:
I agree this is suboptimal. The architectural decision to include both
grpc and non-grpc metadata in the proto_language
<https://github.com/pubref/rules_protobuf/blob/master/cpp/BUILD#L5-L19>
rule entangles these concepts. I will need to think about how to address
this and don't have a satisfactory answer at the moment.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#48 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ALF-0hbQgr-qvyAjnr-5et_8gVZm5CYpks5rH2eFgaJpZM4LL18L>
.
|
I can't tell if this is related, but cc_proto_library fails to compile if with_grpc is false: $bazel build proto:cpp
INFO: Found 1 target...
ERROR: /home/test/blah/proto/BUILD:11:1: C++ compilation of rule '//proto:cpp' failed: linux-sandbox failed: error executing command /home/test/blah/.cache/bazel/_bazel_test/0fb2f33edd989c698a85a6fc0489350b/execroot/k2/_bin/linux-sandbox ... (remaining 47 argument(s) skipped).
In file included from bazel-out/local-fastbuild/genfiles/proto/k2.grpc.pb.cc:6:0:
bazel-out/local-fastbuild/genfiles/proto/k2.grpc.pb.h:12:46: fatal error: grpc++/impl/codegen/async_stream.h: No such file or directory
#include <grpc++/impl/codegen/async_stream.h>
^
compilation terminated.
Target //proto:cpp failed to build # proto/BUILD
package(default_visibility = ["//visibility:public"])
load("@org_pubref_rules_protobuf//cpp:rules.bzl", "cc_proto_library")
load("@org_pubref_rules_protobuf//java:rules.bzl", "java_proto_library")
filegroup(
name = "protos",
srcs = ["myproto.proto"],
)
cc_proto_library(
name = "cpp",
protos = [":protos"],
verbose = 0,
with_grpc = False,
imports = [
"external/com_github_google_protobuf/src/",
],
inputs = [
"@com_github_google_protobuf//:well_known_protos",
],
)
java_proto_library(
name = "java",
protos = [":protos"],
verbose = 0,
with_grpc = False,
imports = [
"external/com_github_google_protobuf/src/",
],
inputs = [
"@com_github_google_protobuf//:well_known_protos",
],
) bazel build proto:java works just fine though. Ideas? PS: thank you for releasing this tool. It has saved me many hours! |
@thaidn Thanks for reporting this. I'm looking into it now. |
I think I see the problem. I've moved your issue to #50 to close it with #51, but the larger issue raised by @pmbethe09 is still present. |
Note: these rules have been re-written and migrated to https://github.com/stackb/rules_proto. Please re-open there if issue persists, thanks. Cleaning up all issues on this repo, apologies in advance for closing without proper response. |
Simple proto of the form:
load("@com_github_pubref_rules_protobuf//cpp:rules.bzl", "cc_proto_library")
cc_proto_library(
name = "cards_proto_cc",
protos = ["cards.proto"],
)
Should not fail on:
no such package '@com_github_grpc_grpc//'
This dependency should only be sucked in when 'grpc_plugin' is set to something.
The text was updated successfully, but these errors were encountered: