-
Notifications
You must be signed in to change notification settings - Fork 386
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
Let bazel_dep
s replace Go deps
#1526
Conversation
dc9b88c
to
a6b2fbe
Compare
@Wyverald @tyler-french Could you review? I implemented two different approaches (first commit vs. both commits), with one making the association explicit and one deriving it from |
We probably need to introduce some kind of version resolution to ensure that outdated |
0d6dcff
to
13250ef
Compare
I added version resolution. @tyler-french @seh Could you take a look if time permits? |
I read it all, but I need to think about both the problem and the solution more before I have anything constructive to add. |
@seh Thank, that's exactly what I'm looking for. Take your time! |
Tested at Uber |
I rebased onto master and pushed a new commit that adds support for the generalized semver format used by Bazel module registries. I will wait for bazelbuild/bazel-central-registry#674 and add a test based on it before merging. |
03e4809
to
060f8c0
Compare
When a Go dependency is brought in both as a `bazel_dep` and as a regular Go dependency via e.g. go.mod, the `bazel_dep` repo should be used instead of creating a separate `go_repository`, which would risk duplicate packages. This is realized by registering every Bazel module with a `go_deps.from_file` tag as providing the Go module of the same version with the module path given in `go.mod`. Gazelle is aware of these tags and generates either a canonical label or a label using the apparent module repo name (the latter only for the root module and only if it directly depends on the `bazel_dep`).
I hadn't seen a module like circl before, where there's an upstream Go module with no "awareness" of Bazel, but then we publish a Bazel module that adds in all the necessary Bazel control files to allow building and testing the library. Now, coming back to this patch, I understand it better now, in that we're reading all the uses of the The description mentions a "provides" tag on the |
bazel_dep
s to replace Go depsbazel_dep
s replace Go deps
@seh Sorry, the PR description was outdated, updated it to the description of the first commit. |
What's not clear to me yet is why one would express such a Bazel module dependency, as opposed to just letting the go.mod file express the dependency and rely on Are we getting more from that Bazel module than what Gazelle would generate on its own by reading the go.mod file? |
Having to use a Bazel module as a Go module should be rare, but there are important exceptions: When a Go module requires patches beyond Gazelle's capabilities. A The particular case of Another use case is for something like |
Thank you for the detailed explanation. The dependency landscape is more complex than I had considered. |
I've just tested this for rules_ll which uses that exact circl dependency as a transitive dependency. We were previously running into an issue where users could not only not import our go tooling, but couldn't import rules_ll at all because the go toolchain misconfiguration would block the entire build. Keeping patches like the ones required for circl maintained isn't nice if it requires custom deps and overrides. This patch fixes these issues in an IMO very elegant manner. So this will also fix #1543 in a very satisfactory way ❤️ |
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [bazel_gazelle](https://github.com/bazelbuild/bazel-gazelle) | http_archive | patch | `v0.31.0` -> `v0.31.1` | --- ### ⚠ Dependency Lookup Warnings ⚠ Warnings were logged while processing this repo. Please check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>bazelbuild/bazel-gazelle</summary> ### [`v0.31.1`](https://github.com/bazelbuild/bazel-gazelle/releases/tag/v0.31.1) [Compare Source](https://github.com/bazelbuild/bazel-gazelle/compare/v0.31.0...v0.31.1) #### What's Changed - point sync.Once in walkConfig by [@​jmhodges](https://github.com/jmhodges) in [https://github.com/bazelbuild/bazel-gazelle/pull/1532](https://github.com/bazelbuild/bazel-gazelle/pull/1532) - add copylock vet to nogo by [@​jmhodges](https://github.com/jmhodges) in [https://github.com/bazelbuild/bazel-gazelle/pull/1534](https://github.com/bazelbuild/bazel-gazelle/pull/1534) - bzlmod: Remove deprecated override attributes on `go_deps.module` by [@​fmeum](https://github.com/fmeum) in [https://github.com/bazelbuild/bazel-gazelle/pull/1548](https://github.com/bazelbuild/bazel-gazelle/pull/1548) - Add default directives for github.com/envoyproxy/protoc-gen-validate by [@​mortenmj](https://github.com/mortenmj) in [https://github.com/bazelbuild/bazel-gazelle/pull/1553](https://github.com/bazelbuild/bazel-gazelle/pull/1553) - cmd/gazelle: do not use the epoch as timestamp in diff output by [@​siddharthab](https://github.com/siddharthab) in [https://github.com/bazelbuild/bazel-gazelle/pull/1552](https://github.com/bazelbuild/bazel-gazelle/pull/1552) - fileinfo: fix not detecting 'unix' files to be OS specific by [@​sluongng](https://github.com/sluongng) in [https://github.com/bazelbuild/bazel-gazelle/pull/1554](https://github.com/bazelbuild/bazel-gazelle/pull/1554) - language/go: Emit apparent repo name of rules_go in select keys by [@​fmeum](https://github.com/fmeum) in [https://github.com/bazelbuild/bazel-gazelle/pull/1555](https://github.com/bazelbuild/bazel-gazelle/pull/1555) - Let `bazel_dep`s replace Go deps by [@​fmeum](https://github.com/fmeum) in [https://github.com/bazelbuild/bazel-gazelle/pull/1526](https://github.com/bazelbuild/bazel-gazelle/pull/1526) #### New Contributors - [@​mortenmj](https://github.com/mortenmj) made their first contribution in [https://github.com/bazelbuild/bazel-gazelle/pull/1553](https://github.com/bazelbuild/bazel-gazelle/pull/1553) **Full Changelog**: bazel-contrib/bazel-gazelle@v0.31.0...v0.31.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/kreempuff/rules_unreal_engine). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTAuMCIsInVwZGF0ZWRJblZlciI6IjM1LjExMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [bazel_gazelle](https://github.com/bazelbuild/bazel-gazelle) | http_archive | patch | `v0.31.0` -> `v0.31.1` | --- ### Release Notes <details> <summary>bazelbuild/bazel-gazelle</summary> ### [`v0.31.1`](https://github.com/bazelbuild/bazel-gazelle/releases/tag/v0.31.1) [Compare Source](https://github.com/bazelbuild/bazel-gazelle/compare/v0.31.0...v0.31.1) #### What's Changed - point sync.Once in walkConfig by [@​jmhodges](https://github.com/jmhodges) in [https://github.com/bazelbuild/bazel-gazelle/pull/1532](https://github.com/bazelbuild/bazel-gazelle/pull/1532) - add copylock vet to nogo by [@​jmhodges](https://github.com/jmhodges) in [https://github.com/bazelbuild/bazel-gazelle/pull/1534](https://github.com/bazelbuild/bazel-gazelle/pull/1534) - bzlmod: Remove deprecated override attributes on `go_deps.module` by [@​fmeum](https://github.com/fmeum) in [https://github.com/bazelbuild/bazel-gazelle/pull/1548](https://github.com/bazelbuild/bazel-gazelle/pull/1548) - Add default directives for github.com/envoyproxy/protoc-gen-validate by [@​mortenmj](https://github.com/mortenmj) in [https://github.com/bazelbuild/bazel-gazelle/pull/1553](https://github.com/bazelbuild/bazel-gazelle/pull/1553) - cmd/gazelle: do not use the epoch as timestamp in diff output by [@​siddharthab](https://github.com/siddharthab) in [https://github.com/bazelbuild/bazel-gazelle/pull/1552](https://github.com/bazelbuild/bazel-gazelle/pull/1552) - fileinfo: fix not detecting 'unix' files to be OS specific by [@​sluongng](https://github.com/sluongng) in [https://github.com/bazelbuild/bazel-gazelle/pull/1554](https://github.com/bazelbuild/bazel-gazelle/pull/1554) - language/go: Emit apparent repo name of rules_go in select keys by [@​fmeum](https://github.com/fmeum) in [https://github.com/bazelbuild/bazel-gazelle/pull/1555](https://github.com/bazelbuild/bazel-gazelle/pull/1555) - Let `bazel_dep`s replace Go deps by [@​fmeum](https://github.com/fmeum) in [https://github.com/bazelbuild/bazel-gazelle/pull/1526](https://github.com/bazelbuild/bazel-gazelle/pull/1526) #### New Contributors - [@​mortenmj](https://github.com/mortenmj) made their first contribution in [https://github.com/bazelbuild/bazel-gazelle/pull/1553](https://github.com/bazelbuild/bazel-gazelle/pull/1553) **Full Changelog**: bazel-contrib/bazel-gazelle@v0.31.0...v0.31.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/cgrindel/rules_swift_package_manager). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTAuMCIsInVwZGF0ZWRJblZlciI6IjM1LjExMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [bazel_gazelle](https://github.com/bazelbuild/bazel-gazelle) | http_archive | patch | `v0.31.0` -> `v0.31.1` | --- ### Release Notes <details> <summary>bazelbuild/bazel-gazelle</summary> ### [`v0.31.1`](https://github.com/bazelbuild/bazel-gazelle/releases/tag/v0.31.1) [Compare Source](https://github.com/bazelbuild/bazel-gazelle/compare/v0.31.0...v0.31.1) #### What's Changed - point sync.Once in walkConfig by [@​jmhodges](https://github.com/jmhodges) in [https://github.com/bazelbuild/bazel-gazelle/pull/1532](https://github.com/bazelbuild/bazel-gazelle/pull/1532) - add copylock vet to nogo by [@​jmhodges](https://github.com/jmhodges) in [https://github.com/bazelbuild/bazel-gazelle/pull/1534](https://github.com/bazelbuild/bazel-gazelle/pull/1534) - bzlmod: Remove deprecated override attributes on `go_deps.module` by [@​fmeum](https://github.com/fmeum) in [https://github.com/bazelbuild/bazel-gazelle/pull/1548](https://github.com/bazelbuild/bazel-gazelle/pull/1548) - Add default directives for github.com/envoyproxy/protoc-gen-validate by [@​mortenmj](https://github.com/mortenmj) in [https://github.com/bazelbuild/bazel-gazelle/pull/1553](https://github.com/bazelbuild/bazel-gazelle/pull/1553) - cmd/gazelle: do not use the epoch as timestamp in diff output by [@​siddharthab](https://github.com/siddharthab) in [https://github.com/bazelbuild/bazel-gazelle/pull/1552](https://github.com/bazelbuild/bazel-gazelle/pull/1552) - fileinfo: fix not detecting 'unix' files to be OS specific by [@​sluongng](https://github.com/sluongng) in [https://github.com/bazelbuild/bazel-gazelle/pull/1554](https://github.com/bazelbuild/bazel-gazelle/pull/1554) - language/go: Emit apparent repo name of rules_go in select keys by [@​fmeum](https://github.com/fmeum) in [https://github.com/bazelbuild/bazel-gazelle/pull/1555](https://github.com/bazelbuild/bazel-gazelle/pull/1555) - Let `bazel_dep`s replace Go deps by [@​fmeum](https://github.com/fmeum) in [https://github.com/bazelbuild/bazel-gazelle/pull/1526](https://github.com/bazelbuild/bazel-gazelle/pull/1526) #### New Contributors - [@​mortenmj](https://github.com/mortenmj) made their first contribution in [https://github.com/bazelbuild/bazel-gazelle/pull/1553](https://github.com/bazelbuild/bazel-gazelle/pull/1553) **Full Changelog**: bazel-contrib/bazel-gazelle@v0.31.0...v0.31.1 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/cgrindel/bazel-starlib). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTAuMCIsInVwZGF0ZWRJblZlciI6IjM1LjExMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
When a Go dependency is brought in both as a
bazel_dep
and as aregular Go dependency via e.g. go.mod, the
bazel_dep
repo should beused instead of creating a separate
go_repository
, which would riskduplicate packages.
This is realized by registering every Bazel module with a
go_deps.from_file
tag as providing the Go module of the same versionwith the module path given in
go.mod
.Gazelle is aware of these tags and generates either a canonical label or
a label using the apparent module repo name (the latter only for the
root module and only if it directly depends on the
bazel_dep
).Fixes #1525
Fixes #1543