From 44c6cdcc1ea5c1c675f9432b3e53645ebd4762b8 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Mon, 31 Aug 2026 17:44:59 +0000 Subject: [PATCH 1/3] fix: no compiled Python artifact is tracked, and the tree ignores them (#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 -- 135 commits earlier -- and the .pyc did not follow, so the tree carried 6,028 bytes of 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 cec104f, a logical replication fix, where the diffstat reads "Bin 6028 -> 6028 bytes" and 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, and git recorded a binary diff in a commit about logical replication. .gitignore had no Python rule at all and now has two. The guard is a new selftest part, anchored on what git records rather than on what is in the working tree: a developer who has just run the interpreter has a dirty tree, which is not this project's business, while what the repository RECORDS is. Built test-first. On the unfixed tree the part reds 4 of 176 and names the file; fixed, 176 pass. Each check reddens alone: restoring the tracked file with both ignore rules in place still reds two, because .gitignore has no effect on a file git already tracks; dropping __pycache__/ reds one; dropping *.pyc reds the other. Pointing the part at a directory that is not a checkout reds three premises, which is what stops the two tracked-file checks passing on an empty answer. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014SNUrAGntEBQXuhcK2qeo9 --- .gitignore | 2 + CHANGELOG.md | 19 ++++ CONTEXT.md | 6 + test/__pycache__/ste_check.cpython-312.pyc | Bin 6028 -> 0 bytes .../310-a-compiled-artifact-must-not-be.sh | 103 ++++++++++++++++++ 5 files changed, 130 insertions(+) delete mode 100644 test/__pycache__/ste_check.cpython-312.pyc create mode 100644 test/selftest/310-a-compiled-artifact-must-not-be.sh 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..2fb68d3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,25 @@ 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` 135 commits earlier, 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. + - 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..2aaefe18 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -279,6 +279,12 @@ 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 its + source by 135 commits 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 cae7206441b74564bd8df144f0c621b41fc1260d..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6028 zcmbtYUu+x4ncw9u|E)+#wj@h-{AVO9t|&{C|Hn}j2Z|`yN-XDGxwRuxk(b;Nxzuu( z*yE$fl!SOqL8Fg;1*#n1)`jDs19w>JoKS?yTd)4RM`!?(1UT;hZ-mfM=o%U zpYEF_my{hfL2(OdcKFTCH{ajid^7*)_j43Hzx<1o2-Q>6|BxT{<1BX`{uw$;lt7JA z0xdWsdX%=_j!}p8W=0tYh3SOHo>+Yg4A89=sx;EO?I`^ob;T%w{bzdAEjWKbjd}zZ zv|hmtEr)9a&twNhsb>Xx)Q263eUI}XzXY$~*hht^F+W9()?y#X!QtB5bm_|sq59_Q zSPJNl#KEu5$3E2|cxBMXo*vZ8&ITlQsz}8DA9Bh%Q~-%ii|o$O-o~R_{6iv13`|v5*IZTjpOK~X3Z;c z(c`?1c$DO$aZ$!7%FCs1dXWZQyqMfBd{jxMB#{TkK{Qb5BT?f7JOLsIh>!82tm&4J zk{HFZ21HOFg~TXPj8|unGJ*J1N>y%($pju@# ziFsL+W0oRhAjPXXVW_1rj>d!N3NeXLT?@=6D>}zd@uFn8LRUaGpu+kI9M!v!MhMjv zx~G&VA4y9@P-10J4#^ngz$(Hr_$`VB4t`+ZtClQ8)Rt$aEutt}+#Yi#`0pwa3pm*v zdN~fEPIO5@IB7Wq0W&OHUE-z3xmk{fRVGA;ouEBaP|_r};xnl@%uEwQ^BRN$@h<{7 zKtm1u)DQmL{vo(W6hL1zK>|boZU{vcphP4kI!T1&(a7bCKueNvv@8uo2~R*@0-Kiv z=�Dlvh;|LIzY!D&YGQr^`;XOaXC^Brz%KR?rJri>hKu7pE|qRz)3rSy3nzS9uMi z2~|m2Gp(3~xxg}lC1n~Sl|(TS(aIg-dQd}36ERiH3*aIWyOv7eb>evlK}!SP{(yv( zk_KT)(+1@jED$HTLuRO0Q=}=}g{I>o#9xx11ieJv;z-uCBEpJF^2^e8iU&^C8iXW4 z((7nS)WnD=iTVuqexPDptqbWS2;m(EaY1)58f2s-%9Ajn#;^_q2rYawDy0SDaoZMl zLUWJ+fGU`{rVGpx<-zfwD5L^p0Z3q(r{a3cc%lx}sFIcmccPAllxdl>S@xoYB3emB z6d)hNIHYLF@+wX#Dw&AD_pz$eE$oq%h$75@BNZLOg8Qz<$8aw~I@a_aO~>O_;09AO z=={*dL3G_}gTde!H+U1`K?AHygFGd7%IPE^5LtOjR3r-p!A_8!JUB;^V2I!wVGS}u zimQ;l60z@92qOSHuoVVJ5@fBSYLL56fcsMBSU3WZYJhQ#X1w6EcLLjFZwf4DfnCN60SFNT%V%mx@<}{I zo-&c)0L+P9C#0E7EeX7mn@~i#nW>3`_q$aEv=cI(X;4W(=~TuuDMKc9D{{tf#hWz) ztdsFYFl-dSJt@jU#tWSZNsPtyXnD`F_9!>B4E#U*5wuH`UKvZ2_f6}oazplDgVHOc z1Y7A=I-fAN%hcI#;>JF~%~fGi>HHnsoPx`sZWCSL`Np-~f@k~JF>01JsD!sPdwVN* zZ2I>VA8hZkt(DmIRi3xYq!>NxoR1jJoun~bUpKQV%lsQG z$oT))$zZ{nuY)z+hWqPQRu6ssc%tE}`U;g=PNnBhZu8bowPu~+w!H-s9RZ0vhUe=f zBI6o8zsI0=w4-O6tvXn0USlEF=FfuF=n*PVf9a9;#*G`n&KICHsgG&XIU(^eE#rMH ztaXNsU}t;AedGFTH^w?|nDn(wQ}}wv_1D58a0-|<4<^Vfr|FN_h?3vi}G#U`VrBPKm!(!-`x zgDOuqolu@df0VtP0qF)`@lo(YO;u@HtJE%S=`gI+?Ho@uHvpe zTa?RtB-g)Lw`<|%+|9L?t`+`H-Lc;{>?t zv~C|CAzkbl1f)aM0uyz_sHkJC6;hPZ$!2Fg2D1}@+Z=5k0zDamV4>%c!7MuF53|%P zJIj>#zG-tlKv6$r^ctIUj+uZ>@&_K>XH!DdlYD{W{yD<&5O8z?M|vkv7Z_WPzcJ3* z;~7{AHkb;CCwALwSKk8J9%&s8-lZqeX!ok`Pu!KQ+g-KQHokkR-`YLcMk`Qg%L8D? z8cxGycqZ5pus#j{v&{6iUcj4qae$PVa9o2UB&m4pGb+?~<-?YJ{Ly-!JeQI)3mk3k z?*%Cy3no%|oP_^}FGBM!{W3*EO=D1FAUN#r^qeD64k#F|JhpqLs2|ZwG)>L7u+$9o zugo+Zpf3dgY~e70)c|8Ug2zl+RLS99BLRs}Ry_poN8Y{|mhsJ$dMwZLC9$rON-5(BkD%^u6dnoqOjZHVHCb}3u~5!r;3%&WZ<`F94^8J#Xy_Gy zYOAhSNo=a5x=?G$7j8I=3sA8VXio8Z+@t}`32JJVuEYRxY}|BOCoI!dvcBoGjtwRw zL?j!ZZ&>(j*~H|khEUJKDv(K(_8VwG?>~5axhrpR3w3jK8=k${{!L#~o`36NcCgrT z;CH^$`QtxrC|v&N>IYXpeD%YFt8E*tr+#>O&38IG2n~Glxi3UM*=Qg5^>^>I58Q1X zAj2Ut94h*27p~4-&0pK_A1v(psQH8Dm6nax6YKsH+4Fz!)h?W$JD=|>>@HmXB=R%i zW8sePWOi_q^Up^X5_5^=rwVwZwrhjy1_-aI&p*Aqr_iug)4s;Gf4SwS{Aa#2eIczG zl=zp{fll@_A4}Ti0|PzmXQx?c9}7leVB*4uZ$QxPjB8%pmiH8 zw4R^T*b>ozy8>L0S=VFT78X?yS>U$uawRGV3`&$AJN;~Kz#EET;OE;%G^n5)BxVm3 zM2^Zz4Nt7(-!^NT-t$eQ_=Ga5_L54@G#L*<$|Q+Y(mUTjbP;-vUX*cLXVxx4F5iD; z{BYsYQ@Lo#G?s2j%2y=arMr~-MAr;wg}&9meCt-K7u~-`hIUnlEWW)cz*;Gx9))3& zM{p{%ay8&qj{!*~5T>3Y4IDQp^%-bPS}p+(v0#~DDbZY|Gq-vU1}?+D_9tkdxIvvO zr#8^Bb@o7ZFn2nCWcl!>rzSV}7WdA`a?{(R>z+e}BkLZhD%L&0Y=6=1$@RZ6wdwKa zJ}jcRZ+ASHH=6a~JYg^5>V^emqiWx;46damD=$W_9rA+=}t(*`mJzhM__! zG_4)!eZbH)eSe{7=kxdcyYnaV{kd1S$gdnt7;`^9x%~S3)9+2+nz?muZC~d~$LjIb zk=4lRnbnT9+UM?i`e6POQ@gCGYxS*#*1|yHJB1^KmbKc>yPl)u)2@3pzU&1RHh0SO zjE^VbGI4zTK7sf^z3CnwhkLd0aSQFBUY*gP3{6J>wW3(lOgFimgc~xG9x~nk&_6OV z^u0k9f|s)Jv*aIQQo>l~lX!gGWFkD&wQ%t=Wzs2=#c(U}F=ee@+6Fv*NkxE5z;o&_ zOeA7x{{#(4O4DDk`)T%ndnx*<4Qk&#t})lP&^gz+)^y?D+y1NlKiW6ASF-Lchr{~{ zopo+`_S1D+eH{Jt);=HYf3TloeYwnUnY~4>27t+5zi@5tTAt5Ga$#cKy*aw*shOJs o2%TB?w9GNYG<)x{P=~&G;U6z79-h0reEcqZ;DMK7_gUipAGglbLjV8( 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..51fbdd4c --- /dev/null +++ b/test/selftest/310-a-compiled-artifact-must-not-be.sh @@ -0,0 +1,103 @@ +# 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, and the two arms below have no +# meaning without it: `git ls-files` printing nothing and `git ls-files` finding +# nothing wrong are the same output. Every environment this suite runs in is a +# checkout -- CI checks out with .git, run_all_versions.sh stages the tree with +# `cp -a "$SRCDIR/."` which copies it, and the container worktrees are clones. +# +# 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)" + +# PREMISE. git has to be able to answer, and it has to be answering about THIS +# tree. Without this the two arms below pass on an empty answer. +check_text "premise: the source tree is a git checkout" \ + "$(git -C "$_pyc_root" rev-parse --is-inside-work-tree 2>/dev/null || echo no)" \ + "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" \ + "$(git -C "$_pyc_root" check-ignore -q -- foo.o && echo ignored || echo not-ignored)" \ + "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" \ + "$(git -C "$_pyc_root" check-ignore -q -- test/lib.sh && echo ignored || echo not-ignored)" \ + "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="$(git -C "$_pyc_root" ls-files -- '*.pyc' '*.pyo' '*/__pycache__/*' '__pycache__/*' \ + | sort | tr '\n' ' ')" +check_num "no compiled Python artifact is tracked" \ + "$(printf '%s' "$_pyc_tracked" | wc -w)" "0" +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" \ + "$(git -C "$_pyc_root" check-ignore -q -- test/__pycache__/x.cpython-312.pyc.140234 \ + && echo ignored || echo not-ignored)" \ + "ignored" + +check_text "and a compiled artifact written beside its source" \ + "$(git -C "$_pyc_root" check-ignore -q -- test/x.pyc && echo ignored || echo not-ignored)" \ + "ignored" + +unset _pyc_root _pyc_tracked From 87eebfb2d05be772319cc817ff3d553fd4208d86 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Mon, 31 Aug 2026 18:20:48 +0000 Subject: [PATCH 2/3] fix: devloop stages a gitfile, so the tracked-artifact rule is answerable (#854) test/devloop.sh stages its build directory with `tar --exclude=.git` and runs the suites out of it, so the tree harness_selftest ran in was not a checkout. The new part asks git two questions about that tree, and outside a repository neither can be answered: five checks went red on a tree with nothing wrong with it. Reported by jdatcmd on #855 and reproduced here, both arms from the same tree: the checkout gives 176 PASS / 0 FAIL, the same tree through devloop.sh gave 171 PASS / 5 FAIL. My claim that "every environment this suite runs in is a checkout" was false, and devloop.sh:47 was in a grep I had already run. devloop.sh now writes a one-line gitfile into the build directory. That is 45 bytes against the 32 MB of copying the object database, so the loop stays as cheap as it was, and `rev-parse --show-toplevel` in the staged tree returns the staged tree, so check-ignore reads its own .gitignore. `--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 -- which is exactly the case in the audit container, and it resolves correctly there. Two arms remain, and the second is the one that was misleading: outside a repository `git ls-files` prints nothing, which is indistinguishable from a clean tree, and `check-ignore` answers "not ignored", which reports a rule that is present as missing. Every check in the part now answers `no-repo` instead, so no arm can pass on an empty answer or fail for a reason it does not name. Pointing the part at a non-checkout used to red three premises and silently pass two rule arms; it now reds all eight, each saying `no-repo`. Acceptance: the devloop arm that was 171/5 is 176/0. Red, green and the three rule mutations are unchanged -- 172/4 unfixed, 176/0 fixed, and restoring the tracked file with both ignore rules in place still reds exactly two. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014SNUrAGntEBQXuhcK2qeo9 --- CHANGELOG.md | 9 +++ test/devloop.sh | 23 +++++++ .../310-a-compiled-artifact-must-not-be.sh | 69 ++++++++++++------- 3 files changed, 78 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fb68d3f..2e4113ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,15 @@ true until the next version shipped. 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/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 index 51fbdd4c..9c75b34a 100644 --- a/test/selftest/310-a-compiled-artifact-must-not-be.sh +++ b/test/selftest/310-a-compiled-artifact-must-not-be.sh @@ -29,11 +29,26 @@ # 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, and the two arms below have no -# meaning without it: `git ls-files` printing nothing and `git ls-files` finding -# nothing wrong are the same output. Every environment this suite runs in is a -# checkout -- CI checks out with .git, run_all_versions.sh stages the tree with -# `cp -a "$SRCDIR/."` which copies it, and the container worktrees are clones. +# 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; @@ -42,12 +57,23 @@ # 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. Without this the two arms below pass on an empty answer. -check_text "premise: the source tree is a git checkout" \ - "$(git -C "$_pyc_root" rev-parse --is-inside-work-tree 2>/dev/null || echo no)" \ - "true" +# 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. @@ -59,23 +85,22 @@ check_text "premise: and git ls-files sees the harness it is being asked about" # 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" \ - "$(git -C "$_pyc_root" check-ignore -q -- foo.o && echo ignored || echo not-ignored)" \ - "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" \ - "$(git -C "$_pyc_root" check-ignore -q -- test/lib.sh && echo ignored || echo not-ignored)" \ - "not-ignored" + "$(_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="$(git -C "$_pyc_root" ls-files -- '*.pyc' '*.pyo' '*/__pycache__/*' '__pycache__/*' \ - | sort | tr '\n' ' ')" -check_num "no compiled Python artifact is tracked" \ - "$(printf '%s' "$_pyc_tracked" | wc -w)" "0" +_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}]" "[]" @@ -92,12 +117,10 @@ check_text "and the tracked list names none of them" \ # 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" \ - "$(git -C "$_pyc_root" check-ignore -q -- test/__pycache__/x.cpython-312.pyc.140234 \ - && echo ignored || echo not-ignored)" \ - "ignored" + "$(_pyc_ignored test/__pycache__/x.cpython-312.pyc.140234)" "ignored" check_text "and a compiled artifact written beside its source" \ - "$(git -C "$_pyc_root" check-ignore -q -- test/x.pyc && echo ignored || echo not-ignored)" \ - "ignored" + "$(_pyc_ignored test/x.pyc)" "ignored" -unset _pyc_root _pyc_tracked +unset _pyc_root _pyc_tracked _pyc_repo +unset -f _pyc_tracked_list _pyc_ignored From e46f4be3777ab7542459720418b0cae0ee893347 Mon Sep 17 00:00:00 2001 From: OffgridwithJD Date: Mon, 31 Aug 2026 19:56:40 +0000 Subject: [PATCH 3/3] docs: anchor the commit count the rebase moved, or drop it (#854) Rebasing onto fe1f3a2 moved a number in two of the three places that name it, and left the third alone. The difference is the point. test/selftest/310-... "was tracked at 0e4884c1 ... renamed in e9de048, 135 commits earlier" STILL TRUE, unchanged. It names the commit it measured at, so the rebase cannot touch it. CHANGELOG.md "renamed 135 commits earlier", unanchored. 146 at the base this now sits on. Rewritten to name both commits. CONTEXT.md "outlived its source by 135 commits", unanchored, in a document read indefinitely. The count is decoration there; the mechanism is the point, so it is gone. Nothing about the defect or the fix changed: the tracked artifact, the two-byte PEP 552 rewrite in cec104f, the ignore rules and the guard are all as they were. This is only the arithmetic that measures a distance between two commits, and the far end moved. Two other counts in the pull request body are measurements of the same tree and have moved with it: 26 tracked .py files is now 27, since #851 added test/parquet_stats.py, and compiling by hand now produces 27 .pyc across three __pycache__ directories rather than the twelve entries git status showed against 0e4884c1. The population this change removes is unmoved and re-measured: exactly one tracked build artifact at fe1f3a2, and zero after. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014SNUrAGntEBQXuhcK2qeo9 --- CHANGELOG.md | 8 +++++--- CONTEXT.md | 7 ++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e4113ba..33365057 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,9 +38,11 @@ true until the next version shipped. - 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` 135 commits earlier, 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. + `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 diff --git a/CONTEXT.md b/CONTEXT.md index 2aaefe18..1b648e9c 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -282,9 +282,10 @@ PostgreSQL 15 through 19 when given none. - **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 its - source by 135 commits and re-committed itself, two bytes of PEP 552 timestamp, - inside an unrelated logical-replication fix (#854). + 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