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"