diff --git a/docs/GAME_PROJECT_SETUP.md b/docs/GAME_PROJECT_SETUP.md index 9e4a2888..258baee1 100644 --- a/docs/GAME_PROJECT_SETUP.md +++ b/docs/GAME_PROJECT_SETUP.md @@ -434,11 +434,24 @@ submodules land: (Linux x64, Windows x64, macOS arm64 + Intel) - `scripts/package_setup_release.sh` ← zip prefix / exe / display name -`--exe-name` / `codegen_setup.exe_basename` must match CMake -`OUTPUT_NAME` (`MAKE_C_IDENTIFIER(WINDOW_TITLE)`, e.g. `TwistedMetal4_Recompiled`), -not the bare project folder name (`TwistedMetal4Recomp`). Scaffolding derives +`--exe-name` / `codegen_setup.exe_basename` should match CMake `OUTPUT_NAME` +(`MAKE_C_IDENTIFIER(WINDOW_TITLE)`, e.g. `TwistedMetal4_Recompiled`), not the +bare project folder name (`TwistedMetal4Recomp`). Scaffolding derives `EXE_BASENAME` from `WINDOW_TITLE` for that reason. +It is no longer fatal when they disagree. CMake is the single source of truth: +`runtime.cmake` writes the `OUTPUT_NAME` it chose to +`/psxrecomp_exe_name-.txt`, and both the build CLI and the +in-runtime self-compiler read that in preference to their own copy. A build +tree predating the marker still falls back to `exe_basename`. + +This matters because the two values are separate copies of the same window +title, and renaming a game updates one of them. Before the marker existed, that +drift produced `build succeeded but binary missing` on a build with no errors +at all — the executable was sitting in the build directory under the name CMake +had chosen. To pin the name explicitly instead of deriving it, pass `EXE_NAME` +to `psxrecomp_add_game_runtime()`. + Zip prefix defaults to a short acronym from `--name` (e.g. `MastersOfTerasKasiRecomp` → `motk`); override with `--zip-prefix` / `-ZipPrefix`. diff --git a/host/psxrecomp_codegen_host.c b/host/psxrecomp_codegen_host.c index 23aff86d..44f3a8fd 100644 --- a/host/psxrecomp_codegen_host.c +++ b/host/psxrecomp_codegen_host.c @@ -83,6 +83,30 @@ static int path_is_file(const char* path) { #endif } +/* Read the first line of a small text file into out, trimming EOL and any + * trailing spaces. Returns 1 on success. Used for the exe-name marker CMake + * writes; anything unreadable just leaves the caller on its fallback. */ +static int read_first_line(const char* path, char* out, size_t cap) { + FILE* f; + size_t n; + if (!path || !out || cap == 0) + return 0; + f = fopen(path, "rb"); + if (!f) + return 0; + if (!fgets(out, (int)cap, f)) { + fclose(f); + out[0] = '\0'; + return 0; + } + fclose(f); + n = strlen(out); + while (n > 0 && (out[n - 1] == '\n' || out[n - 1] == '\r' || + out[n - 1] == ' ' || out[n - 1] == '\t')) + out[--n] = '\0'; + return out[0] != '\0'; +} + static int path_is_absolute(const char* path) { if (!path || !path[0]) return 0; #if defined(_WIN32) @@ -1084,11 +1108,28 @@ static int resolve_build_paths(void) { } } + /* Prefer the name CMake published over the one baked into codegen_setup.c. + * Both are MAKE_C_IDENTIFIER(WINDOW_TITLE), but from two separate copies of + * the title, so renaming a game leaves this one pointing at an executable + * that is never produced. runtime.cmake writes the name it really used to + * psxrecomp_exe_name-.txt beside the build. */ + char basename[256]; + snprintf(basename, sizeof(basename), "%s", g_exe_basename); + if (g_cfg && g_cfg->cmake_target && g_cfg->cmake_target[0]) { + char marker[1200], published[256]; + snprintf(published, sizeof(published), "psxrecomp_exe_name-%s.txt", + g_cfg->cmake_target); + if (join_path(marker, sizeof(marker), g_build_dir, published) && + read_first_line(marker, published, sizeof(published)) && + published[0]) + snprintf(basename, sizeof(basename), "%s", published); + } + char exe_name[300]; #if defined(_WIN32) - snprintf(exe_name, sizeof(exe_name), "%s.exe", g_exe_basename); + snprintf(exe_name, sizeof(exe_name), "%s.exe", basename); #else - snprintf(exe_name, sizeof(exe_name), "%s", g_exe_basename); + snprintf(exe_name, sizeof(exe_name), "%s", basename); #endif return join_path(g_exe_path, sizeof(g_exe_path), g_build_dir, exe_name); } diff --git a/psxrecomp_cli.py b/psxrecomp_cli.py index 06bee47e..8442f1d9 100755 --- a/psxrecomp_cli.py +++ b/psxrecomp_cli.py @@ -1306,6 +1306,59 @@ def _cmake_configure( +def _runtime_exe_candidates(build_dir: Path, target: str, exe_basename: str): + """Names to try for the built runtime, best evidence first. + + runtime.cmake writes the OUTPUT_NAME it actually set to + psxrecomp_exe_name-.txt at configure time. That file is the answer; + exe_basename is only a guess re-derived from a second copy of the window + title, and the two drift the moment a game is renamed. + """ + published = build_dir / f"psxrecomp_exe_name-{target}.txt" + names, seen = [], set() + try: + name = published.read_text(encoding="utf-8").strip() + except OSError: + name = "" + for candidate in (name, exe_basename): + if candidate and candidate not in seen: + seen.add(candidate) + names.append(candidate) + return names, name + + +def _resolve_runtime_exe(build_dir: Path, target: str, exe_basename: str): + """(path, None) once found, or (None, message) explaining what went wrong.""" + names, published = _runtime_exe_candidates(build_dir, target, exe_basename) + for name in names: + for path in (build_dir / name, build_dir / f"{name}.exe"): + if path.is_file(): + return path, None + + # Nothing matched. Distinguish the two very different reasons, because + # "binary missing" after a clean build reads as a build failure and is not + # one: a name disagreement means the executable is sitting right there. + built = sorted( + p.name + for p in build_dir.glob("*") + if p.is_file() and (p.suffix.lower() == ".exe" or os.access(p, os.X_OK)) + ) + if published and published != exe_basename: + return None, ( + f"the runtime was built as '{published}' but this project expects " + f"'{exe_basename}'. The build itself succeeded — nothing is wrong " + f"with the code. CMake derives the name from WINDOW_TITLE in " + f"CMakeLists.txt; --exe-name / codegen_setup.exe_basename carries a " + f"second copy of that title which has drifted. Make them agree, or " + f"pin it by passing EXE_NAME to psxrecomp_add_game_runtime()." + ) + hint = f" Executables present: {', '.join(built)}." if built else "" + return None, ( + f"build reported success but no runtime binary was found in " + f"{build_dir} (looked for {', '.join(names)}).{hint}" + ) + + def _cmake_build( build_dir: Path, target: str, @@ -1380,6 +1433,7 @@ def run_pgo_train( build_dir: Path, *, exe_basename: str, + target: str, disc: Path, config: Path, train_secs: int, @@ -1388,11 +1442,9 @@ def run_pgo_train( hide_video: bool, progress: ProgressReporter, ) -> None: - exe = build_dir / exe_basename - if not exe.is_file(): - exe = build_dir / f"{exe_basename}.exe" - if not exe.is_file(): - raise RuntimeError(f"train binary missing: {exe_basename} under {build_dir}") + exe, exe_err = _resolve_runtime_exe(build_dir, target, exe_basename) + if exe is None: + raise RuntimeError(f"PGO training cannot start: {exe_err}") pgo_dir = build_dir / "pgo" if pgo_dir.exists(): @@ -1641,6 +1693,7 @@ def cmd_rebuild(args: argparse.Namespace, progress: ProgressReporter) -> int: project_root, build_dir, exe_basename=exe_basename, + target=target, disc=Path(disc), config=config, train_secs=train_secs, @@ -1662,11 +1715,9 @@ def cmd_rebuild(args: argparse.Namespace, progress: ProgressReporter) -> int: progress.error(str(exc), code=EXIT_ERROR) return EXIT_ERROR - exe = build_dir / exe_basename - if not exe.is_file(): - exe = build_dir / f"{exe_basename}.exe" - if not exe.is_file(): - progress.error(f"build succeeded but binary missing: {exe}", code=EXIT_ERROR) + exe, exe_err = _resolve_runtime_exe(build_dir, target, exe_basename) + if exe is None: + progress.error(exe_err, code=EXIT_ERROR) return EXIT_ERROR prune_raw = (getattr(args, "prune_after", None) or "").strip() diff --git a/recompiler/CMakeLists.txt b/recompiler/CMakeLists.txt index 8527f226..e695784e 100644 --- a/recompiler/CMakeLists.txt +++ b/recompiler/CMakeLists.txt @@ -456,6 +456,13 @@ if(BUILD_TESTING) ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_cli_project_packaging.py WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/..) + # The runtime exe name must come from CMake, not be re-derived. + add_test( + NAME runtime_exe_name + COMMAND ${Python3_EXECUTABLE} + ${CMAKE_CURRENT_SOURCE_DIR}/tests/test_runtime_exe_name.py + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/..) + # These two invoke psxrecomp-game, whose BIOS-profile resolution # probes fixed working-directory-relative paths (bios/.toml, # then psxrecomp/bios/.toml) -- run them from the framework diff --git a/recompiler/tests/test_runtime_exe_name.py b/recompiler/tests/test_runtime_exe_name.py new file mode 100644 index 00000000..c4ac56aa --- /dev/null +++ b/recompiler/tests/test_runtime_exe_name.py @@ -0,0 +1,120 @@ +#!/usr/bin/env python3 +"""The runtime's executable name has one source of truth: CMake. + +runtime.cmake sets OUTPUT_NAME from MAKE_C_IDENTIFIER(WINDOW_TITLE) and writes +that name to psxrecomp_exe_name-.txt. Two consumers used to re-derive +it instead — psxrecomp_cli.py from --exe-name, and the in-runtime self-compiler +from codegen_setup.exe_basename — each applying the same rule to its own copy +of the window title. The rule is identical; the copies drift the moment a game +is renamed, and the build then links .exe while both consumers hunt for +.exe. + +That surfaced on Revelations: Persona as "build succeeded but binary missing" +after a build with zero errors — CMake had emitted +Revelations__Persona_Recompiled.exe while the setup host wanted +Revelations_Persona__Recompiled.exe. Same algorithm, colon in a different +place. What this pins: + + 1. the published marker wins over a stale exe_basename; + 2. an older build tree with no marker still resolves via the fallback; + 3. when nothing resolves, the error names the executables that ARE present + instead of implying the compile failed; + 4. a drifted name is reported as a name mismatch, not a build failure. +""" + +from pathlib import Path +import importlib.util +import stat +import sys +import tempfile + + +ROOT = Path(__file__).resolve().parents[2] + +# The two names the wild failure produced, kept verbatim so the regression is +# recognisable if it ever comes back. +BUILT = "Revelations__Persona_Recompiled" # MAKE_C_IDENTIFIER of the title +STALE = "Revelations_Persona__Recompiled" # a drifted second copy + +failures = 0 + + +def check(cond: bool, what: str) -> None: + global failures + if not cond: + print(f"FAIL: {what}", file=sys.stderr) + failures += 1 + + +def load_cli(): + spec = importlib.util.spec_from_file_location( + "psxrecomp_cli", ROOT / "psxrecomp_cli.py") + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +def main() -> int: + cli = load_cli() + + # runtime.cmake must actually publish the name, or every consumer below is + # back to guessing. + cmake = (ROOT / "runtime/runtime.cmake").read_text(encoding="utf-8") + check("psxrecomp_exe_name-${target}.txt" in cmake, + "runtime.cmake writes psxrecomp_exe_name-.txt") + check('MAKE_C_IDENTIFIER "${PSXRT_WINDOW_TITLE}"' in cmake, + "OUTPUT_NAME still derives from WINDOW_TITLE (marker must match it)") + + with tempfile.TemporaryDirectory() as tmp: + build = Path(tmp) + # Executable bit matters: the "what IS present" listing filters on it + # (or a .exe suffix), the way a real build output would be. + built_path = build / BUILT + built_path.write_text("", encoding="utf-8") + built_path.chmod(built_path.stat().st_mode | stat.S_IXUSR) + marker = build / "psxrecomp_exe_name-psx-runtime.txt" + marker.write_text(BUILT + "\n", encoding="utf-8") + + # 1. The case that broke in the wild. + exe, err = cli._resolve_runtime_exe(build, "psx-runtime", STALE) + check(exe is not None and exe.name == BUILT, + "a stale exe_basename resolves via the published marker") + check(err is None, "no error when the marker resolves it") + + # A correct exe_basename must keep working unchanged. + exe, _ = cli._resolve_runtime_exe(build, "psx-runtime", BUILT) + check(exe is not None and exe.name == BUILT, + "a matching exe_basename still resolves") + + # 2. Build trees predating the marker fall back to the old behaviour. + marker.unlink() + exe, _ = cli._resolve_runtime_exe(build, "psx-runtime", BUILT) + check(exe is not None and exe.name == BUILT, + "no marker + correct name falls back to the derived name") + + # 3. Genuinely unresolvable: say what is there rather than accusing the + # compiler. + exe, err = cli._resolve_runtime_exe(build, "psx-runtime", STALE) + check(exe is None, "no marker + stale name cannot resolve") + check(err is not None and BUILT in err, + "the error lists the executables actually present") + + # 4. Marker present, binary gone: a name mismatch, reported as one. + marker.write_text(BUILT + "\n", encoding="utf-8") + built_path.unlink() + exe, err = cli._resolve_runtime_exe(build, "psx-runtime", STALE) + check(exe is None, "a missing binary still fails") + check(err is not None and "build itself succeeded" in err, + "a drifted name is not reported as a build failure") + check(err is not None and BUILT in err and STALE in err, + "the error names both the built and the expected name") + + if failures: + print(f"test_runtime_exe_name: {failures} failure(s)", file=sys.stderr) + return 1 + print("PASS: runtime exe name has a single source of truth") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/runtime/runtime.cmake b/runtime/runtime.cmake index c99c5069..98f1ab6d 100644 --- a/runtime/runtime.cmake +++ b/runtime/runtime.cmake @@ -962,6 +962,28 @@ function(psxrecomp_add_runtime_target target) endif() set_target_properties(${target} PROPERTIES OUTPUT_NAME "${_psxrt_exe_name}") + # Publish the name we just chose, so nothing downstream has to re-derive it. + # + # Two other places used to compute this independently — psxrecomp_cli.py + # from --exe-name, and the in-runtime self-compiler from + # codegen_setup.exe_basename — each re-running the same + # MAKE_C_IDENTIFIER(WINDOW_TITLE) rule against its own copy of the title. + # The rules agree; the copies drift. Rename a game and the build links + # .exe while both consumers look for .exe, which surfaces as the + # flatly untrue "build succeeded but binary missing" on a build that had no + # errors at all. Seen in the wild on Revelations: Persona, where CMake + # emitted Revelations__Persona_Recompiled.exe and the setup host wanted + # Revelations_Persona__Recompiled.exe — same algorithm, colon in a + # different place. + # + # Written at configure time (not file(GENERATE)) because _psxrt_exe_name is + # a plain variable here: it carries no generator expression and does not + # vary per configuration, so a fixed filename is correct on multi-config + # generators too. Name only, never a path — the consumers already know + # their build directory, and only ever got the basename wrong. + file(WRITE "${CMAKE_BINARY_DIR}/psxrecomp_exe_name-${target}.txt" + "${_psxrt_exe_name}\n") + # ---- Windows / desktop app icon --------------------------------------- # Prefer an explicit APP_ICON, then the game-repo copy under assets/, then # the framework default shipped in psxrecomp/assets (RetComM-themed pad).