Skip to content
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

Go/Bazel: fix gazelle invocation to use bundled bazel go #17145

Merged
merged 2 commits into from
Aug 6, 2024
Merged

Conversation

redsun82
Copy link
Contributor

@redsun82 redsun82 commented Aug 5, 2024

If I recall correctly, we were avoiding using the gazelle rule because of some compilation problem on macOS, which seems to be gone with the current versions of rules_go and/or gazelle.

This will make sure that gazelle uses the bundled go version: prior to this, the build through make was misbehaving if go was not installed.

@github-actions github-actions bot added the Go label Aug 5, 2024
@redsun82 redsun82 marked this pull request as ready for review August 5, 2024 12:30
@redsun82 redsun82 requested review from a team as code owners August 5, 2024 12:30
Copy link
Collaborator

@criemen criemen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@mbg mbg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Happy to sign off on this on behalf of the Go team, but I can't say I have much insight into the effects of these changes. I also don't think I have a way of testing this. It may be worth linking this PR in the issue that prompted this change and ask the user there to test it, to verify that this resolves the problem for them?

@redsun82
Copy link
Contributor Author

redsun82 commented Aug 5, 2024

Happy to sign off on this on behalf of the Go team, but I can't say I have much insight into the effects of these changes. I also don't think I have a way of testing this. It may be worth linking this PR in the issue that prompted this change and ask the user there to test it, to verify that this resolves the problem for them?

The way I tested this, is to do bazel clean --expunge from a codeql checkout, remove go from the system (in my case it was brew remove go). This was causing make to fail before this PR, and succeed after it. I will link this PR on the user ticket!

@redsun82 redsun82 merged commit 79740ed into main Aug 6, 2024
23 checks passed
@redsun82 redsun82 deleted the redsun82/go branch August 6, 2024 08:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants