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

Adopt Go toolchains for language-version-control #6063

Merged
merged 4 commits into from
May 28, 2024

Conversation

Groxx
Copy link
Member

@Groxx Groxx commented May 26, 2024

Go 1.21 brought us "toolchains":
We can now describe the version we want, and the Go CLI will automagically download, install, and use that version, with no extra effort. It should give us a MUCH more reliable way to ensure identical builds and formats and whatnot, and all us devs will auto-switch as we jump between git SHAs.

So let's use that!

This does a few things:

  • requires Go 1.21+ to be your go version to work on Cadence.
    • this does not force our users to use 1.21+, that's still 1.20, controlled by the go 1.20 lines. just development in this workspace.
  • sets a go.work toolchain (if there's a go.work file, toolchains in individual go.mod files are ignored)
    • not all IDEs follow this. we could set it + lint it in the go.mod files too if it helps, it just won't have any effect on CLI go use so it may be misleading.
  • exports GOTOOLCHAIN=... inside the Makefile, so ALL makefile-driven builds by everyone use exactly the same version (you can override this with an explicit GOTOOLCHAIN)
  • updates our dockerfiles (it would just download [wrong version in docker image] and then auto-download the right one, which is a waste of time)
  • adds a lint script to ensure ^ all that stays up to date

For future upgrades, upgrading the toolchain should be very simple:

  • change go.work to a new toolchain version
    • everything now uses the new language version, tools and binaries will rebuild, etc.
  • make lint or make pr or CI will tell you to change the Dockerfiles / other locations, if you don't git grep -F 1.22.3 to find and update those first.

And temporarily trying a different version to check a bug / performance / try the new version is:

  • GOTOOLCHAIN=go1.23.4 make whatever and it'll do exactly that (except for the version check in make lint)
  • you may want to make clean as well, to rebuild tools if that's relevant
    • they will not automatically rebuild with this env var changing. they could, if we used .build/go1.23.4/etc folders, if we want that.

@Groxx
Copy link
Member Author

Groxx commented May 27, 2024

❯ GOTOOLCHAIN=auto make
Makefile:46: warning: your Go toolchain is explicitly set to GOTOOLCHAIN=auto, ignoring go.work version go1.22.3...

which seems pretty good.

CI has a bunch of lines like

Makefile:46: warning: your Go toolchain is explicitly set to GOTOOLCHAIN=local, ignoring go.work version go1.22.3...

which could be handled more quietly by querying go version... but that'd potentially force a download before it responds, which could cause a big surprising lag if it happened on a dev's machine.

but maybe. or maybe there's a different, better option somewhere. not sure at the moment.

Copy link

codecov bot commented May 27, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.01%. Comparing base (2f08de5) to head (aedc290).

Additional details and impacted files

see 8 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f08de5...aedc290. Read the comment docs.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 018fb74e-8f78-4b91-acc2-561eb11fc052

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 46 unchanged lines in 9 files lost coverage.
  • Overall coverage decreased (-0.02%) to 69.503%

Files with Coverage Reduction New Missed Lines %
service/history/queue/timer_queue_processor_base.go 1 77.66%
common/cache/lru.go 2 92.89%
service/history/replication/task_processor.go 2 81.25%
common/archiver/filestore/historyArchiver.go 4 80.95%
service/matching/tasklist/task_list_manager.go 4 76.09%
service/frontend/api/handler.go 6 62.2%
service/history/task/transfer_standby_task_executor.go 9 85.8%
service/history/engine/engineimpl/poll_mutable_state.go 9 74.16%
service/frontend/wrappers/metered/metered.go 9 63.18%
Totals Coverage Status
Change from base Build 018facb4-1556-4d0a-a60a-2fec545c95e9: -0.02%
Covered Lines: 102502
Relevant Lines: 147479

💛 - Coveralls

@Groxx Groxx changed the title Lock down a single Go version for everything, forever Lock down a single Go version for everything May 27, 2024
@Groxx Groxx changed the title Lock down a single Go version for everything Adopt Go toolchains for language-version-control May 27, 2024
@Groxx Groxx merged commit 5f45da5 into cadence-workflow:master May 28, 2024
20 checks passed
@Groxx Groxx deleted the gotoolchain branch May 28, 2024 19:21
timl3136 pushed a commit to timl3136/cadence that referenced this pull request Jun 6, 2024
Go 1.21 brought us "toolchains":
We can now describe the version we want, and the Go CLI will automagically download, install, and use that version, with no extra effort.  It should give us a MUCH more reliable way to ensure identical builds and formats and whatnot, and all us devs will auto-switch as we jump between git SHAs.

So let's use that!

This does a few things:
- requires Go 1.21+ to be your `go` version to *work on* Cadence.
  - this does not force our *users* to use 1.21+, that's still 1.20, controlled by the `go 1.20` lines.  just development in this workspace.
- sets a `go.work` toolchain (if there's a `go.work` file, toolchains in individual `go.mod` files are ignored)
  - not all IDEs follow this.  we could set it + lint it in the `go.mod` files too if it helps, it just won't have any effect on CLI `go` use so it may be misleading.
- exports `GOTOOLCHAIN=...` inside the Makefile, so ALL makefile-driven builds by everyone use exactly the same version (you can override this with an explicit `GOTOOLCHAIN`)
- updates our dockerfiles (it would just download [wrong version in docker image] and then auto-download the right one, which is a waste of time)
- adds a lint script to ensure ^ all that stays up to date

---

For future upgrades, upgrading the toolchain should be very simple:
- change `go.work` to a new toolchain version
  - everything now uses the new language version, tools and binaries will rebuild, etc.
- `make lint` or `make pr` or CI will tell you to change the Dockerfiles / other locations, if you don't `git grep -F 1.22.3` to find and update those first.

And temporarily trying a different version to check a bug / performance / try the new version is:
- `GOTOOLCHAIN=go1.23.4 make whatever` and it'll do exactly that (except for the version check in `make lint`)
- you may want to `make clean` as well, to rebuild tools if that's relevant
  - they will not automatically rebuild with this env var changing.  they *could*, if we used `.build/go1.23.4/etc` folders, if we want that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants