Repository navigation
Conversation
Chesterton's fence: 6977921 moved regression_rule_test's openroad to cfg=exec so that `bazelisk test -c opt ...` built OpenROAD once, because bazel-orfs flow tests in this repo also ran openroad as a build tool (exec). Those tests are gone (The-OpenROAD-Project#11546); nothing here uses //:openroad as a tool any more. The fence now causes the duplicate build it was put up to avoid: cc_tests, hier_case_test's runner and the openroadpy extension that the Python regression tests load are all cfg=target, so a mixed test run compiled the OpenROAD libraries in both configurations. cquery of //src/gpl/test:ar01-tcl_test + //src/odb/test:test_block-py_test: before: //src/gpl:gpl in 2 configurations (exec + target) after: //src/gpl:gpl in 1 configuration (target) It also needed ba87b1b's workaround, since the sanitizer configs only instrument the target configuration: a second openroad_sanitized attr, a select() pair and //bazel:sanitizer_build. With the binary under test in cfg=target, all of that goes away and the sanitizer configs instrument the Tcl tests without a special case. POLA: `bazelisk build :openroad` followed by `bazelisk test` now reuses the same binary; drop the docs explaining why it did not. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
oharboe
requested review from
maliberty
and removed request for
precisionmoon and
sombraSoft
October 7, 2026 07:40
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request simplifies the Bazel build configuration by consolidating the OpenROAD executable to always use the cfg = "target" configuration, removing the separate openroad_sanitized attribute and the sanitizer_build config setting group. Documentation has been updated to reflect that building and testing now share the same binary, preventing redundant builds. Feedback on these changes suggests marking the openroad attribute as mandatory = True in test/regression.bzl to avoid potential analysis-time failures if the rule is instantiated directly.
The impl dereferences it unconditionally. The sanitizer select() that could leave it unset is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
POLA: the regression tests now use the same
cfg=targetopenroadthatbazelisk build :openroadbuilds, removing a vestige of bazel-orfs.Chesterton's fence
6977921 switched the regression tests'
openroadtocfg = "exec"because bazel-orfs tests in this repo also usedopenroadas a build tool (exec), and sharing that configuration avoided building OpenROAD twice.Those bazel-orfs tests were removed in #11546, so the reason for the fence is gone. This PR removes the leftovers:
cfg = "exec"on the regression tests'openroadattr becomescfg = "target".openroad_sanitizedattr, itsselect()and//bazel:sanitizer_build, which were added only to work around the exec configuration, are removed.🤖 Generated with Claude Code