feat: add idempotency checks for third-party dependencies#413
feat: add idempotency checks for third-party dependencies#41313eholder wants to merge 1 commit intoeloqdata:mainfrom
Conversation
WalkthroughAdded an Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/install_dependency_ubuntu2404.sh (2)
19-27: Avoidevalinis_installed; execute probes as argv.Using
evalhere is unnecessary and riskier than direct command invocation. All five call sites use simple command probes suitable for conversion. Refactor the function to use"$@"instead, then update each call site to pass the command as separate arguments:Refactoring pattern
is_installed() { - if eval "$2" >/dev/null 2>&1; then - echo "--- [SKIP] $1 is already installed. ---" + local library_name="$1" + shift + if "$@" >/dev/null 2>&1; then + echo "--- [SKIP] ${library_name} is already installed. ---" return 0 fi return 1 }Apply to all five call sites (lines 65, 75, 85, 100, 120), e.g.:
-if ! is_installed "glog" "ls /usr/local/lib/libglog.so"; then +if ! is_installed "glog" test -f /usr/local/lib/libglog.so; thenNote:
test -fis more efficient thanlsfor existence checks.🤖 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 19 - 27, The is_installed function currently uses eval to run the probe command; change it to accept the probe as argv (e.g., use "$@" to run the passed command) and update every call site that invokes is_installed to pass the command and its arguments as separate parameters instead of a single string (convert probes like "ls /usr/include/some.h" to is_installed "SomeLib" test -f /usr/include/some.h or similar); replace any uses of ls with test -f for file existence checks and ensure return behavior and logging (echo "--- [SKIP] $1 is already installed. ---") remain unchanged in is_installed so callers keep the same flow.
75-82: Use pkg-config or ldconfig instead of hardcoded path for liburing detection.Line 75 hardcodes
/usr/lib/liburing.so.2.6, which is fragile across install layouts and targets the wrong version identifier. liburing's SONAME isliburing.so.2(not.2.6), and the path doesn't account for/usr/lib64, multiarch paths, or/usr/local/lib. Instead:
- Build-time: Use
pkg-config --exists liburing(liburing installsliburing.pc)- Runtime: Use
ldconfig -p | grep liburing.so.2to probe the dynamic linker cache🤖 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 75 - 82, The current install check in scripts/install_dependency_ubuntu2404.sh uses a hardcoded path "/usr/lib/liburing.so.2.6" which is fragile; update the detection logic around the is_installed call to first check build-time availability with "pkg-config --exists liburing" (use liburing.pc), and for runtime probe use "ldconfig -p | grep liburing.so.2" to detect the SONAME across /usr/lib, /usr/lib64, /usr/local/lib and multiarch layouts; change the condition that calls is_installed (or replace it) to return true if either pkg-config succeeds or ldconfig finds liburing.so.2, otherwise proceed with the git clone/build steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/install_dependency_ubuntu2404.sh`:
- Around line 19-27: The current is_installed() helper only tests presence and
must be made version-aware: change is_installed to accept a required-version
argument and, for each dependency (liburing, aws-sdk-cpp, Catch2), run a version
query (e.g. pkg-config --modversion, dpkg-query -W -f='${Version}', or cmake
--find-package with VERSION) and compare the installed version against the
required pin (liburing-2.6, aws-sdk-cpp 1.11.446, Catch2 3.3.2); update all
invocations of is_installed to pass the expected version and only skip
installation when the installed version >= required version, otherwise return
non-zero so the install path runs. Ensure the function logs both detected and
required versions in its messages for clarity.
---
Nitpick comments:
In `@scripts/install_dependency_ubuntu2404.sh`:
- Around line 19-27: The is_installed function currently uses eval to run the
probe command; change it to accept the probe as argv (e.g., use "$@" to run the
passed command) and update every call site that invokes is_installed to pass the
command and its arguments as separate parameters instead of a single string
(convert probes like "ls /usr/include/some.h" to is_installed "SomeLib" test -f
/usr/include/some.h or similar); replace any uses of ls with test -f for file
existence checks and ensure return behavior and logging (echo "--- [SKIP] $1 is
already installed. ---") remain unchanged in is_installed so callers keep the
same flow.
- Around line 75-82: The current install check in
scripts/install_dependency_ubuntu2404.sh uses a hardcoded path
"/usr/lib/liburing.so.2.6" which is fragile; update the detection logic around
the is_installed call to first check build-time availability with "pkg-config
--exists liburing" (use liburing.pc), and for runtime probe use "ldconfig -p |
grep liburing.so.2" to detect the SONAME across /usr/lib, /usr/lib64,
/usr/local/lib and multiarch layouts; change the condition that calls
is_installed (or replace it) to return true if either pkg-config succeeds or
ldconfig finds liburing.so.2, otherwise proceed with the git clone/build steps.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d0410b7-c293-482d-987d-6db6e1ad76cb
📒 Files selected for processing (1)
scripts/install_dependency_ubuntu2404.sh
| # 参数1: 库名(用于显示) | ||
| # 参数2: 检测命令 (检测核心头文件或库文件是否存在) | ||
| is_installed() { | ||
| if eval "$2" >/dev/null 2>&1; then | ||
| echo "--- [SKIP] $1 is already installed. ---" | ||
| return 0 | ||
| fi | ||
| return 1 | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Pinned versions / tags in installer:"
rg -n 'git checkout tags/|git clone -b v|is_installed' scripts/install_dependency_ubuntu2404.sh -C1
echo
echo "CMake dependency resolution without explicit versions:"
rg -n 'find_package\((glog|AWSSDK|Catch2)\b' CMakeLists.txt tests/CMakeLists.txtRepository: eloqdata/eloqstore
Length of output: 1410
🏁 Script executed:
sed -n '50,65p' CMakeLists.txtRepository: eloqdata/eloqstore
Length of output: 432
🏁 Script executed:
sed -n '1,10p' tests/CMakeLists.txtRepository: eloqdata/eloqstore
Length of output: 253
Installation skip logic should validate dependency versions, not just presence.
Line 22 checks only whether a dependency is installed, but the script pins specific versions (liburing-2.6, aws-sdk-cpp 1.11.446, Catch2 v3.3.2) while CMake resolution accepts any version (find_package(glog/AWSSDK/Catch2 REQUIRED) without VERSION constraints). This allows stale or incompatible system versions to be skipped, potentially causing build inconsistencies or runtime failures.
🔧 Suggested approach (version-aware skip)
is_installed() {
- if eval "$2" >/dev/null 2>&1; then
+ local name="$1"
+ local probe_cmd="$2"
+ local version_cmd="${3:-}"
+ local expected_version="${4:-}"
+ if eval "$probe_cmd" >/dev/null 2>&1; then
+ if [ -n "$version_cmd" ] && [ -n "$expected_version" ]; then
+ local actual_version
+ actual_version="$(eval "$version_cmd" 2>/dev/null || true)"
+ if [ "$actual_version" != "$expected_version" ]; then
+ echo "--- [REINSTALL] ${name}: ${actual_version} != ${expected_version} ---"
+ return 1
+ fi
+ fi
- echo "--- [SKIP] $1 is already installed. ---"
+ echo "--- [SKIP] ${name} is already installed. ---"
return 0
fi
return 1
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # 参数1: 库名(用于显示) | |
| # 参数2: 检测命令 (检测核心头文件或库文件是否存在) | |
| is_installed() { | |
| if eval "$2" >/dev/null 2>&1; then | |
| echo "--- [SKIP] $1 is already installed. ---" | |
| return 0 | |
| fi | |
| return 1 | |
| } | |
| # 参数1: 库名(用于显示) | |
| # 参数2: 检测命令 (检测核心头文件或库文件是否存在) | |
| is_installed() { | |
| local name="$1" | |
| local probe_cmd="$2" | |
| local version_cmd="${3:-}" | |
| local expected_version="${4:-}" | |
| if eval "$probe_cmd" >/dev/null 2>&1; then | |
| if [ -n "$version_cmd" ] && [ -n "$expected_version" ]; then | |
| local actual_version | |
| actual_version="$(eval "$version_cmd" 2>/dev/null || true)" | |
| if [ "$actual_version" != "$expected_version" ]; then | |
| echo "--- [REINSTALL] ${name}: ${actual_version} != ${expected_version} ---" | |
| return 1 | |
| fi | |
| fi | |
| echo "--- [SKIP] ${name} is already installed. ---" | |
| return 0 | |
| fi | |
| return 1 | |
| } |
🤖 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 19 - 27, The current
is_installed() helper only tests presence and must be made version-aware: change
is_installed to accept a required-version argument and, for each dependency
(liburing, aws-sdk-cpp, Catch2), run a version query (e.g. pkg-config
--modversion, dpkg-query -W -f='${Version}', or cmake --find-package with
VERSION) and compare the installed version against the required pin
(liburing-2.6, aws-sdk-cpp 1.11.446, Catch2 3.3.2); update all invocations of
is_installed to pass the expected version and only skip installation when the
installed version >= required version, otherwise return non-zero so the install
path runs. Ensure the function logs both detected and required versions in its
messages for clarity.
13eholder
force-pushed
the
feat/opt_install_dependency
branch
from
afeab49 to
c911ad0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/install_dependency_ubuntu2404.sh (1)
85-97:⚠️ Potential issue | 🟡 MinorPin brpc to a specific tag or commit.
Similar to glog, brpc is cloned without a version pin, making builds non-reproducible and vulnerable to upstream breakage.
🔧 Suggested fix
if ! is_installed "brpc" "ls /usr/lib/libbrpc.so"; then -git clone https://github.com/eloqdata/brpc.git brpc +git clone --branch <pinned-tag-or-commit> https://github.com/eloqdata/brpc.git brpc cd brpc🤖 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 - 97, The brpc clone step is unpinned which makes builds non-reproducible; update the block that clones and builds brpc (the if ! is_installed "brpc" "ls /usr/lib/libbrpc.so" branch) to checkout a specific tag or commit after git clone (e.g., run git fetch && git checkout <TAG_OR_COMMIT>) before building, and document the chosen tag/commit; ensure the rest of the steps (cmake, cmake --build, and sudo cp ...) remain unchanged so the pinned revision is built instead of the repository tip.
🧹 Nitpick comments (1)
scripts/install_dependency_ubuntu2404.sh (1)
75-82: Quote command substitution to prevent word splitting.Per shellcheck SC2046, quote
$(nproc)on line 80.🔧 Suggested fix
- make -j$(nproc) && sudo make install + make -j "$(nproc)" && sudo make install🤖 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 75 - 82, The make invocation uses unquoted command substitution which can cause word-splitting (SC2046); update the build step in the liburing install block (the make -j$(nproc) call inside the if checking is_installed) to quote the substitution, i.e., use "$(nproc)" so the command becomes make -j"$(nproc)"; ensure only that substitution is quoted and no other behavior changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/install_dependency_ubuntu2404.sh`:
- Around line 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.
---
Outside diff comments:
In `@scripts/install_dependency_ubuntu2404.sh`:
- Around line 85-97: The brpc clone step is unpinned which makes builds
non-reproducible; update the block that clones and builds brpc (the if !
is_installed "brpc" "ls /usr/lib/libbrpc.so" branch) to checkout a specific tag
or commit after git clone (e.g., run git fetch && git checkout <TAG_OR_COMMIT>)
before building, and document the chosen tag/commit; ensure the rest of the
steps (cmake, cmake --build, and sudo cp ...) remain unchanged so the pinned
revision is built instead of the repository tip.
---
Nitpick comments:
In `@scripts/install_dependency_ubuntu2404.sh`:
- Around line 75-82: The make invocation uses unquoted command substitution
which can cause word-splitting (SC2046); update the build step in the liburing
install block (the make -j$(nproc) call inside the if checking is_installed) to
quote the substitution, i.e., use "$(nproc)" so the command becomes make
-j"$(nproc)"; ensure only that substitution is quoted and no other behavior
changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1e7101ff-58a1-45f0-b957-5bfe58545418
📒 Files selected for processing (1)
scripts/install_dependency_ubuntu2404.sh
| 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 |
There was a problem hiding this comment.
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.
|
|
||
| # 参数1: 库名(用于显示) | ||
| # 参数2: 检测命令 (检测核心头文件或库文件是否存在) | ||
| is_installed() { |
There was a problem hiding this comment.
Dependency libs may be updated. The idempotency check should be enabled by a parameter which is by default false.
13eholder
force-pushed
the
feat/opt_install_dependency
branch
from
c911ad0 to
79cb862
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/install_dependency_ubuntu2404.sh`:
- Around line 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.
- 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 559fcd85-9782-4bee-98b2-1f8e93b0ecfc
📒 Files selected for processing (1)
scripts/install_dependency_ubuntu2404.sh
| cd liburing | ||
| git checkout tags/liburing-2.6 | ||
| ./configure --cc=gcc --cxx=g++ | ||
| make -j$(nproc) && sudo make install |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nF '-j$(nproc)' scripts/install_dependency_ubuntu2404.shRepository: 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.shRepository: 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.shRepository: 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.
| if ! is_installed "brpc" "ls /usr/lib/libbrpc.so"; then | ||
| git clone https://github.com/eloqdata/brpc.git brpc |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'git clone .*brpc' scripts/install_dependency_ubuntu2404.sh -C1Repository: 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/ -C1Repository: 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 +55Repository: 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 brpcOr 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.
2 participants
Here are some reminders before you submit the pull request
is_installedhelper to skip already installed libraries.fixes eloqdb/eloqstore#314ctest --test-dir build/tests/Summary by CodeRabbit