WIP ci: fix the Windows "Install GMT" step in tests.yml and docs.yml - #9134
Open
Esteban82 wants to merge 3 commits into
Open
WIP ci: fix the Windows "Install GMT" step in tests.yml and docs.yml#9134Esteban82 wants to merge 3 commits into
Esteban82 wants to merge 3 commits into
Conversation
PR #9097 fixed the Windows install step in build.yml but the same fix was never applied to tests.yml and docs.yml, so the Windows jobs of both workflows have been failing at "Install GMT" ever since. The step ran `cmake --build . --target install` with the default `bash -el {0}` shell and no OS guard, so on Windows it executed outside the MSVC environment set up by vcvars64.bat. It also inherited INSTALLDIR with backslashes, which CMake treats as escape sequences. Consequences on Windows: - tests.yml: "Run full tests" and "Run DOS batch examples" were reported as `skipped` in every recent run, i.e. the Windows test suite has not been executed at all while still reporting a red job. - docs.yml: the Windows job failed before packaging. Apply the same fix as build.yml: move the install into the existing `cmd` step that already sets up vcvars64 (normalising INSTALLDIR and the vcpkg root along the way), and guard the Unix install step with `if: runner.os != 'Windows'`. The Windows step bodies are now identical across build.yml, tests.yml and docs.yml. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Esteban82
added
the
AI-assisted
Esteban82
changed the title
With the Windows install step fixed, the Windows job now reaches
"Check a few simple commands" for the first time and crashes immediately on
the bare `gmt` call:
+ gmt
*** stack smashing detected ***: terminated
The cause is the shell, not GMT. The workflow default is `bash -el {0}`, and
the login shell sources Git Bash's /etc/profile, which puts /usr/bin and
/mingw64/bin ahead of the vcpkg directories in PATH. gmt.exe then resolves its
netCDF/GDAL/curl DLLs to the MSYS2/MinGW copies instead of the vcpkg ones it
was linked against, and the ABI mismatch corrupts the stack.
build.yml runs the same script against the same binary with
`shell: bash` (i.e. `--noprofile --norc`) and passes, printing the splash
screen normally.
Split the step so Windows uses the same non-login shell as build.yml while
Linux and macOS keep the login shell they need for conda. Doing this per-OS
rather than globally is deliberate: macOS currently passes this step in
tests.yml, and build.yml's macOS job -- which does use `shell: bash` for it --
fails later in `gmt end` with `gmtinit_process_figures returned error 79`, so
that shell is not safe to adopt for every platform here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit's theory was wrong: giving Windows the same non-login
`shell: bash` and `export PATH` that build.yml uses did not help, and gmt.exe
still dies on the bare `gmt` call:
+ gmt
*** stack smashing detected ***: terminated
So the crash is not caused by the login shell. What does work, and is proven
green in build.yml, is running the checks from cmd via
ci/simple-gmt-tests.bat: the same binary prints its splash screen normally
there.
Restrict the bash check to Linux/macOS and let Windows use the cmd script
alone. This also removes a genuine duplication: until now Windows ran the
simple checks twice, once through simple-gmt-tests.sh and once through
simple-gmt-tests.bat.
`where gmt` is added to the cmd step so the log records which gmt.exe is
picked up, which is the first thing needed if this ever regresses.
If the cmd script also crashes, the problem is in the binary itself rather
than in the shell environment -- the Windows test build differs from the
build.yml one only by SUPPORT_EXEC_IN_BINARY_DIR and the DO_* test flags --
and it will need a fix in the C code rather than in CI.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
This PR fixes the Windows build problem in
tests.ymlanddocs.yml.It applies the same fix already merged for build.yml in #9097: the install moves into the cmd step that sources
vcvars64.bat, and the Unix install step is guarded withif: runner.os != 'Windows'.Assisted-by: Claude Opus 5 (extra effort)