Dust off make_full_schema and add CI using it to show schema diffs.

.ci/scripts/schema_diff.py

@@ -0,0 +1,227 @@
+#!/usr/bin/env python3
+#
+# Get a diff showing the change in the database schema.
+#
+# Usage:
+#   # Compare against develop (default):
+#   PGUSER=postgres PGPASSWORD=postgres scripts-dev/schema_diff.py
+#
+#   # Compare against a specific branch/commit:
+#   PGUSER=postgres PGPASSWORD=postgres scripts-dev/schema_diff.py --base origin/release-v1.100
+
+import argparse
+import os
+import subprocess
+import sys
+import tempfile
+from pathlib import Path
+
+REPO_ROOT = Path(__file__).resolve().parent.parent.parent
+SCHEMA_DIR = "synapse/storage/schema"
+MAKE_FULL_SCHEMA_SCRIPT = REPO_ROOT / "scripts-dev" / "make_full_schema.sh"
+
+
+def run_make_full_schema(output_dir: Path) -> None:
+    """Run make_full_schema.sh, piping the password via stdin."""
+    pg_user = os.environ.get("PGUSER", "")
+    pg_password = os.environ.get("PGPASSWORD", "")
+    if not pg_user:
+        print("ERROR: PGUSER environment variable not set.", file=sys.stderr)
+        sys.exit(1)
+    if not pg_password:
+        print("ERROR: PGPASSWORD environment variable not set.", file=sys.stderr)
+        sys.exit(1)
+
+    cmd: list[str] = [
+        "poetry",
+        "run",
+        str(MAKE_FULL_SCHEMA_SCRIPT),
+        "-p",
+        pg_user,
+        "-o",
+        str(output_dir),
+        "-c",
+        "-n",
+        "9999",
+    ]
+
+    print(f"Running: {' '.join(cmd)}", file=sys.stderr)
+
+    proc = subprocess.Popen(
+        cmd,
+        stdin=subprocess.PIPE,
+        stdout=subprocess.PIPE,
+        stderr=subprocess.STDOUT,
+        cwd=REPO_ROOT,
+        text=True,
+    )
+    stdout, _ = proc.communicate(input=pg_password + "\n")
+    # Forward script output to stderr so stdout stays clean for markdown
+    if stdout:
+        print(stdout, file=sys.stderr, end="")
+    if proc.returncode != 0:
+        print(
+            f"ERROR: make_full_schema.sh failed with exit code {proc.returncode}",
+            file=sys.stderr,
+        )
+        sys.exit(proc.returncode)
+
+
+def diff_schemas(
+    before_dir: Path, after_dir: Path, before_ref: str, after_ref: str
+) -> str:
+    """Diff SQLite and Postgres full schemas, return a Markdown report."""
+    parts: list[str] = [
+        "## Schema Diff",
+        "",
+        "Please check that this looks as expected!",
+        "",
+    ]
+
+    for db in ["common", "main", "state"]:
+        for engine in ["sqlite", "postgres"]:
+            filename = f"full.sql.{engine}"
+
+            before_file = before_dir / db / "full_schemas" / "9999" / filename
+
+            after_file = after_dir / db / "full_schemas" / "9999" / filename
+
+            if not before_file.exists():
+                raise RuntimeError(f"No before file found for {db = }, {engine = }")
+            if not after_file.exists():
+                raise RuntimeError(f"No after file found for {db = }, {engine = }")
+
+            result = subprocess.run(
+                ["diff", "-U", "10", str(before_file), str(after_file)],
+                capture_output=True,
+                text=True,
+            )
+
+            if result.returncode == 0:
+                parts.append(f"### {db} ({engine})\n\nUnchanged\n")
+            else:
+                parts.append(f"### {db} ({engine})\n\n```diff\n{result.stdout}\n```\n")
+
+    return "\n".join(parts)
+
+
+def main() -> None:
+    parser = argparse.ArgumentParser(description="Show database schema changes")
+    parser.add_argument(
+        "--base",
+        default="develop",
+        help="Base commit/branch to compare against (default: develop)",
+    )
+    args = parser.parse_args()
+
+    # Create temp output directory with before/after subdirectories
+    with tempfile.TemporaryDirectory(prefix="schema_diff_") as tmpdir:
+        after_dir = Path(tmpdir) / "after"
+        before_dir = Path(tmpdir) / "before"
+        after_dir.mkdir()
+        before_dir.mkdir()
+
+        print("\n--- Running make_full_schema.sh (after) ---", file=sys.stderr)
+        run_make_full_schema(after_dir)
+
+        # Checkout base and run make_full_schema.sh
+        print(
+            f"\n--- Checking out {args.base} and running make_full_schema.sh (before) ---",
+            file=sys.stderr,
+        )
+
+        # Save current ref so we can return to it without detaching.
+        # (Not useful in CI, but is useful for local development.)
+        # If we are on a named branch, use the branch name; otherwise use the SHA.
+        head_ref = subprocess.run(
+            ["git", "rev-parse", "--abbrev-ref", "HEAD"],
+            capture_output=True,
+            text=True,
+            cwd=REPO_ROOT,
+            check=True,
+        ).stdout.strip()
+
+        before_sha = subprocess.run(
+            ["git", "rev-parse", args.base],
+            capture_output=True,
+            text=True,
+            cwd=REPO_ROOT,
+            check=True,
+        ).stdout.strip()
+        after_sha = subprocess.run(
+            ["git", "rev-parse", "HEAD"],
+            capture_output=True,
+            text=True,
+            cwd=REPO_ROOT,
+            check=True,
+        ).stdout.strip()
+
+        # Check if working tree is dirty before potentially stashing
+        status = subprocess.run(
+            [
+                "git",
+                "status",
+                # Machine-readable output for easy parsing
+                "--porcelain",
+            ],
+            capture_output=True,
+            text=True,
+            cwd=REPO_ROOT,
+            check=True,
+        ).stdout.strip()
+
+        did_stash = False
+        if status:
+            print("Stashing local changes before checkout...", file=sys.stderr)
+            subprocess.run(
+                [
+                    "git",
+                    "stash",
+                    "push",
+                    "--include-untracked",
+                    "-m",
+                    "schema_diff temporary stash",
+                ],
+                cwd=REPO_ROOT,
+                check=True,
+            )
+            did_stash = True
+
+        try:
+            subprocess.run(["git", "checkout", args.base], cwd=REPO_ROOT, check=True)
+
+            # Refresh dependencies
+            print("Installing dependencies for base commit...", file=sys.stderr)
+            subprocess.run(
+                ["poetry", "install", "--no-root", "--extras", "postgres"],
+                cwd=REPO_ROOT,
+                check=True,
+                # Poetry install is noisy, so pipe its stdout to stderr
+                stdout=sys.stderr,
+            )
+
+            run_make_full_schema(before_dir)
+        finally:
+            print("Returning to HEAD...", file=sys.stderr)
+            subprocess.run(
+                [
+                    "git",
+                    "checkout",
+                    head_ref,
+                ],
+                cwd=REPO_ROOT,
+                check=True,
+            )
+            if did_stash:
+                subprocess.run(["git", "stash", "pop"], cwd=REPO_ROOT, check=True)
+                print("✓ Restored stashed changes.", file=sys.stderr)
+
+        # Diff
+        print("\n--- Diffing schemas ---", file=sys.stderr)
+        markdown = diff_schemas(before_dir, after_dir, before_sha, after_sha)
+
+        print(markdown)
+
+
+if __name__ == "__main__":
+    main()

.github/workflows/schema_diff.yml

@@ -0,0 +1,101 @@
+name: Schema Diff
+
+on:
+  pull_request:
+    paths:
+      - synapse/storage/schema/*/delta/**
+      - synapse/storage/schema/*/full_schemas/**
+      - .github/workflows/schema_diff.yml
+
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+permissions:
+  contents: read
+
+jobs:
+  # Posts a GitHub PR comment that shows what the effective change to the schema is.
+  # Provides an excuse to run the `make_full_schema.sh` script in CI (so we keep it working)
+  # and can act as a review aid for schema changes, letting you easily see the diff of the
+  # end result, even when background updates or complex schema deltas are present.
+  show-schema-diff:
+    name: Show schema diff
+    runs-on: ubuntu-latest
+
+    permissions:
+      pull-requests: write
+
+    steps:
+      - name: Start postgres with a faked clock
+        background: true
+        id: postgres
+        run: |
+          # Build a docker image with faketime
+          mkdir /tmp/postgres-faketime
+          cat > /tmp/postgres-faketime/Dockerfile <<'EOF'
+            FROM postgres:14-alpine
+            RUN apk add --no-cache libfaketime
+
+            # It seems like it could be harmful to fake the monotonic timer
+            # as it might prevent deadlock detection, etc.
+            # But not sure, just doing out of precaution.
+            ENV FAKETIME_DONT_FAKE_MONOTONIC=1
+            ENTRYPOINT ["faketime", "-f", "2001-05-25 12:42:42", "docker-entrypoint.sh"]
+            CMD ["postgres"]
+          EOF
+          docker build -t localhost/postgres-faketime /tmp/postgres-faketime
+
+          # Run it in the background
+          docker run -d --name postgres -p 5432:5432 \
+            -e POSTGRES_PASSWORD=postgres \
+            -e POSTGRES_INITDB_ARGS="--lc-collate C --lc-ctype C --encoding UTF8" \
+            --health-cmd pg_isready --health-interval 10s \
+            --health-timeout 5s --health-retries 5 \
+            localhost/postgres-faketime
+
+      - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0
+        with:
+          fetch-depth: 0
+
+      - name: Install PostgreSQL client
+        run: sudo apt-get -qq install postgresql-client
+
+      - uses: matrix-org/setup-python-poetry@5bbf6603c5c930615ec8a29f1b5d7d258d905aa4 # v2.0.0
+        with:
+          poetry-version: "2.4.1"
+          extras: "postgres"
+          python-version: "3.x"
+
+      - name: Wait for Postgres to be up
+        run: |
+          until [ "$(docker inspect -f '{{.State.Health.Status}}' postgres)" = healthy ]; do sleep 2; done
+
+      - name: Generate schema diff
+        id: schema_diff
+        env:
+          PGHOST: localhost
+          PGUSER: postgres
+          PGPASSWORD: postgres
+        run: |
+          poetry run python .ci/scripts/schema_diff.py \
+            --base origin/develop \
+            > "${{ runner.temp }}/schema_diff.md"
+
+      - name: Stop postgres
+        cancel: postgres
+
+      # If the generation step failed, write an error message so the sticky
+      # comment step still has a file to read.
+      - name: Ensure output file exists on failure
+        if: always() && steps.schema_diff.outcome == 'failure'
+        run: |
+          echo "⚠️ Schema diff generation failed. See job logs for details." \
+            > "${{ runner.temp }}/schema_diff.md"
+
+      - name: Post sticky PR comment
+        uses: marocchino/sticky-pull-request-comment@3d7b8546315c63df45a03981d50a43ec19237f80 # v3
+        if: always()
+        with:
+          header: schema-diff
+          path: ${{ runner.temp }}/schema_diff.md

changelog.d/20027.misc

@@ -0,0 +1 @@
+Dust off `make_full_schema` and add CI using it to show schema diffs.

docs/development/database_schema.md

@@ -84,17 +84,11 @@ reference only.
 If you want to recreate these schemas, they need to be made from a database that
 has had all background updates run.
 
-To do so, use `scripts-dev/make_full_schema.sh`. This will produce new
-`full.sql.postgres` and `full.sql.sqlite` files.
+To do so, use the `make_full_schema.sh` script. This will produce new
+`full.sql.postgres` and `full.sql.sqlite` files, after having applied ALL
+schema deltas and background updates.
 
-Ensure postgres is installed, then run:
-
-```sh
-./scripts-dev/make_full_schema.sh -p postgres_username -o output_dir/
-```
-
-NB at the time of writing, this script predates the split into separate `state`/`main`
-databases so will require updates to handle that correctly.
+Run `poetry run scripts-dev/make_full_schema.sh -h` for usage instructions.
 
 ## Delta files