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

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

Open
mishas opened this issue Oct 7, 2021 · 20 comments

Comments

@mishas
Copy link

mishas commented Oct 7, 2021

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
Copy link
Author

mishas commented Oct 11, 2021

@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).

@jayconrod
Copy link
Contributor

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.

@mishas
Copy link
Author

mishas commented Oct 11, 2021

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.

@jayconrod
Copy link
Contributor

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.

@mishas
Copy link
Author

mishas commented Oct 12, 2021

@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?

@robfig
Copy link
Contributor

robfig commented Oct 15, 2021

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.

@mishas
Copy link
Author

mishas commented Oct 15, 2021

@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!

@robfig
Copy link
Contributor

robfig commented Oct 17, 2021

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..

@tux21b
Copy link

tux21b commented Oct 19, 2021

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.

@mishas
Copy link
Author

mishas commented Oct 19, 2021

@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).

@bcspragu
Copy link
Contributor

bcspragu commented Feb 3, 2022

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?

@countravioli
Copy link

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 bazel-contrib/bazel-gazelle#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

@pwaterz
Copy link

pwaterz commented Mar 31, 2022

What can we do to get eyes on this issue? I was the original reporter of bazel-contrib/bazel-gazelle#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.

@yufanlusc
Copy link

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

@gonzojive
Copy link

gonzojive commented Jun 6, 2022

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

@glukasiknuro
Copy link
Contributor

Looks like googleapis/google-cloud-go#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: googleapis/google-cloud-go#6336. You can try the pre 1.24 release to see whether it works:

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

@pwaterz
Copy link

pwaterz commented Sep 21, 2022

Can anyone shed light on what the actual issue is? googleapis/google-cloud-go#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.

@sessfeld
Copy link

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",
)

@pwaterz
Copy link

pwaterz commented Oct 3, 2022

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

@gonzojive
Copy link

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.

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

No branches or pull requests