Skip to content

Commit 0ef96b4

Browse files
committed
xlings: replace a vendored xlings from the newest source, once per process (#744)
One function chooses the source: MCPP_VENDORED_XLINGS when set, otherwise the newer of the xlings released with this mcpp and the xlings on PATH, the released one on a tie. The check that decides whether to replace the vendored binary and the copy that replaces it both use its answer, so the version stated is the version copied. Before, the check took the first source that existed, so a released copy older than the pin hid a newer xlings on PATH and mcpp stated that no newer source was available. A home settled once in a process is not examined again, and Updating and Note are each stated at most once. The version of the vendored binary is asked once per process and kept under the home, keyed by the binary's path, size and modification time, so a command that loads the configuration no longer runs xlings --version (0.35 s measured) when the binary has not changed. e2e 846 covers the three source cases; it fails on 2026.9.30.1 with the false note of #744.
1 parent e477852 commit 0ef96b4

4 files changed

Lines changed: 470 additions & 78 deletions

File tree

‎src/config.cppm‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -814,7 +814,8 @@ std::expected<GlobalConfig, ConfigError> load_or_init(
814814
// 6. Acquire xlings binary if needed
815815
if (cfg.xlingsBinaryMode == "bundled") {
816816
auto xbin = mcpp::fallback::acquire_xlings_binary(
817-
cfg.xlingsBinary, quiet, kXlingsPinnedVersion);
817+
cfg.xlingsBinary, quiet, kXlingsPinnedVersion,
818+
cfg.metaCacheDir / "vendored-xlings.versions");
818819
if (!xbin) return std::unexpected(ConfigError{xbin.error()});
819820
} else if (cfg.xlingsBinaryMode == "system") {
820821
auto sysPath = mcpp::platform::fs::which(

‎src/fallback/xlings_binary.cppm‎

Lines changed: 165 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
// mcpp.fallback.xlings_binary — xlings binary acquisition chain.
22
//
3-
// Tries multiple strategies to obtain the xlings binary:
4-
// 1. MCPP_VENDORED_XLINGS env var (explicit override)
5-
// 2. the xlings released with this mcpp, `<prefix>/registry/bin/xlings`
6-
// 3. system `which xlings`
7-
// 4. Fail with user-facing instructions
3+
// One source is chosen (select_xlings_source) from:
4+
// - MCPP_VENDORED_XLINGS (an explicit override, taken when set);
5+
// - the xlings released with this mcpp, `<prefix>/registry/bin/xlings`;
6+
// - the xlings on PATH;
7+
// the newer of the last two, the released one on a tie. With none, the error
8+
// states how to provide one.
89

910
module;
1011
#include <cstdio>
@@ -36,10 +37,35 @@ std::string vendored_xlings_version(const std::filesystem::path& bin);
3637
// released binary is the one that satisfies the pin by construction.
3738
std::filesystem::path released_xlings_source(const std::filesystem::path& destBin);
3839

39-
// The version the acquisition chain WOULD install, without installing it.
40-
// Empty when nothing is available. Replacing a vendored binary is only an
41-
// improvement when this is newer than what is already there.
42-
std::string candidate_source_version(const std::filesystem::path& destBin = {});
40+
// A place an xlings binary can be copied from, with the version it answers.
41+
struct XlingsSource {
42+
std::filesystem::path path;
43+
std::string version; // empty when it cannot be read
44+
std::string origin; // how the acquisition line names it
45+
};
46+
47+
// THE ONE ANSWER TO "WHICH SOURCE" (mcpp#744). `MCPP_VENDORED_XLINGS` when it
48+
// is set, as an explicit choice. Otherwise the newer of the xlings released
49+
// with this mcpp and the xlings on PATH, the released one on a tie or when the
50+
// PATH copy's version cannot be read. The check that decides whether to
51+
// replace a vendored binary and the copy that replaces it both use this
52+
// answer, so the version stated is the version copied. Before, the check took
53+
// the first source that existed, not the newest: a released copy older than
54+
// the pin hid a newer xlings on PATH, and mcpp stated that no newer source
55+
// was available.
56+
std::optional<XlingsSource> choose_xlings_source(std::optional<XlingsSource> override_,
57+
std::optional<XlingsSource> released,
58+
std::optional<XlingsSource> onPath);
59+
// The same choice over this process's sources.
60+
std::optional<XlingsSource> select_xlings_source(const std::filesystem::path& destBin = {});
61+
62+
// The version `bin` answers, asked at most once per process. With `memoFile`,
63+
// the answer is also kept across processes, keyed by the binary's path, size
64+
// and modification time: an update of xlings writes a new file and is asked
65+
// again. Asking costs a process of xlings, measured at 0.35 s, and every
66+
// command that loads the configuration asked (build wall-time plan, F6/W3).
67+
std::string known_xlings_version(const std::filesystem::path& bin,
68+
const std::filesystem::path& memoFile = {});
4369

4470
// True when `have` is strictly older than `want`, comparing dot-separated
4571
// numeric components. Anything unparseable answers false -- a version this
@@ -63,14 +89,27 @@ bool version_is_older(std::string_view have, std::string_view want);
6389
//
6490
// Strictly-older, not not-equal: a user who put a newer xlings there on
6591
// purpose must not be downgraded by an mcpp that happens to pin an older one.
92+
//
93+
// ONCE PER PROCESS. The configuration is loaded from about ten call sites, and
94+
// one `mcpp pack` over a workspace printed its note three times per member
95+
// (mcpp#744). A home settled once in a process is not examined again, and
96+
// `Updating` and `Note` are each stated at most once.
6697
std::expected<std::filesystem::path, std::string>
6798
acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false,
68-
std::string_view pinnedVersion = {}) {
99+
std::string_view pinnedVersion = {},
100+
const std::filesystem::path& versionMemo = {}) {
101+
static std::mutex m;
102+
static std::set<std::filesystem::path> settled;
103+
static bool noted = false, updated = false;
104+
std::lock_guard lock(m);
105+
if (settled.contains(destBin) && std::filesystem::exists(destBin)) return destBin;
69106
if (std::filesystem::exists(destBin)) {
70-
auto have = vendored_xlings_version(destBin);
107+
auto have = known_xlings_version(destBin, versionMemo);
71108
if (pinnedVersion.empty() || have.empty()
72-
|| !version_is_older(have, pinnedVersion))
109+
|| !version_is_older(have, pinnedVersion)) {
110+
settled.insert(destBin);
73111
return destBin;
112+
}
74113

75114
// Behind the pin -- but replacing is only an improvement if what we
76115
// would put there is actually newer. The acquisition chain below ends
@@ -81,28 +120,46 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false,
81120
// re-acquired, which replaced 2026.8.2.1 with the system's 0.4.51 --
82121
// older still, and equally missing the feature the check exists to
83122
// restore. Look before leaping.
84-
auto candidate = candidate_source_version(destBin);
85-
if (candidate.empty() || !version_is_older(have, candidate)) {
123+
auto candidate = select_xlings_source(destBin);
124+
if (!candidate || candidate->version.empty()
125+
|| !version_is_older(have, candidate->version)) {
86126
// stderr, not stdout. This is a remark about the environment,
87127
// not output of the command that happens to be running -- and
88128
// `mcpp test --json` promises every stdout line is NDJSON, a
89129
// promise this line broke the moment a machine fell behind the
90130
// pin (e2e 155).
91-
if (!quiet)
131+
if (!quiet && !noted)
92132
std::println(stderr,
93133
"{:>12} vendored xlings {} is older than the "
94134
"pinned {}, but no newer source is available "
95135
"(keeping it; run `xlings self update`)",
96136
"Note", have, pinnedVersion);
137+
noted = true;
138+
settled.insert(destBin);
97139
return destBin;
98140
}
99-
if (!quiet)
100-
std::println(stderr,
101-
"{:>12} vendored xlings {} -> {} (pinned {})",
102-
"Updating", have, candidate, pinnedVersion);
103141
std::error_code rec;
142+
std::filesystem::copy_file(candidate->path, destBin,
143+
std::filesystem::copy_options::overwrite_existing, rec);
144+
if (!rec) {
145+
std::filesystem::permissions(destBin,
146+
std::filesystem::perms::owner_exec
147+
| std::filesystem::perms::group_exec
148+
| std::filesystem::perms::others_exec,
149+
std::filesystem::perm_options::add, rec);
150+
if (!quiet && !updated)
151+
std::println(stderr,
152+
"{:>12} vendored xlings {} -> {} from {} (pinned {})",
153+
"Updating", have, candidate->version, candidate->origin,
154+
pinnedVersion);
155+
updated = true;
156+
// The version is known: it is the one just copied.
157+
(void)known_xlings_version(destBin, versionMemo);
158+
settled.insert(destBin);
159+
return destBin;
160+
}
161+
// The copy failed; the chain below tries again from the start.
104162
std::filesystem::remove(destBin, rec);
105-
// fall through and re-acquire
106163
}
107164

108165
std::error_code ec;
@@ -117,29 +174,11 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false,
117174
std::println("{}{} {}", std::string(W - verb.size(), ' '), verb, msg);
118175
};
119176

120-
// 1. Explicit override
121-
if (auto* e = std::getenv("MCPP_VENDORED_XLINGS"); e && *e) {
122-
std::filesystem::path src{e};
123-
if (std::filesystem::exists(src)) {
124-
std::filesystem::copy_file(src, destBin,
125-
std::filesystem::copy_options::overwrite_existing, ec);
126-
if (!ec) {
127-
std::filesystem::permissions(destBin,
128-
std::filesystem::perms::owner_exec
129-
| std::filesystem::perms::group_exec
130-
| std::filesystem::perms::others_exec,
131-
std::filesystem::perm_options::add, ec);
132-
if (!quiet) print_status("Bundled",
133-
std::format("xlings (from MCPP_VENDORED_XLINGS)"));
134-
return destBin;
135-
}
136-
}
137-
}
138-
139-
// 2. The xlings released with this mcpp. Ahead of the system copy, which
140-
// may be any version (see released_xlings_source).
141-
if (auto released = released_xlings_source(destBin); !released.empty()) {
142-
std::filesystem::copy_file(released, destBin,
177+
// The first acquisition takes the source a replacement would take
178+
// (select_xlings_source): the override, otherwise the newer of the
179+
// released copy and the PATH copy.
180+
if (auto src = select_xlings_source(destBin)) {
181+
std::filesystem::copy_file(src->path, destBin,
143182
std::filesystem::copy_options::overwrite_existing, ec);
144183
if (!ec) {
145184
std::filesystem::permissions(destBin,
@@ -148,34 +187,15 @@ acquire_xlings_binary(const std::filesystem::path& destBin, bool quiet = false,
148187
| std::filesystem::perms::others_exec,
149188
std::filesystem::perm_options::add, ec);
150189
if (!quiet) print_status("Bundled",
151-
std::format("xlings (released with this mcpp: {})", released.string()));
190+
std::format("xlings{} (from {}: {})",
191+
src->version.empty() ? std::string{} : " " + src->version,
192+
src->origin, src->path.string()));
193+
settled.insert(destBin);
152194
return destBin;
153195
}
154-
ec.clear();
155-
}
156-
157-
// 3. Copy from system (`which xlings`)
158-
auto xlings_name = std::string("xlings") + std::string(mcpp::platform::exe_suffix);
159-
auto sysXlings = mcpp::platform::fs::which(xlings_name);
160-
if (sysXlings) {
161-
std::string p = sysXlings->string();
162-
if (!p.empty() && std::filesystem::exists(p)) {
163-
std::filesystem::copy_file(p, destBin,
164-
std::filesystem::copy_options::overwrite_existing, ec);
165-
if (!ec) {
166-
std::filesystem::permissions(destBin,
167-
std::filesystem::perms::owner_exec
168-
| std::filesystem::perms::group_exec
169-
| std::filesystem::perms::others_exec,
170-
std::filesystem::perm_options::add, ec);
171-
if (!quiet) print_status("Bundled",
172-
std::format("xlings (copied from system: {})", p));
173-
return destBin;
174-
}
175-
}
176196
}
177197

178-
// 3. Fail with instructions
198+
// Nothing to copy: say how to provide one.
179199
return std::unexpected(std::format(
180200
"xlings binary not found. Either:\n"
181201
" - install via: curl -fsSL https://raw.githubusercontent.com/d2learn/xlings/refs/heads/main/tools/other/quick_install.sh | bash\n"
@@ -259,18 +279,86 @@ std::filesystem::path released_xlings_source(const std::filesystem::path& destBi
259279
return released;
260280
}
261281

262-
std::string candidate_source_version(const std::filesystem::path& destBin) {
282+
std::optional<XlingsSource> choose_xlings_source(std::optional<XlingsSource> override_,
283+
std::optional<XlingsSource> released,
284+
std::optional<XlingsSource> onPath) {
285+
if (override_) return override_;
286+
if (!released) return onPath;
287+
if (!onPath || onPath->version.empty()) return released;
288+
if (released->version.empty() || version_is_older(released->version, onPath->version))
289+
return onPath;
290+
return released;
291+
}
292+
293+
std::optional<XlingsSource> select_xlings_source(const std::filesystem::path& destBin) {
294+
std::optional<XlingsSource> override_, released, onPath;
295+
std::error_code ec;
263296
if (const char* e = std::getenv("MCPP_VENDORED_XLINGS"); e && *e) {
264-
std::error_code ec;
265-
if (std::filesystem::exists(std::filesystem::path(e), ec))
266-
return vendored_xlings_version(std::filesystem::path(e));
297+
std::filesystem::path p{e};
298+
if (std::filesystem::exists(p, ec))
299+
override_ = XlingsSource{p, vendored_xlings_version(p), "MCPP_VENDORED_XLINGS"};
267300
}
268-
if (auto released = released_xlings_source(destBin); !released.empty())
269-
return vendored_xlings_version(released);
270-
if (auto sys = mcpp::platform::fs::which(
271-
std::string("xlings") + std::string(mcpp::platform::exe_suffix)))
272-
return vendored_xlings_version(*sys);
273-
return {};
301+
if (!override_) {
302+
if (auto r = released_xlings_source(destBin); !r.empty())
303+
released = XlingsSource{r, vendored_xlings_version(r), "the release of this mcpp"};
304+
if (auto sys = mcpp::platform::fs::which(
305+
std::string("xlings") + std::string(mcpp::platform::exe_suffix))) {
306+
const bool isDest = !destBin.empty() && std::filesystem::equivalent(*sys, destBin, ec);
307+
if (!isDest && std::filesystem::exists(*sys, ec))
308+
onPath = XlingsSource{*sys, vendored_xlings_version(*sys), "PATH"};
309+
}
310+
}
311+
return choose_xlings_source(std::move(override_), std::move(released), std::move(onPath));
312+
}
313+
314+
std::string known_xlings_version(const std::filesystem::path& bin,
315+
const std::filesystem::path& memoFile) {
316+
std::error_code ec;
317+
const auto size = std::filesystem::file_size(bin, ec);
318+
if (ec) return {};
319+
const auto mtime = std::filesystem::last_write_time(bin, ec);
320+
if (ec) return {};
321+
const auto u8 = bin.generic_u8string();
322+
const auto key = std::format("{}\t{}\t{}",
323+
std::string(reinterpret_cast<const char*>(u8.data()), u8.size()),
324+
size, mtime.time_since_epoch().count());
325+
326+
static std::mutex m;
327+
static std::map<std::string, std::string> answered; // key -> version
328+
std::lock_guard lock(m);
329+
if (auto it = answered.find(key); it != answered.end()) return it->second;
330+
331+
// The memo holds one line per binary: `<path>\t<size>\t<mtime>\t<version>`.
332+
std::vector<std::string> lines;
333+
if (!memoFile.empty()) {
334+
std::ifstream is(memoFile, std::ios::binary);
335+
for (std::string line; std::getline(is, line);) {
336+
if (line.starts_with(key + "\t")) {
337+
auto v = line.substr(key.size() + 1);
338+
if (!v.empty()) return answered[key] = v;
339+
}
340+
lines.push_back(std::move(line));
341+
}
342+
}
343+
auto version = vendored_xlings_version(bin);
344+
answered[key] = version;
345+
if (!memoFile.empty() && !version.empty()) {
346+
// Replace this binary's line, keep the others, and write the file whole
347+
// beside itself before renaming it into place.
348+
const auto prefix = key.substr(0, key.find('\t') + 1);
349+
std::erase_if(lines, [&](const std::string& l) { return l.starts_with(prefix); });
350+
lines.push_back(key + "\t" + version);
351+
std::filesystem::create_directories(memoFile.parent_path(), ec);
352+
auto tmp = memoFile;
353+
tmp += std::format(".tmp-{}", std::chrono::steady_clock::now().time_since_epoch().count());
354+
{
355+
std::ofstream os(tmp, std::ios::binary | std::ios::trunc);
356+
for (auto const& l : lines) os << l << '\n';
357+
}
358+
std::filesystem::rename(tmp, memoFile, ec);
359+
if (ec) std::filesystem::remove(tmp, ec);
360+
}
361+
return version;
274362
}
275363

276364
} // namespace mcpp::fallback

0 commit comments

Comments
 (0)