Skip to content

Commit 0337b92

Browse files
authored
Fix git branch dependency cache and lock source
Fix stale git branch dependency caches by resolving branch refs into concrete commits for cache identity and lockfile source metadata.\n\nCI: linux, windows, and macOS checks passed.
1 parent 37cbc83 commit 0337b92

3 files changed

Lines changed: 241 additions & 23 deletions

File tree

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

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,9 @@
22

33
## Scope
44

5-
Branch: `codex/imgui-mcpp-dependency-fixes`
5+
Initial branch: `codex/imgui-mcpp-dependency-fixes`
6+
7+
Follow-up branch: `codex/fix-git-dependency-cache-lock`
68

79
This document tracks the mcpp-side changes discovered while validating the
810
`imgui-private` package and its `compat.imgui` / `compat.glfw` /
@@ -41,6 +43,22 @@ The imgui validation exposed two separate mcpp-side problems.
4143
imgui and GLFW consumers, and failed with undefined `glfwInit`,
4244
`glfwCreateWindow`, etc.
4345

46+
3. Git branch dependencies reused stale source clones and wrote misleading
47+
lock entries.
48+
49+
`prepare_build()` cached git dependencies by `url + ref-kind + ref`. For
50+
`branch = "main"`, that cache key stays stable even after the remote branch
51+
advances, so `mcpp update <dep>` removed the lock entry but the next build
52+
still reused the old clone. The lock writer also treated every non-path
53+
dependency as a registry dependency and emitted `source = "index+mcpplibs@"`
54+
for git dependencies.
55+
56+
Impact: a consumer using `imgui = { git = ".../imgui-private.git", branch =
57+
"main" }` could keep compiling an old cached checkout that only provided the
58+
obsolete `imgui` module instead of the current `imgui.core` and
59+
`imgui.backend.*` modules. The lock file also obscured the real dependency
60+
source.
61+
4462
## Changes In This Branch
4563

4664
- `src/build/flags.cppm`
@@ -64,6 +82,18 @@ The imgui validation exposed two separate mcpp-side problems.
6482
- Added a stale-cache regression that creates an old unmarked xpkg layout and
6583
verifies mcpp reinstalls it instead of linking against the stale contents.
6684

85+
- `src/cli.cppm`
86+
- For `branch` git dependencies, resolve the branch to a concrete commit with
87+
`git ls-remote` before creating the cache key.
88+
- Include the resolved commit in the git cache key so an advanced branch maps
89+
to a fresh cache directory while unchanged branches still reuse the cache.
90+
- Record git dependencies in `mcpp.lock` as `git+<url>#branch=<name>@<sha>`
91+
instead of `index+...`.
92+
93+
- `tests/e2e/24_git_dependency.sh`
94+
- Added a branch dependency regression that verifies `mcpp update <dep>`
95+
refreshes a moved branch and that lock source metadata is marked as git.
96+
6797
## Evidence So Far
6898

6999
- Red test for the include bug:
@@ -105,10 +135,42 @@ The imgui validation exposed two separate mcpp-side problems.
105135
- `tests/e2e/57_static_dep_shared_artifact.sh`: `OK`
106136
- `tests/e2e/60_stale_xpkg_cache_reinstall.sh`: `OK`
107137

138+
- Red e2e for git dependency lock/cache behavior before the fix:
139+
- Command:
140+
`MCPP=$(command -v mcpp) bash tests/e2e/24_git_dependency.sh`
141+
- Result: failed with `FAIL: git dep lock source is not marked as git` and a
142+
lock entry containing `source = "index+mcpplibs@"`.
143+
144+
- Green e2e after rebuilding the mcpp CLI with the git dependency fix:
145+
- Command:
146+
`MCPP=target/x86_64-linux-gnu/ea28c45f9dcd4fed/bin/mcpp bash tests/e2e/24_git_dependency.sh`
147+
- Result: `OK`
148+
149+
- Focused regression after the final git dependency change:
150+
- `mcpp test`: `18 passed; 0 failed`
151+
- `tests/e2e/23_remove_update.sh`: `OK`
152+
- `tests/e2e/24_git_dependency.sh`: `OK`
153+
- `tests/e2e/62_dotted_dependency_selector_priority.sh`: `OK`
154+
155+
- Fresh external `imgui-private` git consumer with rebuilt mcpp CLI:
156+
- Command:
157+
`MCPP_HOME=/tmp/imgui-private-fixed-mcpp-home target/x86_64-linux-gnu/ea28c45f9dcd4fed/bin/mcpp run`
158+
- Result:
159+
`external git consumer imported Dear ImGui 1.92.8`
160+
- Lock source:
161+
`git+git@github.com:mcpplibs/imgui-private.git#branch=main@07ea0c3c088331517e1eda898c267d966173206d`
162+
163+
- `imgui-private` package validation with rebuilt mcpp CLI and existing
164+
dependency caches:
165+
- `mcpp clean && mcpp build && mcpp test`: `backend_test ... ok`,
166+
`imgui_test ... ok`
167+
- `examples/basic`: `Dear ImGui 1.92.8 module frame ok`
168+
- `examples/glfw_opengl3`: build completed successfully
169+
108170
## Pending Verification
109171

110-
- Re-run imgui-private with the rebuilt mcpp binary without manually deleting
111-
package directories.
172+
- Before opening a PR, decide whether to run the full e2e suite or rely on the
173+
focused unit/e2e/imgui-private validation plus CI.
112174
- Before opening a PR, decide whether to also run the full e2e suite or leave
113175
that to CI after the focused dependency set above.
114176

src/cli.cppm

Lines changed: 76 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1565,6 +1565,11 @@ prepare_build(bool print_fingerprint,
15651565
std::string version;
15661566
};
15671567
std::vector<DepCacheIdentity> dep_cache_identities;
1568+
struct GitLockIdentity {
1569+
std::string source;
1570+
std::string hash;
1571+
};
1572+
std::map<std::string, GitLockIdentity> root_git_lock_identities;
15681573

15691574
struct ResolvedKey {
15701575
std::string ns;
@@ -2615,20 +2620,44 @@ prepare_build(bool print_fingerprint,
26152620
if (dep_root.is_relative()) dep_root = base / dep_root;
26162621
dep_root = std::filesystem::weakly_canonical(dep_root);
26172622
} else if (spec.isGit()) {
2618-
// Git-based (M4 #5): clone into ~/.mcpp/git/<hash>/<rev>/
2619-
// and treat as a path dep from there.
2623+
// Git-based (M4 #5): clone into ~/.mcpp/git/<hash>/ and treat
2624+
// as a path dep from there. Branch refs are floating, so resolve
2625+
// them to a commit before forming the cache key; this lets
2626+
// `mcpp update <dep>` pick up a moved branch without deleting
2627+
// unrelated git caches.
26202628
auto mcppHome = [] {
26212629
if (auto* e = std::getenv("MCPP_HOME"); e && *e)
26222630
return std::filesystem::path(e);
26232631
if (auto* e = std::getenv("HOME"); e && *e)
26242632
return std::filesystem::path(e) / ".mcpp";
26252633
return std::filesystem::current_path() / ".mcpp";
26262634
}();
2627-
// Cache key: hash(url + refkind + ref). Avoids collisions across
2628-
// different revs of the same repo.
2635+
std::string resolvedGitRev = spec.gitRev;
2636+
if (spec.gitRefKind == "branch") {
2637+
auto ref = std::format("refs/heads/{}", spec.gitRev);
2638+
auto cmd = std::format(
2639+
"git ls-remote {} {} 2>&1",
2640+
mcpp::platform::shell::quote(spec.git),
2641+
mcpp::platform::shell::quote(ref));
2642+
auto r = mcpp::platform::process::capture(cmd);
2643+
if (r.exit_code != 0) {
2644+
return std::unexpected(std::format(
2645+
"git ls-remote of '{}' failed:\n{}", spec.git, r.output));
2646+
}
2647+
std::istringstream is(r.output);
2648+
is >> resolvedGitRev;
2649+
if (resolvedGitRev.empty()) {
2650+
return std::unexpected(std::format(
2651+
"git branch '{}' not found in '{}'", spec.gitRev, spec.git));
2652+
}
2653+
}
2654+
2655+
// Cache key: hash(url + refkind + declared ref + resolved commit).
2656+
// For fixed rev/tag deps the declared ref is also the resolved ref.
26292657
std::hash<std::string> H;
26302658
auto urlHash = std::format("{:016x}",
2631-
H(spec.git + "|" + spec.gitRefKind + "|" + spec.gitRev));
2659+
H(spec.git + "|" + spec.gitRefKind + "|" + spec.gitRev
2660+
+ "|" + resolvedGitRev));
26322661
auto gitRoot = mcppHome / "git" / urlHash;
26332662
std::error_code ec;
26342663
std::filesystem::create_directories(gitRoot.parent_path(), ec);
@@ -2638,10 +2667,12 @@ prepare_build(bool print_fingerprint,
26382667
std::string cloneCmd;
26392668
if (spec.gitRefKind == "branch") {
26402669
cloneCmd = std::format(
2641-
"git clone --depth 1 --branch {} {} {} 2>&1",
2670+
"git clone --depth 1 --branch {} {} {} && cd {} && git checkout --quiet {} 2>&1",
26422671
mcpp::platform::shell::quote(spec.gitRev),
26432672
mcpp::platform::shell::quote(spec.git),
2644-
mcpp::platform::shell::quote(gitRoot.string()));
2673+
mcpp::platform::shell::quote(gitRoot.string()),
2674+
mcpp::platform::shell::quote(gitRoot.string()),
2675+
mcpp::platform::shell::quote(resolvedGitRev));
26452676
} else {
26462677
// For tag/rev: full clone, then checkout (depth-1 may miss the rev).
26472678
cloneCmd = std::format(
@@ -2663,6 +2694,17 @@ prepare_build(bool print_fingerprint,
26632694
}
26642695
}
26652696
}
2697+
if (item.consumerDepIndex == kMainConsumer) {
2698+
auto source = std::format("git+{}#{}={}",
2699+
spec.git, spec.gitRefKind, spec.gitRev);
2700+
if (spec.gitRefKind == "branch") source += "@" + resolvedGitRev;
2701+
root_git_lock_identities[name] = GitLockIdentity{
2702+
.source = std::move(source),
2703+
.hash = std::format("fnv1a:{:016x}", H(spec.git + "|"
2704+
+ spec.gitRefKind + "|" + spec.gitRev + "|"
2705+
+ resolvedGitRev)),
2706+
};
2707+
}
26662708
dep_root = gitRoot;
26672709
}
26682710
// (version-source: dep_root + manifest are loaded together via
@@ -2985,19 +3027,33 @@ prepare_build(bool print_fingerprint,
29853027
if (spec.isPath()) continue;
29863028
mcpp::lockfile::LockedPackage lp;
29873029
lp.name = name;
2988-
lp.namespace_ = spec.namespace_.empty()
2989-
? std::string(mcpp::pm::kDefaultNamespace)
2990-
: spec.namespace_;
2991-
lp.version = spec.version;
2992-
// Use the namespace and resolved version as the source identifier.
2993-
// For custom indices, include the index name for traceability.
2994-
lp.source = std::format("index+{}@{}", lp.namespace_, lp.version);
2995-
// Use a deterministic hash based on namespace + name + version.
2996-
// A future PR can replace this with a real content hash from the
2997-
// xpkg.lua's declared sha256 or from the install plan.
2998-
std::hash<std::string> hasher;
2999-
auto hashInput = std::format("{}:{}@{}", lp.namespace_, name, lp.version);
3000-
lp.hash = std::format("fnv1a:{:016x}", hasher(hashInput));
3030+
if (spec.isGit()) {
3031+
auto gitIt = root_git_lock_identities.find(name);
3032+
lp.version = spec.gitRev;
3033+
if (gitIt == root_git_lock_identities.end()) {
3034+
lp.source = std::format("git+{}#{}={}",
3035+
spec.git, spec.gitRefKind, spec.gitRev);
3036+
std::hash<std::string> hasher;
3037+
lp.hash = std::format("fnv1a:{:016x}", hasher(lp.source));
3038+
} else {
3039+
lp.source = gitIt->second.source;
3040+
lp.hash = gitIt->second.hash;
3041+
}
3042+
} else {
3043+
lp.namespace_ = spec.namespace_.empty()
3044+
? std::string(mcpp::pm::kDefaultNamespace)
3045+
: spec.namespace_;
3046+
lp.version = spec.version;
3047+
// Use the namespace and resolved version as the source identifier.
3048+
// For custom indices, include the index name for traceability.
3049+
lp.source = std::format("index+{}@{}", lp.namespace_, lp.version);
3050+
// Use a deterministic hash based on namespace + name + version.
3051+
// A future PR can replace this with a real content hash from the
3052+
// xpkg.lua's declared sha256 or from the install plan.
3053+
std::hash<std::string> hasher;
3054+
auto hashInput = std::format("{}:{}@{}", lp.namespace_, name, lp.version);
3055+
lp.hash = std::format("fnv1a:{:016x}", hasher(hashInput));
3056+
}
30013057
lock.packages.push_back(std::move(lp));
30023058
}
30033059
if (!lock.packages.empty() || !lock.indices.empty()) {

tests/e2e/24_git_dependency.sh

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,5 +88,105 @@ test -n "$(ls -A "$MCPP_HOME/git" 2>/dev/null)" || { echo "FAIL: git cache empty
8888
build2=$("$MCPP" build 2>&1)
8989
echo "$build2" | grep -q 'Cloning' && { echo "FAIL: re-cloned on rebuild"; exit 1; } || true
9090

91+
# 3. Branch refs should update when the user asks for mcpp update.
92+
mkdir "$TMP/branch-origin" && cd "$TMP/branch-origin"
93+
git init --quiet
94+
git checkout -B main --quiet
95+
git config user.email "test@local"
96+
git config user.name "test"
97+
"$MCPP" new branchlib >/dev/null
98+
mv branchlib/* branchlib/.??* . 2>/dev/null || true
99+
rmdir branchlib 2>/dev/null || true
100+
cat > src/greet.cppm <<'EOF'
101+
export module branchlib.greet;
102+
import std;
103+
export auto greet() -> void { std::println("branch dep v1"); }
104+
EOF
105+
rm src/main.cpp
106+
cat > mcpp.toml <<'EOF'
107+
[package]
108+
name = "branchlib"
109+
version = "0.1.0"
110+
[language]
111+
standard = "c++23"
112+
modules = true
113+
import_std = true
114+
[modules]
115+
sources = ["src/**/*.cppm"]
116+
[targets.branchlib]
117+
kind = "lib"
118+
EOF
119+
git add -A >/dev/null
120+
git commit --quiet -m "v1"
121+
122+
cd "$TMP"
123+
"$MCPP" new branchapp >/dev/null
124+
cd branchapp
125+
cat > src/main.cpp <<'EOF'
126+
import std;
127+
import branchlib.greet;
128+
int main() { greet(); return 0; }
129+
EOF
130+
cat > mcpp.toml <<EOF
131+
[package]
132+
name = "branchapp"
133+
version = "0.1.0"
134+
[language]
135+
standard = "c++23"
136+
modules = true
137+
import_std = true
138+
[modules]
139+
sources = ["src/**/*.cppm", "src/**/*.cpp"]
140+
[targets.branchapp]
141+
kind = "bin"
142+
main = "src/main.cpp"
143+
[dependencies.branchlib]
144+
git = "$TMP/branch-origin"
145+
branch = "main"
146+
EOF
147+
148+
"$MCPP" build > branch-v1.log 2>&1
149+
triple=$(ls -d target/*/ | head -1)
150+
fp_dir=$(ls "$triple")
151+
out=$(${triple}${fp_dir}/bin/branchapp)
152+
[[ "$out" == *"branch dep v1"* ]] || {
153+
echo "FAIL: branch dep v1 not invoked: $out"
154+
cat branch-v1.log; exit 1; }
155+
156+
grep -q 'source = "git+' mcpp.lock || {
157+
echo "FAIL: git dep lock source is not marked as git"
158+
cat mcpp.lock; exit 1; }
159+
grep -q 'branch=main' mcpp.lock || {
160+
echo "FAIL: git dep lock source does not record branch ref"
161+
cat mcpp.lock; exit 1; }
162+
! grep -q 'source = "index+' mcpp.lock || {
163+
echo "FAIL: git dep lock source incorrectly uses index source"
164+
cat mcpp.lock; exit 1; }
165+
166+
cd "$TMP/branch-origin"
167+
cat > src/greet.cppm <<'EOF'
168+
export module branchlib.greet;
169+
import std;
170+
export auto greet() -> void { std::println("branch dep v2"); }
171+
EOF
172+
git add -A >/dev/null
173+
git commit --quiet -m "v2"
174+
175+
cd "$TMP/branchapp"
176+
"$MCPP" update branchlib >/dev/null
177+
"$MCPP" clean >/dev/null
178+
"$MCPP" build > branch-v2.log 2>&1
179+
triple=$(ls -d target/*/ | head -1)
180+
fp_dir=$(ls "$triple")
181+
out=$(${triple}${fp_dir}/bin/branchapp)
182+
[[ "$out" == *"branch dep v2"* ]] || {
183+
echo "FAIL: branch dep did not refresh after mcpp update: $out"
184+
echo "--- branch-v2.log ---"
185+
cat branch-v2.log
186+
echo "--- mcpp.lock ---"
187+
cat mcpp.lock
188+
exit 1
189+
}
190+
91191
# Cleanup so trap doesn't blow up if any subprocess holds files.
92192
echo "OK"

0 commit comments

Comments
 (0)