Override dependency not working for google.golang.org/genproto?

[rytswd]

What version of rules_go are you using?

v0.22.2

What version of gazelle are you using?

v0.20.0

What version of Bazel are you using?

v2.2.0

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

Yes

What operating system and processor architecture are you using?

macOS Catalina v10.15.4, x86_64

What did you do?

I made a full repro repo herer at rytswd/simple-bazel, in branch issue/google-cloud-go-1898.

WORKSPACE (full copy with more comments available in the repo):

workspace(name = "com_github_rytswd_proj")

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

# Go
http_archive(
    name = "io_bazel_rules_go",
    sha256 = "142dd33e38b563605f0d20e89d9ef9eda0fc3cb539a14be1bdb1350de2eda659",
    urls = [
        "https://github.com/bazelbuild/rules_go/releases/download/v0.22.2/rules_go-v0.22.2.tar.gz",
        "https://mirror.bazel.build/github.com/bazelbuild/rules_go/releases/download/v0.22.2/rules_go-v0.22.2.tar.gz",
    ],
)

# Gazelle
http_archive(
    name = "bazel_gazelle",
    sha256 = "d8c45ee70ec39a57e7a05e5027c32b1576cc7f16d9dd37135b0eddde45cf1b10",
    urls = [
        "https://storage.googleapis.com/bazel-mirror/github.com/bazelbuild/bazel-gazelle/releases/download/v0.20.0/bazel-gazelle-v0.20.0.tar.gz",
        "https://github.com/bazelbuild/bazel-gazelle/releases/download/v0.20.0/bazel-gazelle-v0.20.0.tar.gz",
    ],
)

# Protobuf
http_archive(
    name = "com_google_protobuf",
    sha256 = "9748c0d90e54ea09e5e75fb7fac16edce15d2028d4356f32211cfa3c0e956564",
    strip_prefix = "protobuf-3.11.4",
    urls = ["https://github.com/protocolbuffers/protobuf/archive/v3.11.4.zip"],
)

load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
load("@bazel_gazelle//:deps.bzl", "go_repository")
load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")

protobuf_deps()

go_repository(
    name = "org_golang_google_genproto",
    importpath = "google.golang.org/genproto",
    sum = "h1:MRHtG0U6SnaUb+s+LhNE1qt1FQ1wlhqr5E4usBKC0uA=",
    version = "v0.0.0-20200331122359-1ee6d9798940",
    # commit = "c50568487044f7393f957c36b91edbf731220cba",
)

go_rules_dependencies()

go_register_toolchains()

load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies")

gazelle_dependencies()

# gazelle:repo bazel_gazelle

# gazelle generated lines below
# ...

What did you expect to see?

$ bazel run simple-bazel
INFO: Analyzed target //:simple-bazel (112 packages loaded, 1082 targets configured).
INFO: Found 1 target...
Target //:simple-bazel up-to-date:
  bazel-bin/darwin_amd64_stripped/simple-bazel
INFO: Elapsed time: 872.241s, Critical Path: 81.19s
INFO: 516 processes: 516 darwin-sandbox.
INFO: Build completed successfully, 533 total actions
INFO: Build completed successfully, 533 total actions
hello

What did you see instead?

...
(snip)...external/com_google_cloud_go_spanner/batch.go:185:3: unknown field 'QueryOptions' in struct literal of type "google.golang.org/genproto/googleapis/spanner/v1".ExecuteSqlRequest
(snip)...external/com_google_cloud_go_spanner/client.go:286:20: undefined: "google.golang.org/genproto/googleapis/spanner/v1".ExecuteSqlRequest_QueryOptions
(snip)...external/com_google_cloud_go_spanner/pdml.go:76:3: unknown field 'QueryOptions' in struct literal of type "google.golang.org/genproto/googleapis/spanner/v1".ExecuteSqlRequest
(snip)...external/com_google_cloud_go_spanner/transaction.go:222:11: undefined: "google.golang.org/genproto/googleapis/spanner/v1".ExecuteSqlRequest_QueryOptions
(snip)...external/com_google_cloud_go_spanner/transaction.go:230:13: undefined: "google.golang.org/genproto/googleapis/spanner/v1".ExecuteSqlRequest_QueryOptions
(snip)...external/com_google_cloud_go_spanner/transaction.go:343:3: unknown field 'QueryOptions' in struct literal of type "google.golang.org/genproto/googleapis/spanner/v1".ExecuteSqlRequest
Target //:simple-bazel failed to build
...

The full log is pretty long, you can see the copy here.

The error indicates that the version of google.golang.org/genproto used in googleapis/google-cloud-go for Spanner is newer compared to the one used in rules_go. I tried the override path, but that didn't seem to solve the situation. I may be doing the override incorrectly, so tried several cases of adjusting the method call orders, but couldn't make it to work.

Would anyone be able to check if my override setup above is done correctly? And if so, is it possible that the override may not be working as expected for google.golang.org/genproto?

Other details

I have originally raised this in googleapis/google-cloud-go repo with googleapis/google-cloud-go#1898, thinking there may be the need of a patch here similar to #2376 and #2378.

[jayconrod]

When using Gazelle and go_repository with default settings, targets that depend on proto packages in google.golang.org/genproto will depend on @go_googleapis, not @org_golang_google_genproto. @go_googleapis is based on https://github.com/googleapis/googleapis, but the build files are replaced with files generated by Gazelle.

In this case, I'd suggest adding the attribute build_file_proto_mode = "disable_global" to go_repository rules that use or contain proto code. You'll probably also want to add # gazelle:proto disable_global in your root build file.

[rytswd]

Thanks very much for the pointer @jayconrod, I managed to run successfully with override by adding build_file_proto_mode = "disable_global" to the very go_repository rule (which was cloud.google.com/go/spanner in this case).

However, I couldn't get the # gazelle:proto disable_global directive in the root BUILD.bazel file to generate build_file_proto_mode = "disable_global".
I could confirm that, by adding the directive and a local .proto file, the .proto file was being ignored and did not get BUILD.bazel created in that directory. So the directive itself is having some effect, but not the attribute handling. With the contrived example repo again, this shows the change I made to repro that behaviour: rytswd/simple-bazel@520faa8
I must have set up something incorrectly for the Gazelle directive...

This leaves another question, though - the above directive implies I cannot rely on the go_proto_library for my own .proto files. I have been using Gazelle for the .proto compilation, but does this mean I should be checking in the protoc generated codes, and use disable_global for managing my dependencies? I'm aware of some discussion about whether or not to check in auto-generated code on Reddit, and thought both approaches are valid. It may not be the case when using Bazel with rules_go?

[jayconrod]

Gazelle runs in one repository at a time. It's also configured one directory at a time, though configuration is inherited in subdirectories. So disable_global is a bit of a misnomer. It only applies to the repository where it's set (the go_repository rule or the main repository if set with the # gazelle:proto disable_global directive in the root build file). It applies to the directory where it's set and to subdirectories (unless overridden). disable_global tells Gazelle not to generate proto rules and to resolve dependencies as if proto rules are not generated anywhere else.

The specific effects of this are that 1) go_proto_library rules are not generated for the repository where that mode is set, and 2) imports of well known types and Google protos are resolved to targets @org_golang_google_genproto instead of @go_googleapis.

If you're using go_proto_library, you probably want to avoid using disable_global and use one of the other proto modes (hopefully the default works). However, @go_googleapis was last updated on 2020-02-21. It sounds like you are relying on a version of @com_google_cloud_go_spanner that needs features introduced after that. So you may need to downgrade @com_google_cloud_go_spanner.

It's difficult to update @go_googleapis on your own. I update it regularly with rules_go patch releases, about once a quarter. I expect the next release should be in early May.

[jayconrod]

About checking in generated code: I think the test for this is whether any non-Bazel project depend on your project. Only Bazel projects will be able to generate code at build time, so if a non-Bazel project depends on you, you must check in generated code.

[rytswd]

Thanks again for the detailed response! I could see how the disable_global directive affecting the way subdirectories treat proto files. I was also hoping this directive to add the build_file_proto_mode = "disable_global" attribute for the go_repository of @com_google_cloud_go_spanner - but that is probably not a part of the directive scope.
As you pointed out, I've set up all the projects based on Bazel, and local builds were using some custom scripts. The go_proto_library worked nicely and quite easily with the default setup, but I'd probably be better off making changes around the setup and to have generated code checked in. That way I could still upgrade @go_googleapis dependencies, without being blocked by rules_go.

So I suppose this means I will need to know exactly which package depends on the different version of @go_googleapis than rules_go's, and add the build_file_proto_mode attribute manually for them after running gazelle update-repos. Apologies how this is rather becoming a Gazelle topic, I'll raise there if I can't find a way to automate the attribute generation myself.

[jayconrod]

Couple things that might help:

gazelle update-repos accepts the -build_file_proto_mode flag and several other flags used to set go_repository attributes. It will affect any go_repository rules Gazelle updates. When you're using -from_file=go.mod, it will be the rules for all the modules imported from go.mod.

You could also try downgrading to an older version of a module with go get -d google.golang.org/genproto@somecommit. That will also downgrade anything that depends on newer versions (spanner), so you should end up with a consistent module graph. That might be a way to temporarily avoid depending on newer stuff in @go_googleapis.

[rytswd]

The -build_file_proto_mode flag did the trick, and I could confirm the builds went successfully with everything pre-generated. I did not have to go with go get -d ... approach but it is certainly good to know.

As a separate note, I start to see many warnings such as warning: package "github.com/golang/protobuf/ptypes/any" is provided by more than one rule. I was under impression that I shouldn't see this as long as I have build_file_proto_mode = "diasble_global" after looking at the Option 2 use pre-generated .pb.go files. I also tried an override like the spanner case above, but both had no effect. As you provided more information on #2206, if warning level is kept until a tactical solution is out, I'll simply ignore these for now, and keep an eye out for future releases.

Again, appreciate your prompt and detailed support in this!