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

update-repos: set build_naming_convention for modules that already have build files #890

Open
ukai opened this issue Aug 31, 2020 · 16 comments

Comments

@ukai
Copy link

ukai commented Aug 31, 2020

What version of gazelle are you using?

upgrading from to v0.22.0

What version of rules_go are you using?

rules_go 0.24.0

What version of Bazel are you using?

0.3.2

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

What operating system and processor architecture are you using?

linux

What did you do?

upgrade gazelle to 0.22.0 and run gazelle

What did you expect to see?

no build break

What did you see instead?

gazelle modified dependencies to external repositories (managed by bazel) to use import go naming convention, but the external repo still uses go_default_library naming convention.

I can fix this by adding gazelle:go_naming_convention_external go_default_library, but what is the best practice to migrate naming convention (especially if there are multiple external repos)?

0.22.0 changed go_naming_convention to import (https://github.com/bazelbuild/bazel-gazelle/releases/tag/v0.22.0)
but README still says go_default_library is the default? (https://github.com/bazelbuild/bazel-gazelle#directives)

@jayconrod
Copy link
Contributor

How is the external repository declared? Could you provide a small example?

If it's a go_repository in WORKSPACE or in a macro referenced by # gazelle:repository_macro, then Gazelle should resolve dependencies based on the naming convention used in that repository. By default, that's import_alias, so either convention should work.

If the repository is not declared, or if it's declared with a rule other than go_repository, Gazelle will assume go_default_library unless told otherwise.

@ukai
Copy link
Author

ukai commented Aug 31, 2020

go_repository(
    name = "com_github_bazelbuild_remote_apis_sdks",
    commit = "d14bd4ab9d97092a04ef96769c09513eea64566f",
    importpath = "github.com/bazelbuild/remote-apis-sdks",
)

It still use go_default_library, but gazelle rewrote it as import deps.

@jayconrod
Copy link
Contributor

Thanks. Looks like the issue is that this repository already has its own build files. go_repository won't generate them, but Gazelle's external dependency resolution expects them to be generated.

I guess update-repos could try to detect whether a repository has build files already, then set build_file_generation = "off" and build_naming_convention = "go_default_library". That would be expensive though, since update-repos would have to download every module.

You could set those attributes manually for now.

@jayconrod jayconrod changed the title naming convention transition? update-repos: set build_naming_convention for modules that already have build files Sep 1, 2020
@purkhusid
Copy link
Contributor

Just hit the same problem when trying to add https://github.com/go-resty/resty as a dependency.

@ukai
Copy link
Author

ukai commented Sep 2, 2020

thanks!

go_repository(
    name = "com_github_bazelbuild_remote_apis_sdks",
    commit = "d14bd4ab9d97092a04ef96769c09513eea64566f",
    importpath = "github.com/bazelbuild/remote-apis-sdks",
    build_file_generation = "off",
    build_naming_convention = "go_default_library",
)

works without gazelle:go_naming_convention_external go_default_library.

@dragonsinth
Copy link
Contributor

It would actually be really great if update-repos would analyze things more deeply (at the very least, when actually writing an add or update). It's a pain to go fix all these up manually, and super confusing to even understand what's gone wrong.

@zecke
Copy link

zecke commented Nov 29, 2020

It's currently near impossible to use gazelle for something complex (e.g. attempt to build cortexmetrics). Is it an option to revert the naming changes or offer a global kill switch for gazelle (don't do the renaming on update or when invoked through go_repository) until there is a better migration path?

I do run into #892 (I attempt to build cortex, that depends on prometheus, prometheus pulls in grpc_gateway for some of the pre-generated potobuf code). The generated build files for prometheus refer to @grpc_gateway//runtime:runtime. I have put the above workaround into the deps.bzl file. In the local //:BUILD.bazel I tried to use both naming conventions options to use the old way. To no extend.

The most annoying thing is that this is painful to debug. If I could hit a breakpoint to when go_repository runs gazelle and look at the state it would certainly be less painful.

EDIT: I got it to build. In addition to editing the com_github_grpc_ecosystem_grpc_gateway I had to use naming_convention inside the go_repository rule for prometheus. And as prometheus is used by thanos also modify the thanos go_repository rule. Is this transitivity intended?
Can something be done to ease the migration? First generate both old/new name but always use the go_default_library dependency? Can update-repo auto-add the build_file_naming_convention target?
If it is of any help I can push the bazel files generated and how they end up being working.

@jayconrod
Copy link
Contributor

@zecke

Is it an option to revert the naming changes or offer a global kill switch for gazelle (don't do the renaming on update or when invoked through go_repository) until there is a better migration path?

No, this won't be reverted.

In addition to editing the com_github_grpc_ecosystem_grpc_gateway I had to use naming_convention inside the go_repository rule for prometheus.

That doesn't seem right? By default, go_repository will use the import_alias naming convention, which generates targets with the new names and with go_default_library aliases. Dependent code that uses either naming convention should work. If that's not the case, please open a new issue. This issue is about using go_repository on modules that already have build files, in which case, Gazelle is not run by default.

Can update-repo auto-add the build_file_naming_convention target?

That's what this issue is about.

The main problem is that it requires update-repos to fetch every module, which could make it much slower on slow connections. There aren't that many Go repositories with checked-in build files, and if they migrate to the new naming convention, this slowdown won't be necessary.

@dragonsinth
Copy link
Contributor

@zecke if it helps, this is what we ultimately checked in for grpc-gateway:

    go_repository(
        name = "com_github_grpc_ecosystem_grpc_gateway",
        build_directives = ["gazelle:exclude **/**_test.go", "gazelle:exclude examples"],
        build_file_generation = "on",
        build_file_name = "BUILD.bazel",
        build_file_proto_mode = "disable_global",
        importpath = "github.com/grpc-ecosystem/grpc-gateway",
        sum = "h1:YuM9SXYy583fxvSOkzCDyBPCtY+/IMSHEG1dKFMLZsA=",
        version = "v1.14.1",
    )

It's a little heavy-handed, but basically forcing build file generation on to overwrite their build files and then aggressively excluding their tests and examples did it for me. build_file_proto_mode would depend on what you're doing everywhere else in your build.

@base698
Copy link

base698 commented Jan 18, 2021

I added @dragonsinth 's `go_repository. I am now getting an error with prometheus not being able to find prompb_proto.

ERROR: /home/justin/.cache/bazel/_bazel_justin/7cc8161d417427636c362dace4918829/external/com_github_prometheus_prometheus/prompb/BUILD.bazel:18:17: no such package '@com_github_prometheus_prometheus//': BUILD file not found in directory '' of external repository @com_github_prometheus_prometheus. Add a BUILD file to a directory to mark it as a package. and referenced by '@com_github_prometheus_prometheus//prompb:prompb_go_proto'

Adding disable_global to prometheus causes yet another error.

@tanyabouman
Copy link
Contributor

@dragonsinth
Thanks for the helpful go_repository example. It got me on the right track, and then I had to add a few gazelle resolve directives to get it to work.

    go_repository(
        name = "com_github_grpc_ecosystem_grpc_gateway",
        build_directives = [
            "gazelle:exclude **/**_test.go",
            "gazelle:exclude examples",
            "gazelle:resolve go github.com/grpc-ecosystem/grpc-gateway/internal //internal",
            "gazelle:resolve go github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options //protoc-gen-swagger/options",
            ],
        build_file_generation = "on",
        build_file_name = "BUILD.bazel",
        build_file_proto_mode = "disable_global",
        importpath = "github.com/grpc-ecosystem/grpc-gateway",
        sum = "h1:gmcG1KaJ57LophUzW0Hy8NmPhnMZb4M0+kPpLofRdBo=",
        version = "v1.16.0",
    )

@tanyabouman
Copy link
Contributor

@base698
Is this the error you're getting after adding disable_global to prometheus?

ERROR: [...]/9ffa4be79841cb9af73919d4f8c803ad/external/com_github_grpc_ecosystem_grpc_gateway/runtime/BUILD.bazel:5:11: GoCompilePkg external/com_github_grpc_ecosystem_grpc_gateway/runtime/runtime.a failed: (Exit 1): builder failed: error executing command bazel-out/host/bin/external/go_sdk/builder compilepkg -sdk external/go_sdk -installsuffix linux_amd64 -src external/com_github_grpc_ecosystem_grpc_gateway/runtime/context.go -src ... (remaining 75 argument(s) skipped)

Use --sandbox_debug to see verbose messages from the sandbox
compilepkg: missing strict dependencies:
	[...]/9ffa4be79841cb9af73919d4f8c803ad/sandbox/linux-sandbox/370/execroot/__main__/external/com_github_grpc_ecosystem_grpc_gateway/runtime/errors.go: import of "github.com/grpc-ecosystem/grpc-gateway/internal"
	[...]/9ffa4be79841cb9af73919d4f8c803ad/sandbox/linux-sandbox/370/execroot/__main__/external/com_github_grpc_ecosystem_grpc_gateway/runtime/handler.go: import of "github.com/grpc-ecosystem/grpc-gateway/internal"
	[...]/9ffa4be79841cb9af73919d4f8c803ad/sandbox/linux-sandbox/370/execroot/__main__/external/com_github_grpc_ecosystem_grpc_gateway/runtime/proto_errors.go: import of "github.com/grpc-ecosystem/grpc-gateway/internal"
No dependencies were provided.
Check that imports in Go sources match importpath attributes in deps.

When looking at your prompb repo, I got past that error by adding gazelle resolve directives to the grpc_gateway go_repository.

        "gazelle:resolve go github.com/grpc-ecosystem/grpc-gateway/internal //internal",
        "gazelle:resolve go github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/options //protoc-gen-swagger/options",

After that, I added build_file_proto_mode = "disable_global", to the org_golang_google_grpc go_repository. Then everything builds.

jeevatkm pushed a commit to go-resty/resty that referenced this issue Sep 12, 2021
* Update Bazel build to work with 4.0.0

This includes several updates to the BUILD and WORKSPACE files, so that they now work with Bazel 4.0.0, Gazelle 0.23 and rules_go 0.27.

By default, [Gazelle now uses](bazel-contrib/bazel-gazelle#863) the last component of the Bazel package, rather than `go_default_library` as the `go_library name`.

This causes problems when the naming conventions do not match.
bazel-contrib/bazel-gazelle#890 (comment)

Adding support for the new naming convention should not break anything that depends on using the old naming convention, because the Gazelle option `import_alias` makes aliases to the libraries and this allows the old naming convention to still work.
@aw185176
Copy link

I had something very similar to what @tanyabouman shared previously and it worked, but now that I am upgrading to v1.16.0, its breaking with this:

❯ bazel query @com_github_grpc_ecosystem_grpc_gateway//...
Starting local Bazel server and connecting to it...
INFO: SHA256 (https://golang.org/dl/?mode=json&include=all) = 47299df6f0166ee1658fcd04d7e5db43a6b537d18f7886121ad7de9598371be1
ERROR: error loading package '@com_github_grpc_ecosystem_grpc_gateway//examples/internal/proto/examplepb': Unable to find package for @grpc_ecosystem_grpc_gateway//protoc-gen-swagger:defs.bzl: The repository '@grpc_ecosystem_grpc_gateway' could not be resolved.
Loading: 2 packages loaded
    currently loading: @com_github_grpc_ecosystem_grpc_gateway//protoc-gen-swagger/options ... (28 packages)
    Fetching @rules_proto; fetching

I updated to the exact go_repository @tanyabouman shared, and the error persists. Is it possible the gazelle exclude directive isn't being handled properly? Or is bazel query an entirely separate beast that cannot be sorted out with gazelle directives?

DomenicoSola pushed a commit to DomenicoSola/resty that referenced this issue Oct 19, 2022
* Update Bazel build to work with 4.0.0

This includes several updates to the BUILD and WORKSPACE files, so that they now work with Bazel 4.0.0, Gazelle 0.23 and rules_go 0.27.

By default, [Gazelle now uses](bazel-contrib/bazel-gazelle#863) the last component of the Bazel package, rather than `go_default_library` as the `go_library name`.

This causes problems when the naming conventions do not match.
bazel-contrib/bazel-gazelle#890 (comment)

Adding support for the new naming convention should not break anything that depends on using the old naming convention, because the Gazelle option `import_alias` makes aliases to the libraries and this allows the old naming convention to still work.
@AFMiziara
Copy link

AFMiziara commented Aug 17, 2023

I was having the same issue when I introduced this new dependency in my project: github.com/grpc-ecosystem/grpc-gateway/v2/runtime

/home/runner/.cache/bazel/_bazel_runner/5df452d9b7f3b22c13423632ba0d5b97/external/com_github_grpc_ecosystem_grpc_gateway_v2/runtime/BUILD.bazel:5:11: no such package '@go_googleapis//google/api': The repository '@go_googleapis' could not be resolved: Repository '@go_googleapis' is not defined and referenced by '@com_github_grpc_ecosystem_grpc_gateway_v2//runtime:runtime'

I solved by simply adding this to my deps.bzl:

    go_repository(
        name = "com_github_grpc_ecosystem_grpc_gateway_v2",
        importpath = "github.com/grpc-ecosystem/grpc-gateway/v2",
        sum = "h1:1JYBfzqrWPcCclBwxFCPAou9n+q86mfnu7NAeHfte7A=",
+       build_file_name = "BUILD.bazel", # keep: https://github.com/bazelbuild/bazel-gazelle/issues/890
        version = "v2.15.0",
    )

But this shouldn't be necessary, if, according to the go_repository reference, the default value for build_file_name is BUILD.bazel,BUILD:

Comma-separated list of names Gazelle will consider to be build files. If a repository contains files named build that aren't related to Bazel, it may help to set this to "BUILD.bazel", especially on case-insensitive file systems.

Any thoughts?

@lf-jason-jia
Copy link

I solved by simply adding this to my deps.bzl:

I did almost identical thing except the dependency is v1 (so com_github_grpc_ecosystem_grpc_gateway) and have the same error message. But adding that line doesn't help.

@debkanchan
Copy link

debkanchan commented Dec 6, 2023

impacted issue

I'm using the upcoming bzlmod support in rules_buf and I've encountered this error.

ERROR: /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go/celext/BUILD.bazel:3:11: no such target '@gazelle~0.34.0~go_deps~com_github_google_cel_go//cel:cel': target 'cel' not declared in package 'cel' defined by /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_google_cel_go/cel/BUILD.bazel (Tip: use `query "@@gazelle~0.34.0~go_deps~com_github_google_cel_go//cel:*"` to see all the targets in that package) and referenced by '@gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go//celext:celext'
ERROR: /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go/celext/BUILD.bazel:3:11: no such target '@gazelle~0.34.0~go_deps~com_github_google_cel_go//common/overloads:overloads': target 'overloads' not declared in package 'common/overloads' defined by /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_google_cel_go/common/overloads/BUILD.bazel (did you mean 'overloads.go'? Tip: use `query "@@gazelle~0.34.0~go_deps~com_github_google_cel_go//common/overloads:*"` to see all the targets in that package) and referenced by '@gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go//celext:celext'
ERROR: /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go/celext/BUILD.bazel:3:11: no such target '@gazelle~0.34.0~go_deps~com_github_google_cel_go//common/types:types': target 'types' not declared in package 'common/types' defined by /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_google_cel_go/common/types/BUILD.bazel (Tip: use `query "@@gazelle~0.34.0~go_deps~com_github_google_cel_go//common/types:*"` to see all the targets in that package) and referenced by '@gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go//celext:celext'
ERROR: /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go/celext/BUILD.bazel:3:11: no such target '@gazelle~0.34.0~go_deps~com_github_google_cel_go//common/types/ref:ref': target 'ref' not declared in package 'common/types/ref' defined by /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_google_cel_go/common/types/ref/BUILD.bazel (Tip: use `query "@@gazelle~0.34.0~go_deps~com_github_google_cel_go//common/types/ref:*"` to see all the targets in that package) and referenced by '@gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go//celext:celext'
ERROR: /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go/celext/BUILD.bazel:3:11: no such target '@gazelle~0.34.0~go_deps~com_github_google_cel_go//common/types/traits:traits': target 'traits' not declared in package 'common/types/traits' defined by /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_google_cel_go/common/types/traits/BUILD.bazel (Tip: use `query "@@gazelle~0.34.0~go_deps~com_github_google_cel_go//common/types/traits:*"` to see all the targets in that package) and referenced by '@gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go//celext:celext'
ERROR: /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go/celext/BUILD.bazel:3:11: no such target '@gazelle~0.34.0~go_deps~com_github_google_cel_go//ext:ext': target 'ext' not declared in package 'ext' defined by /private/var/tmp/_bazel_debkanchan/969f704a68afbe6aa6f76e61ecfa9c99/external/gazelle~0.34.0~go_deps~com_github_google_cel_go/ext/BUILD.bazel (Tip: use `query "@@gazelle~0.34.0~go_deps~com_github_google_cel_go//ext:*"` to see all the targets in that package) and referenced by '@gazelle~0.34.0~go_deps~com_github_bufbuild_protovalidate_go//celext:celext'

MODULE

bazel_dep(name = "rules_go", version = "0.42.0")
bazel_dep(name = "gazelle", version = "0.34.0")
bazel_dep(name = "rules_pkg", version = "0.9.1")
bazel_dep(name = "rules_buf", version = "0.2.0")
bazel_dep(name = "rules_proto", version = "5.3.0-21.7")
bazel_dep(name = "rules_oci", version = "1.4.0")
bazel_dep(name = "container_structure_test", version = "1.16.0")

git_override(
    module_name = "rules_buf",
    commit = "27e9a19c6227cc3923abb1a4e703d9d42d9a1fa9",
    remote = "https://github.com/bufbuild/rules_buf.git",
)

buf = use_extension("@rules_buf//buf:extensions.bzl", "buf")

# Override the default version of buf
buf.toolchains(version = "v1.28.1")

go_deps = use_extension("@gazelle//:extensions.bzl", "go_deps")
go_deps.from_file(go_mod = "//:go.mod")
use_repo(go_deps, "build_buf_gen_go_bufbuild_protovalidate_protocolbuffers_go", "build_buf_gen_go_ride_wallet_connectrpc_go", "build_buf_gen_go_ride_wallet_protocolbuffers_go", "com_connectrpc_connect", "com_github_bufbuild_protovalidate_go", "com_github_golang_jwt_jwt_v5", "com_github_google_wire", "com_github_ilyakaznacheev_cleanenv", "com_github_micahparks_keyfunc_v2", "com_github_mmcloughlin_geohash", "com_github_onsi_ginkgo_v2", "com_github_onsi_gomega", "com_google_cloud_go_firestore", "com_google_firebase_go_v4", "org_golang_google_genproto", "org_golang_google_genproto_googleapis_api", "org_golang_google_grpc", "org_golang_google_protobuf", "org_golang_x_net", "org_uber_go_mock", "org_uber_go_zap")

oci = use_extension("@rules_oci//oci:extensions.bzl", "oci")
oci.pull(
    name = "distroless",
    digest = "sha256:e7e79fb2947f38ce0fab6061733f7e1959c12b843079042fe13f56ca7b9d178c",
    image = "gcr.io/distroless/static",
    platforms = [
        "linux/amd64",
        "linux/arm64/v8",
    ],
)
use_repo(oci, "distroless")

BUILD

load("@gazelle//:def.bzl", "DEFAULT_LANGUAGES", "gazelle", "gazelle_binary")

gazelle_binary(
    name = "gazelle-buf",
    languages = DEFAULT_LANGUAGES + [
        # Loads the Buf extension
        "@rules_buf//gazelle/buf:buf",
        # NOTE: This needs to be loaded after the proto language
    ],
)

# gazelle:prefix github.com/ride-app/driver-service
# gazelle:build_file_name BUILD.bazel
# gazelle:exclude infra
# gazelle:proto disable_global
gazelle(
    name = "gazelle",
    gazelle = ":gazelle-buf",
)

Since bzlmod support means there's no go_deps.bazel file anymore where I could manually change settings I'm stuck as it seems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests