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

Gazelle is not respecting the go_naming_convention directive for external dependencies #1557

Closed
cgrindel opened this issue Jun 12, 2023 · 2 comments · Fixed by bazelbuild/buildtools#1173

Comments

@cgrindel
Copy link
Member

What version of gazelle are you using?

0.31.0

What version of rules_go are you using?

0.39.1

What version of Bazel are you using?

6.2.1

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

Yes.

What operating system and processor architecture are you using?

MacOS, x86_64

What did you do?

  1. Clone rules_swift_package_manager.
  2. Check out go_naming_convention_ext_repro.
  3. Remove this line from BUILD.bazel: # gazelle:go_naming_convention_external go_default_library.
  4. Run bazel run //:tidy. This will run gazelle's update-repos and update.

What did you expect to see?

I expect the labels for the external Go dependencies to use the import naming convention as is specified here.

What did you see instead?

Some, not all of the labels for the Go dependencies use the import naming convention. For example, a go_library in tools/update_go_repos/BUILD.bazel looks like the following:

go_library(
    name = "update_go_repos_lib",
    srcs = [
        "deps_file.go",
        "main.go",
    ],
    importpath = "github.com/cgrindel/rules_swift_package_manager/tools/update_go_repos",
    visibility = ["//visibility:private"],
    deps = [
        "@bazel_gazelle//rule:go_default_library",          # <=== Still go_default_library?
        "@com_github_bazelbuild_buildtools//build",     # <=== See error below
        "@org_golang_x_exp//slices",
    ],
)

When you try to run bazel test //..., you experience the following error:

ERROR: /Users/chuck/code/cgrindel/rules_swift_package_manager/go_naming_convention_ext_repro/tools/update_go_repos/BUILD.bazel:4:11: no such target '@gazelle~0.31.0~go_deps~com_github_bazelbuild_buildtools//build:build': target 'build' not declared in package 'build' defined by /private/var/tmp/_bazel_chuck/6ba9c49fd9b8d8bef05096e7340868b7/external/gazelle~0.31.0~go_deps~com_github_bazelbuild_buildtools/build/BUILD.bazel (Tip: use `query "@com_github_bazelbuild_buildtools//build:*"` to see all the targets in that package) and referenced by '//tools/update_go_repos:update_go_repos_lib'
INFO: Repository rules_bazel_integration_test~0.12.0~bazel_binaries~build_bazel_bazel_.bazelversion instantiated at:
  callstack not available
Repository rule bazel_binary defined at:
  /private/var/tmp/_bazel_chuck/6ba9c49fd9b8d8bef05096e7340868b7/external/rules_bazel_integration_test~0.12.0/bazel_integration_test/private/bazel_binaries.bzl:113:31: in <toplevel>
ERROR: Analysis of target '//tools/update_go_repos:update_go_repos_lib' failed; build aborted: Analysis failed

Other Info

Related to cgrindel/rules_swift_package_manager#410 (comment).

cc: @fmeum

@fmeum
Copy link
Member

fmeum commented Jun 14, 2023

While we could work around this issue by force generating BUILD files for buildtools, the better solution is for it to use import_alias to generate its packaged BUILD files. I would say that all Bazel-aware Go repos should do that as otherwise the repo needs to be fetched first for Gazelle to learn the naming convention to use.

I filed bazelbuild/buildtools#1173 to fix this. Please let me know if that fix does not resolve the issue.

@derekperkins
Copy link

derekperkins commented Aug 15, 2023

update: this was the actual issue

I think these are the same issue. If I'm reading the PR correctly, the cel-go repo would need to regenerate their build files to use alias?

Related issues:

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

Successfully merging a pull request may close this issue.

3 participants