bazelbuild / bazel-gazelle

Gazelle is a Bazel build file generator for Bazel projects. It natively supports Go and protobuf, and it may be extended to support new languages and custom rule sets.
Apache License 2.0
1.18k stars 374 forks source link

Any way to conditionally apply module_overrides? #1768

Closed EdSchouten closed 5 months ago

EdSchouten commented 5 months ago

What version of gazelle are you using?

0.35.0

What version of rules_go are you using?

0.46.0

What version of Bazel are you using?

7.1.1

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

Yes

What operating system and processor architecture are you using?

macOS Sonoma

What did you do?

In the go-xdr repo, we need to apply a local patch on top of a third party dependency, so we do this:

https://github.com/buildbarn/go-xdr/blob/98907ad75e6da74ebb75e255df284849bf30e21e/MODULE.bazel#L11-L14

go_deps.module_override(
    patches = ["//:patches/com_github_golang_mock/mocks-for-funcs.diff"],
    path = "github.com/golang/mock",
)

Now I have another repo that depends on go-xdr, so I'm dragging it in using bazel_dep() and git_override().

What did you expect to see?

That I can easily use go-xdr from within that other repo.

What did you see instead?

ERROR: Traceback (most recent call last):
    File "/private/var/tmp/_bazel_ed/a6761e4e7c4d3636616263467df83000/external/gazelle~/internal/bzlmod/go_deps.bzl", line 244, column 27, in _go_deps_impl
        _process_overrides(module_ctx, module, "module_override", module_overrides, _process_module_override, archive_overrides)
    File "/private/var/tmp/_bazel_ed/a6761e4e7c4d3636616263467df83000/external/gazelle~/internal/bzlmod/go_deps.bzl", line 130, column 32, in _process_overrides
        _fail_on_non_root_overrides(module_ctx, module, override_type)
    File "/private/var/tmp/_bazel_ed/a6761e4e7c4d3636616263467df83000/external/gazelle~/internal/bzlmod/go_deps.bzl", line 58, column 13, in _fail_on_non_root_overrides
        fail(_FORBIDDEN_OVERRIDE_TAG.format(
Error in fail: Using the "go_deps.module_override" tag in a non-root Bazel module is forbidden, but module "com_github_buildbarn_go_xdr" requests it.

If you need this override for a Bazel module that will be available in a public registry (such as the Bazel Central Registry), please file an issue at https://github.com/bazelbuild/bazel-gazelle/issues/new or submit a PR adding the required directives to the "default_gazelle_overrides.bzl" file at https://github.com/bazelbuild/bazel-gazelle/tree/master/internal/bzlmod/default_gazelle_overrides.bzl.

Now that makes sense. I can imagine that go-xdr applying random patches and such makes it hard for Gazelle to deal with diamond dependency graphs. But my question is, how should I resolve this? Is there a way I can put an if-statement around the go_deps.module_override() call? Right now I have to apply a local patch on top of go-xdr to remove the call.

Maybe it's better if Gazelle didn't throw an error, but simply silently suppressed these overrides when not the root module? I don't think it's reasonable that users add all sorts of conditionals to their MODULE.bazel files to distinguish between the root module/non-root module case.

fmeum commented 5 months ago

Could you try:

go_deps_dev = use_extension(..., dev_dependency = True)
go_deps_dev.module_override(...)

That's how you can get this behavior for any extension without any special support on our side.

EdSchouten commented 5 months ago

Sure, for gomock that would work, but I merely used this as an example. In another repo of mine I need to apply a handful of changes to third-party Go packages, and not all of those are dev dependencies:

https://github.com/buildbarn/bb-storage/blob/69eebfe23bef5e68a4d40c9dea4b2b4f3d02b254/MODULE.bazel#L44-L75

fmeum commented 5 months ago

and not all of those are dev dependencies:

The snippet I posted above also works for non-dev dependencies. It's just the override that is applied only if the module is the root module. Does that help or do you have someone else in mind?

EdSchouten commented 5 months ago

I just tested this, and this doesn't work:

$ bazel build @com_github_buildbarn_go_xdr//pkg/runtime/...
ERROR: no such package '@@[unknown repo 'com_github_stretchr_testify' requested from @@com_github_buildbarn_go_xdr~]//require': The repository '@@[unknown repo 'com_github_stretchr_testify' requested from @@com_github_buildbarn_go_xdr~]' could not be resolved: No repository visible as '@com_github_stretchr_testify' from repository '@@com_github_buildbarn_go_xdr~'
ERROR: /private/var/tmp/_bazel_ed/a6761e4e7c4d3636616263467df83000/external/com_github_buildbarn_go_xdr~/pkg/runtime/BUILD.bazel:16:8: no such package '@@[unknown repo 'com_github_stretchr_testify' requested from @@com_github_buildbarn_go_xdr~]//require': The repository '@@[unknown repo 'com_github_stretchr_testify' requested from @@com_github_buildbarn_go_xdr~]' could not be resolved: No repository visible as '@com_github_stretchr_testify' from repository '@@com_github_buildbarn_go_xdr~' and referenced by '@@com_github_buildbarn_go_xdr~//pkg/runtime:runtime_test'
ERROR: Analysis of target '@@com_github_buildbarn_go_xdr~//pkg/runtime:runtime_test' failed; build aborted: Analysis failed
INFO: Elapsed time: 2.812s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully

Note that I am eventually calling use_repo() against go_deps:

https://github.com/buildbarn/go-xdr/blob/98907ad75e6da74ebb75e255df284849bf30e21e/MODULE.bazel#L15-L23

That stops to function as expected once I use dev_dependency = True.

fmeum commented 5 months ago

@EdSchouten What I had in mind is this:

go_deps = use_extension("@gazelle//:extensions.bzl", "go_deps")
go_deps.from_file(go_mod = "//:go.mod")
use_repo(
    go_deps,
    "cc_mvdan_gofumpt",
    "com_github_antlr_antlr4_runtime_go_antlr",
    "com_github_golang_mock",
    "com_github_gordonklaus_ineffassign",
    "com_github_stretchr_testify",
    "org_golang_x_sync",
)

go_deps_dev = use_extension("@gazelle//:extensions.bzl", "go_deps", dev_dependency = True)
go_deps_dev.module_override(
    patches = ["//:patches/com_github_golang_mock/mocks-for-funcs.diff"],
    path = "github.com/golang/mock",
)

Could you try this?

EdSchouten commented 5 months ago

Woah, that seems to work. Thanks a lot! Kind of impressed that that works. My assumption was that the use_repo() being applied against the non-dev one would cause the overrides on the dev one to get lost. Interesting...

Marking this as closed!