From 9fd71876aac52668de4411451d39e97a25072f4c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 16:20:43 -0500 Subject: [PATCH 1/5] Add sql-lint: style linter for PostgreSQL SQL files Port cat_tools PR #49: vendor the shared Postgres-Extensions style linter as a git submodule (.vendor/linter, pinned to b8632c2, same commit used by cat_tools#49), add a `make lint` target via a thin lint.mk hand-off, and gate it into CI as a new `lint` job. LINT_TARGETS is scoped to sql/extension_drop.sql and test/, excluding the frozen sql/extension_drop--1.0.0.sql release snapshot (RELEASE.md: once a version is released its sql/--.sql is never hand-edited again, so linting it would produce permanent, unfixable findings) -- though the linter's own generated-file skip (first line matching both "GENERATED" and "DO NOT EDIT") already excludes it independently. The lint job runs once, ungated (no PG version dependency), and the existing 12-leg PostgreSQL test matrix now gates behind it (needs: [lint]) per the cost-gating pattern of running cheap checks before expensive ones. Its own checkout deliberately omits submodules: true, matching cat_tools's choice, so `make lint` (lint.mk's self-init via `git submodule update --init`) is exercised the same way a developer would run it locally. No pre-existing SQL style violations were found in this repo's actual source (sql/extension_drop.sql, test/*) -- make lint already passes clean. This branch is independent, based on upstream/master. Other in-flight testing-infrastructure branches in this repo also touch test/sql/*.sql and .github/workflows/ci.yml, so a rebase will likely be needed once those land. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 27 ++++++++++++++++++++++++++- .gitmodules | 3 +++ .vendor/linter | 1 + Makefile | 11 +++++++++++ lint.mk | 11 +++++++++++ 5 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 .gitmodules create mode 160000 .vendor/linter create mode 100644 lint.mk diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aad0d23..83aef43 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,7 +9,32 @@ on: branches: [master] pull_request: jobs: + # Style linter (https://github.com/Postgres-Extensions/linter, vendored at + # .vendor/linter). Deliberately checked out WITHOUT submodules -- `make + # lint` is the same command a developer runs locally, and lint.mk + # self-initializes the submodule on first use (see its comment). Using the + # exact same entry point here is what actually proves that self-init works, + # rather than papering over it with a submodules: true checkout. The + # linter's own test suite (fixtures + scanner edge cases) is that repo's + # own CI's job, not this one's. No PostgreSQL needed -- sql-lint is a + # standalone Perl script -- so this doesn't use the pgxn-tools container. + lint: + name: 🧹 SQL Lint + runs-on: ubuntu-latest + steps: + - name: Check out the repo + uses: actions/checkout@v5 + - name: Lint SQL + run: make lint + test: + # Gated behind lint: the 12-leg PG matrix below is comparatively + # expensive, and every leg would fail anyway on a baseline that's already + # broken by a lint violation. success() is required explicitly once a + # job's `if:` references anything -- GitHub only assumes success() as a + # default when no `if:` is written at all. + needs: [lint] + if: success() strategy: matrix: pg: [17, 16, 15, 14, 13, 12, 11, 10, 9.6, 9.5, 9.4, 9.3] @@ -30,7 +55,7 @@ jobs: # job succeeded or was skipped (e.g. a docs-only push with paths-ignore) and # fails if any failed or were cancelled. all-checks-passed: - needs: [test] + needs: [lint, test] if: always() runs-on: ubuntu-latest steps: diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000..9236c50 --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule ".vendor/linter"] + path = .vendor/linter + url = https://github.com/Postgres-Extensions/linter diff --git a/.vendor/linter b/.vendor/linter new file mode 160000 index 0000000..b8632c2 --- /dev/null +++ b/.vendor/linter @@ -0,0 +1 @@ +Subproject commit b8632c2a3d93de664a45f4622235871e8f19cf78 diff --git a/Makefile b/Makefile index 033cf30..faf7681 100644 --- a/Makefile +++ b/Makefile @@ -15,3 +15,14 @@ install: deps cat_tools: $(DESTDIR)$(datadir)/extension/cat_tools.control $(DESTDIR)$(datadir)/extension/cat_tools.control: pgxn install 'cat_tools>=0.2.1' --sudo + +# Style linter (see https://github.com/Postgres-Extensions/linter, vendored +# at .vendor/linter -- lint.mk is the thin local hand-off, see its comment). +# Scoped to the actively-maintained source rather than the default +# `sql/ test/`: sql/extension_drop--1.0.0.sql is a frozen, already-released +# version file (RELEASE.md's "Ongoing development" section -- once a version +# is released, its sql/--.sql is never hand-edited again), so +# linting it would produce permanent, unfixable findings and make `make +# lint` unusable as a CI gate. Lint the hand-maintained source instead. +LINT_TARGETS = sql/extension_drop.sql test/ +include lint.mk diff --git a/lint.mk b/lint.mk new file mode 100644 index 0000000..0d18abf --- /dev/null +++ b/lint.mk @@ -0,0 +1,11 @@ +# lint.mk — thin wrapper; the whole local footprint for consuming +# https://github.com/Postgres-Extensions/linter. Everything else lives in +# the .vendor/linter submodule; see its README for available targets/rules. +# +# Self-initializing (via the rule below) so `make lint` works right after a +# plain `git clone`, with no --recurse-submodules needed, and so CI can rely +# on the exact same entry point a developer would use locally. +.vendor/linter/lint.mk: + git submodule update --init -- .vendor/linter + +include .vendor/linter/lint.mk From 8198f76af4438923837037758f4250cd3452b54e Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 16:22:39 -0500 Subject: [PATCH 2/5] TEMP: deliberate lint violation to verify CI catches it (will be reverted) --- sql/extension_drop.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index 75319be..e9e6b5f 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -316,3 +316,4 @@ DROP FUNCTION __extension_drop.exec( DROP SCHEMA __extension_drop; -- vim: sw=2 ts=2 expandtab +/* deliberate test violation for CI verification */ From e96e6752ab3157b7fdff2f6205c7fa310531856b Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 16:24:19 -0500 Subject: [PATCH 3/5] Revert "TEMP: deliberate lint violation to verify CI catches it (will be reverted)" This reverts commit 8198f76af4438923837037758f4250cd3452b54e. --- sql/extension_drop.sql | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index e9e6b5f..75319be 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -316,4 +316,3 @@ DROP FUNCTION __extension_drop.exec( DROP SCHEMA __extension_drop; -- vim: sw=2 ts=2 expandtab -/* deliberate test violation for CI verification */ From 957fc833a9426ee2923485adf79f6d8f95c25e13 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 17:21:44 -0500 Subject: [PATCH 4/5] Guard vendored linter include so tarball builds don't break lint.mk unconditionally included the .vendor/linter submodule's lint.mk, with a remake rule that runs `git submodule update --init` if the file is missing. GNU Make tries to satisfy every `include` before running any requested target, so a tree with no .git at all (exactly what git archive produces -- PGXN dist packages, make dist, never include submodule content and strip .git entirely) hit the remake rule on every single `make` invocation, not just `make lint`. `git submodule update` fails immediately outside a git repo, aborting the whole build. CI never caught this because CI always runs from a real checkout with submodules present. Guard the include on a real .git being present (directory for a normal clone, or a gitdir: file for a worktree/submodule checkout -- both match $(wildcard .git)). Lint is simply unavailable in a tarball build, which is correct -- there's no reason to lint a frozen source tarball. --- lint.mk | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lint.mk b/lint.mk index 0d18abf..ffcf327 100644 --- a/lint.mk +++ b/lint.mk @@ -5,7 +5,22 @@ # Self-initializing (via the rule below) so `make lint` works right after a # plain `git clone`, with no --recurse-submodules needed, and so CI can rely # on the exact same entry point a developer would use locally. +# +# Guarded on a real .git being present (directory for a normal clone, or a +# `gitdir:` file for a worktree/submodule checkout -- $(wildcard .git) matches +# both). A source tarball (git archive output -- PGXN dist packages, `make +# dist`) has no .git at all and never contains submodule content, so without +# this guard GNU Make would still try to satisfy the `include` below via the +# remake rule on every invocation, `git submodule update --init` would fail +# immediately ("fatal: not a git repository"), and that failure would abort +# every `make` target -- not just `make lint` -- in a tarball build. Lint +# simply isn't available/attempted outside a real git checkout, which is +# correct: a tarball build has no reason to lint. +ifneq ($(wildcard .git),) + .vendor/linter/lint.mk: git submodule update --init -- .vendor/linter include .vendor/linter/lint.mk + +endif From b8d00cef86fffca4b749b0e7e869723ca28c9481 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Wed, 5 Aug 2026 15:41:40 -0500 Subject: [PATCH 5/5] Trigger claude-review re-run (now that PR #15's checkout fix is on master)