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..e856ce332 --- /dev/null +++ b/port/evidence-baseline.json @@ -0,0 +1,17 @@ +{ + "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" + ] +} 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..ff2db7fc6 --- /dev/null +++ b/port/tools/port_evidence.py @@ -0,0 +1,389 @@ +#!/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 + 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 +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] + +# 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 = { + "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 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(): + 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. + an = report.get("analysis", {}) + fid = an.get("moduleFidelity", {}) + if not (report.get("status") == "passed" and an.get("passed") + and fid.get("differingBytes") == 0): + 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(): + m = re.search(r"/build/src/(.+)\.o$", line.strip().replace("\\", "/")) + 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 rows: + 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 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 + p = REPO / rel + if not p.exists(): + return UNKNOWN + if "NONMATCHING" in p.read_text(encoding="utf-8", errors="replace"): + return BANNER + key = enrolled_key(rel) + if key is not None and key 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. " + "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", + 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 + # `--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.") + + 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]}...") + 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 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"\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)) + 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 FAILED + print(f"\nratchet OK: no new unproven files " + f"({len(current)} known, baseline {len(known)})") + # --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 + + +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..ed7c5d2b1 --- /dev/null +++ b/port/tools/port_linkcheck.py @@ -0,0 +1,262 @@ +#!/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 + 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 +import re +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" + +# 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"] + + +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 + + +# 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" + 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 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: + return skipped("no 32-bit MSVC toolchain found.", args.require) + + cmake, ninja = tool(env, "cmake"), tool(env, "ninja") + if not cmake or not ninja: + 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)}") + 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 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 + # 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: + print(summarize(r.stdout + r.stderr)) + print(f"\nport-linkcheck: FAILED -- the port does not build against " + f"the current src/ and include/.") + return FAILED + + built = ", ".join(targets) if targets else "all gate binaries" + print(f"port-linkcheck: OK ({built})") + return OK + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/hooks/pre-push b/tools/hooks/pre-push index de1721c14..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 @@ -96,5 +102,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