From 6b1b68e66b7d9ad2694237132ec1d0853e9e3422 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 5 Aug 2026 21:59:23 -0500 Subject: [PATCH 1/6] port: make the host build link against current main, and gate it Nothing in the decomp toolchain compiles or links port/, so a src/ or include/ change that shifts a symbol's SPELLING breaks the host build silently. #1049 (decl_*.h extern "C") did exactly that on 2026-08-03 and took 13 of the 14 gate binaries with it; it went unnoticed until someone tried to build. Two more landed on top of it since. Four breaks fixed, all the same family -- a HAL definition kept a spelling the src/ side stopped using: - func_02059650, NestedHeapIterator::Init, func_0206e2f8, data_0209b44c: defined at C++ linkage while decl_*.h now emits C-linkage references. SharedFilePtr::Release needed a real __thiscall method; _ZTV8Platform needed storage. - _Z14ApproachLinearRiii: shims.cpp dropped this bridge because the symbol is "exactly what a host C++ build emits for ApproachLinear(int&, int, int)". That is the ITANIUM mangling. MSVC emits ?ApproachLinear@@YAHAAHHH@Z, so the bridge was load-bearing on Windows. Restored, with the GCC/Clang case noted. - GX::LoadTexPltt: migrated to namespace-style C++ as "language-mode migration only ... nothing outside this file can shift". True for the ROM link, where the filename is the symbol either way; false here, where the name went from hand-spelled to MSVC-mangled. The identical bridge for LoadTex already existed at model_host.cpp:183. - data_020a60b0: reached at C++ linkage while the storage sits in an extern "C" block. ALIASED, not redefined -- a second definition links cleanly and then leaves LoadTexPltt writing to a different VRAM base than the budget code reads. Both of the last two shipped with a premise that is correct for the ROM link and wrong for the host. That is the systematic gap, so this adds the check that closes it: - port/tools/port_linkcheck.py -- builds the port and fails if it does not link. ~2s incremental, skips loudly without MSVC rather than reading as a pass. Passes ninja -k 0: the default -k 1 stops at the first failure, which reported 3 breaks when there were 4 and turned diagnosis into fix-one-discover-more. - port/tools/port_evidence.py -- how much of what the port compiles is proven to be the game's code, by enrollment in the byte-exact ROM build. 19 files are not; --ratchet fails only when that set grows, matching the merge gate's must-not-regress shape rather than being red on day one. build-port.cmd and host_frontier.py now locate MSVC via vswhere: the hardcoded 2022 BuildTools path no longer exists on this machine. Verified on main @ ee0c07fa: 14/14 binaries link, 14/14 gates pass (455 models rendered, 473 animation pairs, 0 faults), ratchet clean at 19. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_017tYjz5v5R228Kh7YCH4gkR --- port/build-port.cmd | 98 +++++++++++- port/evidence-baseline.json | 24 +++ port/hal/actor_vtables.cpp | 4 + port/hal/cxxname_bridge.cpp | 21 ++- port/hal/heap_globals.cpp | 25 ++- port/hal/model_host.cpp | 22 +++ port/hal/os_time.cpp | 12 +- port/hal/shims.cpp | 29 +++- port/tools/host_frontier.py | 65 +++++++- port/tools/port_evidence.py | 290 +++++++++++++++++++++++++++++++++++ port/tools/port_linkcheck.py | 147 ++++++++++++++++++ 11 files changed, 703 insertions(+), 34 deletions(-) create mode 100644 port/evidence-baseline.json create mode 100644 port/tools/port_evidence.py create mode 100644 port/tools/port_linkcheck.py diff --git a/port/build-port.cmd b/port/build-port.cmd index 0ed7646d4..1d5f96c17 100644 --- a/port/build-port.cmd +++ b/port/build-port.cmd @@ -1,12 +1,94 @@ @echo off -rem Build the PC port's gate-1 smoke runner: 32-bit MSVC via VS Build Tools, -rem same toolchain-location pattern as the recomp's build scripts. +rem Build the PC port's gate smoke runners: 32-bit MSVC via CMake + Ninja. +rem +rem The toolchain is LOCATED, not hardcoded. This script used to name +rem "...\2022\BuildTools\..." directly; when the machine moved to VS 18 the +rem path stopped existing and the failure surfaced as a confusing error from +rem cmake rather than "your compiler moved". vswhere is the supported way to +rem ask where the toolset actually is. setlocal -set "PATH=%ProgramFiles(x86)%\Microsoft Visual Studio\Installer;%PATH%" -call "%ProgramFiles(x86)%\Microsoft Visual Studio\2022\BuildTools\VC\Auxiliary\Build\vcvars32.bat" >nul + +set "VSWHERE=%ProgramFiles(x86)%\Microsoft Visual Studio\Installer\vswhere.exe" +if not exist "%VSWHERE%" ( + echo error: vswhere.exe not found at "%VSWHERE%" + echo install Visual Studio 2019+ or the VS Build Tools. + exit /b 1 +) + +rem -products * so Build Tools installs count, not just the IDE SKUs. +set "VSINSTALL=" +for /f "usebackq delims=" %%i in (`"%VSWHERE%" -latest -products * -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath`) do set "VSINSTALL=%%i" +if not defined VSINSTALL ( + echo error: no Visual Studio install carries the MSVC x86/x64 toolset. + echo install the "Desktop development with C++" workload. + exit /b 1 +) + +set "VCVARS=%VSINSTALL%\VC\Auxiliary\Build\vcvars32.bat" +if not exist "%VCVARS%" ( + echo error: vcvars32.bat missing under "%VSINSTALL%" + exit /b 1 +) +rem vcvars32.bat itself shells out to a BARE `vswhere` to resolve the Windows +rem SDK, so the Installer directory has to be on PATH before the call -- drop +rem this and vcvars prints "'vswhere.exe' is not recognized" and then leaves +rem the environment half-built, which surfaces much later as a missing cl. +for %%d in ("%VSWHERE%") do set "PATH=%%~dpd;%PATH%" +call "%VCVARS%" >nul if errorlevel 1 exit /b 1 -set "CMAKEBIN=%ProgramFiles(x86)%\Microsoft Visual Studio\2022\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake" -set "PATH=%CMAKEBIN%\CMake\bin;%CMAKEBIN%\Ninja;%PATH%" -cmake -S "%~dp0." -B "%~dp0..\build\port" -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_MAKE_PROGRAM="%CMAKEBIN%\Ninja\ninja.exe" %* + +rem CMake and Ninja: prefer the copies bundled with the "C++ CMake tools for +rem Windows" component, then anything vcvars or the user put on PATH. +set "VSCMAKE=%VSINSTALL%\Common7\IDE\CommonExtensions\Microsoft\CMake" +set "CMAKE_EXE=" +set "NINJA_EXE=" +if exist "%VSCMAKE%\CMake\bin\cmake.exe" set "CMAKE_EXE=%VSCMAKE%\CMake\bin\cmake.exe" +if exist "%VSCMAKE%\Ninja\ninja.exe" set "NINJA_EXE=%VSCMAKE%\Ninja\ninja.exe" +if not defined CMAKE_EXE call :locate cmake CMAKE_EXE +if not defined NINJA_EXE call :locate ninja NINJA_EXE + +rem Last resort: `pip install cmake ninja` is what the error below tells you +rem to do, and it drops both into Python's Scripts directory -- which is NOT +rem on PATH in a default Windows Python install, so `where` misses them. +rem Take our own advice seriously enough to find the result. +if not defined CMAKE_EXE goto :pyprobe +if not defined NINJA_EXE goto :pyprobe +goto :haveboth +:pyprobe +set "PYSCRIPTS=" +for /f "usebackq delims=" %%i in (`python -c "import sysconfig;print(sysconfig.get_path('scripts'))" 2^>nul`) do set "PYSCRIPTS=%%i" +if defined PYSCRIPTS ( + if not defined CMAKE_EXE if exist "%PYSCRIPTS%\cmake.exe" set "CMAKE_EXE=%PYSCRIPTS%\cmake.exe" + if not defined NINJA_EXE if exist "%PYSCRIPTS%\ninja.exe" set "NINJA_EXE=%PYSCRIPTS%\ninja.exe" +) +:haveboth + +if not defined CMAKE_EXE ( + echo error: cmake.exe not found. + echo add the VS component "C++ CMake tools for Windows", + echo or: pip install cmake ninja + exit /b 1 +) +if not defined NINJA_EXE ( + echo error: ninja.exe not found. + echo add the VS component "C++ CMake tools for Windows", + echo or: pip install cmake ninja + exit /b 1 +) + +echo toolchain: %VSINSTALL% +echo cmake: %CMAKE_EXE% +echo ninja: %NINJA_EXE% + +"%CMAKE_EXE%" -S "%~dp0." -B "%~dp0..\build\port" -G Ninja -DCMAKE_BUILD_TYPE=Release -DCMAKE_MAKE_PROGRAM="%NINJA_EXE%" %* if errorlevel 1 exit /b 1 -ninja -C "%~dp0..\build\port" +"%NINJA_EXE%" -C "%~dp0..\build\port" +exit /b %errorlevel% + +rem :locate -- first PATH hit, or leaves the var unset. +:locate +for /f "usebackq delims=" %%i in (`where %1 2^>nul`) do ( + set "%2=%%i" + goto :eof +) +goto :eof diff --git a/port/evidence-baseline.json b/port/evidence-baseline.json new file mode 100644 index 000000000..8d683c83a --- /dev/null +++ b/port/evidence-baseline.json @@ -0,0 +1,24 @@ +{ + "comment": "Files compiled into the port with no proof they are the game's code. This is a DEBT LEDGER, not a target -- --ratchet fails when it grows. Shrink it by matching the function, not by editing this file.", + "unproven": [ + "src/_ZN14ArrowSignRight16CleanupResourcesEv.cpp", + "src/_ZN18NestedHeapIterator7AddLastEP13HeapAllocator.cpp", + "src/_ZN18NestedHeapIterator8AddFirstEP13HeapAllocator.cpp", + "src/_ZN4Heap8AllocateEji.cpp", + "src/_ZN5Model17UpdateFileOffsetsER8BMD_File.cpp", + "src/_ZN5Model27LoadCompressedTextureToVramEPcjS0_.cpp", + "src/_ZN5Timer10StartTimerEv.cpp", + "src/_ZN5Timer7GetTimeEv.cpp", + "src/_ZN5Timer9StopTimerEv.cpp", + "src/_ZN8Platform21IsClsnInRangeOnScreenE5Fix12IiES1_.cpp", + "src/_ZN9ActorBase22BeforeCleanupResourcesEv.cpp", + "src/_ZN9ActorBasenwEj.cpp", + "src/engine/fader/_ZN15FaderBrightness10SetToStartEv.cpp", + "src/engine/fader/_ZN15FaderBrightness14SetForwardTimeEj.cpp", + "src/engine/fader/_ZN15FaderBrightness15SetBackwardTimeEj.cpp", + "src/engine/fader/_ZN15FaderBrightness20IsBetweenStartAndEndEv.cpp", + "src/engine/fader/_ZN15FaderBrightness7IsAtEndEv.cpp", + "src/engine/fader/_ZN15FaderBrightness8SetToEndEv.cpp", + "src/engine/fader/_ZN15FaderBrightness9IsAtStartEv.cpp" + ] +} diff --git a/port/hal/actor_vtables.cpp b/port/hal/actor_vtables.cpp index 91fbddd66..64e421e34 100644 --- a/port/hal/actor_vtables.cpp +++ b/port/hal/actor_vtables.cpp @@ -171,7 +171,11 @@ short *data_0209b460; /* spawn default position ptr */ signed char data_0209b44c_c; int data_0209b468[4]; /* actor list head the ctor links into */ } +// Both spellings, same storage -- Actor's ctor reaches it by the plain C name +// and other TUs by the MSVC-mangled one. Aliasing only one direction forks the +// storage silently, which is the exact failure this file exists to prevent. #pragma comment(linker, "/alternatename:?data_0209b44c@@3CA=_data_0209b44c_c") +#pragma comment(linker, "/alternatename:_data_0209b44c=_data_0209b44c_c") extern "C" { unsigned char data_0209f2d8_c; /* mega-char state byte: none */ diff --git a/port/hal/cxxname_bridge.cpp b/port/hal/cxxname_bridge.cpp index 91c90f569..ff1c9eb5e 100644 --- a/port/hal/cxxname_bridge.cpp +++ b/port/hal/cxxname_bridge.cpp @@ -81,11 +81,30 @@ void *data_020a0eac_c; } #pragma comment(linker, "/alternatename:?data_020a0eac@@3PAUHeap@@A=_data_020a0eac_c") #pragma comment(linker, "/alternatename:_data_020a0eac=_data_020a0eac_c") -void func_0206e2f8(void *p, int v, unsigned n) +// ActorBase::operator new references the plain C name (decl_*.h extern "C" +// guard); nothing currently wants the mangled spelling, but the alias costs +// nothing and both forms are __cdecl free functions, so aliasing is safe +// here in a way it is NOT for the __thiscall method bridges above. +extern "C" void func_0206e2f8(void *p, int v, unsigned n) { unsigned char *b = (unsigned char *)p; for (unsigned i = 0; i < n; ++i) b[i] = (unsigned char)v; } +#pragma comment(linker, "/alternatename:?func_0206e2f8@@YAXPAXHI@Z=_func_0206e2f8") + +// Platform's ROM ctor (src/_ZN8PlatformC2Ev.c) installs its vtable pointer. +// Platform.h carries no virtuals, so MSVC emits no vtable object -- storage +// only, exactly like the transient base vtables in actor_vtables.cpp. The +// gate never dispatches through it; if that changes it needs real slots. +extern "C" { +void *_ZTV8Platform[20]; +} + +// ArrowSignRight::CleanupResources calls Release as a real __thiscall METHOD +// (SharedFilePtr.h declares the class), while the implementation is the +// C-named free function in src/. Convert the convention rather than alias it. +#include "SharedFilePtr.h" +void SharedFilePtr::Release() { hal_fileptr_release(this); } extern "C" void hal_m43_roty(void *m, int a); void Matrix4x3_FromRotationY(void *m, int a) { hal_m43_roty(m, a); } diff --git a/port/hal/heap_globals.cpp b/port/hal/heap_globals.cpp index 44a11a608..22fe66e45 100644 --- a/port/hal/heap_globals.cpp +++ b/port/hal/heap_globals.cpp @@ -35,6 +35,15 @@ void MultiStore_Int(int val, int *dst, int len) #pragma comment(linker, "/alternatename:_data_020a4d38=__ZN6Memory16rootHeapIteratorE") #pragma comment(linker, "/alternatename:?_ZN6Memory16rootHeapIteratorE@@3DA=__ZN6Memory16rootHeapIteratorE") #pragma comment(linker, "/alternatename:?_ZN6Memory25isRootHeapIterInitializedE@@3HA=__ZN6Memory25isRootHeapIterInitializedE") +// HeapAllocator's ctor reaches the same two globals by their data_-address +// names at C++ LINKAGE (?data_020a4d38@@3DA / ?data_020a4d34@@3HA), which the +// C-spelling alias above does not cover. This surfaced when those declarations +// moved into a decl_*.h. data_020a4d34 is the four bytes below the iterator -- +// isRootHeapIterInitialized (symbols.txt: both bss, 0x020a4d34 / 0x020a4d38). +// Missing either spelling forks the storage exactly as this file's header warns. +#pragma comment(linker, "/alternatename:?data_020a4d38@@3DA=__ZN6Memory16rootHeapIteratorE") +#pragma comment(linker, "/alternatename:?data_020a4d34@@3HA=__ZN6Memory25isRootHeapIterInitializedE") +#pragma comment(linker, "/alternatename:_data_020a4d34=__ZN6Memory25isRootHeapIterInitializedE") // FUNCTION alias only where the conventions MATCH: this reference and the C // definition are both __cdecl free functions. #pragma comment(linker, "/alternatename:?_ZN18NestedHeapIteratorC1Ej@@YAXPAXI@Z=__ZN18NestedHeapIteratorC1Ej") @@ -53,9 +62,15 @@ void _ZN18NestedHeapIterator8AddFirstEP13HeapAllocator(void *self, HeapAllocator int _ZN18NestedHeapIterator4NextEP13HeapAllocator(void *self, HeapAllocator *a) { return ((NestedHeapIterator *)self)->Next(a); } } -// And the reverse direction: AddLast/AddFirst reference Init as a C++ -// __cdecl FREE function (?_ZN..4Init..@@YAXPAD0@Z, char* args) while -// Init.cpp defines the method. C++ linkage on purpose -- extern "C" would -// decorate this wrong. -void _ZN18NestedHeapIterator4InitEP13HeapAllocator(char *self, char *a) +// And the reverse direction: AddLast/AddFirst reference Init as a __cdecl +// FREE function with char* args while Init.cpp defines the method. +// +// This forwarder was C++-linkage "on purpose" until include/decl_*.h grew its +// extern "C" guard. That guard is now the authority (see the rationale in +// decl_NestedHeapIterator.h: an unguarded C++ declaration emits _Z3Fooi, +// which exists nowhere), so every TU including the decl header emits a +// C-linkage reference and the C++-mangled definition resolves nothing. The +// convention still has to be converted by hand -- the caller passes char*, +// the method is __thiscall -- so this stays a real forwarder, not an alias. +extern "C" void _ZN18NestedHeapIterator4InitEP13HeapAllocator(char *self, char *a) { ((NestedHeapIterator *)self)->Init((HeapAllocator *)a); } diff --git a/port/hal/model_host.cpp b/port/hal/model_host.cpp index 35b357e27..4932fade6 100644 --- a/port/hal/model_host.cpp +++ b/port/hal/model_host.cpp @@ -189,6 +189,28 @@ extern "C" void _ZN2GX7LoadTexEPKvjj(const void *s, unsigned o, unsigned z) GX::LoadTex(s, o, z); } +// Same migration, one function later. src/_ZN2GX11LoadTexPlttEPKvjj.cpp moved +// to namespace-style C++ ("language-mode migration only ... nothing outside +// this file can shift"). That holds for the ROM link, where the symbol is the +// filename either way. It does not hold here: the name stopped being spelled +// by hand and started being mangled by MSVC, so gx_upload_bridge.cpp's +// extern "C" _ZN2GX11LoadTexPlttEPKvjj lost its definition and nine gate +// binaries failed to link. Bridge it exactly as LoadTex above. +namespace GX { +void LoadTexPltt(const void *src, unsigned addr, unsigned size); +} +extern "C" void _ZN2GX11LoadTexPlttEPKvjj(const void *s, unsigned a, unsigned z) +{ + GX::LoadTexPltt(s, a, z); +} + +// The migrated TU also reaches its destination-base global as C++-linkage +// ?data_020a60b0@@3IA, while the storage above is inside this file's +// extern "C" block. ALIAS, never a second definition -- two definitions would +// link cleanly and then leave LoadTexPltt writing to a different base than +// the one the Model budget code reads. +#pragma comment(linker, "/alternatename:?data_020a60b0@@3IA=_data_020a60b0") + // DMA fallback DMASyncWordTransfer uses; same synchronous copy semantics. extern "C" void DMAStartTransfer(int ch, int src, int dst, int ctrl); extern "C" void DMAStartTransferFB(unsigned char ch, u32 src, u32 dst, u32 ctrl) diff --git a/port/hal/os_time.cpp b/port/hal/os_time.cpp index 2a4e73f77..b22d6a9ac 100644 --- a/port/hal/os_time.cpp +++ b/port/hal/os_time.cpp @@ -2,9 +2,13 @@ // // On the DS this reads the OS tick counter (timer hardware + IRQ-maintained // high bits). Timer::Start/Stop/GetTime take differences of it, so any -// monotonic s64 with a stable rate is semantically faithful. The Timer TUs -// declare it with C++ linkage, which is why this is a .cpp -- a C definition -// does not mangle to what they reference. +// monotonic s64 with a stable rate is semantically faithful. +// +// LINKAGE: this used to be a C++-linkage definition because the Timer TUs +// referenced the mangled spelling. They no longer do -- they include +// decl_common.h, which declares `extern s64 func_02059650(void)` inside its +// extern "C" guard, so the reference is the plain C name. The file stays .cpp +// for the host-side statics; only the linkage of the entry point changed. // // GATE 1: the tick is a manually-advanced counter so the smoke tests are // deterministic -- sm64ds_hal_advance_ticks() stands in for time passing. @@ -14,7 +18,7 @@ typedef long long s64; static s64 g_ticks; -s64 func_02059650() +extern "C" s64 func_02059650() { return g_ticks; } diff --git a/port/hal/shims.cpp b/port/hal/shims.cpp index 41195a5d1..0d4156e7c 100644 --- a/port/hal/shims.cpp +++ b/port/hal/shims.cpp @@ -29,11 +29,24 @@ int Fader::IsAtEnd() { return 0; } // hardware upload. void FaderBrightness::AdvanceFade() { AdvanceInterp(); } -// The func_0203ae58 bridge that used to live here is gone, on the terms its own -// comment set out: it existed because Fader::AdvanceInterp called the 20.12 -// approach helper by its historical address-shaped name, which the NDS build -// resolves by address and the host cannot. That extern has now been modernised -// to _Z14ApproachLinearRiii -- the real ROM symbol at 0x0203ae58, defined by -// src/_Z14ApproachLinearRiii.cpp -- which is exactly what a host C++ build emits -// for ApproachLinear(int&, int, int). The name now resolves on both sides -// without help, so bridging it would be a duplicate definition. +// The func_0203ae58 bridge was deleted here on the reasoning that +// _Z14ApproachLinearRiii is "exactly what a host C++ build emits for +// ApproachLinear(int&, int, int)", making a bridge a duplicate definition. +// That is true of an ITANIUM-ABI host compiler. It is not true of MSVC, which +// emits ?ApproachLinear@@YAHAAHHH@Z -- _Z... is the GCC/Clang spelling, not a +// universal one. On top of that, decl_common.h now declares the symbol inside +// its extern "C" guard, so Fader::AdvanceInterp emits a reference to the +// C-decorated __Z14ApproachLinearRiii, which nothing on this host defines. +// smoke.exe failed to link on exactly that. +// +// So the bridge returns, one layer lower than before: define the Itanium-shaped +// name at C linkage and forward to the real C++ definition in +// src/_Z14ApproachLinearRiii.cpp. Under MSVC those are two distinct symbols, +// so this is not a duplicate definition. On a GCC/Clang host it WOULD be -- +// guard it by toolchain if the port ever grows a second host compiler. +int ApproachLinear(int &ref, int target, int step); + +extern "C" int _Z14ApproachLinearRiii(int &ref, int target, int step) +{ + return ApproachLinear(ref, target, step); +} diff --git a/port/tools/host_frontier.py b/port/tools/host_frontier.py index 9f43d7b97..72e6384eb 100644 --- a/port/tools/host_frontier.py +++ b/port/tools/host_frontier.py @@ -16,8 +16,8 @@ python port/tools/host_frontier.py --detail python port/tools/host_frontier.py --jobs 8 --batch 40 -Requires the VS Build Tools 32-bit environment (run from build-port.cmd's -shell, or let the script locate vcvars itself, which it does by default). +Requires an MSVC 32-bit environment. The toolchain is located via vswhere, +same as build-port.cmd; set SM64DS_VCVARS to point at a specific vcvars32.bat. """ import argparse import os @@ -34,8 +34,40 @@ INCLUDE = REPO / "include" PORT = REPO / "port" -VCVARS = (r"C:\Program Files (x86)\Microsoft Visual Studio\2022\BuildTools" - r"\VC\Auxiliary\Build\vcvars32.bat") +def find_vcvars(): + """Locate vcvars32.bat, asking vswhere rather than hardcoding a path. + + This was a literal "...\\2022\\BuildTools\\..." string until the machine + moved to VS 18: the install vanished, vcvars silently did nothing, and the + failure surfaced two steps later as "cl not on the captured PATH" -- which + reads like a toolchain bug instead of a missing compiler. Set + SM64DS_VCVARS to override. + """ + override = os.environ.get("SM64DS_VCVARS") + if override: + if not Path(override).exists(): + sys.exit(f"SM64DS_VCVARS points at a missing file: {override}") + return override + + vswhere = (Path(os.environ.get("ProgramFiles(x86)", r"C:\Program Files (x86)")) + / "Microsoft Visual Studio" / "Installer" / "vswhere.exe") + if not vswhere.exists(): + sys.exit(f"vswhere.exe not found at {vswhere} -- " + "install Visual Studio 2019+ or the VS Build Tools") + # -products * so Build Tools installs count, not just the IDE SKUs. + out = subprocess.run( + [str(vswhere), "-latest", "-products", "*", + "-requires", "Microsoft.VisualStudio.Component.VC.Tools.x86.x64", + "-property", "installationPath"], + capture_output=True, text=True, errors="replace") + roots = [ln.strip() for ln in out.stdout.splitlines() if ln.strip()] + if not roots: + sys.exit("no Visual Studio install carries the MSVC x86/x64 toolset -- " + 'install the "Desktop development with C++" workload') + vcvars = Path(roots[0]) / "VC" / "Auxiliary" / "Build" / "vcvars32.bat" + if not vcvars.exists(): + sys.exit(f"vcvars32.bat missing under {roots[0]}") + return str(vcvars) # First-error attribution: MSVC error code -> human bucket. Anything not # listed reports as its raw code so new classes surface instead of hiding @@ -64,16 +96,33 @@ def vc_env(): reports 'cannot find the path specified' with empty output, which then surfaces later as FileNotFoundError on 'cl' -- a confusing distance from the actual failure).""" + vcvars = find_vcvars() + # vcvars32.bat shells out to a BARE `vswhere`; without the Installer + # directory on PATH it prints "'vswhere.exe' is not recognized" into the + # output we are about to parse for KEY=VALUE lines. It recovers and still + # builds a usable environment, but the stray error is pure noise on a + # capture whose failure mode is already hard to read. Same prepend + # build-port.cmd does, for the same reason. + parent = dict(os.environ) + installer = str(Path(vcvars).parents[3] / "Installer") + for probe in (installer, + str(Path(os.environ.get("ProgramFiles(x86)", + r"C:\Program Files (x86)")) + / "Microsoft Visual Studio" / "Installer")): + if Path(probe, "vswhere.exe").exists(): + parent["PATH"] = probe + os.pathsep + parent.get("PATH", "") + break out = subprocess.run( - ["cmd", "/c", "call", VCVARS, ">nul", "&&", "set"], - capture_output=True, text=True, errors="replace") - env = dict(os.environ) + ["cmd", "/c", "call", vcvars, ">nul", "&&", "set"], + capture_output=True, text=True, errors="replace", env=parent) + env = dict(parent) for line in out.stdout.splitlines(): if "=" in line: k, _, v = line.partition("=") env[k] = v if not any("Hostx86" in p or "HostX86" in p for p in env.get("PATH", "").split(";")): - sys.exit("vcvars32 capture failed: cl not on the captured PATH") + sys.exit(f"vcvars32 capture failed (cl not on the captured PATH)\n" + f" vcvars: {vcvars}") # CreateProcess resolves the executable against the PARENT's PATH, not # the child env being passed -- so find cl.exe now and invoke it by # absolute path, or every spawn dies FileNotFoundError despite a correct diff --git a/port/tools/port_evidence.py b/port/tools/port_evidence.py new file mode 100644 index 000000000..bd78c55c2 --- /dev/null +++ b/port/tools/port_evidence.py @@ -0,0 +1,290 @@ +#!/usr/bin/env python3 +"""How much of what the PC port compiles is PROVEN to be the game's code? + +A gate smoke proves the host BEHAVES: 455 models render, an actor lives its +lifecycle. It does not prove the code producing that behaviour is the original +logic. A src/ file existing is not evidence; neither is a smoke passing. The +strongest evidence available is enrollment in a byte-exact ROM build -- if the +ROM comes out bit-identical with that object compiled from our source, that +source IS the game's logic. + +This joins the port's slice manifests against the ROM build and reports, per +gate, how much is proven and what the rest is. + +WHAT "PROVEN" DOES AND DOES NOT MEAN + It means: this SOURCE, compiled by mwccarm for ARM, reproduces the retail + bytes. That is a statement about the source's logic. + It does NOT mean: the host build of it behaves identically. The port + compiles the same text with MSVC for x86-32 -- different codegen, + different ABI, host-supplied seams. Proven source is a necessary + condition for a faithful port, not a sufficient one. + +Buckets: + proven enrolled in the byte-exact ROM build + explained excluded for a structural reason the port expects to own + (unresolvable extern -> HAL supplies it, extra .data/.bss + sections -> HAL storage, lives in .init) + UNPROVEN cleared eligibility and still did not make the ROM -- no + reason recorded, no proof it is the game's code + BANNER carries a NONMATCHING marker in the source itself + replaced port/unmatched/ -- a deliberate logically-correct-but- + unmatched implementation, not a claim of fidelity + unknown not in the eligibility list at all + +Usage: + python port/tools/port_evidence.py # per-gate summary + python port/tools/port_evidence.py --gate 9 # one gate, with files + python port/tools/port_evidence.py --list # every non-proven file + python port/tools/port_evidence.py --strict # exit 1 if UNPROVEN/BANNER +""" +import argparse +import json +import re +import sys +from pathlib import Path + +REPO = Path(__file__).resolve().parents[2] +PORT = REPO / "port" +SRC = REPO / "src" +BUILD = REPO / "build" +BASELINE = PORT / "evidence-baseline.json" + +PROVEN, EXPLAINED, UNPROVEN, BANNER, REPLACED, UNKNOWN = ( + "proven", "explained", "UNPROVEN", "BANNER", "replaced", "unknown") +ORDER = [PROVEN, EXPLAINED, UNPROVEN, BANNER, REPLACED, UNKNOWN] + +# The gate ledger from port/README.md, so `--gate 4b` works the way the docs +# and commit messages talk about the port. +GATE_ALIAS = { + "1": "smoke", "2": "smoke_heap", "3a": "smoke_roots", + "3b": "smoke_fs", "4a": "smoke_gx", "4b": "smoke_model", + "4c": "smoke_anim", "4d": "smoke_soak", "5": "smoke_frames", + "5b": "smoke_soak_anim", "6": "smoke_oam", "7": "smoke_modelanim", + "8": "smoke_clsn", "9": "smoke_actor", +} + +# Exclusion reasons the port legitimately owns: the HAL supplies the symbol or +# the storage on host, so absence from the ROM link says nothing bad about the +# source. Anything NOT matching these is not silently forgiven. +EXPLAINED_RE = re.compile( + r"^(unresolvable:|extra sections:|lives in \.init)", re.I) + + +def load_rom_evidence(): + """(enrolled stems, {relpath: reason}, report) -- or exit with why not.""" + objs = BUILD / "objects.txt" + elig = BUILD / "rombuild-eligibility.json" + rep = BUILD / "rombuild-report.json" + for p in (objs, elig, rep): + if not p.exists(): + sys.exit(f"missing {p.relative_to(REPO)} -- run the ROM build first " + "(the port's evidence comes from it)") + + report = json.loads(rep.read_text(encoding="utf-8", errors="replace")) + # Enrollment only means something if the build actually came out exact. + an = report.get("analysis", {}) + fid = an.get("moduleFidelity", {}) + if not (report.get("status") == "passed" and an.get("passed") + and fid.get("differingBytes") == 0): + sys.exit("the ROM build on record is NOT byte-exact " + f"(status={report.get('status')}, " + f"differingBytes={fid.get('differingBytes')}) -- " + "enrollment proves nothing until it is") + + enrolled = set() + for line in objs.read_text(encoding="utf-8", errors="replace").splitlines(): + m = re.search(r"/build/src/(.+)\.o$", line.strip().replace("\\", "/")) + if m: + enrolled.add(m.group(1)) + + reasons = {} + for row in json.loads(elig.read_text(encoding="utf-8", errors="replace")): + reasons[row["file"].replace("\\", "/")] = row.get("reason") + return enrolled, reasons, report + + +def gates(): + """{gate label: [repo-relative source paths]} from the CMake targets. + + Reads the real build description rather than a hand-kept list, so a gate + that grows its slice is reflected here without anyone remembering to. + """ + cml = (PORT / "CMakeLists.txt").read_text(encoding="utf-8", errors="replace") + + slices = {} # SLICE2_SOURCES -> [paths] + for f in sorted(PORT.glob("slice_gate*.txt")): + n = f.stem.replace("slice_gate", "").upper() + got = [] + for line in f.read_text(encoding="utf-8", errors="replace").splitlines(): + line = line.strip().replace("\\", "/") + if line and not line.startswith("#"): + got.append(line) + slices[f"SLICE{n}_SOURCES"] = got + # Gate 1 predates the numbered convention and is plain SLICE_SOURCES. + # Without this alias the gate-1 target silently reports zero files and + # drops out of the table entirely -- the quietest way for an evidence + # tool to lie. + if n == "1": + slices["SLICE_SOURCES"] = got + + hostgen = {} # GATE4A_GEN -> [paths] + for m in re.finditer(r"set\((GATE\w*)_SYMS\s+(.*?)\)\s*\n", cml, re.S): + got = [] + for s in m.group(2).split(): + for ext in (".c", ".cpp"): + if (SRC / f"{s}{ext}").exists(): + got.append(f"src/{s}{ext}") + break + hostgen[m.group(1) + "_GEN"] = got + + out = {} + for m in re.finditer(r"add_executable\(\s*(\w+)(.*?)\)\s*\n", cml, re.S): + target, body = m.group(1), m.group(2) + files = [] + for var in re.findall(r"\$\{(\w+)\}", body): + files += slices.get(var, hostgen.get(var, [])) + if files: + out[target] = sorted(set(files)) + return out + + +def classify(rel, enrolled, reasons): + if rel.startswith("port/unmatched/"): + return REPLACED + p = REPO / rel + if not p.exists(): + return UNKNOWN + if "NONMATCHING" in p.read_text(encoding="utf-8", errors="replace"): + return BANNER + if p.stem in enrolled: + return PROVEN + if rel not in reasons: + return UNKNOWN + reason = reasons[rel] + if reason is None: + return UNPROVEN + return EXPLAINED if EXPLAINED_RE.match(str(reason)) else UNPROVEN + + +def main(): + ap = argparse.ArgumentParser() + ap.add_argument("--gate", help="show one target (e.g. 9, smoke_actor)") + ap.add_argument("--list", action="store_true", help="list every non-proven file") + ap.add_argument("--strict", action="store_true", + help="exit 1 if any UNPROVEN or BANNER file is compiled") + ap.add_argument("--ratchet", action="store_true", + help="exit 1 only if the unproven set GREW vs the baseline") + ap.add_argument("--update-baseline", action="store_true", + help="rewrite the baseline to the current set") + args = ap.parse_args() + + enrolled, reasons, report = load_rom_evidence() + sha = report.get("romArtifact", {}).get("sha256", "?") + print(f"evidence: byte-exact ROM build, {report['enrolledFiles']:,} files " + f"enrolled, sha256 {sha[:16]}...") + print(f" {report['analysis']['moduleFidelity']['modulesExact']}" + f"/{report['analysis']['moduleFidelity']['modulesChecked']} modules " + f"exact, 0 differing bytes\n") + + g = gates() + if args.gate: + key = args.gate.lower() + want = args.gate if args.gate in g else GATE_ALIAS.get(key) + if want is None: + cand = [k for k in g if key in k.lower()] + if len(cand) != 1: + sys.exit(f"--gate {args.gate!r} matched {cand or 'nothing'}\n" + f" gates: {', '.join(sorted(GATE_ALIAS))}\n" + f" targets: {', '.join(sorted(g))}") + want = cand[0] + if want not in g: + sys.exit(f"target {want!r} is not built by port/CMakeLists.txt") + g = {want: g[want]} + + rows = [] + everything = {} + cache = {} + for target, files in sorted(g.items()): + counts = dict.fromkeys(ORDER, 0) + for rel in files: + b = cache.get(rel) or cache.setdefault( + rel, classify(rel, enrolled, reasons)) + counts[b] += 1 + everything.setdefault(b, set()).add(rel) + n = len(files) + pct = 100.0 * counts[PROVEN] / n if n else 0.0 + rows.append((target, n, pct, counts)) + # UNIQUE files, not the sum down the column: most sources appear in many + # targets, so summing would report a number several times the real one. + worst = len(everything.get(UNPROVEN, ())) + len(everything.get(BANNER, ())) + + w = max(len(r[0]) for r in rows) + print(f"{'target':<{w}} {'files':>5} {'proven':>7} " + f"{'expl':>4} {'UNPROV':>6} {'BANNER':>6} {'repl':>4} {'unk':>4}") + for target, n, pct, c in rows: + print(f"{target:<{w}} {n:>5} {pct:>6.1f}% {c[EXPLAINED]:>4} " + f"{c[UNPROVEN]:>6} {c[BANNER]:>6} {c[REPLACED]:>4} {c[UNKNOWN]:>4}") + + if args.gate or args.list: + for b in (UNPROVEN, BANNER, UNKNOWN, EXPLAINED, REPLACED): + items = sorted(everything.get(b, ())) + if not items: + continue + print(f"\n--- {b} ({len(items)}) ---") + for rel in items: + note = reasons.get(rel) + print(f" {rel}" + (f"\n reason: {note}" if note else "")) + + if worst: + print(f"\n{worst} file(s) compiled into the port are UNPROVEN or " + f"NONMATCHING-bannered.") + print("A passing smoke does not cover this: it proves host behaviour, " + "not that the code is the game's.") + + current = sorted(everything.get(UNPROVEN, set()) | everything.get(BANNER, set())) + if args.update_baseline: + BASELINE.write_text(json.dumps({ + "comment": "Files compiled into the port with no proof they are the " + "game's code. This is a DEBT LEDGER, not a target -- " + "--ratchet fails when it grows. Shrink it by matching the " + "function, not by editing this file.", + "unproven": current, + }, indent=2) + "\n", encoding="utf-8") + print(f"\nbaseline updated: {len(current)} file(s) -> " + f"{BASELINE.relative_to(REPO)}") + return 0 + + if args.ratchet: + # A boolean gate is the wrong shape here: there are already unproven + # files, so --strict would be red from day one and get ignored or + # bypassed. The repo's own merge gate is a set of "must not regress" + # ratchets (notes/pr-validation.md); this matches that. + if not BASELINE.exists(): + print(f"\nno baseline at {BASELINE.relative_to(REPO)} -- " + "run --update-baseline to record the current debt") + return 1 + known = set(json.loads(BASELINE.read_text(encoding="utf-8"))["unproven"]) + added = [f for f in current if f not in known] + removed = sorted(known - set(current)) + if removed: + print(f"\n{len(removed)} file(s) left the unproven set -- " + f"run --update-baseline to bank it:") + for f in removed: + print(f" - {f}") + if added: + print(f"\nREGRESSION: {len(added)} file(s) newly compiled into the " + f"port without proof they are the game's code:") + for f in added: + print(f" + {f}") + return 1 + print(f"\nratchet OK: no new unproven files " + f"({len(current)} known, baseline {len(known)})") + return 0 + + if args.strict and worst: + return 1 + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/port/tools/port_linkcheck.py b/port/tools/port_linkcheck.py new file mode 100644 index 000000000..09379ed3f --- /dev/null +++ b/port/tools/port_linkcheck.py @@ -0,0 +1,147 @@ +#!/usr/bin/env python3 +"""Does the PC port still COMPILE AND LINK against the current src/ and include/? + +This exists because of a real two-day outage. #1049 ("Give the shared +declaration headers C linkage") was correct for the ROM link and broke 13 of +the port's 14 gate binaries: four HAL definitions were deliberately +C++-linkage, the decl headers started emitting C-linkage references, and the +two no longer met. Nothing noticed, because nothing in the decomp toolchain +builds port/. + +What the existing gates do NOT cover: + tools/port_refcheck.py catches a src/ RENAME stranding a port/ reference. + #1049 renamed nothing -- every reference was valid, + the LINKAGE changed. + port/tools/port_evidence.py + asks whether the code is proven to be the game's. + Orthogonal: unproven code links fine, and proven + code can fail to link. +Only linking catches a linkage break. So: link. + +Cost is small because it is incremental -- ninja rebuilds only what the change +touched. The default target set is the two cheapest binaries, which is enough: +#1049 broke both. + +Usage: + python port/tools/port_linkcheck.py # smoke + smoke_heap + python port/tools/port_linkcheck.py --all # every gate binary + python port/tools/port_linkcheck.py --targets smoke_actor +""" +import argparse +import os +import subprocess +import sys +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from host_frontier import find_vcvars # noqa: E402 (same vswhere lookup) + +REPO = Path(__file__).resolve().parents[2] +PORT = REPO / "port" +BUILD = REPO / "build" / "port" + +# The cheapest pair that still spans the HAL seams #1049 broke: gate 1 pulls +# os_time/shims, gate 2 pulls the heap bridges. Both failed in that outage. +DEFAULT_TARGETS = ["smoke", "smoke_heap"] + + +def msvc_env(): + """Environment with the 32-bit MSVC toolchain, or None if unavailable. + + Returns None rather than exiting: a contributor without VS should be told + the check was SKIPPED, never allowed to read silence as a pass. + """ + try: + vcvars = find_vcvars() + except SystemExit: + return None + parent = dict(os.environ) + installer = str(Path(vcvars).parents[3] / "Installer") + if Path(installer, "vswhere.exe").exists(): + parent["PATH"] = installer + os.pathsep + parent.get("PATH", "") + out = subprocess.run(["cmd", "/c", "call", vcvars, ">nul", "&&", "set"], + capture_output=True, text=True, errors="replace", + env=parent) + env = dict(parent) + for line in out.stdout.splitlines(): + if "=" in line: + k, _, v = line.partition("=") + env[k] = v + # LIB is what makes this a LINK check rather than a compile check -- without + # it link.exe dies on kernel32.lib and every target "fails" for a reason + # that has nothing to do with the code. + if not env.get("LIB") or not any( + "Hostx86" in p or "HostX86" in p for p in env.get("PATH", "").split(";")): + return None + return env + + +def tool(env, name): + for d in env.get("PATH", "").split(os.pathsep): + p = Path(d) / f"{name}.exe" + if p.exists(): + return str(p) + # pip install cmake ninja -- Python's Scripts dir is usually not on PATH. + import sysconfig + p = Path(sysconfig.get_path("scripts")) / f"{name}.exe" + return str(p) if p.exists() else None + + +def main(): + ap = argparse.ArgumentParser() + ap.add_argument("--all", action="store_true", help="build every gate binary") + ap.add_argument("--targets", nargs="*", help="explicit ninja targets") + args = ap.parse_args() + + env = msvc_env() + if env is None: + print("port-linkcheck: SKIPPED -- no 32-bit MSVC toolchain found.") + print(" This did NOT pass; it did not run.") + return 0 + + cmake, ninja = tool(env, "cmake"), tool(env, "ninja") + if not cmake or not ninja: + print("port-linkcheck: SKIPPED -- cmake/ninja not found " + "(VS component 'C++ CMake tools for Windows', or " + "pip install cmake ninja).") + print(" This did NOT pass; it did not run.") + return 0 + + if not (BUILD / "build.ninja").exists(): + print(f"port-linkcheck: configuring {BUILD.relative_to(REPO)}") + r = subprocess.run([cmake, "-S", str(PORT), "-B", str(BUILD), "-G", "Ninja", + "-DCMAKE_BUILD_TYPE=Release", + f"-DCMAKE_MAKE_PROGRAM={ninja}"], + env=env, capture_output=True, text=True, errors="replace") + if r.returncode: + print(r.stdout[-3000:] + r.stderr[-3000:]) + print("port-linkcheck: FAILED to configure") + return 1 + + targets = args.targets or ([] if args.all else DEFAULT_TARGETS) + # -k 0 = keep going after failures. Ninja's default (-k 1) stops at the + # first one, so a run reports whichever failures happened to be in flight + # and hides the rest. That turns diagnosis into fix-one-discover-more: + # three linkage breaks looked like the whole story, and fixing them + # revealed nine more behind them. A link check should enumerate the damage + # in one pass. + r = subprocess.run([ninja, "-C", str(BUILD), "-k", "0"] + targets, + env=env, capture_output=True, text=True, errors="replace") + if r.returncode: + out = r.stdout + r.stderr + # Surface the diagnosis, not the 4KB link command line that carries it. + keep = [ln for ln in out.splitlines() + if ("error" in ln.lower() or "FAILED" in ln) + and "cmd.exe /C" not in ln and "link.exe /nologo" not in ln] + print("\n".join(keep[:40]) or out[-3000:]) + print(f"\nport-linkcheck: FAILED -- the port does not build against " + f"the current src/ and include/.") + return 1 + + built = ", ".join(targets) if targets else "all gate binaries" + print(f"port-linkcheck: OK ({built})") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From ab100764b205899900401c5e9f8835a7e6765c8a Mon Sep 17 00:00:00 2001 From: = Date: Wed, 5 Aug 2026 23:03:29 -0500 Subject: [PATCH 2/6] port: make the link gate report all the damage, and fix the evidence join Two defects in the tools the previous commit added, both found by running them against a real break rather than reading them. port_linkcheck.py capped its diagnosis at keep[:40] with no marker that anything was dropped. On the actual failure that is 40 lines of 114, and the truncated report reads as "6 of 14 binaries broke" when 13 did -- the same fix-one-discover-more the -k 0 change was made to end, reintroduced four lines below the comment explaining why it must not be. Group instead: distinct unresolved symbols and failing binaries are both small once deduplicated (11 and 13 here) and are never capped; only the open-ended tail is, and it prints how much it dropped and how to get the rest. A totals footer keeps a capped run honest. Raw line-dedupe would not have worked -- each target compiles its own objects under CMakeFiles/.dir/, so identical failures carry different path prefixes. The symbol capture anchors on " referenced in function" rather than whitespace. MSVC prints C++ linkage as "char data_020a4d38" (?data_020a4d38@@3DA), so a \S+ capture keys every such symbol on the token "char and merges distinct ones -- undercounting in exactly the way this report exists to prevent, in a tool written for a C-vs-C++ linkage outage where both spellings are guaranteed to appear. port_evidence.py joined enrolled objects on Path(rel).stem, but the enrolled keys keep their subdirectory (engine/fader/_ZN15FaderBrightness...), and a stem never contains a slash. No file under a src/ SUBDIRECTORY could ever be proven. 138 of the 9,149 enrolled objects live in subdirectories; it recorded 7 byte-exact FaderBrightness files as unproven debt and shipped that in the committed baseline. Key on the src-relative path instead. No bare-stem fallback for non-src/ paths: that fallback is dead code today and would let a bare name collide with an unrelated top-level object and report a file proven on another file's evidence. No such collision exists now -- the point is that it cannot appear later. Baseline regenerated, 19 -> 12. Also refuse --gate together with --ratchet or --update-baseline. --gate narrows the file set before the unproven set is computed, so `--update-baseline --gate 9` would write one gate's files over the whole ledger and fail much later, as a REGRESSION blamed on whoever next runs a full ratchet. Verified: port/hal/ reverted to main -> 13 binaries, 11 symbols, exit 1; restored -> 14/14 link, exit 0. Ratchet green at 12, unknown bucket empty across all 14 gates. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HgWTKakPywZzfdVbC6YLAu --- port/evidence-baseline.json | 9 +--- port/tools/port_evidence.py | 34 ++++++++++++++- port/tools/port_linkcheck.py | 83 +++++++++++++++++++++++++++++++++--- 3 files changed, 111 insertions(+), 15 deletions(-) diff --git a/port/evidence-baseline.json b/port/evidence-baseline.json index 8d683c83a..e856ce332 100644 --- a/port/evidence-baseline.json +++ b/port/evidence-baseline.json @@ -12,13 +12,6 @@ "src/_ZN5Timer9StopTimerEv.cpp", "src/_ZN8Platform21IsClsnInRangeOnScreenE5Fix12IiES1_.cpp", "src/_ZN9ActorBase22BeforeCleanupResourcesEv.cpp", - "src/_ZN9ActorBasenwEj.cpp", - "src/engine/fader/_ZN15FaderBrightness10SetToStartEv.cpp", - "src/engine/fader/_ZN15FaderBrightness14SetForwardTimeEj.cpp", - "src/engine/fader/_ZN15FaderBrightness15SetBackwardTimeEj.cpp", - "src/engine/fader/_ZN15FaderBrightness20IsBetweenStartAndEndEv.cpp", - "src/engine/fader/_ZN15FaderBrightness7IsAtEndEv.cpp", - "src/engine/fader/_ZN15FaderBrightness8SetToEndEv.cpp", - "src/engine/fader/_ZN15FaderBrightness9IsAtStartEv.cpp" + "src/_ZN9ActorBasenwEj.cpp" ] } diff --git a/port/tools/port_evidence.py b/port/tools/port_evidence.py index bd78c55c2..2e1c44308 100644 --- a/port/tools/port_evidence.py +++ b/port/tools/port_evidence.py @@ -148,6 +148,28 @@ def gates(): return out +def enrolled_key(rel): + """The key load_rom_evidence() stores, for a repo-relative source path. + + Enrollment keys come from build/src/<...>.o and KEEP their subdirectory -- + 'engine/fader/_ZN15FaderBrightness7IsAtEndEv'. This used to be compared + against Path(rel).stem, which can never contain a slash, so no file under a + src/ SUBDIRECTORY could ever be classified proven. 138 of the 9,149 + enrolled objects live in subdirectories, and it put 7 byte-exact + FaderBrightness files into the unproven debt ledger. + + Returns None for anything not under src/ (port/unmatched/, generated + sources), which is then simply not eligible for PROVEN. Deliberately NOT + falling back to the bare stem: a bare name can collide with an unrelated + top-level object and report a file as proven on another file's evidence. + No such collision exists today -- the point is that it cannot appear later. + """ + rel = rel.replace("\\", "/") + if not rel.startswith("src/"): + return None + return rel[len("src/"):].rsplit(".", 1)[0] + + def classify(rel, enrolled, reasons): if rel.startswith("port/unmatched/"): return REPLACED @@ -156,7 +178,8 @@ def classify(rel, enrolled, reasons): return UNKNOWN if "NONMATCHING" in p.read_text(encoding="utf-8", errors="replace"): return BANNER - if p.stem in enrolled: + key = enrolled_key(rel) + if key is not None and key in enrolled: return PROVEN if rel not in reasons: return UNKNOWN @@ -178,6 +201,15 @@ def main(): help="rewrite the baseline to the current set") args = ap.parse_args() + # --gate narrows the file set BEFORE the unproven set is computed, so + # `--update-baseline --gate 9` would rewrite the ledger to one gate's files + # and silently drop the rest. It fails loudly but much later -- as a + # REGRESSION on the next full --ratchet, blamed on whoever runs it. + if args.gate and (args.ratchet or args.update_baseline): + sys.exit("--gate cannot be combined with --ratchet or " + "--update-baseline: both operate on the whole ledger, and a " + "narrowed run would compare against (or write) a partial one.") + enrolled, reasons, report = load_rom_evidence() sha = report.get("romArtifact", {}).get("sha256", "?") print(f"evidence: byte-exact ROM build, {report['enrolledFiles']:,} files " diff --git a/port/tools/port_linkcheck.py b/port/tools/port_linkcheck.py index 09379ed3f..e9bbdd22e 100644 --- a/port/tools/port_linkcheck.py +++ b/port/tools/port_linkcheck.py @@ -29,6 +29,7 @@ """ import argparse import os +import re import subprocess import sys from pathlib import Path @@ -76,6 +77,81 @@ def msvc_env(): return env +# MSVC spells the two linkages differently, and the difference is a trap: +# C: unresolved external symbol _func_02059650 referenced in function ... +# C++: unresolved external symbol "char data_020a4d38" (?data_020a4d38@@3DA) +# referenced in function ... +# A \S+ capture stops at the first space, so every C++ symbol collapses to the +# token "char / "int / "public: and distinct symbols dedupe into one. That is +# the same undercount this report exists to prevent -- and #1049, the outage +# this tool was written for, was a C-vs-C++ linkage flip, so both shapes are +# guaranteed to appear here. Anchor on ' referenced in' instead. +SYM_RE = re.compile(r"unresolved external symbol (.+?)(?= referenced in function|\s*$)") + +# Only the trailing bucket is capped, and never silently -- see summarize(). +OTHER_CAP = 40 + + +def summarize(out): + """Group ninja/MSVC failure output into an untruncated diagnosis. + + The previous version printed the first 40 matching lines and stopped. On a + real linkage break that is about a third of the output, with no marker that + anything was dropped -- a reviewer reads 40 lines, counts 6 broken binaries + and moves on, when 13 are broken. (That happened, during review of the very + PR that added this file.) + + The output is redundant rather than large: each unresolved symbol repeats + once per binary needing it, and each binary compiles its own objects under + CMakeFiles/.dir/, so raw line-dedupe does NOT collapse them -- the + path prefixes differ. Grouping does. The two buckets that carry the + diagnosis are small once deduplicated (~11 symbols, ~14 binaries) and are + never capped; only the open-ended tail is, and it says so. + """ + keep = [ln.rstrip() for ln in out.splitlines() + # Surface the diagnosis, not the 4KB link command line carrying it. + if ("error" in ln.lower() or "FAILED" in ln) + and "cmd.exe /C" not in ln and "link.exe /nologo" not in ln] + if not keep: + return out[-3000:] + + # Buckets are assigned by CONSUMPTION, not by parallel membership tests: a + # line the symbol regex fails to parse must fall through to `other` rather + # than vanish from both. Anything that stops matching still gets printed. + syms, bins, other = {}, {}, {} + for ln in keep: + m = SYM_RE.search(ln) + if m: + syms.setdefault(m.group(1).strip(), None) + elif "LNK1120" in ln: + bins.setdefault(ln.strip(), None) + else: + # Compile-stage breaks (C1083, C2065), LNK1104, ninja's FAILED + # edges. Deduped too: one bad shared header emits the identical + # error once per target that includes it. + other.setdefault(ln.strip(), None) + + parts = [] + if syms: + parts.append(f"unresolved symbols ({len(syms)}):") + parts += [f" {s}" for s in syms] + if bins: + parts.append(f"\nbinaries that failed to link ({len(bins)}):") + parts += [f" {b}" for b in bins] + if other: + parts.append(f"\nother diagnostics ({len(other)}):") + parts += [f" {o}" for o in list(other)[:OTHER_CAP]] + if len(other) > OTHER_CAP: + parts.append(f" ... {len(other) - OTHER_CAP} more suppressed -- " + f"for the full set: ninja -C {BUILD} -k 0") + # Totals last and always honest, so a capped tail still cannot be read as + # the whole story. + parts.append(f"\nsummary: {len(bins)} binaries failed to link, " + f"{len(syms)} distinct unresolved symbols, " + f"{len(other)} other diagnostic line(s)") + return "\n".join(parts) + + def tool(env, name): for d in env.get("PATH", "").split(os.pathsep): p = Path(d) / f"{name}.exe" @@ -128,12 +204,7 @@ def main(): r = subprocess.run([ninja, "-C", str(BUILD), "-k", "0"] + targets, env=env, capture_output=True, text=True, errors="replace") if r.returncode: - out = r.stdout + r.stderr - # Surface the diagnosis, not the 4KB link command line that carries it. - keep = [ln for ln in out.splitlines() - if ("error" in ln.lower() or "FAILED" in ln) - and "cmd.exe /C" not in ln and "link.exe /nologo" not in ln] - print("\n".join(keep[:40]) or out[-3000:]) + print(summarize(r.stdout + r.stderr)) print(f"\nport-linkcheck: FAILED -- the port does not build against " f"the current src/ and include/.") return 1 From ca23a77cfe741fc483837ca4245d248bdb668514 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 5 Aug 2026 23:39:43 -0500 Subject: [PATCH 3/6] port: give both gates a real exit contract, and wire them into pre-push The two tools this branch added were never run by anything. Nothing in the repo referenced either one -- no workflow, no validation script, no hook, no doc; only each other. On top of that, port_linkcheck returned 0 when it SKIPPED, so on any machine or CI image without 32-bit MSVC it would have read green forever while checking nothing. That is the same failure the branch exists to fix, one level up: #1049 went unnoticed for two days because nothing built port/. A gate nobody runs, that cannot fail when it does not run, is not a gate. Exit contract, now shared by both tools: 0 checked, passed 1 checked, FAILED 2 could not check -- "this did not run" --require collapses 2 into 1, for CI or any caller that must not go green without the check actually running. port_evidence.py raises CannotCheck instead of sys.exit for missing ROM artifacts, a non-byte-exact build on record, and a missing baseline. All three used to exit 1, which is indistinguishable from "someone compiled an unproven file into the port" -- and the two demand opposite responses: re-run the ROM build, versus stop the merge. port_linkcheck.py also stops cold-configuring by default. Building 14 binaries from scratch is minutes, which is fine when asked for and wrong to spring on someone mid-`git push`; unconfigured is now "could not check", and --configure opts in. This mirrors what the hook already does for check_references.py: run only when the inputs exist, and say nothing was checked rather than implying a pass. tools/hooks/pre-push runs both in the every-push section, next to port_refcheck. They read the exit CODE rather than grepping stderr for phrases the way the check_references block above them has to -- an exit code cannot drift out of sync with a reworded message. Verified, in a worktree, all six paths: linkcheck no toolchain / unconfigured -> 2; with --require -> 1 clean tree -> 0 (14/14); hal reverted to main -> 1 evidence no artifacts -> 2; with --require -> 1; ratchet -> 0 at 12 hook inputs absent -> skips loudly, exit 0 broken linkage -> refuses, exit 1 (caught by the cheap default pair alone: 2 binaries, 5 symbols) Note for anyone who already installed the hook: it is a template, so re-copy it (cp tools/hooks/pre-push .git/hooks/pre-push). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HgWTKakPywZzfdVbC6YLAu --- port/tools/port_evidence.py | 72 ++++++++++++++++++++++++++++-------- port/tools/port_linkcheck.py | 66 +++++++++++++++++++++++++++------ tools/hooks/pre-push | 34 +++++++++++++++++ 3 files changed, 145 insertions(+), 27 deletions(-) diff --git a/port/tools/port_evidence.py b/port/tools/port_evidence.py index 2e1c44308..28e47de26 100644 --- a/port/tools/port_evidence.py +++ b/port/tools/port_evidence.py @@ -36,6 +36,12 @@ python port/tools/port_evidence.py --gate 9 # one gate, with files python port/tools/port_evidence.py --list # every non-proven file python port/tools/port_evidence.py --strict # exit 1 if UNPROVEN/BANNER + python port/tools/port_evidence.py --ratchet # exit 1 only if debt GREW + python port/tools/port_evidence.py --require # missing evidence = failure + +Exit codes: 0 passed, 1 FAILED, 2 could not check (no ROM build artifacts, the +build on record is not byte-exact, or no baseline). 2 is NOT a pass -- see the +CannotCheck docstring below. """ import argparse import json @@ -53,6 +59,22 @@ "proven", "explained", "UNPROVEN", "BANNER", "replaced", "unknown") ORDER = [PROVEN, EXPLAINED, UNPROVEN, BANNER, REPLACED, UNKNOWN] +# Same three-state exit contract as port_linkcheck.py: +# 0 checked, passed 1 checked, FAILED 2 could not check +OK, FAILED, CANNOT_CHECK = 0, 1, 2 + + +class CannotCheck(Exception): + """The ROM-build evidence this tool reads is absent or not byte-exact. + + Deliberately NOT the same outcome as a regression. --ratchet exiting 1 + because build/rombuild-report.json is missing is indistinguishable, to a + hook or a CI job, from exiting 1 because someone compiled an unproven file + into the port -- and the two demand opposite responses. One means "re-run + the ROM build"; the other means "stop the merge". Conflating them trains + people to ignore the gate, which costs more than the gate ever saved. + """ + # The gate ledger from port/README.md, so `--gate 4b` works the way the docs # and commit messages talk about the port. GATE_ALIAS = { @@ -71,14 +93,18 @@ def load_rom_evidence(): - """(enrolled stems, {relpath: reason}, report) -- or exit with why not.""" + """(enrolled keys, {relpath: reason}, report). + + Raises CannotCheck when the evidence is unavailable -- never sys.exit, so + the caller can distinguish "could not check" from "found a regression". + """ objs = BUILD / "objects.txt" elig = BUILD / "rombuild-eligibility.json" rep = BUILD / "rombuild-report.json" for p in (objs, elig, rep): if not p.exists(): - sys.exit(f"missing {p.relative_to(REPO)} -- run the ROM build first " - "(the port's evidence comes from it)") + raise CannotCheck(f"missing {p.relative_to(REPO)} -- run the ROM " + "build first (the port's evidence comes from it)") report = json.loads(rep.read_text(encoding="utf-8", errors="replace")) # Enrollment only means something if the build actually came out exact. @@ -86,10 +112,10 @@ def load_rom_evidence(): fid = an.get("moduleFidelity", {}) if not (report.get("status") == "passed" and an.get("passed") and fid.get("differingBytes") == 0): - sys.exit("the ROM build on record is NOT byte-exact " - f"(status={report.get('status')}, " - f"differingBytes={fid.get('differingBytes')}) -- " - "enrollment proves nothing until it is") + raise CannotCheck("the ROM build on record is NOT byte-exact " + f"(status={report.get('status')}, " + f"differingBytes={fid.get('differingBytes')}) -- " + "enrollment proves nothing until it is") enrolled = set() for line in objs.read_text(encoding="utf-8", errors="replace").splitlines(): @@ -199,6 +225,9 @@ def main(): help="exit 1 only if the unproven set GREW vs the baseline") ap.add_argument("--update-baseline", action="store_true", help="rewrite the baseline to the current set") + ap.add_argument("--require", action="store_true", + help="treat missing/non-exact ROM evidence as a FAILURE " + "(exit 1) instead of 'could not check' (exit 2)") args = ap.parse_args() # --gate narrows the file set BEFORE the unproven set is computed, so @@ -210,7 +239,15 @@ def main(): "--update-baseline: both operate on the whole ledger, and a " "narrowed run would compare against (or write) a partial one.") - enrolled, reasons, report = load_rom_evidence() + try: + enrolled, reasons, report = load_rom_evidence() + except CannotCheck as e: + print(f"port-evidence: NOT CHECKED -- {e}") + print(" This did NOT pass; it did not run.") + if args.require: + print(" --require was given, so a skip is a failure.") + return FAILED + return CANNOT_CHECK sha = report.get("romArtifact", {}).get("sha256", "?") print(f"evidence: byte-exact ROM build, {report['enrolledFiles']:,} files " f"enrolled, sha256 {sha[:16]}...") @@ -284,17 +321,20 @@ def main(): }, indent=2) + "\n", encoding="utf-8") print(f"\nbaseline updated: {len(current)} file(s) -> " f"{BASELINE.relative_to(REPO)}") - return 0 + return OK if args.ratchet: # A boolean gate is the wrong shape here: there are already unproven # files, so --strict would be red from day one and get ignored or # bypassed. The repo's own merge gate is a set of "must not regress" # ratchets (notes/pr-validation.md); this matches that. + # No baseline is "could not check" too: there is nothing to compare + # against, which is not the same claim as "the debt grew". if not BASELINE.exists(): - print(f"\nno baseline at {BASELINE.relative_to(REPO)} -- " - "run --update-baseline to record the current debt") - return 1 + print(f"\nport-evidence: NOT CHECKED -- no baseline at " + f"{BASELINE.relative_to(REPO)}; run --update-baseline to " + "record the current debt") + return FAILED if args.require else CANNOT_CHECK known = set(json.loads(BASELINE.read_text(encoding="utf-8"))["unproven"]) added = [f for f in current if f not in known] removed = sorted(known - set(current)) @@ -308,14 +348,14 @@ def main(): f"port without proof they are the game's code:") for f in added: print(f" + {f}") - return 1 + return FAILED print(f"\nratchet OK: no new unproven files " f"({len(current)} known, baseline {len(known)})") - return 0 + return OK if args.strict and worst: - return 1 - return 0 + return FAILED + return OK if __name__ == "__main__": diff --git a/port/tools/port_linkcheck.py b/port/tools/port_linkcheck.py index e9bbdd22e..ed7c5d2b1 100644 --- a/port/tools/port_linkcheck.py +++ b/port/tools/port_linkcheck.py @@ -26,6 +26,10 @@ python port/tools/port_linkcheck.py # smoke + smoke_heap python port/tools/port_linkcheck.py --all # every gate binary python port/tools/port_linkcheck.py --targets smoke_actor + python port/tools/port_linkcheck.py --require # a skip is a failure (CI) + +Exit codes: 0 passed, 1 FAILED, 2 could not check (no toolchain / not +configured). 2 is not a pass -- see the OK/FAILED/CANNOT_CHECK note below. """ import argparse import os @@ -41,6 +45,23 @@ PORT = REPO / "port" BUILD = REPO / "build" / "port" +# Exit contract, shared with port_evidence.py, so a hook or a CI job can tell +# the three outcomes apart WITHOUT grepping message text: +# 0 checked, passed +# 1 checked, FAILED +# 2 could not check -- no toolchain. "This did not run." +# A skip used to return 0. That is exactly how an unwired gate reads green +# forever on an image without MSVC -- the same silent pass this tool exists to +# end, one level up. The message always said "this did NOT pass"; now the exit +# code says it too. --require collapses 2 into 1 for callers that must have the +# check actually run. +# +# tools/check_references.py has these same three states and separates them by +# having its CALLER grep stderr for phrases ("report describes", "no commit +# stamp", "missing "). That works, but it couples the hook to wording that no +# test pins. An exit code cannot drift out of sync with a reworded message. +OK, FAILED, CANNOT_CHECK = 0, 1, 2 + # The cheapest pair that still spans the HAL seams #1049 broke: gate 1 pulls # os_time/shims, gate 2 pulls the heap bridges. Both failed in that outage. DEFAULT_TARGETS = ["smoke", "smoke_heap"] @@ -163,25 +184,48 @@ def tool(env, name): return str(p) if p.exists() else None +def skipped(why, require): + """Report a check that did not run, and return the right exit code.""" + print(f"port-linkcheck: SKIPPED -- {why}") + print(" This did NOT pass; it did not run.") + if require: + print(" --require was given, so a skip is a failure.") + return FAILED + return CANNOT_CHECK + + def main(): ap = argparse.ArgumentParser() ap.add_argument("--all", action="store_true", help="build every gate binary") ap.add_argument("--targets", nargs="*", help="explicit ninja targets") + ap.add_argument("--require", action="store_true", + help="treat a missing toolchain as a FAILURE (exit 1) " + "instead of 'could not check' (exit 2) -- for CI and " + "any caller that must not go green without running") + ap.add_argument("--configure", action="store_true", + help="configure build/port if it is not set up yet; " + "without this an unconfigured tree is 'could not " + "check' rather than a minutes-long cold build") args = ap.parse_args() env = msvc_env() if env is None: - print("port-linkcheck: SKIPPED -- no 32-bit MSVC toolchain found.") - print(" This did NOT pass; it did not run.") - return 0 + return skipped("no 32-bit MSVC toolchain found.", args.require) cmake, ninja = tool(env, "cmake"), tool(env, "ninja") if not cmake or not ninja: - print("port-linkcheck: SKIPPED -- cmake/ninja not found " - "(VS component 'C++ CMake tools for Windows', or " - "pip install cmake ninja).") - print(" This did NOT pass; it did not run.") - return 0 + return skipped("cmake/ninja not found (VS component 'C++ CMake tools " + "for Windows', or pip install cmake ninja).", args.require) + + # Configuring builds all 14 binaries from cold, which is minutes -- fine + # when asked for, wrong to spring on someone mid-`git push`. Unconfigured + # is therefore "could not check", the same shape the pre-push hook already + # uses for check_references.py: run only when the inputs are already there, + # and say nothing was checked otherwise rather than implying a pass. + if not (BUILD / "build.ninja").exists() and not args.configure: + return skipped(f"{BUILD.relative_to(REPO)} is not configured. " + "Run port/build-port.cmd once, or pass --configure.", + args.require) if not (BUILD / "build.ninja").exists(): print(f"port-linkcheck: configuring {BUILD.relative_to(REPO)}") @@ -192,7 +236,7 @@ def main(): if r.returncode: print(r.stdout[-3000:] + r.stderr[-3000:]) print("port-linkcheck: FAILED to configure") - return 1 + return FAILED targets = args.targets or ([] if args.all else DEFAULT_TARGETS) # -k 0 = keep going after failures. Ninja's default (-k 1) stops at the @@ -207,11 +251,11 @@ def main(): print(summarize(r.stdout + r.stderr)) print(f"\nport-linkcheck: FAILED -- the port does not build against " f"the current src/ and include/.") - return 1 + return FAILED built = ", ".join(targets) if targets else "all gate binaries" print(f"port-linkcheck: OK ({built})") - return 0 + return OK if __name__ == "__main__": diff --git a/tools/hooks/pre-push b/tools/hooks/pre-push index de1721c14..261cd965c 100644 --- a/tools/hooks/pre-push +++ b/tools/hooks/pre-push @@ -96,5 +96,39 @@ else exit 1 fi +# port/ has to COMPILE AND LINK against src/ and include/, and port_refcheck.py +# above cannot see that: it catches a rename stranding a reference, but #1049 +# renamed nothing -- it changed LINKAGE, every reference stayed valid, and 13 of +# the 14 gate binaries stopped linking for two days with nothing to notice. +# Only linking catches a linkage break. +# +# Unlike the reference check above, these two report "could not check" with a +# distinct EXIT CODE (2) rather than a message this hook has to pattern-match, +# so a missing toolchain can never be mistaken for a pass. Both are cheap when +# their inputs already exist and skip when they do not -- port_linkcheck is +# incremental (~2s) and refuses to cold-configure without --configure, so a +# first push never turns into a minutes-long build. +echo "pre-push: link-checking the port build" +python port/tools/port_linkcheck.py +case $? in + 0) ;; + 2) echo "pre-push: (skipping port link check -- not configured or no MSVC;" + echo " run 'port/build-port.cmd' once to include it)" ;; + *) echo "pre-push: REFUSING to push (see above). Override with --no-verify if you are certain." >&2 + exit 1 ;; +esac + +echo "pre-push: checking port evidence ratchet" +python port/tools/port_evidence.py --ratchet >/dev/null 2>&1 +case $? in + 0) echo "pre-push: port evidence ratchet OK" ;; + 2) echo "pre-push: (skipping evidence ratchet -- no current ROM build report;" + echo " run 'python tools/rombuild.py' to include it)" ;; + *) echo "pre-push: port evidence ratchet FAILED" + python port/tools/port_evidence.py --ratchet + echo "pre-push: REFUSING to push (see above). Override with --no-verify if you are certain." >&2 + exit 1 ;; +esac + remote_ref_check || exit 1 exit 0 From 882bc18b5188d33172febd0aec84ae44a5acbc53 Mon Sep 17 00:00:00 2001 From: = Date: Wed, 5 Aug 2026 23:40:48 -0500 Subject: [PATCH 4/6] port: document core.hooksPath as the install that cannot go stale The hook only documented `cp tools/hooks/pre-push .git/hooks/pre-push`, so the commit before this one told people to re-copy it. That is right for the cp method and unnecessary for the other one: this repo is configured with core.hooksPath=tools/hooks, where git runs the tracked file directly and the new checks went live on the very push that added them. Worth writing down because the failure mode of the cp method is silent -- a stale copy still runs, just without whatever check was added since, which is the same "looks like it is being checked, is not" shape the port gates in this branch exist to close. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HgWTKakPywZzfdVbC6YLAu --- tools/hooks/pre-push | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/hooks/pre-push b/tools/hooks/pre-push index 261cd965c..af82033b1 100644 --- a/tools/hooks/pre-push +++ b/tools/hooks/pre-push @@ -17,8 +17,14 @@ # push rather than only main-targeted ones: neither needs the ROM, and both catch mistakes # that are invisible in review because every byte still matches. # -# Install: +# Install, either way: +# git config core.hooksPath tools/hooks # tracked copy runs directly; +# # updates land with a pull # cp tools/hooks/pre-push .git/hooks/pre-push && chmod +x .git/hooks/pre-push +# The second needs re-copying whenever this file changes, and a stale copy is +# invisible -- it still runs, just without whatever check was added. Prefer the +# first. (If core.hooksPath is set, git ignores .git/hooks entirely, so an old +# copy left there is dead weight rather than a conflict.) # Bypass once (you had better be sure): # git push --no-verify From 6665c3a5acab1db4add8364927a4975b6a80c93a Mon Sep 17 00:00:00 2001 From: = Date: Wed, 5 Aug 2026 23:47:49 -0500 Subject: [PATCH 5/6] port: make --strict compose with --ratchet instead of being ignored Every path inside the `if args.ratchet:` block returned, and the --strict check sits below it, so passing both ran only the ratchet. --strict was not rejected and not warned about -- it silently did nothing, which is the worst of the three ways to handle it: the caller reads the exit code believing both gates ran. --strict 1 --ratchet 0 --ratchet --strict 0 <- strict ignored; now 1 The two are not in conflict. --strict is the absolute floor, --ratchet the derivative; "did not regress AND never above zero" is coherent, and becomes the natural gate once the ledger empties. So the ratchet's OK path falls through to the strict check rather than returning. A regression still short-circuits with its REGRESSION list, so the more actionable diagnosis is still the one printed. --update-baseline continues to return without consulting --strict, which is deliberate -- it writes the ledger rather than gating on it -- and now says so. Also print an explicit "FAILED --strict" verdict. With both flags the last line printed was "ratchet OK", and exiting 1 immediately after that reads as a bug in the tool rather than a verdict from the other gate. Today --ratchet --strict therefore always fails, because 12 unproven files exist. That is honest rather than useful, and it is why --ratchet was added in the first place (--strict alone is red from day one). The combination earns its keep when the debt reaches zero; until then a flag that fails loudly still beats one that does nothing. Verified: --strict 1, --ratchet 0, --ratchet --strict 1, no-flags 0, missing artifacts 2 (could-not-check still outranks both), and a simulated baseline regression exits 1 still naming the files. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HgWTKakPywZzfdVbC6YLAu --- port/tools/port_evidence.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/port/tools/port_evidence.py b/port/tools/port_evidence.py index 28e47de26..9d1120a59 100644 --- a/port/tools/port_evidence.py +++ b/port/tools/port_evidence.py @@ -220,7 +220,8 @@ def main(): ap.add_argument("--gate", help="show one target (e.g. 9, smoke_actor)") ap.add_argument("--list", action="store_true", help="list every non-proven file") ap.add_argument("--strict", action="store_true", - help="exit 1 if any UNPROVEN or BANNER file is compiled") + help="exit 1 if any UNPROVEN or BANNER file is compiled. " + "Composes with --ratchet: both must pass") ap.add_argument("--ratchet", action="store_true", help="exit 1 only if the unproven set GREW vs the baseline") ap.add_argument("--update-baseline", action="store_true", @@ -351,9 +352,25 @@ def main(): return FAILED print(f"\nratchet OK: no new unproven files " f"({len(current)} known, baseline {len(known)})") - return OK - + # --strict used to be dead whenever --ratchet was passed: every path in + # this block returned, and the strict check sits below it. The flag was + # not rejected and not warned about, it just did nothing -- the worst of + # the three options, since the caller reads the exit code as though both + # gates ran. They are not in conflict: --strict is the absolute floor, + # --ratchet the derivative, and "did not regress AND never above zero" + # is coherent -- it becomes the natural gate once the ledger empties. + # So fall through and let the stricter one also have its say. + if not args.strict: + return OK + + # Note: --update-baseline returns above without consulting --strict. That + # one is deliberate -- it writes the ledger rather than gating on it. if args.strict and worst: + # Say so explicitly. With --ratchet --strict the last thing printed is + # "ratchet OK", and exiting 1 straight after that reads as a bug in the + # tool rather than a verdict from the other gate. + print(f"\nport-evidence: FAILED --strict: {worst} file(s) compiled into " + f"the port are UNPROVEN or NONMATCHING-bannered.") return FAILED return OK From 0fee0a697102f6a72ac6874be9e0a8a7625a2ebb Mon Sep 17 00:00:00 2001 From: = Date: Mon, 10 Aug 2026 03:53:45 -0500 Subject: [PATCH 6/6] port: read the eligibility report through the reader that knows both vintages The evidence ratchet crashed on every run, which meant this PR could not be pushed at all -- its own pre-push hook installs the ratchet, and the ratchet aborted with `AttributeError: 'str' object has no attribute 'get'`. `build/rombuild-eligibility.json` gained a `{commit, dirty, files}` wrapper after this branch was written; it used to be a bare list. Iterating the stamped shape as a list yields its three KEYS, and the first thing done to a row is `.get`, so it dies on a string instead of saying the format moved. `tools/eligible.py:load_report` already reads both shapes and exists precisely so consumers do not have to care. Using it rather than adding a third opinion about the file. `python port/tools/port_evidence.py --ratchet` now runs: OK, 2 known unproven files against a baseline of 12. `port_refcheck` 403/403. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015AXm5k53WFPjCYcDHRDX3x --- port/tools/port_evidence.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/port/tools/port_evidence.py b/port/tools/port_evidence.py index 9d1120a59..ff2db7fc6 100644 --- a/port/tools/port_evidence.py +++ b/port/tools/port_evidence.py @@ -123,8 +123,18 @@ def load_rom_evidence(): if m: enrolled.add(m.group(1)) + # build/rombuild-eligibility.json has two vintages: a bare list, and the + # stamped {commit, dirty, files} shape eligible.py writes now. Iterating the + # stamped one as a list yields its KEYS -- three strings -- and the first + # thing done to a row is `.get`, so it dies with AttributeError on a str + # rather than saying the format moved. tools/eligible.py:load_report is the + # reader that knows both; use it rather than keeping a third opinion. + sys.path.insert(0, str(REPO / "tools")) + import eligible as ELIG # noqa: E402 + rows, _commit, _dirty = ELIG.load_report(elig) + reasons = {} - for row in json.loads(elig.read_text(encoding="utf-8", errors="replace")): + for row in rows: reasons[row["file"].replace("\\", "/")] = row.get("reason") return enrolled, reasons, report