bufbuild / protoc-gen-validate

Protocol Buffer Validation - Being replaced by github.com/bufbuild/protovalidate
https://github.com/bufbuild/protovalidate
Apache License 2.0
3.79k stars 582 forks source link

Bazel 1.0+ failures due to virtual source root (/_virtual_imports/) #308

Closed gertvdijk closed 4 years ago

gertvdijk commented 4 years ago

(This is a quick report, I can make a minimal example some time later.)

Has anybody tried to use the combination of:

I'm having issues with (at least) the build of the java_proto_gen_validate srcjars.

[...]
ERROR: /path/to/BUILD.bazel:13:1: Generating bazel-out/k8-fastbuild/bin/path/to/my_proto-validate-gensrc.jar failed (Exit 1)
google/protobuf/descriptor.proto: File not found.
google/protobuf/duration.proto: File not found.
google/protobuf/timestamp.proto: File not found.
validate/validate.proto:7:1: Import "google/protobuf/descriptor.proto" was not found or had errors.
validate/validate.proto:8:1: Import "google/protobuf/duration.proto" was not found or had errors.
validate/validate.proto:9:1: Import "google/protobuf/timestamp.proto" was not found or had errors.
[...]

with sandbox debugging I see protoc invocations that explain this error:

(cd /path/to/execroot/workspacename && \
  exec env - \
  bazel-out/host/bin/external/com_google_protobuf/protoc '--plugin=protoc-gen-validate=bazel-out/host/bin/external/com_envoyproxy_protoc_gen_validate/linux_amd64_stripped/protoc-gen-validate' '--validate_out=lang=java:bazel-out/k8-fastbuild/bin/external/com_envoyproxy_protoc_gen_validate/validate/validate_proto-validate-gensrc.jar' '--proto_path=validate/validate.proto=external/com_envoyproxy_protoc_gen_validate/validate/validate.proto' '--proto_path=_virtual_imports/descriptor_proto/google/protobuf/descriptor.proto=bazel-out/k8-fastbuild/bin/external/com_google_protobuf/_virtual_imports/descriptor_proto/google/protobuf/descriptor.proto' '--proto_path=_virtual_imports/duration_proto/google/protobuf/duration.proto=bazel-out/k8-fastbuild/bin/external/com_google_protobuf/_virtual_imports/duration_proto/google/protobuf/duration.proto' '--proto_path=_virtual_imports/timestamp_proto/google/protobuf/timestamp.proto=bazel-out/k8-fastbuild/bin/external/com_google_protobuf/_virtual_imports/timestamp_proto/google/protobuf/timestamp.proto' validate/validate.proto)

Having a look at the argument

--proto_path=_virtual_imports/timestamp_proto/google/protobuf/timestamp.proto=bazel-out/k8-fastbuild/bin/external/com_google_protobuf/_virtual_imports/timestamp_proto/google/protobuf/timestamp.proto

is not helping to make google/protobuf/timestamp.proto importable...

I believe https://github.com/bazelbuild/bazel/issues/9215 is the cause for this and the Bazel rules require updates, similar to adjustments made in e.g. https://github.com/bazelbuild/rules_swift/pull/298.

gertvdijk commented 4 years ago

This quick-'n-dirty patch allowed me to build at least the jars. Doesn't look like the most stable change, but I'll just leave it here for reference.

--- bazel/protobuf.bzl
+++ bazel/protobuf.bzl
@@ -5,6 +5,9 @@ def _proto_path(proto):
     The proto path is not really a file path
     It's the path to the proto that was seen when the descriptor file was generated.
     """
+    if is_in_virtual_imports(proto):
+        return _strip_virtual_import(proto.path)
+
     path = proto.path
     root = proto.root.path
     ws = proto.owner.workspace_root
@@ -18,6 +21,11 @@ def _proto_path(proto):
         path = path[1:]
     return path

+def _strip_virtual_import(path):
+    pos = path.find(_VIRTUAL_IMPORTS)
+    path = path[pos + len(_VIRTUAL_IMPORTS):]
+    return path.split("/", 1)[-1]
+
 def _protoc_cc_output_files(proto_file_sources):
     cc_hdrs = []
     cc_srcs = []
@@ -298,3 +306,21 @@ python_proto_gen_validate = rule(
     output_to_genfiles = True,
     implementation = _protoc_gen_validate_python_impl,
 )
+
+# From https://github.com/grpc/grpc/blob/2e7d6b94eaf6b0e11add27606b4fe3d0b7216154/bazel/protobuf.bzl:
+
+_VIRTUAL_IMPORTS = "/_virtual_imports/"
+
+def is_in_virtual_imports(source_file, virtual_folder = _VIRTUAL_IMPORTS):
+    """Determines if source_file is virtual (is placed in _virtual_imports
+    subdirectory). The output of all proto_library targets which use
+    import_prefix  and/or strip_import_prefix arguments is placed under
+    _virtual_imports directory.
+    Args:
+        source_file: A proto file.
+        virtual_folder: The virtual folder name (is set to "_virtual_imports"
+            by default)
+    Returns:
+        True if source_file is located under _virtual_imports, False otherwise.
+    """
+    return not source_file.is_source and virtual_folder in source_file.path
gattytto commented 4 years ago

I'm hit by this using the following:

WORKSPACE:

##############################################################################
# gRPC-web
##############################################################################

http_archive(
    name = "io_bazel_rules_closure",
    strip_prefix = "rules_closure-master",
    urls = [
        "https://github.com/bazelbuild/rules_closure/archive/master.zip",
    ],
)

load("@io_bazel_rules_closure//closure:repositories.bzl", "rules_closure_dependencies", "rules_closure_toolchains")

rules_closure_dependencies()

rules_closure_toolchains()

http_archive(
    name = "com_github_grpc_grpc_web",
    strip_prefix = "grpc-web-1.0.7", # 1.0.7
    urls = ["https://github.com/grpc/grpc-web/archive/1.0.7.zip"],
)

#@unused
load("@com_github_grpc_grpc_web//bazel:closure_grpc_web_library.bzl", "closure_grpc_web_library")

BUILD.bzl

##############################################################################
# JS gRPC-web
##############################################################################
load("@com_github_grpc_grpc_web//bazel:closure_grpc_web_library.bzl", "closure_grpc_web_library")

closure_grpc_web_library(
    name = "product_web_lib",
    deps = [
        ":product_proto",
    ]
)
INFO: Found 1 target...
ERROR: /projects/gopos/gopos/product/v1alpha1/BUILD.bazel:171:1: Generating GRPC Web product.grpc.js 
failed (Exit 1) process-wrapper failed: error executing command 
  (cd /home/theia/.cache/bazel/_bazel_user/89380f62e48dd36cf68d64cb2e93a9c4/sandbox/processwrapper-sa
ndbox/181/execroot/com_gopos_crudapis && \
  exec env - \
    TMPDIR=/tmp \
  /home/theia/.cache/bazel/_bazel_user/install/9085cf605a813a3c26c5757f2ad47de3/process-wrapper '--ti
meout=0' '--kill_delay=15' bazel-out/host/bin/external/com_google_protobuf/protoc -I. -I. -Iexternal/com_google_googleapis -Iexternal/com_google_googleapis -Ibazel-out/k8-fastbuild/bin/external/com_google_protobuf '--plugin=protoc-gen-grpc-web=bazel-out/host/bin/external/com_github_grpc_grpc_web/javascript/net/grpc/web/protoc-gen-grpc-web' '--grpc-web_out=import_style=closure,mode=grpcwebtext,out=product.grpc.js:bazel-out/k8-fastbuild/bin/gopos/product/v1alpha1' gopos/product/v1alpha1/product.proto)
google/protobuf/descriptor.proto: File not found.
google/api/annotations.proto:20:1: Import "google/protobuf/descriptor.proto" was not found or had errors.
google/api/annotations.proto:28:8: "google.protobuf.MethodOptions" is not defined.
gopos/product/v1alpha1/product.proto:5:1: Import "google/api/annotations.proto" was not found or had errors.
Target //gopos/product/v1alpha1:product_web_lib failed to build
INFO: Elapsed time: 486.543s, Critical Path: 0.58s
INFO: 180 processes: 180 remote cache hit.
FAILED: Build did NOT complete successfully
sh-5.0$ find ~/.cache/bazel |grep "protobuf/descriptor\.proto"                /home/theia/.cache/bazel/_bazel_user/89380f62e48dd36cf68d64cb2e93a9c4/execroot/com_gopos_crudapis/bazel-out/k8-fastbuild/bin/external/com_google_protobuf/_virtual_imports/descriptor_proto/google/protobuf/descriptor.proto
/home/theia/.cache/bazel/_bazel_user/89380f62e48dd36cf68d64cb2e93a9c4/external/com_google_protobuf/java/compatibility_tests/v2.5.0/more_protos/src/proto/google/protobuf/descriptor.proto
/home/theia/.cache/bazel/_bazel_user/89380f62e48dd36cf68d64cb2e93a9c4/external/com_google_protobuf/java/compatibility_tests/v2.5.0/protos/src/proto/google/protobuf/descriptor.proto
/home/theia/.cache/bazel/_bazel_user/89380f62e48dd36cf68d64cb2e93a9c4/external/com_google_protobuf/python/compatibility_tests/v2.5.0/protos/src/proto/google/protobuf/descriptor.proto
/home/theia/.cache/bazel/_bazel_user/89380f62e48dd36cf68d64cb2e93a9c4/external/com_google_protobuf/src/google/protobuf/descriptor.proto
/home/theia/.cache/bazel/_bazel_user/89380f62e48dd36cf68d64cb2e93a9c4/sandbox/processwrapper-sandbox/181/execroot/com_gopos_crudapis/bazel-out/k8-fastbuild/bin/external/com_google_protobuf/_virtual_imports/descriptor_proto/google/protobuf/descriptor.proto
gertvdijk commented 4 years ago

@gattytto Your example does not include anything from this project (protoc-gen-validate). What makes you post it here? More Bazel projects using Protobuf have issues. In this case maybe check with com_github_grpc_grpc_web.

gattytto commented 4 years ago

@gertvdijk you are right I'm sorry for the offtopic, this seems to be a bazel specific issue

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

gertvdijk commented 4 years ago

This appears to be solved in another PR which is merged already. :tada:

https://github.com/envoyproxy/protoc-gen-validate/commit/0f2bc6c0fdac9113e3863ea6e30e5b2bd33e3b40 (#327)

(I did not validate it yet, but it looks like it fixes this in the similar way as my dirty hack posted above.)

Kernald commented 4 years ago

I still encounter it when using the v0.4.0 tag which seems to include #327. This is what I have:

ERROR: /private/var/tmp/_bazel_kernald/dc8fa02be43b6315d36f6bf2bf056429/external/com_google_googleapis/google/api/BUILD.bazel:171:14: Generating bazel-out/darwin-fastbuild/bin/external/com_google_googleapis/google/api/resource_proto-validate-gensrc.jar failed (Exit 1) sandbox-exec failed: error executing command
  (cd /private/var/tmp/_bazel_kernald/dc8fa02be43b6315d36f6bf2bf056429/sandbox/darwin-sandbox/5547/execroot/fr_enoent && \
  exec env - \
    TMPDIR=/var/folders/n5/1sh28p253wx09wnym0w45dlw0000gn/T/ \
  /usr/bin/sandbox-exec -f /private/var/tmp/_bazel_kernald/dc8fa02be43b6315d36f6bf2bf056429/sandbox/darwin-sandbox/5547/sandbox.sb /var/tmp/_bazel_kernald/install/e74efe234cbfbe8b17b6baeb8c33e7ff/process-wrapper '--timeout=0' '--kill_delay=15' bazel-out/host/bin/external/com_google_protobuf/protoc '--plugin=protoc-gen-validate=bazel-out/host/bin/external/com_envoyproxy_protoc_gen_validate/protoc-gen-validate_/protoc-gen-validate' '--validate_out=lang=java:bazel-out/darwin-fastbuild/bin/external/com_google_googleapis/google/api/resource_proto-validate-gensrc.jar' '--proto_path=google/api/resource.proto=external/com_google_googleapis/google/api/resource.proto' '--proto_path=_virtual_imports/descriptor_proto/google/protobuf/descriptor.proto=bazel-out/darwin-fastbuild/bin/external/com_google_protobuf/_virtual_imports/descriptor_proto/google/protobuf/descriptor.proto' google/api/resource.proto)
google/protobuf/descriptor.proto: File not found.
google/api/resource.proto:20:1: Import "google/protobuf/descriptor.proto" was not found or had errors.

Note this path: --proto_path=_virtual_imports/descriptor_proto/google/protobuf/descriptor.proto=bazel-out/darwin-fastbuild/bin/external/com_google_protobuf/_virtual_imports/descriptor_proto/google/protobuf/descriptor.proto

It starts with _virtual_imports/, no leading slash. Whereas the logic introduced in #327 checks with a leading slash.

gertvdijk commented 4 years ago

@Kernald I still haven't validated a newer release with this this PR yet, but what I can say is that the check including a leading slash works for me/us (check my local patch). I'm wondering what leads up to your situation. Do you have proto files in the root of your project or something? :confused:

Kernald commented 4 years ago

@gertvdijk I haven't. The structure I have is a proto target in //a/package/of/mine, with a dependency on a third party proto library (Google APIs, in the error log above, google/api/resource.proto), fetched through a http_archive, which in turn has a dependency on google/protobuf/descriptor.proto. (I'm on my phone right now so have a hard time finding the exact packages and whatnot, but the Google APIs library I'm using exposes a BUILD file with the proper dependency etc, what I have is pretty straightforward).