Repository management jungling #9230
Conversation
This commit implements a complete parallel repository management system
that allows building and publishing Debian repositories in parallel,
significantly reducing build time for multiple distributions.
- `update-main`: Builds common/main component once for all releases
- `update -R <release>`: Builds release-specific components in isolated DBs
- `merge`: Combines common + release-specific components into final repos
- Isolated databases (aptly-isolated-<release>) avoid locking during parallel builds
- Common component built once, not duplicated per release
- Release-specific components (utils, desktop) built independently
- Final merge combines all components with proper GPG signing
- Fixed GPG signing to target top-level Release files (dists/{release}/Release)
- Pool cleanup before publishing avoids "file already exists" errors
- Smart package import skips duplicates during merge
- Proper handling of empty repositories and missing components
- Improved error handling and logging throughout
1. update-main: Build common component (once)
2. update -R <release>: Parallel workers build release-specific components
3. merge: Combine all components and publish with GPG signatures
This enables GitHub Actions to run multiple release builders in parallel,
reducing total repository build time from hours to minutes.
Signed-off-by: Igor Pecovnik <[email protected]>
This fixes the case where repositories like debs-beta only have packages in the main/common component (e.g., sid with only kernel packages). Previously, the merge command would skip publishing if both utils and desktop repos were empty, resulting in an incomplete repository. Now we always publish at minimum the main/common component, ensuring all distributions with any packages get properly published.
Fixes an issue where subsequent repository runs would fail with 404 errors for Release files when no new packages were added. The problem was that snapshots were only created conditionally, leading to missing components in the published repository. Changes: - update_main: always drop and recreate common snapshot (remove check that prevented updates if snapshot was already published) - process_release: always create utils/desktop snapshots even if repos are empty, ensuring all components are included in publish - merge_repos: always create snapshots for all repos and create repos if they don't exist, preventing missing components on merge This ensures the repository structure is complete on every run, regardless of whether new packages are added. Signed-off-by: Igor Pecovnik <[email protected]>
📝 WalkthroughWalkthroughThis PR refactors Changes
Sequence Diagram(s)sequenceDiagram
participant Worker as Worker Process
participant Update as update-main
participant PubDB as Main DB
participant Merge as merge/publish Flow
participant Signing as Signing Flow
Worker->>Update: Trigger (SINGLE_RELEASE mode)
Update->>PubDB: Create common-${timestamp} snapshot
Update->>Update: Save timestamp to .common-snapshot
Note over PubDB: Isolated workers build release-specific components
Worker->>PubDB: Build & publish utils/desktop components
Merge->>Update: Read .common-snapshot filename
Merge->>PubDB: Fetch timestamped common snapshot
Merge->>Merge: Import isolated release packages into main DB
Merge->>PubDB: Re-publish all releases with common component
Signing->>Merge: Discover Release files
Signing->>Signing: Sign top-level and component-level Release files
Signing->>PubDB: Update signed metadata
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
github-actions
bot
added
size/large
github-actions
bot
added
Needs review
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/repository/repo.sh (1)
1261-1262: Bug: Option parsing mismatch for-Fand-Pflags.The options are defined as requiring arguments (with
:suffix), but they're parsed as boolean flags:# Definition says they require arguments: SHORT=...,F:,P: LONG=...,force-add:,force-publish: # But parsing treats them as flags (no $2 consumed): -F | --force-add ) FORCE_ADD=true shift # Should be shift 2 if argument expected ;;This will cause argument parsing to fail or consume the next option as the argument.
🐛 Fix: Remove the colon to make them boolean flags
-SHORT=i:,l:,o:,c:,p:,r:,h,d,k,R:,F:,P: -LONG=input:,list:,output:,command:,password:,releases:,help,dry-run,keep-sources,single-release:,force-add:,force-publish: +SHORT=i:,l:,o:,c:,p:,r:,h,d,k,R:,F,P +LONG=input:,list:,output:,command:,password:,releases:,help,dry-run,keep-sources,single-release:,force-add,force-publishAlso applies to: 1309-1315
🤖 Fix all issues with AI agents
In @tools/repository/repo.sh:
- Around line 799-820: The variable deb_info is assigned without being declared
local, causing it to leak into global scope; fix by declaring deb_info as local
where it’s used (e.g., add deb_info to the local declaration alongside deb_name,
deb_version, deb_arch in the while loop that processes .deb files and apply the
same local declaration at the other occurrence where deb_info is assigned) so
both instances use a function-local deb_info instead of a global variable.
🧹 Nitpick comments (5)
tools/repository/repo.sh (5)
353-360: Potential issue: stderr suppression may hide aptly errors.The
2>/dev/nullafterrun_aptlypipes stderr to /dev/null, butrun_aptlyalready exits on failure. However, when piping tologger, any diagnostic output from aptly is lost. Consider capturing both stdout and stderr to the logger:- run_aptly -config="${CONFIG}" snapshot drop ${release}-utils | logger -t repo-management 2>/dev/null + run_aptly -config="${CONFIG}" snapshot drop ${release}-utils 2>&1 | logger -t repo-management
565-566: Unused variable:COMMON_SNAPSHOTis set but never used.The comment says "Store in a variable for use later in this function," but
COMMON_SNAPSHOTis not referenced after this assignment. The snapshot name is already saved to the file at line 562, which is used by downstream processes. Consider removing the unused variable assignment.# Save the snapshot name to a file for use by publish processes echo "$snapshot_name" > "${2}/.common-snapshot" log "Common snapshot name saved: $snapshot_name" - - # Store in a variable for use later in this function - COMMON_SNAPSHOT="$snapshot_name"
748-754: Temp config file not cleaned up on error path.When the function returns 1 at line 753 due to missing common snapshot, the temp config file at
$main_db_configis properly cleaned up at line 752. Good error handling!However, if any
run_aptlycommand fails later in the function (which callsexit 1), the temp config file won't be cleaned up. Consider using a trap for cleanup.♻️ Suggested improvement
merge_repos() { local input_folder="$1" local output_folder="$2" log "Merge mode: combining common component with release-specific components" # Create a temp config pointing to the main DB (not isolated) local main_db_config main_db_config="$(mktemp)" + trap "rm -f '$main_db_config'" RETURN sed 's|"rootDir": ".*"|"rootDir": "'$output_folder'"|g' tools/repository/aptly.conf > "$main_db_config"Then remove the manual
rm -f "$main_db_config"calls at lines 743, 752, and 1013.
823-867: Code duplication: utils and desktop import logic is nearly identical.The import logic for
${release}-utils(lines 773-821) and${release}-desktop(lines 823-867) are almost identical. Consider extracting a helper function to reduce duplication and maintenance burden.♻️ Suggested refactor
# Helper function to import repo from isolated DB to main DB import_repo_from_isolated() { local repo_name="$1" local isolated_config="$2" local main_db_config="$3" local isolated_pool="$4" if ! aptly -config="$isolated_config" repo show "$repo_name" &>/dev/null; then return 0 fi log "Importing $repo_name from isolated DB" # Create repo in main DB if it doesn't exist if ! aptly -config="$main_db_config" repo show "$repo_name" &>/dev/null; then local release="${repo_name%-*}" local component="${repo_name##*-}" run_aptly -config="$main_db_config" repo create \ -component="$repo_name" \ -distribution="$release" \ -comment="Armbian $repo_name repository" \ "$repo_name" fi # ... rest of import logic }Then call it for both utils and desktop.
916-928: Redundant file read inside loop.The common snapshot file is read on every iteration of the releases loop (lines 917-928), but the value doesn't change between iterations. Consider hoisting this read outside the loop for efficiency.
+ # Read the common snapshot name once before the loop + local common_snapshot_file="${output_folder}/.common-snapshot" + local common_snapshot_name="" + if [[ -f "$common_snapshot_file" ]]; then + common_snapshot_name=$(cat "$common_snapshot_file") + log "Using common snapshot from file: $common_snapshot_name" + else + log "WARNING: .common-snapshot file not found, using 'common'" + common_snapshot_name="common" + fi + for release in "${releases[@]}"; do log "Publishing $release with common component..." # Determine which components to publish local components_to_publish=() local snapshots_to_publish=() - # Read the timestamped common snapshot name from file - local common_snapshot_file="${output_folder}/.common-snapshot" - if [[ -f "$common_snapshot_file" ]]; then - local common_snapshot_name=$(cat "$common_snapshot_file") - log "Using common snapshot from file: $common_snapshot_name" - components_to_publish=("main") - snapshots_to_publish=("$common_snapshot_name") - else - log "WARNING: .common-snapshot file not found at $common_snapshot_file" - log "Falling back to 'common' snapshot name" - components_to_publish=("main") - snapshots_to_publish=("common") - fi + components_to_publish=("main") + snapshots_to_publish=("$common_snapshot_name")
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 (1)
tools/repository/repo.sh
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: igorpecovnik
Repo: armbian/build PR: 9087
File: .github/workflows/pr-check-pictures.yml:138-146
Timestamp: 2025-12-16T13:40:07.649Z
Learning: In the Armbian build repository, when introducing new requirements or checks (like the board assets verification workflow), the project prefers an initial educational-only period where violations post helpful PR comments and warnings but don't block merges. This allows contributors to become familiar with new requirements before enforcement is enabled (typically after ~6 months).
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub API (https://api.github.com/repos/armbian/build/pulls/{pr_number}/files) to get the complete picture of what files are being added or modified, especially for U-Boot patches that will be applied during the build process.
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
tools/repository/repo.sh
🧬 Code graph analysis (1)
tools/repository/repo.sh (1)
tools/repository/extract-repo.sh (1)
log(21-23)
🔇 Additional comments (6)
tools/repository/repo.sh (6)
274-285: LGTM!The timestamped snapshot approach with file-based handoff is a clean solution for parallel builds. The snapshot name is unique per run (using Unix timestamp), and persisting it to
.common-snapshotallows downstream processes to retrieve the correct snapshot without race conditions.
306-311: LGTM!Clear separation of concerns - isolated workers skip common repo processing, which avoids duplicate work and potential conflicts in parallel execution.
404-410: LGTM!Early return in isolated mode is the correct behavior - the merge command will handle publishing with the common component included. This cleanly separates the build phase (workers) from the publish phase (merge).
647-660: LGTM with a note on fragility.The slash-counting approach to identify top-level Release files works correctly for the expected structure (
dists/{release}/Release). The logic properly filters out component-level and binary-subdirectory Release files.
994-1008: LGTM!Good error handling pattern - the publish command failure is caught, diagnostic information is logged, and the function returns non-zero to signal failure upstream.
1242-1257: LGTM!The updated help text clearly documents the parallel workflow:
- Build common once with
update-main- Workers build release-specific components only
- Final merge combines common with release-specific
This accurately reflects the code changes.
| find "$isolated_pool" -name "*.deb" -type f | while read -r deb_file; do | ||
| # Get package info to check if it belongs to this repo | ||
| local deb_name deb_version deb_arch | ||
| deb_info=$(dpkg-deb -f "$deb_file" Package Version Architecture 2>/dev/null) | ||
| deb_name=$(echo "$deb_info" | sed -n '1s/Package: //p') | ||
| deb_version=$(echo "$deb_info" | sed -n '2s/Version: //p') | ||
| deb_arch=$(echo "$deb_info" | sed -n '3s/Architecture: //p') | ||
| local deb_key="${deb_name}_${deb_version}_${deb_arch}" | ||
|
|
||
| # Check if this package is in the utils repo (isolated) | ||
| # aptly output has leading spaces, so grep without anchors | ||
| if echo "$packages" | grep -qw "${deb_key}"; then | ||
| # Check if package already exists in main DB repo to avoid conflicts | ||
| if echo "$main_db_packages" | grep -qw "${deb_key}"; then | ||
| # Package already in main DB, skip it | ||
| continue | ||
| fi | ||
| run_aptly -config="$main_db_config" repo add -force-replace "${release}-utils" "$deb_file" | ||
| fi | ||
| done | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Variable scope issue: deb_info not declared as local.
At lines 802 and 848, deb_info is assigned without local, making it a global variable. This could cause unexpected behavior if the variable is used elsewhere.
find "$isolated_pool" -name "*.deb" -type f | while read -r deb_file; do
# Get package info to check if it belongs to this repo
local deb_name deb_version deb_arch
- deb_info=$(dpkg-deb -f "$deb_file" Package Version Architecture 2>/dev/null)
+ local deb_info=$(dpkg-deb -f "$deb_file" Package Version Architecture 2>/dev/null)Apply the same fix at line 848.
🤖 Prompt for AI Agents
In @tools/repository/repo.sh around lines 799 - 820, The variable deb_info is
assigned without being declared local, causing it to leak into global scope; fix
by declaring deb_info as local where it’s used (e.g., add deb_info to the local
declaration alongside deb_name, deb_version, deb_arch in the while loop that
processes .deb files and apply the same local declaration at the other
occurrence where deb_info is assigned) so both instances use a function-local
deb_info instead of a global variable.
Description
Do not merge, WIP.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.