Fix outdated CI for modern runners - #211
Merged
jitsedesmet merged 12 commits intoJul 30, 2026
Merged
Conversation
actions/checkout@v2, actions/setup-node@v2, and actions/cache@v2 all run on a Node.js runtime GitHub has since deprecated and now hard- blocks, so every job on every workflow run has been failing instantly at "Set up job", before any of our own steps execute. This affects every push and pull_request run, including all pending Renovate PRs. Bump all three actions to v4 everywhere they're used (lint, test matrix, and docker jobs). Node.js version matrix left untouched here; that's a separate, larger decision.
hdt is an optionalDependency (native module), so it may not be installed if its native build fails on a given machine or CI runner. Requiring it unconditionally at module load time meant any package that merely imports datasource-hdt -- like datasource-composite's tests -- would crash with "Cannot find module 'hdt'" the moment hdt failed to build, even though those code paths never use it. Move the require into _initialize(), so it's only loaded when an in-process (non-external) HdtDatasource is actually initialized.
12.x/14.x/16.x are all long past end-of-life. Their bundled node-gyp toolchains no longer build native modules (like hdt's) cleanly on current Ubuntu runner images, causing yarn to silently drop the optionalDependency and every test touching it to fail with "Cannot find module 'hdt'" -- the deeper cause behind the previous two CI fixes not being enough on their own. Bump lint and docker to 22.x, and the test matrix to 20.x/22.x/24.x, matching the versions already validated on the TypeScript migration branches.
hdt's native addon couldn't be compiled on any currently-supported Node.js version, for two separate reasons: 1. The transitively-bundled node-gyp (v7.1.2, via @npmcli/run-script) crashes immediately with "Cannot assign to read only property 'cflags'" -- a known node-gyp<9 vs Node>=17 incompatibility. 2. hdt@3.0.1's C++ source (via the nan library) fails a static_assert in V8's newer Local<> type hierarchy, unrelated to node-gyp. Force a modern node-gyp via a resolutions override, and bump hdt to 3.3.2, whose native binding has been updated for current V8. Verified locally: yarn install now actually compiles hdt.node, and all 597 tests (including the real HDT-backed datasource-composite and datasource-hdt suites) pass.
actions/checkout@v4, actions/setup-node@v4, and actions/cache@v4 all still declare a node20 runtime, which GitHub Actions has deprecated and is now force-running on node24 instead -- printing a deprecation warning on every run. coverallsapp/github-action was pinned to the floating @master ref, whose current node16-runtime build triggers the same warning. Bump checkout to v7, setup-node to v7, and cache to v6 -- all three now declare node24 natively. Pin coverallsapp/github-action to the v2 tag instead of @master: v2 is a composite action (no direct Node.js runtime at all), which sidesteps this class of warning entirely and stops floating on a mutable branch ref.
sinon@1.17.4 no longer satisfies sinon-chai's peer range (>=4.0.0), and sinon-chai@3.0.0's own peer range is narrower than what's needed elsewhere -- both were long stale, only surfacing as non-fatal yarn install warnings so far. Bump sinon to 22.1.0 and sinon-chai to 3.7.0 (the latest 3.x, which still targets our pinned chai@^4 rather than chai 5/6's ESM-only peer range that a sinon-chai 4.x bump would otherwise force). Modern sinon removed stub/spy.reset(), split into resetHistory() and resetBehavior(). Every test here only ever needed history reset (behavior like .returns()/.throws() is configured once and reused across cases), so replace all 20 call sites with .resetHistory(). Verified: lint and all 597 tests pass, and the sinon-chai/eslint- plugin-import peer warnings from yarn install are gone.
node:16-slim is Debian buster, whose apt repositories are archived and return 404 on deb.debian.org, so `apt-get update` in the docker CI job fails outright before it can even install the build tools needed to compile hdt. Also drop the ENV key-value legacy syntax warning and swap apt's python for python3 (the old "python" package targeted Python 2, which the image no longer ships). Matches the node:22-slim base already validated on the TypeScript migration branches.
package.json's eslint range (^7.0.0) already permitted this, but yarn.lock had never been refreshed past the very first 7.0.0 release, which is below eslint-plugin-import's stated peer floor (^7.2.0) -- triggering an incorrect-peer-dependency warning on every install. Bump the resolved version to 7.32.0 (the final 7.x release). No declared range changed. Verified: lint and all 597 tests still pass, and the warning is gone.
This reverts commit dee8705.
The prior fix forced a modern node-gyp via a resolutions override, working around old, conflicting node-gyp copies bundled inside lerna's own dependency tree (@lerna/run-lifecycle and pacote both vendored different ancient node-gyp versions, and yarn's hoisting handed hdt's native build to one of those instead of anything we declared). Neither copy belongs to hdt or anything we actually depend on; they're internal to lerna@4. Bumping lerna to 9.0.7 removes both stale copies -- it now pulls in a single, current node-gyp (12.4.0) with no conflicts, so hdt builds without any override or extra devDependency at all. lerna 9 removed the "useWorkspaces" lerna.json option (workspaces are now auto-detected from package.json), so drop it. No packages declare a "prepare" script, so `lerna run prepare` (postinstall) was already a no-op and stays one. Verified: fresh yarn install compiles hdt.node with zero warnings, lint passes, and all 597 tests pass. lerna's publish/exec paths (lerna.json's command.publish config, `lerna publish`, `lerna exec`) are unverified by this change -- they're not exercised by CI or by anything in this diff.
lerna's publish/exec paths aren't exercised by CI or verified by the prior commit, so pin exactly rather than allowing ^9.0.7 to drift to a newer minor/patch unnoticed.
sinon-chai@2.14.0's peer range (^1.4.0 || ^2.1.0 || ^3.0.0 || ^4.0.0) already covers our existing sinon@1.17.4 -- the incorrect-peer- dependency warning was only ever caused by sinon-chai being on 3.x, whose peer range starts at sinon@4. No need to touch sinon at all. This reverts the sinon@22.1.0 bump and the .reset() -> resetHistory() rewrite across 20 call sites: sinon@1.17.4's spy.reset() (which stubs inherit) is history-only, exactly like modern resetHistory(), so the original .reset() calls are correct as they were. Verified: fresh yarn install has no sinon-chai or eslint-plugin-import peer warnings, lint passes, and all 597 tests pass.
Michiel-VandeVelde
changed the title
jitsedesmet
marked this pull request as ready for review
July 30, 2026 11:53
jitsedesmet
merged commit Jul 30, 2026
3926468
into
LinkedDataFragments:modernize-repository
10 checks passed
|
Thank you! |
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
2 participants
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.
This starts the CI, dependency, and build modernization, targeting the shared modernize-repository branch @jitsedesmet set up upstream.
Covers: GitHub Actions bumped to non-deprecated versions targeting Node.js 24, Docker base image updated to node:22-slim, HDT's native build fixed for modern Node, sinon/sinon-chai modernization attempted then reverted (pinned sinon-chai to 2.14.0 instead the newer sinon API broke stub.reset() usage elsewhere), and lerna pinned to a version that keeps HDT's native build working at the root.
Recap from previous discussion: that referencing this fork's own PR from a commit message with a bare #N will auto-link to an unrelated issue/PR once the commit lands in LinkedDataFragments/Server.js, since GitHub resolves #N against whichever repo it's viewed in. @rubensworks fix: use the cross-repo form Michiel-VandeVelde/Server.js#N instead. this was later discussed and the upstream branch modernize-repository was made to have a cleaner history.
original PR can be found at: Michiel-VandeVelde/Server.js/pull/7