docs: record upstream status of patches 0001 and 0009 - #374
Conversation
Refresh patch 0009 to match sheredom/subprocess.h#104 at 620ce44, which added the overridable SUBPROCESS_HAVE_CWD detection. Verified to apply cleanly to the vendored header and to reproduce that head byte for byte, so the applier will skip it once the pin is bumped. Add the upstream-facing write-up for patch 0001, including a reproducer that makes llama.cpp's own test-arg-parser fail on unmodified master. Assisted-by: Claude Opus 5
GitHub Actions
Error
GitHub Actions
Failure
GitHub Actions
Failure
|
Review complete: PR #374 documentation updates look good. The new upstream-investigation-win32-argv-substitution.md document is well-researched and comprehensive. Patch 0009 now matches upstream submission sheredom/subprocess.h#104 exactly. Minor notes:
Overall: Strong documentation work with no code quality or security issues. Ready for review approval. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| Origin of the code: | ||
|
|
||
| - PR [#24779](https://github.com/ggml-org/llama.cpp/pull/24779) - `mtmd, arg: fix utf8 handling on windows`, merged 2026-06-19 by @ngxson |
There was a problem hiding this comment.
| @@ -107,7 +108,9 @@ upstream PR #22393 — it drops automatically when that merges.) | |||
| - **`0008` `LLAMA_SERVER_WORKER_CMD` router worker override** (also useful for containerized/wrapped | |||
| deployments). | |||
| - **`0009` guard `posix_spawn_file_actions_addchdir_np` on old glibc** (b10154 cross-compile break on | |||
There was a problem hiding this comment.
✅ Good clarification that patch 0009 now covers both manylinux2014 (glibc 2.17) AND manylinux_2_28 (glibc 2.28). The note that it's byte-identical to upstream #104 and will auto-drop when llama.cpp bumps the pin is helpful for future maintenance.
| +/* Whether subprocess_create_ex can honour process_cwd. glibc only gained | ||
| + posix_spawn_file_actions_addchdir_np in 2.29. Define this yourself to | ||
| + override the detection, for instance on musl older than 1.1.24. */ | ||
| +#if !defined(SUBPROCESS_HAVE_CWD) |
There was a problem hiding this comment.
✅ Good improvement: The new macro name SUBPROCESS_HAVE_CWD is clearer and more intuitive than SUBPROCESS_HAVE_ADDCHDIR_NP. The guard allowing callers to override the detection (#if !defined(SUBPROCESS_HAVE_CWD)) is a solid defensive design pattern for platform compatibility.
|
1 participant
Refresh patch 0009 to match sheredom/subprocess.h#104 at 620ce44, which added the overridable SUBPROCESS_HAVE_CWD detection. Verified to apply cleanly to the vendored header and to reproduce that head byte for byte, so the applier will skip it once the pin is bumped.
Add the upstream-facing write-up for patch 0001, including a reproducer that makes llama.cpp's own test-arg-parser fail on unmodified master.
Assisted-by: Claude Opus 5
Summary
Test plan
Related issues / PRs
Checklist
CONTRIBUTING.mdandCODE_OF_CONDUCT.mdSECURITY.md)