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.

Sign up for GitHub

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

Jump to bottom

rules_go + protobufs + experimental_sibling_repository_layout fails to build #3947

Open
ted-xie opened this issue May 22, 2024 · 3 comments · May be fixed by #3966
Open

rules_go + protobufs + experimental_sibling_repository_layout fails to build #3947

ted-xie opened this issue May 22, 2024 · 3 comments · May be fixed by #3966

Comments

@ted-xie
Copy link

ted-xie commented May 22, 2024

What version of rules_go are you using?

0.48. The issue also reproduces at commit 354a98f.

What version of gazelle are you using?

N/A

What version of Bazel are you using?

7.1.2

Does this issue reproduce with the latest releases of all the above?

Yes

What operating system and processor architecture are you using?

Debian-based Linux variant, x86_64 architecture.

Any other potentially useful information about your toolchain?

N/A

What did you do?

See repro at https://github.com/ted-xie/rules_go_proto_bug. TL;DR building a go_proto_library rule that's part of an external dependency with --experimental_sibling_repository_layout fails. Failure message:

Could not find file in descriptor database: ../maybe_rules_go_proto_bug~/proto/foo.proto: No such file or directory

What did you expect to see?

I expected the build to succeed.

What did you see instead?

The build fails due to the go proto compiler not being able to find the proto definition.

/cc @ahumesky
@fmeum
Copy link
Member

fmeum commented May 23, 2024

I looked into this and the root cause is that:

  • without the flag, ProtoInfo.proto_source_root is "external/maybe_rules_go_proto_bug~" and the path of the proto is "external/maybe_rules_go_proto_bug~/proto/foo.proto"
  • with the flag, ProtoInfo.proto_source_root is "" and the path of the proto is "../maybe_rules_go_proto_bug~/proto/foo.proto"

This looks like a bug in ProtoInfo, probably in this line
https://github.com/bazelbuild/bazel/blob/3a1b336ec8250547651f6c148f79ba1ced2448f2/src/main/starlark/builtins_bzl/common/proto/proto_info.bzl#L122

@ahumesky
Copy link

/cc @comius

ahumesky added a commit to ahumesky/rules_jvm_external that referenced this issue May 23, 2024
@ahumesky
Update rules_jvm_external to use the Starlark version of aar_import a…
dc493e4 
…utomatically

if the version of Bazel used does not contain the native version of aar_import.

This also updates the version of rules_android to the latest commit, because
version 0.1.1 is basically just wrappers around the native versions of the rules.

This also disbles `--experimental_sibling_repository_layout` added in
bazel-contrib@b6631f9
because the latest rules_android uses protos in go, and there is a problem compiling protos in go
with `--experimental_sibling_repository_layout`: bazel-contrib/rules_go#3947

Fixes bazel-contrib#1139.
@ahumesky ahumesky mentioned this issue May 24, 2024
Merged
@lberki lberki linked a pull request Jun 20, 2024 that will close this issue
Open
ahumesky added a commit to ahumesky/rules_jvm_external that referenced this issue Jul 9, 2024
@ahumesky
Update rules_jvm_external to use the Starlark version of aar_import a…
f7d1887 
…utomatically

if the version of Bazel used does not contain the native version of aar_import.

This also updates the version of rules_android to the latest commit, because
version 0.1.1 is basically just wrappers around the native versions of the rules.

This also disbles `--experimental_sibling_repository_layout` added in
bazel-contrib@b6631f9
because the latest rules_android uses protos in go, and there is a problem compiling protos in go
with `--experimental_sibling_repository_layout`: bazel-contrib/rules_go#3947

Fixes bazel-contrib#1139.
@arrdem
Copy link

arrdem commented Jul 10, 2024 •
edited
Loading

Just chiming in to say I got bitten by this today and that I wasn't able to find an obvious workaround on the rules_go side :/. It appears that the mentioning PR #3966 which seems to reorder the WORKSPACE to change which rules_proto wins and force an 6.0.0 upgrade isn't actually a fix. After applying the same upgrade I'm seeing the same behavior on Bazel 7.0.

I can't quite follow how this is supposed to be behaving but if folks can provide pointers I'd be happy to take a crack at fixing this since it has other knock-on consequences for us.

Specifically we encountered this in the buildozer build; sample sandbox error below.

ERROR: /private/var/tmp/_bazel_rmckenzie/91c8ca1d1d93ea92333230a8f066dbe4/external/com_github_bazelbuild_buildtools/api_proto/BUILD.bazel:18:17: Generating into bazel-out/com_github_bazelbuild_buildtools/darwin_arm64-fastbuild/bin/api_proto/api_proto_go_proto_/github.com/bazelbuild/buildtools/api_proto failed: (Exit 1): sandbox-exec failed: error executing GoProtocGen command 
  (cd /private/var/tmp/_bazel_rmckenzie/91c8ca1d1d93ea92333230a8f066dbe4/sandbox/darwin-sandbox/279/execroot/abnormalsecurity && \
  exec env - \
    CGO_ENABLED=1 \
    GOARCH=arm64 \
    GOEXPERIMENT=nocoverageredesign \
    GOOS=darwin \
    GOPATH='' \
    GOROOT=bazel-out/io_bazel_rules_go/darwin_arm64-fastbuild/bin/stdlib_ \
    GOROOT_FINAL=GOROOT \
    GOTOOLCHAIN=local \
    PATH=../local_config_cc:/usr/bin:/bin \
    TMPDIR=/var/folders/p9/q08xsds941xcsfj7rf311p8c0000gn/T/ \
    ZERO_AR_DATE=1 \
  /usr/bin/sandbox-exec -f /private/var/tmp/_bazel_rmckenzie/91c8ca1d1d93ea92333230a8f066dbe4/sandbox/darwin-sandbox/279/sandbox.sb /var/tmp/_bazel_rmckenzie/install/c15aaa505bff3c1d2abdefcf159a3893/process-wrapper '--timeout=0' '--kill_delay=15' '--stats=/private/var/tmp/_bazel_rmckenzie/91c8ca1d1d93ea92333230a8f066dbe4/sandbox/darwin-sandbox/279/stats.out' bazel-out/io_bazel_rules_go/darwin_arm64-opt-exec-ST-a828a81199fe/bin/go/tools/builders/go-protoc/go-protoc-bin -protoc bazel-out/io_bazel_rules_go/darwin_arm64-opt-exec-ST-a828a81199fe/bin/proto/protoc/protoc -importpath github.com/bazelbuild/buildtools/api_proto -out_path bazel-out/com_github_bazelbuild_buildtools/darwin_arm64-fastbuild/bin/api_proto/api_proto_go_proto_/ -plugin bazel-out/io_bazel_rules_go/darwin_arm64-opt-exec-ST-a828a81199fe/bin/proto/go_proto_reset_plugin_/protoc-gen-go -descriptor_set bazel-out/com_github_bazelbuild_buildtools/darwin_arm64-fastbuild/bin/api_proto/api_proto_proto-descriptor-set.proto.bin -expected bazel-out/com_github_bazelbuild_buildtools/darwin_arm64-fastbuild/bin/api_proto/api_proto_go_proto_/github.com/bazelbuild/buildtools/api_proto/api.pb.go -import '../com_github_bazelbuild_buildtools/api_proto/api.proto=github.com/bazelbuild/buildtools/api_proto' ../com_github_bazelbuild_buildtools/api_proto/api.proto)

jin pushed a commit to bazel-contrib/rules_jvm_external that referenced this issue Aug 7, 2024
@ahumesky
Update rules_jvm_external to use the Starlark version of aar_import a…
ba7310c 
…fter the native version was removed from Bazel (#1149)

* Update rules_jvm_external to use the Starlark version of aar_import automatically
if the version of Bazel used does not contain the native version of aar_import.

This also updates the version of rules_android to the latest commit, because
version 0.1.1 is basically just wrappers around the native versions of the rules.

This also disbles `--experimental_sibling_repository_layout` added in
b6631f9
because the latest rules_android uses protos in go, and there is a problem compiling protos in go
with `--experimental_sibling_repository_layout`: bazel-contrib/rules_go#3947

Fixes #1139.

* Restore .bazelrc

* Don't overwrite bazelrc

* Update rules_android commit to inclue bazelbuild/rules_android@d25741d

* Update comments

* Update to latest rules_android commit

* Load the rules_android rules before rules_kotlin to avoid the version of rules_android that rules_kotlin uses

* Update .bazelversion from 7.1.0 to 7.2.1

* Add dependencies for "aar_import_that_consumes_the_downloaded_file_directly" because Starlark aar_import checks that there are no missing class imports in aars

* Workarounds for aar_import ImportDepsChecker checks

* Remove unnecessary com.google.ar.sceneform:assets:1.10.0 dep and revert regression_testing_coursier_install.json

* Use the native Android rules in WORKSPACE via rules_android 0.1.1 for use with bazel 5 and bazel 6 in CI

* Update to latest rules_android

* Update to latest rules_android

* Update to rules_android 0.5.1, address review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Upgrade Stardoc to 0.6.2 . bazel-contrib/rules_go
4 participants
@arrdem @fmeum @ted-xie @ahumesky