fix(build): link when the build files folder's path has a comma #2137

Merged
mooki000 merged 1 commit from link-path-comma into main 2026-11-11 21:29:48 +00:00
mooki000 commented 2026-11-11 04:39:48 +00:00 (Migrated from github.com)

A build stops at the link, after the whole compile, when the folder chosen for the build files has a comma in its path
(for example D:\Games, Zelda picked in the builder, or --work): the compiler splits the linker options that name the
link map and the link order at that comma. A failed link also makes the next build translate and compile everything
again (make_game.py), and it fails the same way.

Fixes #2257.

What changed

  • nx2/build.py: linker() passes linker options each after -Xlinker, as one argument (with llvm-mingw as
    -Xlink=<option>, as toolchain.link_opt does). It is used for the options that name a file in the build files
    folder, which went through -Wl, (directly on Linux, through toolchain.link_opt on Windows):

    • the link map of the game (-Map= on Linux, /map: on Windows), of the Vulkan back end, and of --dll;
    • the link order, hot_order.txt in that folder (--symbol-ordering-file= on Linux, /order:@ on Windows).

    The other linker options are constants and stay as they are.

  • tests/test_link_path_comma.py (new, game-free):

    • no -Wl, option in build.py carries anything but a constant, and toolchain.link_opt is given only constants;
    • what linker() and HOT_ORDER() give for Linux, the MSVC target and llvm-mingw, with a build files folder named
      Games, Zelda;
    • where clang and LLVM's lld are installed: a small program linked into that folder with these options, through ld.lld,
      lld-link and the MinGW driver, writes its map there. Without lld that part skips itself (CI's Ubuntu has clang but
      perhaps not lld); the first two parts need nothing.

How it was tested

Linux (Arch, clang and lld 23.1.3), no GPU involved:

  • nx2/build.py --standalone with OOT_WORK set to a folder named x,y (a copy of a full build's files from main, so
    only the files changed since were compiled). On main (fab8cb3) the link stops:

    link FAILED (0s), 0 undefined symbols
    ld.lld: error: cannot open <build files folder>/x: No such file or directory
    ld.lld: error: cannot open y/sa/build/hookshot.map: No such file or directory
    ld.lld: error: cannot open y/hot_order.txt: No such file or directory
    

    With this change both links pass and write hookshot.map and hookshot_vk.so.map in x,y. All 3682 functions of the
    link order are in the new map at the same addresses as in main's own build (built in a folder with no comma), and in
    the list's order: the order still takes effect through -Xlinker.

  • The Windows options, linked from Linux with the same clang for both Windows targets: a small program linked into a
    folder named a,b with the old options fails (lld-link: error: could not open 'b/hookshot.map'; llvm-mingw's ld.lld
    gives ld.lld: error: could not open 'b/hookshot.map'; /order:@ the same with b/hot_order.txt). With -Xlinker it
    links and writes the map.

  • The compile options that also carry paths in the build files folder (-I, -include, -fprofile-instr-use=) are not
    split at commas: a small compile with a profile in a,b passes.

  • The new test passes; run against main's build.py (--build-script) it fails 8 times, and names the 7 options
    that carried a path (build.py lines 136, 137, 1073, 1092, 1142, 1147, 1161 on main).

  • python -I -B tests/run_all.py: 362 passed, 2 failed, 10 skipped. The same 2 fail on main here, for
    reasons of this machine (tests/test_updater_asset_digest.py: dotnet cannot restore its packages offline;
    tests/test_wayland_libdecor_hint.py: clang 23's new -Wunused-but-set-global on its harness). Every other check
    has the same result as on main; the new one passes.

  • After a rebase onto current main (e0578ede) the new test was run again and passes (6 tests, with the three real
    links); the full set above was run on fab8cb3, before the rebase.

Not tested

  • A Windows build: there is no Windows here. The Windows options were checked only by linking for the two Windows targets
    from Linux with clang 23's lld-link and ld.lld (MinGW).
    2026-11-11: @Arkex-ai ran the new test's six cases on Windows with llvm-mingw's clang 23 and LLVM's lld, with the
    three real links, at 0d240b30 (see the comments). Still not run: a full Builder build on Windows.
  • A full make_game.py build into a folder with a comma: the lift and the full compile were not run there, only
    build.py's compile of the changed files and its two links.
  • The game was not started from these binaries.
  • macOS (#1872 is where its link lines come in; its -Wl,-map,<path> would have the same fault).

Assisted-by: Claude Opus 5.5
Co-Authored-By: Claude Opus 5.5 [email protected]

A build stops at the link, after the whole compile, when the folder chosen for the build files has a comma in its path (for example `D:\Games, Zelda` picked in the builder, or `--work`): the compiler splits the linker options that name the link map and the link order at that comma. A failed link also makes the next build translate and compile everything again (`make_game.py`), and it fails the same way. Fixes #2257. ## What changed - `nx2/build.py`: `linker()` passes linker options each after `-Xlinker`, as one argument (with llvm-mingw as `-Xlink=<option>`, as `toolchain.link_opt` does). It is used for the options that name a file in the build files folder, which went through `-Wl,` (directly on Linux, through `toolchain.link_opt` on Windows): - the link map of the game (`-Map=` on Linux, `/map:` on Windows), of the Vulkan back end, and of `--dll`; - the link order, `hot_order.txt` in that folder (`--symbol-ordering-file=` on Linux, `/order:@` on Windows). The other linker options are constants and stay as they are. - `tests/test_link_path_comma.py` (new, game-free): - no `-Wl,` option in `build.py` carries anything but a constant, and `toolchain.link_opt` is given only constants; - what `linker()` and `HOT_ORDER()` give for Linux, the MSVC target and llvm-mingw, with a build files folder named `Games, Zelda`; - where clang and LLVM's lld are installed: a small program linked into that folder with these options, through ld.lld, lld-link and the MinGW driver, writes its map there. Without lld that part skips itself (CI's Ubuntu has clang but perhaps not lld); the first two parts need nothing. ## How it was tested Linux (Arch, clang and lld 23.1.3), no GPU involved: - `nx2/build.py --standalone` with `OOT_WORK` set to a folder named `x,y` (a copy of a full build's files from main, so only the files changed since were compiled). On main (fab8cb3) the link stops: ``` link FAILED (0s), 0 undefined symbols ld.lld: error: cannot open <build files folder>/x: No such file or directory ld.lld: error: cannot open y/sa/build/hookshot.map: No such file or directory ld.lld: error: cannot open y/hot_order.txt: No such file or directory ``` With this change both links pass and write `hookshot.map` and `hookshot_vk.so.map` in `x,y`. All 3682 functions of the link order are in the new map at the same addresses as in main's own build (built in a folder with no comma), and in the list's order: the order still takes effect through `-Xlinker`. - The Windows options, linked from Linux with the same clang for both Windows targets: a small program linked into a folder named `a,b` with the old options fails (`lld-link: error: could not open 'b/hookshot.map'`; llvm-mingw's ld.lld gives `ld.lld: error: could not open 'b/hookshot.map'`; `/order:@` the same with `b/hot_order.txt`). With `-Xlinker` it links and writes the map. - The compile options that also carry paths in the build files folder (`-I`, `-include`, `-fprofile-instr-use=`) are not split at commas: a small compile with a profile in `a,b` passes. - The new test passes; run against main's `build.py` (`--build-script`) it fails 8 times, and names the 7 options that carried a path (build.py lines 136, 137, 1073, 1092, 1142, 1147, 1161 on main). - `python -I -B tests/run_all.py`: 362 passed, 2 failed, 10 skipped. The same 2 fail on main here, for reasons of this machine (`tests/test_updater_asset_digest.py`: dotnet cannot restore its packages offline; `tests/test_wayland_libdecor_hint.py`: clang 23's new `-Wunused-but-set-global` on its harness). Every other check has the same result as on main; the new one passes. - After a rebase onto current `main` (e0578ede) the new test was run again and passes (6 tests, with the three real links); the full set above was run on fab8cb3, before the rebase. ## Not tested - A Windows build: there is no Windows here. The Windows options were checked only by linking for the two Windows targets from Linux with clang 23's lld-link and ld.lld (MinGW). 2026-11-11: @Arkex-ai ran the new test's six cases on Windows with llvm-mingw's clang 23 and LLVM's lld, with the three real links, at `0d240b30` (see the comments). Still not run: a full Builder build on Windows. - A full `make_game.py` build into a folder with a comma: the lift and the full compile were not run there, only `build.py`'s compile of the changed files and its two links. - The game was not started from these binaries. - macOS (#1872 is where its link lines come in; its `-Wl,-map,<path>` would have the same fault). Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <[email protected]>
Arkex-ai commented 2026-11-11 13:16:09 +00:00 (Migrated from github.com)

Additional Windows validation at 0d240b30: all six tests pass with llvm-mingw clang23 and LLVM lld, including the real no-CRT links for the ELF, MSVC and MinGW routes. Each writes its map in Games, Zelda; the order file is supplied too.

I also reproduced the original failure on main 49f75dc8 with separate authored programs on Windows and Linux: both the map path and order-file path break at a comma, while the same paths with spaces alone link successfully. This looks relevant to the new report #2257.

These are game-free linker fixtures, not a full make_game.py build or game runtime test.

Additional Windows validation at `0d240b30`: all six tests pass with llvm-mingw clang23 and LLVM lld, including the real no-CRT links for the ELF, MSVC and MinGW routes. Each writes its map in `Games, Zelda`; the order file is supplied too. I also reproduced the original failure on main `49f75dc8` with separate authored programs on Windows and Linux: both the map path and order-file path break at a comma, while the same paths with spaces alone link successfully. This looks relevant to the new report #2257. These are game-free linker fixtures, not a full `make_game.py` build or game runtime test.
mooki000 commented 2026-11-11 13:19:56 +00:00 (Migrated from github.com)

Thanks for running it on Windows: that was the part I could not do here.

And yes, #2257 is this fault: its two error lines are the map's path and hot_order.txt's, each beginning right after the comma in the reporter's folder name. I've added Fixes #2257 and a note about your run to the description, and told the reporter the way around it until this is merged (a build files folder with no comma in its path).

Thanks for running it on Windows: that was the part I could not do here. And yes, #2257 is this fault: its two error lines are the map's path and `hot_order.txt`'s, each beginning right after the comma in the reporter's folder name. I've added `Fixes #2257` and a note about your run to the description, and told the reporter the way around it until this is merged (a build files folder with no comma in its path).
mooki000 commented 2026-11-11 17:08:02 +00:00 (Migrated from github.com)

Rebased onto main b29967bf so the workflows run here: they had waited for an approval on the old head, and main's game-free tests are green again since #2380. The patch is unchanged (git range-diff: the commit is equal), and its test passes on the new base.

Rebased onto main `b29967bf` so the workflows run here: they had waited for an approval on the old head, and main's game-free tests are green again since #2380. The patch is unchanged (`git range-diff`: the commit is equal), and its test passes on the new base.
hckrman101 (Migrated from github.com) approved these changes 2026-11-11 17:34:46 +00:00
hckrman101 (Migrated from github.com) left a comment

Looks good. The cause is right: clang splits -Wl, at every comma, and the map and hot_order.txt paths sit in the user-chosen build files folder. I checked every option in the diff that carries a path and each now goes through -Xlinker as its own argv entry. The comma is the only character that clang's own -Wl, splitting treats specially, so spaces and non-ASCII (UTF-8) paths are fine, since this is a list-form subprocess.run, not a shell string. The @rsp files are not touched by this change. The remaining -Wl, options are constants (--gc-sections, -Bsymbolic, --export-dynamic-symbol=hos_posix_*), and the test guards that with the AST check.

The llvm-mingw route (-Xlinker -Xlink=/map:...) matches what toolchain.link_opt already emitted, minus the -Wl, wrapper. Arkex-ai's Windows run with llvm-mingw clang 23 and lld, including the real links, confirms it. That was the one thing I could not check from here. The test is game-free, needs no dump, and skips the real links when lld is absent.

CI: the two red jobs look unrelated to this change. tests/test_native_astc_texture_count.py on windows-2022 fails with 'scanf' is deprecated (-Werror) in its own fixture, and appimage fails in its System packages step on network timeouts. Both are in files this PR does not touch, but check that main's latest run shows the same. The Conventional Commit title check was still pending when I looked.

Nit, not blocking: the linker() docstring is long for a three-line helper. It is fine to keep as is.

Looks good. The cause is right: clang splits `-Wl,` at every comma, and the map and `hot_order.txt` paths sit in the user-chosen build files folder. I checked every option in the diff that carries a path and each now goes through `-Xlinker` as its own argv entry. The comma is the only character that clang's own `-Wl,` splitting treats specially, so spaces and non-ASCII (UTF-8) paths are fine, since this is a list-form `subprocess.run`, not a shell string. The `@rsp` files are not touched by this change. The remaining `-Wl,` options are constants (`--gc-sections`, `-Bsymbolic`, `--export-dynamic-symbol=hos_posix_*`), and the test guards that with the AST check. The llvm-mingw route (`-Xlinker -Xlink=/map:...`) matches what `toolchain.link_opt` already emitted, minus the `-Wl,` wrapper. Arkex-ai's Windows run with llvm-mingw clang 23 and lld, including the real links, confirms it. That was the one thing I could not check from here. The test is game-free, needs no dump, and skips the real links when lld is absent. CI: the two red jobs look unrelated to this change. `tests/test_native_astc_texture_count.py` on windows-2022 fails with `'scanf' is deprecated` (-Werror) in its own fixture, and `appimage` fails in its System packages step on network timeouts. Both are in files this PR does not touch, but check that main's latest run shows the same. The Conventional Commit title check was still pending when I looked. Nit, not blocking: the `linker()` docstring is long for a three-line helper. It is fine to keep as is.
Joey-Magnus (Migrated from github.com) approved these changes 2026-11-11 21:22:42 +00:00
Joey-Magnus (Migrated from github.com) left a comment

Paths go to the linker through -Xlinker (and -Xlink= for llvm-mingw) instead of -Wl,, which splits on commas. In the merged result I found no other -Wl, or link_opt carrying a path. The red checks are main's old ASTC test failure (#2397) and a Launchpad PPA timeout on appimage, neither from this PR.

Paths go to the linker through `-Xlinker` (and `-Xlink=` for llvm-mingw) instead of `-Wl,`, which splits on commas. In the merged result I found no other `-Wl,` or `link_opt` carrying a path. The red checks are main's old ASTC test failure (#2397) and a Launchpad PPA timeout on appimage, neither from this PR.
Commenting is not possible because the repository is archived.
No description provided.