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

Cannot generate swagger with generated protobuf #1094

Closed
prestonvanloon opened this issue Dec 8, 2019 · 4 comments · Fixed by #1095
Closed

Cannot generate swagger with generated protobuf #1094

prestonvanloon opened this issue Dec 8, 2019 · 4 comments · Fixed by #1095

Comments

@prestonvanloon
Copy link
Contributor

prestonvanloon commented Dec 8, 2019

Steps you follow to reproduce the error:

  • Clone example: git clone git@github.com:prestonvanloon/grpc-gateway.git
  • Checkout branch git checkout genrule-failure
  • Build bazel build //examples/proto/examplepb:examplepb_protoc_gen_swagger
INFO: Invocation ID: 0e6eb834-56c4-4d11-85ea-31833501c464
INFO: Analyzed target //examples/proto/examplepb:examplepb_protoc_gen_swagger (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
ERROR: /tmp/grpc-gateway/examples/proto/examplepb/BUILD.bazel:86:1: Action examples/proto/examplepb/_virtual_imports/examplepb_proto/examples/proto/examplepb/generated_output.swagger.json failed (Exit 1)
examples/proto/examplepb/a_bit_of_everything.proto: File not found.
examples/proto/examplepb/_virtual_imports/examplepb_proto/examples/proto/examplepb/generated_output.proto:7:1: Import "examples/proto/examplepb/a_bit_of_everything.proto" was not found or had errors.
examples/proto/examplepb/_virtual_imports/examplepb_proto/examples/proto/examplepb/generated_output.proto:14:20: "ABitOfEverything" is not defined.
examples/proto/examplepb/_virtual_imports/examplepb_proto/examples/proto/examplepb/generated_output.proto:8:1: warning: Import examples/proto/sub/message.proto but not used.
Target //examples/proto/examplepb:examplepb_protoc_gen_swagger failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 0.834s, Critical Path: 0.29s
INFO: 0 processes.
FAILED: Build did NOT complete successfully

What did you expect to happen instead:

It works

What's your theory on why it isn't working:

I think something is wrong with the includes when using a generated file. Haven't started to investigate a fix yet.

@prestonvanloon
Copy link
Contributor Author

This feels like it has something to do with protobuf virtual imports. I think its fine for bazel clients before v1.

I added a quick hack and was able to get the json files to build, but in the wrong place.

diff --git a/protoc-gen-swagger/defs.bzl b/protoc-gen-swagger/defs.bzl
index 652e4a2..08123ae 100644
--- a/protoc-gen-swagger/defs.bzl
+++ b/protoc-gen-swagger/defs.bzl
@@ -24,6 +24,10 @@ def _collect_includes(gen_dir, srcs):
             workspace_root = src.owner.workspace_root
             ref_path = ref_path[len(workspace_root):].lstrip("/")

+        if ref_path.startswith("examples/proto/examplepb/_virtual_imports/examplepb_proto/"):
+            ref_path = ref_path[len("examples/proto/examplepb/_virtual_imports/examplepb_proto/"):]
+
         include = ref_path + "=" + src.path
         if include not in includes:
             includes.append(include)

This produces the following error:

ERROR: /tmp/grpc-gateway/examples/proto/examplepb/BUILD.bazel:86:1: output 'examples/proto/examplepb/_virtual_imports/examplepb_proto/examples/proto/examplepb/a_bit_of_everything.swagger.json' was not created
ERROR: /tmp/grpc-gateway/examples/proto/examplepb/BUILD.bazel:86:1: not all outputs were created or valid

This file was created under bazel-out/k8-fastbuild/bin as expected from the code, but not expected from bazel / starlark for some reason in the virtual imports subdirectory.

@prestonvanloon
Copy link
Contributor Author

Another update, I was able to hack this target together with the following patch.

diff --git a/protoc-gen-swagger/defs.bzl b/protoc-gen-swagger/defs.bzl
index 652e4a2..59caf90 100644
--- a/protoc-gen-swagger/defs.bzl
+++ b/protoc-gen-swagger/defs.bzl
@@ -24,6 +24,9 @@ def _collect_includes(gen_dir, srcs):
             workspace_root = src.owner.workspace_root
             ref_path = ref_path[len(workspace_root):].lstrip("/")

+        if ref_path.startswith("examples/proto/examplepb/_virtual_imports/examplepb_proto/"):
+            ref_path = ref_path[len("examples/proto/examplepb/_virtual_imports/examplepb_proto/"):]
+
         include = ref_path + "=" + src.path
         if include not in includes:
             includes.append(include)
@@ -87,6 +90,7 @@ def _run_proto_gen_swagger(ctx, direct_proto_srcs, transitive_proto_srcs, action
             output_dir = ctx.bin_dir.path
             if proto.owner.workspace_root:
                 output_dir = "/".join([output_dir, proto.owner.workspace_root])
+            output_dir =  output_dir + "/examples/proto/examplepb/_virtual_imports/examplepb_proto/"

             args = actions.args()
             args.add("--plugin=protoc-gen-swagger=%s" % protoc_gen_swagger.path)

This, of course, is not a sustainable solution because it will not work for other targets. However, it does outline a resolution path for virtual imports.

@achew22
Copy link
Collaborator

achew22 commented Dec 8, 2019

Ultimately I believe that rules_proto is going to be our savior here. It's kind of silly that we are writing this at all when we should be able to just create a toolchain and plug into their ecosystem. Something like proto_lang_toolchain.

Due to that, I'm happy to accept hacks if you think that we can get a few months of use out of the hack.

@Yannic
Copy link
Contributor

Yannic commented Dec 9, 2019

There were some sharp edges in proto rules that got fixed in Bazel 1.0 (e.g., generated files may have been not under ProtoLibrary#proto_source_root). We'll provide guidance for Protobuf rules and rules_proto in the next couple of months :).

I've seen the issue above before (it's actually pretty common because the path of proto files changed recently; IIRC, gRPC also had it), so let me compile together a patch.

Yannic added a commit to Yannic/grpc-gateway that referenced this issue Dec 9, 2019
This can happen if:
  1. `proto_library` contains at least one of `import_prefix/strip_import_prefix`, or
  2. (for Bazel >= 1.0) at least one of `proto_library`'s `srcs` is generated.

Fixes grpc-ecosystem#1094
Yannic added a commit to Yannic/grpc-gateway that referenced this issue Dec 9, 2019
This can happen if:
  1. `proto_library` contains at least one of `import_prefix/strip_import_prefix`, or
  2. (for Bazel >= 1.0) at least one of `proto_library`'s `srcs` is generated.

Fixes grpc-ecosystem#1094
Yannic added a commit to Yannic/grpc-gateway that referenced this issue Dec 9, 2019
This can happen if:
  1. `proto_library` contains at least one of `import_prefix/strip_import_prefix`, or
  2. (for Bazel >= 1.0) at least one of `proto_library`'s `srcs` is generated.

Fixes grpc-ecosystem#1094
Yannic added a commit to Yannic/grpc-gateway that referenced this issue Dec 9, 2019
This can happen if:
  1. `proto_library` contains at least one of `import_prefix/strip_import_prefix`, or
  2. (for Bazel >= 1.0) at least one of `proto_library`'s `srcs` is generated.

Fixes grpc-ecosystem#1094
Yannic added a commit to Yannic/grpc-gateway that referenced this issue Dec 9, 2019
This can happen if:
  1. `proto_library` contains at least one of `import_prefix/strip_import_prefix`, or
  2. (for Bazel >= 1.0) at least one of `proto_library`'s `srcs` is generated.

Fixes grpc-ecosystem#1094
achew22 pushed a commit that referenced this issue Dec 16, 2019
This can happen if:
  1. `proto_library` contains at least one of `import_prefix/strip_import_prefix`, or
  2. (for Bazel >= 1.0) at least one of `proto_library`'s `srcs` is generated.

Fixes #1094
adasari pushed a commit to adasari/grpc-gateway that referenced this issue Apr 9, 2020
This can happen if:
  1. `proto_library` contains at least one of `import_prefix/strip_import_prefix`, or
  2. (for Bazel >= 1.0) at least one of `proto_library`'s `srcs` is generated.

Fixes grpc-ecosystem#1094
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

Successfully merging a pull request may close this issue.

3 participants