Skip to content

Commit 4e6444d

Browse files
committed
fix(packaging): address code review feedback
- 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
1 parent acf9bcf commit 4e6444d

File tree

4 files changed

+31
-14
lines changed

4 files changed

+31
-14
lines changed

components/core/tools/packaging/alpine-apk/package.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ cd "${abuild_dir}"
104104
# -d: disable dependency checking
105105
# -P: set PKGDEST (where the .apk is written)
106106
abuild -F checksum
107-
REPODEST="/tmp/clp-apk-out" abuild -F -d -P "/tmp/clp-apk-out"
107+
abuild -F -d -P "/tmp/clp-apk-out"
108108

109109
# Copy the built package to the output directory
110110
find /tmp/clp-apk-out -name "*.apk" -exec cp {} "${output_dir}/" \;

components/core/tools/packaging/build.sh

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
# --arch ARCH Target architecture: aarch64, x86_64, or all (default: host)
2121
# --cores N Parallel build jobs (default: nproc)
2222
# --version VER Package version (default: from taskfile.yaml)
23-
# --output DIR Output directory for packages (default: ./build)
23+
# --output DIR Output directory for packages (default: ./packages)
2424
# --clean Remove build artifacts before building
2525
# --help Show this help message
2626

@@ -35,19 +35,24 @@ repo_root="$(cd "${script_dir}/../../../.." && pwd)"
3535
format="all"
3636
cores="$(nproc 2>/dev/null || echo 4)"
3737
version=""
38-
output_dir="${repo_root}/build"
38+
output_dir="${repo_root}/packages"
3939
target_arches=""
4040
clean=false
4141

4242
# --- Argument parsing --------------------------------------------------------
4343

4444
while [[ $# -gt 0 ]]; do
4545
case $1 in
46-
--format) format="$2"; shift 2 ;;
47-
--arch) target_arches="$2"; shift 2 ;;
48-
--cores) cores="$2"; shift 2 ;;
49-
--version) version="$2"; shift 2 ;;
50-
--output) output_dir="$2"; shift 2 ;;
46+
--format) [[ -n "${2:-}" ]] || { echo "ERROR: --format requires a value" >&2; exit 1; }
47+
format="$2"; shift 2 ;;
48+
--arch) [[ -n "${2:-}" ]] || { echo "ERROR: --arch requires a value" >&2; exit 1; }
49+
target_arches="$2"; shift 2 ;;
50+
--cores) [[ -n "${2:-}" ]] || { echo "ERROR: --cores requires a value" >&2; exit 1; }
51+
cores="$2"; shift 2 ;;
52+
--version) [[ -n "${2:-}" ]] || { echo "ERROR: --version requires a value" >&2; exit 1; }
53+
version="$2"; shift 2 ;;
54+
--output) [[ -n "${2:-}" ]] || { echo "ERROR: --output requires a value" >&2; exit 1; }
55+
output_dir="$2"; shift 2 ;;
5156
--clean) clean=true; shift ;;
5257
--help) sed -n '/^# Usage:/,/^[^#]/{ /^#/s/^# \?//p; }' "$0"; exit 0 ;;
5358
*) echo "Unknown option: $1"; echo "Use --help for usage"; exit 1 ;;
@@ -113,8 +118,11 @@ echo " Arches: ${arch_list[*]}"
113118
echo " Output: ${output_dir}"
114119
echo ""
115120

116-
# Remove stale packages from the output directory
117-
rm -f "${output_dir}"/clp-core*.deb "${output_dir}"/clp-core*.rpm "${output_dir}"/clp-core*.apk
121+
# Remove stale packages from the output directory (only for formats being built)
122+
for _fmt in "${format_list[@]}"; do
123+
_fmt=$(echo "${_fmt}" | xargs)
124+
rm -f "${output_dir}"/clp-core*."${_fmt}"
125+
done
118126

119127
# --- Helper: point build/ and .task/ at the right image family ---------------
120128
#
@@ -129,6 +137,15 @@ activate_build_family() {
129137
# Create family-specific directories if they don't exist
130138
mkdir -p "${repo_root}/build-${target_family}" "${repo_root}/.task-${target_family}"
131139

140+
# Remove any real directory at the symlink target (ln -sfn cannot atomically
141+
# replace a real directory — it would create the symlink inside it instead).
142+
for dir_name in build .task; do
143+
local target="${repo_root}/${dir_name}"
144+
if [[ -d "${target}" && ! -L "${target}" ]]; then
145+
rm -rf "${target}"
146+
fi
147+
done
148+
132149
# Point build/ and .task/ at the target family
133150
ln -sfn "build-${target_family}" "${repo_root}/build"
134151
ln -sfn ".task-${target_family}" "${repo_root}/.task"

components/core/tools/packaging/common/bundle-libs.sh

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ lib_install_dir="${LIB_INSTALL_DIR:-/usr/lib/clp}"
4444

4545
# --- Constants ---------------------------------------------------------------
4646

47+
# Keep in sync with CMakeLists.txt build targets and the %files list in
48+
# universal-rpm/package.sh.
4749
BINARIES=(clg clo clp clp-s indexer log-converter reducer-server)
4850

4951
# Libraries provided by the base system (libc, libstdc++, libgcc).
@@ -62,8 +64,8 @@ echo "==> Collecting shared library dependencies..."
6264
for bin in "${BINARIES[@]}"; do
6365
bin_path="${BIN_DIR}/${bin}"
6466
if [[ ! -f "${bin_path}" ]]; then
65-
echo " WARNING: ${bin} not found at ${bin_path} (skipping)"
66-
continue
67+
echo "ERROR: ${bin} not found at ${bin_path}" >&2
68+
exit 1
6769
fi
6870

6971
ldd "${bin_path}" 2>/dev/null | while read -r line; do

components/core/tools/packaging/universal-deb/package.sh

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,6 @@ Description: CLP core universal binaries for log compression and search
6969
Debian 10+, Ubuntu 20.04+, and other glibc-based Debian derivatives.
7070
.
7171
Includes clp-s, clp, clo, clg, indexer, log-converter, and reducer-server.
72-
.
73-
License: Apache-2.0
7472
CTRL
7573

7674
# --- Build .deb --------------------------------------------------------------

0 commit comments

Comments
 (0)