bazel: replace genrule sed/cat with skylib rules for hermeticity#9737
bazel: replace genrule sed/cat with skylib rules for hermeticity#9737oharboe wants to merge 1 commit intoThe-OpenROAD-Project:masterfrom
Conversation
Use expand_template and copy_file from bazel_skylib instead of genrule with sed/cat in the OpenMP overlay BUILD file. This avoids depending on host shell tools and sets a better example for contributors who copy existing patterns. Remaining hermeticity nits for later: - BUILD.bazel OpenRoadVersion genrule uses grep/cut/printf - src/sta StaConfig genrule uses echo -e (fix pending in sta repo) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Øyvind Harboe <oyvind.harboe@zylin.com>
|
@hzeller FYI |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves build hermeticity by replacing non-hermetic genrule calls that use shell tools like sed and cat with their bazel_skylib equivalents, expand_template and copy_file. The changes are correct and achieve the stated goal. I've added a couple of suggestions to further improve maintainability by addressing hardcoded version information and ensuring build rules correctly reflect the intent for template files.
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@codex review |
|
Codex Review: Didn't find any major issues. Swish! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Nice! |
3 participants
Use expand_template and copy_file from bazel_skylib instead of genrule with sed/cat in the OpenMP overlay BUILD file. This avoids depending on host shell tools and sets a better example for contributors who copy existing patterns.
Remaining hermeticity nits for later: