fix(test): make test_stops build on Windows (mkdtemp compat shim)#352
Open
woolcoxm wants to merge 1 commit into
Open
fix(test): make test_stops build on Windows (mkdtemp compat shim)#352woolcoxm wants to merge 1 commit into
woolcoxm wants to merge 1 commit into
Conversation
test_stops.c uses POSIX mkdtemp() to make a scratch dir in the CWD, but
MinGW-w64 does not declare mkdtemp, so the test failed to compile on the
Windows job - and only there:
tests/test_stops.c:73:9: error: implicit declaration of function 'mkdtemp';
did you mean 'mktemp'? [-Wimplicit-function-declaration]
make: *** [Makefile:318: tests/test_stops.exe] Error 1
That halted `make test` at the C-test stage on Windows (test_uring is
correctly Linux-only, so test_stops was the only blocker).
Added a compat_mkdtemp shim to compat.h, following the file's existing
convention (every platform difference lives there; the .c stays clean):
_mktemp fills the trailing X's in place (same contract as mkdtemp), then
_mkdir creates the directory. Also added <direct.h> for _mkdir. On Linux
compat.h is a complete no-op, so POSIX mkdtemp is untouched there.
Verified: test_stops builds clean on MinGW and all 5 sub-cases pass:
tokenizer special-flag parsing, config/generation_config eos union,
no-generation_config fallback, both-configs-mutilated tokenizer sweep,
and the T=NULL validation path.
This was referenced Jul 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
make testhalted on the Windows job:tests/test_stops.cuses POSIXmkdtemp(), which MinGW-w64 doesn't declare, so the test failed to compile on Windows (and only there).This blocked the entire C-test phase on Windows. (
test_uringis correctly gated to Linux viaifneq (,$(LINUX)), sotest_stopswas the only Windows blocker.)Fix
Added a
compat_mkdtempshim toc/compat.h, following the file's stated convention — "every platform difference lives here; the.cfiles stay clean." POSIXmkdtemptakes achar[]template ending inXXXXXX, replaces the X's randomly, mkdirs0700, and returns the pointer (or NULL). On Windows:_mktemp(CRT,<io.h>) has the sameXXXXXXcontract;_mkdirneeded<direct.h>, added. On Linuxcompat.his a complete no-op — POSIXmkdtempis untouched.Why a shim and not a test-local fix
The earlier fix in
test_stops.c(commit on this file) already moved off/tmp/...to a CWD-relative path to fix the Windows job, butmkdtempitself was still POSIX-only. Acompat.hshim is the project's established pattern for exactly this (seegetline,setenv,posix_fadvise,pread,rename...) and keeps the test portable without#ifdef _WIN32inside the test source.Verification
On MinGW:
The two unrelated
-Wallwarnings that still show in this branch's build (compat.h:241pragma,glm.c:1210g_numa_nodes) are addressed by #350 — this PR is based onorigin/devand is independent of it.