Add a portability workflow beside the existing matrix - #113
Conversation
cmake.yml covers x86-64 Linux, Windows and macOS across four build types and
four compilers, and it covers them well. What it cannot reach is anything that
is not selected by the runner it happens to be on: the capability probes only
ever resolve one way per machine, every runner is 64-bit, and no BSD or non-x86
architecture is present at all.
The new workflow adds one job per dimension:
capability-paths force each combination of SUBPROCESS_HAVE_CWD,
SUBPROCESS_SPAWN_REPORTS_EXEC_ERRORS and
SUBPROCESS_SPAWN_VIA_FORK on an ordinary runner
linux-32-bit, windows-x86 ILP32, where uint64_t becomes unsigned long long
musl a second libc
foreign-arch ppc64le, riscv64, s390x, armv7 under qemu-user
bsd real FreeBSD, NetBSD 9.4 and 10.1, OpenBSD VMs
macos-deployment-target 10.14, 10.15 and 11.0, either side of the boundary
that SUBPROCESS_HAVE_CWD turns on
capability-paths is the cheapest and catches the most. The library selects its
implementation at compile time, so on any given runner the other paths are never
executed — including the ones added by sheredom#104 and sheredom#106, which as far as I can tell
had never actually run anywhere before, only compiled.
NetBSD is listed at both 9.4 and 10.1 on purpose: posix_spawn_file_actions_addchdir
arrived in 10.0 and is spelled without the _np suffix, so the two take different
paths through the header.
foreign-arch runs under qemu-user, which is faithful for instruction-set
semantics but not for posix_spawn's exec-error reporting — it does not propagate
the errno, and a create_ex failure there would be an artifact rather than a
platform property. That is noted in the file so nobody reads it the wrong way.
The first run of this workflow failed all four foreign-arch jobs, and for a
reason that has nothing to do with the architectures: qemu-user does not
propagate the errno that glibc's posix_spawn uses to report a failed exec, so
create_ex_subprocess_create_failure_preserves_error fails there regardless of
the target. s390x failed identically to ppc64le despite having no architectural
difference in play, which is what gives it away.
Running the whole suite under emulation therefore cannot go green, and a job
that is permanently red is worse than no job. These now build the whole suite —
which still covers compilation for each architecture — and run only the tests
whose outcome actually depends on the instruction set:
./subprocess_test '--filter=*divzero'
That is the reason the jobs exist: integer division by zero traps on x86, Arm
and s390x but not on PowerPC or RISC-V. The comment explains the narrowing so it
does not read as suppressing inconvenient failures.
Also noted in the file that the SUBPROCESS_SPAWN_VIA_FORK combination in
capability-paths is a no-op until sheredom#112 lands, so its green tick is not mistaken
for evidence today.
|
The red checks are the point rather than a defect in the workflow. Each one is a
I would rather leave them red than paper over them — they are exactly what the |
| cmake --build build -j | ||
| ctest --test-dir build --output-on-failure | ||
|
|
||
| # On ILP32, uint64_t is unsigned long long, and -Wpedantic then rejects both |
There was a problem hiding this comment.
LLM has put unwarranted context in the yml. Please do a pass to remove comments like these.
There was a problem hiding this comment.
Ty! I've improved, please check again.
Reviewer feedback on sheredom#113: the file carried explanatory prose that repeated what the job and matrix names state. The paragraphs above capability-paths, linux-32-bit, musl and macos-deployment-target are gone entirely — "no chdir file action, no exec error reporting" and "macOS deployment target 10.14" already say it. Two survive as single lines, both where deleting the comment would invite a wrong edit rather than merely lose background: foreign-arch why it runs only *divzero. Without it the narrowing looks like an oversight and the obvious "fix" is a permanently red job. bsd why NetBSD is listed twice, so the duplicate does not get tidied away and take the 9.x path with it. The note that SUBPROCESS_SPAWN_VIA_FORK is a no-op until sheredom#112 lands is dropped as well. It was accurate when written but describes a moment rather than the workflow, and it goes stale the day sheredom#112 merges. No job, step or matrix entry is touched: 7 jobs, 17 runs, before and after. The reasoning that left the file is preserved in the two commits that added it and in the PR description.
|
Here a thread where used It is not yet commited and it can be part of the next release: 8.0 |
Where all of this standssubprocess.h
utest.h
All three are on All seven are finished and need nothing further from me. I would rather stop Happy to keep helping — I have AIX access and the portability matrix set up, so |
@catap Thanks — I checked Until it ships, the behaviour in #112 is already correct for OpenBSD: it takes I would rather not gate on a release that does not carry it yet — that would be |
|
Your other PRs are merged. Time to update this to |
The old copy is a pristine snapshot of utest.h@f4610ee. Since then upstream fixed the ILP32 constants and the NetBSD PRIu64 problem (#188), the test filter dropping tests it should run (#190) and added AIX support (#189).
<sys/param.h>, included here since sheredom#112 for __NetBSD_Version__, pulls in <sys/inttypes.h>. That header gates <machine/int_fmtio.h> on __STDC_FORMAT_MACROS for C++ before C++11 and sits behind an include guard, so anything defining the macro afterwards cannot get PRId64 and PRIu64 back. Consumers including subprocess.h first lost them silently; the test suite's C++98 translation unit is one.
sheredom#105 added -std=c++2a for GCC 8 and clang 9, which do not know -std=c++20. GCC 7 and older know neither, so the fallback passed a flag those compilers reject and the build stopped before running anything. NetBSD 9.4 ships GCC 7.5.0 and hit exactly this. test20.cpp now drops the standard flag when neither spelling is available, which is what already happens for test11.c without c_std_11 and for MSVC, where no -std flag is passed at all. The suite then runs that translation unit at the compiler default rather than not at all. Measured in GCC 7.5.0: -std=c++20 and -std=c++2a are both rejected, and the file compiles clean at the default gnu++14 under the flags CMakeLists sets. GCC 13 still selects -std=c++20 and the suite stays at 444/444.
|
Very good work here! |
3 participants
cmake.ymlcovers x86-64 Linux, Windows and macOS across four build types andfour compilers, and covers them well. What no matrix over
runs-oncan reach isanything the runner does not select for you.
subprocess.h chooses its implementation from compile-time probes. On any given
machine exactly one combination is ever taken and the others are, at best,
compiled. The paths added by #104 and #106 have — as far as I can tell — never
been executed anywhere, only compiled. Every runner is also 64-bit, and no BSD
or non-x86 architecture appears at all.
This adds
portability.ymlnext tocmake.ymlrather than extending it, so theexisting matrix stays readable and either can be adjusted without disturbing the
other.
The red checks are the point
This PR is expected to be red, and every failure is a real defect on current
mainrather than a problem with the workflow. Where each one stands today:windows-x86subprocess_size_tis notSIZE_Ton Win32linux-32-bit,linux armv7utest.h's 64-bit constants needunsigned long longon ILP32, andPRIu64then expands to allmodifier that C++98 rejectsnetbsd 10.1PRIu64is undefined - NetBSD makes the PRI macros visible in C++ only with__STDC_FORMAT_MACROSor from C++11 onnetbsd 9.4posix_spawn_file_actions_addchdironly exists from NetBSD 10.0, but #102's guard has no version testopenbsd 7.9#elsesays it doeslinux ppc64le,linux riscv64subprocess_fail_divzeroasserts a trap that those architectures do not performlinux s390xpasses, which is the useful contrast for the last row: s390x doestrap on integer division by zero, so the same assertion is correct there and only
PowerPC and RISC-V need the guard.
windows-x86was not in this table when the PR was opened. It went red onthe merge that brought #116 in, days later, and #117 fixed it. A defect found
within a day by a job that exists only because of this workflow is the clearest
argument for it, so it belongs on the record here.
The three utest.h rows need one more step, and the earlier wording glossed
over it. This PR used to say that once sheredom/utest.h#188 landed those jobs
would go green. It has landed and they are still red, because the fix reaches
this repository only through the vendored
test/utest.h, which is still the oldcopy. Refreshing that file turns exactly those three green - measured on my fork
against a control branch differing in that one file, with the control
reproducing all eight failures job for job. That is routine vendor maintenance
and belongs in its own PR, not in this one.
What each job covers
capability-pathsSUBPROCESS_HAVE_CWD,SUBPROCESS_SPAWN_REPORTS_EXEC_ERRORS,SUBPROCESS_SPAWN_VIA_FORKlinux-32-bit,windows-x86uint64_tbecomesunsigned long longmuslforeign-archbsdmacos-deployment-targetWhy these, specifically
capability-pathsis the cheapest and catches the most. It is also theanswer to your question on Initial port for AIX #109 about testing for AIX: with Support AIX by forking where posix_spawn cannot change directory #112, the fork
implementation is selected by a macro, so
-DSUBPROCESS_SPAWN_VIA_FORK=1exercises it on an ordinary Ubuntu runner with no AIX in sight. That macro
does not exist yet, so that one combination is a no-op until Support AIX by forking where posix_spawn cannot change directory #112 lands; the
workflow says so in a comment rather than letting the green tick look like
evidence.
uint64_tisunsigned longon LP64 andunsigned long longon ILP32, and-Wpedanticrejects the latter in the C90and C++98 modes this suite compiles under. Nothing in CI is currently 32-bit.
posix_spawn_file_actions_addchdirarrived in 10.0 and is spelled without the_npsuffix, so the two releases take different paths through the header.SUBPROCESS_HAVE_CWDflips at 10.15, andbuilding only against the runner's own SDK never sees the older side.
Why the emulated jobs run only part of the suite
foreign-archbuilds everything but runs only--filter=*divzero.The first run of this workflow failed all four architectures on
create_ex_subprocess_create_failure_preserves_error, includings390x, whichhas no architectural difference in play at all. qemu-user does not propagate the
errno that glibc's
posix_spawnuses to report a failed exec — I confirmed thesame behaviour on emulated aarch64, where glibc 2.36 also fails to report, which
nobody would claim is a property of aarch64.
Running the whole suite under emulation therefore cannot go green regardless of
the code, and a permanently red job is worse than none. The build still covers
compilation for each architecture; only the process-creation tests are skipped,
and the file explains why so it does not read as suppressing inconvenient
failures.
Not included
interactive SSH rather than a CI runner.
Action versions were checked against their current documentation:
cross-platform-actions/action@v1.4.0anduraimo/run-on-arch-action@v3.Separately and unrelated:
cmake.ymlstill usesactions/checkout@v2, whichruns on a deprecated Node version. Happy to send that as its own one-line PR if
useful.