mirror of
https://github.com/logos-co/logos-protocol.git
synced 2026-08-30 21:41:10 +00:00
Dario LipicarandClaude Opus 5
480f40ff63
* feat(abi): publish the module-impl export list as data, for backends to check against
logos-protocol DECLARES the module-impl C ABI; every language backend
(logos-cpp-sdk, logos-rust-sdk, and the Nim path now in flight) must
DEFINE every entry. Those are independent facts, and the gap between them
has shipped twice — grant_host_services at 0.3, the teardown pair at 0.5.
Each time it surfaced three repos downstream as an "undefined symbol" at
dlopen, on Linux only, and each time the runtime still reported the module
as LOADED, so what anyone actually saw was other modules timing out on a
replica that never appeared.
Both breakages happened at PERFECT version agreement between the caller
and the module. Version agreement is necessary and not sufficient: it says
nothing about which symbols a given backend's emitter happens to write.
So derive the list once, here, in the repo that owns the ABI, and ship it
as a build output:
packages.<sys>.module-impl-abi
exports.txt — the declared names
version — the protocol version that header belongs to
bin/logos-module-impl-diff — the assertion, and the explanation
Two properties follow from putting it here rather than in each backend.
There is ONE parser to keep working, rather than one regex per language
that can each silently stop matching. And the list is version-correct with
no version arithmetic anywhere: the header is itself versioned — at 0.4 it
declared eight exports, at 0.5 it declares ten — so "what this protocol
requires" is just "what this header declares". A backend pinning 0.4 reads
eight and is right to define eight. No @since tags, no MINOR comparisons,
nothing for a backend to get wrong.
The extractor parses LOGICAL declarations rather than lines (a reflowed
header must not silently drop one) and refuses to emit a list it is unsure
of: under-reporting is the dangerous direction, because a short list makes
every consumer's diff pass over an ABI nobody checked. The floor it checks
against is asserted rather than derived, so a broken parse cannot satisfy
it. logos-protocol failing to build is the right consequence of
logos-protocol being unable to state its own ABI.
checks.<sys>.module-impl-abi-tests proves all of that can still fail: empty
header, renamed macro, a founding export removed, an empty defined-set, and
a reflowed declaration — eleven cases, each a way this could have decayed
into a green check over nothing.
Also corrects the compatibility note above logos_module_about_to_unload.
It argued the pair was safe because "the glue is generated alongside the
module". That does not follow, and is the reasoning the 0.5 break rested on.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* ci: run the ABI manifest check — `nix build .#tests` never reaches `checks`
The step added here is not incidental. `nix build '.#tests'` builds the
PACKAGE; nothing in this workflow evaluated the `checks` attrset at all, so
the manifest self-test added in the previous commit would have sat there
green-by-absence — which is precisely the failure mode it exists to catch.
`ws test` is not a substitute either: it evaluates exactly one check per
repo (scripts/ws truncates the checks JSON at the first comma), so a green
`ws test logos-protocol` says nothing about whether this ran.
builtins.currentSystem rather than a literal, so one line is correct on both
matrix runners.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(abi): the helper must not depend on the consumer's PATH — and the suite must notice
Two bugs, and the second is the interesting one.
1. The installed helper carried `#!/usr/bin/env bash`. Consumers execute it
from inside their own nix builds, whose PATH is whatever THEIR
nativeBuildInputs provide. It resolved on macOS and not in the Linux
sandbox, so the helper simply did not run there. patchShebangs pins an
absolute interpreter.
2. The self-test did not notice, and the reason is worth keeping. expect_fail
accepted ANY non-zero exit as a correct refusal — but a script that cannot
be executed exits 126/127, so all five refusal cases reported PASS while
proving nothing at all. Only the two POSITIVE cases failed, which is the
only reason this surfaced.
That is precisely the failure this whole change exists to prevent, one
level up: a check that reports green over something it never examined. So
expect_fail now asserts a deliberate refusal and rejects 126/127 by name.
Caught by CI on ubuntu-latest while macOS was green — the same
platform-asymmetry that let the original ABI break through.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
103 lines
4.8 KiB
Nix
103 lines
4.8 KiB
Nix
# Does the ABI manifest actually notice when something is wrong?
|
|
#
|
|
# A check that cannot fail is worse than no check: it reports "green" over an
|
|
# unexamined ABI and buys false confidence. The manifest in nix/module-impl-abi.nix
|
|
# is consumed by every language backend, so its failure modes are asserted here,
|
|
# in the repo that owns it, rather than trusted.
|
|
#
|
|
# Each case below is a way the extractor or the diff could silently degrade into
|
|
# a pass. They are the controls for exactly those, not decoration.
|
|
{ pkgs, common, src, module-impl-abi }:
|
|
|
|
pkgs.runCommand "logos-protocol-module-impl-abi-tests"
|
|
{
|
|
nativeBuildInputs = [ pkgs.gawk pkgs.gnugrep pkgs.coreutils pkgs.diffutils ];
|
|
meta = common.meta or { };
|
|
}
|
|
''
|
|
set -uo pipefail
|
|
X=${src}/nix/module-impl-abi/extract-exports.sh
|
|
D=${module-impl-abi}/bin/logos-module-impl-diff
|
|
H=${src}/cpp/logos_module_impl.h
|
|
fails=0
|
|
|
|
ok() { echo " PASS $1"; }
|
|
bad() { echo " FAIL $1"; fails=$((fails + 1)); }
|
|
|
|
# Expect a DELIBERATE refusal — exit 1, and specifically not 126/127.
|
|
#
|
|
# "Any non-zero exit counts as a refusal" is how a suite like this goes
|
|
# vacuous: a script that cannot be executed at all exits 126 or 127, and every
|
|
# negative case then reports PASS while proving nothing. This file caught
|
|
# exactly that in its own first version — the helper's `#!/usr/bin/env bash`
|
|
# did not resolve inside the Linux sandbox, so all five refusal cases "passed"
|
|
# and only the two positive cases exposed it.
|
|
expect_fail() { local what="$1"; shift
|
|
local rc=0; "$@" >/dev/null 2>&1 || rc=$?
|
|
case "$rc" in
|
|
0) bad "$what (exited 0; it must refuse)" ;;
|
|
126|127) bad "$what (exit $rc — could not execute; this is NOT a refusal)" ;;
|
|
*) ok "$what" ;;
|
|
esac; }
|
|
expect_pass() { local what="$1"; shift
|
|
local rc=0; "$@" >/dev/null 2>&1 || rc=$?
|
|
if [ "$rc" -eq 0 ]; then ok "$what"; else bad "$what (exit $rc)"; fi; }
|
|
|
|
echo "-- the extractor agrees with the published manifest --"
|
|
bash "$X" "$H" > got.txt
|
|
if diff -u ${module-impl-abi}/exports.txt got.txt > /dev/null; then
|
|
ok "extractor output matches the shipped exports.txt"
|
|
else
|
|
bad "extractor drifted from the shipped exports.txt"
|
|
diff -u ${module-impl-abi}/exports.txt got.txt || true
|
|
fi
|
|
|
|
echo "-- the extractor REFUSES rather than under-reporting --"
|
|
# Under-reporting is the dangerous direction: a short list makes every
|
|
# backend's diff pass over an ABI nobody checked.
|
|
: > empty.h
|
|
expect_fail "empty header is refused" bash "$X" empty.h
|
|
sed 's/LOGOS_MODULE_IMPL_EXPORT/LOGOS_MODULE_API/g' "$H" > renamed.h
|
|
expect_fail "renamed export macro is refused" bash "$X" renamed.h
|
|
expect_fail "missing file is refused" bash "$X" /nonexistent.h
|
|
# A founding export removed: the floor in extract-exports.sh is asserted, not
|
|
# derived, precisely so this cannot be satisfied by the same broken parse.
|
|
grep -v 'logos_module_string_free' "$H" > shrunk.h
|
|
expect_fail "a founding export going missing is refused" bash "$X" shrunk.h
|
|
|
|
echo "-- the extractor survives a reflowed declaration --"
|
|
# A line-oriented parser loses a declaration the moment someone puts the
|
|
# macro on its own line. That would under-report, and silently.
|
|
awk '{ if ($0 ~ /^LOGOS_MODULE_IMPL_EXPORT[^\n]*logos_module_about_to_unload/) {
|
|
print "LOGOS_MODULE_IMPL_EXPORT"; sub(/^LOGOS_MODULE_IMPL_EXPORT[[:space:]]*/, "");
|
|
} print }' "$H" > reflowed.h
|
|
bash "$X" reflowed.h > reflowed.txt 2>/dev/null || true
|
|
if diff -u got.txt reflowed.txt > /dev/null; then
|
|
ok "a macro on its own line still yields the full set"
|
|
else
|
|
bad "reflowing a declaration changed the extracted set"
|
|
diff -u got.txt reflowed.txt || true
|
|
fi
|
|
|
|
echo "-- the diff helper --"
|
|
expect_pass "identical sets pass" "$D" got.txt got.txt "self" ""
|
|
grep -v logos_module_about_to_unload got.txt > short.txt
|
|
expect_fail "a backend missing one export fails" "$D" got.txt short.txt "mock" "some/emitter.rs"
|
|
: > nothing.txt
|
|
# If a consumer's symbol extraction silently yields nothing, `comm` reports no
|
|
# missing symbols and the check passes. That is the vacuum this rejects.
|
|
expect_fail "an EMPTY defined-set is refused, not treated as a match" \
|
|
"$D" got.txt nothing.txt "mock" ""
|
|
expect_fail "an empty declared-set is refused" "$D" nothing.txt got.txt "mock" ""
|
|
# Extra symbols on the backend side are its own business (the Rust scaffold
|
|
# declares an extern "Rust" install hook, for one), so the comparison is
|
|
# one-directional and this must NOT fail.
|
|
{ cat got.txt; echo logos_module_something_extra; } | sort > extra.txt
|
|
expect_pass "extra backend symbols are allowed" "$D" got.txt extra.txt "mock" ""
|
|
|
|
echo
|
|
if [ "$fails" -ne 0 ]; then echo "$fails case(s) failed"; exit 1; fi
|
|
echo "module-impl ABI manifest: all cases passed"
|
|
touch $out
|
|
''
|