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: prep for Go 1.22 by using GOEXPERIMENT=loopvar for tests #546

Merged
merged 3 commits into from
Oct 4, 2023

Conversation

Roasbeef
Copy link
Member

@Roasbeef Roasbeef commented Oct 4, 2023

In this commit, we prep for some upcoming changes in
Go
by running our tests with
GOEXPERIMENT=loopvar. This changes the loop semantics to fix a common
bug where a scoping issue causes a variable to be re-used, which can
cause unintended bugs down the line.

If everything passes with this flag on, then we can keep it on, and also
rest a bit easier knowing that the compiler will fix a class of bugs
that would previously pop up for us.

@guggero
Copy link
Member

guggero commented Oct 4, 2023

go: go.mod file indicates go 1.21, but maximum version supported by tidy is 1.20

I think we need to use an explicit version of Go in the mod-check job in .github/workflows/main.yaml, as the default of the GitHub Actions runner seems to be v1.20.
Adding this should fix:

      - name: Setup go environment
        uses: ./.github/actions/setup-go

@Roasbeef
Copy link
Member Author

Roasbeef commented Oct 4, 2023

Updated with that. Will land this if CI green, one less thing to have to think about when writing+reviewing code.

This is needed so we can use `GOEXPERIMENT=loopvar`.
In this commit, [we prep for some upcoming changes in
Go](https://go.dev/blog/loopvar-preview) by running our tests with
`GOEXPERIMENT=loopvar`. This changes the loop semantics to fix a common
bug where a scoping issue causes a variable to be re-used, which can
cause unintended bugs down the line.

If everything passes with this flag on, then we can keep it on, and also
rest a bit easier knowing that the compiler will fix a class of bugs
that would previously pop up for us.
The prior commit made sure that no unit tests failed with the new logic
applied. We'll also update our build+install commands so all the
binaries are built with the new loop semantics.
@Roasbeef Roasbeef merged commit 5373ccc into lightninglabs:main Oct 4, 2023
12 of 13 checks passed
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.

None yet

2 participants