fix: Standardize error handling in shell scripts with errexit, nounset, and pipefail (fixes #1556). by junhaoliao · Pull Request #1562 · y-scope/clp

junhaoliao · 2025-11-05T22:52:47Z

Description

Add the long form

set -o errexit
set -o nounset
set -o pipefail

to all shell scripts in the repo.

Checklist

The PR satisfies the contribution guidelines.
This is a breaking change and that has been indicated in the PR title, OR this isn't a
breaking change.
Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Ran start_clp.sh on a host without Docker installed and observed the script exited immediately after an error was reported
Did a global search and confirmed all shell scripts in the repo have included those shell options:

Summary by CodeRabbit

Refactor
- Enhanced error handling consistency across build and deployment scripts by enabling strict shell options that enforce immediate exit on command failures, treat unset variables as errors, and properly propagate pipeline failures.
- Improved dependency validation in installation scripts to ensure required tools are present before execution, providing explicit error messages when dependencies are missing.

…t, and pipefail (fixes y-scope#1556).

coderabbitai · 2025-11-05T22:53:41Z

Walkthrough

This pull request standardizes error handling across 32 shell scripts by replacing shorthand Bash options (set -e, set -u) with their explicit POSIX-compliant equivalents (set -o errexit, set -o nounset, set -o pipefail), ensuring consistent fail-fast behaviour for errors, undefined variables, and pipeline failures.

Changes

Cohort / File(s)	Summary
Docker image build scripts — Ubuntu/CentOS/manylinux/musllinux variants `components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh`, `components/core/tools/docker-images/clp-env-base-centos-stream-9/build.sh`, `components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/build.sh`, `components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh`, `components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh`, `components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/build.sh`, `components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh`	Replaced `set -e`/`set -eu` with explicit `set -o errexit`, `set -o nounset`, and `set -o pipefail` for standardized error handling.
Library installation scripts `components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh`, `components/core/tools/scripts/lib_install/install-curl.sh`, `components/core/tools/scripts/lib_install/libarchive.sh`, `components/core/tools/scripts/lib_install/liblzma.sh`, `components/core/tools/scripts/lib_install/lz4.sh`, `components/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.sh`, `components/core/tools/scripts/lib_install/mariadb-connector-c.sh`, `components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh`, `components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh`, `components/core/tools/scripts/lib_install/zstandard.sh`	Standardized shell error handling by replacing shorthand flags with explicit `set -o` options; `liblzma.sh` also adds dependency checks for curl, make, and gcc.
Utility and container scripts `components/core/tools/scripts/utils/run-in-container.sh`	Replaced `set -e` with explicit `set -o errexit`, `set -o nounset`, and `set -o pipefail`.
Package template admin and system scripts `components/package-template/src/sbin/.common-env.sh`, `components/package-template/src/sbin/admin-tools/archive-manager.sh`, `components/package-template/src/sbin/admin-tools/dataset-manager.sh`, `components/package-template/src/sbin/compress-from-s3.sh`, `components/package-template/src/sbin/compress.sh`, `components/package-template/src/sbin/decompress.sh`, `components/package-template/src/sbin/search.sh`, `components/package-template/src/sbin/start-clp.sh`, `components/package-template/src/sbin/stop-clp.sh`	Added strict Bash error handling with `set -o errexit`, `set -o nounset`, and `set -o pipefail` at script start.
Deployment configuration scripts `tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh`, `tools/deployment/presto-clp/scripts/set-up-config.sh`, `tools/deployment/presto-clp/worker/scripts/generate-configs.sh`	Replaced `set -eu` with explicit `set -o errexit` and `set -o nounset` while preserving `set -o pipefail`.
Docker image and dependency scripts `tools/docker-images/clp-package/build.sh`, `tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh`, `tools/scripts/deps-download/init.sh`	Replaced `set -e`/`set -eu` with explicit error handling flags for consistency and portability.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Rationale: This change exhibits high homogeneity across 32 files—the same pattern (replacing shorthand shell options with explicit POSIX-compliant equivalents) is applied consistently. No logic changes, no new functionality, and no structural modifications. The single exception (liblzma.sh adding a dependency check) is a minor addition that is straightforward to verify.
Specific areas requiring attention:
- components/core/tools/scripts/lib_install/liblzma.sh — verify the new dependency check loop logic (curl, make, gcc validation).

Possibly related issues

Add error handling flags to shell scripts in package #1556 — Implements strict shell options (errexit, nounset, pipefail) across package-template and related scripts, directly addressed by this PR's standardization effort.

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the main change: standardizing error handling in shell scripts by adding errexit, nounset, and pipefail options across the codebase.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 45c201d and 2b0ec8f.

📒 Files selected for processing (33)

components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh (1 hunks)
components/core/tools/docker-images/clp-env-base-centos-stream-9/build.sh (1 hunks)
components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/build.sh (1 hunks)
components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh (1 hunks)
components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh (1 hunks)
components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/build.sh (1 hunks)
components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh (1 hunks)
components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh (1 hunks)
components/core/tools/scripts/lib_install/install-curl.sh (1 hunks)
components/core/tools/scripts/lib_install/libarchive.sh (1 hunks)
components/core/tools/scripts/lib_install/liblzma.sh (1 hunks)
components/core/tools/scripts/lib_install/lz4.sh (1 hunks)
components/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.sh (1 hunks)
components/core/tools/scripts/lib_install/mariadb-connector-c.sh (1 hunks)
components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh (1 hunks)
components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh (1 hunks)
components/core/tools/scripts/lib_install/zstandard.sh (1 hunks)
components/core/tools/scripts/utils/run-in-container.sh (1 hunks)
components/package-template/src/sbin/.common-env.sh (1 hunks)
components/package-template/src/sbin/admin-tools/archive-manager.sh (1 hunks)
components/package-template/src/sbin/admin-tools/dataset-manager.sh (1 hunks)
components/package-template/src/sbin/compress-from-s3.sh (1 hunks)
components/package-template/src/sbin/compress.sh (1 hunks)
components/package-template/src/sbin/decompress.sh (1 hunks)
components/package-template/src/sbin/search.sh (1 hunks)
components/package-template/src/sbin/start-clp.sh (1 hunks)
components/package-template/src/sbin/stop-clp.sh (1 hunks)
tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh (1 hunks)
tools/deployment/presto-clp/scripts/set-up-config.sh (1 hunks)
tools/deployment/presto-clp/worker/scripts/generate-configs.sh (1 hunks)
tools/docker-images/clp-package/build.sh (1 hunks)
tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh (1 hunks)
tools/scripts/deps-download/init.sh (1 hunks)

🧰 Additional context used

🧠 Learnings (22)

📓 Common learnings

Learnt from: jackluo923
Repo: y-scope/clp PR: 1054
File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh:3-5
Timestamp: 2025-07-07T17:43:04.349Z
Learning: In CLP project build scripts (specifically build.sh files in docker-images directories), maintain consistency with the established pattern of using separate `set -eu` and `set -o pipefail` commands rather than combining them into `set -euo pipefail`, to ensure uniform script structure across all platform build scripts.

Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1261
File: .github/workflows/clp-core-build.yaml:294-332
Timestamp: 2025-08-25T06:29:59.610Z
Learning: In the CLP project, Bill-hbrhbr prefers a "fail fast" approach for CI workflows - allowing potential command availability issues (like getconf in musllinux) to surface through CI failures rather than preemptively adding fallback logic, as they will fix issues when they occur.

Learnt from: haiqi96
Repo: y-scope/clp PR: 594
File: components/clp-package-utils/clp_package_utils/scripts/del_archives.py:56-65
Timestamp: 2024-11-18T16:49:20.248Z
Learning: When reviewing wrapper scripts in `components/clp-package-utils/clp_package_utils/scripts/`, note that it's preferred to keep error handling simple without adding extra complexity.

Learnt from: jackluo923
Repo: y-scope/clp PR: 1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:27-32
Timestamp: 2025-07-07T17:41:15.655Z
Learning: In CLP installation scripts, consistency across platform scripts is prioritized over defensive programming improvements. For example, when extracting Task binaries with tar in `install-prebuilt-packages.sh`, the extraction pattern should remain consistent with other platform scripts rather than adding defensive flags like `--strip-components=1` to handle potential tarball layout changes.

📚 Learning: 2024-11-15T16:28:08.644Z

Learnt from: haiqi96
Repo: y-scope/clp PR: 594
File: components/package-template/src/sbin/del-archives.sh:6-9
Timestamp: 2024-11-15T16:28:08.644Z
Learning: For wrapper scripts in the `components/package-template/src/sbin/` directory, keep them simple and avoid adding additional validation code.

Applied to files:

components/package-template/src/sbin/search.sh
components/package-template/src/sbin/start-clp.sh
components/package-template/src/sbin/.common-env.sh
components/package-template/src/sbin/admin-tools/archive-manager.sh

📚 Learning: 2025-07-07T17:43:04.349Z

Learnt from: jackluo923
Repo: y-scope/clp PR: 1054
File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh:3-5
Timestamp: 2025-07-07T17:43:04.349Z
Learning: In CLP project build scripts (specifically build.sh files in docker-images directories), maintain consistency with the established pattern of using separate `set -eu` and `set -o pipefail` commands rather than combining them into `set -euo pipefail`, to ensure uniform script structure across all platform build scripts.

Applied to files:

components/package-template/src/sbin/search.sh
components/core/tools/scripts/lib_install/zstandard.sh
components/package-template/src/sbin/compress-from-s3.sh
components/package-template/src/sbin/compress.sh
tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh
components/core/tools/scripts/lib_install/install-curl.sh
components/package-template/src/sbin/start-clp.sh
components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh
tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh
components/core/tools/scripts/lib_install/lz4.sh
components/core/tools/scripts/lib_install/libarchive.sh
tools/docker-images/clp-package/build.sh
components/core/tools/docker-images/clp-env-base-centos-stream-9/build.sh
components/package-template/src/sbin/admin-tools/dataset-manager.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh
components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh
components/package-template/src/sbin/decompress.sh
components/package-template/src/sbin/.common-env.sh
tools/deployment/presto-clp/scripts/set-up-config.sh
components/package-template/src/sbin/stop-clp.sh
tools/deployment/presto-clp/worker/scripts/generate-configs.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh
components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh
tools/scripts/deps-download/init.sh
components/package-template/src/sbin/admin-tools/archive-manager.sh
components/core/tools/scripts/lib_install/liblzma.sh
components/core/tools/scripts/lib_install/mariadb-connector-c.sh
components/core/tools/scripts/utils/run-in-container.sh
components/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.sh
components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh

📚 Learning: 2024-11-18T16:49:20.248Z

Learnt from: haiqi96
Repo: y-scope/clp PR: 594
File: components/clp-package-utils/clp_package_utils/scripts/del_archives.py:56-65
Timestamp: 2024-11-18T16:49:20.248Z
Learning: When reviewing wrapper scripts in `components/clp-package-utils/clp_package_utils/scripts/`, note that it's preferred to keep error handling simple without adding extra complexity.

Applied to files:

components/package-template/src/sbin/search.sh
components/core/tools/scripts/lib_install/zstandard.sh
components/package-template/src/sbin/compress.sh
tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh
components/package-template/src/sbin/start-clp.sh
components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh
tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh
tools/docker-images/clp-package/build.sh
components/package-template/src/sbin/admin-tools/dataset-manager.sh
components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh
components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh
tools/deployment/presto-clp/scripts/set-up-config.sh
components/package-template/src/sbin/stop-clp.sh
components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh
components/package-template/src/sbin/admin-tools/archive-manager.sh
components/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.sh
components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh

📚 Learning: 2025-07-07T17:41:15.655Z

Learnt from: jackluo923
Repo: y-scope/clp PR: 1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:27-32
Timestamp: 2025-07-07T17:41:15.655Z
Learning: In CLP installation scripts, consistency across platform scripts is prioritized over defensive programming improvements. For example, when extracting Task binaries with tar in `install-prebuilt-packages.sh`, the extraction pattern should remain consistent with other platform scripts rather than adding defensive flags like `--strip-components=1` to handle potential tarball layout changes.

Applied to files:

components/core/tools/scripts/lib_install/zstandard.sh
components/package-template/src/sbin/compress.sh
tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh
components/core/tools/scripts/lib_install/install-curl.sh
components/package-template/src/sbin/start-clp.sh
components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh
tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh
components/core/tools/scripts/lib_install/lz4.sh
components/core/tools/scripts/lib_install/libarchive.sh
tools/docker-images/clp-package/build.sh
components/core/tools/docker-images/clp-env-base-centos-stream-9/build.sh
components/package-template/src/sbin/admin-tools/dataset-manager.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh
components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh
components/package-template/src/sbin/decompress.sh
tools/deployment/presto-clp/scripts/set-up-config.sh
components/package-template/src/sbin/stop-clp.sh
tools/deployment/presto-clp/worker/scripts/generate-configs.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh
components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh
tools/scripts/deps-download/init.sh
components/package-template/src/sbin/admin-tools/archive-manager.sh
components/core/tools/scripts/lib_install/liblzma.sh
components/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.sh
components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh

📚 Learning: 2025-07-01T14:51:19.172Z

Learnt from: jackluo923
Repo: y-scope/clp PR: 1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh:6-8
Timestamp: 2025-07-01T14:51:19.172Z
Learning: In CLP installation scripts within `components/core/tools/scripts/lib_install/`, maintain consistency with existing variable declaration patterns across platforms rather than adding individual improvements like `readonly` declarations.

Applied to files:

components/core/tools/scripts/lib_install/zstandard.sh
tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh
components/core/tools/scripts/lib_install/install-curl.sh
components/package-template/src/sbin/start-clp.sh
components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh
tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh
components/core/tools/scripts/lib_install/lz4.sh
components/core/tools/scripts/lib_install/libarchive.sh
components/core/tools/docker-images/clp-env-base-centos-stream-9/build.sh
components/package-template/src/sbin/admin-tools/dataset-manager.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh
components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh
components/package-template/src/sbin/.common-env.sh
tools/deployment/presto-clp/scripts/set-up-config.sh
components/package-template/src/sbin/stop-clp.sh
tools/deployment/presto-clp/worker/scripts/generate-configs.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh
components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh
components/core/tools/scripts/lib_install/liblzma.sh
components/core/tools/scripts/lib_install/mariadb-connector-c.sh
components/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.sh
components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh

📚 Learning: 2025-09-04T12:26:54.788Z

Learnt from: quinntaylormitchell
Repo: y-scope/clp PR: 1170
File: tools/deployment/presto-clp/scripts/generate-metadata-filter-file.py:0-0
Timestamp: 2025-09-04T12:26:54.788Z
Learning: The `set-up-config.sh` script in tools/deployment/presto-clp/scripts/ ensures that the output directory for metadata-filter.json already exists before calling generate-metadata-filter-file.py, so directory creation in the Python script is not strictly necessary.

Applied to files:

tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh
tools/deployment/presto-clp/scripts/set-up-config.sh
tools/deployment/presto-clp/worker/scripts/generate-configs.sh

📚 Learning: 2025-08-25T16:27:50.549Z

Learnt from: davemarco
Repo: y-scope/clp PR: 1198
File: components/webui/server/src/plugins/app/Presto.ts:38-43
Timestamp: 2025-08-25T16:27:50.549Z
Learning: In the CLP webui Presto configuration, host and port are set via package settings (configurable), while user, catalog, and schema are set via environment variables (environment-specific). This mixed approach is intentional - settings are typically set by package and some values don't need to be package-configurable.

Applied to files:

tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh

📚 Learning: 2025-05-06T09:48:55.408Z

Learnt from: kirkrodrigues
Repo: y-scope/clp PR: 881
File: components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh:35-41
Timestamp: 2025-05-06T09:48:55.408Z
Learning: For installation scripts in the CLP project, prefer explicit error handling over automatic dependency resolution (like `apt-get install -f`) when installing packages to give users more control over their system.

Applied to files:

components/package-template/src/sbin/start-clp.sh
components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh
tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh
tools/docker-images/clp-package/build.sh
components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh
components/package-template/src/sbin/stop-clp.sh
components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh

📚 Learning: 2025-02-12T22:24:17.723Z

Learnt from: jackluo923
Repo: y-scope/clp PR: 718
File: components/core/tools/scripts/utils/create-debian-package.py:41-41
Timestamp: 2025-02-12T22:24:17.723Z
Learning: For the clp-core Debian package creation script, strict version format validation is considered unnecessary complexity and should be avoided.

Applied to files:

components/package-template/src/sbin/start-clp.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/build.sh

📚 Learning: 2025-08-25T06:29:59.610Z

Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1261
File: .github/workflows/clp-core-build.yaml:294-332
Timestamp: 2025-08-25T06:29:59.610Z
Learning: In the CLP project, Bill-hbrhbr prefers a "fail fast" approach for CI workflows - allowing potential command availability issues (like getconf in musllinux) to surface through CI failures rather than preemptively adding fallback logic, as they will fix issues when they occur.

Applied to files:

components/package-template/src/sbin/start-clp.sh
tools/docker-images/clp-package/build.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh
components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh

📚 Learning: 2025-08-19T14:41:28.901Z

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1152
File: components/clp-package-utils/clp_package_utils/general.py:0-0
Timestamp: 2025-08-19T14:41:28.901Z
Learning: In the CLP codebase, prefer explicit failures over automatic directory creation in utility functions like dump_config. The user junhaoliao prefers to let file operations fail when parent directories don't exist, as this helps catch implementation errors during development rather than masking setup issues with automatic directory creation.

Applied to files:

components/package-template/src/sbin/start-clp.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh

📚 Learning: 2025-05-06T10:07:04.654Z

Learnt from: kirkrodrigues
Repo: y-scope/clp PR: 881
File: components/core/tools/scripts/lib_install/macos/install-all.sh:11-12
Timestamp: 2025-05-06T10:07:04.654Z
Learning: In CLP installation scripts, temporary directories with downloaded files should not be automatically cleaned up on failure (e.g., with EXIT traps) to preserve artifacts for debugging purposes.

Applied to files:

components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh
components/package-template/src/sbin/stop-clp.sh
components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh

📚 Learning: 2025-07-01T14:52:02.418Z

Learnt from: jackluo923
Repo: y-scope/clp PR: 1054
File: components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86/build.sh:18-24
Timestamp: 2025-07-01T14:52:02.418Z
Learning: In the CLP project, consistency across platform build scripts is prioritized over defensive programming when it comes to git remote handling. All build.sh files in docker-images directories should follow the same pattern for git metadata injection.

Applied to files:

tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh
tools/docker-images/clp-package/build.sh
components/core/tools/docker-images/clp-env-base-centos-stream-9/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh
components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh

📚 Learning: 2025-07-01T14:52:15.217Z

Learnt from: jackluo923
Repo: y-scope/clp PR: 1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:6-15
Timestamp: 2025-07-01T14:52:15.217Z
Learning: For installation scripts in the CLP project, maintain consistency in command patterns across platforms rather than applying platform-specific optimizations. When a platform follows a pattern of separate update and install commands (like `apt-get update && apt-get install` or `apk update && apk add`), preserve this pattern for uniform script structure.

Applied to files:

tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh
tools/docker-images/clp-package/build.sh
components/package-template/src/sbin/stop-clp.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh

📚 Learning: 2025-10-20T21:05:30.417Z

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1413
File: tools/docker-images/clp-package/Dockerfile:22-24
Timestamp: 2025-10-20T21:05:30.417Z
Learning: In the clp repository's Dockerfiles, ENV directives should be consolidated into multi-line ENV statements when possible to reduce image layers. ENV statements should only be split into separate commands when consolidation is not possible due to dependencies (e.g., when later variables must reference earlier ones that need to be set first, or when PATH must be modified sequentially).

Applied to files:

tools/docker-images/clp-package/build.sh
components/core/tools/docker-images/clp-env-base-centos-stream-9/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh

📚 Learning: 2025-10-13T03:32:19.293Z

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1414
File: tools/docker-images/clp-package/Dockerfile:20-24
Timestamp: 2025-10-13T03:32:19.293Z
Learning: In the clp repository's Dockerfiles (e.g., tools/docker-images/clp-package/Dockerfile), ENV directives should be split into separate lines for readability rather than consolidated to reduce layer count. This is especially true for PATH modifications, as agreed upon in PR #1166. Later ENV settings may depend on earlier ones (e.g., referencing CLP_HOME).

Applied to files:

tools/docker-images/clp-package/build.sh
components/core/tools/docker-images/clp-env-base-centos-stream-9/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh
components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/build.sh
components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh
components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh

📚 Learning: 2025-08-13T14:48:49.020Z

Learnt from: haiqi96
Repo: y-scope/clp PR: 1144
File: components/clp-package-utils/clp_package_utils/scripts/dataset_manager.py:106-114
Timestamp: 2025-08-13T14:48:49.020Z
Learning: For the dataset manager scripts in components/clp-package-utils/clp_package_utils/scripts/, the native script (native/dataset_manager.py) is designed to only be called through the wrapper script (dataset_manager.py), so dataset validation is only performed at the wrapper level rather than duplicating it in the native script.

Applied to files:

components/package-template/src/sbin/admin-tools/dataset-manager.sh

📚 Learning: 2025-05-26T16:03:05.519Z

Learnt from: quinntaylormitchell
Repo: y-scope/clp PR: 918
File: .github/workflows/clp-execution-image-build.yaml:77-97
Timestamp: 2025-05-26T16:03:05.519Z
Learning: In .github/workflows/clp-execution-image-build.yaml, the ubuntu-jammy-execution-image and ubuntu-noble-execution-image jobs are intentionally kept separate (rather than using a matrix strategy) to make it easier to remove individual platform versions when they reach end of life, such as when jammy eventually becomes obsolete.

Applied to files:

components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh
components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh

📚 Learning: 2025-08-25T00:45:05.464Z

Learnt from: junhaoliao
Repo: y-scope/clp PR: 1242
File: components/core/tools/scripts/lib_install/ubuntu-jammy/install-prebuilt-packages.sh:39-41
Timestamp: 2025-08-25T00:45:05.464Z
Learning: Task v3.44.1 has a regression that breaks shell command processing, particularly rsync commands with brace expansion (e.g., `file.{d.ts,js,wasm}`). This causes CI failures in clp-ffi-js project (issue #110), so CLP should avoid v3.44.1 and use v3.44.0 instead, which fixes the dynamic variable bug without the shell processing regression.

Applied to files:

components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh

📚 Learning: 2025-07-23T09:54:45.185Z

Learnt from: Bill-hbrhbr
Repo: y-scope/clp PR: 1122
File: components/core/src/clp/clp/CMakeLists.txt:175-195
Timestamp: 2025-07-23T09:54:45.185Z
Learning: In the CLP project, when reviewing CMakeLists.txt changes that introduce new compression library dependencies (BZip2, LibLZMA, LZ4, ZLIB), the team prefers to address conditional linking improvements in separate PRs rather than expanding the scope of focused migration PRs like the LibArchive task-based installation migration.

Applied to files:

components/core/tools/scripts/lib_install/liblzma.sh

📚 Learning: 2025-07-01T14:52:15.217Z

Learnt from: jackluo923
Repo: y-scope/clp PR: 1054
File: components/core/tools/scripts/lib_install/musllinux_1_2/install-prebuilt-packages.sh:6-15
Timestamp: 2025-07-01T14:52:15.217Z
Learning: For installation scripts in the CLP project, maintain consistency in command patterns across different platforms (e.g., using separate update and install commands like `apk update && apk add`, `apt update && apt install`, `yum update && yum install`) rather than platform-specific optimizations, to ensure uniform script structure and readability.

Applied to files:

components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh

⏰ 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). (11)

GitHub Check: manylinux_2_28-x86_64-dynamic-linked-bins
GitHub Check: manylinux_2_28-x86_64-static-linked-bins
GitHub Check: musllinux_1_2-x86_64-dynamic-linked-bins
GitHub Check: musllinux_1_2-x86_64-static-linked-bins
GitHub Check: package-image
GitHub Check: ubuntu-jammy-lint
GitHub Check: ubuntu-jammy-static-linked-bins
GitHub Check: ubuntu-jammy-dynamic-linked-bins
GitHub Check: centos-stream-9-dynamic-linked-bins
GitHub Check: centos-stream-9-static-linked-bins
GitHub Check: build-macos (macos-14, false)

🔇 Additional comments (32)

components/package-template/src/sbin/admin-tools/archive-manager.sh (1)

3-5: Excellent—strict mode options correctly applied.

The addition of set -o errexit, set -o nounset, and set -o pipefail standardizes error handling and aligns with the project's established pattern of using separate set commands (as seen in other build and installation scripts). All variables referenced in the script are properly defined before use, and the "$@" pass-through at line 16 is safe under nounset. Since the script already requires Bash-specific syntax (line 7: ${BASH_SOURCE[0]}), enforcing strict Bash semantics is appropriate.

components/core/tools/scripts/lib_install/lz4.sh (1)

9-11: Correct implementation aligned with prior guidance.

The three set -o options are properly added as separate commands, which aligns with the established pattern from PR 1054 that favours separate declarations over combined syntax. All variables referenced throughout the script are initialized before use, so the addition of set -o nounset poses no functional issues. The script should execute correctly with these enhanced safety options.

components/core/tools/scripts/lib_install/zstandard.sh (1)

9-11: Standardized error handling applied correctly.

The explicit long-form options provide better readability and align with the established CLP project pattern of using separate set -o commands. The existing set +e/set -e blocks at lines 32–35 and 72–75 continue to work correctly for capturing exit codes.

tools/docker-images/clp-package/build.sh (1)

3-4: Standardized error handling applied correctly.

The explicit long-form options enhance clarity and follow the established CLP project pattern. All variables are properly defined and quoted where necessary.

components/core/tools/docker-images/clp-env-base-musllinux_1_2-aarch64/build.sh (1)

3-4: Standardized error handling applied correctly.

The change to explicit long-form options aligns with the established pattern across CLP docker-image build scripts. All variables and command substitutions are properly quoted.

tools/docker-images/clp-package/setup-scripts/install-prebuilt-packages.sh (1)

3-4: Standardized error handling applied correctly.

The explicit options follow the established pattern and cause no issues in this straightforward package installation script.

tools/deployment/presto-clp/coordinator/scripts/generate-configs.sh (1)

3-4: Standardized error handling applied correctly.

The explicit options improve clarity and are particularly beneficial here since set -o pipefail (line 5) ensures the file substitution pipeline (lines 10–16) properly fails if any command returns non-zero.

components/core/tools/scripts/utils/run-in-container.sh (1)

3-5: Strict error handling standardized.

The addition of explicit strict-mode options improves script robustness. All variables are properly quoted throughout the docker run invocation.

components/core/tools/docker-images/clp-env-base-manylinux_2_28-aarch64/build.sh (1)

3-4: Standardized error handling applied correctly.

The explicit options follow the established pattern across all platform-specific build scripts in this PR. Script structure and variable usage remain correct.

components/core/tools/scripts/lib_install/libarchive.sh (1)

8-10: Standardized error handling applied correctly.

The explicit long-form options match the established CLP project pattern. The existing set +e/set -e blocks at lines 31–34 and 72–75 continue to work correctly for capturing exit codes during conditional checks.

tools/deployment/presto-clp/worker/scripts/generate-configs.sh (1)

3-5: Standard error handling applied correctly.

The explicit options are added as separate commands, consistent with the established project pattern for build scripts (as per PR 1054 learnings). All variables in the script are properly handled for strict mode.

components/package-template/src/sbin/admin-tools/dataset-manager.sh (1)

3-5: Standard error handling applied correctly with improved variable safety.

The strict mode options are properly added as separate commands. The subsequent sourcing of common_env_path and use of $CLP_HOME will now fail fast if variables are undefined, which is the correct behaviour for a wrapper script.

components/package-template/src/sbin/compress-from-s3.sh (1)

3-5: Standard error handling applied correctly with improved variable safety.

The strict mode options follow the established pattern. Like dataset-manager.sh, this wrapper script will now properly fail if the sourced file doesn't define expected variables.

components/core/tools/scripts/lib_install/manylinux_2_28/install-packages-from-source.sh (1)

3-5: Standard error handling applied correctly.

The strict mode options are properly separated. This installation coordinator script calls subscripts with static version strings, which is compatible with strict mode.

components/core/tools/scripts/lib_install/musllinux_1_2/install-packages-from-source.sh (1)

3-5: Standard error handling applied correctly.

The strict mode options are properly separated, consistent with the manylinux_2_28 variant and the established project pattern.

components/core/tools/scripts/lib_install/install-curl.sh (1)

3-5: Standard error handling applied correctly.

The strict mode options are properly added as separate commands. The script's input validation (line 8) ensures the version argument is checked before use, which is compatible with set -o nounset. The set -o pipefail is particularly important here for build commands (tar, make).

components/core/tools/docker-images/clp-env-base-musllinux_1_2-x86_64/build.sh (1)

3-5: Standard error handling applied correctly.

The strict mode options are properly separated. Docker build commands and git introspection at lines 19-25 will now properly fail if any step encounters an issue, and all variable expansions are correctly quoted for strict mode.

components/core/tools/docker-images/clp-env-base-ubuntu-jammy/build.sh (1)

3-5: Standard error handling applied correctly.

The strict mode options are properly added as separate lines. This build script follows the same error-handling pattern as the musllinux_1_2 variant, with proper quoting for all variable expansions and git command handling.

components/core/tools/scripts/lib_install/ubuntu-jammy/install-packages-from-source.sh (1)

3-5: Proper strict mode initialization. The separate set -o commands follow the established CLP project pattern for consistency across platform build scripts. Based on learnings.

tools/scripts/deps-download/init.sh (1)

3-5: Strict mode correctly applied. The three explicit options follow the standardized pattern and prevent errors from propagating silently.

tools/deployment/presto-clp/scripts/set-up-config.sh (1)

3-4: Augmentation of existing strict mode is sound. Adding set -o errexit and set -o nounset while preserving the existing set -o pipefail aligns with the standardization goals.

components/core/tools/docker-images/clp-env-base-centos-stream-9/build.sh (1)

3-5: Strict mode applied consistently. This build script properly initializes all three error-handling options in the standardized separate-line format.

components/core/tools/docker-images/clp-env-base-manylinux_2_28-x86_64/build.sh (1)

3-4: Consistent augmentation of strict mode. Adding the remaining two strict-mode options completes the standardization while respecting the pre-existing pipefail setting.

components/core/tools/docker-images/clp-core-ubuntu-jammy/build.sh (1)

3-5: Strict mode properly initialized. The three separate options are applied in the standardized format, ensuring consistent error handling across docker build scripts.

components/core/tools/scripts/lib_install/centos-stream-9/install-packages-from-source.sh (1)

3-5: Strict mode applied correctly. The separate set -o commands ensure consistent fail-fast behaviour across all platform installation scripts.

components/package-template/src/sbin/stop-clp.sh (1)

3-5: Strict mode additions align with project standards.

The use of separate set -o commands follows the established CLP pattern and maintains consistency across build scripts.

components/package-template/src/sbin/.common-env.sh (1)

3-5: Strict mode compatible with existing error-handling patterns.

The sourced environment file's conditional error checks (using return 1), fallback patterns (||, 2>/dev/null), and pipelines are all compatible with the new strict options. Error propagation to wrapper scripts will improve reliability.

components/package-template/src/sbin/start-clp.sh (1)

3-5: Strict mode additions are appropriate for this wrapper.

Changes follow the established CLP pattern and are compatible with the sourced .common-env.sh environment.

components/package-template/src/sbin/compress.sh (1)

3-5: Strict mode additions are appropriate.

Changes follow the established CLP pattern.

components/package-template/src/sbin/search.sh (1)

3-5: Strict mode additions are appropriate.

Changes follow the established CLP pattern.

components/package-template/src/sbin/decompress.sh (1)

3-5: Strict mode additions are appropriate.

Changes follow the established CLP pattern.

components/core/tools/scripts/lib_install/liblzma.sh (1)

9-11: Changes align well with PR objectives and standardization goals.

The three set -o commands are added as separate, explicit long-form options, which standardizes error handling per the PR's POSIX-compliant approach. Variable usage throughout the script is safe under set -o nounset, and the existing set +e / set -e pattern on lines 78–81 (used to capture exit status) will continue to work correctly with these settings.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Bill-hbrhbr

lgtm

…t, and pipefail (fixes y-scope#1556). (y-scope#1562)

fix: Standardize error handling in shell scripts with errexit, nounse…

705a9ab

…t, and pipefail (fixes y-scope#1556).

junhaoliao and others added 2 commits November 6, 2025 00:54

Merge branch 'main' into shell-err

b8846b2

move shell options after comments

2b0ec8f

junhaoliao requested a review from Bill-hbrhbr November 6, 2025 06:06

junhaoliao marked this pull request as ready for review November 6, 2025 06:06

junhaoliao requested a review from a team as a code owner November 6, 2025 06:06

Bill-hbrhbr approved these changes Nov 6, 2025

View reviewed changes

junhaoliao merged commit d7c65f5 into y-scope:main Nov 6, 2025
29 checks passed

junhaoliao deleted the shell-err branch May 7, 2026 19:46

junhaoliao added a commit to junhaoliao/clp that referenced this pull request May 17, 2026

fix: Standardize error handling in shell scripts with errexit, nounse…

0bc5136

…t, and pipefail (fixes y-scope#1556). (y-scope#1562)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Standardize error handling in shell scripts with errexit, nounset, and pipefail (fixes #1556).#1562

fix: Standardize error handling in shell scripts with errexit, nounset, and pipefail (fixes #1556).#1562
junhaoliao merged 3 commits into
y-scope:mainfrom
junhaoliao:shell-err

junhaoliao commented Nov 5, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Nov 5, 2025 •

edited

Loading

Uh oh!

Bill-hbrhbr left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

junhaoliao commented Nov 5, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Validation performed

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related issues

Pre-merge checks and finishing touches

Uh oh!

Bill-hbrhbr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

junhaoliao commented Nov 5, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Nov 5, 2025 •

edited

Loading