Skip to content

ci: detect zapi packages via zapi.targets in native portability check #9644

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nflaig merged 1 commit into from
Jul 10, 2026
Merged

ci: detect zapi packages via zapi.targets in native portability check #9644

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 15 additions & 3 deletions scripts/ci/check_native_portability.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,10 @@ function hasBundledPrebuild(prebuildFiles, targetName) {
}
}

function hasZapiTarget(zapiTargets, target) {
return zapiTargets.some((zapiTarget) => target.aliases.includes(zapiTarget));
}
Comment on lines +129 to +131

@gemini-code-assist gemini-code-assist Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To make the target matching more robust and defensive, we should ensure that zapiTarget is a string and perform case-insensitive matching with whitespace trimming. This prevents potential issues if targets in package.json have accidental leading/trailing spaces or different casing.

Suggested change
function hasZapiTarget(zapiTargets, target) {
return zapiTargets.some((zapiTarget) => target.aliases.includes(zapiTarget));
}
function hasZapiTarget(zapiTargets, target) {
return zapiTargets.some((zapiTarget) => typeof zapiTarget === "string" && target.aliases.includes(zapiTarget.trim().toLowerCase()));
}

@lodekeeper lodekeeper Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — I'd rather keep the strict match here. zapi.targets are canonical target triples, and this check should match exactly what the build tooling consumes to name the zig-out/lib/*.node artifacts. Lower-casing/trimming would let a non-canonical entry (stray whitespace, odd casing) satisfy the check even though it isn't the exact triple — i.e. a false pass, which is worse than a false fail for a portability guard: a strict miss is immediately visible and fixable, whereas a lenient pass can hide a genuinely missing/misnamed binary (the regression this check exists to catch). It also keeps hasZapiTarget consistent with the strict matching in hasBundledPrebuild / optionalDependencyForTarget.

On the type guard: zapiTargets is already guaranteed to be an array (Array.isArray(...) ? ... : []), and Array.prototype.includes returns false for any non-string entry without throwing, so it's crash-safe as-is — the typeof check is only needed to support the .trim()/.toLowerCase() calls, which I'm leaving out for the reason above.


function visitManifest(manifestPath) {
const realManifestPath = fs.realpathSync(manifestPath);
if (visited.has(realManifestPath)) {
Expand All @@ -142,8 +146,12 @@ function visitManifest(manifestPath) {
const hasTargetPackages = Object.keys(optionalDependencies).some((name) =>
requiredTargets.some((target) => target.aliases.some((alias) => name.endsWith(`-${alias}`)))
);
// zapi packages (e.g. @chainsafe/lodestar-z) declare their platform matrix as `zapi.targets` and
// build the `.node` under `zig-out/lib`, so they have neither per-target optionalDependencies nor a
// `prebuilds/` dir. Treat `zapi.targets` as a native-target signal so they aren't silently skipped.
const zapiTargets = Array.isArray(manifest.zapi?.targets) ? manifest.zapi.targets : [];

if (hasTargetPackages || prebuildFiles.length > 0) {
if (hasTargetPackages || prebuildFiles.length > 0 || zapiTargets.length > 0) {
nativePackageCount++;
const packageErrors = [];
const knownPackageGaps = [];
Expand All @@ -154,8 +162,12 @@ function visitManifest(manifestPath) {
if (!optionalDependencyForTarget(optionalDependencies, target)) {
error = `missing optional dependency for ${target.name}`;
}
} else if (!hasBundledPrebuild(prebuildFiles, target.name)) {
error = `missing bundled prebuild for ${target.name}`;
} else if (prebuildFiles.length > 0) {
if (!hasBundledPrebuild(prebuildFiles, target.name)) {
error = `missing bundled prebuild for ${target.name}`;
}
} else if (!hasZapiTarget(zapiTargets, target)) {
error = `missing zapi target for ${target.name}`;
}

if (error !== null) {
Expand Down