diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 09b9436..cf4c2ea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -537,6 +537,40 @@ jobs: test -x "$HOME/.local/bin/eigenlsp" "$HOME/.local/bin/eigenscript" --version + # #904: this job is the ONLY leg that creates the configuration a + # contributor has after following the README — a build-tree binary + # *and* an installed stdlib in ~/.local/lib/eigenscript (install.sh + # writes both). It just never looked at it: `--version` imports + # nothing. So a stdlib import that reported the stdlib as shadowing + # itself, and resolved to the installed copy over the one shipped + # next to the binary being run, was green here and 4-red on any + # machine that had run install.sh. Both binaries are checked: the + # installed one (single stdlib) and the tree one (two stdlibs — the + # arrangement that actually forked resolution). + - if: needs.scope.outputs.code == 'true' + name: A stdlib import is clean with an installed stdlib present (#904) + run: | + # install.sh's second pass (`./build.sh lsp`) starts with `rm -f + # eigenscript` inside src/, so the tree binary is gone by the time + # install.sh returns — rebuild it. The two binaries differ only in + # where they SIT, which is the whole point: `/../lib/` is a + # resolution step, so the tree binary sees two stdlibs and the + # installed one sees a single stdlib under `/../lib/eigenscript/`. + ./build.sh + test -x src/eigenscript + printf 'import json\nprint of (json.json_from_pairs of ([["k", 1]]))\n' > /tmp/inst904.eigs + for BIN in "$HOME/.local/bin/eigenscript" "$PWD/src/eigenscript"; do + OUT=$(cd / && "$BIN" /tmp/inst904.eigs 2>&1) + echo "$BIN -> $OUT" + if ! echo "$OUT" | grep -q '{"k": 1}'; then + echo "FAIL: stdlib did not resolve for $BIN"; exit 1 + fi + if echo "$OUT" | grep -q "Warning: import"; then + echo "FAIL: spurious shadowing warning from $BIN"; exit 1 + fi + done + echo "no import shadowed itself" + # Performance regression gate (#398). Wall-clock flakes on shared runners, so # the gate compares deterministic cachegrind instruction counts (Ir) of THIS # commit against origin/main — both built in the same runner, so the diff is a diff --git a/CHANGELOG.md b/CHANGELOG.md index bb4d8b6..352ffc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,42 @@ All notable changes to EigenScript are documented here. ### Fixed +- **An installed stdlib no longer shadows itself (#904).** `import` + resolves project-first (#821), probing `.eigs` before + `lib/.eigs`. But the resolver chain's tail steps are the *install + roots* — `/lib/eigenscript/` and `~/.local/lib/eigenscript/`, + which is what `make install` writes — and they answer the bare + `.eigs` request just as readily as `lib/.eigs`. So on any + machine that had ever run `make install`, the installed stdlib came + back as the *project* hit, and every stdlib import produced two defects + at once: + + - a spurious `Warning: import 'json' matches both a project file and a + stdlib module` on stderr, once per name — the diagnostic reporting + the stdlib as shadowing itself; + - a real resolution bug behind it: the installed copy **won**, over the + stdlib shipped next to the binary actually running and over a + bundle's own extracted `lib/`. A bundle — the one artifact of this + project designed to be copied to a machine you don't control — quietly + ran the host's stdlib instead of the one it carries. + + Bundle replay took the visible damage: the replayed run was byte-identical + to the recorded one, same tape, same draw, same stdout, and failed its + byte-identity check on the one prepended warning line. + A resolution hit now reports *which* half of the chain answered + (`resolve_eigenscript_file_from_ex`), and an install-root hit is the + stdlib arm — never the project arm. Genuine project shadowing warns + exactly as before. + + Found on a second machine, not by CI — though CI had the configuration + the whole time. Every suite leg runs from the build tree, and the one + leg that installs (`install-smoke`, via `install.sh`) asserted only + that the binaries existed: `--version` imports nothing. So that runner + sat in the broken state and reported green, while the suite was red for + anyone who followed the README's install path first. That leg now + imports a stdlib module, and both new suite gates simulate the install + root through `HOME` so every leg carries the check. + - **`--pkg add` resolves the remote's default branch instead of fabricating `main` (#879).** `lib/pkg.eigs` hardcoded `tag is "main"` when no tag was given, so the clone ran diff --git a/docs/SPEC.md b/docs/SPEC.md index 049f587..7cdcd6d 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -974,6 +974,13 @@ name matches **both**, the runtime prints a one-line warning to stderr (once per name per process) naming the file used and the file shadowed — rename the project file if the stdlib module is the one you want. +The *project* arm means a file you wrote. An **installed** stdlib +(`/lib/eigenscript/`, what `make install` writes) answers the bare +`name.eigs` shape as readily as `lib/name.eigs`, but it is the stdlib arm +either way: it never counts as a project file, so it neither warns nor +displaces the stdlib shipped alongside the running binary or extracted +from a bundle (#904). + ```eigenscript import math print of (math.clamp of [15, 0, 10]) diff --git a/src/builtins_host.c b/src/builtins_host.c index b676449..60a90d1 100644 --- a/src/builtins_host.c +++ b/src/builtins_host.c @@ -34,6 +34,14 @@ int resolve_eigenscript_file_from(const char *base, const char *path, return 0; /* nothing resolves without a filesystem */ } +int resolve_eigenscript_file_from_ex(const char *base, const char *path, + char *resolved, size_t resolved_cap, + int *origin) { + (void)base; (void)path; (void)resolved; (void)resolved_cap; + if (origin) *origin = EIGS_RESOLVE_PROJECT; + return 0; +} + #else /* host profile */ #include @@ -852,10 +860,21 @@ static int try_eigs_modules_walk(const char *base, const char *path, return 0; } -int resolve_eigenscript_file_from(const char *base, const char *path, - char *resolved, size_t resolved_cap) { +int resolve_eigenscript_file_from_ex(const char *base, const char *path, + char *resolved, size_t resolved_cap, + int *origin) { char candidate[8192]; + /* #904: report which half of the chain answered. The tail steps below + * are the *installed stdlib roots* (`/lib/eigenscript/`, from + * `make install`), and they answer a bare `.eigs` request just as + * readily as `lib/.eigs` — so a hit there is the stdlib wearing a + * project-shaped request, not a project file. Callers that must tell + * the two apart (import's collision diagnostic) pass `origin`. */ +#define RESOLVED(step) \ + do { if (origin) *origin = (step); return 1; } while (0) + + if (origin) *origin = EIGS_RESOLVE_PROJECT; if (!path || !resolved || resolved_cap == 0) return 0; if (!base || !base[0]) base = g_script_dir; @@ -877,25 +896,31 @@ int resolve_eigenscript_file_from(const char *base, const char *path, if (try_resolve_path(candidate, resolved, resolved_cap)) return 1; snprintf(candidate, sizeof(candidate), "%.4000s/../lib/eigenscript/%.4000s", g_exe_dir, path); - if (try_resolve_path(candidate, resolved, resolved_cap)) return 1; + if (try_resolve_path(candidate, resolved, resolved_cap)) RESOLVED(EIGS_RESOLVE_STDLIB_ROOT); if (strncmp(path, "lib/", 4) == 0) { snprintf(candidate, sizeof(candidate), "%.4000s/../lib/eigenscript/%.4000s", g_exe_dir, path + 4); - if (try_resolve_path(candidate, resolved, resolved_cap)) return 1; + if (try_resolve_path(candidate, resolved, resolved_cap)) RESOLVED(EIGS_RESOLVE_STDLIB_ROOT); } const char *home = getenv("HOME"); if (home) { snprintf(candidate, sizeof(candidate), "%.2000s/.local/lib/eigenscript/%.4000s", home, path); - if (try_resolve_path(candidate, resolved, resolved_cap)) return 1; + if (try_resolve_path(candidate, resolved, resolved_cap)) RESOLVED(EIGS_RESOLVE_STDLIB_ROOT); if (strncmp(path, "lib/", 4) == 0) { snprintf(candidate, sizeof(candidate), "%.2000s/.local/lib/eigenscript/%.4000s", home, path + 4); - if (try_resolve_path(candidate, resolved, resolved_cap)) return 1; + if (try_resolve_path(candidate, resolved, resolved_cap)) RESOLVED(EIGS_RESOLVE_STDLIB_ROOT); } } return 0; +#undef RESOLVED +} + +int resolve_eigenscript_file_from(const char *base, const char *path, + char *resolved, size_t resolved_cap) { + return resolve_eigenscript_file_from_ex(base, path, resolved, resolved_cap, NULL); } Value* builtin_load_file(Value *arg) { diff --git a/src/eigenscript.h b/src/eigenscript.h index b346bfe..05e2794 100644 --- a/src/eigenscript.h +++ b/src/eigenscript.h @@ -1259,6 +1259,16 @@ int resolve_eigenscript_file(const char *path, char *resolved, size_t resolved_c * module's own imports relative to that module's directory. */ int resolve_eigenscript_file_from(const char *base, const char *path, char *resolved, size_t resolved_cap); +/* #904: which half of the chain answered. The chain's tail steps are the + * installed stdlib roots (`/lib/eigenscript/`, `~/.local/lib/ + * eigenscript/`), and they answer a bare `.eigs` request as well as + * `lib/.eigs` — so a STDLIB_ROOT hit on a bare request is the stdlib + * itself, not a project file shadowing it. */ +#define EIGS_RESOLVE_PROJECT 0 +#define EIGS_RESOLVE_STDLIB_ROOT 1 +int resolve_eigenscript_file_from_ex(const char *base, const char *path, + char *resolved, size_t resolved_cap, + int *origin); Value* eigs_json_parse_value(const char *s, int *pos); /* #777: the ONLY entry point for a top-level (non-recursive) JSON parse. * Clears both thread-local parse flags (g_json_parse_err, diff --git a/src/vm.c b/src/vm.c index ce1159a..7124e87 100644 --- a/src/vm.c +++ b/src/vm.c @@ -5377,8 +5377,6 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) { char request[4096]; char path_buf[8192]; - extern int resolve_eigenscript_file_from(const char *base, const char *name, - char *out, size_t outlen); extern char *read_file_util(const char *path, long *size); /* Per-file resolution base (Phase 0b): an `import` inside a @@ -5399,12 +5397,26 @@ static Value *vm_run_ex(EigsChunk *chunk, Env *env, Task *resume) { * both is a collision worth a diagnostic whichever way * resolution goes. */ char stdlib_buf[8192]; + int user_origin = EIGS_RESOLVE_PROJECT; snprintf(request, sizeof(request), "%.1024s.eigs", name); - int user_hit = resolve_eigenscript_file_from(resolve_base, request, - path_buf, sizeof(path_buf)); + int user_hit = resolve_eigenscript_file_from_ex(resolve_base, request, + path_buf, sizeof(path_buf), + &user_origin); snprintf(request, sizeof(request), "lib/%.1024s.eigs", name); - int stdlib_hit = resolve_eigenscript_file_from(resolve_base, request, - stdlib_buf, sizeof(stdlib_buf)); + int stdlib_hit = resolve_eigenscript_file_from_ex(resolve_base, request, + stdlib_buf, sizeof(stdlib_buf), + NULL); + + /* #904: the bare `.eigs` request also probes the installed + * stdlib roots, so on a machine that has run `make install` EVERY + * stdlib import came back with a "project" hit at + * `~/.local/lib/eigenscript/.eigs` — a phantom collision + * (spurious warning on every import) AND a resolution bug: the + * installed copy won over the stdlib shipped with the binary + * being run, and over a bundle's own extracted lib/. A stdlib-root + * hit is the stdlib arm; it is never the project arm. */ + if (user_hit && stdlib_hit && user_origin == EIGS_RESOLVE_STDLIB_ROOT) + user_hit = 0; if (!user_hit && !stdlib_hit) { rt_error(EK_IO, current_line, "import: module '%s' not found " diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index 7e37230..01a263a 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -1468,6 +1468,41 @@ else printf '%s\n' "$SH821_ERR" | head -3 fi rm -rf "$SH821_DIR" + +# #904: an INSTALLED stdlib is not a project file. The bare `.eigs` +# half of import's project-first probe reaches the install roots too +# (`/lib/eigenscript/`, `~/.local/lib/eigenscript/` — what +# `make install` writes), so on any machine that had run it, EVERY stdlib +# import reported the stdlib as shadowing itself, and the installed copy +# won over the stdlib shipped with the binary being run. CI HAD that +# configuration all along — the install-smoke leg runs install.sh, which +# writes both — and asserted nothing about it, which is why this stayed +# invisible here and was found on a second machine. HOME is the lever +# that puts the install root in front of EVERY leg, not just the one that +# installs. Asserted: no warning, the RIGHT file resolves (the planted +# install copy has no `abs`, so a wrong pick fails outright), and a real +# project shadow still warns. +SH904_DIR=$(mktemp -d) +SH904_BIN="$PWD/eigenscript" +mkdir -p "$SH904_DIR/home/.local/lib/eigenscript" +printf 'INSTALLED_COPY is 1\n' > "$SH904_DIR/home/.local/lib/eigenscript/math.eigs" +printf 'import math\nprint of (math.abs of -5)\n' > "$SH904_DIR/clean.eigs" +printf 'MARKER is 42\n' > "$SH904_DIR/physics.eigs" +printf 'import physics\nprint of physics.MARKER\n' > "$SH904_DIR/shadow.eigs" +SH904_OUT=$(cd "$SH904_DIR" && HOME="$SH904_DIR/home" "$SH904_BIN" clean.eigs 2>/dev/null) +SH904_WARNS=$(cd "$SH904_DIR" && HOME="$SH904_DIR/home" "$SH904_BIN" clean.eigs 2>&1 >/dev/null \ + | grep -c "Warning: import") +SH904_SHADOW=$(cd "$SH904_DIR" && HOME="$SH904_DIR/home" "$SH904_BIN" shadow.eigs 2>&1 >/dev/null \ + | grep -c "Warning: import 'physics'") +TOTAL=$((TOTAL + 3)) +if [ "$SH904_WARNS" = "0" ] && [ "$SH904_OUT" = "5" ] && [ "$SH904_SHADOW" = "1" ]; then + PASS=$((PASS + 3)) + echo " PASS: installed stdlib is not a project file (#904)" +else + FAIL=$((FAIL + 3)) + echo " FAIL: installed stdlib is not a project file (#904) — warnings=$SH904_WARNS (want 0), math.abs of -5 = '$SH904_OUT' (want 5), real-shadow warnings=$SH904_SHADOW (want 1)" +fi +rm -rf "$SH904_DIR" echo "" # [38] Pattern matching @@ -1661,7 +1696,7 @@ echo "" # [42g] --bundle (#413): single-file distribution — script + eigs_modules + # stdlib in one executable; tape-attached bundles replay byte-identically. -echo "[42g] Bundle (14 checks)" +echo "[42g] Bundle (16 checks)" BN_OUTPUT=$(bash "$TESTS_DIR/test_bundle.sh" 2>&1) BN_PASS=$(echo "$BN_OUTPUT" | grep -c "PASS:" || true) BN_FAIL=$(echo "$BN_OUTPUT" | grep -c "FAIL:" || true) diff --git a/tests/test_bundle.sh b/tests/test_bundle.sh index a6250c4..186d028 100644 --- a/tests/test_bundle.sh +++ b/tests/test_bundle.sh @@ -183,5 +183,34 @@ case "$ROUT" in *) fail "plain interpreter still starts the REPL" "rc=$RC out=$ROUT" ;; esac +# ---- 11. #904: an INSTALLED stdlib must not shadow the bundle's own +# extracted lib/. import resolves project-first, and the bare +# `.eigs` half of that probe reaches the install roots too +# (`~/.local/lib/eigenscript/`, what `make install` writes) — so the +# installed copy came back as a "project file" and the bundle was told it +# was shadowing itself. The warning went to stderr on every stdlib import, +# which is what broke replay's byte-identity check: same tape, same draw, +# same stdout, one extra line. The install root is simulated through HOME +# so every leg carries this check — CI's one installing leg (install-smoke) +# runs on a different runner and never reaches the bundle tests. +FAKE_HOME="$TMPDIR/fakehome" +mkdir -p "$FAKE_HOME/.local/lib/eigenscript" +# A distinguishable stand-in, not a copy: if the INSTALLED file wins, +# json_from_pairs is missing and the run fails outright. +echo 'INSTALLED_COPY is 1' > "$FAKE_HOME/.local/lib/eigenscript/json.eigs" +IOUT=$(cd / && HOME="$FAKE_HOME" "$APP/out" 2>&1); RC=$? +if [ "$RC" -eq 0 ] && echo "$IOUT" | grep -q '{"k": 1}' \ + && ! echo "$IOUT" | grep -q "Warning: import"; then + ok "installed stdlib does not shadow the bundle's own lib/" +else + fail "installed stdlib does not shadow the bundle's own lib/" \ + "rc=$RC: $(echo "$IOUT" | head -2)" +fi +IA=$(cd / && HOME="$FAKE_HOME" "$APP/out2" --replay 2>&1) +[ "$IA" = "$REC_OUT" ] \ + && ok "replay stays byte-identical with an installed stdlib present" \ + || fail "replay stays byte-identical with an installed stdlib present" \ + "recorded: $REC_OUT / replayed: $IA" + echo "BUNDLE: $PASS passed, $FAIL failed" [ "$FAIL" -eq 0 ]