Skip to content

Commit 65af80c

Browse files
SunrisepeakSunrisepeak
committed
fix(build): cache custom-index dependency objects
1 parent e29373e commit 65af80c

3 files changed

Lines changed: 246 additions & 6 deletions

File tree

‎.agents/docs/2026-05-30-bmi-cache-custom-index-fix.md‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# BMI Cache And Custom Index Build Fix
2+
3+
## Context
4+
5+
`xlings` now builds through `mcpp` and uses a local project index:
6+
7+
```toml
8+
[indices]
9+
xlings = { path = "mcpp" }
10+
```
11+
12+
During a first build, `mcpp build` currently prints `Fetching custom index repos (first use)` followed by `xlings update` output, then emits warnings such as:
13+
14+
```text
15+
warning: bmi cache populate failed for tinyhttps@0.2.3: expected build output missing: .../obj/tinyhttps.m.o
16+
warning: bmi cache populate failed for xlings.libarchive@3.8.7: expected build output missing: .../obj/xxhash.o
17+
```
18+
19+
The build itself can still succeed, but the dependency BMI cache is not populated for affected packages.
20+
21+
## Root Cause
22+
23+
There are two related build-tool issues.
24+
25+
1. Project custom-index bootstrap is too broad.
26+
27+
`mcpp` seeds `.mcpp/.xlings.json` for non-builtin `[indices]` and treats the project index data as uninitialized when no `xim-indexrepos.json` or cloned `pkgs/` directory exists. It then runs `xlings update`, which prints all configured xlings index update output. For a local path index, the local `pkgs/` tree already exists and can be read directly, so the update path is mostly noise and can create extra first-build work.
28+
29+
2. BMI cache records object basenames instead of object paths relative to `obj/`.
30+
31+
The build plan correctly writes objects under collision-avoiding subdirectories, for example:
32+
33+
```text
34+
obj/mcpplibs_tinyhttps_src/tinyhttps.m.o
35+
obj/xlings_libarchive_libarchive/xxhash.o
36+
obj/xlings_zlib_zlib-1.3.2/compress.o
37+
```
38+
39+
The cache populate list currently stores only `cu.object.filename()`, so it later searches for:
40+
41+
```text
42+
obj/tinyhttps.m.o
43+
obj/xxhash.o
44+
obj/compress.o
45+
```
46+
47+
Those files do not exist, so `populate_from()` returns `expected build output missing`.
48+
49+
There is also a cache-key correctness gap: all dependency cache entries currently use the default index name. Custom-index dependencies should carry their real namespace/index identity so cache entries from different indices do not collide.
50+
51+
## Fix Plan
52+
53+
1. Add regression coverage first.
54+
- Add an e2e regression that builds a local-path custom-index dependency with duplicate object basenames.
55+
- Assert that local-path indices do not trigger first-use `xlings update` noise.
56+
- Assert that the cache manifest is written under the custom index name and preserves nested object paths.
57+
- Assert that a second cold build reuses the dependency BMI cache.
58+
59+
2. Fix artifact path recording.
60+
- When collecting dependency artifacts in `src/cli.cppm`, store object paths relative to `ctx.outputDir / "obj"` instead of only the object filename.
61+
- Keep BMI filenames as basenames because compiler BMI output is intentionally flat in the active BMI directory.
62+
63+
3. Fix dependency cache identity.
64+
- Use the dependency namespace or custom index name in `CacheKey::indexName` instead of always using the global default index.
65+
- Preserve builtin/default behavior for existing `mcpplibs` packages.
66+
67+
4. Reduce local custom-index update noise.
68+
- Treat local `{ path = ... }` indices as initialized when the source path has a `pkgs/` directory.
69+
- Keep remote custom indices on the existing project `xlings update` path.
70+
71+
5. Verify.
72+
- Run targeted BMI cache unit tests.
73+
- Run the broader unit suite or project test command available in the repository.
74+
- Run a local `mcpp build` scenario when feasible to ensure the original warning no longer appears.
75+
76+
## Dynamic Notes
77+
78+
- If tests reveal that `CacheKey::indexName` is used as a namespace rather than a repository identity, prefer the package namespace as the cache partition and document the remaining repository-revision hardening as follow-up.
79+
- If local path indices still need xlings project metadata for install operations, skip only the update call, not `.mcpp/.xlings.json` seeding.
80+
- Implemented regression as `tests/e2e/49_bmi_cache_nested_custom_index.sh` because the defect is in CLI dependency resolution plus build-plan artifact collection, not only the lower-level cache copy functions.
81+
- Local verification: `mcpp build` succeeded, then `MCPP=target/x86_64-linux-gnu/4d24c8b57fdbbbb4/bin/mcpp bash tests/e2e/49_bmi_cache_nested_custom_index.sh` returned `OK`.
82+
- `mcpp test -- --gtest_filter=BmiCache.*` initially exposed an unrelated local environment issue: `~/.mcpp/registry/data/xpkgs/xim-x-binutils/2.42` only had `.xpkg.lua` and no `bin/as`. Re-running with a temporary `MCPP_HOME` pointed at the complete installed mcpp registry passed: 15 test binaries ok, 0 failed.

‎src/cli.cppm‎

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,9 +1390,17 @@ prepare_build(bool print_fingerprint,
13901390
if (hasCustomIndices) {
13911391
bool needsClone = !mcpp::config::project_index_data_initialized(*root);
13921392
if (needsClone) {
1393-
mcpp::ui::status("Fetching", "custom index repos (first use)");
1394-
auto projEnv = mcpp::config::make_project_xlings_env(**cfg2, *root);
1395-
mcpp::xlings::update_index(projEnv);
1393+
bool needsRemoteUpdate = false;
1394+
for (auto& [idxName, spec] : m->indices) {
1395+
if (spec.is_builtin() || spec.is_local()) continue;
1396+
needsRemoteUpdate = true;
1397+
break;
1398+
}
1399+
if (needsRemoteUpdate) {
1400+
mcpp::ui::status("Fetching", "custom index repos (first use)");
1401+
auto projEnv = mcpp::config::make_project_xlings_env(**cfg2, *root);
1402+
mcpp::xlings::update_index(projEnv);
1403+
}
13961404
}
13971405
}
13981406
}
@@ -1406,6 +1414,11 @@ prepare_build(bool print_fingerprint,
14061414
// unique_ptr is not load-bearing for liveness — it's a leftover from
14071415
// an earlier design and harmless).
14081416
std::vector<std::unique_ptr<mcpp::manifest::Manifest>> dep_manifests;
1417+
std::vector<std::string> dep_cache_indices;
1418+
auto cache_index_name = [](std::string_view ns) {
1419+
if (ns.empty()) return std::string(mcpp::pm::kDefaultNamespace);
1420+
return std::string(ns);
1421+
};
14091422

14101423
struct ResolvedKey {
14111424
std::string ns;
@@ -1581,8 +1594,9 @@ prepare_build(bool print_fingerprint,
15811594
// Route xpkg.lua reading through the appropriate index.
15821595
std::optional<std::string> luaContent;
15831596
if (idxSpec && idxSpec->is_local()) {
1597+
auto indexPath = mcpp::config::resolve_project_index_path(*root, *idxSpec);
15841598
luaContent = mcpp::fetcher::Fetcher::read_xpkg_lua_from_path(
1585-
idxSpec->path, shortName);
1599+
indexPath, shortName);
15861600
} else if (idxSpec && !idxSpec->is_builtin()) {
15871601
luaContent = mcpp::fetcher::Fetcher::read_xpkg_lua_from_project_data(
15881602
*root, ns, shortName);
@@ -1930,6 +1944,7 @@ prepare_build(bool print_fingerprint,
19301944

19311945
dep_manifests.push_back(
19321946
std::make_unique<mcpp::manifest::Manifest>(std::move(stagedManifest)));
1947+
dep_cache_indices.push_back(cache_index_name(key.ns));
19331948
packages.push_back({secStage, *dep_manifests.back()});
19341949
auto added = propagateIncludeDirs(secStage, *dep_manifests.back());
19351950

@@ -2154,6 +2169,7 @@ prepare_build(bool print_fingerprint,
21542169
// by depIndex (the SemVer merger needs to overwrite the slot).
21552170
dep_manifests.push_back(
21562171
std::make_unique<mcpp::manifest::Manifest>(std::move(*dep_manifest)));
2172+
dep_cache_indices.push_back(cache_index_name(key.ns));
21572173
packages.push_back({dep_root, *dep_manifests.back()});
21582174

21592175
// Record this dep as resolved so future encounters of the same
@@ -2288,6 +2304,27 @@ prepare_build(bool print_fingerprint,
22882304
if (cfg2) {
22892305
std::error_code mkEc;
22902306
std::filesystem::create_directories(ctx.outputDir, mkEc);
2307+
auto usable_object_rel = [](const std::filesystem::path& rel)
2308+
-> std::optional<std::string>
2309+
{
2310+
auto s = rel.generic_string();
2311+
if (s.empty() || s == "." || s == ".." || s.starts_with("../")) {
2312+
return std::nullopt;
2313+
}
2314+
return s;
2315+
};
2316+
auto object_cache_path = [&](const std::filesystem::path& objectPath) {
2317+
if (objectPath.is_absolute()) {
2318+
if (auto s = usable_object_rel(
2319+
objectPath.lexically_relative(ctx.outputDir / "obj"))) {
2320+
return *s;
2321+
}
2322+
}
2323+
if (auto s = usable_object_rel(objectPath.lexically_relative("obj"))) {
2324+
return *s;
2325+
}
2326+
return objectPath.filename().generic_string();
2327+
};
22912328
for (std::size_t i = 1; i < packages.size(); ++i) { // skip [0] = main
22922329
const auto& pkgRoot = packages[i];
22932330
const auto& depName = pkgRoot.manifest.package.name;
@@ -2312,7 +2349,9 @@ prepare_build(bool print_fingerprint,
23122349
mcpp::bmi_cache::CacheKey key {
23132350
.mcppHome = (*cfg2)->mcppHome,
23142351
.fingerprint = fp.hex,
2315-
.indexName = (*cfg2)->defaultIndex,
2352+
.indexName = i - 1 < dep_cache_indices.size()
2353+
? dep_cache_indices[i - 1]
2354+
: (*cfg2)->defaultIndex,
23162355
.packageName = depName,
23172356
.version = depVer,
23182357
.bmiDirName = std::string(bmiT.bmiDir),
@@ -2336,7 +2375,7 @@ prepare_build(bool print_fingerprint,
23362375
bmi += std::string(bmiT.bmiExt);
23372376
arts.bmiFiles.push_back(std::move(bmi));
23382377
}
2339-
arts.objFiles.push_back(cu.object.filename().string());
2378+
arts.objFiles.push_back(object_cache_path(cu.object));
23402379
}
23412380

23422381
if (mcpp::bmi_cache::is_cached(key)) {

‎tests/e2e/49_bmi_cache_nested_custom_index.sh‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
#!/usr/bin/env bash
2+
# requires: gcc fresh-sandbox
3+
# Regression test for dependency BMI cache population when a custom-index
4+
# package produces object files under collision-avoidance subdirectories.
5+
set -e
6+
7+
TMP=$(mktemp -d)
8+
trap "rm -rf $TMP" EXIT
9+
10+
export MCPP_HOME="$TMP/mcpp-home"
11+
source "$(dirname "$0")/_inherit_toolchain.sh"
12+
13+
INDEX_DIR="$TMP/local-index"
14+
mkdir -p "$INDEX_DIR/pkgs/c"
15+
cat > "$INDEX_DIR/pkgs/c/collision-lib.lua" <<'EOF'
16+
package = {
17+
spec = "1",
18+
name = "collision-lib",
19+
description = "Collision object path package",
20+
licenses = {"MIT"},
21+
type = "package",
22+
xpm = {
23+
linux = {
24+
["1.0.0"] = {
25+
url = "https://example.invalid/collision-lib-1.0.0.tar.gz",
26+
sha256 = "0000000000000000000000000000000000000000000000000000000000000000",
27+
},
28+
},
29+
},
30+
mcpp = {
31+
language = "c++23",
32+
import_std = true,
33+
sources = { "src/**/*.cppm" },
34+
targets = { ["collision-lib"] = { kind = "lib" } },
35+
deps = {},
36+
},
37+
}
38+
EOF
39+
40+
mkdir -p "$TMP/project/app"
41+
cd "$TMP/project/app"
42+
43+
mkdir -p src .mcpp/.xlings/data/xpkgs/local-dev.collision-lib/1.0.0/src/a \
44+
.mcpp/.xlings/data/xpkgs/local-dev.collision-lib/1.0.0/src/b
45+
46+
cat > src/main.cpp <<'EOF'
47+
import std;
48+
import collision.a;
49+
import collision.b;
50+
int main() {
51+
std::println("{} {}", collision_a(), collision_b());
52+
return 0;
53+
}
54+
EOF
55+
56+
cat > .mcpp/.xlings/data/xpkgs/local-dev.collision-lib/1.0.0/src/a/foo.cppm <<'EOF'
57+
export module collision.a;
58+
export int collision_a() { return 1; }
59+
EOF
60+
61+
cat > .mcpp/.xlings/data/xpkgs/local-dev.collision-lib/1.0.0/src/b/foo.cppm <<'EOF'
62+
export module collision.b;
63+
export int collision_b() { return 2; }
64+
EOF
65+
66+
cat > mcpp.toml <<EOF
67+
[package]
68+
name = "app"
69+
version = "0.1.0"
70+
71+
[indices]
72+
local-dev = { path = "$INDEX_DIR" }
73+
74+
[dependencies]
75+
"local-dev.collision-lib" = "1.0.0"
76+
77+
[targets.app]
78+
kind = "bin"
79+
main = "src/main.cpp"
80+
EOF
81+
82+
"$MCPP" build > build.log 2>&1 || { cat build.log; exit 1; }
83+
84+
if grep -q "Fetching custom index repos" build.log; then
85+
echo "FAIL: local path index should not trigger xlings update noise"
86+
cat build.log
87+
exit 1
88+
fi
89+
90+
if grep -q "bmi cache populate failed" build.log; then
91+
echo "FAIL: BMI cache populate warning should not appear"
92+
cat build.log
93+
exit 1
94+
fi
95+
96+
manifest="$(find "$MCPP_HOME/bmi" -path "*/deps/local-dev/*collision-lib@1.0.0/manifest.txt" | head -1)"
97+
[[ -n "$manifest" ]] || {
98+
echo "FAIL: missing custom-index BMI manifest"
99+
find "$MCPP_HOME/bmi" -maxdepth 6 -type f | sort
100+
cat build.log
101+
exit 1
102+
}
103+
104+
grep -Eq '^obj: .+/foo\.m\.o$' "$manifest" || {
105+
echo "FAIL: manifest did not preserve nested object path"
106+
cat "$manifest"
107+
exit 1
108+
}
109+
110+
rm -rf target
111+
"$MCPP" build > build2.log 2>&1 || { cat build2.log; exit 1; }
112+
113+
grep -q "Cached local-dev.collision-lib v1.0.0" build2.log || {
114+
echo "FAIL: second cold build did not reuse BMI cache"
115+
cat build2.log
116+
exit 1
117+
}
118+
119+
echo "OK"

0 commit comments

Comments
 (0)