docs: Add DependencyInstaller.sh functionalities(-local and -prefix) to Build.md#9649
docs: Add DependencyInstaller.sh functionalities(-local and -prefix) to Build.md#9649alokkumardalei-wq wants to merge 3 commits intoThe-OpenROAD-Project:masterfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request aims to improve build process documentation and scripting by adding documentation for DependencyInstaller.sh arguments to Build.md and extending Build.sh to support -local and -prefix arguments for specifying installation directories. However, a critical command injection vulnerability has been identified in etc/Build.sh. The new -prefix argument is appended to a string that is later executed using eval, allowing for arbitrary code execution if a malicious path is provided. It is highly recommended to avoid eval and use bash arrays for command arguments to ensure safe execution. Furthermore, consider completing the documentation in Build.md and improving argument handling in Build.sh to warn users about overriding installation paths, which would enhance usability and prevent confusion.
etc/Build.sh
Outdated
| cmakeOptions+=" -DCMAKE_INSTALL_PREFIX=${HOME}/.local" | ||
| ;; | ||
| -prefix=*) | ||
| cmakeOptions+=" -DCMAKE_INSTALL_PREFIX=${1#*=}" |
There was a problem hiding this comment.
A critical command injection vulnerability exists here. The newly added -prefix argument on line 89 allows arbitrary input to be appended to cmakeOptions without sanitization, which is later executed by eval. This can be exploited to execute arbitrary shell commands (e.g., -prefix='$(touch /tmp/pwned)'). It is highly recommended to avoid eval and use bash arrays for command arguments to ensure safe execution. Furthermore, when multiple install-path arguments like -local and -prefix are used, the last one silently overrides others, which can be confusing. Consider adding a warning when an existing install path is being overridden, similar to etc/DependencyInstaller.sh.
docs/user/Build.md
Outdated
| Options: | ||
| -all Install all dependencies (base and common). Requires privileged access. | ||
| -base Install base dependencies using package managers. Requires privileged access. | ||
| -common Install common dependencies. | ||
| -eqy Install equivalence dependencies (yosys, eqy, sby). | ||
| -prefix=DIR Install common dependencies in a user-specified directory. | ||
| -local Install common dependencies in ${HOME}/.local. | ||
| -ci Install dependencies required for CI. | ||
| -nocert Disable certificate checks for downloads. | ||
| -skip-system-or-tools Skip searching for a system-installed or-tools library. | ||
| -save-deps-prefixes=FILE Save OpenROAD build arguments to FILE. | ||
| -constant-build-dir Use a constant build directory instead of a random one. | ||
| -threads=<N> Limit the number of compiling threads. | ||
| -yosys-ver=<VERSION> Specify a custom Yosys version. Used for ORFS. | ||
| -verbose Show all output from build commands. |
There was a problem hiding this comment.
The list of options from DependencyInstaller.sh is incomplete. It's missing the -h, -help option. For completeness and to avoid user confusion, it should be included.
| Options: | |
| -all Install all dependencies (base and common). Requires privileged access. | |
| -base Install base dependencies using package managers. Requires privileged access. | |
| -common Install common dependencies. | |
| -eqy Install equivalence dependencies (yosys, eqy, sby). | |
| -prefix=DIR Install common dependencies in a user-specified directory. | |
| -local Install common dependencies in ${HOME}/.local. | |
| -ci Install dependencies required for CI. | |
| -nocert Disable certificate checks for downloads. | |
| -skip-system-or-tools Skip searching for a system-installed or-tools library. | |
| -save-deps-prefixes=FILE Save OpenROAD build arguments to FILE. | |
| -constant-build-dir Use a constant build directory instead of a random one. | |
| -threads=<N> Limit the number of compiling threads. | |
| -yosys-ver=<VERSION> Specify a custom Yosys version. Used for ORFS. | |
| -verbose Show all output from build commands. | |
| Options: | |
| -all Install all dependencies (base and common). Requires privileged access. | |
| -base Install base dependencies using package managers. Requires privileged access. | |
| -common Install common dependencies. | |
| -eqy Install equivalence dependencies (yosys, eqy, sby). | |
| -prefix=DIR Install common dependencies in a user-specified directory. | |
| -local Install common dependencies in ${HOME}/.local. | |
| -ci Install dependencies required for CI. | |
| -nocert Disable certificate checks for downloads. | |
| -skip-system-or-tools Skip searching for a system-installed or-tools library. | |
| -save-deps-prefixes=FILE Save OpenROAD build arguments to FILE. | |
| -constant-build-dir Use a constant build directory instead of a random one. | |
| -threads=<N> Limit the number of compiling threads. | |
| -yosys-ver=<VERSION> Specify a custom Yosys version. Used for ORFS. | |
| -verbose Show all output from build commands. | |
| -h, -help Show this help message. |
caa7d76 to
11ba05a
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@gemini-cli /review |
|
Hello @maliberty |
|
The problem with old issues is that some information may be out of date. The recommendation is now to run |
|
Thank you @maliberty for the clarification. I have updated Build.md to remove the direct instructions for DependencyInstaller.sh and instead recommend using setup.sh scripts to install dependencies. Please a have look onto this and tell me if any other add ons are to be made . Thank you. |
e3af96c to
296af7c
Compare
…efix in Build.sh Signed-off-by: alokkumardalei-wq <alokkumardalei2@gmail.com>
Signed-off-by: alokkumardalei-wq <alokkumardalei2@gmail.com>
Signed-off-by: alokkumardalei-wq <alokkumardalei2@gmail.com>
296af7c to
463e983
Compare
What does this PR do?
Fixes #4044.
In this PR we have:
With these instructions if users follow they
will install the OpenRoad dependencies to ~/.local
rather directly into /usr/local which will make difficult for users to uninstall things later.
Verification for build.sh script support for -local and -prefix arguments:
Run this command in your terminal:
./etc/Build.sh -local -cmake="-DRUN_CMAKE=OFF"Output:
if you found the line saying -- Install prefix: /Users/username/.local ,you are good to go.
Run this command on your terminal providing a custom OpenRoad folder .
./etc/Build.sh -prefix=/tmp/my_custom_openroad_folder -cmake="-DRUN_CMAKE=OFF"Output:
if you see the line saying -- Install prefix: /tmp/my_custom_openroad_folder ,then you are good to go.