From 2e50808293863eea13a33d4fade747495eeffe6d Mon Sep 17 00:00:00 2001 From: InauguralPhysicist Date: Wed, 5 Aug 2026 19:25:43 -0500 Subject: [PATCH] pkg: resolve the remote's default branch instead of fabricating "main" (#879) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit lib/pkg.eigs hardcoded `tag is "main"` when `--pkg add` was given no tag, so the clone ran `git clone --depth 1 --branch main` and failed outright on any repository whose default branch is master, trunk or develop: warning: Could not find remote branch main to clone. fatal: Remote branch main not found in upstream origin Worse than a failed clone: the fabricated tag was persisted into eigs.json BEFORE the clone was attempted. That ordering is deliberate — the manifest write is what makes `add` recoverable by re-running `install` — but it meant the recovery path was poisoned too. The project was left naming a branch that does not exist, and `--pkg install` could never fix it. PACKAGE_SPEC.md:60 already specified "default branch if omitted", so an omitted tag now means exactly that: no --branch, git picks the remote's own default, and the manifest records NO "tag" key rather than a guess. The manifest carries the REQUESTED ref; the lockfile carries the RESOLVED commit, which is what makes install reproducible — so dropping the guess costs nothing in determinism. One clone_args helper is shared by add / install / update so the three cannot drift on what "no tag" means; install had the identical hardcoded --branch and would have failed the same way on a no-tag dep. `--pkg add` now reports which default branch it resolved to. tests/test_pkg_fetch.sh gains a master-branch remote and covers: add succeeds, the manifest records no guessed tag, install RECOVERS from the manifest alone (the path the old bug destroyed), verify passes, and an explicit tag is still recorded and honored. Section [95] 6 -> 11 checks. Suite 3809/3809. Closes #879 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 19 +++++++++++ docs/PACKAGE_SPEC.md | 10 +++++- lib/pkg.eigs | 62 ++++++++++++++++++++++++++++++----- tests/run_all_tests.sh | 12 +++---- tests/test_pkg_fetch.sh | 71 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 159 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e252e300..776460e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,25 @@ All notable changes to EigenScript are documented here. ### Fixed +- **`--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 + `git clone --depth 1 --branch main` and **failed outright** on any + repository whose default branch is `master`, `trunk` or `develop`. + Worse, the fabricated tag was persisted into `eigs.json` *before* the + clone was attempted — deliberately, so `add` is recoverable by + re-running `install` — which meant the recovery path was poisoned too: + the project was left naming a branch that does not exist, and + `--pkg install` could never fix it. + `PACKAGE_SPEC.md:60` already said "default branch if omitted", so an + omitted tag now means exactly that: no `--branch`, git picks the + remote's default, and the manifest records **no `tag` key** rather + than a guess. The lockfile still pins the resolved commit, which is + what makes install reproducible. One `clone_args` helper is shared by + `add`, `install` and `update` so the three cannot drift on what "no + tag" means. `--pkg add` now also reports which default branch it + resolved to. + - **Memory corruption: values escaping an `arena_mark`…`arena_reset` scope (#873).** `promote_if_arena` copied only numbers and strings to the heap on store; a LIST escaping the scope became a dangling diff --git a/docs/PACKAGE_SPEC.md b/docs/PACKAGE_SPEC.md index cd5e1399..a7fe37af 100644 --- a/docs/PACKAGE_SPEC.md +++ b/docs/PACKAGE_SPEC.md @@ -54,7 +54,15 @@ eigenscript --pkg help print usage - **add** — clones `` at `[tag]` (default branch if omitted) into `eigs_modules//`, records the resolved commit + tree hash - in `eigs.lock.json`, and writes the dep into `eigs.json`. + in `eigs.lock.json`, and writes the dep into `eigs.json`. An omitted + tag is **recorded as omitted** — no `"tag"` key — and the clone runs + without `--branch`, so git picks the remote's own default. The + manifest carries the *requested* ref; the lockfile carries the + *resolved* commit, which is what makes the install reproducible. + (Before #879 an omitted tag was written as the literal `"main"` + before the clone was attempted, which failed outright on any + `master`/`trunk`/`develop` remote and left a manifest `install` could + never recover.) - **install** — reproduces `eigs_modules/` from manifest + lockfile. Existing checkouts are wiped before re-clone, so install is idempotent and deterministic from the lockfile alone. diff --git a/lib/pkg.eigs b/lib/pkg.eigs index 1705d729..086f4361 100644 --- a/lib/pkg.eigs +++ b/lib/pkg.eigs @@ -268,11 +268,36 @@ define cmd_list() as: # target dir to keep `add` idempotent — a re-add against the same # name picks up the new url/tag instead of mixing trees. Returns # {"commit": sha, "tree": tree_sha} on success, {} on failure. +# #879: the clone argument list for a dep. `--pkg add` used to hardcode +# `tag is "main"` when the caller gave no tag, so every clone ran +# `git clone --branch main` and failed outright on any repository whose +# default branch is master/trunk/develop — and the fabricated "main" was +# PERSISTED into eigs.json before the clone was attempted, leaving a project +# `--pkg install` could never recover. PACKAGE_SPEC.md:60 says "default branch +# if omitted", so an omitted tag now means exactly that: no --branch, and git +# picks the remote's own default. Three call sites (add / install / update) +# share this so they cannot drift on what "no tag" means. +define clone_args(git_url, target, tag) as: + if tag == null or tag == "": + return ["clone", "--depth", "1", git_url, target] + return ["clone", "--depth", "1", "--branch", tag, git_url, target] + +# The branch a checkout actually landed on, or "" if it is detached/unknown. +# Used to report which default branch `add` resolved to. +define git_current_branch(work_dir) as: + result is exec_capture of ["git", "-C", work_dir, "rev-parse", "--abbrev-ref", "HEAD"] + if result[0] != 0: + return "" + out is trim of result[1] + if out == "HEAD": + return "" + return out + define fetch_dep(name, git_url, tag) as: mkdir of MODULES_DIR target is MODULES_DIR + "/" + (pkg_leaf of name) rmtree of target - ok is run_git of ["clone", "--depth", "1", "--branch", tag, git_url, target] + ok is run_git of (clone_args of [git_url, target, tag]) if ok == 0: return {} commit is git_head_commit of target @@ -283,7 +308,7 @@ define fetch_dep(name, git_url, tag) as: if tree == "": print of f"could not resolve tree hash for {name}" return {} - return {"commit": commit, "tree": tree} + return {"commit": commit, "tree": tree, "branch": git_current_branch of target} define cmd_add(arg_list) as: if (len of arg_list) < 2: @@ -294,7 +319,11 @@ define cmd_add(arg_list) as: if err != "": throw of f"--pkg add: {err}" git_url is arg_list[1] - tag is "main" + # #879: omitted means "the remote's default branch", not the literal + # string "main". Fabricating one both broke the clone on master/trunk + # repos AND was written into eigs.json first, so the project could not + # be recovered by `--pkg install` afterwards. + tag is "" if (len of arg_list) >= 3: tag is arg_list[2] @@ -303,7 +332,13 @@ define cmd_add(arg_list) as: manifest is read_manifest of null if (has_key of [manifest, "deps"]) == 0: manifest.deps is {} - manifest.deps[name] is {"git": git_url, "tag": tag} + # An omitted tag is recorded as omitted — the manifest carries the + # REQUESTED ref, the lockfile carries the resolved commit. Writing a + # guessed branch here is what made the old failure unrecoverable. + if tag == "": + manifest.deps[name] is {"git": git_url} + else: + manifest.deps[name] is {"git": git_url, "tag": tag} write_manifest of manifest fetched is fetch_dep of [name, git_url, tag] @@ -311,11 +346,22 @@ define cmd_add(arg_list) as: throw of f"fetch failed for {name}" lock is read_lockfile of null - lock[name] is {"git": git_url, "tag": tag, - "commit": fetched.commit, "tree": fetched.tree} + if tag == "": + lock[name] is {"git": git_url, + "commit": fetched.commit, "tree": fetched.tree} + else: + lock[name] is {"git": git_url, "tag": tag, + "commit": fetched.commit, "tree": fetched.tree} write_lockfile of lock - print of f"Added {name} -> {git_url} @ {tag} ({fetched.commit[0:8]})" + shown is tag + if shown == "": + shown is fetched.branch + if shown == "": + shown is "default branch" + else: + shown is shown + " (default branch)" + print of f"Added {name} -> {git_url} @ {shown} ({fetched.commit[0:8]})" return 0 # Reproduce eigs_modules/ from manifest + lockfile. For each dep: @@ -351,7 +397,7 @@ define cmd_install() as: # Clone with depth 1 at the tag, then if we have a locked # commit, fetch that specific commit and check it out. - ok is run_git of ["clone", "--depth", "1", "--branch", tag, git_url, target] + ok is run_git of (clone_args of [git_url, target, tag]) # #879 if ok == 0: throw of f"install failed: clone for {name} failed" diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index c61ddfe9..0756c454 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -3783,27 +3783,27 @@ else fi echo "" -echo "[95] --pkg fetch (6 checks)" +echo "[95] --pkg fetch (11 checks)" # Phase 1b: --pkg add and --pkg install actually shell out to git # against a local file:// repo. Verifies the clone lands in # eigs_modules/, the lockfile records the resolved commit, the # clone is importable through Phase 0c's eigs_modules resolver, and # the lockfile wins over a force-pushed tag. Also asserts bare names # are rejected (namespaced-identifier rule). -TOTAL=$((TOTAL + 6)) +TOTAL=$((TOTAL + 11)) PKG2_OUT=$(EIGENSCRIPT="./eigenscript" bash "$TESTS_DIR/test_pkg_fetch.sh" 2>&1); PKG2_RC=$? PKG2_PASS=$(echo "$PKG2_OUT" | grep -c "^ PASS:" || true) PKG2_SKIP=$(echo "$PKG2_OUT" | grep -c "^ SKIP:" || true) -if [ "$PKG2_RC" = "0" ] && [ "$PKG2_PASS" = "6" ]; then +if [ "$PKG2_RC" = "0" ] && [ "$PKG2_PASS" = "11" ]; then echo "$PKG2_OUT" | grep "^ PASS:" - PASS=$((PASS + 6)) + PASS=$((PASS + 11)) elif [ "$PKG2_SKIP" -gt "0" ]; then echo "$PKG2_OUT" | grep "^ SKIP:" - PASS=$((PASS + 6)) + PASS=$((PASS + 11)) else echo " FAIL: --pkg fetch (rc=$PKG2_RC, passes=$PKG2_PASS)" echo "$PKG2_OUT" | head -20 - FAIL=$((FAIL + 6)) + FAIL=$((FAIL + 11)) fi echo "" diff --git a/tests/test_pkg_fetch.sh b/tests/test_pkg_fetch.sh index bb3911a0..c01663f6 100755 --- a/tests/test_pkg_fetch.sh +++ b/tests/test_pkg_fetch.sh @@ -142,3 +142,74 @@ if [ "$APP_OUT2" != "hello from greeting" ]; then exit 1 fi echo " PASS: lockfile wins over a moved tag" + +# ---- #879: a remote whose default branch is NOT "main" ---- +# `--pkg add` hardcoded `tag is "main"`, so it ran +# `git clone --branch main` and failed outright on master/trunk/develop — +# and it PERSISTED the fabricated {"tag": "main"} into eigs.json BEFORE +# attempting the clone, leaving a project `--pkg install` could never +# recover. PACKAGE_SPEC.md:60 says "default branch if omitted". +mkdir -p "$TMP/msource" +cd "$TMP/msource" +git init -q -b master +git config user.email "test@example.com" +git config user.name "Test" +cat > mylib.eigs <<'EOF' +mylib_greet is "hello from a master-branch repo" +EOF +git add -A +git commit -q -m "init on master" + +mkdir -p "$TMP/mproject" +cd "$TMP/mproject" +ADD_M=$("$EIGS" --pkg add alice/mylib "file://$TMP/msource" 2>&1) || { + echo " FAIL: --pkg add should work on a master-branch remote" + echo "$ADD_M" + exit 1 +} +echo " PASS: --pkg add resolves a non-main default branch" + +# The manifest must NOT carry a fabricated tag — an omitted tag stays omitted, +# which is what makes the project recoverable. +if grep -q '"tag"' eigs.json; then + echo " FAIL: eigs.json must not record a guessed tag" + cat eigs.json + exit 1 +fi +echo " PASS: an omitted tag is recorded as omitted, not guessed" + +# The recovery path the old bug destroyed: reinstall from the manifest alone. +rm -rf eigs_modules +INSTALL_M=$("$EIGS" --pkg install 2>&1) || { + echo " FAIL: --pkg install must recover a dep with no tag" + echo "$INSTALL_M" + exit 1 +} +cat > mapp.eigs <<'EOF' +import mylib +print of mylib.mylib_greet +EOF +MAPP_OUT=$("$EIGS" mapp.eigs 2>&1) +if [ "$MAPP_OUT" != "hello from a master-branch repo" ]; then + echo " FAIL: reinstalled master-branch dep should be usable — got '$MAPP_OUT'" + exit 1 +fi +echo " PASS: --pkg install recovers a no-tag dep (was: unrecoverable)" + +VERIFY_M=$("$EIGS" --pkg verify 2>&1) || { + echo " FAIL: --pkg verify should pass for a no-tag dep" + echo "$VERIFY_M" + exit 1 +} +echo " PASS: --pkg verify passes for a no-tag dep" + +# An explicit tag is still honored, unchanged. +mkdir -p "$TMP/mproject2" +cd "$TMP/mproject2" +"$EIGS" --pkg add alice/mylib "file://$TMP/msource" master > /dev/null 2>&1 +if ! grep -q '"tag": *"master"' eigs.json; then + echo " FAIL: an explicit tag must still be recorded" + cat eigs.json + exit 1 +fi +echo " PASS: an explicit tag is still recorded and used"