rules_go icon indicating copy to clipboard operation
rules_go copied to clipboard

rules_go (or gazelle?) fails to resolve google/rpc/code.proto in com_github_googleapis_gax_go_v2

Open drigz opened this issue 1 year ago • 18 comments

Workaround / Solution

See mpawlowski's solution or linzhp's explanation.

What version of rules_go are you using?

0.41.0

What version of gazelle are you using?

0.32.0

What version of Bazel are you using?

6.2.0

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

I didn't try Bazel 6.2.1. rules_go and gazelle are latest. It does not reproduce with the previous releases, 0.40.1 and 0.31.1.

What operating system and processor architecture are you using?

Similar to Debian testing, x86.

Any other potentially useful information about your toolchain?

n/a

What did you do?

Updated to rules_go 0.41 and gazelle 0.32 and tried to build https://github.com/googlecloudrobotics/core.

What did you expect to see?

Build succeeds.

What did you see instead?

ERROR: /usr/local/google/home/rodrigoq/.cache/bazel/_bazel_rodrigoq/f3eae32d9daee2b06e08ee03f5238858/external/com_github_googleapis_gax_go_v2/apierror/internal/proto/BUILD.bazel:18:17: no such package '@com_github_googleapis_gax_go_v2//google/rpc': BUILD file not found in directory 'google/rpc' of external repository @com_github_googleapis_gax_go_v2. Add a BUILD file to a directory to mark it as a package. and referenced by '@com_github_googleapis_gax_go_v2//apierror/internal/proto:jsonerror_go_proto'

The error appears to be because code.proto is resolved incorrectly to //google/rpc:code_proto instead of to something inside @org_golang_google_genproto as the release notes suggest.

drigz avatar Jul 14 '23 12:07 drigz

I also tried updating to com_github_googleapis_gax_go_v2 2.12 (latest) but it didn't help. You can repro with:

git clone https://github.com/googlecloudrobotics/core -b rodrigoq/bump-rules-go-2
bazel build //...

EDIT: if you see external/io_opencensus_go_contrib_exporter_stackdriver/stats.go:622:39: cannot use mdr (variable of type *"google.golang.org/genproto/googleapis/monitoring/v3".CreateMetricDescriptorRequest) as type *monitoringpb.CreateMetricDescriptorRequest in argument to c.CreateMetricDescriptor or similar I think it's unrelated as I see it with 0.40.1 and updated deps.

drigz avatar Jul 14 '23 12:07 drigz

Possibly related: https://bazelbuild.slack.com/archives/CDBP88Z0D/p1689260233455729?thread_ts=1689168021.180399&cid=CDBP88Z0D

fmeum avatar Jul 14 '23 12:07 fmeum

Just wanted to comment that I'm seeing the exact same thing - GAX is included as a transitive dependency of cloud.google.com/go/translate, and whatever is causing this is also likely causing my builds to fail.

In the meantime until a patch comes out, I'm going to downgrade to 0.40.1 and 0.31.1.

AlexisGoodfellow avatar Jul 15 '23 01:07 AlexisGoodfellow

Could you try adding a gazelle:resolve directive to the go_repository for com_github_googleapis_gax_go_v2 in the format described in the release notes? You probably also need to set build_file_generation = "on".

Cc @linzhp

fmeum avatar Jul 15 '23 07:07 fmeum

Please read the release notes of rules_go 0.41 and Gazelle 0.32 for migration steps

linzhp avatar Jul 17 '23 17:07 linzhp

I had a similar issue. My bazel build uses pre-generated protobuf source files, so according to the docs, I had to add build_file_proto_mode = "disable_global" to the go_repository target generated by gazelle.

   go_repository(
        name = "com_github_googleapis_gax_go_v2",
        build_file_proto_mode = "disable_global",
        importpath = "github.com/googleapis/gax-go/v2",
        sum = "h1:UBtEZqx1bjXtOQ5BVTkuYghXrr3N4V123VKJK67vJZc=",
        version = "v2.8.0",
    )

mpawlowski avatar Jul 18 '23 23:07 mpawlowski

Please read the release notes of rules_go 0.41 and Gazelle 0.32 for migration steps

The release notes overlook the case of a dependency on a third-party repo that depends on googleapis - unfortunately this probably affects a larger number of clueless maintainers like me.

all Go code importing generated code from Google APIs will depend on @org_golang_google_genproto, which is resolved by Go modules. For proto files importing Google APIs proto and generating Go code, users need to: [...] Resolve the proto manually. If Gazelle is being used, directives like the following need to be added to a parent directory of the proto files [...]

I don't understand whether com_github_googleapis_gax_go_v2 is "importing Google APIs proto and generating Go code" or whether it should be. It was also not clear how/where to add these resolution directives. fmeum's comment adds the missing info (thank you) and suggests that I should tell go_repository to generate Go code.

mpawlowski's comment implies that the go_repository does not need to generate the Go code, but that Gazelle's update-repos command is unable to guess this. This seems like a much simpler workaround that is less likely to break if the repo adds new dependencies on googleapis in future, but I (obviously) don't understand everything that's going on here.

@fmeum Is build_file_proto_mode = "disable_global" a good way to solve the problem? Or is there a reason the maintainers did not suggest it?

drigz avatar Jul 19 '23 08:07 drigz

Is build_file_proto_mode = "disable_global" a good way to solve the problem? Or is there a reason the maintainers did not suggest it?

build_file_proto_mode = "disable_global" is a good idea when a repository has pre-generated Go files and you only use the Go packages, i.e., you don't have any proto files that imports the proto files in that repository. This covers the majority of use cases, because owners of an open source Go repo has to support Go modules by default and Go modules only works with pre-generated Go files. com_github_googleapis_gax_go_v2 belongs to this case.

However, when some proto files don't have pre-generated Go files, or the pre-generated files are out of sync with the proto files, but you need the Go packages, build_file_proto_mode needs to be set accordingly for Gazelle to generate both proto_library and go_proto_library targets.

There are also cases when your own proto files need to import the proto files from other repos. Even though those proto to files come with pre-generated Go package, you still need to set build_file_proto_mode so Gazelle generated proto_library targets for you. However, in this case, it's easier to set gazelle:go_generate_proto false so you don't need to worry about the dependency between third-party protos.

linzhp avatar Jul 19 '23 19:07 linzhp

Thank you for the explanation! Sounds like build_file_proto_mode = "disable_global" is the right solution for repositories that don't rely on Gazelle+Bazel to generate proto code themselves. Am I right in understanding that you don't think you can fix this in gazelle/rules_go and that users need to apply the new configuration to the go_repository rules (maybe based on updated release notes)?

FYI, I tried to do it the way fmeum suggested, but it didn't work:

    go_repository(
        name = "com_github_googleapis_gax_go_v2",
        build_directives = [
            "gazelle:resolve proto proto google/rpc/status.proto @googleapis//google/rpc:status_proto",
            "gazelle:resolve proto go google/rpc/status.proto  @org_golang_google_genproto//googleapis/rpc/status",
            "gazelle:resolve proto google/longrunning/operations.proto @googleapis//google/longrunning:operations_proto",
            "gazelle:resolve proto go google/longrunning/operations.proto @org_golang_google_genproto//googleapis/longrunning",
        ],
        build_file_generation = "on",
        importpath = "github.com/googleapis/gax-go/v2",
        sum = "h1:A+gCJKdRfqXkr+BIRGtZLibNXf0m1f9E4HG56etFpas=",
        version = "v2.12.0",
    )

still fails with:

ERROR: /usr/local/google/home/rodrigoq/.cache/bazel/_bazel_rodrigoq/f3eae32d9daee2b06e08ee03f5238858/external/com_github_googleapis_gax_go_v2/apierror/internal/proto/BUILD.bazel:18:17: no such package '@com_github_googleapis_gax_go_v2//google/rpc': BUILD file not found in directory 'google/rpc' of external repository @com_github_googleapis_gax_go_v2. Add a BUILD file to a directory to mark it as a package. and referenced by '@com_github_googleapis_gax_go_v2//apierror/internal/proto:jsonerror_go_proto'

drigz avatar Jul 20 '23 07:07 drigz

With Bzlmod, we have a registry of default build_directives which would free users from having to specify them manually.

fmeum avatar Jul 20 '23 08:07 fmeum

@drigz

FYI, I tried to do it the way fmeum suggested, but it didn't work

As you mentioned in the description of this issue, com_github_googleapis_gax_go_v2 actually depends on //google/rpc:code_proto. :)

So this is what would be necessary:

    go_repository(
        name = "com_github_googleapis_gax_go_v2",
        build_directives = [
            "gazelle:resolve proto go google/rpc/code.proto @org_golang_google_genproto_googleapis_rpc//code",  # keep
            "gazelle:resolve proto proto google/rpc/code.proto @googleapis//google/rpc:code_proto",  # keep
        ],
        importpath = "github.com/googleapis/gax-go/v2",
        sum = "h1:A+gCJKdRfqXkr+BIRGtZLibNXf0m1f9E4HG56etFpas=",
        version = "v2.12.0",
    )

This is assuming that you have the googleapis archive using the name googleapis in your WORKSPACE:

http_archive(
    name = "googleapis",
    sha256 = "78aae8879967e273044bc786e691d9a16db385bd137454e80cd0b53476adfc2d",
    strip_prefix = "googleapis-c09efadc6785560333d967f0bd40f1d1c3232088",
    urls = ["https://github.com/googleapis/googleapis/archive/c09efadc6785560333d967f0bd40f1d1c3232088.tar.gz"],
)

load("@googleapis//:repository_rules.bzl", "switched_rules_by_language")

switched_rules_by_language(
    name = "com_google_googleapis_imports",
)

Also, build_file_generation = "on" does not seem necessary.

mathieubergeron avatar Jul 20 '23 12:07 mathieubergeron

@drigz The gazelle:resolve lines in the release notes are examples, not meant to be comprehensive or necessary everywhere.

There are so many protos in Google APIs. If we provide all gazelle:resolve to all of them as part of rules_go or Gazelle, we will have to update them when Google APIs protos change, which is what we are trying to avoid in the latest releases.

linzhp avatar Jul 20 '23 15:07 linzhp

The gazelle:resolve lines in the release notes are examples, not meant to be comprehensive or necessary everywhere.

Thanks for clarifying, this is obvious in hindsight although I didn't realize it when reading the release notes or fmeum's comment. I guess the keywords "like the following" didn't jump out to me, sorry.

It seems that https://github.com/bazelbuild/bazel-gazelle/blob/d2032781c7d4611ce778360ca345d86a97e06956/internal/bzlmod/default_gazelle_overrides.bzl would address the issue for brainless users who picked this up somewhere in a transitive dependency tree - @fmeum, should I send a PR? How can I test my change?

drigz avatar Jul 21 '23 07:07 drigz

It seems that https://github.com/bazelbuild/bazel-gazelle/blob/d2032781c7d4611ce778360ca345d86a97e06956/internal/bzlmod/default_gazelle_overrides.bzl would address the issue for brainless users who picked this up somewhere in a transitive dependency tree - @fmeum, should I send a PR? How can I test my change?

You can add the directives to a local checkout of bazel-gazelle and use a local_path_override in your MODULE.bazel file to replace the gazelle module with that checkout. If your build still succeeds after you delete the gazelle_override from your module file, the directives should be correct. Please send a PR once you get to that point.

fmeum avatar Jul 22 '23 12:07 fmeum

You can add the directives to a local checkout of bazel-gazelle and use a local_path_override in your MODULE.bazel file to replace the gazelle module with that checkout. If your build still succeeds after you delete the gazelle_override from your module file, the directives should be correct. Please send a PR once you get to that point.

I gave this a try but gave up after failing to use MODULE.bazel or --override_repository (I added a syntax error into my local clone's root BUILD.bazel and ran bazel clean --expunge but it still seemed to be ignored). I'm afraid I've spent too much time on this issue, so I'll stick with the workaround from https://github.com/bazelbuild/rules_go/issues/3625#issuecomment-1641108398 and leave this for someone else to pick up. Thanks for your patience with my lack of understanding!

drigz avatar Aug 02 '23 14:08 drigz

I hit this trying to upgrade rules_go in order to upgrade to Go 1.21. I think the instructions could use a bit more hand-holding. I spent about 3 hours trying to upgrade. The sequence that ended up working for me:

  1. Paste in example gazelle resolve.
  2. Figure out I need to add all my google proto deps as gazelle resolves.
  3. Use grep and sed to "parse" the proto files and print out the gazelle resolve any google protobuf imports.
  4. Manually clean up a bunch of gazelle resolves to match the path in @googleapis by looking at the GitHub repo structure.
  5. Remove google/protobuf since those are special cased as well-known-types.
  6. Run gazelle.
  7. Find this bug since com_github_googleapis_gax_go_v2 can't find google/rpc/code.proto
  8. Adapt the provided snippet to point to working targets.

My gazelle resolves ended up like:

# Setup googleapis.
# https://github.com/bazelbuild/rules_go/releases/tag/v0.41.0
# gazelle:resolve proto    google/api/field_behavior.proto      @googleapis//google/api:field_behavior_proto
# gazelle:resolve proto go google/api/field_behavior.proto      @org_golang_google_genproto//googleapis/api/annotations
# gazelle:resolve proto    google/api/resource.proto            @googleapis//google/api:resource_proto
# gazelle:resolve proto go google/api/resource.proto            @org_golang_google_genproto//googleapis/api/annotations
# gazelle:resolve proto    google/longrunning/operations.proto  @googleapis//google/longrunning:operations_proto
# gazelle:resolve proto go google/longrunning/operations.proto  @org_golang_google_genproto//googleapis/longrunning
# gazelle:resolve proto    google/rpc/code.proto                @googleapis//google/rpc:code_proto
# gazelle:resolve proto go google/rpc/code.proto                @org_golang_google_genproto//googleapis/rpc/code
# gazelle:resolve proto    google/rpc/status.proto              @googleapis//google/rpc:status_proto
# gazelle:resolve proto go google/rpc/status.proto              @org_golang_google_genproto//googleapis/rpc/status

# gazelle:resolve proto    google/type/date.proto               @googleapis//google/type:date_proto
# gazelle:resolve proto go google/type/date.proto               @org_golang_google_genproto//googleapis/type/date
# gazelle:resolve proto    google/type/decimal.proto            @googleapis//google/type:decimal_proto
# gazelle:resolve proto go google/type/decimal.proto            @org_golang_google_genproto//googleapis/type/decimal
# gazelle:resolve proto    google/type/interval.proto           @googleapis//google/type:interval_proto
# gazelle:resolve proto go google/type/interval.proto           @org_golang_google_genproto//googleapis/type/interval
# gazelle:resolve proto    google/type/latlng.proto             @googleapis//google/type:latlng_proto
# gazelle:resolve proto go google/type/latlng.proto             @org_golang_google_genproto//googleapis/type/latlng
# gazelle:resolve proto    google/type/money.proto              @googleapis//google/type:money_proto
# gazelle:resolve proto go google/type/money.proto              @org_golang_google_genproto//googleapis/type/money
# gazelle:resolve proto    google/type/postal_address.proto     @googleapis//google/type:postal_address_proto
# gazelle:resolve proto go google/type/postal_address.proto     @org_golang_google_genproto//googleapis/type/postaladdress
# gazelle:resolve proto    google/type/timeofday.proto          @googleapis//google/type:timeofday_proto
# gazelle:resolve proto go google/type/timeofday.proto          @org_golang_google_genproto//googleapis/type/timeofday

I ended up with this:

    go_repository(
        name = "com_github_googleapis_gax_go_v2",
        build_directives = [
            "gazelle:resolve proto proto google/rpc/code.proto @googleapis//google/rpc:code_proto",  # keep
            "gazelle:resolve proto go    google/rpc/code.proto @org_golang_google_genproto//googleapis/rpc/code",  # keep
        ],
        importpath = "github.com/googleapis/gax-go/v2",
        sum = "h1:dp3bWCh+PPO1zjRRiCSczJav13sBvG4UhNyVTa1KqdU=",
        version = "v2.1.1",
    )

With Bzlmod, we have a registry of default build_directives which would free users from having to specify them manually.

Have any large commercial users migrated to bzlmod? It looks a lot nicer, but I'd rather not be the first to forge that path.

jschaf avatar Aug 11 '23 00:08 jschaf

With Bzlmod, we have a registry of default build_directives which would free users from having to specify them manually.

For those using Bzlmod and struggling to resolve this manually (as I have been for many hours over the last couple days), adding the following to MODULE.bazel worked for our setup:

go_deps.gazelle_override(
    path = "github.com/googleapis/gax-go/v2",
    directives = [
        "gazelle:proto disable",
    ]
)

I suppose this might be a useful addition to the registry of default build directives.

andrewmbenton avatar Aug 11 '23 06:08 andrewmbenton

This did the trick for me

        build_directives = [
            "gazelle:resolve proto proto google/rpc/code.proto @googleapis//google/rpc:code_proto",  # keep
            "gazelle:resolve proto go    google/rpc/code.proto @org_golang_google_genproto_googleapis_rpc//code",  # keep
        ],

loeffel-io avatar Aug 31 '23 14:08 loeffel-io