feat(packaging): add universal package build scripts for deb, rpm, and apk#1996
Open
jackluo923 wants to merge 9 commits intoy-scope:mainfrom
Open
feat(packaging): add universal package build scripts for deb, rpm, and apk#1996jackluo923 wants to merge 9 commits intoy-scope:mainfrom
jackluo923 wants to merge 9 commits intoy-scope:mainfrom
Conversation
…d apk Add packaging infrastructure that builds portable CLP core packages for three Linux package formats. Binaries are built on broad-compatibility base images (manylinux_2_28 for glibc, musllinux_1_2 for musl) and bundled with their non-system shared library dependencies via patchelf. Snapshot versions use date+hash from git HEAD (e.g., 0.9.1-dev becomes 0.9.1-20260211.64d0d2e7), mapped to each format's convention: - deb: 0.9.1~20260211.64d0d2e7-1 (tilde pre-release, debian revision) - rpm: 0.9.1~20260211.64d0d2e7 (tilde pre-release, Release: 1 in spec) - apk: 0.9.1_git20260211 (_git suffix, commit hash stored in pkgdesc since apk versions only allow digits)
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a packaging subsystem: orchestrator and scripts to build universal deb/rpm/apk packages, Dockerfiles for packaging environments, and a shared bundling utility that collects and patches non-system shared libraries into staging trees for package creation. Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Orch as Build Orchestrator
participant Docker as Docker Engine
participant Container as Build Container
participant PkgScript as Format Package Script
participant Bundle as bundle-libs.sh
participant Tool as Packaging Tool
User->>Orch: run build.sh (formats, archs, version)
Orch->>Orch: validate args & determine targets
Orch->>Docker: build base & builder images
Docker-->>Orch: images ready
loop per format × arch
Orch->>Container: start builder container
Container->>Container: build CLP core binaries
Container->>PkgScript: execute package.sh (env vars, BIN_DIR)
PkgScript->>PkgScript: validate env, compute pkg version
PkgScript->>Bundle: bundle-libs.sh(STAGING_DIR, BIN_DIR)
Bundle->>Bundle: ldd → copy libs → patchelf RPATHs → install/strip
Bundle-->>PkgScript: staged binaries + libs
PkgScript->>Tool: invoke dpkg-deb / rpmbuild / abuild
Tool-->>PkgScript: package artifacts (.deb/.rpm/.apk)
PkgScript->>Orch: copy artifacts to host OUTPUT_DIR
end
Orch-->>User: summary of produced artifacts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@components/core/tools/packaging/alpine-apk/package.sh`:
- Around line 106-107: The script sets REPODEST and also passes -P to abuild
which is redundant; remove the REPODEST=... environment prefix and call abuild
with the existing flags (e.g., abuild -F -d -P "/tmp/clp-apk-out") so only the
-P flag controls the package destination (or alternatively remove the -P flag
and rely on REPODEST consistently); update the invocation in package.sh where
the abuild -F -d -P command is used to keep a single source of truth for the
output directory.
In `@components/core/tools/packaging/build.sh`:
- Around line 81-89: The version extraction block that sets the variable version
from taskfile.yaml is brittle; modify it to first try a YAML-aware extractor if
available (check for yq/command -v yq) and use yq to read the G_PACKAGE_VERSION
value from "${repo_root}/taskfile.yaml" into version, falling back to a more
flexible shell parsing if yq is not present (make the fallback accept single or
double quotes and surrounding whitespace and handle inline colon spacing for the
G_PACKAGE_VERSION key). Update the code that currently references version and
${repo_root}/taskfile.yaml so the new logic sets the same version variable and
preserves the existing error exit path if no version is found.
- Around line 126-135: activate_build_family currently uses ln -sfn to point
build and .task to family-specific dirs, but ln won't replace an existing real
directory; modify activate_build_family to detect if "${repo_root}/build" or
"${repo_root}/.task" already exist as non-symlink directories and remove them
first (e.g., check [ -d path ] && [ ! -L path ] then rm -rf path) before running
ln -sfn to create symlinks to "build-${target_family}" and
".task-${target_family}" so the symlink replacement is robust.
- Around line 44-55: The flag parsing loop (while ... case ... in the build.sh
snippet) allows value-taking flags like --format, --arch, --cores, --version,
and --output to consume an absent or dash-prefixed next token, causing an
unhelpful "unbound variable" error; update the case branches for these flags to
validate that a next argument exists and does not start with '-' before
assigning (e.g., check that "$2" is non-empty and "${2:0:1}" != "-"), and if the
check fails print a clear message like "Missing value for --format" (or the
respective flag) and exit 1; apply the same guard pattern to target_arches,
cores, version, and output_dir while leaving --clean and --help as-is.
- Around line 116-117: The cleanup currently always removes all package formats
via rm -f "${output_dir}"/clp-core*.deb "${output_dir}"/clp-core*.rpm
"${output_dir}"/clp-core*.apk which causes previously-built formats to be
deleted when running a separate build; change this to only delete extensions for
the format(s) being built by checking the build format variable(s) used in the
script (e.g., FORMAT, FORMATS, or similar) and either loop over that list or
conditionally rm only the matching globs (e.g., remove clp-core*.deb only if
"deb" is in the requested formats, same for "rpm" and "apk") so stale-file
removal is limited to current target formats while still using ${output_dir} and
the clp-core* filename pattern.
- Around line 66-69: The problem is that output_dir is resolved (created and
turned into a real directory) before activate_build_family runs, which prevents
ln -sfn from replacing it and mixes families; also the cleanup removes all
package types unconditionally. Fix by delaying resolution of output_dir (the
mkdir/pwd assignment) until after the build-family activation and per-family
loop, or change the default output path to a separate non-symlinked directory
name (e.g., packages) so activate_build_family can create the family symlink;
update the cleanup logic that currently removes all formats (the glob that
deletes *.deb, *.rpm, *.apk from output_dir) to only delete the formats being
built based on format_list/format so separate invocations don’t clobber
unrelated packages; keep references to output_dir, activate_build_family,
format, and format_list to locate the changes.
In `@components/core/tools/packaging/common/bundle-libs.sh`:
- Line 47: The hardcoded BINARIES=(clg clo clp clp-s indexer log-converter
reducer-server) array in bundle-libs.sh can drift from actual build outputs;
change it to derive the list dynamically from the build output directory (e.g.,
list executables in the build/bin or target directory, filter by expected naming
pattern, and assign the result to BINARIES) and fall back to a documented static
list only if no build artifacts are found; update or add a short comment in
bundle-libs.sh referencing the build system (or the build output dir variable
you use) so future maintainers know the coupling.
In `@components/core/tools/packaging/universal-deb/package.sh`:
- Around line 60-74: The DEBIAN/control heredoc that writes to
"${staging}/DEBIAN/control" for Package: clp-core includes a non-standard
"License" field; remove that "License:" line from the control file generation
and instead add packaging steps to create a copyright file under the package
docs (e.g. write a /usr/share/doc/clp-core/copyright into the staging tree) so
license information is shipped correctly; update the same script section where
the control file is created (the heredoc writing to "${staging}/DEBIAN/control")
and add a small block to populate "${staging}/usr/share/doc/clp-core/copyright"
with the Apache-2.0 text or a pointer to licensing.
In `@components/core/tools/packaging/universal-rpm/package.sh`:
- Around line 86-95: The RPM failure is caused by an explicit %files list in
package.sh that expects seven binaries while bundle-libs.sh currently only warns
and skips missing binaries; fix by adding a validation gate: in bundle-libs.sh
(the section that checks/copies binaries and currently emits a warning for
missing files) make missing required binaries cause an immediate non-zero exit
(fail the build) or alternatively change package.sh to generate the %files
section dynamically from the staged buildroot contents (e.g., enumerate
/usr/bin/* staged files and produce the %files entries) so DEB/APK and RPM are
consistent; update references to the missing-binary check in bundle-libs.sh and
the static %files block in package.sh accordingly.
- Add argument validation for all value-taking flags in build.sh - Change default output dir from build/ to packages/ to avoid conflict with activate_build_family symlink management - Guard activate_build_family against pre-existing real directories - Scope stale-package cleanup to only formats being built - Fail on missing binaries in bundle-libs.sh instead of warning - Remove redundant REPODEST env var in apk/package.sh - Remove invalid License field from DEBIAN/control - Add coupling comment on BINARIES array
Replace short flags with long-form equivalents for less obvious options: ln -sfn → --symbolic --force --no-dereference cp -L → --dereference grep -qE → --quiet --extended-regexp Kept short form for universally known flags (mkdir -p, rm -rf, chown -R, docker -v/-w/-e, etc.).
jackluo923
force-pushed
the
feat/universal-packages
branch
from
999946c to
c006d82
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@components/core/tools/packaging/alpine-apk/package.sh`:
- Around line 106-110: After running abuild and copying APKs, add a guard that
checks for any produced .apk files in /tmp/clp-apk-out (e.g., using glob or find
to count matches) and fail the script with a clear error message if none are
found; modify the section after abuild -F -d -P "/tmp/clp-apk-out" and
before/around the find ... -exec cp {} "${output_dir}/" \; to verify files exist
(referencing the /tmp/clp-apk-out directory, the .apk glob, and the output_dir
variable) and exit non‑zero when the count is zero so the CI detects the
failure.
In `@components/core/tools/packaging/build.sh`:
- Around line 233-238: The clean block currently inside the architecture loop
(checks "${clean}" and removes "${repo_root}/build" "${repo_root}/.task" and
"${repo_root}"/build-* "${repo_root}"/.task-*) causes re-cleans on each arch
when using --arch all; move this clean logic out of the per-architecture loop
(e.g., before the format loop or at the start of the arch loop wrapper) so it
runs once at the start of the build, or if you intend per-arch isolation add a
clarifying comment above the block; update the placement that references the
variables clean and repo_root accordingly to avoid deleting intermediate
build-*/.task-* artifacts between architectures.
- Around line 156-185: In the rpm case of the format resolution switch, add a
short inline comment next to the dockerfile_dir assignment explaining that rpm
uses the same Dockerfile as deb (both dpkg and rpm-build are installed so Docker
layer caching is shared), e.g. annotate the
dockerfile_dir="${script_dir}/universal-deb" line in the rpm branch to reference
the shared builder image/behavior and the existing builder_image_prefix and
base_image_family settings.
In `@components/core/tools/packaging/common/bundle-libs.sh`:
- Around line 71-85: The ldd call for "${bin_path}" currently silences stderr
(2>/dev/null) which hides failures; change the logic in bundle-libs.sh so you
capture ldd's stderr and exit status instead of discarding it: run ldd on
"${bin_path}" and save both stdout and stderr (or redirect stderr to a temp
var/log), then check the ldd exit code before iterating—if non-zero, emit a
clear warning that includes the captured stderr and skip processing that binary
(or handle statically-linked/corrupt binaries explicitly) while leaving the rest
of the loop unchanged; update references around bin_path, the ldd pipeline, and
the subsequent dependency-copying block (EXCLUDE_PATTERN, STAGING_DIR,
lib_install_dir) to use this new guarded flow.
- Add error guard when abuild produces zero .apk files - Add comment on rpm case explaining shared deb Dockerfile - Move --clean block outside the architecture loop to avoid re-cleaning - Detect ldd failures instead of silently suppressing stderr
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@components/core/tools/packaging/build.sh`:
- Around line 290-296: The script hardcodes BIN_DIR=/clp/build/core which
couples the container packaging step to a specific CMake output layout (so
package.sh and the use of '"${format_dir##*/}"' can silently break if the output
dir changes); update the build.sh invocation to either derive BIN_DIR from an
existing config/variable or make it configurable via an environment
variable/parameter and add a clear comment above the BIN_DIR assignment
documenting this coupling (reference BIN_DIR, package.sh, and format_dir and
mirror the style used in the BINARIES array comment in bundle-libs.sh).
- Around line 264-296: The Docker invocation currently overwrites
CFLAGS/CXXFLAGS via -e "CFLAGS=-U_FORTIFY_SOURCE" and -e
"CXXFLAGS=-U_FORTIFY_SOURCE", which discards any preexisting flags; change this
to preserve and append the undefine flag instead—either pass the original env
through and append inside the container (e.g., in the bash -c script set
CFLAGS="${CFLAGS:-} -U_FORTIFY_SOURCE" and export it, same for CXXFLAGS) or
update the -e assignments to merge current values rather than replace them;
apply this change near the docker run environment variables so task deps:core,
task core, and the package.sh invocation see the merged flags.
- Around line 121-125: The stale-package removal loop runs over format_list
before formats are validated by the later case statement, causing a
non-fail-fast UX; fix this by validating or normalizing format_list entries
before the rm loop (either move the existing case-format
validation/normalization logic earlier or add a small validation function that
checks each format in format_list against the allowed set and exits on invalid
entries), and then iterate only over the validated/normalized formats when
running the rm -f "${output_dir}"/clp-core*."${_fmt}" loop; reference
format_list and the existing case-based validation logic to implement this
change.
In `@components/core/tools/packaging/common/bundle-libs.sh`:
- Around line 90-94: The loop that patches bundled shared libraries uses
patchelf --set-rpath inside the for loop over
"${STAGING_DIR}${lib_install_dir}/"*.so* and currently suppresses failures with
`2>/dev/null || true`; remove the error-suppression so patchelf failures surface
and abort the build (i.e., stop silencing stderr and/or remove the `|| true`) so
a failed `patchelf --set-rpath` on any library causes the script to fail fast
and match the binary patching behavior.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@components/core/tools/packaging/alpine-apk/package.sh`:
- Around line 75-78: The abuild-keygen and cp calls currently swallow all errors
(abuild-keygen -an 2>/dev/null || true and cp /root/.abuild/*.rsa.pub
/etc/apk/keys/ 2>/dev/null || true), which can hide real failures; change this
so you capture and check their exit status and surface stderr before proceeding:
run abuild-keygen -an while redirecting stderr to a variable or temp file, if it
fails print a clear error message including the captured stderr and exit
non‑zero; do the same for copying /root/.abuild/*.rsa.pub to /etc/apk/keys/
(check for no-matches, log the reason and exit), so that subsequent abuild
failures show the real root cause rather than a confusing signing error.
In `@components/core/tools/packaging/build.sh`:
- Around line 316-334: The echoed test command strings in the loop over
format_list (cur_format) do not quote ${output_dir}, so if output_dir contains
spaces the displayed docker run lines are misleading; update the echo statements
inside the case branches (deb, rpm, apk) to wrap ${output_dir} in quotes (e.g.,
\"${output_dir}\") wherever it appears in the printed docker run -v ...:/pkgs
fragments to ensure the displayed commands correctly reflect a quoted volume
path.
- Around line 196-201: The clean block unconditionally deletes all build-*
artifacts which causes redundant rebuilds when multiple formats share the same
base_image_family; modify the logic around the clean variable and
activate_build_family to track which base_image_family values have already been
cleaned (e.g., maintain an associative array or exported space-separated
variable cleaned_families and check membership of "${base_image_family}" before
running rm -rf and activate_build_family), so if a family is already cleaned in
this invocation you skip the rm -rf "${repo_root}"/build-* and skip
re-activating the same family; alternatively add a clear comment documenting
that --clean with --format all will perform full rebuilds for shared families.
- Validate formats early before stale-package cleanup - Add comment clarifying CFLAGS override is safe in controlled container - Add comment documenting BIN_DIR coupling with CMake output directory - Remove silent patchelf error suppression on bundled libraries - Improve abuild-keygen error handling (warn instead of suppress) - Track cleaned build families to skip redundant re-cleans - Quote output_dir in displayed test commands
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@components/core/tools/packaging/build.sh`:
- Around line 315-321: The copy step may silently copy nothing; add a guard
after the find/cp block in the packaging loop to detect zero matches and fail
fast: count files matching "${repo_root}/build"/clp-core*.${cur_format} (or
capture the list from the find used in the cp) and if the count is zero, emit an
error (including context like cur_format and output_dir) and exit non‑zero;
update the block around the find -exec cp {} "${output_dir}/" \; in build.sh so
the outer loop fails when no package was produced.
- Around line 75-82: The shellcheck warning indicates the quoted right-hand side
of the =~ operator in the loop (using variables format_list, _fmt and
valid_formats) is treated as a literal; remove the quotes around the RHS so the
=~ performs the intended substring/regex match (or alternatively build a pattern
variable like pattern=" ${_fmt} " and use an unquoted variable on the RHS) and
apply the same change to the other occurrence mentioned (line with the second =~
usage) to silence SC2076 while preserving behavior.
- Remove quotes from RHS of =~ to silence ShellCheck SC2076 - Add zero-package guard after Docker build step
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a portable packaging pipeline that builds CLP core binaries into universal deb, rpm, and apk packages. Binaries are compiled on broad-compatibility base images (manylinux_2_28 for glibc, musllinux_1_2 for musl) and bundled with their non-system shared library dependencies via patchelf, so they work on supported distributions without extra installs.
Compatible distributions
Architecture support
Builds target aarch64 and x86_64. Native builds run directly; cross-architecture builds use QEMU user-mode emulation via Docker's buildx/binfmt infrastructure:
Version conventions
Each format maps the upstream version to its native convention:
clp-core_0.9.1~20260211.64d0d2e7-1_arm64.deb~pre-release,-1debian revisionclp-core-0.9.1~20260211.64d0d2e7-1.aarch64.rpm~pre-release,Release: 1in specclp-core-0.9.1_git20260211-r0.apk_gitsuffix (digits only), commit hash in pkgdesc metadataUsage
Test plan
clp-s --helpclp-s --helpclp-s --helpSummary by CodeRabbit