ci: pair macOS-amd64 tcc.exe rebuilds with a freshly-rebuilt, dynamically-linked libgc - #27982
Conversation
…ally-linked libgc thirdparty-macos-amd64_tcc.sh only ever rebuilds tcc.exe - it explicitly preserves whatever lib/libgc* was already committed, unchanged, on every automated update_tccbin.yml run. This produced a genuinely broken pair (vlang/tccbin commit d712d0f: new tcc.exe, old never-rebuilt libgc.a), which vlang/tccbin's CI correctly rejects - that check must stay blocking; this PR fixes the producer, not the check. A freshly-rebuilt STATIC libgc.a alone would not fix this either: vlang/tccbin PR vlang#74 already proved tcc can't reliably link a modern- toolchain-built static archive on macOS-amd64 (GC_init well-defined, archive well-formed, tcc still reports it unresolved) - the same limitation macOS-arm64 already solved via libgc.dylib + rpath. This PR: - Adds thirdparty-macos-amd64_bdwgc.sh, mirroring the established arm64 pattern, with amd64-specific corrections grounded in PR vlang#74's already-proven recipe (CC=clang, pinned MACOSX_DEPLOYMENT_TARGET, stale-artifact cleanup before staging, and explicit readlink/otool re-verification that libgc.dylib actually landed as a real symlink - confirmed via the GitHub API that the *existing* arm64 bundle's libgc.dylib is committed as a plain file, not a symlink, i.e. this exact failure mode has already happened once, silently). - Pins the deployment target on the tcc.exe half of the pair too, and records a toolchain-identity provenance file - neither half pinned this before, so a rebuilt binary silently inherited the CI runner's own floor. - Adds thirdparty-macos-amd64_bdwgc_validate.sh, porting the hardened dylib-blocking / no-GC-fallback / static-signature-XFAIL checks already proven in vlang/tccbin's build-and-test.yml, corrected to use V's actual full flag set (GC_THREADS, THREAD_LOCAL_ALLOC, not just the subset tccbin's own CI currently tests) and adapted to the current (not the old pinned) run.sh output format - the direct symbol/error-text checks stay authoritative either way. - Expands update_tccbin.yml's rebuild-skip decision, for macos-amd64 only, to fingerprint the whole pair (TinyCC + bdwgc + libatomic_ops SHAs, deployment target, toolchain identity, a bumpable recipe version, and required-file presence) rather than TinyCC alone - resolved SHAs are threaded through explicitly end to end, not re-resolved from a floating ref at build time. - After staging and committing the pair, clones that exact new commit into a genuinely independent, fresh checkout and re-validates against it - the check that actually catches "file exists in the build directory but was never git-added" (the same class of gap that produced arm64's non-symlinked libgc.dylib). - Builds V itself in a separate, freshly-cloned workspace for a real compile-and-execute smoke test (-cc tcc -gc boehm) against the staged bundle, deliberately never running `make` against the staged checkout itself - GNUmakefile's default target depends on latest_tcc, which runs `git clean -xf && git pull --rebase` inside thirdparty/tcc unless invoked with `local=1`, which would silently destroy the staged, uncommitted GC files before they're ever validated or committed. - Uploads the bundle as a tar (preserves symlinks/executable bits, unlike actions/upload-artifact on a raw directory) alongside the existing raw-directory upload. Does NOT include the vlib/builtin/builtin_d_gcboehm.c.v selector change (amd64 still selects libgc.a there) - that's a deliberately separate, later PR, held back until this pair is actually published and vlang/tccbin's checked-in-package gate (itself needing a coordinated follow-up: it currently hard-fails if that package unexpectedly starts passing, since today it's supposed to still be broken) is confirmed green under it. The smoke test above applies the selector collapse as an in-memory-only patch to the isolated workspace, never written back to this checkout or committed anywhere, specifically so this PR can validate the real end-to-end path without prematurely shipping the selector change itself. Not touched: macOS-arm64 has the identical latent gap (its update_tccbin.yml entry also never rebuilds GC) but is out of scope here. linux-amd64's _tcc.sh has the same preserve-libgc pattern but wasn't reported broken. No credential/push-repair changes.
|
Nice work — this matches the plan very closely. I only spotted three small items:
Once those are addressed and the macos-15-intel run is green, it looks good to me. Thanks! |
Reviewer feedback on vlang#27982: - thirdparty-macos-amd64_bdwgc.sh cloned libatomic_ops's default branch and recorded whatever HEAD it happened to land on, without ever checking out the SHA update_tccbin.yml had already resolved and fingerprinted moments earlier - the same class of drift risk already fixed for LIBGC_COMMIT, just missed for libatomic_ops. Added LIBATOMIC_OPS_COMMIT (passed through from the workflow's resolved steps.bdwgc.outputs.libatomic_ops_hash) and an explicit `git checkout` before recording its hash. - bundle_checksums.txt hashed lib/$libgc_dylib_target (the real versioned file) but not lib/libgc.dylib itself - the exact path V's own #flag references. shasum follows the symlink and produces the same hash either way, but recording it under the path that's actually used matters for anyone verifying the bundle. Also updated the PR's suggested first validation dispatch to include force_rebuild=true, since the current repo state has none of the new provenance files this fingerprint check would otherwise compare against - matching this same PR's own documented rule for the first repair run.
|
Thanks for the quick review! All three addressed in c493456:
Ready for the macos-15-intel validation run whenever you're able to dispatch it against a real |
|
Thanks, this is very close and it covers our plan well, including the three earlier points. I found three blocking workflow issues: the fresh-checkout validator prefixes an already absolute path with $PWD; the isolated V clone uses After that, I launch codex review. |
… SONAME deletions, publish lock - thirdparty-macos-amd64_bdwgc_validate.sh: don't blindly prefix $PWD onto TCC_FOLDER - one call site passes it relative (thirdparty/tcc), the other passes it already-absolute (/tmp/tccbin-fresh-checkout), and $PWD/<abs> collapsed into a nonexistent path for the latter. - update_tccbin.yml isolated-workspace step: use plain `make -j4`, not `make local=1` - this is a genuinely fresh clone with no vc/ or thirdparty/tcc yet (both gitignored), and `local=1` skips exactly the git-clone bootstrap this workspace needs, unlike bootstrapping_ci.yml where a prior plain `make` already populated vc/ first. - update_tccbin.yml commit step: git add -A against libgc*.dylib/libgc*.a globs instead of a hardcoded file list, so a SONAME bump's old versioned file (already rm -f'd from disk) gets its deletion staged too, instead of reappearing from history on every future checkout. - update_tccbin.yml: gate the macos-amd64 tccbin push on a repo variable (MACOS_AMD64_LIBGC_PUBLISH_UNLOCKED) in addition to PUBLISH, so neither the monthly cron nor a manual publish=true dispatch can push before vlang/tccbin's gate conversion (deliverable B) is confirmed live. Co-Authored-By: WOZCODE <contact@withwoz.com>
|
Thanks - all three fixed in
Also added the publish lock you flagged: the "Push tccbin branch" step now additionally requires a repo variable ( Ready for another look, and for the |
|
Good job! @codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Please run the native validation from a temporary branch on your fork. Adjust only the github.repository guard so the workflow can run there, keep publish=false and force_rebuild=true, and share the Actions run link with us. The required result is the macos-15-intel job. Thanks! |
|
Dispatched on the fork: https://github.com/quaesitor-scientiam/v/actions/runs/30544664222 (branch |
|
Run completed, all green: https://github.com/quaesitor-scientiam/v/actions/runs/30544664222
I'll delete the |
|
@medvednikov Alex, it's ready to be merged. Thanks :) |
…ass (#83) * ci: flip checked-in tcc.exe/libgc gate from must-stay-broken to must-pass Deliverable (B) of the macos-amd64 libgc-dylib-pairing plan, following vlang/v#27982 (deliverable A, merged): vlang/v's update_tccbin.yml now rebuilds libgc.dylib in lockstep with every tcc.exe rebuild for this platform, mirroring macos-arm64's already-proven dylib+rpath pattern, instead of silently preserving a stale libgc.a across every rebuild. The "verify the checked-in tcc.exe (as distributed, before rebuild)" step previously hard-failed if the checked-in pair unexpectedly passed the conformance suite, because until now it genuinely could not (ancient v0.9.27 tcc.exe, broken libc.dylib symlink, static-only libgc.a). That assumption no longer holds and would immediately misfire the moment a correct pair is published. This strengthens the gate rather than relaxing it: split into five steps mirroring the already-proven shape used below for the freshly-rebuilt copy - tcc.exe executable-bit check, independent libgc.dylib symlink/install-name re-verification, libgc.a archive validation (now fat-or-thin aware via `lipo -archs`, since a pair published through vlang/v's amd64-native rebuild is expected to be a thin x86_64-only archive, unlike the FAT universal binary this replaces), a blocking dylib lane, a blocking no-GC fallback lane, and a signature- checked XFAIL-only static lane - each one now required to actually PASS instead of required to fail a specific known way. Note: this PR's own CI run is expected to fail against the current tip of thirdparty-macos-amd64, since the checked-in pair hasn't been replaced by vlang/v#27982's producer-side fix yet (that publish is intentionally gated behind vlang/v's MACOS_AMD64_LIBGC_PUBLISH_UNLOCKED repo variable until this PR lands). Merging this PR is expected to keep CI red until the real publish happens next. Co-Authored-By: WOZCODE <contact@withwoz.com> * ci: make checked-in libgc.a archive check independent of the dylib check The libgc.dylib check's expected failure (pre-publish) was skipping this step entirely by default, so the new lipo-archs-based fat-or-thin detection never actually ran against the real checked-in archive in this PR's own validation run. It validates a wholly separate file (libgc.a, not libgc.dylib) - a dylib-check failure shouldn't hide whether the archive is also fine or also broken, mirroring the !cancelled() pattern already used below for the no-GC/static-XFAIL lanes. Co-Authored-By: WOZCODE <contact@withwoz.com> --------- Co-authored-by: Richard Wheeler <18647491+PythonWillRule@users.noreply.github.com> Co-authored-by: WOZCODE <contact@withwoz.com>
Problem
thirdparty-macos-amd64_tcc.shonly ever rebuildstcc.exe- it explicitly preserves whateverlib/libgc*was already committed, unchanged, on every automatedupdate_tccbin.ymlrun. This produced a genuinely broken pair (vlang/tccbincommitd712d0f: newtcc.exe, old never-rebuiltlibgc.a), whichvlang/tccbin's CI correctly rejects - that check must stay blocking; this PR fixes the producer, not the check.A freshly-rebuilt static
libgc.aalone would not fix this either:vlang/tccbinPR #74 already proved tcc can't reliably link a modern-toolchain-built static archive on macOS-amd64 (GC_initwell-defined, archive well-formed, tcc still reports it unresolved) - the same limitation macOS-arm64 already solved vialibgc.dylib+ rpath.What this PR does
thirdparty-macos-amd64_bdwgc.sh, mirroring the established arm64 pattern, with amd64-specific corrections grounded in PR suggestions for syntax & improvements #74's already-proven recipe (CC=clang, pinnedMACOSX_DEPLOYMENT_TARGET, stale-artifact cleanup before staging, and explicitreadlink/otoolre-verification thatlibgc.dylibactually landed as a real symlink - confirmed via the GitHub API that the existing arm64 bundle'slibgc.dylibis committed as a plain file, not a symlink, i.e. this exact failure mode has already happened once, silently).tcc.exehalf of the pair too, and records a toolchain-identity provenance file - neither half pinned this before.thirdparty-macos-amd64_bdwgc_validate.sh, porting the hardened dylib-blocking / no-GC-fallback / static-signature-XFAIL checks already proven invlang/tccbin'sbuild-and-test.yml, corrected to use V's actual full flag set (GC_THREADS,THREAD_LOCAL_ALLOC, not just the subset tccbin's own CI currently tests) and adapted to the currentrun.shoutput format.update_tccbin.yml's rebuild-skip decision, for macos-amd64 only, to fingerprint the whole pair (TinyCC + bdwgc + libatomic_ops SHAs, deployment target, toolchain identity, a bumpable recipe version, required-file presence) rather than TinyCC alone.git added" (the same class of gap that produced arm64's non-symlinkedlibgc.dylib).-cc tcc -gc boehm) against the staged bundle. Deliberately never runsmakeagainst the staged checkout itself:GNUmakefile's default target depends onlatest_tcc, which runsgit clean -xf && git pull --rebaseinsidethirdparty/tccunless invoked withlocal=1- this would silently destroy the staged, uncommitted GC files before they're ever validated or committed.actions/upload-artifacton a raw directory) alongside the existing raw-directory upload.What this PR deliberately does NOT do
Does not include the
vlib/builtin/builtin_d_gcboehm.c.vselector change (amd64 still selectslibgc.athere). That's a separate, later PR, held back until this pair is actually published andvlang/tccbin's checked-in-package gate (itself needing a coordinated follow-up: it currently hard-fails if that package unexpectedly starts passing, since today it's supposed to still be broken) is confirmed green under it. The smoke test above applies the selector collapse as an in-memory-only patch to the isolated workspace, never written back to this checkout or committed anywhere, specifically so this PR can validate the real end-to-end path without prematurely shipping the selector change.Not touched: macOS-arm64 has the identical latent gap (its
update_tccbin.ymlentry also never rebuilds GC) but is out of scope here.linux-amd64's_tcc.shhas the same preserve-libgc pattern but wasn't reported broken. No credential/push-repair changes.Verification plan
This needs to run on a real
macos-15-intelGitHub Actions runner to validate - I can't do that from here. Suggested path:force_rebuild=trueis deliberate here, not just a default: the currentthirdparty-macos-amd64tree has none of the new fingerprint files (libgc_build_source_hash.txt, etc.) this PR's expanded rebuild-skip check compares against, so relying on that check alone for the very first run against this new logic is untested territory - force it explicitly instead.workflow_dispatch --refneeds a real ref invlang/vitself (a fork branch won't resolve) - happy to have this validated on a test branch, or if a maintainer wants to dispatch it directly, I'll iterate on any failures.Once this is merged and validated, the plan is: land the
vlang/tccbingate-conversion follow-up → publish the fix withforce_rebuild=true→ confirmvlang/tccbinCI is green → only then open the selector-change PR.