fix(backend): use the zig toolchain for vendored prerequisites and zlib on host-native zigcc builds - #253
Open
euxaristia wants to merge 1 commit into
Conversation
…target The vendored quickjs and mbedTLS recipes and the zlib link keyed their toolchain choices on `driver.target === null`, which conflates a build being host-native with a POSIX toolchain and system libraries being present. Those are independent: under SCRIPTC_CC=zigcc a host build handed a clang-built vendor archive to a zig link, and on Windows it could not work at all, since there is no system `ar` and no system libz. Dispatch on the driver instead, matching the rule ensureLreObjects and ensureZlibObjects already follow. Behavior for the bare-clang host driver and for every cross build is unchanged. Refs vercel-labs#252
Contributor
|
@euxaristia is attempting to deploy a commit to the Vercel Labs Team on Vercel. A member of the Team first needs to authorize it. |
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.
Summary
Refs #252
The vendored-prerequisite recipes and the zlib link keyed their toolchain choices on
driver.target === null— "is this a host build?" — which conflates the build being native with a POSIX toolchain and system libraries being present. Those are independent properties.Under
SCRIPTC_CC=zigccwith noSCRIPTC_TARGET, the quickjs and mbedTLS archives were therefore built with a systemclangand packed with a systemar, and the link took a system-lzwhile the vendored zlib headers and objects were skipped — even though the program TU, the lre/zlib objects, and the final link all usedzig cc.On Linux/macOS that silently mixes toolchains but usually links, which is why CI stays green. On Windows it fails outright, in three stages:
ENOENTonar, thenunable to find dynamic system library 'z', thenscr_fetch.c:95: 'zlib.h' file not found.SCRIPTC_TARGET=<host triple>already avoids all of it, because every one of these sites is correct on the cross path.ensureLreObjectsandensureZlibObjectsalready solve this correctly, under the comment "a zig-cc-built object set must never be handed to a clang link (or vice versa)". This makes the remaining sites agree with that rule.Changes
Three helpers in
vendor-archives.ts, which is the leaf module here (native-toolchain.tsalready imports runtime values from it, and it takesCcDriveras a type only, so there is no cycle):driverUsesZig(driver)—driver.argv[0] === "zig"vendorArArgv(driver)—zig arfor a zig driver, elsearusesVendoredZlib(driver)— every cross build, and every zig-driven host buildApplied at the sites that were keyed on
driver.target:buildEngineArchiveDirect(quickjs) andensureTlsArchive(mbedTLS) now compile withdriver.argvand archive withvendorArArgv(driver).currentVendorCacheBuildIdentityonly addsclang/arto the identity for a bare-clang host build, so a zig host build no longer records tools it does not use.native-toolchain.tsuseusesVendoredZlib(driver). Because the-I vendorZlibDir()spread is already keyed onzlibObjects.length > 0, the header path follows from the gate change.No cache-key change is needed:
currentVendorCacheBuildIdentitystarts its command list withdriver.argv[0], so clang-driven and zig-driven builds already resolve to differentbuildIdentityvalues and never share an archive.Behavior for the bare-clang host driver and for every cross build is unchanged.
Test plan
New co-located
vendor-archives.test.ts(5 tests), driven off realresolveCcoutput rather than hand-built driver objects:arand a system-lzzig arargv[0]rather than a literalReverting the helper bodies to the previous
driver.targetsemantics fails the two host-zig tests while the bare-clang control still passes, so these pin the corrected behavior rather than merely restating it.Also run:
tsc --noEmitclean;eslint0 errors with no new warnings (the pre-existing 40no-non-null-assertionwarnings innative-toolchain.tsare unchanged).Not run: the full
test:sandboxgate, which needs Vercel credentials. Worth a maintainer running both lanes before merge.End-to-end, an out-of-tree project that previously failed on Windows builds, links, and runs under plain
SCRIPTC_CC=zigccwith this change and no PATH shims, with results identical to theSCRIPTC_TARGET=x86_64-windows-gnupath.