-
Notifications
You must be signed in to change notification settings - Fork 12
build: restore libc discriminator on linux lockfile entries #1163
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.
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| #!/usr/bin/env node | ||
| // Verifies that @optave/codegraph-linux-* entries in package-lock.json declare | ||
| // the `libc` discriminator. npm 11 silently strips this field when generating | ||
| // the lockfile on non-Linux hosts (and sometimes on Linux too), even though the | ||
| // published packages declare it. Without it, npm cannot disambiguate | ||
| // linux-x64-gnu vs linux-x64-musl when resolving from the lockfile and may | ||
| // install (or load) the wrong native binary on Alpine/musl hosts. | ||
| // | ||
| // Run via `npm run lint` (or directly) in CI to catch silent regressions from | ||
| // Dependabot bumps and contributor `npm install` runs. | ||
| import { readFileSync } from 'node:fs'; | ||
|
|
||
| const EXPECTED_LIBC = { | ||
| '@optave/codegraph-linux-arm64-gnu': 'glibc', | ||
| '@optave/codegraph-linux-x64-gnu': 'glibc', | ||
| '@optave/codegraph-linux-x64-musl': 'musl', | ||
| }; | ||
|
|
||
| const lock = JSON.parse(readFileSync('package-lock.json', 'utf8')); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch — fixed in b72df7e. The script now resolves |
||
| const failures = []; | ||
|
|
||
| for (const [pkgName, expectedLibc] of Object.entries(EXPECTED_LIBC)) { | ||
| const entry = lock.packages?.[`node_modules/${pkgName}`]; | ||
| if (!entry) { | ||
| failures.push(`${pkgName}: missing from package-lock.json`); | ||
| continue; | ||
| } | ||
| const libc = entry.libc; | ||
| if (!Array.isArray(libc) || !libc.includes(expectedLibc)) { | ||
| failures.push( | ||
| `${pkgName}: expected libc=["${expectedLibc}"], got ${JSON.stringify(libc)}`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (failures.length > 0) { | ||
| console.error('package-lock.json libc discriminator check failed:\n'); | ||
| for (const f of failures) console.error(` - ${f}`); | ||
| console.error( | ||
| '\nnpm install may have stripped the libc field. Restore it by editing\n' + | ||
| 'package-lock.json so each @optave/codegraph-linux-* entry includes\n' + | ||
| 'its libc field (see expected values above). Tracked in #1160.', | ||
| ); | ||
| process.exit(1); | ||
| } | ||
|
|
||
| console.log('package-lock.json libc discriminators OK'); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arm64-muslnot covered by the verifierThe CI
parityjob's inline Node script already mapslinux-arm64-musl→@optave/codegraph-linux-arm64-musl, suggesting the package is expected to ship. If that package is added to the lockfile in a future PR, the verifier won't catch npm 11 stripping itslibc: ["musl"]field — the same silent regression this PR is designed to prevent. Adding it toEXPECTED_LIBCnow (even if the package is currently missing from the lockfile, themissing from package-lock.jsonbranch handles that gracefully) would make the guard future-proof.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good future-proofing thought, but adding
@optave/codegraph-linux-arm64-musltoEXPECTED_LIBCtoday would actually break CI: the package isn't inpackage.json'soptionalDependenciesyet, so it's absent frompackage-lock.json, and the current verifier treats a missing entry as a hard failure (failures.push(${pkgName}: missing from package-lock.json)), not a graceful skip.The CI parity-job mapping references the package optimistically, but it's not actually shipped today. I've opened #1168 to track adding it to
EXPECTED_LIBCthe moment it lands inoptionalDependencies, and to consider whether the "missing" branch should be relaxed to a warn-and-skip so future additions can be made future-proof without immediately failing.