rules_go icon indicating copy to clipboard operation
rules_go copied to clipboard

Can't build a project that depends on gocloud.dev v0.24.0 and some other packages

Open mishas opened this issue 2 years ago • 18 comments

What version of rules_go are you using?

Tried with both v0.28.0 and v0.29.0

What version of gazelle are you using?

Tried with both v0.23.0 and Master

What version of Bazel are you using?

v4.2.1

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

Yes

What operating system and processor architecture are you using?

Tried both Mac and Linux (amd64 on both)

Any other potentially useful information about your toolchain?

Nothing special.

What did you do?

  1. Create an empty project with a Go file depending on both gocloud.dev v0.24.0 and firebase.google.com/go/v4 v4.6.0 (it happens with gocloud.dev v0.24.0 and other packages as well. firebase is just a simple example).
  2. Run go mod tidy and then bazel run //:gazelle -- update-repos -to_macro=repositories.bzl%go_repositories -from_file=./go.mod (I also tried running with -build_file_proto_mode=disable, same problem)
  3. Try to build

(Example can be found here: https://github.com/mishas/bazel-cross-compile-rebuild/tree/example/gocloud.dev_v0.24.0) (Run output can be found here: https://github.com/mishas/bazel-cross-compile-rebuild/runs/3830586153?check_suite_focus=true)

What did you expect to see?

Successful build

What did you see instead?

INFO: Analyzed target //pkg/demo:demo (210 packages loaded, 2136 targets configured).
INFO: Found 1 target...
ERROR: /.../pkg/demo/BUILD.bazel:14:10: GoLink pkg/demo/demo_/demo failed: (Exit 1): builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder '-param=bazel-out/darwin-fastbuild/bin/pkg/demo/demo_/demo-0.params' -- -extld external/local_config_cc/cc_wrapper.sh '-buildid=redacted' -extldflags ... (remaining 1 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
link: package conflict error: google.golang.org/genproto/googleapis/api/annotations: multiple copies of package passed to linker:
	@org_golang_google_genproto//googleapis/api/annotations:annotations
	@go_googleapis//google/api:annotations_go_proto
Set "importmap" to different paths or use 'bazel cquery' to ensure only one
package with this path is linked.
Target //pkg/demo:demo failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 8.852s, Critical Path: 0.14s
INFO: 2 processes: 2 internal.
FAILED: Build did NOT complete successfully

Additional information

  1. Everything works just fine with gocloud.dev v0.23.0, but I believe it's because v0.24.0 added a bunch of new dependencies...
  2. I've added build_file_proto_mode = "disable_global" to a bunch of go_repository targets, and made it work (specifically to com_github_googleapis_gax_go_v2, com_google_cloud_go, com_google_cloud_go_firestore, com_google_cloud_go_storage, org_golang_google_grpc). But in my real code it wasn't enough: I have .proto files that has import "google/rpc/status.proto", which still created the problem :(.

mishas avatar Oct 07 '21 18:10 mishas

@robfig , @jayconrod, Friendly ping on that? (It's somewhat of a blocker on our side :( )...

BTW, I also tried it with the latest Gazelle (v0.24.0).

mishas avatar Oct 11 '21 20:10 mishas

Avoiding Conflicts has some info on resolving conflicts like this.

If you want to use pre-generated .pb.go files and not generate them at build time, use build_file_proto_mode = "disable_global" on all go_repository rules containing protos, and use # gazelle:proto disable_global in your own repo. Use bazel cquery 'somepath(//pkg/demo, @go_googleapis//google/api:annotations_go_proto) (for example) to find paths to unwanted dependencies.

jayconrod avatar Oct 11 '21 21:10 jayconrod

Hello @jayconrod , thank you for your quick reply!

Regarding using disable_global, please note that I stated it in number (2) of "Additional information" above:

I've added build_file_proto_mode = "disable_global" to a bunch of go_repository targets, and made it work (specifically to com_github_googleapis_gax_go_v2, com_google_cloud_go, com_google_cloud_go_firestore, com_google_cloud_go_storage, org_golang_google_grpc). But in my real code it wasn't enough: I have .proto files that have import "google/rpc/status.proto", which still created the problem :(.

I actually also tried adding # gazelle:proto disable_global in my //BUILD.bazel file, but that stops generating any go_library build rules for my protos, which is something I do want to have.

BTW, I can manually change the deps = to have the @org_golang_google_genproto// dependency rather than the @go_googleapis// one for my own PBs (and add a # keep marker on them), but that also means I can't rely on Gazelle to make any further changes to those PBs if anything changes.

Am I missing something?

Thank you again.

mishas avatar Oct 11 '21 21:10 mishas

I actually also tried adding # gazelle:proto disable_global in my //BUILD.bazel file, but that stops generating any go_library build rules for my protos, which is something I do want to have.

Do you mean go_library or go_proto_library here? # gazelle:proto disable_global should stop Gazelle from generating go_proto_library targets and go_library targets that embed them. However, it should still generate go_library targets that include .pb.go files. If you're using this strategy, you must generate and check in .pb.go files separately from Bazel.

If you want to use go_proto_library though, don't use build_file_proto_mode = "disable_global". You may need to make some patches or other modifications to build files, and it may help if everything is vendored. Ideally go_proto_library should be easier, but because there are so many customized ways of organizing .proto files, it's very difficult in practice for Gazelle to generate correct build files across repositories.

jayconrod avatar Oct 11 '21 22:10 jayconrod

@jayconrod,

Do you mean go_library or go_proto_library here?

I mean both the go_proto_library, and the go_library embedding it, just as you noted that.

My use case is somewhat simple: I have Go code, and a bunch .proto files, and I want to build everything with Bazel. I'd rather not pre-build (and check in) .pb.go files separately from Bazel, as those are still source files that needs to be built.

I've updated the example https://github.com/mishas/bazel-cross-compile-rebuild/tree/example/gocloud.dev_v0.24.0 to make it closer to my actual code:

  1. Ran gazelle with -build_file_proto_mode=disable_global -- This makes it build if my proto file (below) is not part of the build tree,
  2. Added a .proto file importing google/rpc/status.proto -- this now fails after (1).

As you can see here: https://github.com/mishas/bazel-cross-compile-rebuild/runs/3864702501?check_suite_focus=true, the build still fails with:

ERROR: /home/runner/work/bazel-cross-compile-rebuild/bazel-cross-compile-rebuild/pkg/demo/BUILD.bazel:15:10: GoLink pkg/demo/demo_/demo failed: (Exit 1): builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder '-param=bazel-out/k8-fastbuild/bin/pkg/demo/demo_/demo-0.params' -- -extld /usr/bin/gcc '-buildid=redacted' -extldflags ... (remaining 1 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
link: package conflict error: google.golang.org/genproto/googleapis/rpc/status: multiple copies of package passed to linker:
	@go_googleapis//google/rpc:status_go_proto
	@org_golang_google_genproto//googleapis/rpc/status:status
Set "importmap" to different paths or use 'bazel cquery' to ensure only one

I understand that it's difficult for Gazelle to generate correct builds for every different customized way of organizing .proto files, but I feel like the example above is pretty "standard".

Another thing that I don't understand is - why did it work with gocloud.dev v0.23.0, but doesn't work with v0.24.0? What have changed? I looked at the diff of the go.sum file, and I couldn't find a specific culprit.

Bottom line is:

  1. I think using gocloud.dev in a project is standard enough for it to work without any custom patches.
  2. If you disagree on (1), it might make sense to have some kind of #gazelle:proto use_org_golang_google_genproto (or something), that generates go_proto_library depending on @org_golang_google_genproto rather than @go_googleapis.
  3. Maybe make (2) default if org_golang_google_genproto is present in go_repositories

Also, is it possible that solving #1986 will make this better? if yes, maybe it makes sense to prioritize it?

mishas avatar Oct 12 '21 01:10 mishas

BTW, I can manually change the deps = to have the @org_golang_google_genproto// dependency rather than the @go_googleapis// one for my own PBs (and add a # keep marker on them), but that also means I can't rely on Gazelle to make any further changes to those PBs if anything changes.

You could use a set of # gazelle:resolve directives to override the dependency resolution for the packages you need, which are probably not too many. It's not ideal, I know.

I'm supportive of having things work out of the box, but gocloud.dev does have a ton of dependencies, so it's not surprising that it pulls in something that doesn't "just work". We don't have much development bandwidth, but in the ideal scenario there could be a set of overrides or patches for popular libraries that allow the community to share the tweaks to make them "just work" with gazelle. They would need some way to target specific versions of the dependency though.

It does seem like this particular error would be resolved by solving #1986. Maybe that would do the trick. Nice find.

robfig avatar Oct 15 '21 15:10 robfig

@robfig, I tried the # gazelle:resolve idea, as, indeed, it seems nicer than using # keep, but I wasn't able to make it work:

  • I tried # gazelle:resolve go google.golang.org/genproto/googleapis/rpc/status @org_golang_google_genproto//googleapis/rpc/status, but that didn't affect the build files at all
  • I tried # gazelle:resolve proto google/rpc/status.proto @org_golang_google_genproto//googleapis/rpc/status, but that affected both go_proto_library (which is good) as well as proto_library (which is bad), and led to an error: in deps attribute of proto_library rule //pkg/pb:pb_proto: go_library rule '@org_golang_google_genproto//googleapis/rpc/status:status' is misplaced here (expected proto_library).

Am I missing something?

Regarding #1986 , is there any progress there? I know @jayconrod said "I'd rather not take PRs from anyone else", but - can I help with it in any way? (Not sure I have the skill though...)

Thank you again for all the help!

mishas avatar Oct 15 '21 15:10 mishas

No, that's what I would have tried. I don't personally use go_proto_library, so it sounds like I was mistaken that resolve would solve this, sorry! It does sound like completing #1986 would be the ideal solution..

robfig avatar Oct 17 '21 13:10 robfig

I just experienced a similar problem. We do not use gocloud.dev, but we do use cloud.google.com/go/storage (one of the dependencies of gocloud.dev). I spent nearly the whole day trying to find a configuration that works.

I ran into the error mentioned by this issue as well as:

external/com_google_cloud_go_storage/storage.go:1416:53: o.GetCustomerEncryption().GetKeySha256 undefined (type *"google.golang.org/genproto/googleapis/storage/v2".Object_CustomerEncryption has no field or method GetKeySha256)
external/com_google_cloud_go_storage/writer.go:433:10: q.GetCommittedSize undefined (type *"google.golang.org/genproto/googleapis/storage/v2".QueryWriteStatusResponse has no field or method GetCommittedSize)

After reading this issue, I tried to downgrade to cloud.google.com/go/storage v1.15.0 (the version used by gocloud.dev v0.23.0) and everything worked. Unfortunately all newer versions are broken. I've tested every version from v1.16.0 until v1.18.2.

tux21b avatar Oct 19 '21 19:10 tux21b

@tux21b,

Thanks for sharing, it indeed might help us zero-in on what actually needs fixing.

Regarding your problem with has no field or method, from my experience, this is due to incompatibility of the go_googleapis version embedded in rules_go (https://github.com/bazelbuild/rules_go/blob/master/go/private/repositories.bzl#L253-L277) and the version of google.golang.org/api you have in your go_repository rule.

I used to have the same problem before, but as long as I make sure that the version in go_repository matches the one in the aforementioned repositories.bzl, you won't have those errors.

(make sure that you have v0.58.0 in your go_repository rule if you're using the latest rules_go).

mishas avatar Oct 19 '21 20:10 mishas

I ran into this problem again recently, using the latest version of Gazelle, rules_go, etc, and the only solution that worked for me was to downgrade com_google_cloud_go_storage to v1.15.0.

As that version is getting increasingly old, I imagine it will eventually cause compatibility problems, are there any other workarounds that have worked for folks?

bcspragu avatar Feb 03 '22 22:02 bcspragu

In case this helps shed some light on it we were having a similar issue on old rules_go and bazel. That error led me to this https://github.com/bazelbuild/bazel-gazelle/issues/1153 which I commented on. Following a Slack thread, it seemed the only way to resolve was to update rules_go (and most everything else along with it bazel, gazelle, etc...).

However, after getting everything working and turning back to trying to upgrade gocloud.dev from 0.23 to 0.24 I'm getting one of the errors above reported by tux21b.

external/com_google_cloud_go_storage/storage.go:1416:53: o.GetCustomerEncryption().GetKeySha256 undefined (type *"google.golang.org/genproto/googleapis/storage/v2".Object_CustomerEncryption has no field or method GetKeySha256)

bazel - 5.0.0 rules_go - 0.30

countravioli avatar Feb 24 '22 03:02 countravioli

What can we do to get eyes on this issue? I was the original reporter of https://github.com/bazelbuild/bazel-gazelle/issues/1153. I'm at the point where I am considering abandoning bazel. We rely heavily on google apis to connect to gcp and I can't keep backing my versions down because of my build system.

pwaterz avatar Mar 31 '22 15:03 pwaterz

Ran into this issue too and downgrading google storage to v1.15.0 really does the magic... ❤️

yufanlusc avatar Apr 01 '22 03:04 yufanlusc

@pwaterz, have you considered filing a bug with https://github.com/googleapis/google-cloud-go ?

gonzojive avatar Jun 06 '22 02:06 gonzojive

Looks like https://github.com/googleapis/google-cloud-go/pull/6266 fixed the issue for google storage, verified that the issue is present before this PR and vanishes after this PR.

This fix is not released yet, but 1.24 release is being prepared from what I see: https://github.com/googleapis/google-cloud-go/pull/6336. You can try the pre 1.24 release to see whether it works:

go get -u cloud.google.com/go/storage@0da97cafde217f8e578319e9d8c2a2b427bfe708

glukasiknuro avatar Jul 20 '22 01:07 glukasiknuro

Can anyone shed light on what the actual issue is? https://github.com/googleapis/google-cloud-go/pull/6266 appears to move a dependency to an internal folder. Why does that fix this? Asking because I am now running into a similar problem with a unrelated library now.

pwaterz avatar Sep 21 '22 17:09 pwaterz

We had the same issue at one point. The issue is basically version drift between the go_googleapis workspace, and the google.golang.org/genproto/googleapis module. When a target uses both as a dependency, for the protobufs Gazelle prefers to resolve to go_googleapis, but that may or may not contain code that would be in google.golang.org/genproto/googleapis.

When something like this comes up, Gazelle can be taught to always resolve to go_googleapis with something like

go_repository(
    name = "org_golang_google_genproto",
    # These build directives are required so that we avoid pulling in @org_golang_google_genproto
    # types that have "weird" build files that are incompatible with rules_go's way of building
    # Go packages. They get pulled in because the override in rules_go for those packages (they
    # have special handling there) isn't up-to-date with the current version of these required
    # by google's storage package. Since there's no change in the _content_ of these types, but
    # just in the version metadata, we can get away with simply replacing them.
    build_directives = [
        "gazelle:resolve go google.golang.org/genproto/googleapis/api/annotations @go_googleapis//google/api:annotations_go_proto",  # keep
    ],
    importpath = "google.golang.org/genproto",
    sum = "h1:ucpgjuzWqWrj0NEwjUpsGTf2IGxyLtmuSk0oGgifjec=",
    version = "v0.0.0-20220919141832-68c03719ef51",
)

sessfeld avatar Sep 22 '22 07:09 sessfeld

Thank you! I was able to resolve an issue with another repo using this method.

pwaterz avatar Oct 03 '22 15:10 pwaterz

Does anyone else find this error message could be more explicit about the suggested corrections and diagnostic commands?

ERROR: /.../pkg/demo/BUILD.bazel:14:10: GoLink pkg/demo/demo_/demo failed: (Exit 1): builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder '-param=bazel-out/darwin-fastbuild/bin/pkg/demo/demo_/demo-0.params' -- -extld external/local_config_cc/cc_wrapper.sh '-buildid=redacted' -extldflags ... (remaining 1 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
link: package conflict error: google.golang.org/genproto/googleapis/api/annotations: multiple copies of package passed to linker:
	@org_golang_google_genproto//googleapis/api/annotations:annotations
	@go_googleapis//google/api:annotations_go_proto
Set "importmap" to different paths or use 'bazel cquery' to ensure only one
  1. I wonder if it could be populated with code that could be copy pasted into the WORKSPACE file with the suggested importmap modifications
  2. What bazel cquery command is being suggested? I hardly ever use bazel cquery, and this sends me on a quest for the proper incantation.

gonzojive avatar Nov 08 '22 03:11 gonzojive