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.

Sign up for GitHub

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

Jump to bottom

Gopkg.lock => go.mod checklist #50148

Closed
85 tasks done
otan opened this issue Jun 12, 2020 · 40 comments
Closed
85 tasks done

Gopkg.lock => go.mod checklist #50148

otan opened this issue Jun 12, 2020 · 40 comments
Assignees
@otan
Labels
A-build-system C-investigation Further steps needed to qualify. C-label will change.

Comments

@otan
Copy link
Contributor

otan commented Jun 12, 2020 •
edited by tbg
Loading

Generated from: https://github.com/otan-cockroach/otan-scripts/blob/master/gomodupdate/diff.py
PR: #49447

new to pick up sentry-go / errors update

  • google.golang.org/grpc v1.29.1 (vs old version v1.27.1)
  • gopkg.in/yaml.v2 v2.3.0 (vs old version v2.2.8)

etcd change

  • golang.org/x/net a8b05e91 => 627f9648deb9
  • etcd becomes github.com/cockroachdb/etcd v0.4.7-0.20200615211340-a17df30d5955
@otan otan added C-investigation Further steps needed to qualify. C-label will change. A-build-system labels Jun 12, 2020
@otan otan mentioned this issue Jun 12, 2020
Merged
@otan
Copy link
Contributor Author

otan commented Jun 12, 2020 •
edited
Loading

ticking off apd and modvendor changes because they were upgraded deliberately.
ticking off dep and dependency Masterminds, go-radix, nuts, boltdb/bolt, go-toml because it was deleted deliberately

@jordanlewis
Copy link
Member

Checked errcheck, go-test-teamcity neither of which are runtime deps.

@otan
Copy link
Contributor Author

otan commented Jun 13, 2020

ticking linkedin/goavro because that was manually fixed to be go.mod compatible
ticking testify as test only.

@knz
Copy link
Contributor

knz commented Jun 14, 2020 •
edited
Loading

Checking off on:

  • sentry-go
  • pkg/errors
  • go-uuid
  • pflag
  • cobra
  • go-isatty
  • go-runewidth
  • kr/pretty, kr/text

@knz
Copy link
Contributor

knz commented Jun 14, 2020

We also need to double check exactly where this repo is used: github.com/jcmturner/gofork

It contains "modified Go standard library packages for use as work arounds until issues are addressed in the official distribution" however the package is 3 years old so the issues may have been addressed since.

Also it forks crypto-related content, so this is suspicious.

cc @bdarnell @aaron-crl

@knz
Copy link
Contributor

knz commented Jun 14, 2020

@otan I retract my comment on sentry-go. The anchor on 0.3.999 is incorrect. We absolutely need to anchor on tag v0.6.1-cockroachdb

@knz
Copy link
Contributor

knz commented Jun 14, 2020

I think there's a go mod brokenness in the sentry-go and cockroachdb/errors packages. looking into it

@knz
Copy link
Contributor

knz commented Jun 14, 2020

Ok I have bumped the go.mod in cockroachdb/errors and sentry-go.
This should get v0.6.1-cockroachdb.2 for sentry properly.

@knz knz mentioned this issue Jun 14, 2020
Closed
@bdarnell
Copy link
Contributor

gofork (and other jcmturner repos including gokrb5) appear to be brought in by the update to sarama, the kafka client we use, which gained kerberos support since the last time we updated it. It's not clear to me what exactly has changed in that fork, but apparently it is still needed since sarama tried to take it out but just put it back in.

If sarama is the only use of the jcmturner repos, I'm OK with the additions (but someone from CDC should sign off on the update to sarama itself). What repo/branch would I need to check out to be able to grep for this?

@otan
Copy link
Contributor Author

otan commented Jun 14, 2020

@bdarnell #49447 is the PR

@bdarnell
Copy link
Contributor

The etcd changes look good to me; they're picking up some relevant changes from @tbg (and nothing else in the raft package). @tbg, is there any reason your changes like etcd-io/etcd@9018b3d haven't been picked up in CRDB yet?

@tbg
Copy link
Member

tbg commented Jun 15, 2020 via email

@otan
Copy link
Contributor Author

otan commented Jun 15, 2020 •
edited
Loading

mmm, i may have rolled back etcd instead of going forward because of this when using the same version:

i could not use 4a2b4c8 because it errors

go: finding go.etcd.io/etcd 4a2b4c8f
go get: go.etcd.io/etcd@v0.5.0-alpha.5.0.20190816082442-4a2b4c8f7e0a requires
	golang.org/x/net@v0.0.0-20190813000000-74dc4d7220e7: invalid pseudo-version: does not match version-control timestamp (2019-08-13T14:13:03Z)

@otan
Copy link
Contributor Author

otan commented Jun 15, 2020

it looks like they never pulled 4a2b4c8f7e0a3754fdd5a3341a27c2431c2a5385 to any of their release branches...

@tbg
Copy link
Member

tbg commented Jun 15, 2020

Can you just use a newer commit from their latest release branch? Usually there are not many changes in raft.

@otan
Copy link
Contributor Author

otan commented Jun 15, 2020 •
edited
Loading

the master branch requires a heavy refactor on CRDB; i've cherry picked a go.mod fix (cockroachdb/etcd@a17df30) and added it to our fork of etcd.

@otan
Copy link
Contributor Author

otan commented Jun 16, 2020

ticking zglob as it is part of modvendor; ticking all azure ones as they affect roachprod only.

@otan otan self-assigned this Jun 16, 2020
@otan
Copy link
Contributor Author

otan commented Jun 22, 2020

  • mismatching version: github.com/kisielk/errcheck - go.sum v1.2.0 vs Gopkg.lock v1.1.0
  • mismatching version: github.com/konsorten/go-windows-terminal-sequences - go.sum v1.0.2 vs Gopkg.lock v1.0.1

both look good as they upgrade to use go.mod

@otan
Copy link
Contributor Author

otan commented Jun 22, 2020

mismatching version: github.com/Microsoft/go-winio - go.sum v0.4.14 vs Gopkg.lock v0.4.11

lgtm - go mod support and only needed for go-geom indirectly.

@otan
Copy link
Contributor Author

otan commented Jun 22, 2020 •
edited
Loading

sarama (lz4, zstd, jcmturner*) et all looks good.

@otan
Copy link
Contributor Author

otan commented Jun 22, 2020

iostat same SHA, different version tag

@otan
Copy link
Contributor Author

otan commented Jun 22, 2020

logrus / docker/go-units is a docker related upgrade, so checking

@nvanbenschoten
Copy link
Member

btree same SHA, different version tag

@tbg
Copy link
Member

tbg commented Jun 23, 2020

Signed off on opentracing/opentracing-go@v1.0.2...v1.1.0

@tbg
Copy link
Member

tbg commented Jun 23, 2020

Signed off on golang/text@470f45b...v0.3.2

There are a ton of commits, I scanned the titles only and nothing stood out. I vaguely remember something about how the collation tables must never change or CRDB will break, however I wasn't able to find a reference to that in the code. Nevertheless, I searched for collate and table through the commit history of x/test and found nothing.

@tbg
Copy link
Member

tbg commented Jun 23, 2020

https://github.com/golang/lint/compare/959b441a...910be7a9?tab=doc is fine, nothing changed

@tbg
Copy link
Member

tbg commented Jun 23, 2020

https://github.com/golang/exp/compare/01c40f57...00229845?tab=doc looks ok too (not saying this with any scientific precision, but I did look through the commit titles)

@tbg
Copy link
Member

tbg commented Jun 23, 2020

https://github.com/golang/crypto/compare/ff983b9c...0ec3e997?tab=doc as well. A number of bug fixes and test improvements, along with some new features, though none that I expect would affect existing behavior in the small subset of the package we're using.

@tbg
Copy link
Member

tbg commented Jun 23, 2020

https://github.com/golang/sync/compare/37e7f081...cd5d95a4?tab=doc is fine, almost a noop

@tbg
Copy link
Member

tbg commented Jun 23, 2020

Jeez, https://github.com/golang/sys/compare/a457fd03...fe76b779?tab=doc has 325 commits, no way I can review that. But reasonably if something regresses here we will have to find out the hard way anyway, it's not like anyone could tell looking at the commits. Signing off.

@tbg
Copy link
Member

tbg commented Jun 23, 2020

https://github.com/golang/time/compare/85acf8d2...9d24e822?tab=doc is noop

http://github.com/golang/tools/compare/63859f38...2212a7e1?tab=doc has >1000 commits. Signing off blindly. We'll find out if something breaks.

@tbg
Copy link
Member

tbg commented Jun 23, 2020

https://github.com/googleapis/go-genproto/compare/db91494d...dad8c97a?tab=doc is fine

@tbg
Copy link
Member

tbg commented Jun 23, 2020

dominikh/go-tools@1f0868a...v0.0.1-2019.2.3 has >100 updates, but nothing sticks out. If anything it'll probably work better (if it passes lints, which is where we use this mostly)

@tbg
Copy link
Member

tbg commented Jun 23, 2020

Also checking off prometheus and flatbuffers.

@tbg
Copy link
Member

tbg commented Jun 23, 2020

Also checking off arrow. Usually I would ask Dan to do it but he's out, and I strongly suspect that if there's any problem with this update, we should have been seeing it in our testing where the arrow format is exercised.

@tbg
Copy link
Member

tbg commented Jun 23, 2020

beorn7/perks@3a771d9...v1.0.1 is basically noop
golang/snappy@2e65f85...ff6b7dc ditto
golang/net@a8b05e9...627f964 is fine, mostly fixing builds on exotic archs

grpc/grpc-go@v1.27.1...v1.29.1 is ok too. I found it interesting that they added some built-in profiling helpers but then had to revert them again because of a perf regression:
grpc/grpc-go#3378

@tbg
Copy link
Member

tbg commented Jun 23, 2020

go-yaml/yaml@v2.2.8...v2.3.0 is a single commit which looks fine, too.

I think we've got them all!

@tbg
Copy link
Member

tbg commented Jun 23, 2020

the master branch requires a heavy refactor on CRDB; i've cherry picked a go.mod fix (cockroachdb/etcd@a17df30) and added it to our fork of etcd.

if you still remember, what was the heavy refactor? That seems unlikely
Either way I signed off on the cherry-pick ref change, that's fine for now

@otan
Copy link
Contributor Author

otan commented Jun 23, 2020

if you still remember, what was the heavy refactor? That seems unlikely

oops, i mean heavy rebase*.

@otan
Copy link
Contributor Author

otan commented Jun 23, 2020

thanks @tbg !

craig bot pushed a commit that referenced this issue Jun 24, 2020
@otan
Merge #49447
06c282d 
49447: *: switch to go.mod r=jordanlewis,petermattis a=otan

Checklist: #50148

----

This PR switches our dependency from `./bin/dep` to `go mod`.

Changes:
* delete Gopkg.toml and Gopkg.lock and replace with go.mod and go.sum.
* Use a patched version of `github.com/goware/modvendor` that can pull
  in imports from packages not in `vendor/modules.txt'.
* delete `c-deps/protobuf/example`, which causes problem with the build.
* change `Makefile` to install vendor directory on `make vendor_update.
* update apd to apd/v2.
* update goavro to goavro/v2.
* fix some lint related tests.
* build: delete references to ./bin/dep. update README instructions.

Release note: None


Co-authored-by: Oliver Tan <otan@cockroachlabs.com>
@otan otan closed this as completed Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@otan otan

Labels
A-build-system C-investigation Further steps needed to qualify. C-label will change.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@jordanlewis @bdarnell @knz @otan @tbg @nvanbenschoten