Repository navigation
ci: modernize the matrix, add a 32-bit Linux row, require Nim >= 2.0 - #90
Merged
Merged
Conversation
The CI config had drifted away from what it claimed to test: - `version-1-6` asserted nothing. nim-lang/nightlies stopped producing dated 1.6 builds and its `latest-version-1-6` alias has resolved to a 2024-07-10 binary ever since, so those rows exercised a two-year-old compiler. Drop them and bump the `.nimble` floor to `nim >= 2.0.0` to match; `version-2-0` is now the oldest row and holds that floor honest. - The macOS rows asked setup-nim for `amd64`, but `macos-latest` is arm64 — that installed an x64 Nim running under Rosetta. Now `cpu: arm64` on a pinned `macos-15`, as on the other pinned runners. - "Restore Nim from cache" was dead: it gated on its own step id (always empty, so the condition was always true) and cached `nim-<branch>-<cpu>`, a path setup-nim never writes — it unpacks into `nim/`. Removed. Adds the 32-bit Linux rows this repo has wanted since they were first commented out. There is no 32-bit runner, so the target is reached the way Nimony's CI reaches it: `gcc`/`g++` wrappers that prepend `-m32`, placed ahead of the real ones on PATH. gcc-multilib also pulls in the libc6-i386 the 32-bit Nim nightly needs in order to run at all. This is far cheaper coverage of the pointer-sized atomics and channel alignment assumptions than the windows-i386 row, which is the only place they were exercised before. tests/nim.cfg grows a matching branch: 32-bit x86 builds with gcc and without ThreadSanitizer. TSan ships no 32-bit x86 runtime, and clang is deliberately not wrapped — left on clang this row would have silently emitted 64-bit objects and failed at link time. Verified via --genScript that all three branches resolve as intended: amd64 -> clang + TSan, linux-i386 -> gcc without TSan, windows-i386 -> clang + -m32 without TSan. Also: `--gc:arc` -> `--mm:arc`, and the test step is plain `nimble test`. Passing `--threads:on --gc:arc` there only restated tests/nim.cfg and earned a deprecation warning on every single compile. actions/cache v3 -> v5, winlibs MinGW bumped to the GCC+LLVM bundle (nim.cfg uses clang on Windows too, so clang and ld.lld have to ship alongside gcc), plus concurrency cancellation for superseded runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
also a good time to remove the atomics module |
The two windows-i386 rows were the only red ones in the previous run, and
they never reached a compiler. `nimble test` refreshes the official
package list on a runner with no `~/.nimble` yet, and the 32-bit Windows
nimble binary segfaults doing it:
Downloading Official package list
SIGSEGV: Illegal storage access. (Attempt to read from nil?)
Making the test step a bare `nimble test` is what put that refresh on the
critical path. The crash is nimble's own, not this library's and not the
stdlib's: the same 32-bit Nim completes an HTTPS fetch of the same URL
through std/httpclient, a plain-http list URL crashes identically (so it
is not SSL), 64-bit nimble refreshes fine from the same machine, and
`--offline` does not skip the download. Compiling and running the tests
directly with the row's own toolchain passes 14/14, so nothing under
threading/ or tests/nim.cfg is at fault.
Writing the list ourselves leaves nimble nothing to refresh. It is taken
from the packages repo rather than packages.nim-lang.org, which answers
403 to anything that is not nimble; the result is checksum-identical to
what a working `nimble refresh` leaves behind.
Verified against the CI toolchain reproduced locally -- the windows_x32
devel and version-2-4 nightlies, the same winlibs i686 GCC+LLVM bundle
and windeps DLLs, with a pristine nimble home. Both rows segfault without
this step and report `Success: All tests passed` with it. On a 64-bit row
the step is inert: nimble skips the download and goes straight to
compiling.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
That would be a separate PR now the builds on master are green again. |
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.
The CI config had drifted away from what it claimed to test:
version-1-6asserted nothing. nim-lang/nightlies stopped producing dated 1.6 builds and itslatest-version-1-6alias has resolved to a 2024-07-10 binary ever since, so those rows exercised a two-year-old compiler. Drop them and bump the.nimblefloor tonim >= 2.0.0to match;version-2-0is now the oldest row and holds that floor honest.amd64, butmacos-latestis arm64 — that installed an x64 Nim running under Rosetta. Nowcpu: arm64on a pinnedmacos-15, as on the other pinned runners.nim-<branch>-<cpu>, a path setup-nim never writes — it unpacks intonim/. Removed.Adds the 32-bit Linux rows this repo has wanted since they were first commented out. There is no 32-bit runner, so the target is reached the way Nimony's CI reaches it:
gcc/g++wrappers that prepend-m32, placed ahead of the real ones on PATH. gcc-multilib also pulls in the libc6-i386 the 32-bit Nim nightly needs in order to run at all. This is far cheaper coverage of the pointer-sized atomics and channel alignment assumptions than the windows-i386 row, which is the only place they were exercised before.tests/nim.cfg grows a matching branch: 32-bit x86 builds with gcc and without ThreadSanitizer. TSan ships no 32-bit x86 runtime, and clang is deliberately not wrapped — left on clang this row would have silently emitted 64-bit objects and failed at link time. Verified via --genScript that all three branches resolve as intended: amd64 -> clang + TSan, linux-i386 -> gcc without TSan, windows-i386 -> clang + -m32 without TSan.
Also:
--gc:arc->--mm:arc, and the test step is plainnimble test. Passing--threads:on --gc:arcthere only restated tests/nim.cfg and earned a deprecation warning on every single compile. actions/cache v3 -> v5, winlibs MinGW bumped to the GCC+LLVM bundle (nim.cfg uses clang on Windows too, so clang and ld.lld have to ship alongside gcc), plus concurrency cancellation for superseded runs.