pkg: resolve the remote's default branch instead of fabricating "main" (#879)

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

docs/PACKAGE_SPEC.md

@@ -54,7 +54,15 @@ eigenscript --pkg help                                 print usage
 
 - **add** — clones `<git-url>` at `[tag]` (default branch if omitted)
   into `eigs_modules/<name>/`, 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.

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,19 +332,36 @@ 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]
     if (len of (keys of fetched)) == 0:
         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"
 

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 ""
 

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"