etc: Fixes for DependencyInstaller.sh by titan73 · Pull Request #10037 · The-OpenROAD-Project/OpenROAD

titan73 · 2026-04-02T17:14:25Z

No description provided.

The eval was lost in 9273cce. Signed-off-by: Christian Costa <titan.costa@gmail.com>

…scription with them. Signed-off-by: Christian Costa <titan.costa@gmail.com>

gemini-code-assist

Code Review

This pull request modifies etc/DependencyInstaller.sh by adjusting the logging output in the _execute function and prepending eval to several wget commands across various installation functions. The review feedback consistently identifies the use of eval as a security risk that can lead to word-splitting issues, recommending the implementation of a shell function as a safer and more robust alternative for handling command aliases or flags.

gemini-code-assist · 2026-04-02T17:16:22Z

etc/DependencyInstaller.sh

                cmake_checksum=${CMAKE_CHECKSUM_X86_64}
            fi
-            _execute "Downloading CMake..." wget "https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION_SMALL}/cmake-${CMAKE_VERSION_SMALL}-linux-${arch}.sh"
+            _execute "Downloading CMake..." eval wget "https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION_SMALL}/cmake-${CMAKE_VERSION_SMALL}-linux-${arch}.sh"


Using eval to force alias expansion is discouraged in shell scripts. It introduces security risks and can cause issues with word splitting if variables contain spaces. A more robust approach is to define a shell function instead of an alias for wget when the -nocert flag is used. This allows the command to be called normally without eval. For example, in the -nocert block, you could use: wget() { command wget --no-check-certificate "$@"; }

Suggested change

_execute "Downloading CMake..." eval wget "https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION_SMALL}/cmake-${CMAKE_VERSION_SMALL}-linux-${arch}.sh"

_execute "Downloading CMake..." wget "https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION_SMALL}/cmake-${CMAKE_VERSION_SMALL}-linux-${arch}.sh"

gemini-code-assist · 2026-04-02T17:16:23Z

etc/DependencyInstaller.sh

                local url="${mirror}/bison-${BISON_VERSION}.tar.gz"
                log "Trying to download bison from: $url"
-                if wget "$url"; then
+                if eval wget "$url"; then


Using eval here is risky. If the $url variable contains spaces or special characters, eval will cause the shell to re-parse the command, leading to incorrect argument splitting. Using a shell function instead of an alias for wget is a safer and cleaner alternative.

Suggested change

if eval wget "$url"; then

if wget "$url"; then

gemini-code-assist · 2026-04-02T17:16:23Z

etc/DependencyInstaller.sh

        (
            cd "${BASE_DIR}"
-            _execute "Downloading Flex..." wget https://github.com/westes/flex/releases/download/v${FLEX_VERSION}/flex-${FLEX_VERSION}.tar.gz
+            _execute "Downloading Flex..." eval wget https://github.com/westes/flex/releases/download/v${FLEX_VERSION}/flex-${FLEX_VERSION}.tar.gz


Avoid using eval to expand aliases. Use a shell function for wget instead to maintain security and avoid word-splitting issues.

Suggested change

_execute "Downloading Flex..." eval wget https://github.com/westes/flex/releases/download/v${FLEX_VERSION}/flex-${FLEX_VERSION}.tar.gz

_execute "Downloading Flex..." wget https://github.com/westes/flex/releases/download/v${FLEX_VERSION}/flex-${FLEX_VERSION}.tar.gz

gemini-code-assist · 2026-04-02T17:16:23Z

etc/DependencyInstaller.sh

            cd "${BASE_DIR}"
            local tar_name="v${SWIG_VERSION}.tar.gz"
-            _execute "Downloading SWIG..." wget "https://github.com/swig/swig/archive/${tar_name}"
+            _execute "Downloading SWIG..." eval wget "https://github.com/swig/swig/archive/${tar_name}"


Avoid using eval to expand aliases. Use a shell function for wget instead.

Suggested change

_execute "Downloading SWIG..." eval wget "https://github.com/swig/swig/archive/${tar_name}"

_execute "Downloading SWIG..." wget "https://github.com/swig/swig/archive/${tar_name}"

gemini-code-assist · 2026-04-02T17:16:23Z

etc/DependencyInstaller.sh

        cd "${BASE_DIR}"
        local pcre_tar_name="pcre2-${PCRE_VERSION}.tar.gz"
-        _execute "Downloading PCRE..." wget "https://github.com/PCRE2Project/pcre2/releases/download/pcre2-${PCRE_VERSION}/${pcre_tar_name}"
+        _execute "Downloading PCRE..." eval wget "https://github.com/PCRE2Project/pcre2/releases/download/pcre2-${PCRE_VERSION}/${pcre_tar_name}"


Avoid using eval to expand aliases. Use a shell function for wget instead.

Suggested change

_execute "Downloading PCRE..." eval wget "https://github.com/PCRE2Project/pcre2/releases/download/pcre2-${PCRE_VERSION}/${pcre_tar_name}"

_execute "Downloading PCRE..." wget "https://github.com/PCRE2Project/pcre2/releases/download/pcre2-${PCRE_VERSION}/${pcre_tar_name}"

gemini-code-assist · 2026-04-02T17:16:23Z

etc/DependencyInstaller.sh

        (
            cd "${BASE_DIR}"
-            _execute "Downloading gtest..." wget "https://github.com/google/googletest/archive/refs/tags/v${GTEST_VERSION}.zip"
+            _execute "Downloading gtest..." eval wget "https://github.com/google/googletest/archive/refs/tags/v${GTEST_VERSION}.zip"


Avoid using eval to expand aliases. Use a shell function for wget instead.

Suggested change

_execute "Downloading gtest..." eval wget "https://github.com/google/googletest/archive/refs/tags/v${GTEST_VERSION}.zip"

_execute "Downloading gtest..." wget "https://github.com/google/googletest/archive/refs/tags/v${GTEST_VERSION}.zip"

gemini-code-assist · 2026-04-02T17:16:23Z

etc/DependencyInstaller.sh

        (
            cd "${BASE_DIR}"
-            _execute "Downloading Abseil..." wget "https://github.com/abseil/abseil-cpp/releases/download/${ABSL_VERSION}/abseil-cpp-${ABSL_VERSION}.tar.gz"
+            _execute "Downloading Abseil..." eval wget "https://github.com/abseil/abseil-cpp/releases/download/${ABSL_VERSION}/abseil-cpp-${ABSL_VERSION}.tar.gz"


Avoid using eval to expand aliases. Use a shell function for wget instead.

Suggested change

_execute "Downloading Abseil..." eval wget "https://github.com/abseil/abseil-cpp/releases/download/${ABSL_VERSION}/abseil-cpp-${ABSL_VERSION}.tar.gz"

_execute "Downloading Abseil..." wget "https://github.com/abseil/abseil-cpp/releases/download/${ABSL_VERSION}/abseil-cpp-${ABSL_VERSION}.tar.gz"

gemini-code-assist · 2026-04-02T17:16:23Z

etc/DependencyInstaller.sh

            fi
            local or_tools_file="or-tools_${arch}_${os}-${os_version}_cpp_v${OR_TOOLS_VERSION_SMALL}.tar.gz"
-            _execute "Downloading or-tools..." wget "https://github.com/google/or-tools/releases/download/v${OR_TOOLS_VERSION_BIG}/${or_tools_file}"
+            _execute "Downloading or-tools..." eval wget "https://github.com/google/or-tools/releases/download/v${OR_TOOLS_VERSION_BIG}/${or_tools_file}"


Avoid using eval to expand aliases. Use a shell function for wget instead.

Suggested change

_execute "Downloading or-tools..." eval wget "https://github.com/google/or-tools/releases/download/v${OR_TOOLS_VERSION_BIG}/${or_tools_file}"

_execute "Downloading or-tools..." wget "https://github.com/google/or-tools/releases/download/v${OR_TOOLS_VERSION_BIG}/${or_tools_file}"

gemini-code-assist · 2026-04-02T17:16:24Z

etc/DependencyInstaller.sh

    local arch=amd64
    local pandoc_version="3.1.11.1"
-    _execute "Downloading pandoc..." wget "https://github.com/jgm/pandoc/releases/download/${pandoc_version}/pandoc-${pandoc_version}-linux-${arch}.tar.gz"
+    _execute "Downloading pandoc..." eval wget "https://github.com/jgm/pandoc/releases/download/${pandoc_version}/pandoc-${pandoc_version}-linux-${arch}.tar.gz"


Avoid using eval to expand aliases. Use a shell function for wget instead.

Suggested change

_execute "Downloading pandoc..." eval wget "https://github.com/jgm/pandoc/releases/download/${pandoc_version}/pandoc-${pandoc_version}-linux-${arch}.tar.gz"

_execute "Downloading pandoc..." wget "https://github.com/jgm/pandoc/releases/download/${pandoc_version}/pandoc-${pandoc_version}-linux-${arch}.tar.gz"

gemini-code-assist · 2026-04-02T17:16:24Z

etc/DependencyInstaller.sh


    if _version_compare "${1}" -lt "24.04"; then
-        _execute "Downloading LLVM install script..." wget https://apt.llvm.org/llvm.sh
+        _execute "Downloading LLVM install script..." eval wget https://apt.llvm.org/llvm.sh


Avoid using eval to expand aliases. Use a shell function for wget instead.

Suggested change

_execute "Downloading LLVM install script..." eval wget https://apt.llvm.org/llvm.sh

_execute "Downloading LLVM install script..." wget https://apt.llvm.org/llvm.sh

github-actions · 2026-04-02T17:19:04Z

clang-tidy review says "All clean, LGTM! 👍"

Divinesoumyadip · 2026-04-02T17:19:40Z

The eval approach for handling --no-check-certificate will work but it does introduce word-splitting risks if any URL variable ever contains spaces.
A cleaner way would be to define a shell function in the -nocert block:
wget() { command wget --no-check-certificate "$@"; }
This way all subsequent wget calls work normally without needing eval at all.

Signed-off-by: Christian Costa <titan.costa@gmail.com>

github-actions · 2026-04-02T18:42:00Z

clang-tidy review says "All clean, LGTM! 👍"

titan73 · 2026-04-02T18:44:07Z

I replaced the eval with a variable which is empty or with the nocert option

maliberty · 2026-04-03T22:11:51Z

Please provide a description of the problem being fixed.

titan73 · 2026-04-04T09:10:46Z

Please provide a description of the problem being fixed.

I added the -nocert option to disable certificate checks as I had issue with my company firewall with 1234ff1.
It puts a alias on wget but as alias are expanded when the file is read and an eval was needed.
The "eval" was lost during a refactoring with 9273cce.

The 1st patch fixes that but actually I changed the method with the 3rd patch by using a variable OPT_NOCERT instead as Gemini complained.

The 2nd patch is unrelated and just cosmetic. When running the script. A message is printed with 6 dots:
Downloading CMake......
because 3 dots are printed in the _execute function:
echo -n "${BLUE}${BOLD}[INFO]${NC} ${description}..."
and for all the execute calls:
_execute "Downloading CMake..." wget XXX
The patch remove the redundant dots in _execute.

If needed,I can split the PR with patch 1 & 3 squashed and another for the patch 2.

github-actions · 2026-04-04T09:22:26Z

clang-tidy review says "All clean, LGTM! 👍"

titan73 added 2 commits April 2, 2026 19:06

etc: Fix -no-cert option of deps installer.

0ece531

The eval was lost in 9273cce. Signed-off-by: Christian Costa <titan.costa@gmail.com>

etc: Remove ... from _execute message as all _execute calls give a de…

3716a51

…scription with them. Signed-off-by: Christian Costa <titan.costa@gmail.com>

gemini-code-assist bot reviewed Apr 2, 2026

View reviewed changes

etc: Actually use a variable instead of an alias in deps installer.

cd4bbb3

Signed-off-by: Christian Costa <titan.costa@gmail.com>

titan73 closed this Apr 4, 2026

titan73 reopened this Apr 4, 2026

maliberty requested a review from sombraSoft April 6, 2026 14:41

	_execute "Downloading CMake..." eval wget "https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION_SMALL}/cmake-${CMAKE_VERSION_SMALL}-linux-${arch}.sh"
	_execute "Downloading CMake..." wget "https://github.com/Kitware/CMake/releases/download/v${CMAKE_VERSION_SMALL}/cmake-${CMAKE_VERSION_SMALL}-linux-${arch}.sh"

	_execute "Downloading Flex..." eval wget https://github.com/westes/flex/releases/download/v${FLEX_VERSION}/flex-${FLEX_VERSION}.tar.gz
	_execute "Downloading Flex..." wget https://github.com/westes/flex/releases/download/v${FLEX_VERSION}/flex-${FLEX_VERSION}.tar.gz

	_execute "Downloading SWIG..." eval wget "https://github.com/swig/swig/archive/${tar_name}"
	_execute "Downloading SWIG..." wget "https://github.com/swig/swig/archive/${tar_name}"

	_execute "Downloading PCRE..." eval wget "https://github.com/PCRE2Project/pcre2/releases/download/pcre2-${PCRE_VERSION}/${pcre_tar_name}"
	_execute "Downloading PCRE..." wget "https://github.com/PCRE2Project/pcre2/releases/download/pcre2-${PCRE_VERSION}/${pcre_tar_name}"

	_execute "Downloading gtest..." eval wget "https://github.com/google/googletest/archive/refs/tags/v${GTEST_VERSION}.zip"
	_execute "Downloading gtest..." wget "https://github.com/google/googletest/archive/refs/tags/v${GTEST_VERSION}.zip"

	_execute "Downloading Abseil..." eval wget "https://github.com/abseil/abseil-cpp/releases/download/${ABSL_VERSION}/abseil-cpp-${ABSL_VERSION}.tar.gz"
	_execute "Downloading Abseil..." wget "https://github.com/abseil/abseil-cpp/releases/download/${ABSL_VERSION}/abseil-cpp-${ABSL_VERSION}.tar.gz"

	_execute "Downloading or-tools..." eval wget "https://github.com/google/or-tools/releases/download/v${OR_TOOLS_VERSION_BIG}/${or_tools_file}"
	_execute "Downloading or-tools..." wget "https://github.com/google/or-tools/releases/download/v${OR_TOOLS_VERSION_BIG}/${or_tools_file}"

	_execute "Downloading pandoc..." eval wget "https://github.com/jgm/pandoc/releases/download/${pandoc_version}/pandoc-${pandoc_version}-linux-${arch}.tar.gz"
	_execute "Downloading pandoc..." wget "https://github.com/jgm/pandoc/releases/download/${pandoc_version}/pandoc-${pandoc_version}-linux-${arch}.tar.gz"

	_execute "Downloading LLVM install script..." eval wget https://apt.llvm.org/llvm.sh
	_execute "Downloading LLVM install script..." wget https://apt.llvm.org/llvm.sh

Conversation

titan73 commented Apr 2, 2026

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 2, 2026

Uh oh!

Divinesoumyadip commented Apr 2, 2026

Uh oh!

github-actions bot commented Apr 2, 2026

Uh oh!

titan73 commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maliberty commented Apr 3, 2026

Uh oh!

titan73 commented Apr 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

titan73 commented Apr 2, 2026 •

edited

Loading

titan73 commented Apr 4, 2026 •

edited

Loading