Merge cockroachdb#75806

75806: bazel,kvclient/rangefeed,util/log: stop using source mockgen generation r=ajwerner a=ajwerner

The source mode for mockgen is maddening because it requires the entire
go tree to be copied out. This is excedingly slow. We fix that by not using
that mode. To remedy that we have to export the interfaces we're mocking.
I can eat that.

Ideally we'd put the log one into a build-tagged away file, but
github.com/jmhodges/bazel_gomock doesn't provide a way for us to plumb tags
into its generator. That would be easy to fix, but not right now.

This commit also removes some references to the `--source` flag in
go:generate directives where they weren't doing anything.

Fixes: cockroachdb#71854

Release note: None

Co-authored-by: Andrew Werner <[email protected]>

Makefile

@@ -801,7 +801,11 @@ GENERATED_TARGETS = \
   pkg/kv/kvclient/rangefeed/mocks_generated.go \
   pkg/roachprod/vm/aws/embedded.go \
   pkg/security/securitytest/embedded.go \
-  pkg/security/certmgr/mocks_generated.go
+  pkg/security/certmgr/mocks_generated.go \
+  pkg/kv/kvclient/kvcoord/mocks_generated.go \
+  pkg/kv/kvclient/rangecache/mocks_generated.go \
+  pkg/roachpb/mocks_generated.go \
+  pkg/util/log/mock_generated.go
 
 EXECGEN_TARGETS = \
   pkg/col/coldata/vec.eg.go \
@@ -972,6 +976,8 @@ BUILD_TAGGED_RELEASE =
 ## Override for .buildinfo/tag
 BUILDINFO_TAG :=
 
+$(GENERATED_TARGETS): $(PROTOBUF_TARGETS) $(OPGEN_TARGETS) $(EXECGEN_TARGETS) $(SQLPARSER_TARGETS)
+
 $(go-targets): bin/.bootstrap $(BUILDINFO) $(CGO_FLAGS_FILES) $(PROTOBUF_TARGETS) $(LIBPROJ) $(GENERATED_TARGETS) $(CLEANUP_TARGETS)
 $(go-targets): $(LOG_TARGETS) $(SQLPARSER_TARGETS) $(OPTGEN_TARGETS)
 $(go-targets): override LINKFLAGS += \
@@ -1452,10 +1458,10 @@ ui-maintainer-clean: ## Like clean, but also remove installed dependencies
 ui-maintainer-clean: ui-clean
 	rm -rf pkg/ui/node_modules pkg/ui/workspaces/db-console/node_modules pkg/ui/yarn.installed pkg/ui/workspaces/cluster-ui/node_modules
 
-pkg/cmd/roachtest/prometheus/mock_generated.go: bin/.bootstrap pkg/cmd/roachtest/prometheus/prometheus.go
+pkg/cmd/roachtest/prometheus/mock_generated.go: bin/.bootstrap pkg/cmd/roachtest/prometheus/prometheus.go pkg/roachprod/vm/aws/embedded.go pkg/security/securitytest/embedded.go $(OPTGEN_TARGETS)
 	(cd pkg/cmd/roachtest/prometheus && $(GO) generate)
 
-pkg/cmd/roachtest/tests/drt_generated.go: bin/.bootstrap pkg/cmd/roachtest/tests/drt.go
+pkg/cmd/roachtest/tests/drt_generated.go: bin/.bootstrap pkg/cmd/roachtest/tests/drt.go pkg/roachprod/vm/aws/embedded.go $(OPTGEN_TARGETS) pkg/security/securitytest/embedded.go
 	(cd pkg/cmd/roachtest/tests && $(GO) generate)
 
 pkg/kv/kvclient/rangefeed/mocks_generated.go: bin/.bootstrap pkg/kv/kvclient/rangefeed/rangefeed.go

build/bazelutil/check.sh

@@ -10,16 +10,16 @@ pkg/roachprod/vm/aws/config.go://go:generate go-bindata -mode 0600 -modtime 1400
 pkg/roachprod/vm/aws/config.go://go:generate gofmt -s -w embedded.go
 pkg/roachprod/vm/aws/config.go://go:generate goimports -w embedded.go
 pkg/roachprod/vm/aws/config.go://go:generate terraformgen -o terraform/main.tf
-pkg/cmd/roachtest/prometheus/prometheus.go://go:generate mockgen -package=prometheus -destination=mock_generated.go -source=prometheus.go . Cluster
-pkg/cmd/roachtest/tests/drt.go://go:generate mockgen -source drt.go -package tests -destination drt_generated.go
+pkg/cmd/roachtest/prometheus/prometheus.go://go:generate mockgen -package=prometheus -destination=mock_generated.go . Cluster
+pkg/cmd/roachtest/tests/drt.go://go:generate mockgen -package tests -destination drt_generated.go . PromClient
 pkg/kv/kvclient/kvcoord/transport.go://go:generate mockgen -package=kvcoord -destination=mocks_generated.go . Transport
 pkg/kv/kvclient/rangecache/range_cache.go://go:generate mockgen -package=rangecache -destination=mocks_generated.go . RangeDescriptorDB
-pkg/kv/kvclient/rangefeed/rangefeed.go://go:generate mockgen -package=rangefeed -source rangefeed.go -destination=mocks_generated.go .
+pkg/kv/kvclient/rangefeed/rangefeed.go://go:generate mockgen -destination=mocks_generated.go --package=rangefeed . DB
 pkg/kv/kvserver/concurrency/lock_table.go://go:generate ../../../util/interval/generic/gen.sh *lockState concurrency
 pkg/kv/kvserver/spanlatch/manager.go://go:generate ../../../util/interval/generic/gen.sh *latch spanlatch
 pkg/roachpb/api.go://go:generate mockgen -package=roachpb -destination=mocks_generated.go . InternalClient,Internal_RangeFeedClient
 pkg/roachpb/batch.go://go:generate go run -tags gen-batch gen/main.go
-pkg/security/certmgr/cert.go://go:generate mockgen -package=certmgr -destination=mocks_generated.go -source=cert.go . Cert
+pkg/security/certmgr/cert.go://go:generate mockgen -package=certmgr -destination=mocks_generated.go . Cert
 pkg/security/securitytest/securitytest.go://go:generate go-bindata -mode 0600 -modtime 1400000000 -pkg securitytest -o embedded.go -ignore README.md -ignore regenerate.sh test_certs
 pkg/security/securitytest/securitytest.go://go:generate gofmt -s -w embedded.go
 pkg/security/securitytest/securitytest.go://go:generate goimports -w embedded.go
@@ -39,7 +39,7 @@ pkg/util/log/channels.go://go:generate go run gen/main.go logpb/log.proto channe
 pkg/util/log/channels.go://go:generate go run gen/main.go logpb/log.proto log_channels.go log_channels_generated.go
 pkg/util/log/channels.go://go:generate go run gen/main.go logpb/log.proto logging.md ../../../docs/generated/logging.md
 pkg/util/log/channels.go://go:generate go run gen/main.go logpb/log.proto severity.go severity/severity_generated.go
-pkg/util/log/sinks.go://go:generate mockgen -package=log -source=sinks.go -destination=mock_generated.go -mock_names=logSink=MockLogSink logSink
+pkg/util/log/sinks.go://go:generate mockgen -package=log -destination=mocks_generated_test.go --mock_names=TestingLogSink=MockLogSink . TestingLogSink
 pkg/util/timeutil/zoneinfo.go://go:generate go run gen/main.go
 "
 

pkg/cmd/roachtest/prometheus/prometheus.go

@@ -8,7 +8,7 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-//go:generate mockgen -package=prometheus -destination=mock_generated.go -source=prometheus.go . Cluster
+//go:generate mockgen -package=prometheus -destination=mock_generated.go . Cluster
 
 package prometheus
 

pkg/cmd/roachtest/tests/drt.go

@@ -10,7 +10,7 @@
 
 package tests
 
-//go:generate mockgen -source drt.go -package tests -destination drt_generated.go
+//go:generate mockgen -package tests -destination drt_generated.go . PromClient
 
 import (
 	"context"

pkg/kv/kvclient/rangefeed/BUILD.bazel

@@ -48,11 +48,10 @@ go_library(
 gomock(
     name = "mock_rangefeed",
     out = "mocks_generated.go",
-    interfaces = [""],  # work-around for https://github.com/jmhodges/bazel_gomock/issues/58
+    interfaces = ["DB"],
     library = ":rangefeed",
     package = "rangefeed",
     self_package = "github.com/cockroachdb/cockroach/pkg/kv/kvclient/rangefeed",
-    source = "rangefeed.go",
 )
 
 go_library(

pkg/kv/kvclient/rangefeed/db_adapter.go

@@ -29,14 +29,14 @@ import (
 	"github.com/cockroachdb/errors"
 )
 
-// dbAdapter is an implementation of the kvDB interface using a real *kv.DB.
+// dbAdapter is an implementation of the DB interface using a real *kv.DB.
 type dbAdapter struct {
 	db         *kv.DB
 	st         *cluster.Settings
 	distSender *kvcoord.DistSender
 }
 
-var _ kvDB = (*dbAdapter)(nil)
+var _ DB = (*dbAdapter)(nil)
 
 var maxScanParallelism = settings.RegisterIntSetting(
 	settings.TenantWritable,
@@ -45,7 +45,7 @@ var maxScanParallelism = settings.RegisterIntSetting(
 	64,
 )
 
-// newDBAdapter construct a kvDB using a *kv.DB.
+// newDBAdapter construct a DB using a *kv.DB.
 func newDBAdapter(db *kv.DB, st *cluster.Settings) (*dbAdapter, error) {
 	var distSender *kvcoord.DistSender
 	{
@@ -67,7 +67,7 @@ func newDBAdapter(db *kv.DB, st *cluster.Settings) (*dbAdapter, error) {
 	}, nil
 }
 
-// RangeFeed is part of the kvDB interface.
+// RangeFeed is part of the DB interface.
 func (dbc *dbAdapter) RangeFeed(
 	ctx context.Context,
 	spans []roachpb.Span,
@@ -96,7 +96,7 @@ func (ba *concurrentBoundAccount) Shrink(ctx context.Context, x int64) {
 	ba.BoundAccount.Shrink(ctx, x)
 }
 
-// Scan is part of the kvDB interface.
+// Scan is part of the DB interface.
 func (dbc *dbAdapter) Scan(
 	ctx context.Context,
 	spans []roachpb.Span,

pkg/kv/kvclient/rangefeed/helpers_test.go

@@ -23,8 +23,8 @@ var NewDBAdapter = newDBAdapter
 // NewFactoryWithDB allows tests to construct a factory with an injected db.
 var NewFactoryWithDB = newFactory
 
-// KVDB forwards the definition of kvDB to tests.
-type KVDB = kvDB
+// KVDB forwards the definition of DB to tests.
+type KVDB = DB
 
 // ScanConfig forwards the definition of scanConfig to tests.
 type ScanConfig = scanConfig

pkg/kv/kvclient/rangefeed/rangefeed.go

@@ -33,7 +33,7 @@ import (
 	"github.com/cockroachdb/logtags"
 )
 
-//go:generate mockgen -package=rangefeed -source rangefeed.go -destination=mocks_generated.go .
+//go:generate mockgen -destination=mocks_generated.go --package=rangefeed . DB
 
 // TODO(ajwerner): Expose hooks for metrics.
 // TODO(ajwerner): Expose access to checkpoints and the frontier.
@@ -42,8 +42,8 @@ import (
 // TODO(yevgeniy): Instead of rolling our own logic to parallelize scans, we should
 // use streamer API instead (https://github.com/cockroachdb/cockroach/pull/68430)
 
-// kvDB is an adapter to the underlying KV store.
-type kvDB interface {
+// DB is an adapter to the underlying KV store.
+type DB interface {
 
 	// RangeFeed runs a rangefeed on a given span with the given arguments.
 	// It encapsulates the RangeFeed method on roachpb.Internal.
@@ -71,7 +71,7 @@ type kvDB interface {
 // Factory is used to construct RangeFeeds.
 type Factory struct {
 	stopper *stop.Stopper
-	client  kvDB
+	client  DB
 	knobs   *TestingKnobs
 }
 
@@ -98,7 +98,7 @@ func NewFactory(
 	return newFactory(stopper, kvDB, knobs), nil
 }
 
-func newFactory(stopper *stop.Stopper, client kvDB, knobs *TestingKnobs) *Factory {
+func newFactory(stopper *stop.Stopper, client DB, knobs *TestingKnobs) *Factory {
 	return &Factory{
 		stopper: stopper,
 		client:  client,
@@ -154,7 +154,7 @@ type OnValue func(ctx context.Context, value *roachpb.RangeFeedValue)
 type RangeFeed struct {
 	config
 	name    string
-	client  kvDB
+	client  DB
 	stopper *stop.Stopper
 	knobs   *TestingKnobs
 

pkg/kv/kvclient/rangefeed/rangefeed_mock_test.go

@@ -69,7 +69,7 @@ func (m *mockClient) Scan(
 
 var _ rangefeed.KVDB = (*mockClient)(nil)
 
-// TestRangefeedMock utilizes the kvDB interface to test the behavior of the
+// TestRangefeedMock utilizes the DB interface to test the behavior of the
 // RangeFeed.
 func TestRangeFeedMock(t *testing.T) {
 	defer leaktest.AfterTest(t)()
@@ -347,7 +347,7 @@ func TestBackoffOnRangefeedFailure(t *testing.T) {
 	defer stopper.Stop(ctx)
 
 	ctrl := gomock.NewController(t)
-	db := rangefeed.NewMockkvDB(ctrl)
+	db := rangefeed.NewMockDB(ctrl)
 
 	// Make sure scan failure gets retried.
 	db.EXPECT().Scan(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("scan failed"))

pkg/security/certmgr/cert.go

@@ -15,7 +15,7 @@ import (
 	"crypto/tls"
 )
 
-//go:generate mockgen -package=certmgr -destination=mocks_generated.go -source=cert.go . Cert
+//go:generate mockgen -package=certmgr -destination=mocks_generated.go . Cert
 
 // Cert is a generic presentation on managed certificate that can be reloaded.
 // A managed certificate allows the certificates to be rotated by a cert manager.

pkg/util/log/BUILD.bazel

@@ -1,7 +1,7 @@
 load("@bazel_gomock//:gomock.bzl", "gomock")
 load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
 
-# gazelle:exclude mock_generated.go
+# gazelle:exclude mocks_generated_test.go
 
 go_library(
     name = "log",
@@ -206,10 +206,12 @@ genrule(
 
 gomock(
     name = "mock_logsink",
-    out = "mock_generated.go",
-    interfaces = ["logSink"],
+    out = "mocks_generated_test.go",
+    interfaces = [
+        "TestingLogSink",
+    ],
     library = ":log",
-    mock_names = {"logSink": "MockLogSink"},
+    mock_names = {"TestingLogSink": "MockLogSink"},
     package = "log",
-    source = "sinks.go",
+    self_package = "github.com/cockroachdb/cockroach/pkg/util/log",
 )

pkg/util/log/sinks.go

@@ -12,7 +12,15 @@ package log
 
 import "github.com/cockroachdb/cockroach/pkg/cli/exit"
 
-//go:generate mockgen -package=log -source=sinks.go -destination=mock_generated.go -mock_names=logSink=MockLogSink logSink
+//go:generate mockgen -package=log -destination=mocks_generated_test.go --mock_names=TestingLogSink=MockLogSink . TestingLogSink
+
+// TestingLogSink is exported for mock generation.
+// This is painful, but it seems to be the only way, for the moment, to
+// generate this mock.
+//
+// The reason is that there's no way to inject build tags into the current
+// bazel rules for gomock.
+type TestingLogSink = logSink
 
 // sinkOutputOptions provides various options for a logSink.output call.
 type sinkOutputOptions struct {