-
-
Notifications
You must be signed in to change notification settings - Fork 8
Add armbian-images.json generator and workflow #91
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: main
Are you sure you want to change the base?
Conversation
WalkthroughAdds a GitHub Actions workflow (.github/workflows/update-download-index.yml) that runs on push, repository_dispatch (type "Webindex update"), and workflow_dispatch; checks out this repo plus armbian/build, armbian/os, and armbian.github.io; installs jq, jc, rsync; runs scripts/generate-armbian-images-json.sh to produce armbian-images.json; commits and pushes the generated JSON into armbian.github.io at data/armbian-images.json and emits a repository_dispatch event "Jira update". Adds scripts/generate-armbian-images-json.sh which validates tools, aggregates feeds from a SOURCE_OF_TRUTH and multiple GitHub release sources, extracts per-image metadata (board, vendor, variant, distro, URLs, sizes, timestamps, exposure), composes proxy/redirect URLs, and writes a consolidated assets JSON. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
b9c42fb to
719efc0
Compare
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: 1
🧹 Nitpick comments (6)
.github/workflows/update-download-index.yml (1)
59-62: Consider pinning dependency versions for reproducibility.While Ubuntu's latest available versions of jq, jc, and rsync are generally stable, pinning specific versions (e.g.,
jq=1.x.x) would improve workflow reproducibility and prevent unexpected behavior changes if major versions are released.scripts/generate-armbian-images-json.sh (5)
156-159: Consolidate repetitive metadata extraction logic.Each field (board, version, release, kernel_branch) is extracted with nearly identical patterns:
grep -Po | grep -Po | cut. This is inefficient and difficult to maintain. Each line also lacks comments explaining what the field positions represent or validation that the extraction succeeded.Consider extracting metadata once per IMAGE_URL. For example:
# Extract version string like "24.11_boardname_jammy_edge" VERSION_STRING="$(echo "$IMAGE_URL" | grep -Po 'Armbian.*[0-9][0-9]\.[0-9].*' | grep -Po '[0-9][0-9]\.[0-9].*')" # Parse fields from version string ARMBIAN_VERSION="$(cut -d_ -f1 <<<"$VERSION_STRING")" BOARD="$(cut -d_ -f2 <<<"$VERSION_STRING")" IMAGE_RELEASE="$(cut -d_ -f3 <<<"$VERSION_STRING")" KERNEL_BRANCH="$(cut -d_ -f4 <<<"$VERSION_STRING")"This avoids running grep four times and makes the field positions explicit.
171-181: Add documentation or simplify theIMAGE_VARIANTextraction logic.The multi-step variant extraction involving grep, cut, sed, and the conditional
STEP_Alogic is difficult to understand without comments. The purpose of each transformation (e.g., why remove "-kisak" or "-backported-mesa") and the condition triggeringSTEP_A=2should be clearly documented.Add inline comments explaining:
- What IMAGE_VARIANT represents (user-facing image variant/flavor).
- Why certain strings are stripped (e.g., "kisak" is a graphics stack variant that should be hidden from the variant name).
- When and why
STEP_A=2is set (desktop environments with extra modifiers).
183-192: Simplify or documentFILE_EXTENSIONextraction.The
FILE_EXTENSIONextraction is cryptic: it reverses the string, extracts the last field, reverses again, and applies multiple sed transformations. The intent—extracting the final file extension (e.g.,.img.xz)—is unclear without careful reading. The sed pattern at line 190 ('s/.*[^0-9]\([0-9]*\.[0-9]*\.[0-9]*\)//) is particularly opaque.Consider extracting the extension more explicitly:
FILE_EXTENSION="$(echo "$IMAGE_URL" | sed -E 's/.*\.(tar\.gz|xz|gz|img|zip)$/.\1/')"Or, if the logic is more nuanced, add detailed comments explaining each sed transformation and its purpose.
230-230: Add error handling and validation for JSON conversion.The jc conversion at line 230 will silently fail if the CSV header or data is malformed. While
set -euo pipefailwill cause the script to exit, there's no explicit error message or fallback. Additionally, the script should verify that$OUTwas created successfully with expected content.Add validation after JSON generation:
} | jc --csv | jq '{"assets": .}' > "$OUT" || { echo "ERROR: Failed to generate JSON" >&2; exit 1; } # Validate output [[ -s "$OUT" ]] || { echo "ERROR: Output file is empty" >&2; exit 1; } jq empty "$OUT" || { echo "ERROR: Invalid JSON generated" >&2; exit 1; }
221-225: Ensureexposed.mappatterns are properly escaped for regex matching.The exposed flag check uses regex matching (
[[ =~ ]]) at line 224, which means patterns inexposed.mapare treated as extended regex. If the file contains special regex characters (.,*,+, etc.), they must be escaped or the matching behavior could be unexpected. Consider documenting the expected format ofexposed.mapor adding validation.If patterns are intended to be literal strings, use fixed-string matching instead:
[[ "$IMAGE_URL" == *"$exposed"* ]]Or, if regex patterns are intentional, document the expected format in comments or README.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/update-download-index.yml(1 hunks)scripts/generate-armbian-images-json.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/generate-armbian-images-json.sh
[warning] 169-169: STEP_B appears unused. Verify use (or export if used externally).
(SC2034)
🔇 Additional comments (6)
.github/workflows/update-download-index.yml (3)
77-91: Verifygit checkout dataandgit pushwill work reliably on the armbian.github.io checkout.The auth token is persisted in the local git config, enabling scripts to run authenticated git commands. However, after checking out armbian.github.io at line 50 (default branch), line 81 attempts to switch to the
databranch without ensuring it exists locally or tracking the remote. This could fail if the data branch hasn't been fetched yet. Similarly, line 91'sgit pushwithout an explicit branch reference could push to the wrong branch or fail if the tracking relationship isn't set up correctly.Verify:
- That
git checkout datasucceeds consistently (consider usinggit checkout -B data origin/datato ensure the tracking relationship).- That
git pushexplicitly targets the correct branch and remote (e.g.,git push origin data).- Test the workflow with a scenario where the
databranch doesn't exist yet locally to confirm robustness.
64-75: Script execution and output handling look good.The
set -euo pipefailensures strict error handling, environment variables are properly passed, and the output is correctly copied to the workspace root for subsequent steps. The working directory context is appropriately set.
93-97: Verify repository-dispatch permissions for the target repository.The auth token is persisted in the local git config, enabling scripts to run authenticated git commands. However, triggering a
repository_dispatchevent on a different repository may require additional permissions beyond the defaultGITHUB_TOKEN. Confirm that the token has permission to trigger events on the armbian/armbian.github.io repository, or consider using a PAT with appropriate permissions if cross-repository event triggering fails.scripts/generate-armbian-images-json.sh (3)
74-90: Board metadata loading is well-structured.The approach of extracting
BOARD_NAMEandBOARD_VENDORfrom config files and populating associative maps is sound. The use of multiple file extensions (.conf,.csc,.wip,.tvb) ensures broad coverage of board definitions.
117-141: Feed building combines multiple sources effectively.The script appropriately filters sidecar files (.asc, .sha, .torrent) at the feed level to avoid processing them as standalone entries, then reconstructs them as URL fields later. The combination of rsync (local mirror) and GitHub releases (community builds) provides comprehensive coverage.
Verify that the
SOURCE_OF_TRUTHrsync endpoint (line 17:rsync://fi.mirror.armbian.de) is reliable and accessible from CI runners. If the rsync source is occasionally unavailable, consider adding a retry mechanism or fallback.
31-69: Configuration parsing function is robust.The
extract_cfg_varfunction properly handles multiple declaration styles (direct assignment,export,declare -g, etc.) and correctly parses quoted/unquoted values. The internal helper functions (ltrim,rtrim,trim) ensure consistent whitespace handling.
719efc0 to
f6f8a27
Compare
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: 0
♻️ Duplicate comments (1)
scripts/generate-armbian-images-json.sh (1)
167-169: Remove unusedSTEP_Bvariable.The
STEP_Bvariable is declared and conditionally set but never referenced in the subsequent output. OnlySTEP_Ais used at line 179 in thecutcommand. This is the same issue flagged in the previous review and should be removed.STEP_A=1 - STEP_B=2 - [[ $IMAGE_URL == *i3-wm* || $IMAGE_URL == *kde-* || $IMAGE_URL == *-rc* ]] && STEP_A=2 && STEP_B=3 + [[ $IMAGE_URL == *i3-wm* || $IMAGE_URL == *kde-* || $IMAGE_URL == *-rc* ]] && STEP_A=2
🧹 Nitpick comments (1)
scripts/generate-armbian-images-json.sh (1)
156-159: Consolidate repetitive image metadata extraction.The same grep/cut pattern is repeated four times to extract
ARMBIAN_VERSION,BOARD,IMAGE_RELEASE, andKERNEL_BRANCH. This is fragile and difficult to maintain if the pattern needs to change.Extract once into a temporary variable:
+ extracted="$(echo "$IMAGE_URL" | grep -Po 'Armbian.*[0-9][0-9]\.[0-9].*' | grep -Po '[0-9][0-9]\.[0-9].*')" - BOARD="$(echo "$IMAGE_URL" | grep -Po 'Armbian.*[0-9][0-9]\.[0-9].*' | grep -Po '[0-9][0-9]\.[0-9].*' | cut -d_ -f2)" - ARMBIAN_VERSION="$(echo "$IMAGE_URL" | grep -Po 'Armbian.*[0-9][0-9]\.[0-9].*' | grep -Po '[0-9][0-9]\.[0-9].*' | cut -d_ -f1)" - IMAGE_RELEASE="$(echo "$IMAGE_URL" | grep -Po 'Armbian.*[0-9][0-9]\.[0-9].*' | grep -Po '[0-9][0-9]\.[0-9].*' | cut -d_ -f3)" - KERNEL_BRANCH="$(echo "$IMAGE_URL" | grep -Po 'Armbian.*[0-9][0-9]\.[0-9].*' | grep -Po '[0-9][0-9]\.[0-9].*' | cut -d_ -f4)" + ARMBIAN_VERSION="$(cut -d_ -f1 <<<"$extracted")" + BOARD="$(cut -d_ -f2 <<<"$extracted")" + IMAGE_RELEASE="$(cut -d_ -f3 <<<"$extracted")" + KERNEL_BRANCH="$(cut -d_ -f4 <<<"$extracted")"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/update-download-index.yml(1 hunks)scripts/generate-armbian-images-json.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.11.0)
scripts/generate-armbian-images-json.sh
[warning] 169-169: STEP_B appears unused. Verify use (or export if used externally).
(SC2034)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Generate JSON Index
🔇 Additional comments (7)
.github/workflows/update-download-index.yml (2)
77-91: Verifydatabranch exists and is properly handled in subsequent pushes.Line 81 assumes the
databranch exists in the armbian.github.io repository without error handling. Additionally, the push on line 91 will always attempt to push regardless of whether a commit was made; this should be conditional on successful commit.Consider refactoring to ensure the branch is checked out safely and the push is only executed if there are actual changes:
- name: Commit changes if any run: | set -euo pipefail cd armbian.github.io - git checkout data + git checkout data 2>/dev/null || git checkout -b data mkdir -p data cp "${GITHUB_WORKSPACE}/armbian-images.json" data/ git config user.name "github-actions" git config user.email "github-actions@github.com" git add data/armbian-images.json - git diff --cached --quiet || git commit -m "Update armbian-images.json" + git diff --cached --quiet || { git commit -m "Update armbian-images.json" && git push; }
59-75: Workflow setup and script execution look solid.Dependencies are installed correctly, environment variables are passed to the script, and output is copied to workspace root for subsequent steps. The concurrency group with
cancel-in-progress: falseappropriately serializes JSON generation and pushes to prevent race conditions.scripts/generate-armbian-images-json.sh (5)
108-141: Feed generation and filtering logic is sound.Feed A correctly excludes signature/checksum/torrent files as standalone entries (aligning with PR objectives to move these to sidecar URL fields). Feed B/C/D GitHub releases filtering is appropriate. The combined feed properly merges multiple data sources.
221-227: Verify the emptypreinstalled_applicationfield is intentional.The CSV output at line 227 includes an empty field for
preinstalled_application(double pipe||). This appears deliberate since no data source provides this information from image URLs. Confirm this is the intended behavior—whether the field should remain empty for all entries or if there are future plans to populate it.
31-106: Board metadata parsing is robust and well-structured.The
extract_cfg_varhelper comprehensively handles various declaration formats (export, declare, quoted/unquoted). The metadata loading loop correctly populates associative arrays and warns once per board for missing data without failing the job, aligning with PR requirements.
1-20: Initialization and error handling are solid.The script properly sets strict shell options (
set -euo pipefailand IFS), validates all required tools and directories early with clear error messages, and establishes cleanup viatrap. This prevents cascading failures downstream.
230-233: Output generation and reporting are correct.The
jc --csv | jqpipeline properly converts the CSV stream to the expected JSON structure, and the final reporting statements provide useful verification that the file was generated with the expected asset count.
b94f49c to
0b51fb4
Compare
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/update-download-index.yml(1 hunks)scripts/generate-armbian-images-json.sh(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/update-download-index.yml
🔇 Additional comments (1)
scripts/generate-armbian-images-json.sh (1)
25-50: Remove unverified claim about PR objectives.The original review cites "PR objectives state that the script supports declare/export/assign forms," but no such objectives are documented in this repository. The commit message, README, and code contain no evidence of these requirements. While
extract_cfg_var()does use basic AWK-based assignment matching (VAR=value), the claim that this violates stated PR objectives cannot be verified and should not be included without citing the actual requirement source.If you have access to the original PR or requirements documentation, please share it so we can assess whether the current implementation actually falls short of the stated goals.
Likely an incorrect or invalid review comment.
0b51fb4 to
a16ccf1
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
What
scripts/generate-armbian-images-json.shto generatearmbian-images.jsondata/armbian-images.jsonto armbian.github.io (databranch)Details
.asc/.sha/.torrentas standalone JSON entriesfile_url_asc,file_url_sha,file_url_torrentredi_url_asc,redi_url_sha,redi_url_torrenthttps://cache.armbian.com/artifacts/<board_slug>/archive/<IMAGENAME>.(asc|sha|torrent)board_nameandboard_vendorby parsing board config files (declare/export/assignforms supported)Testing
scripts/generate-armbian-images-json.shwithOS_DIRandBOARD_DIRsetUpdate download JSON indexworkflowChange of JSON structure
Before
https://github.armbian.com/all-images.json
After
https://github.armbian.com/armbian-images.json
Changes are also in all images that are serving via GitHub. There sha, asc and torrent files are served via different locations to workaround GH limitations of release artifacsts (cache.armbian.com).