deps: update errors and the sentry-go dependency #76498

knz · 2022-02-14T11:53:19Z

This updates the sentry SDK version from 0.6.1 to 0.12.0.

cockroach-teamcity · 2022-02-14T11:53:27Z

This change is

rail

cockroachdb/sentry-go is still in go.sum, probably this is why the test fails.

Reviewed 10 of 10 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz)

go.sum, line 440 at r1 (raw file):

github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd h1:KFOt5I9nEKZgCnOSmy8r4Oykh8BYQO8bFOTgHDS8YZA=
github.com/cockroachdb/returncheck v0.0.0-20200612231554-92cdbca611dd/go.mod h1:AN708GD2FFeLgUHMbD58YPe4Nw8GG//3rwgyG4L9gR0=
github.com/cockroachdb/sentry-go v0.6.1-cockroachdb.2/go.mod h1:8BT+cPK6xvFOcRlk0R8eg+OTkcqI6baNH4xAkpiYVvQ=

Looks like this line makes the linter unhappy.

pkg/cli/flags_test.go, line 74 at r1 (raw file):

			"github.com/cockroachdb/cockroach/pkg/testutils", // meant for testing code only
		},
		// Sentry and the errors library use go/build to determine

Can you also update the comments here to mention only the errors library.

Code quote (from pkg/util/log/logcrash/crash_reporting.go):

cockroachdb/sentry-go

knz

I don't see any test failure, other than the unrelated test flakes in #76501 and #76502.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rail)

go.sum, line 440 at r1 (raw file):

Previously, rail (Rail Aliiev) wrote…

Looks like this line makes the linter unhappy.

Done. But what is this "linter" you speak of?

pkg/cli/flags_test.go, line 74 at r1 (raw file):

Previously, rail (Rail Aliiev) wrote…

Can you also update the comments here to mention only the errors library.

Done.

knz · 2022-02-14T14:07:52Z

Actually the test failures are due to the changes in the upstream dep github.com/kr/pretty.

rail

Reviewed 2 of 2 files at r2, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz)

go.sum, line 440 at r1 (raw file):

Previously, knz (kena) wrote…

Done. But what is this "linter" you speak of?

"check generated code", see https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_UnitTests_CheckGeneratedCode/4371077?showRootCauses=true&expandBuildChangesSection=true&expandBuildDeploymentsSection=true&expandBuildProblemsSection=true
It's more like a build system linter.

The error is still there 🤔, maybe something special in https://github.com/cockroachdb/cockroach/blob/master/build/bazelutil/bazel-generate.sh

knz · 2022-02-14T14:33:53Z

I fixed the broken tests. RFAL

rail

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz and @rail)

go.sum, line 440 at r1 (raw file):

Previously, rail (Rail Aliiev) wrote…

"check generated code", see https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_UnitTests_CheckGeneratedCode/4371077?showRootCauses=true&expandBuildChangesSection=true&expandBuildDeploymentsSection=true&expandBuildProblemsSection=true
It's more like a build system linter.

The error is still there 🤔, maybe something special in https://github.com/cockroachdb/cockroach/blob/master/build/bazelutil/bazel-generate.sh

cockroach/pkg/cmd/mirror/mirror.go

Line 177 in bbb473c

cmd := exec.Command(gobin, "list", "-mod=readonly", "-m", "-json", "all")

calls go list -mod=readonly -m all.

go list -m: github.com/cockroachdb/datadriven@v1.0.1-0.20211007161720-b558070c3be0 requires
	github.com/cockroachdb/errors@v1.6.1 requires
	github.com/cockroachdb/sentry-go@v0.6.1-cockroachdb.2: missing go.sum entry; to add it:
	go mod download github.com/cockroachdb/sentry-go

knz

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rail)

go.sum, line 440 at r1 (raw file):

Previously, rail (Rail Aliiev) wrote…

cockroach/pkg/cmd/mirror/mirror.go

Line 177 in bbb473c

cmd := exec.Command(gobin, "list", "-mod=readonly", "-m", "-json", "all")

calls go list -mod=readonly -m all.
go list -m: github.com/cockroachdb/datadriven@v1.0.1-0.20211007161720-b558070c3be0 requires
	github.com/cockroachdb/errors@v1.6.1 requires
	github.com/cockroachdb/sentry-go@v0.6.1-cockroachdb.2: missing go.sum entry; to add it:
	go mod download github.com/cockroachdb/sentry-go

But this logic is broken, right? The dependency graph should smoothly upgrade the dependency of datadriven to errors 1.8.7 since it's compatible with 1.6.1, and drop the dependency to cockroachdb/sentry-go. We don't need it any more in the top level.

So it looks to me that mirror.go has incorrect logic.

Why does this use go list -mod=readonly and not go list -mod=mod btw?

rail

Reviewed 2 of 2 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz)

DEPS.bzl, line 1311 at r4 (raw file):

        importpath = "github.com/cockroachdb/sentry-go",
        sha256 = "fbb2207d02aecfdd411b1357efe1192dbb827959e36b7cab7491731ac55935c9",
        strip_prefix = "github.com/cockroachdb/sentry-go@v0.6.1-cockroachdb.2",

hmmm, I'm not sure we want to keep it around just to make the test pass. 🤔

go.sum, line 440 at r1 (raw file):

Previously, knz (kena) wrote…

But this logic is broken, right? The dependency graph should smoothly upgrade the dependency of datadriven to errors 1.8.7 since it's compatible with 1.6.1, and drop the dependency to cockroachdb/sentry-go. We don't need it any more in the top level.

So it looks to me that mirror.go has incorrect logic.

Why does this use go list -mod=readonly and not go list -mod=mod btw?

I think go list thinks they are different modules and this is why it's still there. datadriven still thinks that our fork is required. Probably we have to update datadriven's deps too.

knz · 2022-02-14T16:14:13Z

ok see cockroachdb/datadriven#35

knz · 2022-02-14T17:17:56Z

I have iterated back and forth between datadriven and errors, and was not able to get them to remove the offending package from go.sum. I suspect there's yet another 3rd party dependency, perhaps used indirectly, that depends on an older version of cockroachdb/errors.

I'm not sure what else we should do here?

This updates the sentry SDK version from 0.6.1 to 0.12.0. Release note: None Co-authored-by: Rail Aliiev <rail@iqchoice.com>

knz · 2022-02-15T10:51:13Z

okay I've done my best here. I think this is good enough to say "we dropped the fork" since it's not copied into the vendor/ tree any more. I'm not too concerned by go.sum at this point - it doesn't seem to hurt.

rail

Reviewed 1 of 4 files at r5, 2 of 3 files at r6, 4 of 4 files at r7, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @knz)

knz · 2022-02-15T15:31:42Z

thanks!

bors r=rail

craig · 2022-02-15T16:42:22Z

Build failed (retrying...):

GitHub CI (Cockroach)

craig · 2022-02-15T18:38:04Z

Build succeeded:

GitHub CI (Cockroach)

knz requested a review from rail February 14, 2022 11:53

knz requested a review from a team as a code owner February 14, 2022 11:53

rail requested changes Feb 14, 2022

View reviewed changes

knz force-pushed the 20220214-sentry branch from 14cfcc5 to 076ac47 Compare February 14, 2022 13:19

knz commented Feb 14, 2022

View reviewed changes

rail reviewed Feb 14, 2022

View reviewed changes

knz force-pushed the 20220214-sentry branch from 076ac47 to ab70b4b Compare February 14, 2022 14:33

knz requested a review from a team as a code owner February 14, 2022 14:33

rail requested changes Feb 14, 2022

View reviewed changes

knz commented Feb 14, 2022

View reviewed changes

knz force-pushed the 20220214-sentry branch from ab70b4b to 09eedbe Compare February 14, 2022 15:02

rail reviewed Feb 14, 2022

View reviewed changes

knz force-pushed the 20220214-sentry branch from 09eedbe to 572e614 Compare February 14, 2022 17:16

knz force-pushed the 20220214-sentry branch 2 times, most recently from 10e6ada to 1c379e4 Compare February 15, 2022 10:04

deps: update errors and the sentry-go dependency

ecc691f

This updates the sentry SDK version from 0.6.1 to 0.12.0. Release note: None Co-authored-by: Rail Aliiev <rail@iqchoice.com>

knz force-pushed the 20220214-sentry branch from 1c379e4 to ecc691f Compare February 15, 2022 10:18

rail approved these changes Feb 15, 2022

View reviewed changes

craig bot merged commit b16a845 into cockroachdb:master Feb 15, 2022

knz deleted the 20220214-sentry branch February 16, 2022 13:34

This was referenced Mar 9, 2022

release-21.2: deps: upgrade cockroachdb/errors #77527

Merged

release-21.1: deps: upgrade gogoproto and cockroachdb/errors #77677

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

deps: update errors and the sentry-go dependency #76498

deps: update errors and the sentry-go dependency #76498

knz commented Feb 14, 2022

cockroach-teamcity commented Feb 14, 2022

rail left a comment

knz left a comment

knz commented Feb 14, 2022

rail left a comment

knz commented Feb 14, 2022

rail left a comment

knz left a comment

rail left a comment

knz commented Feb 14, 2022

knz commented Feb 14, 2022

knz commented Feb 15, 2022

rail left a comment

knz commented Feb 15, 2022

craig bot commented Feb 15, 2022

craig bot commented Feb 15, 2022

deps: update errors and the sentry-go dependency #76498

deps: update errors and the sentry-go dependency #76498

Conversation

knz commented Feb 14, 2022

cockroach-teamcity commented Feb 14, 2022

rail left a comment

Choose a reason for hiding this comment

knz left a comment

Choose a reason for hiding this comment

knz commented Feb 14, 2022

rail left a comment

Choose a reason for hiding this comment

knz commented Feb 14, 2022

rail left a comment

Choose a reason for hiding this comment

knz left a comment

Choose a reason for hiding this comment

rail left a comment

Choose a reason for hiding this comment

knz commented Feb 14, 2022

knz commented Feb 14, 2022

knz commented Feb 15, 2022

rail left a comment

Choose a reason for hiding this comment

knz commented Feb 15, 2022

craig bot commented Feb 15, 2022

craig bot commented Feb 15, 2022