dev,generate-cgo: externalize cgo files gen into a seperate binary #103140
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
healthy-pod
force-pushed
the
fix-gcassert
branch
2 times, most recently
from
fded78d
to
a1d4386
Compare
Previously, we only needed to gen cgo files using `dev` so that was were that logic lived. Now, we need to gen cgo files outside dev to fix an issue with gcassert [see cockroachdb#65485]. This code change externalizes the code into a separate binary and let's `dev gen cgo` point to it. Release note: None Epic: None Informs cockroachdb#65485
healthy-pod
force-pushed
the
fix-gcassert
branch
4 times, most recently
from
fec22ef
to
4b5f433
Compare
rickystewart
approved these changes
May 15, 2023
| @@ -4,6 +4,9 @@ set -xeuo pipefail | |||
|
|
|||
| bazel build @com_github_jordanlewis_gcassert//cmd/gcassert:gcassert --config=ci | |||
| bazel run //pkg/gen:code | |||
| WORKSPACE=$(bazel info workspace --color=no) | |||
| BAZEL_BIN=$(bazel info bazel-bin --color=no) | |||
| bazel run //pkg/cmd/generate-cgo:generate-cgo -- --workspace $WORKSPACE --bazel-bin $BAZEL_BIN | |||
There was a problem hiding this comment.
Nit: Can just use $PWD or pwd instead of bazel info workspace.
|
|
||
| func init() { | ||
| flag.StringVar(&workspace, "workspace", "", "workspace path") | ||
| flag.StringVar(&bazelBin, "bazel-bin", "", "path to bazel/bin") |
There was a problem hiding this comment.
Nit: Consider that generate-cgo can just call into bazel info bazel-bin itself, since it is the one doing the builds anyway, instead of requiring it to be passed in as an argument.
Release note: None Epic: none Closes cockroachdb#65485
|
superseded by #104554 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, we only needed to gen cgo files using
devso that was were that logic lived. Now, we need to gen cgo files outside dev to fix an issue with gcassert [see #65485].The first commit externalizes the code into a separate binary and let's
dev gen cgopoint to it. The second commit upgradesgcassertand generatescgofiles before runninggcassert.Release note: None
Epic: None
Closes #65485