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
121 changes: 98 additions & 23 deletions .github/workflows/engine-drift.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,27 @@ name: Engine drift
#
# It WARNS on both rather than failing. Upstream moving is not a defect in
# this repo, and a job that goes red for something outside it teaches people
# to ignore the job. It DOES fail when the range resolves to nothing, which
# means the dependency is unreachable and an install here is broken.
# to ignore the job. It DOES fail when the pin is unresolvable, which means
# an install here is broken.
#
# A GIT PIN IS THE THIRD SHAPE, and it is the shape this repo is in whenever
# it needs a node no release carries yet. `npm view` cannot resolve one - it
# answers "GitFetcher requires an Arborist constructor to pack a tarball" - so
# the range half of this job read that as an EMPTY resolution and failed with
# "resolves to nothing on the registry, an install here is broken" while
# `npm ci` was installing the pin perfectly well. The job went red the first
# morning after the pin landed and said the opposite of the truth, which is
# worth more than the drift it was watching for.
#
# So the pin's shape decides which comparison is meaningful:
#
# - a RANGE is compared against the registry, as before;
# - a COMMIT is compared against carve-js `main` directly, which is the same
# "how far behind the language is this repo" question with the registry
# taken out of the middle. What DOES fail there is a commit carve-js does
# not have - a rebased-away or force-pushed sha is an install that cannot
# be reproduced, which is the real version of the error the range path was
# reporting falsely.

on:
schedule:
Expand All @@ -41,43 +60,76 @@ jobs:
with:
node-version: 22

- name: Compare the range against the registry
- name: Compare the pin against the registry
id: registry
run: |
set -euo pipefail
pkg='@markup-carve/carve'
range="$(node -p "const p=require('./package.json');const d=p.dependencies||{},v=p.devDependencies||{};d['${pkg}']||v['${pkg}']||''")"
if [ -z "${range}" ]; then
spec="$(node -p "const p=require('./package.json');const d=p.dependencies||{},v=p.devDependencies||{};d['${pkg}']||v['${pkg}']||''")"
if [ -z "${spec}" ]; then
echo "::error::${pkg} is not a dependency of this repo - this workflow is watching nothing"
exit 1
fi
# `npm view <pkg>@<range> version` prints ONE bare version when the
# range matches a single release, and `<pkg>@<v> '<v>'` lines when it
# matches several. Taking the last WHITESPACE-SEPARATED FIELD reads
# both; `tail -1` alone concatenates the spec onto the version the
# moment a second matching release exists, yielding
# `@markup-carve/carve@0.1.20.1.2` rather than a version.
resolved="$(npm view "${pkg}@${range}" version 2>/dev/null | tail -1 | tr -d "'\"" | awk '{print $NF}')"
latest="$(npm view "${pkg}" version 2>/dev/null | tr -d "'\"" | awk '{print $NF}')"
if [ -z "${resolved}" ]; then
echo "::error::${pkg}@${range} resolves to nothing on the registry - an install here is broken"
exit 1
fi
# Both reads are checked for the SHAPE of a version before anything is

# WHICH SHAPE IS IT. A dependency npm fetches from git carries the
# commit after a `#`; anything else is a semver range. The two are
# asked different questions, and asking a git pin the range question
# is what made this job report a broken install against a healthy one.
case "${spec}" in
*://*|git+*|github:*)
commit="${spec##*#}"
case "${commit}" in
# A branch or tag after the `#` pins nothing reproducible, and
# no `#` at all leaves `commit` holding the whole url.
????????????????????????????????????????) ;;
*) echo "::error::${pkg} is fetched from git as \"${spec}\", which names no 40-character commit - an install here is not reproducible"; exit 1 ;;
esac
echo "pinned to carve-js ${commit}"
echo "commit=${commit}" >> "${GITHUB_OUTPUT}"
;;
*)
# `npm view <pkg>@<range> version` prints ONE bare version when
# the range matches a single release, and `<pkg>@<v> '<v>'` lines
# when it matches several. Taking the last WHITESPACE-SEPARATED
# FIELD reads both; `tail -1` alone concatenates the spec onto the
# version the moment a second matching release exists, yielding
# `@markup-carve/carve@0.1.20.1.2` rather than a version.
#
# `|| true` because the message below is the point. Under
# `set -e -o pipefail` an `npm view` that exits non-zero - which
# is exactly what an unresolvable spec does - kills the step
# THERE, before the `::error::` explaining it ever runs, and the
# job fails with an empty log. That is how this step failed for
# real: `--log-failed` printed nothing at all.
resolved="$(npm view "${pkg}@${spec}" version 2>/dev/null | tail -1 | tr -d "'\"" | awk '{print $NF}' || true)"
if [ -z "${resolved}" ]; then
echo "::error::${pkg}@${spec} resolves to nothing on the registry - an install here is broken"
exit 1
fi
echo "resolved=${resolved}" >> "${GITHUB_OUTPUT}"
;;
esac

latest="$(npm view "${pkg}" version 2>/dev/null | tr -d "'\"" | awk '{print $NF}' || true)"
# Every read is checked for the SHAPE of a version before anything is
# compared. Without this the workflow's failure mode is silence: a
# mis-read compares unequal to everything, so the drift warning fires
# with a nonsense string in it and reads as a result (carve#755).
for pair in "resolved=${resolved}" "latest=${latest}"; do
for pair in ${resolved:+"resolved=${resolved}"} "latest=${latest}"; do
case "${pair#*=}" in
[0-9]*.[0-9]*.[0-9]*) ;;
*) echo "::error::${pair%%=*} read back as \"${pair#*=}\" from the registry, which is not a version - every comparison below it would be meaningless"; exit 1 ;;
esac
done
echo "range ${range}"
echo "resolves to ${resolved}"
echo "spec ${spec}"
echo "registry latest ${latest}"
if [ "${resolved}" != "${latest}" ]; then
echo "::warning::${pkg} ${latest} is published, but the range ${range} takes ${resolved} - a major-bumped release does not arrive on its own"
if [ -n "${resolved:-}" ]; then
echo "resolves to ${resolved}"
if [ "${resolved}" != "${latest}" ]; then
echo "::warning::${pkg} ${latest} is published, but the range ${spec} takes ${resolved} - a major-bumped release does not arrive on its own"
fi
else
echo "::warning::${pkg} is on a GIT PIN, so the published ${latest} is not what installs here - the pin returns to a range at the next engine release"
fi
echo "latest=${latest}" >> "${GITHUB_OUTPUT}"

Expand All @@ -88,7 +140,30 @@ jobs:
path: carve-js
fetch-depth: 0

- name: Compare the pinned commit against carve-js main
if: steps.registry.outputs.commit != ''
run: |
set -euo pipefail
commit='${{ steps.registry.outputs.commit }}'
# THE ONE FAILURE THIS PATH OWNS. A pinned commit carve-js does not
# have is a dependency nobody can install - a rebase or a force-push
# upstream, or a sha typed by hand. It is checked before it is
# measured, because `rev-list` on an unknown ref exits non-zero with
# a message about a bad revision, which reads as a workflow bug
# rather than as the finding it is.
if ! git -C carve-js cat-file -e "${commit}^{commit}" 2>/dev/null; then
echo "::error::carve-js has no commit ${commit} - the pin in package.json cannot be installed"
exit 1
fi
behind="$(git -C carve-js rev-list --count "${commit}..origin/main")"
echo "pinned ${commit} ($(git -C carve-js log -1 --format=%s "${commit}"))"
echo "carve-js main is ${behind} commit(s) ahead of it"
if [ "${behind}" -gt 0 ]; then
echo "::warning::the PINNED engine is ${behind} commit(s) behind carve-js main - a git pin does not move on its own, so nothing here will notice the language moving until someone bumps it"
fi

- name: Compare the published engine against carve-js main
if: steps.registry.outputs.commit == ''
run: |
set -euo pipefail
latest='${{ steps.registry.outputs.latest }}'
Expand Down
6 changes: 4 additions & 2 deletions src/convert.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1258,8 +1258,10 @@ function figure(ctx: Ctx, n: CNode): P.Block[] {
const img = inline(ctx, target);
return [P.Figure(toAttr(n.attrs), caption, [P.Plain(img)], shortCaption)];
}
// any other captionable target (outside a group a quote never reaches
// here - see above; as a PANEL it does, and lowers as a nested Figure)
// Any other captionable target, a QUOTE INCLUDED - this is the arm a
// captioned quote takes, at document level and as a §4c panel alike. It
// used to be unreachable for a quote outside a group, because §4a rerouted
// that case; nothing reroutes it now.
return [P.Figure(toAttr(n.attrs), caption, untight(ctx, () => block(ctx, target)), shortCaption)];
}

Expand Down
11 changes: 8 additions & 3 deletions src/reverse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1062,9 +1062,14 @@ function figure(ctx: Ctx, c: never): CNode[] {
// The wrapper and the Table collapse into ONE Carve node, so their
// attrs merge rather than the inner one silently winning: pandoc's
// readers put the label on the Figure, not on the Table it wraps, and
// dropping it took the id a `</#id>` resolves against with it. Same
// collapse rule as the §4a quote branch above - the outer id wins,
// classes union, key/values merge with the outer taking precedence.
// dropping it took the id a `</#id>` resolves against with it. The
// outer id wins, classes union, key/values merge with the outer taking
// precedence.
//
// A table is the only host that collapses this way, because it is the
// only one Carve gives a caption of its own. Every other host keeps
// both nodes - the wrapper stays a `figure` and the host's own attrs
// ride on the target - which is what the Div arm below sorts out.
//
// It matters most for a §4c table PANEL, whose id is what resolves as
// the group's number plus a letter.
Expand Down
34 changes: 34 additions & 0 deletions test/ast-json.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,40 @@ test('the serialized document validates against the spec AST schema', () => {
assertConforms(carveToCarveAst(SOURCE), 'serialized source');
});

test('the pinned engine hands a captioned quote over as a figure, not an attribution', () => {
// WHERE THE ENGINE PIN IS ACTUALLY CHECKED. PART 9 section 4a briefly made a
// caption on a quote an `attribution` field on the quote itself; the clause
// is withdrawn (carve#1213) and `figure` is the generic captioned wrapper
// again, so `resources/ast-schema.json` has no `attribution` property and an
// unknown property is rejected on ingest (PART 12 section 11).
//
// Every other assertion about this shape in the suite reads the CONVERTED
// pandoc tree, which cannot tell a correct engine from a stale one that
// `convert.ts` compensated for - the failure mode markup-carve/carve#755
// collects. This one reads what the engine itself serialized, before the
// converter sees it, so a pin that slipped back behind the withdrawal has
// nowhere downstream to hide.
const ast = carveToCarveAst('> To be, or not to be.\n^ Hamlet\n');
assertConforms(ast, 'captioned quote');

const [figure] = ast.children;
assert.equal(figure.type, 'figure');
assert.equal(figure.target.type, 'block_quote');
// The caption is the half a `figure` has and an `attribution` had elsewhere,
// so it is asserted rather than assumed to have come along. Compared by type
// and text because a serialized node also carries its `pos`.
assert.deepEqual(
figure.caption.map((n) => [n.type, n.value]),
[['text', 'Hamlet']],
);
assert.ok(!JSON.stringify(ast).includes('attribution'), 'no attribution field anywhere');

// The control: an UNCAPTIONED quote is a bare `block_quote`, so the `figure`
// above is the caption's doing and not the engine wrapping every quote.
const [bare] = carveToCarveAst('> To be, or not to be.\n').children;
assert.equal(bare.type, 'block_quote');
});

// --- The other direction: a tree that arrived already serialized ---

const WIRE = {
Expand Down
45 changes: 45 additions & 0 deletions test/participants.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,51 @@ test('no test source carries a literal control byte where an escape belongs', ()
);
});

test('the engine under test is the engine the manifest pins', () => {
// A THIRD POPULATION THAT CAN GO UNCOMPARED: the dependency itself.
//
// `package.json` names a commit of carve-js, `package-lock.json` resolves it,
// and `node_modules/.package-lock.json` records what npm actually wrote to
// disk. Nothing in this suite reads the last one, so a checkout whose
// `node_modules` predates a pin bump runs every engine-facing test against a
// DIFFERENT engine and reports the same green.
//
// Measured, not hypothesized. A long-lived checkout of this repo held:
//
// package-lock.json git+...carve-js.git#3f5dd8cb
// node_modules/.package-lock.json registry.npmjs.org/...carve-0.1.3.tgz
//
// 0.1.3 is the last released tag and predates the PART 9 section 4b
// withdrawal (carve#1213), so that tree still carried `block_quote`'s
// `attribution` field. `npm test` there would have run the quote-figure
// assertions against an engine that cannot satisfy them - or, worse, passed
// them one day and not the next with no file in the diff to explain it.
//
// The fix is `npm ci`, and this is the line that says so.
const root = join(dirname(fileURLToPath(import.meta.url)), '..');
const read = (p) => JSON.parse(readFileSync(join(root, p), 'utf8'));
const dep = '@markup-carve/carve';

const manifest = read('package.json').dependencies[dep];
const locked = read('package-lock.json').packages[`node_modules/${dep}`].resolved;
const installed = read('node_modules/.package-lock.json').packages[`node_modules/${dep}`].resolved;

// A git pin is `<url>#<sha>`; the sha is the only part that identifies code.
// The url half legitimately differs between the three (npm rewrites the
// manifest's `git+https:` to `git+ssh:` when it resolves), so comparing whole
// strings would fail on a correct tree and teach everyone to ignore this.
const sha = (spec) => (typeof spec === 'string' ? (spec.split('#')[1] ?? null) : null);

assert.ok(sha(manifest), `${dep} is not pinned to a commit in package.json: ${manifest}`);
assert.equal(sha(locked), sha(manifest), 'package-lock.json did not follow the manifest bump');
assert.equal(
sha(installed),
sha(manifest),
`node_modules holds ${installed}, not the pinned commit. Run \`npm ci\`: every ` +
'engine-facing test in this suite is currently measuring a different engine.',
);
});

test('a short count is a finding, and a sufficient one is not', () => {
assert.equal(shortfall({ label: 'X', actual: 4, atLeast: 4 }), null);
assert.match(
Expand Down
Loading