Correct the WITH_VERIFIC_CHECK in the build_openroad.sh docker build.#4221
Correct the WITH_VERIFIC_CHECK in the build_openroad.sh docker build.#4221maliberty wants to merge 2 commits intoThe-OpenROAD-Project:masterfrom
Conversation
Fixes The-OpenROAD-Project#4207 Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
![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 updates the conditional logic for Verific support in the build_openroad.sh script. While the change refines the logic, it introduces a potential regression in the Docker build process where an empty string argument is passed to DockerHelper.sh when Verific is disabled. It is recommended to use a Bash array for options or adjust the quoting, and to utilize the more idiomatic [[ ... ]] syntax for the condition.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request updates build_openroad.sh to use a bash array for the options variable and modifies the conditional check for WITH_VERIFIC. The feedback suggests declaring the options variable as local to restrict its scope and quoting the variable in the conditional check to improve script robustness.
After the WITH_VERIFIC guard fix, `options` is empty when --with-verific
is not passed, and the quoted "${options}" expansion sends one empty
positional argument to DockerHelper.sh, whose catch-all aborts with
`unknown option: `. Switch to a Bash array so the empty case expands
to zero args.
Signed-off-by: Matt Liberty <mliberty@precisioninno.com>
openroad-ci
force-pushed
the
build-openroad-fix-check
branch
from
6212823 to
04fd4f7
Compare
1 participant
Fixes #4207