aspect-build / rules_ts

Bazel rules for the `tsc` compiler from http://typescriptlang.org
https://docs.aspect.build/rules/aspect_rules_ts
Apache License 2.0
108 stars 64 forks source link

[Bug]: ts_proto_library descriptor database error in nested workspace #451

Open alexeagle opened 1 year ago

alexeagle commented 1 year ago

What happened?

repro: https://github.com/binoche9/repros/tree/ts-proto-library-nested-workspace

bazel build @inner//:settings_ts --override_repository=aspect_rules_ts=$HOME/Projects/rules_ts

DEBUG: (this is the args to protoc)  /shared/cache/bazel/user_base/6756fcacb63b1eda5fcd3118a702b979/external/aspect_rules_ts/ts/private/ts_proto_library.bzl:38:10: 
  --plugin=protoc-gen-es=bazel-out/k8-opt-exec-2B5CBBC6/bin/external/inner/_settings_ts.gen_es.sh |
  --es_opt=keep_empty_files=true --es_opt=target=js+dts 
  --es_out=bazel-out/k8-fastbuild/bin 
  --descriptor_set_in bazel-out/k8-fastbuild/bin/external/inner/settings_proto-descriptor-set.proto.bin 
  external/inner/settings.proto

INFO: Analyzed target @inner//:settings_ts (1 packages loaded, 24 targets configured).
INFO: Found 1 target...
ERROR: /shared/cache/bazel/user_base/6756fcacb63b1eda5fcd3118a702b979/external/inner/BUILD:11:17: Generating .js/.d.ts from @inner//:settings_ts failed: (Exit 1): protoc failed: error executing command (from target @inner//:settings_ts) bazel-out/k8-opt-exec-2B5CBBC6/bin/external/com_google_protobuf/protoc '--plugin=protoc-gen-es=bazel-out/k8-opt-exec-2B5CBBC6/bin/external/inner/_settings_ts.gen_es.sh' '--es_opt=keep_empty_files=true' ... (remaining 5 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
Could not find file in descriptor database: external/inner/settings.proto: No such file or directory
Target @inner//:settings_ts failed to build

Reported by @binoche9 and I think @pcj as well.

Version

HEAD

How to reproduce

No response

Any other information?

No response

alexeagle commented 1 year ago

I think this is a protoc bug, or maybe a bug in rules_proto, in the sense that the paths and arguments we pass to protoc seem correct.

The content of the descriptor set is something like (ignore binary goop)

�
settings.protocom.settings"
RequestP
id (Rid"1
    ResponseP$
primary_key_id (    RprimaryKeyId2B
Settings6
Set.com.settings.RequestP.com.settings.ResponsePbproto3

so it looks like protoc expects the name "settings.proto" without the full path it was seen at. Indeed, if I hack the rules to strip the external/inner/ prefix from the final argument to protoc, it gets past this error, and reaches the next one where the output isn't created in the expected location. That error in turn would have to be repaired inside proto_common.declare_generated_files

@thesayyn do you agree it's a bug in bazelbuild/rules_proto?

eaplatanios commented 1 year ago

@alexeagle is there a workaround for this currently? I only hit this issue with TS and not with Python for the same protos so I'm not sure if it's a protoc bug.

eaplatanios commented 1 year ago

Also, I realize that in my case there is no nested workspace but I get the same error. My setup is a single workspace file at the root and then in a directory in the BUILD file I have this:

proto_library(
    name = "proto-library",
    srcs = glob(["proto/**/*.proto"]),
    import_prefix = "sc",
    visibility = ["//visibility:public"],
    deps = ["@com_google_protobuf//:any_proto"],
)

ts_proto_library(
    name = "ts-proto-library",
    node_modules = "//:node_modules",
    proto = ":proto-library",
    visibility = ["//visibility:public"],
)

which seems like a pretty standard setup. I get this error:

Could not find file in descriptor database: bazel-out/darwin_arm64-opt/bin/util/_virtual_imports/proto-library/sc/util/proto/v1/util.proto: No such file or directory

but this file does exist under bazel-out in the workspace. This makes me think something is off with the directory from where the command gets executed.

cameron-martin commented 10 months ago

Just ran into this. I can confirm that the path to the proto does exist, but the proto descriptor contains a truncated path, as @alexeagle says. What I'm wondering is, how do any of the other proto rules work?

cameron-martin commented 10 months ago

One thing that's different about almost all other proto library rules, is they use proto_common.compile. I haven't looked into it enough yet, but I wonder if that would fix these issues.

cameron-martin commented 10 months ago

I think I've worked out what's going on here. Firstly, setting -I to external/inner, as proto_common.proto_compile does gets us past the first error. However, it then fails to generate the outputs in the correct place. To fix this, the --es_out and --connect-es_out attributes must include a slightly modified version of proto_info.proto_source_root, as e.g. the python rules do.

I think if we use proto_common.compile, this commit suggests that we don't need to do the modifications of proto_source_root, as the python rules do.