Skip to content
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

Issues building protos, and using them #388

Open
sgammon opened this issue Jun 7, 2019 · 12 comments
Open

Issues building protos, and using them #388

sgammon opened this issue Jun 7, 2019 · 12 comments

Comments

@sgammon
Copy link
Contributor

sgammon commented Jun 7, 2019

Hey there rules_closure folks,

I was wondering if someone could help me out with compilation and use of protos from Soy (in JS and Java). First things first, I'm aware there is not yet support in rules_closure for proto use from Java, although I was not able to answer that question fully for JS.

SomeModel.proto:

syntax = "proto3";
package sample;

message Sample {
    string example = 1;
}

WORKSPACE:

workspace(name = "some_workspace")

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

## Closure
http_archive(
    name = "io_bazel_rules_closure",
    strip_prefix = "rules_closure-9e82c789d83ddc57e98e11ecb6c0f0479ebbbb5b",
    url = "https://github.com/bazelbuild/rules_closure/archive/9e82c789d83ddc57e98e11ecb6c0f0479ebbbb5b.zip")

BUILD:

package(default_visibility = ["//visibility:public"])

load("@io_bazel_rules_closure//closure:defs.bzl",
     "closure_js_proto_library",
     "closure_proto_library")


# Protos
proto_library(
    name = "SomeModel",
    srcs = ["SomeModel.proto"])

closure_js_proto_library(
    name = "SomeModel_js_proto",
    srcs = ["SomeModel.proto"])

closure_proto_library(
    name = "SomeModel_closure_proto",
    deps = [":SomeModel"])

I can't get Soy to recognize the proto types, whether I'm using closure_js_proto_library or closure_proto_library. The behavior seems rather strange:

When using closure_js_proto_library:

  • I depend on it from a closure_js_template_library rule, like so:
closure_js_template_library(
    name = "labtesting-tpl",
    srcs = ["labtesting.soy"],
    deps = ["@schema//:SomeModel_js_proto"])
  • But I get the following output (with verbose_failures on):
INFO: Analysed 18 targets (159 packages loaded, 2483 targets configured).
INFO: Found 18 targets...
ERROR: /private/var/tmp/_bazel_sam/.../external/.../BUILD.bazel:25:1: Generating JavaScript Protocol Buffer file @schema//:SomeModel_closure_proto_gen failed (Exit 1): bash failed: error executing command 
  (cd /private/var/tmp/_bazel_sam/.../execroot/SomeWorkspace && \
  exec env - \
    PATH='(really-long-path-variable)' \
  /bin/bash -c 'source external/bazel_tools/tools/genrule/genrule-setup.sh; bazel-out/host/bin/external/com_google_protobuf/protoc -I. --js_out=library=SomeModel_closure_proto,error_on_name_conflict,binary:bazel-out/darwin-fastbuild/genfiles/external/schema/SomeModel --descriptor_set_out=bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel_closure_proto.descriptor bazel-out/darwin-fastbuild/genfiles/external/schema/SomeModel-descriptor-set.proto.bin')
Execution platform: @bazel_tools//platforms:host_platform
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:2:1: Interpreting non ascii codepoint 179.
[libprotobuf WARNING external/com_google_protobuf/src/google/protobuf/compiler/parser.cc:562] No syntax specified for the proto file: bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin. Please use 'syntax = "proto2";' or 'syntax = "proto3";' to specify a syntax version. (Defaulted to proto2 syntax.)
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:2:1: Expected top-level statement (e.g. "message").
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:2:2: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:3:39: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:3:73: Interpreting non ascii codepoint 145.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:3:74: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:4:13: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:5:5: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:6:7: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:7:7: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:8:5: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:10:7: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:11:7: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:13:8: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:14:5: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:15:5: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:16:7: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:17:5: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:17:7: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:18:5: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:18:7: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:19:7: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:20:1: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:20:18: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:21:27: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:22:10: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:23:14: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:24:7: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:25:14: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:26:10: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:27:48: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:27:70: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:27:72: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:27:73: Interpreting non ascii codepoint 162.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:27:74: Invalid control characters encountered in text.
bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin:27:80: Invalid control characters encountered in text.
INFO: Elapsed time: 52.657s, Critical Path: 24.80s
INFO: 581 processes: 553 local, 28 worker.
FAILED: Build did NOT complete successfully
  • When I examine the file bazel-out/darwin-fastbuild/genfiles/external/schema/.../SomeModel-descriptor-set.proto.bin, it contains a binary representation of the proto. So, it does compile the proto.

  • I can also accurately find a JS representation of the proto, in bazel-out. It's in Closure format (goog.require/goog.provide), which is how I want it, but I don't see how I can reference that file.

It seems to me, that one of closure_js_template_library or closure_js_proto_library is expecting the protos to be in source form (hence the message No syntax specified for the proto file)? What am I doing wrong?

@sgammon
Copy link
Contributor Author

sgammon commented Jun 7, 2019

When using closure_proto_library:

  • This seems to be the new version of the first rule I mentioned. So I tried to depend on it, but, I get the following output:
INFO: Analysed 18 targets (159 packages loaded, 2478 targets configured).
INFO: Found 18 targets...
ERROR: /long/path/to/soy/templates/base/BUILD:10:1: Generating 1 SOY v2 JS file(s) failed (Exit 1): SoyToJsSrcCompiler failed: error executing command 
  (cd /private/var/tmp/_bazel_sam/.../execroot/SomeWorkspace && \
  exec env - \
  bazel-out/host/bin/external/com_google_template_soy/SoyToJsSrcCompiler '--outputPathFormat=bazel-out/darwin-fastbuild/genfiles/{INPUT_DIRECTORY}/{INPUT_FILE_NAME}.js' '--bidiGlobalDir=1' '--srcs=soy/templates/base/sample.soy')
Execution platform: @bazel_tools//platforms:host_platform
errors during Soy compilation
soy/templates/base/sample.soy:11: error: Unknown type 'proto.sample.Sample'.
    {@param proto1: proto.sample.Sample}  // Sample proto.
                     ^

soy/templates/base/sample.soy:66: error: Unknown type 'sample.Sample'.
    {@param proto2: sample.Sample}  // Sample proto with unqualified path.
                         ^
2 errors

INFO: Elapsed time: 74.218s, Critical Path: 49.91s
INFO: 630 processes: 572 local, 58 worker.
FAILED: Build did NOT complete successfully

@Yannic
Copy link
Contributor

Yannic commented Jun 7, 2019

Regarding closure_proto_library: Unfortunately, that's a known limitation of the new rule. #314 aimed to fix that, but it was decided that it's a bit weird to have this special handling for templates. @gkdn I agree with your previous comment in #314, but it might be worth to merge it anyway, so people don't have to use the legacy closure_js_proto_library rule.

Regarding closure_js_proto_library: I haven't used that in a very long time. I think https://github.com/bazelbuild/rules_closure/blob/master/closure/templates/test/BUILD might help you to figure out the right way to do what you're trying to do.

@sgammon
Copy link
Contributor Author

sgammon commented Jun 7, 2019

@Yannic, i actually got it to work somehow, i'm still trying to figure out now if i'm using a fork and it's my local state, or if it was fixed in master for my use case. but thank you for your response in any case.

regarding closure_proto_library, the restriction for only one entry in deps is rather cumbersome. are there still plans to lift this restriction? we keep a whole tree of dependencies in sync with proto_library definitions, so, if there would be some way to make that sufficient, we would be very much in favor of it, and would help in any way we could to see that happen.

@sgammon
Copy link
Contributor Author

sgammon commented Jun 7, 2019

@Yannic then again, if that one dep supports aliases, and transitive dependencies, that would be fine with us (in terms of closure_proto_library). thank you again, this project is awesome and we want to help any way we can

@sgammon
Copy link
Contributor Author

sgammon commented Jun 7, 2019

@gkdn please, could we merge that PR, pending a better approach? 😁

@Yannic
Copy link
Contributor

Yannic commented Jun 7, 2019

@sgammon We have this restriction to one entry to be in sync what {cc,java}_proto_library do, so I'd rather not lift that restriction.

I'm not sure what you mean by alias, but transitive dependencies should just work.

proto_library(
    name = "a_proto",
    srcs = ["a.proto"],
)

closure_proto_library(
    name = "a_closure_proto",
    deps = [":a_proto"],
)

proto_library(
    name = "b_proto",
    srcs = ["b.proto"],
    deps = [":a_proto"],
)

proto_library(
    name = "c_proto",
    srcs = ["c.proto"],
    deps = [":b_proto"],
)

closure_proto_library(
    name = "c_closure_proto",
    deps = [":c_proto"],
)

closure_js_library(
    name = "my_lib",
    srcs = [
        # Use everything from `a.proto` and `c.proto`, no need to add an explicit dependency on `b.proto`.
        # Using stuff from `b.proto` directly (e.g. `goog.require`'ing it) is possible if you suppress the indirect dependency warnings.
    ],
    deps = [":a_closure_proto", ":c_closure_proto"],
)

Does this answer your question?

@sgammon
Copy link
Contributor Author

sgammon commented Jun 7, 2019

yes, i think that does help. so it's just that we need to get that PR merged, so we can use that new rule with Soy (just clarifying)?

@Yannic
Copy link
Contributor

Yannic commented Jun 7, 2019

I think so, but that's @gkdn's decision.

@sgammon
Copy link
Contributor Author

sgammon commented Jun 7, 2019

@Yannic one more question, if you don't mind. our proto_library definitions include a strip_import_prefix value, which we need to be able to compile them correctly.

i would use closure_js_proto_library in the meantime, because it does work for what we need it to do, but when i try to invoke it, it complains about not being able to import dependencies (which is likely an issue with strip_import_prefix, which fixed that for us).

is there any equivalent to strip_import_prefix with closure_js_proto_library?

AFAICT, the new rule fixes this by using binary descriptors, rather than needing proto sources. am I right about that?

@Yannic
Copy link
Contributor

Yannic commented Jun 7, 2019

AFAICT, there is no equivalent to strip_import_prefix with closure_js_proto_library.

I'm not sure if strip_import_prefix works with closure_proto_library because it's a rather new attribute. I don't think we use binary descriptors yet, but maybe that's a good idea. Would you mind trying out whether this works, create a new issue if it doesn't and ask @gkdn to assign it to me? If that doesn't work, I'll also have to fix it for closure_grpc_library...

@sgammon
Copy link
Contributor Author

sgammon commented Jun 7, 2019

@Yannic actually, using closure_proto_library seems to work perfectly, when depending on proto_library declarations with strip_import_prefix.

i'm using it with soy now, building templates (that reference protos) that render correctly, using the factuno-db fork on the soy-fix branch, specifically 2736f1d.

@sgammon
Copy link
Contributor Author

sgammon commented Jun 7, 2019

@Yannic, pardon me, i spoke too soon. it was working perfectly because the underlying proto had no imports in it.

with a more complex example, i get the following output (i'll file an issue and list this there as well):

ERROR: /private/var/tmp/_bazel_sam/.../external/schema/vendor/google/bq/BUILD.bazel:16:1: Generating JavaScript Protocol Buffer bq failed (Exit 1): protoc failed: error executing command 
  (cd /private/var/tmp/_bazel_sam/.../execroot/SomeWorkspace && \
  exec env - \
  bazel-out/host/bin/external/com_google_protobuf/protoc -Ibazel-out/darwin-fastbuild/genfiles/external/schema -Ibazel-out/darwin-fastbuild/genfiles/external/com_google_protobuf '--js_out=import_style=closure,library=bq,add_require_for_enums:bazel-out/darwin-fastbuild/bin/external/schema/vendor/google/bq')
Execution platform: @bazel_tools//platforms:host_platform
Missing input file.
INFO: Elapsed time: 74.141s, Critical Path: 15.87s
INFO: 239 processes: 229 local, 10 worker.
FAILED: Build did NOT complete successfully

The key item there, of course, is Missing input file.. It's looking for a vendored proto file and can't find it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants