Complain on compilation issues out loud

.github/gcc-problem-matcher.json

@@ -0,0 +1,17 @@
+{
+  "problemMatcher": [
+    {
+      "owner": "gcc",
+      "pattern": [
+        {
+          "regexp": "^(.+?):(\\d+):(\\d+):\\s+(?:fatal\\s+)?(warning|error):\\s+(.+)$",
+          "file": 1,
+          "line": 2,
+          "column": 3,
+          "severity": 4,
+          "message": 5
+        }
+      ]
+    }
+  ]
+}

.github/workflows/build-werror.yml

@@ -0,0 +1,180 @@
+#
+# Build lolor with -Werror
+#
+# Compiles lolor against every supported PostgreSQL major with warnings
+# promoted to errors.  Only the build is checked here; the Docker test
+# workflow builds lolor without -Werror, so a new warning fails this one
+# job with a clear cause instead of hiding the results of a test run.
+#
+# The matrix covers the build switches lolor's code branches on.  Switches
+# that live in PostgreSQL's pg_config.h or Makefile.global need a
+# PostgreSQL build of their own, so they form the matrix axes:
+#
+#   pgver   lolor compiles against the server headers of each major and
+#           carries PG_VERSION_NUM conditionals, so every supported major
+#           is a separate build.  The list matches the Docker test matrix
+#           in workflow.yml.
+#
+#   cc      gcc and clang produce different warning sets.  PostgreSQL is
+#           built with the same compiler, so the CFLAGS PGXS hands to lolor
+#           were probed against the compiler that receives them.
+#
+#   config  stock: plain configure (-O2), the configuration packagers use.
+#           full:  --enable-cassert, mirroring a developer build.  lolor
+#           branches on no other configure switch: variables used only
+#           inside Assert() turn into unused-variable warnings in "stock",
+#           while the Assert() bodies themselves are only compiled in
+#           "full".  Both use -O2, which enables the flow-sensitive
+#           warnings (maybe-uninitialized and friends) that an -O0 build
+#           never sees.
+#
+# lolor is built with PGXS against the installation, so it needs no
+# PostgreSQL rebuild of its own.  FSDB is lolor's only compile-time
+# switch (it adds elog(DEBUG4) calls to the fsstubs entry points), so
+# each job compiles lolor twice against the one installation: plain and
+# -DFSDB=1.
+#
+# lolor is a plain PGXS extension and does not include Spock headers, so
+# vanilla PostgreSQL is enough here -- neither Spock nor the Spock patches
+# are needed to compile it.
+#
+# A weekly run catches warnings that appear without a change to lolor: a
+# new PostgreSQL minor release, or a newer compiler on the runner image.
+#
+# The gcc problem matcher turns each diagnostic into an annotation, so it
+# shows up on the offending line in the PR's "Files changed" tab.  clang
+# prints diagnostics in the same format.
+#
+
+name: Build with -Werror
+run-name: Compile lolor with warnings as errors
+
+on:
+  workflow_dispatch:
+  pull_request:
+    types: [opened, synchronize, reopened]
+  push:
+    branches: [main]
+  schedule:
+    - cron: '0 22 * * 0'  # Sunday, 22:00 (UTC)
+
+permissions:
+  contents: read
+
+concurrency:
+  group: build-werror-${{ github.ref }}
+  cancel-in-progress: true
+
+defaults:
+  run:
+    # Explicit shell: GitHub then runs with -eo pipefail.
+    shell: bash
+
+jobs:
+  build:
+    strategy:
+      fail-fast: false
+      matrix:
+        pgver: [16, 17, 18, 19]
+        cc: [gcc, clang]
+        config: [stock, full]
+
+    name: PG ${{ matrix.pgver }} ${{ matrix.cc }} ${{ matrix.config }}
+
+    runs-on: ubuntu-latest
+    timeout-minutes: 45
+
+    env:
+      CC: ${{ matrix.cc }}
+      PG_PREFIX: ${{ github.workspace }}/../pg-install
+      PG_SRCDIR: ${{ github.workspace }}/../postgres
+
+    steps:
+      # Checked out at the workspace root, so the relative paths the
+      # compiler prints (src/lolor_inv_api.c) match repository paths for
+      # the annotations.
+      - name: Checkout lolor
+        uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+
+      - name: Install build dependencies
+        run: |
+          sudo apt-get update -qq
+          sudo apt-get install -y --no-install-recommends \
+            build-essential clang bison flex pkg-config \
+            libicu-dev libreadline-dev zlib1g-dev
+
+      # Same ladder as the Docker test workflow: the newest stable minor
+      # tag of the major, falling back to the newest RC and then BETA for
+      # a major that has not gone GA yet, and finally to the REL_x_STABLE
+      # branch for a major that has been branched but not yet tagged.
+      #
+      # --refs drops the peeled ^{} entries, so a failing git is the only
+      # thing that can fail the fetch.  Each selection instead ends in
+      # "|| true": grep exits 1 when a major has no tag of that kind,
+      # which pipefail would otherwise turn into a failed step -- that is
+      # the normal case for every pre-GA major, and the ladder exists
+      # precisely to fall through it.
+      - name: Resolve PostgreSQL source ref
+        id: pgtag
+        run: |
+          PG_TAGS=$(git ls-remote --tags --refs \
+            https://github.com/postgres/postgres.git "REL_${{ matrix.pgver }}_*" \
+            | awk -F/ '{print $NF}')
+          pick() { printf '%s\n' "$PG_TAGS" | grep -E "$1" | sort -V | tail -n 1 || true; }
+          PG_REF=$(pick "^REL_${{ matrix.pgver }}_[0-9]+$")
+          [ -n "$PG_REF" ] || PG_REF=$(pick "^REL_${{ matrix.pgver }}_RC[0-9]+$")
+          [ -n "$PG_REF" ] || PG_REF=$(pick "^REL_${{ matrix.pgver }}_BETA[0-9]+$")
+          if [ -z "$PG_REF" ] \
+             && git ls-remote --exit-code --heads \
+                  https://github.com/postgres/postgres.git \
+                  "REL_${{ matrix.pgver }}_STABLE" >/dev/null; then
+            PG_REF="REL_${{ matrix.pgver }}_STABLE"
+          fi
+          if [ -z "$PG_REF" ]; then
+            echo "::error::Failed to resolve a source ref for PostgreSQL ${{ matrix.pgver }}"
+            exit 1
+          fi
+          echo "PG ${{ matrix.pgver }} resolved to $PG_REF"
+          echo "pg_ref=$PG_REF" >> "$GITHUB_OUTPUT"
+
+      - name: Fetch PostgreSQL source
+        run: |
+          git init "$PG_SRCDIR"
+          git -C "$PG_SRCDIR" remote add origin \
+            https://github.com/postgres/postgres.git
+          git -C "$PG_SRCDIR" fetch --depth 1 origin "${{ steps.pgtag.outputs.pg_ref }}"
+          git -C "$PG_SRCDIR" checkout --detach FETCH_HEAD
+
+      - name: Configure, build, install PostgreSQL
+        id: pg
+        run: |
+          cd "$PG_SRCDIR"
+          CONFIGURE_OPTS=(--prefix="$PG_PREFIX")
+          if [ "${{ matrix.config }}" = "full" ]; then
+            CONFIGURE_OPTS+=(--enable-cassert)
+          fi
+          echo "configure ${CONFIGURE_OPTS[*]}"
+          ./configure "${CONFIGURE_OPTS[@]}"
+          make -j"$(nproc)" -s
+          make install -s
+          echo "$PG_PREFIX/bin" >> "$GITHUB_PATH"
+
+      - name: Register problem matcher
+        run: echo "::add-matcher::.github/gcc-problem-matcher.json"
+
+      # COPT is appended to CFLAGS by PGXS, so it reaches every object.
+      # -k keeps going after the first failing file, so a single run
+      # reports every warning.  The FSDB variant runs even when the plain
+      # build fails, as long as PostgreSQL built, so one run reports the
+      # warnings of both variants.
+      - name: Build lolor
+        if: ${{ !cancelled() && steps.pg.outcome == 'success' }}
+        run: |
+          make USE_PGXS=1 clean
+          make USE_PGXS=1 -k -j"$(nproc)" COPT=-Werror
+
+      - name: Build lolor with FSDB
+        if: ${{ !cancelled() && steps.pg.outcome == 'success' }}
+        run: |
+          make USE_PGXS=1 clean
+          make USE_PGXS=1 -k -j"$(nproc)" COPT="-Werror -DFSDB=1"

src/lolor.c

@@ -61,7 +61,7 @@ get_lobj_table_oid(const char *table)
 }
 
 Oid
-get_LOLOR_LargeObjectRelationId()
+get_LOLOR_LargeObjectRelationId(void)
 {
 	if (!OidIsValid(LOLOR_LargeObjectRelationId))
 		LOLOR_LargeObjectRelationId = get_lobj_table_oid(LOLOR_LARGEOBJECT_CATALOG);
@@ -70,7 +70,7 @@ get_LOLOR_LargeObjectRelationId()
 }
 
 Oid
-get_LOLOR_LargeObjectLOidPNIndexId()
+get_LOLOR_LargeObjectLOidPNIndexId(void)
 {
 	if (!OidIsValid(LOLOR_LargeObjectLOidPNIndexId))
 		LOLOR_LargeObjectLOidPNIndexId = get_lobj_table_oid(LOLOR_LARGEOBJECT_PKEY);
@@ -79,7 +79,7 @@ get_LOLOR_LargeObjectLOidPNIndexId()
 }
 
 Oid
-get_LOLOR_LargeObjectMetadataRelationId()
+get_LOLOR_LargeObjectMetadataRelationId(void)
 {
 	if (!OidIsValid(LOLOR_LargeObjectMetadataRelationId))
 		LOLOR_LargeObjectMetadataRelationId = get_lobj_table_oid(LOLOR_LARGEOBJECT_METADATA);
@@ -88,7 +88,7 @@ get_LOLOR_LargeObjectMetadataRelationId()
 }
 
 Oid
-get_LOLOR_LargeObjectMetadataOidIndexId()
+get_LOLOR_LargeObjectMetadataOidIndexId(void)
 {
 	if (!OidIsValid(LOLOR_LargeObjectMetadataOidIndexId))
 		LOLOR_LargeObjectMetadataOidIndexId = get_lobj_table_oid(LOLOR_LARGEOBJECT_METADATA_PKEY);

src/lolor_fsstubs.c

@@ -63,8 +63,10 @@
 
 #include "lolor.h"
 
-/* define this to enable debug logging */
+/* build with -DFSDB=1 to enable debug logging */
+#ifndef FSDB
 #define FSDB 0
+#endif
 /* chunk size for lo_import/lo_export transfers */
 #define BUFSIZE			8192
 
@@ -129,7 +131,7 @@ lolor_lo_open(PG_FUNCTION_ARGS)
 	LargeObjectDesc *lobjDesc;
 	int			fd;
 
-#ifdef FSDB
+#if FSDB
 	elog(DEBUG4, "lo_open(%u,%d)", lobjId, mode);
 #endif
 
@@ -170,7 +172,7 @@ lolor_lo_close(PG_FUNCTION_ARGS)
 				(errcode(ERRCODE_UNDEFINED_OBJECT),
 				 errmsg("invalid large-object descriptor: %d", fd)));
 
-#ifdef FSDB
+#if FSDB
 	elog(DEBUG4, "lo_close(%d)", fd);
 #endif