Skip to content

Commit 51579d7

Browse files
fix: accept space-separated toolchain remove specs (#808)
* fix: accept split toolchain remove specs * fix: toolchain remove takes an exact version, and says so when it is missing `remove` does not resolve a partial version the way `default` does (no resolve_version_match): `mcpp toolchain remove gcc 15` with 15.1.0 installed reports "gcc@15 is not installed". The help no longer claims a partial version works. A removal deletes one payload, so it names that payload. A family with no version (`mcpp toolchain remove gcc`) still printed "invalid spec 'gcc'", the message #284 reports as reading like a wrong family name. A spec the parser rejects now carries the parser's own reason, as `default` does, and a family alone says the exact version is missing. e2e 26 asserts the missing-version message and drops the repeated `install gcc@16.1.0`, which only re-ran an install that was already done. --------- Co-authored-by: Sunrisepeak <speakshen@163.com>
1 parent b5630e2 commit 51579d7

4 files changed

Lines changed: 53 additions & 8 deletions

File tree

‎src/cli.cppm‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -795,7 +795,14 @@ int run(int argc, char** argv) {
795795
"Default build target <triple> (omit = host)")))
796796
.subcommand(cl::App("remove")
797797
.description("Uninstall a toolchain")
798-
.arg(cl::Arg("spec").help("<family>@<version>").required())
798+
// Same dual-form as `default`: `gcc@16.1.0` or `gcc 16.1.0`.
799+
// NOT partial versions: `default` resolves `gcc 15` to the
800+
// highest installed 15.x.y, but a removal must name the one
801+
// payload it deletes, so the version here is exact.
802+
.arg(cl::Arg("spec")
803+
.help("<family>[@<version>] (exact version)")
804+
.required())
805+
.arg(cl::Arg("version").help("(optional, alternative to @-form)"))
799806
.option(cl::Option("target").takes_value().help(
800807
"Remove the payload for <triple> instead of the host one")))
801808
.action(wrap_rc(cmd_toolchain)))

‎src/cli/cmd_toolchain.cppm‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ export int cmd_toolchain(const mcpplibs::cmdline::ParsedArgs& parsed) {
5656
return mcpp::toolchain::toolchain_set_default(
5757
*cfg, sub_parsed.positional(0), sub_parsed.positional(1), targetArg);
5858
if (subname == "remove")
59-
return mcpp::toolchain::toolchain_remove(*cfg, sub_parsed.positional(0), targetArg);
59+
return mcpp::toolchain::toolchain_remove(
60+
*cfg, sub_parsed.positional(0), sub_parsed.positional(1), targetArg);
6061

6162
mcpp::ui::error(std::format("unknown toolchain subcommand '{}'", subname));
6263
return 2;

‎src/toolchain/lifecycle.cppm‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,19 +1269,31 @@ export int toolchain_set_default(const mcpp::config::GlobalConfig& cfg,
12691269
// `mcpp toolchain remove <spec> [--target <triple>]` — uninstall a payload.
12701270
export int toolchain_remove(const mcpp::config::GlobalConfig& cfg,
12711271
const std::string& pos0,
1272+
const std::string& pos1,
12721273
const std::string& targetArg = {}) {
12731274
auto xlEnv = mcpp::config::make_xlings_env(cfg);
1274-
auto parsedSpec = mcpp::toolchain::parse_toolchain_spec(pos0);
1275-
if (parsedSpec && mcpp::toolchain::is_system_toolchain(*parsedSpec)) {
1275+
auto parsedSpec = mcpp::toolchain::parse_toolchain_spec(pos0, pos1);
1276+
if (!parsedSpec) {
1277+
mcpp::ui::error(std::format(
1278+
"{}; e.g. `mcpp toolchain remove gcc 16.1.0`", parsedSpec.error()));
1279+
return 2;
1280+
}
1281+
if (mcpp::toolchain::is_system_toolchain(*parsedSpec)) {
12761282
mcpp::ui::error(
12771283
"msvc@system is the machine's own Visual Studio, managed by the "
12781284
"Visual Studio Installer — mcpp cannot remove it.\n"
12791285
" A toolset mcpp installed can be removed by naming it:\n"
12801286
" mcpp toolchain remove msvc@<toolset>");
12811287
return 1;
12821288
}
1283-
if (!parsedSpec || parsedSpec->version.empty()) {
1284-
mcpp::ui::error(std::format("invalid spec '{}'", pos0));
1289+
// A family alone is a well-formed spec that names no payload. Say
1290+
// that, rather than "invalid spec 'gcc'", which reads as if the family
1291+
// itself were wrong (#284).
1292+
if (parsedSpec->version.empty()) {
1293+
mcpp::ui::error(std::format(
1294+
"`toolchain remove` needs the exact version to uninstall; "
1295+
"e.g. `mcpp toolchain remove {0} <version>` or `{0}@<version>`",
1296+
mcpp::toolchain::family_name(parsedSpec->family)));
12851297
return 2;
12861298
}
12871299
mcpp::toolchain::print_compat_hint(*parsedSpec);

‎tests/e2e/26_toolchain_management.sh‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,10 @@ echo "$out" | grep -q '16.1.0' || { echo "FAIL: list missing version: $out"; exi
7171
grep -q 'Default' /tmp/_def.log || { cat /tmp/_def.log; echo "FAIL: no Default msg"; exit 1; }
7272
out=$("$MCPP" toolchain list 2>&1)
7373
echo "$out" | grep -q '\*' || { echo "FAIL: default mark missing: $out"; exit 1; }
74+
"$MCPP" toolchain default gcc 16.1.0 > /tmp/_def_split.log 2>&1 || {
75+
cat /tmp/_def_split.log; echo "FAIL: space-separated default failed"; exit 1; }
76+
grep -q 'Default' /tmp/_def_split.log || {
77+
cat /tmp/_def_split.log; echo "FAIL: no Default msg for space-separated form"; exit 1; }
7478

7579
# 6) Build now succeeds (project has no [toolchain]; uses global default)
7680
"$MCPP" build > /tmp/_build.log 2>&1 || { cat /tmp/_build.log; echo "FAIL: build failed"; exit 1; }
@@ -98,8 +102,29 @@ echo "$out" | grep -q 'default toolchain = gcc@16.1.0' || {
98102

99103
# 9) Remove
100104
rm -rf target
101-
"$MCPP" toolchain remove gcc@16.1.0 > /tmp/_rm.log 2>&1
102-
grep -q 'Removed' /tmp/_rm.log || { cat /tmp/_rm.log; echo "FAIL: no Removed msg"; exit 1; }
105+
# A family alone names no payload: the error says the version is missing,
106+
# not that the spec is invalid (which reads as a wrong family name, #284).
107+
rc=0
108+
"$MCPP" toolchain remove gcc > /tmp/_rm_nover.log 2>&1 || rc=$?
109+
[[ "$rc" -eq 2 ]] || { cat /tmp/_rm_nover.log; echo "FAIL: remove without a version should exit 2 (got $rc)"; exit 1; }
110+
grep -q 'needs the exact version' /tmp/_rm_nover.log || {
111+
cat /tmp/_rm_nover.log; echo "FAIL: remove without a version did not name the missing version"; exit 1; }
112+
"$MCPP" toolchain remove gcc 16.1.0 > /tmp/_rm.log 2>&1
113+
grep -q 'Removed' /tmp/_rm.log || { cat /tmp/_rm.log; echo "FAIL: no Removed msg for space-separated form"; exit 1; }
114+
# The issue's cross-target spelling should parse too and reach the payload
115+
# lookup, even when that target is not installed in this test workspace.
116+
rc=0
117+
"$MCPP" toolchain remove gcc 16.1.0 --target x86_64-linux-musl > /tmp/_rm_target.log 2>&1 || rc=$?
118+
[[ "$rc" -ne 0 ]] || { echo "FAIL: removing an absent target toolchain should fail"; exit 1; }
119+
grep -q 'is not installed' /tmp/_rm_target.log || {
120+
cat /tmp/_rm_target.log; echo "FAIL: split form with --target did not reach installed-payload check"; exit 1; }
121+
# The compact spelling remains valid too; after removal it should reach the
122+
# installed-payload check rather than fail spec parsing.
123+
rc=0
124+
"$MCPP" toolchain remove gcc@16.1.0 > /tmp/_rm_at.log 2>&1 || rc=$?
125+
[[ "$rc" -ne 0 ]] || { echo "FAIL: removing an absent toolchain should fail"; exit 1; }
126+
grep -q 'is not installed' /tmp/_rm_at.log || {
127+
cat /tmp/_rm_at.log; echo "FAIL: @-form remove did not reach installed-payload check"; exit 1; }
103128

104129
# 10) After removing, the toolchain dir is gone, but config.toml still
105130
# points to it as default. Verify list shows nothing for that compiler now.

0 commit comments

Comments
 (0)