Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 6 additions & 13 deletions .devcontainer/post-create.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,13 @@ readonly FISHER_COMMIT=a04308be92daa6cfecdbb0ca58b1e8508664cff2 # 4.4.8
readonly NVM_FISH_COMMIT=abd3002b6d2d578d484a5aea94dd1517dded6d42 # 2.2.17

# Each file exists for its own tool -- .nvmrc for nvm.fish, engines.node for
# pnpm -- so all that is left to guard is that they agree. Exact versions only:
# an alias like lts/iron floats and will eventually stop matching the pin.
required="$(node -p 'require("./package.json").engines.node')"
declared="$(tr -d '[:space:]' < .nvmrc)"

if [ "${declared}" != "${required}" ]; then
cat >&2 <<EOF
.nvmrc says "${declared}" but package.json requires exactly "${required}".
# pnpm -- so all that is left to guard is that they agree. The rule lives in
# `verify.runtimes`, which the checks run too: stated twice it would be two
# rules, and the one nobody runs is the one that rots. It reads only what node
# ships with, so it works here, before anything is installed.
node build/tasks/verify/verify-runtimes.mts || exit 1

nvm.fish resolves \`nvm use\` from .nvmrc, so they have to agree or the version
you get in fish will be rejected by engine-strict.
EOF
exit 1
fi
required="$(node -p 'require("./package.json").engines.node')"

echo "==> Node ${required} (image ships $(node -v))"

Expand Down
109 changes: 109 additions & 0 deletions build/tasks/verify/verify-runtimes.mts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
/**
* @file Verify the runtime versions this project pins in several places agree.
* @author The OpenINF Authors & Friends
* @license MIT OR Apache-2.0 OR BlueOak-1.0.0
* @module {type ES6Module} build/tasks/verify/verify-runtimes
*
* Each file exists for its own tool, so none of them can be removed in favour
* of the others: .nvmrc is what nvm reads for a bare `nvm use`, `engines` is
* what pnpm enforces and what setup-node is pointed at, and `packageManager`
* is the version pnpm fetches to run as. What is left to guard is that they
* say the same thing.
*
* post-create.sh already compares .nvmrc against `engines.node`, but it runs
* when somebody builds the dev container and nowhere else. CI reads
* package.json alone, so a change that moved one and not the other would pass
* every check and then fail for the next person to open the container.
*/

import { readFile } from 'node:fs/promises';

/** What package.json says, of the fields this task compares. */
type Manifest = {
engines?: { node?: string; pnpm?: string };
packageManager?: string;
};

const manifest: Manifest = JSON.parse(await readFile('package.json', 'utf8'));
const nvmrc = (await readFile('.nvmrc', 'utf8')).trim();
const workspace = await readFile('pnpm-workspace.yaml', 'utf8');

/**
* Whether pnpm is told to treat `engines` as a requirement rather than a
* note. Read with a line match rather than a YAML parser because
* pnpm-workspace.yaml is a flat settings file: the key is either at column
* zero or it is not this setting. That reasoning does not extend to a nested
* document, and `verify.workflows` parses its own for exactly that reason.
*
* The space after the colon is required, not tidiness. `engineStrict:true` is
* not a mapping in YAML at all -- it parses as the string
* `"engineStrict:true"` -- so a file saying that has the setting nowhere,
* while a pattern tolerant of the missing space would report it present.
* `\r?` is there so a checkout with CRLF endings reads the same.
*
* A trailing `# comment` is allowed, because YAML reads the value as `true`
* either way and refusing it would fail the dev container's setup over a
* note somebody left themselves.
*
* Column zero is required as well. YAML would accept a root mapping that
* is indented, and this rejects one -- but accepting indentation would
* equally accept `engineStrict:` nested under some other key, which sets
* nothing. Of the two ways to be wrong, complaining about a file nobody
* writes is the better one.
*/
const engineStrict = /^engineStrict:[ \t]+true[ \t]*(?:#.*)?\r?$/m.test(
workspace
);
const node = manifest.engines?.node ?? '';
const pnpm = manifest.engines?.pnpm ?? '';

/**
* `packageManager` is `pnpm@<version>` and may carry a `+sha…` integrity
* suffix, which is not part of the version.
*/
const packaged =
(manifest.packageManager ?? '').replace(/^pnpm@/, '').split('+')[0] ?? '';

const problems: string[] = [];

// An exact version, not a range: `engineStrict` turns `engines` into a
// requirement, and a range would let the two drift apart while still
// technically agreeing.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
for (const [what, value] of [
['engines.node', node],
['engines.pnpm', pnpm],
]) {
if (!/^\d+\.\d+\.\d+$/.test(value)) {
problems.push(`${what} is "${value}"; it has to be an exact version`);
}
}

// Everything below rests on this. Without it `engines` is advisory, the
// versions may disagree with what is actually installed, and a task that
// checked only that three files match would report success over a promise
// nothing keeps.
if (!engineStrict) {
problems.push(
'pnpm-workspace.yaml does not set `engineStrict: true`, so `engines` is a note rather than a requirement and nothing enforces these versions.'
);
}

if (nvmrc !== node) {
problems.push(
`.nvmrc says "${nvmrc}" and engines.node says "${node}". nvm resolves a bare \`nvm use\` from .nvmrc, so the version it gives you would be refused by engine-strict.`
);
}

if (packaged !== pnpm) {
problems.push(
`packageManager pins pnpm ${packaged} and engines.pnpm requires ${pnpm}. pnpm fetches the first and then holds itself to the second, so every install fails.`
);
}

if (problems.length > 0) {
console.error('The pinned runtime versions disagree:\n');
for (const problem of problems) console.error(` ${problem}`);
process.exitCode = 1;
} else {
console.log(`Node ${node} and pnpm ${pnpm}, pinned the same everywhere.`);
}
1 change: 1 addition & 0 deletions package-scripts.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ scripts:
# Outside verify/ on purpose: it needs a pull request in the environment,
# and verify.all runs everything in that directory.
pullRequest: node build/tasks/verify-pull-request.mts
runtimes: node build/tasks/verify/verify-runtimes.mts
spelling: node build/tasks/verify/verify-spelling.mts
toml: node build/tasks/verify/verify-toml.mts
ts: node build/tasks/verify/verify-ts.mts
Expand Down
Loading