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

build: Bump golang to 1.17.9/Mac M1 Support #3919

Merged
merged 6 commits into from
May 10, 2022

Conversation

Eric-Warehime
Copy link
Contributor

Summary

Bump go version to 1.17.9 (latest 1.17 version).
It's good to upgrade compiler version regularly, but we also need this for #3808

Also includes //+build => //go:build generated via make sanity.

Blocking merge on #3909 / #3917 which contains go1.16 version bump.

Test Plan

Run CI/Perf tests to benchmark against current version.

go.mod Outdated Show resolved Hide resolved
@Eric-Warehime
Copy link
Contributor Author

Additional commit modifies node.mu to a RWLock and adds RLock()/RUnlock() calls to fix failures caused by the data race detector found in node.go.

node/node.go Show resolved Hide resolved
Makefile Show resolved Hide resolved
@Eric-Warehime Eric-Warehime changed the title build: Bump golang to 1.17.9 build: Bump golang to 1.17.9/Mac M1 Support May 4, 2022
@Eric-Warehime Eric-Warehime force-pushed the golang-1.17-upgrade branch from 2e83ebb to da3fa3c Compare May 5, 2022 14:18
cce added a commit to cce/go-algorand that referenced this pull request May 5, 2022
cce added a commit to cce/go-algorand that referenced this pull request May 5, 2022
@@ -407,11 +407,11 @@ func (node *AlgorandFullNode) startMonitoringRoutines() {

// PKI TODO: Remove this with #2596
// Periodically check for new participation keys
go node.checkForParticipationKeys()
go node.checkForParticipationKeys(node.ctx.Done())
Copy link
Contributor

Choose a reason for hiding this comment

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

@AlgoStephenAkiki fyi - there is going to be at least this merge conflict with your integration branch.

@@ -1,6 +1,6 @@
module github.com/algorand/go-algorand/scripts/buildtools

go 1.16
go 1.17

require (
github.com/algorand/msgp v1.1.50
Copy link
Contributor

Choose a reason for hiding this comment

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

This is where you'll update msgp after your other PR is merged and tagged

Copy link
Contributor

Choose a reason for hiding this comment

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

@Eric-Warehime please update the version and I'll merge this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Also had to run go get and go mod tidy which ended up separating deps into separate require blocks. See https://go-review.googlesource.com/c/go/+/325922/ and golang/go#45965 for details.

Copy link
Contributor

@winder winder left a comment

Choose a reason for hiding this comment

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

This looks ready to go once your msgp PR is merged and available.

cce
cce previously approved these changes May 9, 2022
algorandskiy
algorandskiy previously approved these changes May 9, 2022
@Eric-Warehime Eric-Warehime dismissed stale reviews from algorandskiy and cce via ab5fb9d May 9, 2022 22:07
@algorandskiy algorandskiy merged commit 5925aff into algorand:master May 10, 2022
@Eric-Warehime Eric-Warehime deleted the golang-1.17-upgrade branch May 10, 2022 01:27
algorandskiy pushed a commit that referenced this pull request May 17, 2022
This should ensure make msgp has been run for changes
that impact msgp serialization on CI builds.

It looks like #3829 was merged but missed changes to agreement/msgp_gen.go
and gci updates from algorand/msgp#14 were not incorporated into #3919.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

M1 Support: update configure_dev.sh
6 participants