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.

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

bazel,kvclient/rangefeed,util/log: stop using source mockgen generation #75806

Merged
merged 1 commit into from
Feb 5, 2022

Conversation

ajwerner
Copy link
Contributor

@ajwerner ajwerner commented Feb 1, 2022

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: #71854

Release note: None

@ajwerner ajwerner requested review from a team as code owners February 1, 2022 17:23
@ajwerner ajwerner requested a review from a team February 1, 2022 17:23
@ajwerner ajwerner requested a review from a team as a code owner February 1, 2022 17:23
@ajwerner ajwerner requested review from stevendanna and removed request for a team February 1, 2022 17:23
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@ajwerner ajwerner force-pushed the ajwerner/fix-go-mocks-in-bazel branch 2 times, most recently from d345382 to cf77d93 Compare February 2, 2022 03:26
Copy link
Collaborator

@rickystewart rickystewart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you just need to tweak the go:generate stuff and then we're good:

[03:31:44 ](https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Ci_Tests_CheckGeneratedCode/4275261?buildTab=log&focusLine=4746&linesState=4746)    Detected an unknown //go:generate comment:
[03:31:44 ](https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Ci_Tests_CheckGeneratedCode/4275261?buildTab=log&focusLine=4747&linesState=4747)    pkg/cmd/roachtest/tests/drt.go://go:generate mockgen -package tests -destination drt_generated.go . PromClient
[03:31:44 ](https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Ci_Tests_CheckGeneratedCode/4275261?buildTab=log&focusLine=4748&linesState=4748)    Please ensure that the equivalent logic to generate these files is
[03:31:44 ](https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Ci_Tests_CheckGeneratedCode/4275261?buildTab=log&focusLine=4749&linesState=4749)    present in the Bazel build as well, then add the line to the
[03:31:44 ](https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_Ci_Tests_CheckGeneratedCode/4275261?buildTab=log&focusLine=4750&linesState=4750)    EXISTING_GO_GENERATE_COMMENTS in build/bazelutil/check.sh.

@ajwerner ajwerner force-pushed the ajwerner/fix-go-mocks-in-bazel branch 5 times, most recently from fa43cbd to f311114 Compare February 5, 2022 04:40
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.

This commit also removes some previously checked in generated mock
files. I wonder if that's going to cause pain?

Release note: None
@ajwerner ajwerner force-pushed the ajwerner/fix-go-mocks-in-bazel branch from f311114 to 4a4dfea Compare February 5, 2022 05:43
@ajwerner
Copy link
Contributor Author

ajwerner commented Feb 5, 2022

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Feb 5, 2022

Build succeeded:

@craig craig bot merged commit 7d9040a into cockroachdb:master Feb 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bazel: gomock bottlenecks build significantly
5 participants