Skip to content

Commit f886901

Browse files
committed
fix(platform): #254 host and target platform axes become distinct types; xpkg per-OS sections key on the target
mcpp resolves "which platform?" along two axes that coincide only for a native build: the host (where mcpp runs — correct for toolchain payloads and other host tools) and the target (where the produced binaries run — correct for anything compiled INTO the user's build). Both were std::string_view, so mixing them cost nothing and read identically, and they had drifted apart: every xpkg per-OS splice and every xpm version/asset table keyed on `mcpp::platform::xpkg_platform`, a COMPILE-TIME HOST constant, while `[target.'cfg(...)']` was evaluated against the resolved target. The same decision, derived two different ways. Cross-compiling therefore spliced the host's section into a dependency being built for the target: its per-OS sources, flags, deps and prebuilt asset all came from the wrong leg. Native builds cannot observe this, which is why three-platform CI never caught it. Introduces mcpp.platform.axis: HostPlatform and TargetPlatform, with no conversion between them and no constructor from a bare string. APIs take the common PlatformKey base, so a function states which axis it accepts and a call site must NAME the axis it supplies. `synthesize_from_xpkg_lua`'s platform parameter loses its host default and becomes required — a silent default is precisely how this bug survived. The triple-vs-xpkg vocabulary translation ("macos" vs "macosx") moves into TargetPlatform::for_os so no call site has to remember it. The type change and the behaviour fix land together because they are the same edit: once the parameter is required, every call site must decide, and those decisions ARE the fix. Classified per site: prepare.cppm dep manifest synthesis (3), pm/resolver version+asset selection -> TARGET; these are code compiled into the user's build. toolchain/lifecycle version listing -> HOST; a toolchain payload runs on this machine. Its existing comment already said so. cmd_xpkg --all-os -> for_lint_of(); neither axis, and named awkwardly on purpose so that reads as deliberate at the call site. scaffold `mcpp add` and the legacy resolver overloads -> HOST, stated explicitly rather than inherited from a default. Tests: static_asserts that the axes are mutually inconvertible and neither is constructible from a string; per-OS splice selects each target's own section and none of the others; and a native-build guard asserting host == target produces an identical splice, which is the safety net for every existing platform's behaviour.
1 parent 0d74672 commit f886901

8 files changed

Lines changed: 293 additions & 74 deletions

File tree

‎src/build/prepare.cppm‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ export module mcpp.build.prepare;
1111

1212
import std;
1313
import mcpp.diag;
14+
import mcpp.platform.axis;
1415
import mcpp.libs.json;
1516
import mcpp.log;
1617
import mcpp.manifest;
@@ -595,6 +596,15 @@ prepare_build(bool print_fingerprint,
595596
return std::unexpected("no mcpp.toml found in current directory or any parent");
596597
}
597598

599+
// #254: everything compiled INTO this build is resolved for the TARGET —
600+
// an xpkg descriptor's per-OS sections (sources, flags, deps) and its xpm
601+
// asset/version table all describe code that will run on the target, not
602+
// on the machine building it. This used to be a compile-time host
603+
// constant, which is invisible natively (host == target) and picks the
604+
// wrong leg under --target.
605+
const auto targetPlatform = mcpp::platform::TargetPlatform::for_os(
606+
cfgpred::context_for(overrides.target_triple).os);
607+
598608
auto m = mcpp::manifest::load(*root / "mcpp.toml");
599609
if (!m) return std::unexpected(m.error().format());
600610

@@ -1395,7 +1405,7 @@ prepare_build(bool print_fingerprint,
13951405
// 0.0.10+: use structured namespace from DependencySpec.
13961406
auto resolved = mcpp::pm::resolve_semver(
13971407
s.namespace_, s.shortName.empty() ? depName : s.shortName,
1398-
s.version, fetcher);
1408+
s.version, fetcher, targetPlatform);
13991409
if (!resolved) return std::unexpected(resolved.error());
14001410
mcpp::ui::info("Resolved",
14011411
std::format("{} {} → v{}", depName, s.version, *resolved));
@@ -1587,7 +1597,7 @@ prepare_build(bool print_fingerprint,
15871597
}
15881598
if (field.kind == mcpp::manifest::McppField::TableBody) {
15891599
auto dm = mcpp::manifest::synthesize_from_xpkg_lua(
1590-
*luaContent, depName, version);
1600+
*luaContent, depName, version, targetPlatform);
15911601
if (!dm) return false;
15921602
for (auto const& [generatedPath, _] : dm->buildConfig.generatedFiles) {
15931603
if (!generatedPath.empty()) return true;
@@ -1635,7 +1645,7 @@ prepare_build(bool print_fingerprint,
16351645
auto field = mcpp::manifest::extract_mcpp_field(*luaContent);
16361646
if (field.kind == mcpp::manifest::McppField::TableBody) {
16371647
auto depManifest = mcpp::manifest::synthesize_from_xpkg_lua(
1638-
*luaContent, depName, version);
1648+
*luaContent, depName, version, targetPlatform);
16391649
if (!depManifest) {
16401650
return std::unexpected(std::format(
16411651
"dependency '{}': {}", depName, depManifest.error().format()));
@@ -1811,7 +1821,8 @@ prepare_build(bool print_fingerprint,
18111821
"(expected exactly one)", depName, field.value, matches.size()));
18121822
if (auto r = loadFrom(matches.front()); !r) return std::unexpected(r.error());
18131823
} else if (field.kind == mcpp::manifest::McppField::TableBody) {
1814-
auto dm = mcpp::manifest::synthesize_from_xpkg_lua(*luaContent, depName, version);
1824+
auto dm = mcpp::manifest::synthesize_from_xpkg_lua(
1825+
*luaContent, depName, version, targetPlatform);
18151826
if (!dm) return std::unexpected(std::format(
18161827
"dependency '{}': {}", depName, dm.error().format()));
18171828
warn_unknown_xpkg_keys(*dm, depName);
@@ -2396,7 +2407,7 @@ prepare_build(bool print_fingerprint,
23962407
key.ns, key.shortName,
23972408
it->second.constraint,
23982409
item.originalConstraint,
2399-
fetcher);
2410+
fetcher, targetPlatform);
24002411
if (!merged) {
24012412
// Level 1 fallback: multi-version mangling. Two
24022413
// versions can't be reconciled by SemVer, but they

‎src/cli/cmd_xpkg.cppm‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export module mcpp.cli.cmd_xpkg;
1818
import std;
1919
import mcpplibs.cmdline;
2020
import mcpp.manifest;
21+
import mcpp.platform.axis;
2122
import mcpp.ui;
2223

2324
namespace mcpp::cli {
@@ -90,7 +91,8 @@ export int cmd_xpkg_parse(const mcpplibs::cmdline::ParsedArgs& parsed) {
9091
std::map<std::string, std::vector<std::string>> versions;
9192
std::string anyVersion;
9293
for (auto plat : kPlatforms) {
93-
auto v = mcpp::manifest::list_xpkg_versions(lua, plat);
94+
auto v = mcpp::manifest::list_xpkg_versions(
95+
lua, mcpp::platform::TargetPlatform::for_lint_of(plat));
9496
if (!v.empty() && anyVersion.empty()) anyVersion = v.front();
9597
versions.emplace(std::string(plat), std::move(v));
9698
}
@@ -130,7 +132,8 @@ export int cmd_xpkg_parse(const mcpplibs::cmdline::ParsedArgs& parsed) {
130132
continue;
131133
}
132134
auto pm = mcpp::manifest::synthesize_from_xpkg_lua(
133-
lua, fqn, anyVersion, plat);
135+
lua, fqn, anyVersion,
136+
mcpp::platform::TargetPlatform::for_lint_of(plat));
134137
if (!pm) {
135138
mcpp::ui::error(std::format("{} [{}]: {}", file, plat,
136139
pm.error().format()));
@@ -153,7 +156,10 @@ export int cmd_xpkg_parse(const mcpplibs::cmdline::ParsedArgs& parsed) {
153156
}
154157

155158
// The parse users get at build time — same function, same grammar.
156-
auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, fqn, anyVersion);
159+
// Host axis here: `mcpp xpkg parse` with no --all-os is inspecting what
160+
// THIS machine would read, which is a diagnostic question about the host.
161+
auto m = mcpp::manifest::synthesize_from_xpkg_lua(
162+
lua, fqn, anyVersion, mcpp::platform::HostPlatform::current());
157163
if (!m) {
158164
mcpp::ui::error(std::format("{}: {}", file, m.error().format()));
159165
return 1;

‎src/manifest/xpkg.cppm‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import std;
88
import mcpp.pm.dep_spec;
99
import mcpp.pm.dependency_selector;
1010
import mcpp.platform;
11+
import mcpp.platform.axis;
1112

1213
export namespace mcpp::manifest {
1314

@@ -24,7 +25,8 @@ McppField extract_mcpp_field(std::string_view luaContent);
2425
// "windows") from an xpkg .lua's xpm.<platform> = { ["X.Y.Z"] = {...}, ... }.
2526
// Returns an empty vector if the platform table is missing or has no entries.
2627
std::vector<std::string>
27-
list_xpkg_versions(std::string_view luaContent, std::string_view platform);
28+
list_xpkg_versions(std::string_view luaContent,
29+
const mcpp::platform::PlatformKey& platform);
2830
// Extract the `namespace` field from an xpkg .lua's `package = { ... }` block.
2931
// Returns empty string if the field is absent (legacy descriptors).
3032
std::string extract_xpkg_namespace(std::string_view luaContent);
@@ -92,16 +94,22 @@ bool xpkg_lua_identity_matches(std::string_view luaContent,
9294
// The resulting Manifest is in-memory only; sourcePath is set to the
9395
// supplied package name + version so error messages can refer to it.
9496
//
95-
// `osOverride` selects which per-OS section (linux/macosx/windows) is
96-
// spliced into the parse instead of the running host's — the seam behind
97-
// `mcpp xpkg parse --all-os`, which validates the sections a host build
98-
// never reads (they are skip-tabled, so a typo in the `windows` block is
99-
// otherwise invisible on linux CI). Empty = host platform (build path).
97+
// `platform` selects which per-OS section (linux/macosx/windows) is spliced
98+
// into the parse. It is REQUIRED and axis-typed (#254): the per-OS sections
99+
// carry sources, flags, deps and the xpm asset table, so a package compiled
100+
// into the user's build must be spliced for the TARGET, while a host tool
101+
// wants the HOST. Before #254 this defaulted to a compile-time host
102+
// constant, which silently gave cross builds the wrong leg.
103+
//
104+
// TargetPlatform::for_lint_of() is the seam behind `mcpp xpkg parse
105+
// --all-os`, which validates the sections a build never reads (they are
106+
// skip-tabled, so a typo in the `windows` block is otherwise invisible on
107+
// linux CI).
100108
std::expected<Manifest, ManifestError>
101109
synthesize_from_xpkg_lua(std::string_view luaContent,
102110
std::string_view packageName,
103111
std::string_view packageVersion,
104-
std::string_view osOverride = {});
112+
const mcpp::platform::PlatformKey& platform);
105113

106114
// mcpp#237: the mcpp-segment key vocabulary is a CLOSED whitelist (the parse
107115
// loop's else-if chain). An unrecognised key is collected into
@@ -708,7 +716,9 @@ bool xpkg_lua_identity_matches(std::string_view luaContent,
708716
}
709717

710718
std::vector<std::string>
711-
list_xpkg_versions(std::string_view luaContent, std::string_view platform) {
719+
list_xpkg_versions(std::string_view luaContent,
720+
const mcpp::platform::PlatformKey& platformAxis) {
721+
const std::string_view platform = platformAxis.key();
712722
// Locate `xpm = { ... <platform> = { ["X.Y.Z"] = {...}, ... } ... }`.
713723
// We work on a sanitized copy so quoted version keys remain locatable
714724
// by their offsets in the original text.
@@ -873,7 +883,7 @@ std::expected<Manifest, ManifestError>
873883
synthesize_from_xpkg_lua(std::string_view luaContent,
874884
std::string_view packageName,
875885
std::string_view packageVersion,
876-
std::string_view osOverride)
886+
const mcpp::platform::PlatformKey& platform)
877887
{
878888
auto body = extract_mcpp_segment_body(luaContent);
879889
if (body.empty()) {
@@ -885,8 +895,7 @@ synthesize_from_xpkg_lua(std::string_view luaContent,
885895
std::format("xpkg-lua of {}@{}", packageName, packageVersion),
886896
0, 0});
887897
}
888-
const std::string_view osKey =
889-
osOverride.empty() ? mcpp::platform::xpkg_platform : osOverride;
898+
const std::string_view osKey = platform.key();
890899
if (auto platformBody = top_level_table_body_for_key(body, osKey);
891900
!platformBody.empty()) {
892901
body += "\n";

‎src/platform/axis.cppm‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// mcpp.platform.axis — the host/target platform axes, as distinct types.
2+
//
3+
// mcpp resolves "which platform?" along TWO axes that are equal only for a
4+
// native build:
5+
//
6+
// host — the machine mcpp is running on. Correct for host TOOLS: the
7+
// toolchain payload, ninja, patchelf.
8+
// target — the platform the produced binaries will run on. Correct for
9+
// anything compiled INTO the user's build: a dependency's
10+
// sources, flags, deps, and the prebuilt asset chosen for it.
11+
//
12+
// Both are spelled in the xpkg descriptor vocabulary (linux | macosx |
13+
// windows) and both were plain std::string_view, so mixing them cost
14+
// nothing and read identically. #254: every xpkg per-OS section and every
15+
// xpm version table was keyed on a compile-time HOST constant, while
16+
// `[target.'cfg(...)']` was evaluated against the RESOLVED TARGET — the same
17+
// decision derived two different ways. Cross-compiling therefore spliced the
18+
// host's section into a dependency built for the target. Native builds hide
19+
// it, because there the two axes coincide, which is why three-platform CI
20+
// never caught it.
21+
//
22+
// Making them separate types means an API states which axis it wants, and a
23+
// call site must name the axis it is supplying. There is deliberately no
24+
// conversion between them and no constructor from a bare string: the
25+
// interesting bugs here are all "the wrong axis was passed", and a string
26+
// parameter cannot express the difference.
27+
28+
export module mcpp.platform.axis;
29+
30+
import std;
31+
import mcpp.platform;
32+
33+
export namespace mcpp::platform {
34+
35+
// A resolved xpkg platform key. Only obtainable through one of the axis
36+
// types below, so possession of one implies somebody decided which axis it
37+
// came from.
38+
class PlatformKey {
39+
public:
40+
std::string_view key() const { return key_; }
41+
42+
bool operator==(const PlatformKey& other) const { return key_ == other.key_; }
43+
44+
protected:
45+
explicit PlatformKey(std::string key) : key_(std::move(key)) {}
46+
47+
private:
48+
std::string key_;
49+
};
50+
51+
// The machine mcpp itself is running on.
52+
class HostPlatform : public PlatformKey {
53+
public:
54+
static HostPlatform current() {
55+
return HostPlatform(std::string(mcpp::platform::xpkg_platform));
56+
}
57+
58+
private:
59+
explicit HostPlatform(std::string key) : PlatformKey(std::move(key)) {}
60+
};
61+
62+
// The platform the artifacts being produced will run on.
63+
class TargetPlatform : public PlatformKey {
64+
public:
65+
// From a resolved triple's `os` token. The triple vocabulary says
66+
// "macos" where xpkg descriptors say "macosx"; that translation lives
67+
// here so no caller has to remember it.
68+
static TargetPlatform for_os(std::string_view tripleOs) {
69+
if (tripleOs == "macos" || tripleOs == "macosx") return TargetPlatform("macosx");
70+
if (tripleOs == "windows") return TargetPlatform("windows");
71+
if (tripleOs == "linux") return TargetPlatform("linux");
72+
// Unknown/absent os token: fall back to the host, which is what the
73+
// whole code base did unconditionally before #254.
74+
return TargetPlatform(std::string(mcpp::platform::xpkg_platform));
75+
}
76+
77+
// For tooling that walks platforms it is not running on and is not
78+
// building for — `mcpp xpkg parse --all-os` lints every per-OS section.
79+
// Named awkwardly on purpose: it should be obvious at the call site that
80+
// this is neither axis.
81+
static TargetPlatform for_lint_of(std::string_view xpkgKey) {
82+
return TargetPlatform(std::string(xpkgKey));
83+
}
84+
85+
private:
86+
explicit TargetPlatform(std::string key) : PlatformKey(std::move(key)) {}
87+
};
88+
89+
} // namespace mcpp::platform

‎src/pm/resolver.cppm‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export module mcpp.pm.resolver;
1818
import std;
1919
import mcpp.manifest;
2020
import mcpp.platform;
21+
import mcpp.platform.axis;
2122
import mcpp.pm.compat;
2223
import mcpp.pm.package_fetcher;
2324
import mcpp.version_req;
@@ -44,15 +45,17 @@ bool is_version_constraint(std::string_view v);
4445
std::expected<std::string, std::string>
4546
resolve_semver(std::string_view ns, std::string_view shortName,
4647
std::string_view constraint,
47-
mcpp::pm::Fetcher& fetcher);
48+
mcpp::pm::Fetcher& fetcher,
49+
const mcpp::platform::PlatformKey& platform);
4850

4951
// Try to AND-merge two version constraints and resolve to a single
5052
// concrete version satisfying both. Uses structured (ns, shortName).
5153
std::expected<std::string, std::string>
5254
try_merge_semver(std::string_view ns, std::string_view shortName,
5355
std::string_view a,
5456
std::string_view b,
55-
mcpp::pm::Fetcher& fetcher);
57+
mcpp::pm::Fetcher& fetcher,
58+
const mcpp::platform::PlatformKey& platform);
5659

5760
// ─── Legacy overloads (COMPAT, remove in 1.0.0) ─────────────────────
5861

@@ -85,7 +88,8 @@ bool is_version_constraint(std::string_view v) {
8588
std::expected<std::string, std::string>
8689
resolve_semver(std::string_view ns, std::string_view shortName,
8790
std::string_view constraint,
88-
mcpp::pm::Fetcher& fetcher)
91+
mcpp::pm::Fetcher& fetcher,
92+
const mcpp::platform::PlatformKey& platform)
8993
{
9094
namespace vr = mcpp::version_req;
9195
auto qname = mcpp::pm::compat::qualified_name(ns, shortName);
@@ -105,11 +109,11 @@ resolve_semver(std::string_view ns, std::string_view shortName,
105109
qname, constraint, req.error()));
106110
}
107111

108-
auto rawVersions = mcpp::manifest::list_xpkg_versions(*luaContent, kXpkgPlatform);
112+
auto rawVersions = mcpp::manifest::list_xpkg_versions(*luaContent, platform);
109113
if (rawVersions.empty()) {
110114
return std::unexpected(std::format(
111115
"dependency '{}': index entry has no versions for platform '{}'",
112-
qname, kXpkgPlatform));
116+
qname, platform.key()));
113117
}
114118

115119
std::vector<vr::Version> parsed;
@@ -141,7 +145,8 @@ std::expected<std::string, std::string>
141145
try_merge_semver(std::string_view ns, std::string_view shortName,
142146
std::string_view a,
143147
std::string_view b,
144-
mcpp::pm::Fetcher& fetcher)
148+
mcpp::pm::Fetcher& fetcher,
149+
const mcpp::platform::PlatformKey& platform)
145150
{
146151
auto canon = [](std::string_view v) -> std::string {
147152
if (v.empty() || v == "*") return std::string{};
@@ -157,7 +162,7 @@ try_merge_semver(std::string_view ns, std::string_view shortName,
157162
else if (!cb.empty()) merged = cb;
158163
else merged = "*";
159164

160-
return resolve_semver(ns, shortName, merged, fetcher);
165+
return resolve_semver(ns, shortName, merged, fetcher, platform);
161166
}
162167

163168
// ─── Legacy overloads (COMPAT, remove in 1.0.0) ─────────────────────
@@ -168,8 +173,11 @@ resolve_semver(std::string_view name,
168173
mcpp::pm::Fetcher& fetcher)
169174
{
170175
auto resolved = mcpp::pm::compat::resolve_package_name(name, "");
176+
// Legacy overload: no target is threaded through it, so it names the host
177+
// axis explicitly rather than inheriting a silent default (#254).
171178
return resolve_semver(resolved.namespace_, resolved.shortName,
172-
constraint, fetcher);
179+
constraint, fetcher,
180+
mcpp::platform::HostPlatform::current());
173181
}
174182

175183
std::expected<std::string, std::string>
@@ -179,8 +187,10 @@ try_merge_semver(std::string_view name,
179187
mcpp::pm::Fetcher& fetcher)
180188
{
181189
auto resolved = mcpp::pm::compat::resolve_package_name(name, "");
190+
// Legacy overload — see the resolve_semver note above.
182191
return try_merge_semver(resolved.namespace_, resolved.shortName,
183-
a, b, fetcher);
192+
a, b, fetcher,
193+
mcpp::platform::HostPlatform::current());
184194
}
185195

186196
} // namespace mcpp::pm

‎src/scaffold/create.cppm‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import mcpp.fetcher;
1414
import mcpp.fetcher.progress;
1515
import mcpp.manifest;
1616
import mcpp.pm.compat;
17+
import mcpp.platform.axis;
1718
import mcpp.pm.resolver;
1819
import mcpp.scaffold;
1920
import mcpp.ui;
@@ -67,7 +68,10 @@ fetch_template_package(const mcpp::scaffold::TemplateSpec& spec) {
6768

6869
std::string version = spec.version;
6970
if (version.empty()) {
70-
auto v = mcpp::pm::resolve_semver(ns, shortName, "*", fetcher);
71+
// `mcpp add` has no target concept — the axis it means is the host,
72+
// named explicitly rather than inherited from a default (#254).
73+
auto v = mcpp::pm::resolve_semver(ns, shortName, "*", fetcher,
74+
mcpp::platform::HostPlatform::current());
7175
if (!v) return std::unexpected(v.error());
7276
version = *v;
7377
}

0 commit comments

Comments
 (0)