-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add multiple Linux build variants for GLIBC compatibility #312
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.
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
base: master
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces comprehensive support for multiple Linux build variants, including standard, compatible, and AppImage builds, in the GitHub Actions workflow. It adds extensive documentation and test scripts for verifying build compatibility across various distributions and GLIBC versions, updates the README with detailed installation instructions, and refines artifact packaging and release documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHubActions
participant Docker
participant LinuxDistros
participant ReleasePage
User->>GitHubActions: Push code / tag release
GitHubActions->>Docker: Build (modern, compatible, AppImage) for x86_64/ARM64
Docker->>GitHubActions: Return build artifacts
GitHubActions->>ReleasePage: Upload all build variants as artifacts
User->>ReleasePage: Download desired build variant
User->>LinuxDistros: Run build/test scripts to verify compatibility
Possibly related PRs
Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces multiple Linux build variants for the Dash Evo Tool to improve GLIBC compatibility and updates CI and documentation accordingly.
- Adds Docker-based test scripts for standard, compatible, and AppImage builds across various distributions.
- Extends the GitHub Actions workflow to build, package, and release the new Linux variants (
modern,compat,appimage). - Enhances documentation with detailed instructions for testing and installing the new Linux packages.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| build-tests/test-single-build.sh | New quick script to manually test a single build |
| build-tests/test-linux-builds.sh | Automated script to test all Linux build variants |
| build-tests/test-linux-builds-manual.md | Manual guide for Docker-based testing |
| build-tests/test-glibc-compatibility.sh | Script to report GLIBC versions in containers |
| build-tests/test-build-locally.sh | Local Docker build & test simulation script |
| build-tests/quick-test-instructions.md | Quick reference for testing the workflow changes |
| build-tests/README.md | Overview of available test scripts and docs |
| README.md | Installation section updated with multi-variant info |
| .github/workflows/release.yml | CI matrix updated to build/package new Linux variants |
Comments suppressed due to low confidence (2)
.github/workflows/release.yml:75
- [nitpick] This job name uses a different casing than the other matrix entries. Consider renaming it to lowercase (e.g., 'windows') to maintain consistency.
- name: "Windows"
.github/workflows/release.yml:192
- The here-document for the desktop file is indented, which will introduce leading spaces in the generated .desktop file. Adjust indentation so that entries start in column zero or use '<<-EOF' to strip tabs.
[Desktop Entry]
|
|
||
| if docker run --rm \ | ||
| -v "$PWD/test_appimage.sh:/test_appimage.sh" \ | ||
| -v "$PWD/$3:/test/$3" \ |
Copilot
AI
Jun 2, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The function 'test_appimage' uses '$3' for the build file without declaring a local variable; consider adding 'local build_file=$3' at the top of the function for clarity and to avoid unexpected behavior.
| -v "$PWD/$3:/test/$3" \ | |
| -v "$PWD/$build_file:/test/$build_file" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (18)
build-tests/test-glibc-compatibility.sh (4)
1-2: Use a portable shebang and enable strict mode.Switch to:
-#!/bin/bash +#!/usr/bin/env bash +set -euo pipefailThis enhances portability and ensures the script exits on errors or undefined variables.
3-9: Verify Docker availability upfront.If Docker is missing, the script will misleadingly report failures. Add a check:
+# Check for Docker +if ! command -v docker &>/dev/null; then + echo "Error: Docker is required but not found." >&2 + exit 1 +fiPlace this before defining
check_glibc.
10-16: Improve GLIBC extraction and error handling.Separate absence of GLIBC vs. container errors:
- docker run --rm "$image" bash -c "ldd --version 2>/dev/null | head -n 1 | grep -oE '[0-9]+\.[0-9]+' | head -1" 2>/dev/null || echo "Failed to check" + docker run --rm --quiet "$image" \ + bash -c "ldd --version 2>/dev/null | head -n1 | grep -oE '[0-9]+\.[0-9]+' || echo 'N/A'" \ + || echo "Failed to check"Reporting
N/Afor missing version makes the output clearer.
21-40: Refactor distro list to a loop for maintainability.Hard-coded calls can be replaced with an array-driven loop:
declare -a DISTROS=( "Ubuntu 18.04 LTS|ubuntu:18.04" "Ubuntu 20.04 LTS|ubuntu:20.04" # Add more entries as needed ) for entry in "${DISTROS[@]}"; do IFS="|" read -r name image <<< "$entry" check_glibc "$name" "$image" doneThis simplifies adding/removing distributions.
build-tests/README.md (1)
21-29: Suggest adding execution-permissions reminder.New users may forget to
chmod +xthe scripts before running. Consider adding:**Note:** Before running the scripts, make them executable: ```bash chmod +x build-tests/*.sh</blockquote></details> <details> <summary>build-tests/test-single-build.sh (2)</summary><blockquote> `1-2`: **Use a portable shebang and strict mode.** Replace: ```diff -#!/bin/bash +#!/usr/bin/env bash +set -euo pipefailThis improves portability and error handling.
7-11: Send usage output to stderr.On missing arguments, the usage message should go to stderr:
-if [ $# -lt 2 ]; then - echo "Usage: $0 <docker-image> <build-file>" +if [ $# -lt 2 ]; then + echo "Usage: $0 <docker-image> <build-file>" >&2 + echo "Example: $0 ubuntu:20.04 dash-evo-tool-x86_64-linux-compat.zip" >&2 exit 1 fiThis distinguishes normal vs. error output.
build-tests/quick-test-instructions.md (3)
5-5: Remove trailing punctuation in headings
Markdown headings should not end with a colon to comply with markdownlint (MD026). Please remove the trailing:from the following headings:
## GLIBC Versions by Distribution:## What the changes will do:## To test the changes:## Expected outcomes:Also applies to: 13-13, 30-30, 66-66
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
5-5: Trailing punctuation in heading
Punctuation: ':'(MD026, no-trailing-punctuation)
47-47: Convert bare URL to markdown link
Avoid MD034 by replacing the raw URL with a descriptive link. For example:[Release workflow](https://github.com/dashpay/dash-evo-tool/actions)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
47-47: Bare URL used
null(MD034, no-bare-urls)
75-75: Add missing article
Insert "a" before "universal fallback" for grammatical correctness:- Any issues: Use AppImage as universal fallback + Any issues: Use AppImage as a universal fallback🧰 Tools
🪛 LanguageTool
[uncategorized] ~75-~75: You might be missing the article “a” here.
Context: ...ble build - Any issues: Use AppImage as universal fallback(AI_EN_LECTOR_MISSING_DETERMINER_A)
build-tests/test-linux-builds.sh (3)
1-7: Add Docker availability check
Before running tests, verify that thedockercommand is available to avoid confusing failures later:if ! command -v docker &> /dev/null; then echo "Error: Docker is required for testing."; exit 1 fi
28-49: Use secure temporary files
Writingtest_in_container.shto the working directory risks collisions and leftover files. Consider usingmktempto create a temporary file and atrapto clean it up, or embed the here-doc directly within thedocker runcommand to avoid touching the host filesystem.
84-110: Refactortest_appimagescript creation
Similarly, generatingtest_appimage.shon disk can be simplified with an inline here-doc in thedocker runinvocation or by using a temporary file. This reduces host file clutter and potential race conditions.build-tests/test-linux-builds-manual.md (1)
100-100: Reduce weak intensifier
Consider removing "very" for clearer phrasing:- ❌ May fail on very old systems (Ubuntu 18.04 and older) + ❌ May fail on old systems (Ubuntu 18.04 and older)build-tests/test-build-locally.sh (2)
9-12: Remove unused variableREDor utilize it
TheREDcolor constant is defined but never used. Either remove its definition or apply it to error messages to keep the code clean.🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 9-9: RED appears unused. Verify use (or export if used externally).
(SC2034)
73-75: Limit source copy context
Usingcp -r ../* .may unintentionally include unwanted files (e.g.,.git, build artifacts). Copy only the required directories and files (for example,src/,Cargo.toml,Cargo.lock) to reduce the Docker build context size..github/workflows/release.yml (2)
34-34: Remove trailing spaces
These lines have unintended trailing whitespace. Please remove it to satisfy YAML linting rules and avoid noisy diffs.Also applies to: 41-41, 48-48, 55-55, 62-62, 72-72
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 34-34: trailing spaces
(trailing-spaces)
321-321: Add newline at end of file
Ensure the YAML file ends with a newline character to prevent lint errors (new-line-at-end-of-file).🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 321-321: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/release.yml(6 hunks).gitignore(1 hunks)README.md(2 hunks)build-tests/README.md(1 hunks)build-tests/quick-test-instructions.md(1 hunks)build-tests/test-build-locally.sh(1 hunks)build-tests/test-glibc-compatibility.sh(1 hunks)build-tests/test-linux-builds-manual.md(1 hunks)build-tests/test-linux-builds.sh(1 hunks)build-tests/test-single-build.sh(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
build-tests/test-linux-builds.sh (1)
build-tests/test-build-locally.sh (2)
test_build(200-228)test_appimage(231-255)
build-tests/test-build-locally.sh (1)
build-tests/test-linux-builds.sh (2)
test_build(19-75)test_appimage(78-128)
🪛 LanguageTool
build-tests/quick-test-instructions.md
[uncategorized] ~75-~75: You might be missing the article “a” here.
Context: ...ble build - Any issues: Use AppImage as universal fallback
(AI_EN_LECTOR_MISSING_DETERMINER_A)
README.md
[uncategorized] ~93-~93: Possible missing preposition found.
Context: ...ropriate .zip file for your system 2. Extract the archive: ```shell unzip dash-...
(AI_HYDRA_LEO_MISSING_TO)
build-tests/test-linux-builds-manual.md
[style] ~101-~101: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...rks on newer systems - ❌ May fail on very old systems (Ubuntu 18.04 and older) 3. **...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.17.2)
build-tests/quick-test-instructions.md
5-5: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
13-13: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
30-30: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
47-47: Bare URL used
null
(MD034, no-bare-urls)
66-66: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 Shellcheck (0.10.0)
build-tests/test-build-locally.sh
[warning] 9-9: RED appears unused. Verify use (or export if used externally).
(SC2034)
🪛 actionlint (1.7.7)
.github/workflows/release.yml
51-51: label "ubuntu-20.04-arm" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 YAMLlint (1.37.1)
.github/workflows/release.yml
[error] 34-34: trailing spaces
(trailing-spaces)
[error] 41-41: trailing spaces
(trailing-spaces)
[error] 48-48: trailing spaces
(trailing-spaces)
[error] 55-55: trailing spaces
(trailing-spaces)
[error] 62-62: trailing spaces
(trailing-spaces)
[error] 72-72: trailing spaces
(trailing-spaces)
[error] 184-184: trailing spaces
(trailing-spaces)
[error] 189-189: trailing spaces
(trailing-spaces)
[error] 200-200: trailing spaces
(trailing-spaces)
[error] 203-203: trailing spaces
(trailing-spaces)
[error] 215-215: trailing spaces
(trailing-spaces)
[error] 218-218: trailing spaces
(trailing-spaces)
[error] 270-270: trailing spaces
(trailing-spaces)
[error] 304-304: trailing spaces
(trailing-spaces)
[error] 306-306: trailing spaces
(trailing-spaces)
[error] 310-310: trailing spaces
(trailing-spaces)
[error] 314-314: trailing spaces
(trailing-spaces)
[error] 317-317: trailing spaces
(trailing-spaces)
[error] 321-321: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (7)
README.md (3)
78-105: Installation instructions are clear and comprehensive.The "Download Pre-built Packages" section correctly details each Linux variant with extraction and execution steps. The AppImage instructions are accurate and well-separated.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~93-~93: Possible missing preposition found.
Context: ...ropriate.zipfile for your system 2. Extract the archive: ```shell unzip dash-...(AI_HYDRA_LEO_MISSING_TO)
154-159: Build-from-source steps look good.The "Build from Source" subsection cleanly outlines cloning, building, and running the application. Numbering and commands are consistent.
164-174: Getting Started commands are consistent.Quick-start instructions for both pre-built and source builds are straightforward and free of errors.
build-tests/README.md (3)
1-4: Overview is concise and informative.The introduction clearly states the purpose of the
build-testsdirectory.
7-13: Content list accurately reflects scripts and docs.All entries are correctly named and described.
37-54: Build variant descriptions are clear.Standard, compatible, and AppImage build types are well-explained.
.github/workflows/release.yml (1)
301-301: Verify prerelease flag
The workflow setsprerelease: true, causing every release to be marked as a prerelease. Confirm whether you intended stable releases to be prereleases, or if this should befalse.
|
|
||
| # Test build directories | ||
| build-test/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the test directory ignore pattern.
The new test scripts live under build-tests/, but .gitignore currently ignores build-test/. This mismatch will still track files in build-tests/. Please update to:
-# Test build directories
-build-test/
+# Test build directories
+build-tests/📝 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.
| # Test build directories | |
| build-test/ | |
| # Test build directories | |
| build-tests/ |
🤖 Prompt for AI Agents
In the .gitignore file around lines 29 to 31, the ignore pattern is set to
"build-test/" but the actual test build directories are named "build-tests/".
Update the ignore pattern to "build-tests/" to correctly ignore the test build
directories and prevent tracking those files.
| docker run --rm -it \ | ||
| -v "$PWD/$BUILD_FILE:/test/$BUILD_FILE" \ | ||
| "$DISTRO" \ | ||
| /bin/bash -c " | ||
| cd /test | ||
| apt-get update && apt-get install -y unzip ldd || yum install -y unzip | ||
| unzip -q $BUILD_FILE | ||
| cd dash-evo-tool | ||
| chmod +x dash-evo-tool | ||
| echo '=== GLIBC Version ===' | ||
| ldd --version | head -n 1 | ||
| echo '=== Binary Dependencies ===' | ||
| ldd ./dash-evo-tool | ||
| echo '=== Testing Binary ===' | ||
| ./dash-evo-tool --version || echo 'Failed to run' | ||
| " No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Detect package manager and support Alpine.
The current fallback (apt-get → yum) doesn't cover Alpine or ensure ldd is installed. Consider:
- apt-get update && apt-get install -y unzip ldd || yum install -y unzip
+ if command -v apt-get &>/dev/null; then
+ apt-get update && apt-get install -y unzip ldd
+ elif command -v yum &>/dev/null; then
+ yum install -y unzip glibc-common
+ elif command -v apk &>/dev/null; then
+ apk add --no-cache unzip libc6-compat
+ else
+ echo "Unsupported package manager. Please install unzip and ldd manually." >&2
+ exit 1
+ fiThis ensures dependencies across Debian, RPM, and Alpine images.
🤖 Prompt for AI Agents
In build-tests/test-single-build.sh around lines 18 to 33, the script only tries
apt-get and yum for package installation, missing Alpine's apk package manager
and not guaranteeing ldd is installed. Modify the script to detect the package
manager by checking for apk, apt-get, or yum, and install unzip and ldd
accordingly to support Alpine, Debian, and RPM-based distros. This ensures all
required dependencies are installed regardless of the base image.
|
|
||
| # Linux x86_64 - Compatible build (Ubuntu 20.04, GLIBC 2.31) | ||
| - name: "linux-x86_64-compat" | ||
| runs-on: "ubuntu-20.04" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a scheduled Ubuntu 20.04 retirement. Ubuntu 20.04 LTS runner will be removed on 2025-04-15.
.github/workflows/release.yml
Outdated
|
|
||
| # AppImage x86_64 build | ||
| - name: "linux-x86_64-appimage" | ||
| runs-on: "ubuntu-20.04" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a scheduled Ubuntu 20.04 retirement. Ubuntu 20.04 LTS runner will be removed on 2025-04-15.
| We provide multiple Linux packages to ensure compatibility: | ||
|
|
||
| - **Standard builds** (`x86_64-linux`, `arm64-linux`): For modern systems with GLIBC 2.39+ | ||
| - **Compatible builds** (`x86_64-linux-compat`, `arm64-linux-compat`): For older systems with GLIBC 2.31+ (Ubuntu 20.04+, Debian 11+, etc.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove this one, AppImage should be good enough
| @@ -0,0 +1,276 @@ | |||
| #!/bin/bash | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fails with:
0.170 cp: cannot stat '.env.example': No such file or directory
| 1. **Standard Build** (built on Ubuntu 24.04): | ||
| - Will require GLIBC 2.39 | ||
| - Only works on Ubuntu 24.04, Fedora 39+, and very recent distros | ||
| - This matches your current situation where users complain about GLIBC 2.39 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
???
| - Only works on Ubuntu 24.04, Fedora 39+, and very recent distros | ||
| - This matches your current situation where users complain about GLIBC 2.39 | ||
|
|
||
| 2. **Compatible Build** (built on Ubuntu 20.04): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ubuntu 20.04 is deprecated, github will remove builders soon.
|
|
||
| ```bash | ||
| # Test compatible build on Ubuntu 20.04 | ||
| docker run --rm -it -v "$PWD:/test" ubuntu:20.04 bash -c " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I think there is a script for that, isn't it?
| echo "" | ||
| echo "Summary:" | ||
| echo "========" | ||
| echo "- GLIBC 2.31 (Ubuntu 20.04 compatible build) works on:" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This summary does not take into account results of the executed check, it's just some static text displayed regardless of test outcome.
This update introduces multiple Linux build variants for the Dash Evo Tool to enhance compatibility across different systems. The new build types include:
Standard Builds:
x86_64-linux: Built on Ubuntu 24.04 (requires GLIBC 2.39+).arm64-linux: Built on Ubuntu 22.04 ARM.Compatible Builds:
x86_64-linux-compat: Built on Ubuntu 20.04 (requires GLIBC 2.31+, compatible with most distributions from 2020+).arm64-linux-compat: Built on Ubuntu 20.04 ARM.AppImage:
x86_64-linux-appimage: A self-contained AppImage that works on most Linux distributions.Additionally, the GitHub Actions workflow has been updated to handle these new builds, and documentation has been added to clarify installation and compatibility details. This change aims to provide users with better options depending on their system requirements.
Summary by CodeRabbit