Skip to content

Commit c5a8739

Browse files
committed
fix(index): quiet refresh and canonical cache labels
Closes #90
1 parent 5423b21 commit c5a8739

5 files changed

Lines changed: 366 additions & 11 deletions

File tree

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
# Index Refresh And Dependency Cache Label Fix
2+
3+
> Date: 2026-05-31 | Status: active | Branch:
4+
> `codex/quiet-index-refresh-cache-labels`
5+
6+
Tracking issue: https://github.com/mcpp-community/mcpp/issues/90
7+
8+
## Scope
9+
10+
This document tracks the mcpp-side fixes found while validating `mcpp build`
11+
against the `xlings` project.
12+
13+
The current work is intentionally limited to build-tool behavior:
14+
15+
- make automatic package-index refresh quiet from the mcpp user's perspective;
16+
- make index freshness use a reliable mcpp-owned timestamp marker;
17+
- make dependency cache status use canonical dependency identities;
18+
- keep local xlings validation as the integration proof.
19+
20+
## Findings
21+
22+
### 1. Auto-refresh leaks xlings update output
23+
24+
`mcpp build` calls `ensure_index_fresh()` when a project has version-source
25+
dependencies. That path currently calls `xlings update` with `quiet=false`.
26+
27+
Observed output:
28+
29+
```text
30+
Updating package index (auto-refresh)
31+
[1/7] awesome::/home/speak/.mcpp/registry/data/xim-index-repos/...
32+
...
33+
index updated
34+
```
35+
36+
The `[N/M] awesome::...` lines are xlings internals. mcpp users should only see
37+
the mcpp-level status line unless they explicitly run a verbose/manual index
38+
update command.
39+
40+
### 2. Freshness check uses an unstable directory mtime
41+
42+
`is_index_fresh()` currently checks:
43+
44+
```text
45+
~/.mcpp/registry/data/mcpplibs/pkgs
46+
```
47+
48+
Local evidence showed:
49+
50+
```text
51+
mcpplibs/pkgs 2026-05-31 01:55:40
52+
mcpplibs/.git/FETCH_HEAD 2026-05-31 04:31:24
53+
xim-indexrepos.json 2026-05-31 04:31:27
54+
```
55+
56+
So `xlings update` can refresh the repository without updating the `pkgs/`
57+
directory mtime. Once the TTL expires, every full `prepare_build()` can decide
58+
that the index is stale again.
59+
60+
### 3. Dependency status labels can say `Compiling` for cached deps
61+
62+
For xlings:
63+
64+
```toml
65+
[dependencies.mcpplibs]
66+
xpkg = "0.0.41"
67+
tinyhttps = "0.2.3"
68+
```
69+
70+
The UI prints dependency names as `mcpplibs.xpkg` and `mcpplibs.tinyhttps`.
71+
The cache hit label is currently built from the dependency package's own
72+
`mcpp.toml` name. For tinyhttps that manifest uses:
73+
74+
```toml
75+
namespace = "mcpplibs"
76+
name = "tinyhttps"
77+
version = "0.2.3"
78+
```
79+
80+
The cache directory is therefore:
81+
82+
```text
83+
~/.mcpp/bmi/<fingerprint>/deps/mcpplibs/tinyhttps@0.2.3
84+
```
85+
86+
The UI compares `tinyhttps` against `mcpplibs.tinyhttps`, so it can print
87+
`Compiling mcpplibs.tinyhttps` even when the cache entry exists.
88+
89+
### 4. libxpkg 0.0.41 has stale embedded mcpp metadata
90+
91+
The installed `mcpplibs.xpkg@0.0.41` archive contains:
92+
93+
```toml
94+
name = "xpkg"
95+
version = "0.0.39"
96+
```
97+
98+
This causes mcpp to cache the resolved package as:
99+
100+
```text
101+
deps/mcpplibs/xpkg@0.0.39
102+
```
103+
104+
while the consumer dependency remains `mcpplibs.xpkg v0.0.41`.
105+
106+
## Implementation Plan
107+
108+
- [x] Add focused regression coverage for default-index refresh quietness.
109+
- [x] Add a reliable mcpp-owned index freshness marker after successful update.
110+
- [x] Make automatic refresh call `update_index(..., quiet=true)`.
111+
- [x] Keep explicit/manual index update output available for diagnostics.
112+
- [x] Canonicalize dependency identity used for cache-hit labels.
113+
- [x] Avoid using stale embedded package version as the user-facing resolved
114+
dependency version when the index resolution already knows the requested
115+
version.
116+
- [x] Validate with the local xlings checkout using the new mcpp binary.
117+
- [ ] Push a draft PR and use it as the multi-commit checkpoint.
118+
119+
## Verification Plan
120+
121+
- [x] Run targeted unit/e2e coverage in mcpp.
122+
- [x] Build mcpp itself.
123+
- [x] Run `mcpp build` in `/home/speak/test/tmp/xlings` with the new binary.
124+
- [x] Confirm auto-refresh no longer prints xlings `[N/M] index::path` lines.
125+
- [x] Confirm repeated xlings full prepare does not refresh the index again
126+
while the marker is inside TTL.
127+
- [x] Confirm cached dependencies display as `Cached` when their artifacts are
128+
staged.
129+
- [ ] Record CI status on the PR.
130+
131+
## Dynamic Notes
132+
133+
- `mcpp build` has a 0.01s fast path when `build.ninja` is newer than source
134+
files; the noisy path happens when full `prepare_build()` runs.
135+
- A no-change `ninja -C target/... -n` in the local xlings checkout reported
136+
`ninja: no work to do`, so the status-line issue is partly UI/cache identity
137+
rather than always an actual compiler invocation.
138+
- Added regression tests:
139+
- `tests/unit/test_xlings.cpp` now requires `.mcpp-index-updated` for a
140+
fresh default index.
141+
- `tests/e2e/53_namespaced_cache_label.sh` verifies quiet auto-refresh,
142+
marker creation, and canonical `Cached compat.widget v1.0.0` output.
143+
- Local mcpp verification:
144+
- `mcpp test -- --gtest_filter=XlingsIndexFreshness.*` passed; the command
145+
built and ran all 16 test binaries, with `test_xlings` covering the filter.
146+
- `mcpp build --no-cache` passed and produced
147+
`target/x86_64-linux-gnu/4d24c8b57fdbbbb4/bin/mcpp`.
148+
- `MCPP=.../bin/mcpp bash tests/e2e/49_bmi_cache_nested_custom_index.sh`
149+
passed.
150+
- `MCPP=.../bin/mcpp bash tests/e2e/52_local_path_namespaced_index.sh`
151+
passed.
152+
- `MCPP=.../bin/mcpp bash tests/e2e/53_namespaced_cache_label.sh` passed.
153+
- Local xlings verification with the new mcpp binary:
154+
- First `build --print-fingerprint`: exit `0`, displayed only
155+
`Updating package index (auto-refresh)` with no xlings `[N/M]` lines.
156+
- Second `build --print-fingerprint`: exit `0`, no index refresh line, and
157+
`mcpplibs.tinyhttps` / `mcpplibs.xpkg` displayed as `Cached`.
158+
- Marker was written at
159+
`~/.mcpp/registry/data/mcpplibs/.mcpp-index-updated`.

src/cli.cppm

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1514,11 +1514,16 @@ prepare_build(bool print_fingerprint,
15141514
// unique_ptr is not load-bearing for liveness — it's a leftover from
15151515
// an earlier design and harmless).
15161516
std::vector<std::unique_ptr<mcpp::manifest::Manifest>> dep_manifests;
1517-
std::vector<std::string> dep_cache_indices;
15181517
auto cache_index_name = [](std::string_view ns) {
15191518
if (ns.empty()) return std::string(mcpp::pm::kDefaultNamespace);
15201519
return std::string(ns);
15211520
};
1521+
struct DepCacheIdentity {
1522+
std::string indexName;
1523+
std::string packageName;
1524+
std::string version;
1525+
};
1526+
std::vector<DepCacheIdentity> dep_cache_identities;
15221527

15231528
struct ResolvedKey {
15241529
std::string ns;
@@ -2049,7 +2054,11 @@ prepare_build(bool print_fingerprint,
20492054

20502055
dep_manifests.push_back(
20512056
std::make_unique<mcpp::manifest::Manifest>(std::move(stagedManifest)));
2052-
dep_cache_indices.push_back(cache_index_name(key.ns));
2057+
dep_cache_identities.push_back({
2058+
.indexName = cache_index_name(key.ns),
2059+
.packageName = mangled,
2060+
.version = spec.version,
2061+
});
20532062
packages.push_back({secStage, *dep_manifests.back()});
20542063
auto added = propagateIncludeDirs(secStage, *dep_manifests.back());
20552064

@@ -2134,6 +2143,8 @@ prepare_build(bool print_fingerprint,
21342143

21352144
it->second.version = *merged;
21362145
it->second.includeDirsAdded = std::move(added);
2146+
if (it->second.depIndex < dep_cache_identities.size())
2147+
dep_cache_identities[it->second.depIndex].version = *merged;
21372148

21382149
// Walk the *new* manifest's deps so their constraints feed
21392150
// future merges. Already-resolved children dedup via the
@@ -2274,7 +2285,13 @@ prepare_build(bool print_fingerprint,
22742285
// by depIndex (the SemVer merger needs to overwrite the slot).
22752286
dep_manifests.push_back(
22762287
std::make_unique<mcpp::manifest::Manifest>(std::move(*dep_manifest)));
2277-
dep_cache_indices.push_back(cache_index_name(key.ns));
2288+
dep_cache_identities.push_back({
2289+
.indexName = cache_index_name(key.ns),
2290+
.packageName = name,
2291+
.version = sourceKind == "version"
2292+
? spec.version
2293+
: dep_manifests.back()->package.version,
2294+
});
22782295
packages.push_back({dep_root, *dep_manifests.back()});
22792296

22802297
// Record this dep as resolved so future encounters of the same
@@ -2433,8 +2450,17 @@ prepare_build(bool print_fingerprint,
24332450
};
24342451
for (std::size_t i = 1; i < packages.size(); ++i) { // skip [0] = main
24352452
const auto& pkgRoot = packages[i];
2436-
const auto& depName = pkgRoot.manifest.package.name;
2437-
const auto& depVer = pkgRoot.manifest.package.version;
2453+
const auto* depIdent = i - 1 < dep_cache_identities.size()
2454+
? &dep_cache_identities[i - 1]
2455+
: nullptr;
2456+
const auto fallbackName = pkgRoot.manifest.package.namespace_.empty()
2457+
? pkgRoot.manifest.package.name
2458+
: std::format("{}.{}", pkgRoot.manifest.package.namespace_,
2459+
pkgRoot.manifest.package.name);
2460+
const auto& depName = depIdent ? depIdent->packageName : fallbackName;
2461+
const auto& depVer = depIdent && !depIdent->version.empty()
2462+
? depIdent->version
2463+
: pkgRoot.manifest.package.version;
24382464

24392465
// Find this dep's spec from the consumer manifest to know
24402466
// if it's path-based or version-based.
@@ -2455,8 +2481,8 @@ prepare_build(bool print_fingerprint,
24552481
mcpp::bmi_cache::CacheKey key {
24562482
.mcppHome = (*cfg2)->mcppHome,
24572483
.fingerprint = fp.hex,
2458-
.indexName = i - 1 < dep_cache_indices.size()
2459-
? dep_cache_indices[i - 1]
2484+
.indexName = depIdent
2485+
? depIdent->indexName
24602486
: (*cfg2)->defaultIndex,
24612487
.packageName = depName,
24622488
.version = depVer,

src/xlings.cppm

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,34 @@ void print_status(std::string_view verb, std::string_view msg) {
270270
}
271271
}
272272

273+
std::filesystem::path default_index_dir(const Env& env) {
274+
return paths::index_data(env) / "mcpplibs";
275+
}
276+
277+
std::filesystem::path default_index_pkgs_dir(const Env& env) {
278+
return default_index_dir(env) / "pkgs";
279+
}
280+
281+
std::filesystem::path default_index_refresh_marker(const Env& env) {
282+
return default_index_dir(env) / ".mcpp-index-updated";
283+
}
284+
285+
void mark_default_index_refreshed(const Env& env) {
286+
if (!env.projectDir.empty()) return;
287+
if (!std::filesystem::exists(default_index_pkgs_dir(env))) return;
288+
289+
std::error_code ec;
290+
std::filesystem::create_directories(default_index_dir(env), ec);
291+
auto marker = default_index_refresh_marker(env);
292+
{
293+
std::ofstream os(marker, std::ios::trunc);
294+
if (!os) return;
295+
os << "ok\n";
296+
}
297+
std::filesystem::last_write_time(
298+
marker, std::filesystem::file_time_type::clock::now(), ec);
299+
}
300+
273301
void write_file(const std::filesystem::path& p, std::string_view content) {
274302
std::error_code ec;
275303
std::filesystem::create_directories(p.parent_path(), ec);
@@ -932,10 +960,13 @@ void ensure_ninja(const Env& env, bool quiet,
932960

933961
bool is_index_fresh(const Env& env, std::int64_t ttlSeconds) {
934962
std::error_code ec;
935-
auto pkgsDir = paths::index_data(env) / "mcpplibs" / "pkgs";
963+
auto pkgsDir = default_index_pkgs_dir(env);
936964
if (!std::filesystem::exists(pkgsDir)) return false;
937965

938-
auto newest = std::filesystem::last_write_time(pkgsDir, ec);
966+
auto marker = default_index_refresh_marker(env);
967+
if (!std::filesystem::exists(marker)) return false;
968+
969+
auto newest = std::filesystem::last_write_time(marker, ec);
939970
if (ec) return false;
940971

941972
// Check TTL
@@ -946,17 +977,19 @@ bool is_index_fresh(const Env& env, std::int64_t ttlSeconds) {
946977

947978
int update_index(const Env& env, bool quiet) {
948979
std::string cmd = build_command_prefix(env) + " update 2>&1";
949-
return mcpp::platform::process::run_streaming(cmd,
980+
int rc = mcpp::platform::process::run_streaming(cmd,
950981
[quiet](std::string_view line) {
951982
if (!quiet) std::println("{}", line);
952983
});
984+
if (rc == 0) mark_default_index_refreshed(env);
985+
return rc;
953986
}
954987

955988
void ensure_index_fresh(const Env& env, std::int64_t ttlSeconds, bool quiet) {
956989
if (is_index_fresh(env, ttlSeconds)) return;
957990
if (!quiet)
958991
print_status("Updating", "package index (auto-refresh)");
959-
update_index(env, quiet);
992+
update_index(env, /*quiet=*/true);
960993
}
961994

962995
} // namespace mcpp::xlings

0 commit comments

Comments
 (0)