gcassert: fix issues when running under bazel#103471
gcassert: fix issues when running under bazel#103471craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
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 |
bb3cbba to
dcfd840
Compare
rickystewart
left a comment
There was a problem hiding this comment.
What the hell is going on with the update to dev??
just testing 😄 |
ea9eeaf to
e23960d
Compare
|
Thank you for updating your pull request. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
fbf3d98 to
bb27891
Compare
da7ad89 to
baebcf7
Compare
baebcf7 to
3f6f9ae
Compare
3f6f9ae to
581517b
Compare
|
This is ready for review. Generated code and GitHub CI failures should be fixed when I merge jordanlewis/gcassert#12 and point to it instead of my fork. |
6524490 to
beade4d
Compare
| set -xeuo pipefail | ||
|
|
||
| # GCAssert requirements -- start | ||
| PATH="$(dirname $(bazel run @go_sdk//:bin/go --run_under=realpath)):$PATH" |
rickystewart
left a comment
There was a problem hiding this comment.
Commit message and PR description are no longer accurate
go.mod
Outdated
| github.com/jaegertracing/jaeger v1.18.1 | ||
| github.com/jordan-wright/email v4.0.1-0.20210109023952-943e75fe5223+incompatible | ||
| github.com/jordanlewis/gcassert v0.0.0-20221027203946-81f097ad35a0 | ||
| github.com/jordanlewis/gcassert v0.0.0-20230607150025-9012d8e73bdd |
There was a problem hiding this comment.
We don't need to/shouldn't be updating gcassert any more, right? The bazel patches that were applied are all faulty and we don't seem to be using them anyway.
There was a problem hiding this comment.
We still need to update gcassert to pull changes where we get the logs and can run in debug mode. In the current version we do not capture build logs.
pkg/cmd/generate-cgo/main.go
Outdated
| cppFlags := fmt.Sprintf("-I%s", filepath.Join(jemallocDir, "include")) | ||
| ldFlags := fmt.Sprintf("-L%s -L%s", filepath.Join(jemallocDir, "lib"), filepath.Join(projDir, "lib")) | ||
|
|
||
| tempToPersistentDirs := make(map[string]string, 3) |
There was a problem hiding this comment.
Nit: srcToPersistentDirs seems more appopriate as a name since these files don't come from "temporary" directories.
Updated |
ensuring that we generate go and cgo files, and pass a path to C and C++ compilers to the lint test process. With these changes, `gcassert` can run as part of the `Lint` build config and doesn't need its own build config so the gcassert build config scripts are deleted. Release note: None Epic: CRDB-8349 Closes cockroachdb#65485
|
TFTR! bors r=rickystewart |
|
Build succeeded: |
|
Nice work figuring this out! |
This code change fixes issues when running gcassert under bazel by ensuring that we generate go and cgo files, and pass a path to C and C++ compilers to the lint test process.
With these changes,
gcassertcan run as part of theLintbuild config and doesn't need its own build config so the gcassert build config scripts are deleted.Release note: None
Epic: CRDB-8349
Closes #65485