Improve consistency by prefering bazel_dep over Go #2065
Conversation
|
Any idea on how to test this? I think we can easily pin to an older version of circl and have the go.mod file have a newer entry. If things build without |
|
That sounds decent. If you can't get that to work, I'm also fine with this PR as it is - it only deletes cases after all. |
zecke
force-pushed
the
zecke-freyth-2060
branch
from
9335ec8 to
73f7823
Compare
|
Upgrading circl to v1.6.1 was enough. Without the patch I get the below build error showing that the go.mod dep was used instead of the bazel_dep (different reponame). |
Letting the bazel_dep and Go dependency participate equally in the
version resolution is creating various inconsistencies. This can lead to
to a root module having to either use `inject_repo` or `use_repo` and
`override_repo`.
Resolve this by consistently preferring the `bazel_dep` over the Go
dependency. Keep the existing code that warns (or can fail) if the
dependencies are at different versions.
Example MODULE.bazel:
```
module(
name = "bazel_dep_should_win",
)
bazel_dep(name = "rules_go", version = "0.53.0")
bazel_dep(name = "gazelle", version = "0.42.0")
bazel_dep(name = "circl", version = "1.3.8")
go_deps = use_extension("@gazelle//:extensions.bzl", "go_deps")
go_deps.from_file(go_mod = "//:go.mod")
```
And the go.mod referring to a newer version.
Fixes bazel-contrib#2060
fmeum
force-pushed
the
zecke-freyth-2060
branch
from
73f7823 to
893a52c
Compare
|
Hi! I'm struggling to update from gazelle 0.42.0 to 0.43.0 because of this change. If I revert it locally my build works. I may be doing something wrong, but so far I couldn't figure out how to make things work. Any help would be appreciated. Here is what I'm doing: Even with 0.42.0 I get a warning: Attempt 1: just bump gazelle. I get the following error: Attempt 2: bump gazelle and The warning with this change looks like this: I don't think the version difference itself causes the problem, but probably the fact of the difference? I cannot use the exact same version since there is no p.s. there is no What am I doing wrong and how do I fix it? 🤔 |
|
Let me have a closer look tonight. Given it says "@@[unknown repo 'dev_cel_expr'.." have you tried to use the following in your attempt #2? |
|
Hm, I did not realize |
|
Something like this is the offender: https://github.com/google/cel-go/blob/master/MODULE.bazel#L82 We stop generating dev_cel_expr as a "go.mod" dependency and instead rely on the bazel_dep (somewhere in the dependency tree). As a workaround (Let me have a closer look later tonight) |
I think the above is accurate. The following workaround does seem to do the right thing (my build on macos fails with gRPC/protobuf/clang incompatibility issues). We should try to get upstream use the bazel-dep instead of a go_repository as dependency for dev_cel_expr. |
|
I wonder if the "right" fix should be to have the Go module map into the BCR module automatically somehow? Right now this is not the case simply because the BCR module has a weird name (?). Would it work if BCR module was This seems like a fundamental issue in the bazel module system (I may be missing something!) - we can go from the BCR module name to the Go module name (by looking at the
This might be problematic for them - there may not be a corresponding BCR version. At the moment the project is self-contained but that would add an "external" dependency they cannot control to it. I wonder why this project even needs to be a BCR module. Probably so that people can consume the proto files from it... |
Gazelle sees both the Go module and the BCR module for the given importpath and will pick one. Before this change it picked the larger version number and this creates difficulties (see #2060) that cannot be automatically resolved (sometimes override_repo, sometimes inject_repo or it is just broken). The repo_name is necessary as google/cel-go has picked that name for the dependency and has BUILD files that use that name (and no repo alias exists).
Given that the cel-spec and cel-go are owned by Google. I am sure they can sprinkle a bit of automation on the cel-spec release process to publish the release to the BCR as well. I ended up creating bazelbuild/bazel-central-registry#4500 to see where it is going. I think the BCR version of cel-spec is useful for non Go languages. In the go only case, we could probably move https://github.com/google/cel-go/blob/845f2a8ec46a147297bb648edf41cfc2b68fb189/MODULE.bazel#L47-L59 to gazelle and be done. |
Letting the bazel_dep and Go dependency participate equally in the version resolution is creating various inconsistencies. This can lead to to a root module having to either use
inject_repooruse_repoandoverride_repo.Resolve this by consistently preferring the
bazel_depover the Go dependency. Keep the existing code that warns (or can fail) if the dependencies are at different versions.Example MODULE.bazel:
And the go.mod referring to a newer version.
Fixes #2060
What type of PR is this?
What package or component does this PR mostly affect?
What does this PR do? Why is it needed?
Which issues(s) does this PR fix?
Fixes #
Other notes for review