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.

Sign up for GitHub

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

Jump to bottom

bazel: update golang and rules_go #17438

Closed
wants to merge 1 commit into from
Closed

bazel: update golang and rules_go #17438

wants to merge 1 commit into from

Conversation

keith
Copy link
Member

@keith keith commented Jul 21, 2021 •
edited by phlax
Loading

These updates are required for things to work nicely when building on
Apple Silicon machines targeting darwin_arm64.

Risk Level: Low

Fix #18339

@repokitteh-read-only
Copy link

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #17438 was opened by keith.

see: more, trace.

@repokitteh-read-only repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Jul 21, 2021
@repokitteh-read-only
Copy link

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).

🐱

Caused by: #17438 was opened by keith.

see: more, trace.

@keith keith force-pushed the ks/bazel-update-golang-and-rules_go branch 3 times, most recently from 81a85c2 to d24279e Compare July 21, 2021 18:48
@keith keith marked this pull request as ready for review July 21, 2021 19:59
@keith keith mentioned this pull request Jul 22, 2021
Closed
@keith keith force-pushed the ks/bazel-update-golang-and-rules_go branch from f168fd0 to 0ace424 Compare July 22, 2021 00:24
htuch
htuch previously approved these changes Jul 22, 2021
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@keith
Copy link
Member Author

keith commented Jul 22, 2021

hrm failure is related but not seeing any details in the log, will have to look later

@htuch htuch self-assigned this Jul 22, 2021
@moderation
Copy link
Contributor

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Jul 22, 2021
@keith keith force-pushed the ks/bazel-update-golang-and-rules_go branch from 0ace424 to f0c2c1f Compare August 4, 2021 23:08
@repokitteh-read-only repokitteh-read-only bot added deps Approval required for changes to Envoy's external dependencies and removed waiting labels Aug 4, 2021
@keith keith force-pushed the ks/bazel-update-golang-and-rules_go branch 2 times, most recently from acca0b7 to 7a59721 Compare August 5, 2021 00:23
@moderation
Copy link
Contributor

/lgtm deps

@repokitteh-read-only repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Aug 5, 2021
@htuch
Copy link
Member

htuch commented Aug 9, 2021

The Go build in linux_x64 api is still failing.
/wait

@keith
Copy link
Member Author

keith commented Aug 12, 2021

Hoping #17082 helps us out here

@github-actions
Copy link

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Sep 12, 2021
@keith
Copy link
Member Author

keith commented Sep 12, 2021

still blocked on the linked PR, but still important

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Sep 12, 2021
@keith
Copy link
Member Author

keith commented Sep 21, 2021

Oops that linked the issue, this is the actual PR #17460

@keith keith force-pushed the ks/bazel-update-golang-and-rules_go branch from 7a59721 to da44933 Compare September 21, 2021 22:32
@repokitteh-read-only repokitteh-read-only bot added deps Approval required for changes to Envoy's external dependencies and removed waiting labels Sep 21, 2021
@htuch
Copy link
Member

htuch commented Sep 22, 2021

Seems still broken on API build.
/wait

@keith keith mentioned this pull request Sep 22, 2021
Merged
aneeshusa
aneeshusa reviewed Sep 27, 2021
View reviewed changes
bazel/dependency_imports.bzl Outdated
@@ -11,7 +11,7 @@ load("@rules_antlr//antlr:deps.bzl", "antlr_dependencies")
load("@proxy_wasm_rust_sdk//bazel:dependencies.bzl", "proxy_wasm_rust_sdk_dependencies")

# go version for rules_go
GO_VERSION = "1.15.5"
GO_VERSION = "1.16.6"
Copy link

Choose a reason for hiding this comment

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

Go 1.17 is now out (latest is Go 1.17.1: https://golang.org/dl/), any chance we can try updating to that? Hopefully a drop-in in this PR but can make it a follow-up PR if needed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Typically the rules_go release updates were very close to Go releases. That seems to have stopped with 0.28.0

Copy link

Choose a reason for hiding this comment

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

FWIW, my read of bazel-contrib/rules_go#2729 is that it should no longer be required to update rules_go for every new Go release.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting. We we need to continue to set the Go version to get reproducible builds (and to stop a potential breakage as the Go version increment in the background). So the changes to rules_go 0.28.0 and bazel-gazelle to 0.23.0 LGTM. We should bump the Go version on line 14 to 1.17.1 (I've been running this setup locally since 1.17 came out

Copy link
Member Author

Choose a reason for hiding this comment

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

I did the minimum bump required to get M1 support at the time I submitted this PR just to keep this change as small as possible, happy to bump more, it will probably be fine

@moderation moderation mentioned this pull request Oct 6, 2021
Closed
@codefromthecrypt codefromthecrypt mentioned this pull request Oct 7, 2021
Closed
6 tasks
@keith keith marked this pull request as draft October 7, 2021 18:58
@phlax phlax mentioned this pull request Oct 8, 2021
Closed
@keith
bazel: update golang and rules_go
82e5eef 
These updates are required for things to work nicely when building on
Apple Silicon machines targeting darwin_arm64.

Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
@keith keith force-pushed the ks/bazel-update-golang-and-rules_go branch from da44933 to 82e5eef Compare November 1, 2021 22:29
@github-actions
Copy link

github-actions bot commented Dec 2, 2021

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@github-actions github-actions bot added the stale stalebot believes this issue/PR has not been touched recently label Dec 2, 2021
@remyleone
Copy link
Contributor

@keith any update on this?

@github-actions github-actions bot removed the stale stalebot believes this issue/PR has not been touched recently label Dec 2, 2021
@phlax phlax added the no stalebot Disables stalebot from closing an issue label Dec 2, 2021
@keith
Copy link
Member Author

keith commented Dec 2, 2021

It is now part of #17460 I just want to make sure that lands so we don't forget about it

@keith
Copy link
Member Author

keith commented Dec 3, 2021

#17460

@keith keith closed this Dec 3, 2021
@keith keith deleted the ks/bazel-update-golang-and-rules_go branch December 3, 2021 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aneeshusa aneeshusa aneeshusa left review comments

@phlax phlax phlax left review comments

@moderation moderation moderation left review comments

@htuch htuch htuch left review comments

Assignees

@htuch htuch

Labels
deps Approval required for changes to Envoy's external dependencies no stalebot Disables stalebot from closing an issue
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Newer release available io_bazel_rules_go: v0.28.0
7 participants
@keith @moderation @htuch @remyleone @phlax @aneeshusa @mattklein123