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

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

Open
drigz opened this issue Jul 14, 2023 · 18 comments

Comments

@drigz
Copy link
Contributor

drigz commented Jul 14, 2023

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 added a commit to googlecloudrobotics/core that referenced this issue Jul 14, 2023
Tested with `bazel build --incompatible_disable_starlark_host_transitions
//...`. Fixes #95.

These aren't the newest rules_go/gazelle because of
bazel-contrib/rules_go#3625.
@drigz
Copy link
Contributor Author

drigz commented Jul 14, 2023

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.

@fmeum
Copy link
Member

fmeum commented Jul 14, 2023

drigz added a commit to googlecloudrobotics/core that referenced this issue Jul 14, 2023
Tested with `bazel build
--incompatible_disable_starlark_host_transitions //...`. Fixes #95.

These aren't the newest rules_go/gazelle because of
bazel-contrib/rules_go#3625.
@AlexisGoodfellow
Copy link

AlexisGoodfellow commented Jul 15, 2023

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.

@fmeum
Copy link
Member

fmeum commented Jul 15, 2023

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

@linzhp
Copy link
Contributor

linzhp commented Jul 17, 2023

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

@mpawlowski
Copy link

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

@drigz
Copy link
Contributor Author

drigz commented Jul 19, 2023

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?

@linzhp
Copy link
Contributor

linzhp commented Jul 19, 2023

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.

@drigz
Copy link
Contributor Author

drigz commented Jul 20, 2023

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'

@fmeum
Copy link
Member

fmeum commented Jul 20, 2023

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

drigz added a commit to googlecloudrobotics/core that referenced this issue Jul 20, 2023
This required a slightly tricky manual change to go_repositories.bzl:
bazel-contrib/rules_go#3625

I also updated the Go dependencies trying to fix this, which didn't seem
to be necessary but is probably worth doing regardless.
@mathieubergeron
Copy link

mathieubergeron commented Jul 20, 2023

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

@linzhp
Copy link
Contributor

linzhp commented Jul 20, 2023

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

@drigz
Copy link
Contributor Author

drigz commented Jul 21, 2023

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?

@fmeum
Copy link
Member

fmeum commented Jul 22, 2023

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.

@drigz
Copy link
Contributor Author

drigz commented Aug 2, 2023

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 #3625 (comment) and leave this for someone else to pick up. Thanks for your patience with my lack of understanding!

@jschaf
Copy link
Contributor

jschaf commented Aug 11, 2023

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.

@andrewmbenton
Copy link

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 added a commit to andrewmbenton/bazel-gazelle that referenced this issue Aug 30, 2023
…_overrides.bzl

When updating to v0.32.0 anyone with a transient dependency on `github.com/googleapis/gax-go/v2` will need to add an override `gazelle:proto disable` directive. See here for discussion: bazel-contrib/rules_go#3625 (comment)

Adding the directive as a default should prevent a bunch of manual effort.
andrewmbenton added a commit to andrewmbenton/bazel-gazelle that referenced this issue Aug 30, 2023
…_overrides.bzl

When updating to v0.32.0 anyone with a transient dependency on `github.com/googleapis/gax-go/v2` will need to add an override `gazelle:proto disable` directive. See here for discussion: bazel-contrib/rules_go#3625 (comment)

Adding the directive as a default should prevent a bunch of manual effort.
@loeffel-io
Copy link

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
        ],

fmeum pushed a commit to bazel-contrib/bazel-gazelle that referenced this issue Sep 1, 2023
…_overrides.bzl (#1623)

* Add an entry for "github.com/googleapis/gax-go/v2" to default_gazelle_overrides.bzl

When updating to v0.32.0 anyone with a transient dependency on `github.com/googleapis/gax-go/v2` will need to add an override `gazelle:proto disable` directive. See here for discussion: bazel-contrib/rules_go#3625 (comment)

Adding the directive as a default should prevent a bunch of manual effort.

* Update module lockfile
jeromep-stripe pushed a commit to jeromep-stripe/bazel-gazelle that referenced this issue Mar 22, 2024
…_overrides.bzl (bazel-contrib#1623)

* Add an entry for "github.com/googleapis/gax-go/v2" to default_gazelle_overrides.bzl

When updating to v0.32.0 anyone with a transient dependency on `github.com/googleapis/gax-go/v2` will need to add an override `gazelle:proto disable` directive. See here for discussion: bazel-contrib/rules_go#3625 (comment)

Adding the directive as a default should prevent a bunch of manual effort.

* Update module lockfile
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

9 participants