diff --git a/.gitignore b/.gitignore index 9b5f0c8b..442128df 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,5 @@ results/ regression.diffs regression.out .pgc_built_for_major +__pycache__/ +*.pyc diff --git a/CHANGELOG.md b/CHANGELOG.md index d8ad9d3a..33365057 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,36 @@ true until the next version shipped. ### Fixed +- No compiled Python artifact is tracked, and the tree ignores the ones the + interpreter writes (#854). `test/__pycache__/ste_check.cpython-312.pyc` was + tracked. Its source, `test/ste_check.py`, was renamed to + `test/plain_language_check.py` in `e9de048`; the `.pyc` outlived it by 146 + commits and was still tracked at `fe1f3a2`, the base of this change. So the + file was compiled code for a module that no longer existed and that CPython + would never open: it reads a `__pycache__` entry only when the matching source + sits beside it. + + A tracked build artifact does not stay still. This one had already re-committed + itself inside an unrelated logical-replication fix, where the diffstat reads + `Bin 6028 -> 6028 bytes` and exactly two bytes differ -- the PEP 552 + source-timestamp word. The compiled code was identical either side. + + `.gitignore` gains `__pycache__/` and `*.pyc`, which it had neither of, and + `test/selftest/` gains a part that fails when a compiled Python artifact is + tracked or when either rule goes missing. Ignoring is not enough on its own and + the suite proves it: restoring the tracked file with both rules in place still + reds two checks, because `.gitignore` has no effect on a file git already + tracks. + + `test/devloop.sh` stages its build directory with `tar --exclude=.git`, so the + tree the suites run out of was not a checkout and the new checks reported five + failures on a clean tree. It now writes a one-line gitfile into the build + directory instead: 45 bytes rather than the 32 MB of copying the object + database, and the loop stays as cheap as it was. Every check in the part also + answers `no-repo` where there is no repository, rather than the answer git + gives by default -- `ls-files` prints nothing, which reads as a clean tree, and + `check-ignore` says "not ignored", which reports a present rule as missing. + - Every test script is executable, so the commands this project documents run as written (#852). No released version is affected: nothing in the extension changed, and this is test tooling only. diff --git a/CONTEXT.md b/CONTEXT.md index ab190d53..1b648e9c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -279,6 +279,13 @@ PostgreSQL 15 through 19 when given none. - Build out of the source tree, or clean between majors. A suite installs into the prefix its `pg_config` names, so two majors sharing a prefix will overwrite each other's `.so`. +- **Build output is never committed.** `.gitignore` covers `*.o`, `*.so`, `*.bc`, + `__pycache__/` and `*.pyc`, and `harness_selftest` fails if a compiled Python + artifact is tracked or if either Python rule goes missing. A tracked artifact + does not stay still: `test/__pycache__/ste_check.cpython-312.pyc` outlived the + source it was compiled from, which had been renamed away, and re-committed + itself -- two bytes of PEP 552 timestamp -- inside an unrelated + logical-replication fix (#854). Where a development environment is containerised, or PostgreSQL is not on the host, that is a property of the machine rather than of the project, and belongs diff --git a/test/__pycache__/ste_check.cpython-312.pyc b/test/__pycache__/ste_check.cpython-312.pyc deleted file mode 100644 index cae72064..00000000 Binary files a/test/__pycache__/ste_check.cpython-312.pyc and /dev/null differ diff --git a/test/devloop.sh b/test/devloop.sh index ed713ac5..64adf09a 100755 --- a/test/devloop.sh +++ b/test/devloop.sh @@ -45,6 +45,29 @@ echo "== devloop: sync $SRC -> $BUILD" rm -rf "$BUILD" mkdir -p "$BUILD" (cd "$SRC" && tar cf - --exclude=.git .) | (cd "$BUILD" && tar xf -) + +# .git is excluded above because it is large -- 32 MB in the audit container -- +# and this loop is meant to be cheap enough to run constantly. But excluding it +# leaves a tree that is not a checkout, and harness_selftest asks git two +# questions about the tree it is running in: whether a compiled Python artifact +# is tracked, and whether `.gitignore` covers one. Outside a repository both are +# unanswerable, and the suite reported five failures on a tree with nothing wrong +# with it (#854). +# +# A gitfile is the whole fix: one line, no bytes copied. git resolves it and +# answers about the recorded tree, which is what the rule is about. +# --absolute-git-dir rather than "$SRC/.git", because SRC may itself be a linked +# worktree, where .git is a FILE and that path is not a git directory. +# +# Only the build dir gets the pointer, and it is wiped on every run. Do not run +# git commands that WRITE in $BUILD: it shares $SRC's index. +if _gd="$(git -C "$SRC" rev-parse --absolute-git-dir 2>/dev/null)"; then + printf 'gitdir: %s\n' "$_gd" > "$BUILD/.git" +else + echo "devloop: $SRC is not a git checkout; harness_selftest's tracked-artifact" \ + "checks cannot be answered in $BUILD" +fi + cd "$BUILD" || exit 1 # Clean rebuild + install + symbol verification. Any failure here is fatal: there diff --git a/test/selftest/310-a-compiled-artifact-must-not-be.sh b/test/selftest/310-a-compiled-artifact-must-not-be.sh new file mode 100644 index 00000000..9c75b34a --- /dev/null +++ b/test/selftest/310-a-compiled-artifact-must-not-be.sh @@ -0,0 +1,126 @@ +# A compiled Python artifact must not be tracked, and the tree must ignore one. +# +# CPython writes a compiled copy of every module it imports into __pycache__. +# Those files are build output: derived, machine-specific, and rewritten by the +# interpreter without anyone asking. Committing one gives it a life of its own. +# +# FOUND THE EXPENSIVE WAY, 2026-08-31 (#854). test/__pycache__/ste_check.cpython-312.pyc +# was tracked at 0e4884c1. Its source, test/ste_check.py, was renamed to +# test/plain_language_check.py in e9de048 -- 135 commits earlier. The .pyc did +# not follow the rename, so the tree carried 6,028 bytes of compiled code for a +# module that no longer exists and that CPython would never open anyway: it +# reads a __pycache__ entry only when the matching source sits beside it. +# +# It had already attached itself to an unrelated commit. cec104f is a logical +# replication fix (#435) and its diffstat carries +# "test/__pycache__/ste_check.cpython-312.pyc | Bin 6028 -> 6028 bytes". Exactly +# two bytes differ, at offsets 9 and 10 -- the PEP 552 source-timestamp word, +# 1785516574 -> 1785525429. The compiled code was identical either side +# (src_size 4164 on both). Somebody's checkout re-stamped the source, Python +# rewrote the header, git recorded a binary diff, and it rode into a commit +# about logical replication. That is the whole failure mode: a tracked build +# artifact joins whichever commit is next. +# +# WHY THE RULE IS ANCHORED ON git AND NOT ON THE FILESYSTEM. "no __pycache__ +# under test/" would redden any developer who has just run the interpreter, and +# their tree being dirty is not this project's business. What is this project's +# business is what the repository RECORDS, and that is a question only git can +# answer. It is also the second half of the rule: ignoring the directory is +# what stops the deletion being undone by the next `git add -A`. +# +# WHY THIS FAILS RATHER THAN SKIPS OUTSIDE A CHECKOUT. A guard that can be +# silenced by deleting .git is not a guard. So every arm below answers `no-repo` +# where there is no repository, rather than the answer git gives by default: +# `ls-files` prints nothing, which is indistinguishable from a clean tree, and +# `check-ignore` says "not ignored", which reports a present rule as missing. +# Both would mislead, in opposite directions, and the second one did -- see #855. +# The premises then name the cause rather than leaving a reader to infer it. +# +# WHERE THIS SUITE ACTUALLY RUNS, surveyed rather than assumed, because the first +# version of this comment claimed "every environment is a checkout" and one was +# not: +# CI actions/checkout writes .git. ok +# run_all_versions.sh stages with `cp -a "$SRCDIR/."`, which copies .git +# (and, from a linked worktree, copies the gitfile, +# whose path is absolute and still resolves). ok +# the audit container worktrees of a clone. ok +# test/devloop.sh stages with `tar --exclude=.git`, deliberately: it +# is 32 MB and the loop is meant to be cheap. That +# produced FIVE reds on a clean tree. devloop.sh now +# writes a one-line gitfile into the build dir, which +# costs no bytes and makes the question answerable. ok +# +# SCOPE. Python only. The tree tracks Iceberg .avro/.puffin and Parquet +# fixtures, which are inputs rather than output and are meant to be there; +# .gitignore already covers the C artifacts (*.o, *.so, *.bc). Whether every +# derived file in the tree deserves one rule is a larger judgement and is +# deliberately not decided here. + +_pyc_root="$(cd "$PGC_TESTDIR/.." && pwd)" +_pyc_repo="$(git -C "$_pyc_root" rev-parse --is-inside-work-tree 2>/dev/null || echo no)" + +# Every arm goes through these two, so that "there is no repository to ask" is +# never reported as an answer about the repository. +_pyc_tracked_list() { + [ "$_pyc_repo" = true ] || { printf 'no-repo'; return; } + git -C "$_pyc_root" ls-files -- '*.pyc' '*.pyo' '*/__pycache__/*' '__pycache__/*' \ + | sort | tr '\n' ' ' +} +_pyc_ignored() { # _pyc_ignored PATH -> ignored | not-ignored | no-repo + [ "$_pyc_repo" = true ] || { echo no-repo; return; } + git -C "$_pyc_root" check-ignore -q -- "$1" && echo ignored || echo not-ignored +} + +# PREMISE. git has to be able to answer, and it has to be answering about THIS +# tree. +check_text "premise: the source tree is a git checkout" "$_pyc_repo" "true" + +# PREMISE. And it has to see a populated tree. A working `git` pointed at the +# wrong directory, or an index nobody has written, returns success and nothing. +check_text "premise: and git ls-files sees the harness it is being asked about" \ + "$(git -C "$_pyc_root" ls-files --error-unmatch test/lib.sh 2>/dev/null)" \ + "test/lib.sh" + +# PREMISE, positive control. check-ignore has to say "ignored" for something the +# tree already ignores, or the second arm below fails for the wrong reason +# before the fix and cannot fail at all after it. +check_text "premise: check-ignore agrees a build object is already ignored" \ + "$(_pyc_ignored foo.o)" "ignored" + +# PREMISE, negative control. And it has to say "not ignored" for a source file, +# or a check-ignore that answers "ignored" to everything makes the arm vacuous. +check_text "premise: and that a tracked source file is not" \ + "$(_pyc_ignored test/lib.sh)" "not-ignored" + +# ---- the rule itself -------------------------------------------------------- + +# Named, not merely counted: the failure has to say which file, because the +# next one will not be this one. +_pyc_tracked="$(_pyc_tracked_list)" +check_text "no compiled Python artifact is tracked" \ + "$([ "$_pyc_tracked" = no-repo ] && echo no-repo \ + || printf '%s tracked' "$(printf '%s' "$_pyc_tracked" | wc -w)")" \ + "0 tracked" +check_text "and the tracked list names none of them" \ + "[${_pyc_tracked}]" "[]" + +# The other half: with no ignore rule, the deletion above lasts until the next +# `git add -A` on a tree where somebody has imported a module. +# +# Two rules are needed and each probe below is chosen so that exactly one of +# them answers it. A probe named `x.cpython-312.pyc` would be ignored by either +# rule, which would leave neither provably load-bearing: +# +# test/__pycache__/x.cpython-312.pyc.140234 only `__pycache__/` catches this. +# Not a contrived name: CPython writes the compiled file atomically, to +# `.` and then renames, so a killed interpreter leaves one. +# test/x.pyc only `*.pyc` catches this, and it +# is where CPython put compiled files before PEP 3147. +check_text "and the tree ignores the directory Python writes them to" \ + "$(_pyc_ignored test/__pycache__/x.cpython-312.pyc.140234)" "ignored" + +check_text "and a compiled artifact written beside its source" \ + "$(_pyc_ignored test/x.pyc)" "ignored" + +unset _pyc_root _pyc_tracked _pyc_repo +unset -f _pyc_tracked_list _pyc_ignored