Skip to content

Commit 35e0f38

Browse files
committed
feat(toolchain): gcc@system is rejected, and says what to do instead
`@system` is available for MSVC only, and that is deliberate rather than incidental. xlings depends on the host as little as it can: a toolchain comes from a payload, which is what makes "the manifest says 14.44.35207" true on every machine instead of on the one that happened to have it. Offering `gcc@system` invites that uncertainty back, and the alternative is one command away. MSVC is the exception because Windows is. Visual Studio is frequently already installed and cannot always be redistributed, so refusing to use it would mean refusing to build. A platform fact, not a general capability -- so it is not generalised. Until now the spelling was merely unimplemented, which means it failed later, somewhere else, with a message about something else. It is now refused at the parse, with the version form and the reason for the one exception. Test asserts the refusal AND that the message names the alternative -- a refusal that does not is just a wall. Verified against a guard that never fires.
1 parent 1070e14 commit 35e0f38

2 files changed

Lines changed: 53 additions & 0 deletions

File tree

src/toolchain/registry.cppm

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,35 @@ parse_toolchain_spec(std::string compilerArg,
229229
"supported alias like mingw / musl-gcc)", compilerArg));
230230
}
231231

232+
// `@system` means "whatever this machine has", and it is deliberately
233+
// available for MSVC ONLY.
234+
//
235+
// xlings depends on the host as little as it can: a toolchain comes from
236+
// a payload, which is what makes "the manifest says 14.44.35207" true on
237+
// every machine instead of on the one that happened to have it. Offering
238+
// `gcc@system` would invite the uncertainty back in, and the alternative
239+
// is one command away.
240+
//
241+
// MSVC is the exception because Windows is: Visual Studio is frequently
242+
// already installed and cannot always be redistributed, so refusing to
243+
// use it would mean refusing to build. That is a platform fact, not a
244+
// general capability, so it is not generalised.
245+
//
246+
// Rejected here rather than left unimplemented: an unimplemented spelling
247+
// fails somewhere further in with a message about something else.
248+
if (norm->version == "system" && norm->family != "msvc") {
249+
return std::unexpected(std::format(
250+
"'{}@system' is not a thing — mcpp does not build with the "
251+
"machine's own {}.\n"
252+
" A toolchain comes from a payload, so a manifest means the same "
253+
"thing on every machine.\n"
254+
" Name a version instead: `{}@<version>` "
255+
"(`mcpp toolchain list` shows what is available).\n"
256+
" Only `msvc@system` exists, because Visual Studio cannot always "
257+
"be redistributed.",
258+
norm->family, norm->family, norm->family));
259+
}
260+
232261
ToolchainSpec spec;
233262
if (norm->family == "llvm") spec.family = Family::Llvm;
234263
else if (norm->family == "msvc") spec.family = Family::Msvc;

tests/unit/test_toolchain_msvc.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,30 @@ TEST(MsvcSpec, SystemOriginIsTheUnversionedSpec) {
8484
EXPECT_FALSE(is_system_toolchain(*gcc));
8585
}
8686

87+
TEST(MsvcSpec, OnlyMsvcHasASystemOrigin) {
88+
// xlings depends on the host as little as it can: a toolchain comes from
89+
// a payload, which is what makes "the manifest says 14.44.35207" true on
90+
// every machine rather than on the one that happened to have it.
91+
// `gcc@system` would invite that uncertainty back.
92+
//
93+
// MSVC is the exception because Windows is — Visual Studio is often
94+
// already installed and cannot always be redistributed. A platform fact,
95+
// not a general capability, so it is not generalised.
96+
//
97+
// Rejected rather than merely unimplemented: an unimplemented spelling
98+
// fails later, somewhere else, with a message about something else.
99+
for (auto s : {"gcc@system", "llvm@system", "clang@system"}) {
100+
auto spec = parse_toolchain_spec(s);
101+
ASSERT_FALSE(spec.has_value()) << s << " was accepted";
102+
// The message has to say what to do instead, or it is just a refusal.
103+
EXPECT_NE(spec.error().find("@<version>"), std::string::npos)
104+
<< s << ": " << spec.error();
105+
EXPECT_NE(spec.error().find("msvc@system"), std::string::npos)
106+
<< s << ": " << spec.error();
107+
}
108+
EXPECT_TRUE(parse_toolchain_spec("msvc@system").has_value());
109+
}
110+
87111
TEST(MsvcSpec, ToolsetVersionIsAManagedPayloadNotASystemSpec) {
88112
// The defect this closes: EVERY msvc spec used to be a system spec, so a
89113
// manifest could name a toolset and silently get whatever the machine

0 commit comments

Comments
 (0)