diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 3562339b..4ec6218e 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -1,17 +1,41 @@ ## Summary - + + +Closes # ## Checklist -- [ ] `./run-ci-local.sh` passes (gofmt, golangci-lint, build, pg_regress, - journey) -- [ ] Tests added/updated (test-first) -- [ ] Docs updated where applicable (README / USAGE / INSTALL / ARCHITECTURE) -- [ ] Bakery / mesh / distributed changes: TLA+ model updated and TLC - re-checked (`docs/formal/`) -- [ ] Commit messages are short and imperative +- [ ] The pull request links a tracking issue. +- [ ] `./run-ci-local.sh` passes (gofmt, golangci-lint, shellcheck, hadolint, + actionlint, build, pg_regress, journey). +- [ ] The change includes unit tests, written before the code, or the Testing + section explains why it has none. +- [ ] The change includes regression tests in `extension/coldfront/test/`, or + the Testing section explains why it has none. +- [ ] The Testing section notes any impact on the integration tests + (`ci/journey.sh` and the CI matrix). +- [ ] The change updates `README.md` and `docs/`, or the Notes for Reviewers + explain why it needs no update. +- [ ] The change adds no secrets, and the Risk section covers its security + impact. +- [ ] A bakery, mesh, or distributed change updates the TLA+ model and re-runs + TLC (`docs/formal/`). +- [ ] Author's AI review completed and findings addressed. +- [ ] Commit messages are short and imperative. -## Related issues +## Testing -Closes # + + +## Risk + + + +## How to Revert + + + +## Notes for Reviewers + + diff --git a/.github/workflows/ci-walkthrough.yml b/.github/workflows/ci-walkthrough.yml index 29d9f603..6f3536ea 100644 --- a/.github/workflows/ci-walkthrough.yml +++ b/.github/workflows/ci-walkthrough.yml @@ -1,6 +1,7 @@ name: CI - Walkthrough on: push: + branches: [main] paths: ['examples/walkthrough/**', '.devcontainer/**', 'docs/walkthrough*.md', '.github/workflows/ci-walkthrough.yml'] pull_request: paths: ['examples/walkthrough/**', '.devcontainer/**', 'docs/walkthrough*.md', '.github/workflows/ci-walkthrough.yml'] @@ -20,8 +21,6 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v7 - - name: Run ShellCheck - run: shellcheck -x --source-path=examples/walkthrough examples/walkthrough/*.sh - name: Validate Docker Compose run: | docker compose -f examples/walkthrough/docker-compose.yml config --quiet diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1c2361ba..90dc222c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -4,7 +4,8 @@ name: CI # steps never diverge from local. Two jobs: # # quick — every push + PR. ci/matrix.sh --quick = preflight (gofmt/vet/lint/ -# unit tests/build) + ONE hermetic cell (PG18 · vanilla · tiered · +# shellcheck/hadolint/actionlint/unit tests/build) + ONE hermetic +# cell (PG18 · vanilla · tiered · # primary · s3) on SeaweedFS + the pg_regress unit layer. No cloud # creds, no real cloud calls. This is the fast gate and is exactly # what run-ci-local.sh runs. diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a5ee3ffd..7e3ef357 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -25,7 +25,8 @@ # 3. Push RPMs to dnf.pgedge.com and DEBs to apt.pgedge.com, back up to S3, # and publish a promotion manifest + Slack notification. # -# common-functions.sh is supplied by pgedge-builder-action at build time. +# pgedge-builder-action runs common/build.sh, which sources the committed copy +# of the shared build helpers in common/common-functions.sh. # ------------------------------------------------------------------------- name: Release diff --git a/.hadolint.yaml b/.hadolint.yaml new file mode 100644 index 00000000..3461eeed --- /dev/null +++ b/.hadolint.yaml @@ -0,0 +1,10 @@ +# Rules the current Dockerfiles break. docker/Dockerfile.duckdb15-base is the +# base image recipe, so any edit to it, an inline ignore comment included, +# needs a base image republish. +failure-threshold: warning +ignored: + - DL3002 # Build stages run as root, and each final stage switches to postgres. + - DL3003 # A build step changes into a cloned source tree within one RUN. + - DL3006 # The manylinux stage aliases pull untagged images. + - DL3029 # The manylinux stage aliases pick one image per architecture. + - DL3041 # dnf installs packages without a pinned version. diff --git a/CLAUDE.md b/CLAUDE.md index 6e7896af..7fe12537 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,7 +16,7 @@ Every change keeps all five. A change that cannot is raised before it is made, n - Every public function should have a unit test - Never rig a test: never change its timing, ordering or inputs so it stops meeting the case where it fails. A failing test gets a product fix. An assertion is weakened only to a guarantee agreed beforehand, and the change says it is weaker. Before changing a test alongside a fix, confirm the changed test still fails on the unfixed code in every interleaving and input it can meet. - Run `make test` after every change -- MUST run `./run-ci-local.sh` before every commit — it runs gofmt, golangci-lint, tests, build, Docker integration tests +- MUST run `./run-ci-local.sh` before every commit — it runs gofmt, golangci-lint, shellcheck (`-S warning`), hadolint, actionlint, tests, build, Docker integration tests - GitHub Actions CI must always be identical in steps to `run-ci-local.sh` — never let them diverge - MUST verify any mesh/distributed/bakery change against the TLA+ model before committing it. Scope (the cold-write serialization protocol): the bakery functions in `extension/coldfront/coldfront--1.0.sql` (`_claim_iceberg_lock`, `_insert_claim`, `_take_iceberg_claim`, `_exec_iceberg_with_claim`, `_enqueue_release`, `_on_claim_apply`, `_on_claim_release`, `ensure_replicated`), the C `XactCallback` in `extension/coldfront/src/coldfront.c` (`coldfront_xact_callback`, which releases the claim at `XACT_EVENT_COMMIT`, after pg_duckdb has committed the Iceberg transaction at `XACT_EVENT_PRE_COMMIT`), the `coldfront.iceberg_async_parquet` ordering, and the spock/`synchronous_*` GUCs gating claim replication. The one question: does the change alter the modeled protocol (claimant set, claim/conditional-update/release ordering, async-vs-stock parquet ordering, defer/drain steps)? If yes, model it first: update `docs/formal/Bakery.tla`, re-translate (`pcal.trans`), and re-run every TLC config (safe configs report "No error has been found"; `Bakery_race.cfg` still violates `NoLakekeeperConflict`, and `Bakery_adopt_race.cfg` still violates `NoDoubleRegistration`). If no (a pure refactor whose emitted claim/lock/release sequence is byte-identical, leaving the `.tla`/`.cfg` untouched), that equivalence argument IS the proof: do NOT re-run TLC, since an unchanged model only reprints the same verdicts. See `docs/formal/README.md`. - Update README.md as you implement functionality; update ARCHITECTURE.md in the same commit as the structural change it describes diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c978262a..0f832590 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -14,9 +14,9 @@ Thanks for your interest in improving ColdFront. - **Test-driven.** Write the test first, then the implementation. Prioritise smoke / integration / endpoint tests over unit tests. - **Run the gate before every commit:** `./run-ci-local.sh` — gofmt, - golangci-lint, unit tests, build, the pg_regress layer, and the canonical - user journey. GitHub Actions runs the identical `ci/matrix.sh` harness, so - local and CI never diverge. + golangci-lint, shellcheck, hadolint, actionlint, unit tests, build, the + pg_regress layer, and the canonical user journey. GitHub Actions runs the + identical `ci/matrix.sh` harness, so local and CI never diverge. - **Bakery / mesh / distributed changes** must be modelled and verified in the TLA+ spec **first** ([docs/formal/](docs/formal/)) before the code change lands. @@ -30,6 +30,40 @@ Thanks for your interest in improving ColdFront. - Open the PR against `main` and complete the [pull-request checklist](.github/PULL_REQUEST_TEMPLATE.md). +## Reviewing Pull Requests + +Every change to `main` goes through a pull request. A pull request merges into +`main` when the pull request meets these requirements: + +- The description links the GitHub issue that the change resolves. +- CI passes, including the tests and the linters described in + [Checking Code Style](#checking-code-style). +- CodeRabbit and Codacy have reviewed the pull request, and the author has + fixed each finding or answered each finding on the pull request with a + reason. +- A code owner listed in [.github/CODEOWNERS](.github/CODEOWNERS), other than + the author, has approved the latest commit. + +[REVIEW.md](REVIEW.md) describes what a defect looks like in ColdFront and +what the reviewer checks. + +A code owner may merge their own pull request without a second approval when +the change alters no code or behaviour, such as a version bump, a changelog +cleanup, or a typo fix in a comment or a document. In the CI workflows under +`.github/workflows/` and in the release and packaging scripts, such a pull +request may change only version numbers. CI and the automated reviews must +still pass, and the author adds a comment to the pull request that explains why +the pull request merged without review. + +## Checking Code Style + +CI checks Go code with gofmt, go vet, and golangci-lint, the shell scripts +with shellcheck, the Dockerfiles with hadolint, and the GitHub workflows with +actionlint. CI also builds the documentation with `mkdocs build --strict` and +scans the commit history for secrets with gitleaks. On each pull request, +CodeRabbit runs shellcheck, hadolint, yamllint, markdownlint, and semgrep, and +Codacy runs its static analysis. + ## Versioning ColdFront uses two independent version numbers. Release tags follow three-part diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 00000000..21dde584 --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,135 @@ +# Reviewing Changes to ColdFront + +This file describes what a defect looks like in ColdFront, which paths the +automated review tools skip, and what a reviewer checks in every pull request. +The author's AI review, CodeRabbit, and the approving reviewer all work from +this file. + +## Recognizing a Failure + +ColdFront runs inside the PostgreSQL server process and moves data between +PostgreSQL and Apache Iceberg, so a defect can lose data or crash the server. +A change fails review when the change can cause any of the following: + +- a row that the archiver, a tiered `INSERT`, or the partitioner loses, + duplicates, or alters as the row moves between partitions or between the hot + and cold tiers. +- a cold read that returns rows or values that differ from what PostgreSQL + returns for the same data. +- two cold writes that commit to one Iceberg table out of turn, or two nodes + that register the same Iceberg table. +- a crash or memory error in `extension/coldfront/src/coldfront.c`, which + terminates the backend and makes PostgreSQL end every other session while + the server recovers from the crash. +- a storage credential or catalog token that appears in a log, an error + message, a query text, or a committed file. +- a compaction or a cold `DROP` that deletes a data file that a live snapshot + or an open transaction still reads. + +## Skipping Paths in Automated Review + +CodeRabbit reviews every tracked file. Codacy skips files that do not ship or +that another check covers, as the `.codacy.yaml` file lists, and a change to +that file updates this section to match. + +The following table describes the paths that Codacy skips: + +| Path | Reason | +|---|---| +| `.claude/**` and `CLAUDE.md` | These files instruct coding agents and do not ship. | +| `config.example.yaml` | This file is an example configuration. | +| `THIRD_PARTY_NOTICES.md` | This file is generated license text. | +| `docker/*.patch` | These patches change duckdb-iceberg, and CodeRabbit and the approving reviewer read them. | +| `docs/formal/**` | The TLC model checker checks the TLA+ model in this directory. | +| `extension/coldfront/test/**` | These files are pg_regress tests and their expected output. | +| `**/*_test.go` | These files are Go tests, which do not ship. | + +## Checking a Change + +A reviewer checks the following points in addition to the checklist in the +pull request template. The linters cover formatting, so the review omits +findings about formatting alone. + +### Checking Cold Writes and the Bakery + +The bakery protocol serializes every cold write through the +`coldfront.claims` table. A change to cold writes follows these rules: + +- A change to the bakery functions in the extension SQL, the commit callback + in `coldfront.c`, the `coldfront.iceberg_async_parquet` ordering, or the + Spock settings that replicate claims updates `docs/formal/Bakery.tla` and + re-runs every TLC configuration. A pure refactor instead explains in the + pull request why the claim, commit, and release sequence is unchanged. + [docs/formal/README.md](docs/formal/README.md) describes the model and every + configuration. +- A cold write runs outside any subtransaction, such as a `SAVEPOINT` or a + PL/pgSQL `EXCEPTION` block, because pg_duckdb refuses every cold write inside + one. +- A cold write commits to the catalog without a retry, because the bakery + prevents commit conflicts. + +### Checking Generated SQL + +The query rewrite emits SQL that DuckDB or a second session runs, and that SQL +must read the same in every session. Generated SQL follows these rules: + +- Every relation, function, and operator reference is schema-qualified, so + the code does not rely on `search_path`. +- Every value that leaves the session as text goes through + `coldfront._canonical_text`. +- A change to the shape of a rendered qual or `INSERT` regenerates every + pg_regress expected file that echoes the statement, including the `_1.out` + variants. + +### Checking Component Boundaries + +Each component keeps a narrow set of dependencies. A change respects these +boundaries: + +- SQL and the C extension make HTTP calls only through the catalog client in + duckdb-iceberg, and only the Go programs call the Lakekeeper management API. +- The archiver orchestrates plain parameterized SQL through pgx and uses no + DuckDB, Iceberg, or Arrow Go libraries. +- The compactor in `cmd/compactor/` stays a separate Go module, so its + dependencies stay out of the archiver. + +### Checking Tests and Dependencies + +Tests prove the behaviour that a change claims, and a dependency workaround +proves that it is still needed. A change meets these conditions: + +- A bug fix includes a test that fails on the unfixed code. +- A test changed alongside a fix still fails on the unfixed code in every + interleaving and input that the test can meet. +- A workaround for a dependency names the dependency and the pinned version + that shows the defect, and includes a test that runs the dependency without + the workaround and fails once the defect is gone. +- A change to the base image recipe (`docker/Dockerfile.duckdb15-base` and the + files that it copies) passes CI only after `base-image.yml` publishes the + base image under the tag that `ci/base-ref.sh` prints. + +### Checking Documentation + +The documentation describes the current behaviour. A change keeps the +documentation consistent in these ways: + +- `README.md` and `docs/index.md` change together where they share the + introduction. +- A new, renamed, or removed guide in `docs/` updates the `mkdocs.yml` + navigation and the `README.md` table of contents in the same commit. +- `mkdocs build --strict` passes with no warnings. + +## Reporting Findings + +Every reviewer reports findings by these rules, so that the reviews of a pull +request converge: + +- Label each finding as Critical, Major, or Minor. A Critical finding can cause + a failure listed in [Recognizing a Failure](#recognizing-a-failure). A Major + finding causes an error or a wrong result on a narrower path. A Minor finding + is any other defect. +- State whether the finding concerns code that the pull request changes or code + that existed before the pull request. +- Omit findings that concern style alone. +- On a later review of the same pull request, review only the new commits, and + do not repeat an earlier finding. diff --git a/ci/journey.sh b/ci/journey.sh index 7185058e..ff76cdd1 100755 --- a/ci/journey.sh +++ b/ci/journey.sh @@ -1849,8 +1849,7 @@ EOSQL # Two vector columns on one table. Each gets its own cluster column and its own # assignment; only the first gets the file sort order, so only its probe prunes. # Its own table, so nothing above depends on the second column existing. - local i2 - for i2 in 1 2 3 4 5; do + for _ in 1 2 3 4 5; do q_may "$HOST" "SELECT coldfront.create_iceberg_table('public','icetwo','[{\"name\":\"id\",\"type\":\"bigint\"},{\"name\":\"ts\",\"type\":\"timestamptz\"},{\"name\":\"embedding\",\"type\":\"vector(3)\"},{\"name\":\"summary\",\"type\":\"vector(2)\"}]'::jsonb);" >/dev/null 2>&1 [ "$(q "$HOST" "SELECT count(*) FROM pg_class WHERE relname='icetwo' AND relkind='v';")" = "1" ] && break sleep 2 @@ -8227,9 +8226,9 @@ story_vector_probe_pruning() { # probed read has two ICEBERG_SCAN nodes, the probe arm first and the # unassigned arm second, each read on its own (strict jsonpath: lax .** # yields every array element twice). - local counters s1 t1 s2 t2 + local counters s1 t1 t2 counters=$(q "$HOST" "SELECT duckdb.raw_query('SET custom_profiling_settings = ''{\"OPERATOR_TYPE\": \"true\", \"OPERATOR_ROW_GROUPS_SCANNED\": \"true\", \"OPERATOR_TOTAL_ROW_GROUPS_TO_SCAN\": \"true\"}'''); SELECT duckdb.raw_query('SET enable_profiling = ''json'''); SELECT duckdb.raw_query('SET profiling_output = ''/tmp/tc193.json'''); SELECT id FROM public.tcvp ORDER BY embedding <=> ARRAY[100,100,100]::real[] LIMIT 1; SELECT duckdb.raw_query('SET enable_profiling = ''no_output'''); SELECT string_agg((j->>'operator_row_groups_scanned') || ' ' || (j->>'operator_total_row_groups_to_scan'), ' ' ORDER BY o) FROM jsonb_path_query(pg_read_file('/tmp/tc193.json')::jsonb, 'strict \$.** ? (@.operator_name == \"ICEBERG_SCAN\")') WITH ORDINALITY AS t(j, o);" | tail -1) - read -r s1 t1 s2 t2 <<< "$counters" + read -r s1 t1 _ t2 <<< "$counters" assert_gt "TC-193: the file holds more than one row group" 1 "$t1" assert_gt "TC-193: the probe arm read at least one row group" 0 "$s1" assert_gt "TC-193: the probe arm skipped the row groups of the cluster it did not visit" "$s1" "$t1" diff --git a/ci/matrix.sh b/ci/matrix.sh index d1e909dc..89b9acf5 100755 --- a/ci/matrix.sh +++ b/ci/matrix.sh @@ -1,7 +1,8 @@ #!/bin/bash # ci/matrix.sh — the ColdFront CI gate. # -# Runs the host-side preflight ONCE (gofmt, go vet, golangci-lint, unit tests, build), +# Runs the host-side preflight ONCE (gofmt, go vet, golangci-lint, shellcheck, +# hadolint, actionlint, unit tests, build), # then exercises the canonical user journey (ci/journey.sh) across deployment # cells. The journey is the single spec; a "cell" is one # PG-major × topology × mode × target combination. Per-cell the topology script @@ -28,7 +29,7 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" # shellcheck source=ci/lib.sh source "$SCRIPT_DIR/lib.sh" -cd "$ROOT" +cd "$ROOT" || exit 1 SCOPE=""; BE_ONLY="" while [ $# -gt 0 ]; do case "$1" in @@ -40,6 +41,10 @@ esac; done [ -n "$SCOPE" ] || { echo "usage: ci/matrix.sh --quick|--full|--backend "; exit 2; } # ── Preflight (host-side, once) ────────────────────────────────────────────── +# lint : run a pinned linter image over the tree, read-only, +# so the local gate and GitHub run the same linter versions. +lint() { docker run --rm -v "$ROOT:/mnt:ro" -w /mnt "$@"; } + preflight() { step "preflight 1: gofmt" if [ -n "$(gofmt -l .)" ]; then gofmt -d .; fail "code is not formatted"; exit 1; fi @@ -58,11 +63,30 @@ preflight() { if ! "$linter" run ./...; then fail "golangci-lint found issues"; exit 1; fi pass "lint clean" - step "preflight 4: unit tests" + step "preflight 4: shellcheck" + local scripts + mapfile -t scripts < <({ git ls-files '*.sh'; git grep -InE '^#!.*[/ ](ba|da)?sh( |$)' | awk -F: '$2 == 1 {print $1}'; } | sort -u) + if ! lint koalaman/shellcheck:v0.11.0 -x --source-path=SCRIPTDIR -S warning "${scripts[@]}"; then + fail "shellcheck found issues"; exit 1 + fi + pass "shellcheck clean (${#scripts[@]} scripts)" + + step "preflight 5: hadolint" + local dockerfiles + mapfile -t dockerfiles < <(git ls-files '*Dockerfile*') + if ! lint hadolint/hadolint:v2.15.1 hadolint "${dockerfiles[@]}"; then fail "hadolint found issues"; exit 1; fi + pass "hadolint clean (${#dockerfiles[@]} Dockerfiles)" + + step "preflight 6: actionlint" + # -u 0: the image's default user (guest) cannot read an owner-only checkout. + if ! lint -u 0 -e SHELLCHECK_OPTS='-S warning' rhysd/actionlint:1.7.12; then fail "actionlint found issues"; exit 1; fi + pass "actionlint clean" + + step "preflight 7: unit tests" if ! make test 2>&1; then fail "unit tests"; exit 1; fi pass "unit tests" - step "preflight 5: build" + step "preflight 8: build" if ! make build 2>&1; then fail "build"; exit 1; fi pass "build ($(ls -lh bin/archiver 2>/dev/null | awk '{print $5}'))" if ! bin/archiver --version | grep -q . || ! bin/partitioner --version | grep -q .; then @@ -70,19 +94,19 @@ preflight() { fi pass "--version ($(bin/archiver --version))" - step "preflight 6: compactor module (separate go.mod — iceberg-go)" + step "preflight 9: compactor module (separate go.mod — iceberg-go)" if ! make compactor GOLANGCI="$linter" 2>&1; then fail "compactor module"; exit 1; fi pass "compactor (vet, lint, test, build $(ls -lh bin/compactor 2>/dev/null | awk '{print $5}'))" if ! bin/compactor --version | grep -q .; then fail "compactor --version"; exit 1; fi - step "preflight 7: docs (mkdocs build --strict)" + step "preflight 10: docs (mkdocs build --strict)" if ! command -v mkdocs >/dev/null 2>&1; then fail "mkdocs not found (pip install mkdocs-material)"; exit 1 fi if ! mkdocs build --strict 2>&1; then fail "mkdocs build --strict"; exit 1; fi pass "docs build --strict" - step "preflight 8: base recipe hash (ci/base-ref.sh)" + step "preflight 11: base recipe hash (ci/base-ref.sh)" local tag tmp tag="$(ci/base-ref.sh)" tmp="$(mktemp -d)"; mkdir -p "$tmp/ci"; cp ci/base-ref.sh "$tmp/ci/"; cp -r docker "$tmp/" diff --git a/ci/ops.sh b/ci/ops.sh index 3855c729..e0d20841 100755 --- a/ci/ops.sh +++ b/ci/ops.sh @@ -32,7 +32,7 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" # shellcheck source=ci/lib.sh source "$SCRIPT_DIR/lib.sh" -cd "$ROOT" +cd "$ROOT" || exit 1 PG="${PG_MAJOR:-18}"; KEEP=0 while [ $# -gt 0 ]; do case "$1" in diff --git a/ci/probe-snowflake.sh b/ci/probe-snowflake.sh index 50d395c9..673c6e4c 100755 --- a/ci/probe-snowflake.sh +++ b/ci/probe-snowflake.sh @@ -16,7 +16,7 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" # shellcheck source=ci/lib.sh source "$SCRIPT_DIR/lib.sh" -cd "$ROOT" +cd "$ROOT" || exit 1 PG="${PG_MAJOR:-18}"; KEEP=0 while [ $# -gt 0 ]; do case "$1" in diff --git a/ci/probe-standby.sh b/ci/probe-standby.sh index c21cf15d..a7a5bd9a 100755 --- a/ci/probe-standby.sh +++ b/ci/probe-standby.sh @@ -32,7 +32,7 @@ while [ $# -gt 0 ]; do case "$1" in *) echo "probe-standby.sh: unknown arg $1"; exit 2;; esac; done -cd "$ROOT" +cd "$ROOT" || exit 1 export PG_MAJOR="$PG" COMPOSE="docker compose -f docker-compose.matrix.yml" DB=coldfront-db-1 diff --git a/ci/topo/mesh.sh b/ci/topo/mesh.sh index 97c56391..39c77edf 100755 --- a/ci/topo/mesh.sh +++ b/ci/topo/mesh.sh @@ -49,7 +49,7 @@ if [ "$BACKEND" = aws ]; then : "${COLDFRONT_AWS_REGION:?--backend aws needs COLDFRONT_AWS_REGION}" fi -cd "$ROOT" +cd "$ROOT" || exit 1 export PG_MAJOR="$PG" require_base "$PG" || exit 1 COMPOSE="docker compose -f $COMPOSE_FILE" diff --git a/ci/topo/vanilla.sh b/ci/topo/vanilla.sh index 542b4982..47bd097b 100755 --- a/ci/topo/vanilla.sh +++ b/ci/topo/vanilla.sh @@ -80,7 +80,7 @@ if [ "$BACKEND" = aws-vended ]; then : "${COLDFRONT_AWS_EXTERNAL_ID:?--backend aws-vended needs COLDFRONT_AWS_EXTERNAL_ID}" fi -cd "$ROOT" +cd "$ROOT" || exit 1 export PG_MAJOR="$PG" # consumed by docker-compose.matrix.yml build arg + entrypoint require_base "$PG" || exit 1 # exports COLDFRONT_BASE for the same build export COMPOSE="docker compose -f $COMPOSE_FILE" # journey TC-277 recreates db with it diff --git a/common/build.sh b/common/build.sh index 42ea7e38..88d2a3e8 100644 --- a/common/build.sh +++ b/common/build.sh @@ -6,6 +6,7 @@ set -euo pipefail # where is a packaging dir under the repo root that holds a # common.sh + build-rpm.sh / build-deb.sh (e.g. "packaging/pg_duckdb"). COMPONENT_NAME=$1 +# shellcheck source=/dev/null source "$(dirname "$0")/../${COMPONENT_NAME}/common.sh" # common-functions.sh is a committed copy of the shared pgEdge build helpers @@ -14,6 +15,7 @@ source "$(dirname "$0")/../${COMPONENT_NAME}/common.sh" # same pattern as ai-dba-workbench's packaging/scripts/common-functions.sh. COMMON_FILE="$(dirname "$0")/common-functions.sh" if [ -f "$COMMON_FILE" ]; then + # shellcheck source=common/common-functions.sh source "$COMMON_FILE" else echo "Error: $COMMON_FILE not found!" >&2 diff --git a/common/common-functions.sh b/common/common-functions.sh index d641a0bb..9dad3261 100755 --- a/common/common-functions.sh +++ b/common/common-functions.sh @@ -315,9 +315,11 @@ configure_pgedge_apt_repo(){ detect_os_type(){ if command -v dnf &>/dev/null || command -v yum &>/dev/null; then echo "Detected RPM-based system" + # shellcheck source=/dev/null source "$(dirname "$0")/../${COMPONENT_NAME}/build-rpm.sh" elif command -v apt-get &>/dev/null; then echo "Detected Debian-based system" + # shellcheck source=/dev/null source "$(dirname "$0")/../${COMPONENT_NAME}/build-deb.sh" else echo "Unsupported platform: No known package manager found" >&2 diff --git a/docs/installation.md b/docs/installation.md index 43069d4d..6af830c9 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -432,9 +432,10 @@ built on the prebuilt ### Pre-Commit Gate `./run-ci-local.sh` runs `ci/matrix.sh --quick`: gofmt, go vet, golangci-lint, -unit tests, build, the compactor module gate (vet, lint, test, build), -`mkdocs build --strict`, the base recipe hash check, the pg_regress unit layer, -and the full journey on one representative cell (PG18 · vanilla · tiered · s3). +shellcheck, hadolint, actionlint, unit tests, build, the compactor module gate +(vet, lint, test, build), `mkdocs build --strict`, the base recipe hash check, +the pg_regress unit layer, and the full journey on one representative cell +(PG18 · vanilla · tiered · s3). The gate is fast and runs on every commit. GitHub Actions ([.github/workflows/ci.yml](https://github.com/pgEdge/ColdFront/blob/main/.github/workflows/ci.yml)) runs the identical `ci/matrix.sh` harness: `--quick` on every pull request and diff --git a/packaging/coldfront-bin/build-rpm.sh b/packaging/coldfront-bin/build-rpm.sh index 98d417c2..580c262a 100644 --- a/packaging/coldfront-bin/build-rpm.sh +++ b/packaging/coldfront-bin/build-rpm.sh @@ -1,6 +1,7 @@ #!/bin/bash set -euo pipefail +# shellcheck disable=SC2034 # setup_dnf_build_env (common-functions.sh) reads $RHEL under `set -u`. RHEL="$(rpm --eval %rhel)" ARCH=$(uname -m) # x86_64 | aarch64 (rpm arch) case "$ARCH" in diff --git a/packaging/coldfront/build-rpm.sh b/packaging/coldfront/build-rpm.sh index 2e535c8a..d1aa1451 100644 --- a/packaging/coldfront/build-rpm.sh +++ b/packaging/coldfront/build-rpm.sh @@ -1,6 +1,7 @@ #!/bin/bash set -euo pipefail +# shellcheck disable=SC2034 # setup_dnf_build_env (common-functions.sh) reads $RHEL under `set -u`. RHEL="$(rpm --eval %rhel)" prepare() { diff --git a/packaging/coldfront/common.sh b/packaging/coldfront/common.sh index 27d57e93..b60db70f 100644 --- a/packaging/coldfront/common.sh +++ b/packaging/coldfront/common.sh @@ -2,11 +2,12 @@ # common.sh - Common environment variables for the ColdFront extension package. # Build once per PostgreSQL major version (fan-out across the matrix). +# shellcheck disable=SC2034 # read by pgedge-detect-build-matrix PER_PG_VERSION=true # Default PostgreSQL version and derived values export PG_VERSION="${PG_VERSION:-17.7}" -export PG_MAJOR_VERSION="$(echo "$PG_VERSION" | cut -d. -f1)" +export PG_MAJOR_VERSION="${PG_VERSION%%.*}" # The coldfront extension source lives in THIS repo (extension/coldfront), so # there is no upstream repo/branch to fetch. The package version is the diff --git a/packaging/lakekeeper/build-rpm.sh b/packaging/lakekeeper/build-rpm.sh index 92f7c09c..379ff9cc 100755 --- a/packaging/lakekeeper/build-rpm.sh +++ b/packaging/lakekeeper/build-rpm.sh @@ -1,7 +1,7 @@ #!/bin/bash set -euo pipefail -# setup_dnf_build_env (common-functions.sh) reads $RHEL under `set -u`. +# shellcheck disable=SC2034 # setup_dnf_build_env (common-functions.sh) reads $RHEL under `set -u`. RHEL="$(rpm --eval %rhel)" # uname arch matches the upstream target-triple prefix (x86_64 / aarch64). diff --git a/packaging/pg_duckdb/build-rpm.sh b/packaging/pg_duckdb/build-rpm.sh index 5be3259c..b9731106 100644 --- a/packaging/pg_duckdb/build-rpm.sh +++ b/packaging/pg_duckdb/build-rpm.sh @@ -1,6 +1,7 @@ #!/bin/bash set -euo pipefail +# shellcheck disable=SC2034 # setup_dnf_build_env (common-functions.sh) reads $RHEL under `set -u`. RHEL="$(rpm --eval %rhel)" CWD="$(pwd)" diff --git a/packaging/pg_duckdb/common.sh b/packaging/pg_duckdb/common.sh index 39b647dc..d22d433a 100644 --- a/packaging/pg_duckdb/common.sh +++ b/packaging/pg_duckdb/common.sh @@ -9,11 +9,12 @@ # .github/workflows/release.yml for the ready-to-use snippet. # Build once per PostgreSQL major version (fan-out across the matrix). +# shellcheck disable=SC2034 # read by pgedge-detect-build-matrix PER_PG_VERSION=true # Default PostgreSQL version and derived values export PG_VERSION="${PG_VERSION:-17.7}" -export PG_MAJOR_VERSION="$(echo "$PG_VERSION" | cut -d. -f1)" +export PG_MAJOR_VERSION="${PG_VERSION%%.*}" # pg_duckdb pinned to the merged PR #1025 commit (DuckDB 1.5.4). No released # pg_duckdb tag carries 1.5.x; this commit pins its duckdb submodule to v1.5.4. diff --git a/run-ci-local.sh b/run-ci-local.sh index beb300ba..a730b16f 100755 --- a/run-ci-local.sh +++ b/run-ci-local.sh @@ -3,7 +3,8 @@ # # Thin wrapper over the CI matrix's quick cell: PG18 · vanilla · tiered · # primary. ci/matrix.sh runs the host-side preflight (gofmt, go vet, -# golangci-lint, unit tests, build), brings up the stack, runs the pg_regress +# golangci-lint, shellcheck, hadolint, actionlint, unit tests, build), brings +# up the stack, runs the pg_regress # unit layer, and # walks the canonical user journey (ci/journey.sh) — which now includes the # race-window regression. The full deployment matrix is `ci/matrix.sh --full`.