Skip to content

Commit 26ec4c9

Browse files
committed
fix(pack): an unsupported --format value writes nothing to stdout
`mcpp pack --format bogus` must write NOTHING to stdout and exit 2. That is the machine-output contract, and 202_machine_output_contract.sh asserts it for exactly this command, because it is the path a client hits when it probes an mcpp for a capability -- the most common machine-facing failure, and the one that used to print to stdout. Moving the refusal from the CLI parser to after `prepare_build` broke it. It had to move: the set of valid values is a property of the RESOLVED GRAPH, so a refusal written in the parser could only compare against a constant, which is the coupling this whole mechanism exists to remove. But prepare narrates what it resolves, so the refusal now arrived after three lines on stdout. FAIL: unsupported value (pack) wrote to stdout: Resolving toolchain Measured on macos-arm64, and it would have failed on every platform -- the macOS shard is simply the one that reached it first. The fix is to be quiet until the value is validated, and only then. Nothing is lost when the value IS valid: the dispatch pass prepares a second time and prints the same lines, so a successful `pack --format <name>` narrates once rather than twice. `--format tar` and `--format dir` are untouched, because their values were never in question. The musl re-prepare is quieted on the same grounds: it also runs before the format has been validated. The assertion is added to 638_pack_format_dispatch.sh as well as living in 202. That is deliberate duplication: the tension is local to this feature -- the valid set needs the graph, and the graph narrates -- so the test for the feature should fail when the contract does, rather than only the general contract test noticing.
1 parent c04dd06 commit 26ec4c9

2 files changed

Lines changed: 34 additions & 0 deletions

File tree

src/pack/pipeline.cppm

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,24 @@ export int build_and_pack(Options opts, bool modeFromUser,
5757
ov.profile = opts.profile;
5858
ov.profile_fallback = "release";
5959

60+
// QUIET FOR A DISPATCHED FORMAT, AND ONLY UNTIL THE VALUE IS VALIDATED.
61+
//
62+
// `mcpp pack --format bogus` must write NOTHING to stdout and exit 2 --
63+
// the machine-output contract, asserted by
64+
// tests/e2e/202_machine_output_contract.sh, because this is the path a
65+
// client hits when it probes an mcpp for a capability. The set of valid
66+
// values is a property of the resolved graph, so the refusal cannot be
67+
// decided until prepare has run, and prepare narrates what it resolves.
68+
//
69+
// Nothing is lost when the value IS valid: the dispatch pass prepares a
70+
// second time and prints the same lines, so a successful
71+
// `pack --format <name>` narrates once rather than twice.
72+
const bool quietUntilValidated =
73+
opts.format == mcpp::pack::Format::Dispatched && !mcpp::ui::is_quiet();
74+
if (quietUntilValidated) mcpp::ui::set_quiet(true);
6075
auto ctx = mcpp::build::prepare_build(/*print_fp=*/false, /*includeDevDeps=*/false,
6176
/*extraTargets=*/{}, ov);
77+
if (quietUntilValidated) mcpp::ui::set_quiet(false);
6278
if (!ctx) {
6379
mcpp::ui::error(ctx.error());
6480
return 2;
@@ -91,7 +107,11 @@ export int build_and_pack(Options opts, bool modeFromUser,
91107
// overrides object left behind here would make that pass differ from
92108
// this build in a way nothing states.
93109
ov.target_triple = "x86_64-linux-musl";
110+
// Quiet on the same grounds as the first prepare: this one also runs
111+
// before `--format` has been validated.
112+
if (quietUntilValidated) mcpp::ui::set_quiet(true);
94113
auto ctx2 = mcpp::build::prepare_build(false, false, {}, ov);
114+
if (quietUntilValidated) mcpp::ui::set_quiet(false);
95115
if (!ctx2) { mcpp::ui::error(ctx2.error()); return 2; }
96116
ctx = std::move(ctx2);
97117
}

tests/e2e/638_pack_format_dispatch.sh

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,20 @@ grep -q "available in this build: tar, dir, zap" b2.log \
130130
# Nothing was compiled to find that out.
131131
grep -q "Compiling app" b2.log \
132132
&& { cat b2.log; echo "FAIL: the refusal arrived after a compile"; exit 1; }
133+
# AND NOTHING REACHED STDOUT. This is the path a client hits when it probes an
134+
# mcpp for a capability, so the machine-output contract
135+
# (202_machine_output_contract.sh) requires an empty stdout, a non-empty stderr
136+
# and exit 2. It is asserted here as well because the tension is local to this
137+
# feature: the valid set is a property of the resolved graph, so the refusal
138+
# cannot be decided until prepare has run -- and prepare narrates what it
139+
# resolves. Deciding it later is what broke the contract once.
140+
set +e
141+
so=$("$MCPP" pack --format bogus 2>/dev/null); rc=$?
142+
se=$("$MCPP" pack --format bogus 2>&1 >/dev/null)
143+
set -e
144+
[ -z "$so" ] || { echo "FAIL: the refusal wrote to stdout: $(echo "$so" | head -1)"; exit 1; }
145+
[ -n "$se" ] || { echo "FAIL: the refusal said nothing on stderr"; exit 1; }
146+
[ "$rc" = 2 ] || { echo "FAIL: the refusal exited $rc, expected 2"; exit 1; }
133147

134148
# ── 3. the dispatched format produces a file, and it saw the staged tree ───
135149
"$MCPP" pack --format zap > b3.log 2>&1 || { cat b3.log; echo "FAIL: pack --format zap failed"; exit 1; }

0 commit comments

Comments
 (0)