From 86c0fe4199a8676d7eae467b23286dd44aacbaf1 Mon Sep 17 00:00:00 2001 From: chrg6 Date: Tue, 10 Nov 2026 12:47:22 -0500 Subject: [PATCH] Let a game build on a network share get past a short dropout and a share that refuses private folders. A player who keeps the game on a mapped network drive could not finish a build. Three things change in tools/make_game.py: - The output folder keeps its drive letter on Windows. It was resolved to the share's UNC name, which is a second connection to the same server, and some servers drop that one under the compilers' load while the mapped drive stays up. - When a source's size cannot be read because its folder stopped answering, the build says so and asks again for up to a minute before it stops with a message that names the drive. It used to end in a traceback. A file that is gone from a folder that answers still ends the build at once. - A finished file is put in place through a temporary file beside it instead of a file in a temporary folder. Such a folder is made for its owner alone, and some shares refuse that while they take ordinary files; the build then reported a permissions fault in the output folder after everything had compiled. From pull request #419: the first and third change are the contributor's, the wait is written again so that a file that is really gone does not wait a minute, and the launcher tests cover all three. Co-Authored-By: Matthias Co-Authored-By: Claude Fable 5.2 --- tests/launcher_test.py | 76 ++++++++++++++++++++++++++++++++++++++++++ tools/make_game.py | 59 ++++++++++++++++++++++++++++---- 2 files changed, 128 insertions(+), 7 deletions(-) diff --git a/tests/launcher_test.py b/tests/launcher_test.py index 92837ed8..915ee895 100644 --- a/tests/launcher_test.py +++ b/tests/launcher_test.py @@ -1685,6 +1685,82 @@ class MakeGameTest(Tmp): self.assertEqual(dst.stat().st_mode & 0o777, src.stat().st_mode & 0o777) self.assertFalse(self.mg.copy_if_changed(src, dst)) + def test_copy_if_changed_makes_no_folder(self): + """A network share can refuse the owner-only folder TemporaryDirectory makes: the copy needs a file only, and + leaves nothing behind, also when the copy fails.""" + out = self.tmp / 'share' + out.mkdir() + src, dst = self.tmp / 'new.bin', out / 'installed.bin' + src.write_bytes(b'new') + dst.write_bytes(b'old') + refused = mock.patch.object(self.mg.tempfile, 'TemporaryDirectory', side_effect=PermissionError('refused')) + with refused, mock.patch.object(self.mg.os, 'mkdir', side_effect=PermissionError('refused')): + self.assertTrue(self.mg.copy_if_changed(src, dst)) + self.assertEqual(dst.read_bytes(), b'new') + self.assertEqual([p.name for p in out.iterdir()], ['installed.bin']) + src.write_bytes(b'newer') + with mock.patch.object(self.mg.shutil, 'copy2', side_effect=OSError('the share went away')): + with self.assertRaises(OSError): + self.mg.copy_if_changed(src, dst) + self.assertEqual(dst.read_bytes(), b'new') # the installed file is as it was + self.assertEqual([p.name for p in out.iterdir()], ['installed.bin']) + + def test_source_size_waits_for_a_share(self): + """A share that stops answering for a moment: the source's folder is not there either, and the size is asked + again. A folder that answers without the file ends the build at once.""" + src = self.tmp / 'lift' / 'a.c' + src.parent.mkdir() + src.write_bytes(b'int a;\n') + real_stat, real_is_dir = Path.stat, Path.is_dir + down = {'left': 3} + + def stat(p, *a, **k): + if p == src and down['left'] > 0: + raise FileNotFoundError(2, 'The network path was not found', str(p)) + return real_stat(p, *a, **k) + + def is_dir(p): # each look at the folder is one try nearer to the share's return + if p == src.parent and down['left'] > 0: + down['left'] -= 1 + return False + return real_is_dir(p) + + lines = [] + with mock.patch.object(Path, 'stat', stat), mock.patch.object(Path, 'is_dir', is_dir), \ + mock.patch.object(self.mg.time, 'sleep') as sleep, mock.patch.object(self.mg, 'say', lines.append): + self.assertEqual(self.mg.source_size(src), 7) + self.assertEqual(sleep.call_count, 3) # three tries found the share down + self.assertEqual(len(lines), 1) # said once, not at every try + # the share never comes back: the build stops after SOURCE_WAIT and names the drive + down['left'] = 10 ** 6 + clock = iter(range(0, 10 ** 6, 5)) + with mock.patch.object(Path, 'stat', stat), mock.patch.object(Path, 'is_dir', is_dir), \ + mock.patch.object(self.mg.time, 'sleep'), mock.patch.object(self.mg, 'say'), \ + mock.patch.object(self.mg.time, 'monotonic', lambda: next(clock)), \ + mock.patch.object(self.mg, 'fail', side_effect=SystemExit) as fail: + with self.assertRaises(SystemExit): + self.mg.source_size(src) + self.assertIn('stopped answering', fail.call_args[0][0]) + # the file is gone and its folder answers: no wait + down['left'] = 0 + src.unlink() + with mock.patch.object(self.mg.time, 'sleep') as sleep, \ + mock.patch.object(self.mg, 'fail', side_effect=SystemExit) as fail: + with self.assertRaises(SystemExit): + self.mg.source_size(src) + sleep.assert_not_called() + self.assertIn('lift folder', fail.call_args[0][0]) + + def test_out_path_keeps_a_drive_letter(self): + """Windows: --out is made absolute without resolve(), which would turn a mapped drive into its UNC name.""" + with mock.patch.object(self.mg, 'WIN', True), \ + mock.patch.object(Path, 'resolve', side_effect=AssertionError('resolve() was called')): + got = self.mg.out_path('game') + self.assertEqual(got, Path(os.path.abspath('game'))) + self.assertTrue(got.is_absolute()) + with mock.patch.object(self.mg, 'WIN', False): + self.assertEqual(self.mg.out_path(self.tmp / 'x' / '..' / 'game'), (self.tmp / 'game').resolve()) + @unittest.skipUnless(shutil.which('cc'), 'needs a C compiler') def test_object_cache(self): src = self.tmp / 'src' diff --git a/tools/make_game.py b/tools/make_game.py index 12d2476c..49cecb37 100644 --- a/tools/make_game.py +++ b/tools/make_game.py @@ -534,7 +534,9 @@ def pe_imports(path): def copy_if_changed(src, dst): """Atomically replace dst unless its contents match, even when source modification times are normalized. - A same-directory temporary copy preserves modes, replaces read-only files on POSIX and leaves dst intact on copy failure.""" + A same-directory temporary copy preserves modes, replaces read-only files on POSIX and leaves dst intact on copy + failure. The copy is a temporary file, not a file in a temporary folder: such a folder is made for its owner alone, + and some network shares refuse that while they take ordinary files.""" dst = Path(dst) s = os.stat(src) try: @@ -543,13 +545,21 @@ def copy_if_changed(src, dst): return False except FileNotFoundError: pass + replacement = None try: - with tempfile.TemporaryDirectory(dir=dst.parent, prefix='.nx2navi-copy-') as tmp: - replacement = Path(tmp) / dst.name - shutil.copy2(src, replacement) - os.replace(replacement, dst) + fd, name = tempfile.mkstemp(dir=dst.parent, prefix='.nx2navi-copy-') + os.close(fd) + replacement = Path(name) + shutil.copy2(src, replacement) + os.replace(replacement, dst) except PermissionError: fail(f'cannot replace {dst}: close the game and check output-directory permissions') + finally: + if replacement is not None: # still there only when the copy or the replace failed + try: + replacement.unlink(missing_ok=True) + except OSError: + pass return True @@ -832,6 +842,34 @@ def earlier_kits(): return sorted(kits) +SOURCE_WAIT, SOURCE_PAUSE = 60, 2 # seconds source_size waits for a drive in all, and between two tries + + +def source_size(src): + """The size of a source file about to be compiled. The sources sit in the build cache beside the game, which a + player may keep on a network share, and a share that loses its connection under the build's load answers 'not there' + until it is back: while the source's folder does not answer either, the question is asked again for up to + SOURCE_WAIT seconds. A folder that answers without the file means the file is gone, which no wait brings back.""" + src = Path(src) + end = time.monotonic() + SOURCE_WAIT + told = False + while True: + try: + return src.stat().st_size + except OSError as e: + away = not src.parent.is_dir() + if not away or time.monotonic() >= end: + fail(f'cannot read {src} ({e.strerror or e}): ' + + ('the drive or network share with the game folder stopped answering; check it and run the ' + 'build again' if away else + 'the file is gone from the build cache; delete the lift folder in the build cache and run ' + 'the build again, which makes it anew')) + if not told: + say(f' {src.parent} does not answer; waiting up to {SOURCE_WAIT} s for the drive to come back') + told = True + time.sleep(SOURCE_PAUSE) + + def compile_all(srcs, cmd_for, cache, jobs, env, progress=None, salt='', stop=None, alts=None): """Compile each (source, object) whose object is not current in cache. cmd_for(src, dst) -> argv without the dependency-file options; salt identifies the compiler. Returns (compiled, reused, failures) with failures as @@ -853,7 +891,7 @@ def compile_all(srcs, cmd_for, cache, jobs, env, progress=None, salt='', stop=No reused = len(srcs) - len(todo) if adopted: say(f' {adopted} objects an earlier version compiled are kept: the same compile of the same sources') - todo.sort(key=lambda job: job[0].stat().st_size, reverse=True) + todo.sort(key=lambda job: source_size(job[0]), reverse=True) def cc(job): src, dst = job @@ -1774,6 +1812,13 @@ def step_key(step, identity, out=None): f'|{sha256_file(xlate_source() / "nx2navi_xlate.id")}') +def out_path(arg): + """--out as an absolute path. On Windows a mapped drive letter stays as given: resolve() would name the folder by + its server and share instead (a UNC path), which is a second connection to the same share, and some servers drop + that one under the compilers' load while the mapped drive stays up.""" + return Path(os.path.abspath(arg)) if WIN else Path(arg).resolve() + + def main(): global EVENTS ap = argparse.ArgumentParser(description=__doc__.splitlines()[0]) @@ -1804,7 +1849,7 @@ def main(): ap.error('give --nup, both --exefs and --romfs, or --rebuild') if a.rebuild and not a.out: ap.error('--rebuild needs --out') - out = Path(a.out).resolve() if a.out else None + out = out_path(a.out) if a.out else None cache = out / 'build-cache' if out else None state_path = cache / 'state.json' if cache else None state = load_state(state_path) if state_path else {}