Skip to content
Closed
Changes from 1 commit
Commits
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
29 changes: 23 additions & 6 deletions .github/workflows/commons-release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -391,12 +391,29 @@ jobs:
fi

mkdir -p android-combined/jniLibs
for dir in dist/android-artifacts/racommons-android-*/jniLibs/*/; do
if [ -d "$dir" ]; then
abi=$(basename "$dir")
mkdir -p "android-combined/jniLibs/${abi}"
cp -r "${dir}"* "android-combined/jniLibs/${abi}/" 2>/dev/null || true
echo "Copied libs for ${abi}: $(ls android-combined/jniLibs/${abi}/ 2>/dev/null | wc -l) files"
# Collect .so files from all build output subdirectories (jni/, commons/, onnx/, llamacpp/, rag/)
# Each artifact was built for a single ABI via the matrix strategy
for artifact_dir in dist/android-artifacts/racommons-android-*/; do
if [ ! -d "$artifact_dir" ]; then
continue
fi
for sub in jni commons onnx llamacpp rag; do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider using for sub in jni commons onnx llamacpp rag; do instead of listing them without quotes for better shell safety and readability.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/commons-release.yml
Line: 400

Comment:
Consider using `for sub in jni commons onnx llamacpp rag; do` instead of listing them without quotes for better shell safety and readability.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

for abi_dir in "${artifact_dir}${sub}"/*/; do
if [ -d "$abi_dir" ]; then
abi=$(basename "$abi_dir")
mkdir -p "android-combined/jniLibs/${abi}"
cp -n "${abi_dir}"*.so "android-combined/jniLibs/${abi}/" 2>/dev/null || true
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cp -n silently skips existing files without logging. If duplicate .so names exist across subdirectories, only the first copy succeeds. Add -v flag for visibility into what's being copied vs skipped.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/commons-release.yml
Line: 405

Comment:
`cp -n` silently skips existing files without logging. If duplicate `.so` names exist across subdirectories, only the first copy succeeds. Add `-v` flag for visibility into what's being copied vs skipped.

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown

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:

cd .github/workflows && wc -l commons-release.yml

Repository: RunanywhereAI/runanywhere-sdks

Length of output: 97


🏁 Script executed:

cat -n .github/workflows/commons-release.yml | sed -n '395,415p'

Repository: RunanywhereAI/runanywhere-sdks

Length of output: 1191


🏁 Script executed:

head -20 .github/workflows/commons-release.yml

Repository: RunanywhereAI/runanywhere-sdks

Length of output: 632


🏁 Script executed:

rg "nullglob|set -o" .github/workflows/commons-release.yml

Repository: RunanywhereAI/runanywhere-sdks

Length of output: 56


🏁 Script executed:

grep -n "#!/bin/bash\|set -e\|set -u\|set -o" .github/workflows/commons-release.yml | head -20

Repository: RunanywhereAI/runanywhere-sdks

Length of output: 56


🏁 Script executed:

rg "2>/dev/null" .github/workflows/commons-release.yml

Repository: RunanywhereAI/runanywhere-sdks

Length of output: 275


Line 405: avoid swallowing all cp failures during release packaging.

2>/dev/null || true hides real copy errors (not just "no matches"), so a broken ZIP can still be published. The glob pattern "${abi_dir}"*.so without nullglob will fail with "cannot stat '*.so'" when no files match, and this error is suppressed along with any actual copy failures. Only skip when no .so exists; fail on actual cp errors.

Suggested hardening
-                  cp -n "${abi_dir}"*.so "android-combined/jniLibs/${abi}/" 2>/dev/null || true
+                  shopt -s nullglob
+                  so_files=("${abi_dir}"*.so)
+                  shopt -u nullglob
+                  if [ ${`#so_files`[@]} -gt 0 ]; then
+                    cp -n "${so_files[@]}" "android-combined/jniLibs/${abi}/"
+                  fi
📝 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.

Suggested change
cp -n "${abi_dir}"*.so "android-combined/jniLibs/${abi}/" 2>/dev/null || true
shopt -s nullglob
so_files=("${abi_dir}"*.so)
shopt -u nullglob
if [ ${`#so_files`[@]} -gt 0 ]; then
cp -n "${so_files[@]}" "android-combined/jniLibs/${abi}/"
fi
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/commons-release.yml at line 405, The cp line is swallowing
all errors via `2>/dev/null || true` and hiding real failures when copying
`${abi_dir}*.so`; replace this with a safe existence check using shell nullglob:
enable nullglob (shopt -s nullglob), build an array like files=(
"${abi_dir}"*.so ), if the array is empty skip the copy, otherwise run cp -n
"${files[@]}" "android-combined/jniLibs/${abi}/" and leave failures to surface;
refer to the existing cp command and the variables abi_dir and abi when making
the change.

fi
done
done
done
# Log what was collected per ABI
for abi_dir in android-combined/jniLibs/*/; do
if [ -d "$abi_dir" ]; then
abi=$(basename "$abi_dir")
count=$(ls "$abi_dir"/*.so 2>/dev/null | wc -l)
echo "Collected ${abi}: ${count} .so files"
ls -la "$abi_dir" 2>/dev/null
fi
Comment on lines +410 to 417
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Line 410-417 should enforce ABI correctness, not only log it.

This block reports counts but never fails on missing/empty ABI folders. Add required-ABI assertions (and ideally an ELF machine check for key libs like x86_64/libc++_shared.so) before zipping.

Suggested validation gate
-          # Log what was collected per ABI
-          for abi_dir in android-combined/jniLibs/*/; do
-            if [ -d "$abi_dir" ]; then
-              abi=$(basename "$abi_dir")
-              count=$(ls "$abi_dir"/*.so 2>/dev/null | wc -l)
-              echo "Collected ${abi}: ${count} .so files"
-              ls -la "$abi_dir" 2>/dev/null
-            fi
-          done
+          # Validate required ABIs and collected libs
+          required_abis=(arm64-v8a armeabi-v7a x86_64 x86)
+          for abi in "${required_abis[@]}"; do
+            abi_dir="android-combined/jniLibs/${abi}"
+            if [ ! -d "$abi_dir" ]; then
+              echo "❌ Missing ABI directory: ${abi}" >&2
+              exit 1
+            fi
+            count=$(find "$abi_dir" -maxdepth 1 -name '*.so' | wc -l)
+            if [ "$count" -eq 0 ]; then
+              echo "❌ No .so files collected for ABI: ${abi}" >&2
+              exit 1
+            fi
+            echo "Collected ${abi}: ${count} .so files"
+            ls -la "$abi_dir"
+          done
Based on learnings: Android outputs must include JNI libraries with proper ABI subdirectories (arm64-v8a, x86_64, armeabi-v7a, x86) and 16KB page alignment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/workflows/commons-release.yml around lines 410 - 417, Replace the
current informational loop (iterating android-combined/jniLibs/*/ using abi_dir,
abi, count) with a validation gate that (1) asserts the presence of all required
ABI subdirs ["arm64-v8a","x86_64","armeabi-v7a","x86"] and fails the workflow if
any are missing or contain zero .so files, and (2) performs an ELF sanity check
on key libraries (e.g., x86_64/libc++_shared.so) using readelf/file to verify
the ELF Machine matches the ABI (EM_X86_64 for x86_64, etc.) and validates
PT_LOAD p_vaddr alignment meets 16KB page alignment; on any missing/incorrect
ABI, ELF machine mismatch, or alignment failure, emit an error message and exit
non‑zero so the job fails instead of continuing to zip.

done

Expand Down
Loading