*: upgrade to go 1.19.1 by rickystewart · Pull Request #86049 · cockroachdb/cockroach

rickystewart · 2022-08-12T17:09:12Z

Closes #85908.

Release justification: Upgrade to latest Go version
Release note (build change): upgrade to go 1.19.1

cockroach-teamcity · 2022-08-12T17:09:21Z

This change is

rickystewart · 2022-08-12T22:50:27Z

gofmt has new behavior in 1.19 where doc comments are formatted, leading to the very big diff.

This breaks go-swagger which we depend on for some generated documentation: go-swagger/go-swagger#2759

ajwerner · 2022-08-12T23:49:13Z

gofmt has new behavior in 1.19 where doc comments are formatted, leading to the very big diff.

Where we do have to use it? Do we have to use go1.19's gofmt? Given we're leaving go.mod at 1.17, I don't think we have to worry about older gofmt not working.

rickystewart · 2022-08-15T16:19:42Z

Do we have to use go1.19's gofmt? Given we're leaving go.mod at 1.17, I don't think we have to worry about older gofmt not working.

gofmt is part of the Go distribution (when you get a new go, you automatically get a new gofmt) so just using a different one is easier said than done. We could conceivably use patched versions of the Go archives and just keep the 1.18 version of gofmt in our custom SDK's, but that's a bit of a high-friction fix and I think go-swagger will have a fix upstream relatively quickly.

Either way when developers upgrade their local go versions, they're going to get gofmt 1.19, and I don't think it's in our best interest if the latest gofmt disagrees with what CI wants the formatting to be.

rickystewart · 2022-09-06T18:11:23Z

It looks like gofmt and dev generate go still disagree about pkg/sql/colexec/and_or_projection.eg.go. @knz Can you look into this?

knz · 2022-09-06T18:26:15Z

here you are #87442

knz · 2022-09-06T18:26:29Z

feel free to cherry pick btw

rickystewart · 2022-09-06T19:03:19Z

This is pretty close, can probably be merged soon.

@knz Looks like that file is still problematic even after the cherry-pick?

rickystewart · 2022-09-06T23:14:54Z

Looks like that file is still problematic even after the cherry-pick?

Oh, maybe I messed up something with the rebase. I'm trying again now.

87442: colexec: remove some empty comment lines r=yuzefovich a=knz For #86049. Release justification: non-code changes Release note: None Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>

knz · 2022-09-07T09:59:53Z

You will probably need #87485 to get over the remaining linter hurdle.

Then for the remaining test failure you need to update the expected test output (I think it's dev test //sql/opt/exec/execbuilder --rewrite)

rickystewart · 2022-09-07T15:44:41Z

You will probably need #87485 to get over the remaining linter hurdle.

I pulled the changes into this commit, except for the uses of ioutil I couldn't get rid of easily, for which I added linter exclusions.

AFAICT this is ready for review now.

knz · 2022-09-07T16:24:16Z

i can spend time on the detailed review but with focused chaos it will likely spread over 2-3 days.

rickystewart · 2022-09-07T16:29:06Z

i can spend time on the detailed review but with focused chaos it will likely spread over 2-3 days.

I'll check if there's someone on the team who has time to review.

mari-crl

Sanity check completed!
A lot of ASCII art in our codebase... Some of it seems like it may have gotten misaligned, but I don't think that's a fix to be made here. Get it in there!

Reviewed 766 of 789 files at r11, 24 of 24 files at r13, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @HonoreDB, @msbutler, and @srosenberg)

knz · 2022-09-07T16:59:17Z

Btw 1.19.1 was just released with applicable security patches. IMHO it would be useful to have this done here right away.

rickystewart · 2022-09-07T16:59:30Z

TFTR

YOLO

bors r=mari-crl p=99

craig · 2022-09-07T16:59:32Z

👎 Rejected by label

knz

I'm ok doing a post-hoc review on this.

Reviewed 7 of 789 files at r11.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @HonoreDB, @msbutler, and @srosenberg)

* [ ] Adjust the Pebble tests to run in new version. * [x] Adjust version in Docker image ([source](./builder/Dockerfile)). * [x] Adjust version in the TeamCity agent image ([setup script](./packer/teamcity-agent.sh)) * [x] Rebuild and push the Docker image (following [Basic Process](#basic-process)) * [x] Update `build/teamcity/internal/release/build-and-publish-patched-go/impl.sh` with the new version and adjust SHA256 sums as necessary. * [x] Run the `Internal / Release / Build and Publish Patched Go` build configuration in TeamCity with your latest version of the script above. This will print out the new URL's and SHA256 sums for the patched Go that you built above. * [x] Bump the version in `WORKSPACE` under `go_download_sdk`. You may need to bump [rules_go](https://github.com/bazelbuild/rules_go/releases). Also edit the filenames listed in `sdks` and update all the hashes to match what you built in the step above. * [x] Run `./dev generate bazel` to refresh `distdir_files.bzl`, then `bazel fetch @distdir//:archives` to ensure you've updated all hashes to the correct value. * [x] Bump the version in `builder.sh` accordingly ([source](./builder.sh#L6)). * [x] Bump the version in `go-version-check.sh` ([source](./go-version-check.sh)), unless bumping to a new patch release. * [ ] Bump the go version in `go.mod`. You may also need to rerun `make vendor_rebuild` if vendoring has changed. * [x] Bump the default installed version of Go in `bootstrap-debian.sh` ([source](./bootstrap/bootstrap-debian.sh)). * [x] Replace other mentions of the older version of go (grep for `golang:<old_version>` and `go<old_version>`). * [ ] Update the `builder.dockerImage` parameter in the TeamCity [`Cockroach`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Cockroach&tab=projectParams) and [`Internal`](https://teamcity.cockroachdb.com/admin/editProject.html?projectId=Internal&tab=projectParams) projects. * [ ] Ask the Developer Infrastructure team to deploy new TeamCity agent images according to [packer/README.md](./packer/README.md) Closes #85908. Release justification: Upgrade to latest Go version Release note (build change): upgrade to go 1.19.1

rickystewart · 2022-09-07T18:28:43Z

bors r=mari-crl,knz p=99

craig · 2022-09-07T19:17:33Z

Build succeeded:

Bazel Essential CI (Cockroach)

124674: build: remove `swagger` r=rail a=rickystewart We stopped generating the Swagger documentation 2 years ago in #86049 to allow us to upgrade to Go 1.19. A year later, in #105291, we then deleted the stale tree. To date no one has complained or even noticed this documentation missing, we've moved on to Go 1.22, and we've had 2 years of releases. Time to just remove support for Swagger altogether, including the build dependency and the `go:swagger` directives. I kept the "structured" documentation comments. It may or may not be out-of-date, but it doesn't seem to be hurting anyone. Epic: none Release note: None Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>

rickystewart added the do-not-merge bors won't merge a PR with this label. label Aug 12, 2022

rickystewart requested a review from a team as a code owner August 12, 2022 17:09

rickystewart requested a review from a team August 12, 2022 17:09

rickystewart requested a review from a team as a code owner August 12, 2022 17:09

rickystewart requested review from srosenberg and removed request for a team August 12, 2022 17:09

rickystewart requested a review from a team August 12, 2022 21:20

rickystewart requested review from a team as code owners August 12, 2022 21:20

rickystewart requested a review from a team August 12, 2022 21:20

rickystewart requested a review from a team as a code owner August 12, 2022 21:20

rickystewart requested a review from a team August 12, 2022 21:20

rickystewart requested review from a team as code owners August 12, 2022 21:20

rickystewart requested a review from a team August 12, 2022 21:20

rickystewart requested review from a team as code owners August 12, 2022 21:20

rickystewart requested review from a team, HonoreDB and msbutler and removed request for a team August 12, 2022 21:20

dhartunian removed the request for review from a team August 15, 2022 14:06

knz mentioned this pull request Sep 6, 2022

bazel: nogo build fails on upstream dependencies containing go 1.18 templates #87420

Open

knz mentioned this pull request Sep 6, 2022

colexec: remove some empty comment lines #87442

Merged

craig Bot pushed a commit that referenced this pull request Sep 7, 2022

Merge #87442

2677a7d

87442: colexec: remove some empty comment lines r=yuzefovich a=knz For #86049. Release justification: non-code changes Release note: None Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net>

knz mentioned this pull request Sep 7, 2022

*: remove remaining stray uses of ioutil #87485

Closed

mari-crl approved these changes Sep 7, 2022

View reviewed changes

rickystewart removed the do-not-merge bors won't merge a PR with this label. label Sep 7, 2022

knz approved these changes Sep 7, 2022

View reviewed changes

knz changed the title *: upgrade to go 1.19 *: upgrade to go 1.19.1 Sep 7, 2022

craig Bot merged commit 018e180 into cockroachdb:master Sep 7, 2022

cockroach-teamcity mentioned this pull request Sep 7, 2022

PR #86049 - *: upgrade to go 1.19.1 cockroachdb/docs#15023

Closed

rickystewart mentioned this pull request Sep 8, 2022

docs: re-enable generated swagger documentation #87653

Closed

erikgrinaker mentioned this pull request Sep 16, 2022

roachperf: regression around 2022-09-09 [Go 1.19] #88038

Closed

rickystewart mentioned this pull request May 24, 2024

build: remove swagger #124674

Merged

Uh oh!

Conversation

rickystewart commented Aug 12, 2022 • edited by RaduBerinde Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cockroach-teamcity commented Aug 12, 2022

Uh oh!

rickystewart commented Aug 12, 2022

Uh oh!

ajwerner commented Aug 12, 2022

Uh oh!

rickystewart commented Aug 15, 2022

Uh oh!

rickystewart commented Sep 6, 2022

Uh oh!

knz commented Sep 6, 2022

Uh oh!

knz commented Sep 6, 2022

Uh oh!

rickystewart commented Sep 6, 2022

Uh oh!

rickystewart commented Sep 6, 2022

Uh oh!

knz commented Sep 7, 2022

Uh oh!

rickystewart commented Sep 7, 2022

Uh oh!

knz commented Sep 7, 2022

Uh oh!

rickystewart commented Sep 7, 2022

Uh oh!

mari-crl left a comment

Choose a reason for hiding this comment

Uh oh!

knz commented Sep 7, 2022

Uh oh!

rickystewart commented Sep 7, 2022

Uh oh!

craig Bot commented Sep 7, 2022

Uh oh!

knz left a comment

Choose a reason for hiding this comment

Uh oh!

rickystewart commented Sep 7, 2022

Uh oh!

craig Bot commented Sep 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

rickystewart commented Aug 12, 2022 •

edited by RaduBerinde

Loading