Skip to content

feat: add idempotency checks for third-party dependencies #413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
13eholder wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: add idempotency checks for third-party dependencies #413

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 60 additions & 38 deletions scripts/install_dependency_ubuntu2404.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,16 @@ kernel_ge() {
(( major > need_major || (major == need_major && minor >= need_minor) ))
}

# arg 1: Library name (for display)
# arg 2: Detection command (to check whether the core header file or library file exists)
is_installed() {
Copy link
Collaborator

@thweetkomputer thweetkomputer Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dependency libs may be updated. The idempotency check should be enabled by a parameter which is by default false.

if eval "$2" >/dev/null 2>&1; then
echo "--- [SKIP] $1 is already installed. ---"
return 0
fi
return 1
}

if ! kernel_ge 6 6; then
echo "Kernel $(uname -r) < 6.6, exit." >&2
exit 1
Expand Down Expand Up @@ -52,22 +62,27 @@ DEBIAN_FRONTEND=noninteractive sudo apt-get install -y --no-install-recommends \
libjsoncpp-dev libleveldb-dev libsnappy-dev zlib1g-dev lcov

# Install glog
git clone https://github.com/eloqdata/glog.git glog
cd glog
cmake -S . -B build -G "Unix Makefiles"
cmake --build build -j$(nproc)
sudo cmake --build build --target install
cd ../ && rm -rf glog
if ! is_installed "glog" "ls /usr/local/lib/libglog.so"; then
git clone https://github.com/eloqdata/glog.git glog
cd glog
cmake -S . -B build -G "Unix Makefiles"
cmake --build build -j$(nproc)
sudo cmake --build build --target install
cd ../ && rm -rf glog
fi
Comment on lines +65 to +72
Copy link

@coderabbitai coderabbitai bot Mar 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Pin glog to a specific tag or commit for reproducible builds.

Unlike liburing, AWS SDK, and Catch2 which specify exact versions, glog is cloned from HEAD of the default branch. Upstream changes could break builds without notice.

Additionally, quote $(nproc) to avoid word splitting (shellcheck SC2046).

🔧 Suggested fix 
 if ! is_installed "glog" "ls /usr/local/lib/libglog.so"; then
-    git clone https://github.com/eloqdata/glog.git glog
+    git clone --branch <pinned-tag-or-commit> https://github.com/eloqdata/glog.git glog
     cd glog
     cmake -S . -B build -G "Unix Makefiles"
-    cmake --build build -j$(nproc)
+    cmake --build build -j "$(nproc)"
     sudo cmake --build build --target install
     cd ../ && rm -rf glog
 fi
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 69-69: Quote this to prevent word splitting.

(SC2046)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/install_dependency_ubuntu2404.sh` around lines 65 - 72, The glog
installation block (guarded by is_installed) clones the repo at HEAD which is
not pinned; change the git clone to fetch a specific tag or commit (e.g., git
clone --branch <TAG_OR_COMMIT> --depth 1 https://github.com/eloqdata/glog.git
glog or git -C glog checkout <COMMIT>) so builds are reproducible, and also
quote the nproc substitution in the cmake build call (use "-j$(nproc)" ->
"-j\"$(nproc)\"" or simply "-j\"$(nproc)\"" in the cmake --build invocation) to
avoid word-splitting; update the glog cleanup (cd ../ && rm -rf glog) only after
successful install in the same block.


# Install liburing
git clone https://github.com/axboe/liburing.git liburing
cd liburing
git checkout tags/liburing-2.6
./configure --cc=gcc --cxx=g++
make -j$(nproc) && sudo make install
cd .. && rm -rf liburing
if ! is_installed "liburing" "ls /usr/lib/liburing.so.2.6"; then
git clone https://github.com/axboe/liburing.git liburing
cd liburing
git checkout tags/liburing-2.6
./configure --cc=gcc --cxx=g++
make -j$(nproc) && sudo make install
Copy link

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -nF '-j$(nproc)' scripts/install_dependency_ubuntu2404.sh

Repository: eloqdata/eloqstore

Length of output: 91

🏁 Script executed:

#!/bin/bash
# First, let's see if the file exists and how many lines it has
wc -l scripts/install_dependency_ubuntu2404.sh

# Read lines around 80 and 115 to verify the pattern
echo "=== Lines 75-85 ==="
sed -n '75,85p' scripts/install_dependency_ubuntu2404.sh

echo "=== Lines 110-120 ==="
sed -n '110,120p' scripts/install_dependency_ubuntu2404.sh

# Search for all occurrences of -j$ pattern
echo "=== All -j\$ patterns ==="
grep -n '\-j$' scripts/install_dependency_ubuntu2404.sh || echo "Pattern not found with literal $"

# Try searching without escape
grep -n 'j.*nproc' scripts/install_dependency_ubuntu2404.sh

Repository: eloqdata/eloqstore

Length of output: 1120

🏁 Script executed:

#!/bin/bash
# Check if ShellCheck is available and what it says about this pattern
which shellcheck > /dev/null 2>&1 && {
  echo "=== ShellCheck output for line 80 context ==="
  shellcheck -S style scripts/install_dependency_ubuntu2404.sh 2>&1 | grep -A2 -B2 "nproc" || echo "No shellcheck issues found or tool not available"
} || echo "ShellCheck not available"

# Verify the exact pattern - all -j$(nproc) occurrences
echo "=== All -j\$(nproc) occurrences ==="
grep -n '\-j\$(' scripts/install_dependency_ubuntu2404.sh

Repository: eloqdata/eloqstore

Length of output: 292

🌐 Web query:

ShellCheck SC2046 unquoted command substitution rule explanation

💡 Result:

ShellCheck SC2046 (“Quote this to prevent word splitting”) warns when you use an unquoted command substitution like:

ls -l $(getfilename)

Because after $(...) is expanded, the shell will still apply word splitting (on spaces/tabs/newlines via $IFS) and globbing (* ? [...]). So if the command outputs something like My File.txt, it may turn into two arguments (My and File.txt) or even expand wildcards unexpectedly. The usual fix is to quote the substitution so it becomes exactly one argument:

ls -l "$(getfilename)"

[1][2]

If the command substitution is meant to produce multiple items, don’t rely on splitting $(...); use a loop or read into an array (e.g., read -ra args / readarray) and then expand safely (e.g., "${args[@]}"). [1]

Exception: in rare cases you do want word splitting (e.g., gcc $(pkg-config --libs openssl) ...), and ShellCheck suggests either disabling SC2046 for that line or using an array-based approach instead. [1]

Sources: ShellCheck SC2046 wiki [1]; Greg’s Wiki on quoting, word splitting, and globbing [2].

🏁 Script executed:

#!/bin/bash
# Test if quoted -j flag works with make and cmake
# We can check the man pages or documentation snippets

# Check make documentation format for -j
man make 2>/dev/null | grep -A5 '\-j' | head -20 || echo "man page not available"

# Verify syntax: both make and cmake should accept -j with quoted numeric values
# The key is whether "-j8" and -j"8" are parsed identically

echo "=== Checking if this is a real issue ==="
echo "$(nproc) returns a single number, e.g., 8"
echo "So -j\$(nproc) becomes -j8 (unquoted)"
echo "And -j\"\$(nproc)\" becomes -j\"8\" (quoted)"
echo "Both should be parsed identically by make/cmake as the -j flag with argument 8"

Repository: eloqdata/eloqstore

Length of output: 354

Quote $(nproc) to satisfy ShellCheck SC2046 and follow shell safety best practices.

ShellCheck flags unquoted command substitutions to prevent word-splitting and globbing. While $(nproc) returns a single numeric value with minimal practical risk, quoting it as "$(nproc)" follows best practice for shell safety and consistency. The -j"8" and -j8 syntaxes are functionally equivalent for make and cmake.

Suggested fix 
-    make -j$(nproc) && sudo make install
+    make -j"$(nproc)" && sudo make install
...
-    cmake --build . --config RelWithDebInfo -j$(nproc)
+    cmake --build . --config RelWithDebInfo -j"$(nproc)"
🧰 Tools
🪛 Shellcheck (0.11.0)

[warning] 80-80: Quote this to prevent word splitting.

(SC2046)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/install_dependency_ubuntu2404.sh` at line 80, The make invocation
uses an unquoted command substitution "make -j$(nproc)" which triggers
ShellCheck SC2046; update the script to quote the substitution so it becomes
make -j"$(nproc)" to prevent potential word-splitting/globbing issues and
satisfy shell safety best practices, leaving the rest of the line (&& sudo make
install) unchanged.

cd .. && rm -rf liburing
fi

# Install brpc
if ! is_installed "brpc" "ls /usr/lib/libbrpc.so"; then
git clone https://github.com/eloqdata/brpc.git brpc
Comment on lines +85 to 86
Copy link

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n 'git clone .*brpc' scripts/install_dependency_ubuntu2404.sh -C1

Repository: eloqdata/eloqstore

Length of output: 186

🏁 Script executed:

#!/bin/bash
# Find all git clone commands in the scripts directory to see if pinning is a pattern
rg -n 'git clone' scripts/ -C1

Repository: eloqdata/eloqstore

Length of output: 1461

🏁 Script executed:

#!/bin/bash
# Check the full context around these git clones - look for any version specifications or comments
head -n 130 scripts/install_dependency_ubuntu2404.sh | tail -n +55

Repository: eloqdata/eloqstore

Length of output: 2844

Pin brpc to a specific version for reproducible builds.

The brpc clone on line 86 is unpinned and pulls from the default branch HEAD, which can silently introduce different binaries across runs and environments. The project already pins other dependencies (liburing, AWS SDK, Catch2), making this an inconsistency.

Either add a git checkout after the clone (matching the pattern used for liburing and AWS SDK) or pin the version in the clone command itself (matching Catch2):

Suggested fix 
if ! is_installed "brpc" "ls /usr/lib/libbrpc.so"; then
-git clone https://github.com/eloqdata/brpc.git brpc
+git clone -b <PINNED_TAG> https://github.com/eloqdata/brpc.git brpc
cd brpc

Or use git checkout tags/<VERSION> after the clone, consistent with other dependencies in this script.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/install_dependency_ubuntu2404.sh` around lines 85 - 86, The brpc
clone is unpinned (git clone https://github.com/eloqdata/brpc.git brpc) causing
non-reproducible builds; update the install block that checks
is_installed("brpc", ...) so after cloning the repo you either checkout a
specific tag/commit (e.g., run git -C brpc checkout tags/<VERSION> or git -C
brpc checkout <COMMIT_HASH>) or change the clone to pin a branch/tag (e.g.,
clone with --branch <TAG> --depth 1) to match how liburing/AWS SDK/Catch2 are
pinned; ensure the chosen <VERSION> or commit is stored as the canonical value
used elsewhere in the script.

cd brpc
mkdir build && cd build
Expand All @@ -79,35 +94,42 @@ cmake --build . -j$(nproc)
sudo cp -r ./output/include/* /usr/include/
sudo cp ./output/lib/* /usr/lib/
cd ../../ && rm -rf brpc
fi

# Install AWSSDK
git clone --recurse-submodules https://github.com/aws/aws-sdk-cpp.git aws
cd aws
git checkout tags/1.11.446
mkdir bld && cd bld
cmake .. \
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
-DCMAKE_INSTALL_PREFIX=./output/ \
-DENABLE_TESTING=OFF \
-DBUILD_SHARED_LIBS=ON \
-DFORCE_SHARED_CRT=OFF \
-DBUILD_ONLY="s3"
cmake --build . --config RelWithDebInfo -j$(nproc)
cmake --install . --config RelWithDebInfo
sudo cp -r ./output/include/* /usr/include/
sudo cp -r ./output/lib/* /usr/lib/
cd ../../ && rm -rf aws
AWS_SDK_CHECK_CMD="grep -q 'AWS_SDK_VERSION_MAJOR 1' /usr/include/aws/core/VersionConfig.h 2>/dev/null && \
grep -q 'AWS_SDK_VERSION_MINOR 11' /usr/include/aws/core/VersionConfig.h 2>/dev/null && \
grep -q 'AWS_SDK_VERSION_PATCH 446' /usr/include/aws/core/VersionConfig.h 2>/dev/null"
if ! is_installed "AWS SDK (S3)" "$AWS_SDK_CHECK_CMD"; then
git clone --recurse-submodules https://github.com/aws/aws-sdk-cpp.git aws
cd aws
git checkout tags/1.11.446
mkdir bld && cd bld
cmake .. \
-DCMAKE_BUILD_TYPE=RelWithDebInfo \
-DCMAKE_INSTALL_PREFIX=./output/ \
-DENABLE_TESTING=OFF \
-DBUILD_SHARED_LIBS=ON \
-DFORCE_SHARED_CRT=OFF \
-DBUILD_ONLY="s3"
cmake --build . --config RelWithDebInfo -j$(nproc)
cmake --install . --config RelWithDebInfo
sudo cp -r ./output/include/* /usr/include/
sudo cp -r ./output/lib/* /usr/lib/
cd ../../ && rm -rf aws
fi

# Install Catch2
git clone -b v3.3.2 https://github.com/catchorg/Catch2.git
cd Catch2 && mkdir bld && cd bld
cmake .. \
-DCMAKE_INSTALL_PREFIX=/usr/ \
-DCATCH_BUILD_EXAMPLES=OFF \
-DBUILD_TESTING=OFF
cmake --build . -j4
sudo cmake --install .
cd ../../ && rm -rf Catch2

if ! is_installed "Catch2" "pkg-config --exact-version=3.3.2 catch2-with-main"; then
git clone -b v3.3.2 https://github.com/catchorg/Catch2.git
cd Catch2 && mkdir bld && cd bld
cmake .. \
-DCMAKE_INSTALL_PREFIX=/usr/ \
-DCATCH_BUILD_EXAMPLES=OFF \
-DBUILD_TESTING=OFF
cmake --build . -j4
sudo cmake --install .
cd ../../ && rm -rf Catch2
fi

echo "All dependencies have been installed successfully!"
Loading