build: allow disabling metamorphic constants with tag#77423
build: allow disabling metamorphic constants with tag#77423craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
0984499 to
37aafa1
Compare
Previously cockroachdb#76309 added the ability to disable metamorphic constants with env variable COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING which disabled the metamorphic constants when set to true. For some tools however (execgen, roachpb/gen) setting an evironment variable may not be always possible. So this PR adds the ability to disable the metamorphic testing through tag `metamorphic_disable`. It would seem that similar thing could be achieved by using the crdb_test_off tag instead. With bazel however this tag is ignored. Fixes cockroachdb#76363 Release justification: Non-production code changes Release note: None
37aafa1 to
633a677
Compare
| go_binary( | ||
| name = "gen", | ||
| embed = [":gen_lib"], | ||
| gotags = ["metamorphic_disable"], |
There was a problem hiding this comment.
Due to the way gotags works, this change would require re-compiling all the stuff that pkg/roachpb/gen and execgen depend on including the entire Go standard library with the new gotags. I suspect this would have a bigger performance impact than we want considering how small this conditional branch is.
I would expect instead that we set the environment variable wherever it needs to be set -- i.e. in the env attribute of the call to ctx.actions.run() in pkg/roachpb/gen.bzl and in the various places we call into execgen as part of the build. Is there a reason we haven't done that?
There was a problem hiding this comment.
We went over this in DM and I don't think this is anything to be concerned about for now, we can follow-up on it if it turns out to be relevant.
| go_binary( | ||
| name = "gen", | ||
| embed = [":gen_lib"], | ||
| gotags = ["metamorphic_disable"], |
There was a problem hiding this comment.
We went over this in DM and I don't think this is anything to be concerned about for now, we can follow-up on it if it turns out to be relevant.
|
bors r+ |
|
Build succeeded: |
Previously #76309 added the ability to disable metamorphic constants
with env variable COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING which
disabled the metamorphic constants when set to true. For some tools
however (execgen, roachpb/gen) setting an evironment variable may not
be always possible. So this PR adds the ability to disable the
metamorphic testing through tag
metamorphic_disable. It would seemthat similar thing could be achieved by using the crdb_test_off tag
instead. With bazel however this tag is ignored.
Fixes #76363
Release justification: Non-production code changes
Release note: None