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

multi: update to go 1.21 #675

Merged
merged 1 commit into from
Nov 3, 2023
Merged

multi: update to go 1.21 #675

merged 1 commit into from
Nov 3, 2023

Conversation

jamaljsr
Copy link
Member

@jamaljsr jamaljsr commented Oct 31, 2023

I needed to update to go v1.21 to build tapd. Now when building litd, it results in the error:

 Installing lightning-terminal.
go install -v -trimpath -tags="litd autopilotrpc signrpc walletrpc chainrpc invoicesrpc watchtowerrpc neutrinorpc peersrpc" -ldflags " -X github.com/lightningnetwork/lnd/build.Commit=lightning-terminal-v0.12.0-alpha-6-gd782cbbfef73145c992e034e3641cef9bcc0f135 -X github.com/lightningnetwork/lnd/build.CommitHash=d782cbbfef73145c992e034e3641cef9bcc0f135 -X github.com/lightningnetwork/lnd/build.GoVersion= -X github.com/lightningnetwork/lnd/build.RawTags=litd,autopilotrpc,signrpc,walletrpc,chainrpc,invoicesrpc,watchtowerrpc,neutrinorpc,peersrpc -X github.com/lightninglabs/lightning-terminal.appFilesPrefix= -X github.com/lightninglabs/lightning-terminal.Commit=v0.12.0-alpha-6-gd782cbbfef73145c992e034e3641cef9bcc0f135 -X github.com/lightninglabs/loop.Commit=v0.26.4-beta -X github.com/lightninglabs/pool.Commit=v0.6.4-beta.0.20231003174306-80d8854a0c4b -X github.com/lightninglabs/taproot-assets.Commit=v0.3.0" github.com/lightninglabs/lightning-terminal/cmd/litd
go: updates to go.mod needed; to update it:
	go mod tidy
make: *** [Makefile:131: go-install] Error 1

When running go mod tidy, it produced the diff here in this PR. I also updated the Docker and GH workflow files to use go v1.21 as well.

I have tested this branch while using go v1.19 and it still works, so this is a backward compatible change.

@jamaljsr jamaljsr requested review from ViktorTigerstrom, guggero and bitromortac and removed request for guggero October 31, 2023 15:38
Copy link
Contributor

@bitromortac bitromortac left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 (Edit: just came to my mind whether we want to wait until all subdaemons have been upgraded to 1.21, to let their unit/integration tests pass)

I used go v1.21.3 to run go mod tidy and it added a line toolchain go1.21.3 which enforces a minimal version of go (see https://go.dev/blog/toolchain). However, starting from a mod file that already contains go 1.21 but not the toolchain line, the toolchain line is not added with go mod tidy, so it is optional. Since the taproot assets repo also doesn't have it, I think it's fine.

@jamaljsr
Copy link
Member Author

jamaljsr commented Nov 1, 2023

just came to my mind whether we want to wait until all subdaemons have been upgraded to 1.21, to let their unit/integration tests pass

I didn't mention this initially but the immediate need for this fix is because the cron job that updates the API Docs site daily has been failing for over a week now due to this issue. Unless there's a compatibility issue with the other sub-daemons, it would be better to not wait.

@bitromortac
Copy link
Contributor

Just looked at the lnd unit/itests, they run with go 1.21, so I think it should be fine

Copy link
Contributor

@ViktorTigerstrom ViktorTigerstrom left a comment

Choose a reason for hiding this comment

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

LGTM after rebase!

@jamaljsr
Copy link
Member Author

jamaljsr commented Nov 3, 2023

Rebased. Merging now.

@jamaljsr jamaljsr merged commit 6e66894 into master Nov 3, 2023
12 checks passed
@jamaljsr jamaljsr deleted the go-v1.21-bump branch November 3, 2023 20:48
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