Fix git branch dependency cache and lock source

Author: Sunrisepeak

.agents/docs/2026-06-02-imgui-mcpp-dependency-fixes.md

@@ -2,7 +2,9 @@
 
 ## Scope
 
-Branch: `codex/imgui-mcpp-dependency-fixes`
+Initial branch: `codex/imgui-mcpp-dependency-fixes`
+
+Follow-up branch: `codex/fix-git-dependency-cache-lock`
 
 This document tracks the mcpp-side changes discovered while validating the
 `imgui-private` package and its `compat.imgui` / `compat.glfw` /
@@ -41,6 +43,22 @@ The imgui validation exposed two separate mcpp-side problems.
    imgui and GLFW consumers, and failed with undefined `glfwInit`,
    `glfwCreateWindow`, etc.
 
+3. Git branch dependencies reused stale source clones and wrote misleading
+   lock entries.
+
+   `prepare_build()` cached git dependencies by `url + ref-kind + ref`. For
+   `branch = "main"`, that cache key stays stable even after the remote branch
+   advances, so `mcpp update <dep>` removed the lock entry but the next build
+   still reused the old clone. The lock writer also treated every non-path
+   dependency as a registry dependency and emitted `source = "index+mcpplibs@"`
+   for git dependencies.
+
+   Impact: a consumer using `imgui = { git = ".../imgui-private.git", branch =
+   "main" }` could keep compiling an old cached checkout that only provided the
+   obsolete `imgui` module instead of the current `imgui.core` and
+   `imgui.backend.*` modules. The lock file also obscured the real dependency
+   source.
+
 ## Changes In This Branch
 
 - `src/build/flags.cppm`
@@ -64,6 +82,18 @@ The imgui validation exposed two separate mcpp-side problems.
   - Added a stale-cache regression that creates an old unmarked xpkg layout and
     verifies mcpp reinstalls it instead of linking against the stale contents.
 
+- `src/cli.cppm`
+  - For `branch` git dependencies, resolve the branch to a concrete commit with
+    `git ls-remote` before creating the cache key.
+  - Include the resolved commit in the git cache key so an advanced branch maps
+    to a fresh cache directory while unchanged branches still reuse the cache.
+  - Record git dependencies in `mcpp.lock` as `git+<url>#branch=<name>@<sha>`
+    instead of `index+...`.
+
+- `tests/e2e/24_git_dependency.sh`
+  - Added a branch dependency regression that verifies `mcpp update <dep>`
+    refreshes a moved branch and that lock source metadata is marked as git.
+
 ## Evidence So Far
 
 - Red test for the include bug:
@@ -105,10 +135,42 @@ The imgui validation exposed two separate mcpp-side problems.
   - `tests/e2e/57_static_dep_shared_artifact.sh`: `OK`
   - `tests/e2e/60_stale_xpkg_cache_reinstall.sh`: `OK`
 
+- Red e2e for git dependency lock/cache behavior before the fix:
+  - Command:
+    `MCPP=$(command -v mcpp) bash tests/e2e/24_git_dependency.sh`
+  - Result: failed with `FAIL: git dep lock source is not marked as git` and a
+    lock entry containing `source = "index+mcpplibs@"`.
+
+- Green e2e after rebuilding the mcpp CLI with the git dependency fix:
+  - Command:
+    `MCPP=target/x86_64-linux-gnu/ea28c45f9dcd4fed/bin/mcpp bash tests/e2e/24_git_dependency.sh`
+  - Result: `OK`
+
+- Focused regression after the final git dependency change:
+  - `mcpp test`: `18 passed; 0 failed`
+  - `tests/e2e/23_remove_update.sh`: `OK`
+  - `tests/e2e/24_git_dependency.sh`: `OK`
+  - `tests/e2e/62_dotted_dependency_selector_priority.sh`: `OK`
+
+- Fresh external `imgui-private` git consumer with rebuilt mcpp CLI:
+  - Command:
+    `MCPP_HOME=/tmp/imgui-private-fixed-mcpp-home target/x86_64-linux-gnu/ea28c45f9dcd4fed/bin/mcpp run`
+  - Result:
+    `external git consumer imported Dear ImGui 1.92.8`
+  - Lock source:
+    `git+git@github.com:mcpplibs/imgui-private.git#branch=main@07ea0c3c088331517e1eda898c267d966173206d`
+
+- `imgui-private` package validation with rebuilt mcpp CLI and existing
+  dependency caches:
+  - `mcpp clean && mcpp build && mcpp test`: `backend_test ... ok`,
+    `imgui_test ... ok`
+  - `examples/basic`: `Dear ImGui 1.92.8 module frame ok`
+  - `examples/glfw_opengl3`: build completed successfully
+
 ## Pending Verification
 
-- Re-run imgui-private with the rebuilt mcpp binary without manually deleting
-  package directories.
+- Before opening a PR, decide whether to run the full e2e suite or rely on the
+  focused unit/e2e/imgui-private validation plus CI.
 - Before opening a PR, decide whether to also run the full e2e suite or leave
   that to CI after the focused dependency set above.
 

src/cli.cppm

@@ -1565,6 +1565,11 @@ prepare_build(bool print_fingerprint,
         std::string version;
     };
     std::vector<DepCacheIdentity> dep_cache_identities;
+    struct GitLockIdentity {
+        std::string source;
+        std::string hash;
+    };
+    std::map<std::string, GitLockIdentity> root_git_lock_identities;
 
     struct ResolvedKey {
         std::string ns;
@@ -2615,20 +2620,44 @@ prepare_build(bool print_fingerprint,
             if (dep_root.is_relative()) dep_root = base / dep_root;
             dep_root = std::filesystem::weakly_canonical(dep_root);
         } else if (spec.isGit()) {
-            // Git-based (M4 #5): clone into ~/.mcpp/git/<hash>/<rev>/
-            // and treat as a path dep from there.
+            // Git-based (M4 #5): clone into ~/.mcpp/git/<hash>/ and treat
+            // as a path dep from there. Branch refs are floating, so resolve
+            // them to a commit before forming the cache key; this lets
+            // `mcpp update <dep>` pick up a moved branch without deleting
+            // unrelated git caches.
             auto mcppHome = [] {
                 if (auto* e = std::getenv("MCPP_HOME"); e && *e)
                     return std::filesystem::path(e);
                 if (auto* e = std::getenv("HOME"); e && *e)
                     return std::filesystem::path(e) / ".mcpp";
                 return std::filesystem::current_path() / ".mcpp";
             }();
-            // Cache key: hash(url + refkind + ref). Avoids collisions across
-            // different revs of the same repo.
+            std::string resolvedGitRev = spec.gitRev;
+            if (spec.gitRefKind == "branch") {
+                auto ref = std::format("refs/heads/{}", spec.gitRev);
+                auto cmd = std::format(
+                    "git ls-remote {} {} 2>&1",
+                    mcpp::platform::shell::quote(spec.git),
+                    mcpp::platform::shell::quote(ref));
+                auto r = mcpp::platform::process::capture(cmd);
+                if (r.exit_code != 0) {
+                    return std::unexpected(std::format(
+                        "git ls-remote of '{}' failed:\n{}", spec.git, r.output));
+                }
+                std::istringstream is(r.output);
+                is >> resolvedGitRev;
+                if (resolvedGitRev.empty()) {
+                    return std::unexpected(std::format(
+                        "git branch '{}' not found in '{}'", spec.gitRev, spec.git));
+                }
+            }
+
+            // Cache key: hash(url + refkind + declared ref + resolved commit).
+            // For fixed rev/tag deps the declared ref is also the resolved ref.
             std::hash<std::string> H;
             auto urlHash = std::format("{:016x}",
-                H(spec.git + "|" + spec.gitRefKind + "|" + spec.gitRev));
+                H(spec.git + "|" + spec.gitRefKind + "|" + spec.gitRev
+                  + "|" + resolvedGitRev));
             auto gitRoot = mcppHome / "git" / urlHash;
             std::error_code ec;
             std::filesystem::create_directories(gitRoot.parent_path(), ec);
@@ -2638,10 +2667,12 @@ prepare_build(bool print_fingerprint,
                 std::string cloneCmd;
                 if (spec.gitRefKind == "branch") {
                     cloneCmd = std::format(
-                        "git clone --depth 1 --branch {} {} {} 2>&1",
+                        "git clone --depth 1 --branch {} {} {} && cd {} && git checkout --quiet {} 2>&1",
                         mcpp::platform::shell::quote(spec.gitRev),
                         mcpp::platform::shell::quote(spec.git),
-                        mcpp::platform::shell::quote(gitRoot.string()));
+                        mcpp::platform::shell::quote(gitRoot.string()),
+                        mcpp::platform::shell::quote(gitRoot.string()),
+                        mcpp::platform::shell::quote(resolvedGitRev));
                 } else {
                     // For tag/rev: full clone, then checkout (depth-1 may miss the rev).
                     cloneCmd = std::format(
@@ -2663,6 +2694,17 @@ prepare_build(bool print_fingerprint,
                     }
                 }
             }
+            if (item.consumerDepIndex == kMainConsumer) {
+                auto source = std::format("git+{}#{}={}",
+                    spec.git, spec.gitRefKind, spec.gitRev);
+                if (spec.gitRefKind == "branch") source += "@" + resolvedGitRev;
+                root_git_lock_identities[name] = GitLockIdentity{
+                    .source = std::move(source),
+                    .hash = std::format("fnv1a:{:016x}", H(spec.git + "|"
+                        + spec.gitRefKind + "|" + spec.gitRev + "|"
+                        + resolvedGitRev)),
+                };
+            }
             dep_root = gitRoot;
         }
         // (version-source: dep_root + manifest are loaded together via
@@ -2985,19 +3027,33 @@ prepare_build(bool print_fingerprint,
             if (spec.isPath()) continue;
             mcpp::lockfile::LockedPackage lp;
             lp.name       = name;
-            lp.namespace_ = spec.namespace_.empty()
-                ? std::string(mcpp::pm::kDefaultNamespace)
-                : spec.namespace_;
-            lp.version    = spec.version;
-            // Use the namespace and resolved version as the source identifier.
-            // For custom indices, include the index name for traceability.
-            lp.source     = std::format("index+{}@{}", lp.namespace_, lp.version);
-            // Use a deterministic hash based on namespace + name + version.
-            // A future PR can replace this with a real content hash from the
-            // xpkg.lua's declared sha256 or from the install plan.
-            std::hash<std::string> hasher;
-            auto hashInput = std::format("{}:{}@{}", lp.namespace_, name, lp.version);
-            lp.hash = std::format("fnv1a:{:016x}", hasher(hashInput));
+            if (spec.isGit()) {
+                auto gitIt = root_git_lock_identities.find(name);
+                lp.version = spec.gitRev;
+                if (gitIt == root_git_lock_identities.end()) {
+                    lp.source = std::format("git+{}#{}={}",
+                        spec.git, spec.gitRefKind, spec.gitRev);
+                    std::hash<std::string> hasher;
+                    lp.hash = std::format("fnv1a:{:016x}", hasher(lp.source));
+                } else {
+                    lp.source = gitIt->second.source;
+                    lp.hash = gitIt->second.hash;
+                }
+            } else {
+                lp.namespace_ = spec.namespace_.empty()
+                    ? std::string(mcpp::pm::kDefaultNamespace)
+                    : spec.namespace_;
+                lp.version    = spec.version;
+                // Use the namespace and resolved version as the source identifier.
+                // For custom indices, include the index name for traceability.
+                lp.source     = std::format("index+{}@{}", lp.namespace_, lp.version);
+                // Use a deterministic hash based on namespace + name + version.
+                // A future PR can replace this with a real content hash from the
+                // xpkg.lua's declared sha256 or from the install plan.
+                std::hash<std::string> hasher;
+                auto hashInput = std::format("{}:{}@{}", lp.namespace_, name, lp.version);
+                lp.hash = std::format("fnv1a:{:016x}", hasher(hashInput));
+            }
             lock.packages.push_back(std::move(lp));
         }
         if (!lock.packages.empty() || !lock.indices.empty()) {

tests/e2e/24_git_dependency.sh

@@ -88,5 +88,105 @@ test -n "$(ls -A "$MCPP_HOME/git" 2>/dev/null)" || { echo "FAIL: git cache empty
 build2=$("$MCPP" build 2>&1)
 echo "$build2" | grep -q 'Cloning' && { echo "FAIL: re-cloned on rebuild"; exit 1; } || true
 
+# 3. Branch refs should update when the user asks for mcpp update.
+mkdir "$TMP/branch-origin" && cd "$TMP/branch-origin"
+git init --quiet
+git checkout -B main --quiet
+git config user.email "test@local"
+git config user.name  "test"
+"$MCPP" new branchlib >/dev/null
+mv branchlib/* branchlib/.??* . 2>/dev/null || true
+rmdir branchlib 2>/dev/null || true
+cat > src/greet.cppm <<'EOF'
+export module branchlib.greet;
+import std;
+export auto greet() -> void { std::println("branch dep v1"); }
+EOF
+rm src/main.cpp
+cat > mcpp.toml <<'EOF'
+[package]
+name        = "branchlib"
+version     = "0.1.0"
+[language]
+standard   = "c++23"
+modules    = true
+import_std = true
+[modules]
+sources = ["src/**/*.cppm"]
+[targets.branchlib]
+kind = "lib"
+EOF
+git add -A >/dev/null
+git commit --quiet -m "v1"
+
+cd "$TMP"
+"$MCPP" new branchapp >/dev/null
+cd branchapp
+cat > src/main.cpp <<'EOF'
+import std;
+import branchlib.greet;
+int main() { greet(); return 0; }
+EOF
+cat > mcpp.toml <<EOF
+[package]
+name        = "branchapp"
+version     = "0.1.0"
+[language]
+standard   = "c++23"
+modules    = true
+import_std = true
+[modules]
+sources = ["src/**/*.cppm", "src/**/*.cpp"]
+[targets.branchapp]
+kind = "bin"
+main = "src/main.cpp"
+[dependencies.branchlib]
+git = "$TMP/branch-origin"
+branch = "main"
+EOF
+
+"$MCPP" build > branch-v1.log 2>&1
+triple=$(ls -d target/*/ | head -1)
+fp_dir=$(ls "$triple")
+out=$(${triple}${fp_dir}/bin/branchapp)
+[[ "$out" == *"branch dep v1"* ]] || {
+    echo "FAIL: branch dep v1 not invoked: $out"
+    cat branch-v1.log; exit 1; }
+
+grep -q 'source  = "git+' mcpp.lock || {
+    echo "FAIL: git dep lock source is not marked as git"
+    cat mcpp.lock; exit 1; }
+grep -q 'branch=main' mcpp.lock || {
+    echo "FAIL: git dep lock source does not record branch ref"
+    cat mcpp.lock; exit 1; }
+! grep -q 'source  = "index+' mcpp.lock || {
+    echo "FAIL: git dep lock source incorrectly uses index source"
+    cat mcpp.lock; exit 1; }
+
+cd "$TMP/branch-origin"
+cat > src/greet.cppm <<'EOF'
+export module branchlib.greet;
+import std;
+export auto greet() -> void { std::println("branch dep v2"); }
+EOF
+git add -A >/dev/null
+git commit --quiet -m "v2"
+
+cd "$TMP/branchapp"
+"$MCPP" update branchlib >/dev/null
+"$MCPP" clean >/dev/null
+"$MCPP" build > branch-v2.log 2>&1
+triple=$(ls -d target/*/ | head -1)
+fp_dir=$(ls "$triple")
+out=$(${triple}${fp_dir}/bin/branchapp)
+[[ "$out" == *"branch dep v2"* ]] || {
+    echo "FAIL: branch dep did not refresh after mcpp update: $out"
+    echo "--- branch-v2.log ---"
+    cat branch-v2.log
+    echo "--- mcpp.lock ---"
+    cat mcpp.lock
+    exit 1
+}
+
 # Cleanup so trap doesn't blow up if any subprocess holds files.
 echo "OK"