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

finish off Go 1.22 upgrade #926

Merged
merged 1 commit into from
Jul 18, 2024
Merged

finish off Go 1.22 upgrade #926

merged 1 commit into from
Jul 18, 2024

Conversation

jmhodges
Copy link
Contributor

@jmhodges jmhodges commented Jul 18, 2024

This patch cleans up some stragglers in the CircleCI config that were
still running on 1.19.

We also change go.mod to include the patch portion of the Go version.
Including x.y.z, not just x.y is what the Go 1.22 tooling (and, I
believe 1.21) does and expects. If we don't include the patch version,
various third party tools like GitHub's CodeQL can break (see
github/codeql#15647 for the last time I ran
into this). This go.mod version change is actually what led me to the
other missing CircleCI version bumps as changing go.mod broke those
CircleCI jobs.

Along the way, we remove some dependency installations from the linting
CircleCI job. While future linting maybe need these dependencies, the
current ones do not.

I've made AUT-158 to consolidate our go linting stuff and avoid some of
these problems. Some (not all) of the rationale for AUT-158 will be
mooted by a move to GitHub Actions.

Along the way, I also ran go mod tidy which correctly noted that the
the google/uuid dep is now a direct dep. Hilariously, this is the change
I did first.

@jmhodges jmhodges requested review from a team as code owners July 18, 2024 10:29
@jmhodges jmhodges force-pushed the finish-golang-1.22 branch 2 times, most recently from 0ffdbcc to e6c630f Compare July 18, 2024 10:37
@jmhodges jmhodges requested review from bhearsum and oskirby July 18, 2024 10:37
@jmhodges jmhodges force-pushed the finish-golang-1.22 branch 5 times, most recently from 0e2f46a to 23cb500 Compare July 18, 2024 10:48
bhearsum
bhearsum previously approved these changes Jul 18, 2024
Copy link
Contributor

@bhearsum bhearsum left a comment

Choose a reason for hiding this comment

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

Looks reasonable. Sorry I missed this stuff :(.

I'm happy to help overall/improve the CI situation as well.

This patch cleans up some stragglers in the CircleCI config that were
still running on 1.19.

We also change `go.mod` to include the patch portion of the Go version.
Including `x.y.z`, not just `x.y` is what the Go 1.22 tooling (and, I
believe 1.21) does and expects. If we don't include the patch version,
various third party tools like GitHub's CodeQL can break (see
github/codeql#15647 for the last time I ran
into this). This go.mod version change is actually what led me to the
other missing CircleCI version bumps as changing go.mod broke those
CircleCI jobs.

Along the way, we remove some dependency installations from the linting
CircleCI job. While future linting maybe need these dependencies, the
current ones do not.

I've made AUT-158 to consolidate our go linting stuff and avoid some of
these problems. Some (not all) of the rationale for AUT-158 will be
mooted by a move to GitHub Actions.

Along the way, I also ran `go mod tidy` which correctly noted that the
the google/uuid dep is now a direct dep. Hilariously, this is the change
I did first.

do less work in the lint-vet-fmt job
@jmhodges
Copy link
Contributor Author

Sorry to request the re-review, but I realized we could just not install deps at all in the linting CircleCI job. Have simplified the code and corrected the PR summary and commit message.

@bhearsum
Copy link
Contributor

Sorry to request the re-review, but I realized we could just not install deps at all in the linting CircleCI job. Have simplified the code and corrected the PR summary and commit message.

Can you clarify why this is?

@jmhodges
Copy link
Contributor Author

jmhodges commented Jul 18, 2024

Sorry to request the re-review, but I realized we could just not install deps at all in the linting CircleCI job. Have simplified the code and corrected the PR summary and commit message.

Can you clarify why this is?

The apt dependencies that were being installed aren't actually being used in any of the linting processes, so they're irrelevant. Once you remove those, the failures of apt-get in the cimg/go:1.22 images stopped mattering because we weren't needing to run it.

@jmhodges
Copy link
Contributor Author

jmhodges commented Jul 18, 2024

Specifically, the errors are here:

Reading package lists... Done
E: List directory /var/lib/apt/lists/partial is missing. - Acquire (13: Permission denied)

Exited with code exit status 100

Note the commit they're on was when I flipped from bookworm to the go CircleCI image.

Copy link
Contributor

@bhearsum bhearsum left a comment

Choose a reason for hiding this comment

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

Thanks for the additional info!

@jmhodges jmhodges merged commit fc71e79 into main Jul 18, 2024
15 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.

2 participants