Bazel configuration to better enable dependent builds

[TristonianJones]

Disable cel.dev/expr gazelle build generation

Closes #1184

[TristonianJones]

FYI @fmeum @ouillie

[ouillie]

Here's an outline for how to verify if a solution will work for #1184:

Start by cloning @jarreds reproduction: https://github.com/jarreds/cel-go-repro.

Click for potentially superfluous details that might be helpful for understanding exactly what's happening.

As the README in that repo says, the problem surfaces when you introduce the bzlmod dependency on grpc then try bazel build cel-test. Looking at the output of bazel mod graph, the only chain of module dependencies that leads to cel-spec is grpc -> xds -> cel-spec. Looking at the only version of xds in BCR, you'll notice it's using a pretty old version of cel-spec (0.15.0). You can override this by adding bazel_dep(name = "cel-spec") to the repro's MODULE file (either with a version, or with a local_path_override). FTR, I just tested with the latest cel-spec version (0.25.1) and observed the same failure as with 0.15.0, but I did the rest of my testing with 0.15.0 anyway.

Then, clone cel-spec locally, check out tag v0.15.0, apply the patches listed for that version in BCR, and use a local_path_override in the reproduction repo to bring cel-spec in from this local clone.

Here's what I know: if you edit go.mod in the local cel-spec clone, and change the module name at the top to anything other than cel.dev/expr, the build in the reproduction repo should start working. For instance, it works when you change it to module cel.dev/exp (missing the final 'r'). I cannot confidently tell you why that works, but I laid out my suspicion in #1184 (comment). Basically, I think it has to do with Gazelle getting confused by conflicting definitions for a Go module called cel.dev/expr.

It's possible that entirely ridding the cel-go repo of the Gazelle dependency on cel.dev/expr would also fix this. I left a comment to that effect.

[ouillie]

I believe the fact that this is still here is a problem. It's still relying on the Go module dependency, and not exclusively on the bzlmod dependency, for this Go module.

edit: this comment was based on a misunderstanding of how Gazelle works.

[fmeum]

This is correct as is, Gazelle recognizes that the Bazel module provides the Go module and only uses the former.

[ouillie]

Well, I tested this out locally using:

replace github.com/google/cel-go => <path to 'bzl-deps' branch of cel-go>

in the go.mod file of the reproduction repo, and the problem persisted, but I'm all out of ideas as to how this can be fixed (other than renaming the Go module as I described in #1228 (comment)).

[fmeum]

A local replace would override the bazel_dep usage, I think, so that is not a realistic test setup.

[ouillie]

Any suggestions as to how this could be tested locally? Or any insights as to why renaming the module in cel-spec, or using my workaround in the root repository, does fix the issue?

[ouillie]

I'm a little confused, because there is no cel-go Bazel module. There is a cel-spec Bazel module which corresponds to the Go import path cel.dev/expr, whereas cel-go refers to the Go import path github.com/google/cel-go.

Are you saying that by replacing cel-go, it's also overriding all the transitive dependencies of cel-go (including cel-spec) so that Gazelle no longer tries to use a bazel_dep for any of those where it normally would have?

[TristonianJones]

I'm happy to also make a cel-go BCR module if that helps the situation as well

[ouillie]

I don't think it will, but should be easy to test that out locally since it already has a MODULE.bazel file.

I'm starting to think this is a bug in Gazelle. It looks like Gazelle should actually already see the transitive cel-spec bazel_dep via grpc -> xds in the Bazel module graph, so adding it to cel-go as well shouldn't actually change anything (especially since, in my case, cel-go is also a transitive dependency pulled in via another go_dep, so its MODULE.bazel file just doesn't come into play for me in the first place).

Gazelle definitely has logic to try to handle Go modules supplied as bazel_deps, but it's possible this only really works for the root module, and not the other go_dep modules generated by Gazelle. Idk. Debugging this is hard.

Then again, if Gazelle were fixed (assuming it's not currently WAI), then everyone who uses gRPC would by default use that ancient version of cel-spec (from xds) to satisfy cel-go's dependency on cel-spec, unless they manually add a bazel_dep on cel-spec to the root module. Or, if you make cel-go into a BCR module too, they could just add that as a bazel_dep. Either way, it's a bit of a leaky abstraction for people like me who don't actually depend on either one directly.

This all gets back to my previous assertion that the cleanest solution would have been to make the cel-spec bazel module completely language agnostic, which would've sidestepped this whole issue, but I'm assuming that's a non-starter.

[TristonianJones]

I've disabled build file generation with gazelle, mirroring your workaround @ouillie. PTAL

[ouillie]

Unfortunately, I don't think that's going to help either. cel-go is probably always consumed as a go_dep (because it's not published to BCR), so I believe Gazelle would not actually bother to even look at its MODULE.bazel file. It would just be ignored.

If it were distributed as a Bazel module, it looks like Gazelle would actually fail if it encountered that gazelle_override in a bazel_dep. So, that's clearly not something Gazelle wants libraries to do anyway (which makes sense; a lot of module-level customizations are only permitted in the root module).

So, that workaround does help, but only if everybody who encounters this problem (which only occurs when cel-spec is included as both bazel_dep and a go_dep via different dependency chains) adds it to the MODULE.bazel file of their root module.

Sorry I can't be more helpful. I'm sure you have a limited debugging budget like the rest of us. I'd be willing to sink a bit more time into this if I had a clearer understanding of whether or not Gazelle actually intends to support this corner case in the first place. There's clearly logic to try to substitute Go modules provided by bazel_deps vs go_deps, but it looks to me like it only works for the root module, and maybe doesn't work for transitive go_dep dependencies as in this case. But I'm not even totally sure of that.