update cockroach to v22.1.22-64-g86fdbfca06

[iliana]

Primarily for debugging (#9427, and time sync problems).

The vast majority of changes here are build system related or updating dependencies that are used in tests only, but there are some actual code changes worth pointing out:

• fix printf-related go vet violations cockroach#46 (r+ @sudomateo)
• oxidecomputer/pebble@e7c3451 "fix lints; run tests in buildomat" (not reviewed)
• oxidecomputer/pebble@06125d2 backport of "sstable: do a bit flip computation on a checksum mismatch in the Reader" (not reviewed)
• improve logging around unhealthy clocks cockroach#60 (@JustinAzoff; r+ @iliana)
• remove analytics and update checks from pkg/ui cockroach#64 (not reviewed)
• remove phone-home diagnostics, update checks, crash reports cockroach#65 (r+ @sudomateo)

Full delta: oxidecomputer/cockroach@367bca4...86fdbfc

[davepacheco]

Wow, there's a lot of work here! Thanks for keeping on top of this. It does make me nervous that we're doing so much work in our fork but I'm not sure that's the wrong thing.

It does seem like some of these forks (at least https://github.com/oxidecomputer/geos.git, https://github.com/cockroachdb/PROJ, https://github.com/oxidecomputer/pebble) don't use RFD 211 and it's not obvious what we've changed from upstream or why. That seems good to fix unless you want to be the sole person responsible for this forever 😀 Do you feel like you've already got sufficient review on those or is that part of what's being looked at here?

Have you tested that the web console still works? I saw there were changes to the ui package.

[iliana]

It does make me nervous that we're doing so much work in our fork but I'm not sure that's the wrong thing.

My maintenance philosophy is to avoid behavior changes unless they're well-understood. There's three behavior changes here: the backport of bit-flip detection in Pebble, a log message in a new else branch, and the removal of the phone-home diagnostics reporting (but not the wholesale removal of generating those diagnostics, which we can still fetch via the HTTP service). I think these are all the "right" thing.

The other changes are related to still being able to build Cockroach on modern toolchains. The PROJ and GEOS forks are specifically for that; we had some hacks in the repo before to work around it but with Helios 3 coming there really wasn't any other option.

It does seem like some of these forks don't use RFD 211 and it's not obvious what we've changed from upstream or why.

RFD 211 seems to primarily concern repositories where we need to make changes and can't immediately integrate them upstream. In the case of these repos, we are starting from the commits that were last used in Cockroach v22.1.22, and applying changes on top. There is no intent to upstream anything because there's nothing to upstream; all the fixes we're making are either related to our own confidence of being able to continue to use the software (e.g. setting up Buildomat in the Pebble repo), or are fixes upstream almost certainly already had by virtue of still being maintained.

The sidebar descriptions of the forks describe exactly what they are: they're for Cockroach. I can add a README.oxide but it won't be immediately visible due to GitHub's choices (hence why I decided to replace the main readme in our Cockroach fork); I'd prefer to wholesale replace the upstream README if we really want the additional context. They're not forks in the sense RFD 211 cares about; they're forks that exist only for continued maintainability of a fossil.

The RFD 211-style comments suggested for Cargo.toml aren't possible for a Go project because Go has decided you can't write anything useful in a go.mod file. I am not joking.

Do you feel like you've already got sufficient review on those or is that part of what's being looked at here?

I don't think the changes to PROJ and GEOS need review -- they are both a couple of lines to build files or #includes to make them work on current toolchains.

For Pebble, the bulk of the changes are related to fixing new/improved lints on the newer Go toolchain, primarily the io/ioutil deprecation which is a fairly obvious operation and difficult to screw up. Perhaps the bitflip detection backport could use some review but I am quite confident in its behavior, it has a test (which we are running in Buildomat), and upstream has had it integrated for some time. (The only difficulty in the backport was figuring out the log message and integrating the changes to the test.)

Have you tested that the web console still works?

Yep, in that I clicked around a bunch and nothing seemed broken. We also continue to run the ui-tests which received minimal changes. The changes to the ui package were largely removing phone-home analytics, to which a surprising amount of code was dedicated.

[davepacheco]

It does seem like some of these forks don't use RFD 211 and it's not obvious what we've changed from upstream or why.

RFD 211 seems to primarily concern repositories where we need to make changes and can't immediately integrate them upstream. In the case of these repos, we are starting from the commits that were last used in Cockroach v22.1.22, and applying changes on top. There is no intent to upstream anything because there's nothing to upstream; all the fixes we're making are either related to our own confidence of being able to continue to use the software (e.g. setting up Buildomat in the Pebble repo), or are fixes upstream almost certainly already had by virtue of still being maintained.

The sidebar descriptions of the forks describe exactly what they are: they're for Cockroach. I can add a README.oxide but it won't be immediately visible due to GitHub's choices (hence why I decided to replace the main readme in our Cockroach fork); I'd prefer to wholesale replace the upstream README if we really want the additional context. They're not forks in the sense RFD 211 cares about; they're forks that exist only for continued maintainability of a fossil.

The RFD 211-style comments suggested for Cargo.toml aren't possible for a Go project because Go has decided you can't write anything useful in a go.mod file. I am not joking.

Yeah, RFD 211 does focus on cases where we're forking for the short-to-medium term. Rereading it, I think it has an unstated goal that does apply here which is being able to quickly determine with high confidence exactly what changes we've made locally compared with what we forked from. As mentioned in chat, maybe a tag makes more sense than an upstream branch in this case? On the other hand, we have pulled in at least one fix from upstream, and I think hope to pull in another. I'm not sure what advantage a tag would have over just keeping the branch.

So: I'm sorry that I just saw (based on your parenthetical above) that the README in this repo is what I was imagining README.oxide containing. It didn't even occur to me to look at that because I just assumed that was the upstream repo's README and wouldn't have anything useful about the fork. (Take that for what it's worth in terms of discoverability!) Anyway, I think this was a big part of my confusion so sorry about that.

Unrelated to this PR, it might be useful to add to that:

• that we rarely (but not never) intend to pull stuff in from upstream
• that we never intend to push stuff upstream
• that we've also bumped the Go toolchain, and that was primarily for keeping TLS up to date (right?)
• links to the other repos we've forked because they're referenced from this one