-
Notifications
You must be signed in to change notification settings - Fork 21
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
[Bug]: updating go deps broken with bzlmod #760
Comments
I will try to create a PR next week if I find some time. Do you know how to deduce the |
I can also add that this is not a problem with bzlmod and rules_go when you use # MODULE.bazel
go_deps = use_extension("@gazelle//:extensions.bzl", "go_deps")
go_deps.from_file(go_mod = "//:go.mod") |
Hmm, this is more complicated than I first thought. Does
|
Here is a repo to reproduce the slow performance behavior. The results of ❯ time bzl configure
Updating BUILD files for go
2 BUILD files visited
0 BUILD files updated
bazelisk configure 18.23s user 3.18s system 140% cpu 15.257 total
❯ time bzl run gazelle
INFO: Analyzed target //:gazelle (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //:gazelle up-to-date:
bazel-bin/gazelle-runner.bash
bazel-bin/gazelle
INFO: Elapsed time: 0.073s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/gazelle
bazelisk run gazelle 0.10s user 0.13s system 88% cpu 0.258 total |
Hmm, is this really fixed? 🤔 |
It is fixed in the aspect private repo. If you're using the cli from this repo you'll have to wait till the change gets pushed here. Sorry I guess that's confusing when it gets closed from the private repo :/ |
Aha, I understand. I'm just glad to hear it's fixed. Thanks!
…On Wed, 6 Nov 2024, 20:24 Jason Bedard, ***@***.***> wrote:
It is fixed in the aspect private repo. If you're using the cli from this
repo you'll have to wait till the change gets pushed here. Sorry I guess
that's confusing when it gets closed from the private repo :/
—
Reply to this email directly, view it on GitHub
<#760 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADM3WLVSJ74FVWZU6MBBMODZ7JUFHAVCNFSM6AAAAABQGW54IKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINRQGU4TGMBRHE>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
The rules_go gazelle integration changed in bzlmod such that it now requires resolving the [bazel_gazelle_go_repository_config](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/internal/bzlmod/go_deps.bzl#L653-L672) repository which contains a cache-able list of the `go_repository` target data (not the actual targets, just data about them). With regular gazelle from a `gazelle_binary` target the `@bazel_gazelle_go_repository_config` repo path is resolved using [a rule label attribute](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/def.bzl#L101-L104) and [this hack](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/internal/go_repository.bzl#L313-L321). However when we are running gazelle within `aspect configure` we need an alternate way of resolving this path. This PR does something similar to the [the go_repository hack](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/internal/go_repository.bzl#L313-L321), except we must launch `bazel info output_base` to get the external directory. Fix #760 --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes The go language in `aspect configure` will now work when rules_go is under bzlmod. ### Test plan - Manual testing; please provide instructions so we can reproduce: run the cli binary on a repo where rules_go is loaded with bzlmod, for example https://github.com/r0bobo/aspect-cli-issue-760-reproduce GitOrigin-RevId: aded45cc9e6566447383b193a201255e55e90730
The rules_go gazelle integration changed in bzlmod such that it now requires resolving the [bazel_gazelle_go_repository_config](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/internal/bzlmod/go_deps.bzl#L653-L672) repository which contains a cache-able list of the `go_repository` target data (not the actual targets, just data about them). With regular gazelle from a `gazelle_binary` target the `@bazel_gazelle_go_repository_config` repo path is resolved using [a rule label attribute](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/def.bzl#L101-L104) and [this hack](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/internal/go_repository.bzl#L313-L321). However when we are running gazelle within `aspect configure` we need an alternate way of resolving this path. This PR does something similar to the [the go_repository hack](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/internal/go_repository.bzl#L313-L321), except we must launch `bazel info output_base` to get the external directory. Fix #760 --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes The go language in `aspect configure` will now work when rules_go is under bzlmod. ### Test plan - Manual testing; please provide instructions so we can reproduce: run the cli binary on a repo where rules_go is loaded with bzlmod, for example https://github.com/r0bobo/aspect-cli-issue-760-reproduce GitOrigin-RevId: aded45cc9e6566447383b193a201255e55e90730
The rules_go gazelle integration changed in bzlmod such that it now requires resolving the [bazel_gazelle_go_repository_config](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/internal/bzlmod/go_deps.bzl#L653-L672) repository which contains a cache-able list of the `go_repository` target data (not the actual targets, just data about them). With regular gazelle from a `gazelle_binary` target the `@bazel_gazelle_go_repository_config` repo path is resolved using [a rule label attribute](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/def.bzl#L101-L104) and [this hack](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/internal/go_repository.bzl#L313-L321). However when we are running gazelle within `aspect configure` we need an alternate way of resolving this path. This PR does something similar to the [the go_repository hack](https://github.com/bazel-contrib/bazel-gazelle/blob/v0.39.1/internal/go_repository.bzl#L313-L321), except we must launch `bazel info output_base` to get the external directory. Fix #760 --- ### Changes are visible to end-users: yes - Searched for relevant documentation and updated as needed: yes - Breaking change (forces users to change their own code or config): no - Suggested release notes appear below: yes The go language in `aspect configure` will now work when rules_go is under bzlmod. ### Test plan - Manual testing; please provide instructions so we can reproduce: run the cli binary on a repo where rules_go is loaded with bzlmod, for example https://github.com/r0bobo/aspect-cli-issue-760-reproduce GitOrigin-RevId: aded45cc9e6566447383b193a201255e55e90730
This will be in the next release which is currently publishing |
What happened?
Crashes or really slow when updating go deps in bzlmod.
Version
Development (host) and target OS/architectures:
Output of
bazel --version
:Version of the Aspect rules, or other relevant rules from your
WORKSPACE
orMODULE.bazel
file:Language(s) and/or frameworks involved:
How to reproduce
Any other information?
See fix in gazelle sh script: https://github.com/bazel-contrib/bazel-gazelle/blob/b160ccd46fb7c19a49530d08906165c73c343dd1/internal/gazelle.bash.in#L101-L106
The text was updated successfully, but these errors were encountered: