From 0b3720b23167c1548729e0d67696d5d805a81dbd Mon Sep 17 00:00:00 2001 From: Eric J Date: Mon, 17 Aug 2026 11:07:35 -0700 Subject: [PATCH] Point read-data at the resolved data dir, not a relative token build_config_ini emitted read-data=__PATH__executable__/../data on every platform. That token is correct for a macOS bundle, where the binary sits at Contents/MacOS/factorio and data is one level up. Windows and Linux put the binary at bin/x64/, so the same token resolves to bin/data and the game refuses to start. Measured on a real 2.1.14 Windows install, 2026-08-17: read-data=__PATH__executable__/../data -> exit 1 "There is no package core in V:/factorio-2.1.14/bin/data" read-data=__PATH__executable__/../../data -> exit 0 resolve_layout already works the data directory out correctly for every layout it supports, and that value was simply not being used. So take it as a parameter and emit it absolute. Branching the token on layout would also work, but it re-derives here what resolve_layout has already established, and a second copy of that knowledge is a second chance to get it wrong. Windows accepts either slash style, so Path::display() emitting native backslashes needs no rewriting: V:\factorio-2.1.14\data and V:/factorio-2.1.14/data both exit 0 and dump identical bytes. Measured, since a backslash in an ini value is the kind of thing that gets eaten. Every test passed before this change because the suite only ever ran on macOS, which is the one layout the token was right for. The new test asserts the token is absent for a bundle path, a Linux path and a Windows path, so the macOS-only blind spot cannot silently return. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01P9FADuTnjE7SFEQWnpNhfc --- src/run.rs | 9 +++++-- src/scaffold.rs | 66 ++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/src/run.rs b/src/run.rs index dcba583..7140860 100644 --- a/src/run.rs +++ b/src/run.rs @@ -97,8 +97,13 @@ pub fn run_probe(request: &RunRequest, spawner: &dyn Spawner) -> anyhow::Result< fs::create_dir_all(&script_output)?; // The isolated config is what makes a stale dump impossible: write-data - // points at a directory that started empty. - fs::write(&config_path, build_config_ini(&write_data))?; + // points at a directory that started empty. read-data comes from the + // resolved layout rather than a relative token, because the token is only + // correct for a macOS bundle. + fs::write( + &config_path, + build_config_ini(&request.layout.data_dir, &write_data), + )?; if let Some(settings) = request.map_gen_settings.as_ref() { fs::write(&map_gen_path, serde_json::to_string_pretty(settings)?)?; } diff --git a/src/scaffold.rs b/src/scaffold.rs index 4241bdb..0567551 100644 --- a/src/scaffold.rs +++ b/src/scaffold.rs @@ -85,13 +85,39 @@ pub fn build_mod_list(mod_name: Option<&str>, disabled: &[String]) -> Value { /// An isolated `config.ini`. /// -/// `read-data` points at the install's bundled data through Factorio's own -/// portable token, and `write-data` at a scratch directory that started empty. -/// That second half is what makes a stale dump from an earlier capture -/// impossible to pick up by accident. -pub fn build_config_ini(write_data: &Path) -> String { +/// `read-data` points at the install's bundled data and `write-data` at a +/// scratch directory that started empty. That second half is what makes a +/// stale dump from an earlier capture impossible to pick up by accident. +/// +/// `read_data` must be the `data_dir` that [`resolve_layout`] worked out, not +/// a relative token. This used to emit Factorio's portable +/// `__PATH__executable__/../data`, which is correct for a macOS bundle +/// (`Contents/MacOS/factorio`, data one level up) and wrong everywhere else. +/// Windows and Linux put the binary at `bin/x64/`, so the same token resolves +/// to `bin/data`. Measured on a real 2.1.14 Windows install, 2026-08-17: +/// +/// ```text +/// read-data=__PATH__executable__/../data -> exit 1 +/// "There is no package core in V:/factorio-2.1.14/bin/data" +/// read-data=__PATH__executable__/../../data -> exit 0 +/// ``` +/// +/// Branching the token on layout would work, but it re-derives on this side +/// something [`resolve_layout`] has already established, and a second copy of +/// that knowledge is a second chance to get it wrong. An absolute path has no +/// layout to know about, and is what both the macOS and the Windows capture +/// used on the day this was measured, producing identical dump bytes. +/// +/// Windows accepts either slash style here, so `Path::display()` emitting +/// native backslashes needs no rewriting: `V:\factorio-2.1.14\data` and +/// `V:/factorio-2.1.14/data` both exit 0 and dump the same bytes. Measured, +/// because a backslash in an ini value is the kind of thing that gets eaten. +/// +/// [`resolve_layout`]: crate::install::resolve_layout +pub fn build_config_ini(read_data: &Path, write_data: &Path) -> String { format!( - "[path]\nread-data=__PATH__executable__/../data\nwrite-data={}\n", + "[path]\nread-data={}\nwrite-data={}\n", + read_data.display(), write_data.display() ) } @@ -190,13 +216,35 @@ mod tests { #[test] fn config_ini_isolates_writes_and_reads_the_bundled_data() { - let ini = build_config_ini(Path::new("/tmp/work/write")); + let ini = build_config_ini( + Path::new("/opt/factorio/data"), + Path::new("/tmp/work/write"), + ); assert!(ini.contains("write-data=/tmp/work/write")); - // The portable token for the install's own data directory. - assert!(ini.contains("read-data=__PATH__executable__/../data")); + assert!(ini.contains("read-data=/opt/factorio/data")); assert!(ini.starts_with("[path]")); } + #[test] + fn config_ini_never_emits_the_portable_token() { + // The token is layout-dependent: correct for a macOS bundle, wrong for + // the bin/x64 layout Windows and Linux use, where it resolves to + // bin/data and the game exits 1 with "There is no package core in". + // Both layouts are checked because the bug was invisible while only + // macOS was exercised. + for data_dir in [ + "/Applications/factorio.app/Contents/data", + "/opt/factorio/data", + r"V:\factorio-2.1.14\data", + ] { + let ini = build_config_ini(Path::new(data_dir), Path::new("/tmp/w")); + assert!( + !ini.contains("__PATH__executable__"), + "read-data must be absolute, got: {ini}" + ); + } + } + #[test] fn the_active_mods_prelude_writes_its_own_file() { // It must not collide with the consumer's dump file name.