Skip to content

Commit 6163f86

Browse files
committed
fix(pack): the distributable is what the request introduced, not what names the staged tree
`mcpp pack --format <name>` reported as the distributable only those artifact actions that named `${mcpp.stage_dir}`, on the assumption that a distributable consumes the staged closure. NOT EVERY FORMAT DOES, and the one that does not is the one the guidance recommends. An `.msi` built from ONE NAMED PROGRAM takes `${mcpp.target_file:<name>}` and never looks at the tree. That is `docs/31`'s "name the input, do not harvest a directory", written after a bind path that resolved to nothing produced a valid, empty, 52 KB installer with no diagnostic. So a member following that guidance failed the check, and `mcpp pack --format msi` reported no action claimed --format 'msi' after a `wix build` that had succeeded. The workaround available to the member was to declare the placeholder as an extra, otherwise-unused input purely to satisfy the engine -- which also gave it a dependency on a tree its one `File` row never reads. The property actually wanted is presence in the dispatch pass. An artifact action present whether or not a format was asked for -- a codesign stamp, a size budget -- existed before anyone asked, and reporting one as the package would be a wrong answer that looks like a right one. So the first pass's artifact actions are collected, and the dispatch reports the difference. Identity is (package, id): an id is unique within the package that declared it and nothing more. `consumesStageDir` keeps its one real job -- the implicit dependency on the staged tree's manifest, so an action that reads the tree is dirty when the staged SET changes and not only when a link output does. Its comment now says what it is not. `638_pack_format_dispatch.sh` gains the case that pins this from both sides: a fixture submitting an UNGATED artifact stamp and a GATED action that names no staged tree at all. The gated one must be reported and the stamp must not, and the stamp must still have been built -- it was simply not the answer. Verified load-bearing by removing the guard, which reports `size.stamp` as the package and fails the test. Found while porting `mcpp.dist.wix`, which is to say by writing the second member. The first one consumed the staged tree and agreed with the check by accident.
1 parent efba585 commit 6163f86

5 files changed

Lines changed: 114 additions & 14 deletions

File tree

docs/30-build-mcpp.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,14 @@ is that format's knowledge and not the engine's.
625625
`mcpp pack --format dir` writes the same tree to a path and stops, which is how
626626
a person inspects what a member will be handed.
627627

628+
**`mcpp pack` reports the artifact actions the request introduced.** An action
629+
present whether or not a format was asked for — a codesign stamp, a size budget
630+
— is not the distributable, and naming one would be a wrong answer that looks
631+
like a right one. Nothing about the criterion is a property of the member: a
632+
format that packages one named program and never reads the staged tree is
633+
recognised exactly as one that consumes the whole closure. A format nothing
634+
submitted for is refused by name.
635+
628636
**An action that names `${mcpp.stage_dir}` gains a dependency on the tree's
629637
manifest.** mcpp writes `<staged tree>.stage-manifest` — a sibling, never a
630638
member, so it does not travel inside anyone's installer — listing each staged

docs/zh/30-build-mcpp.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -534,6 +534,12 @@ pass 和那次暂存已经回答过的。
534534
`mcpp pack --format dir` 把同一棵树写到一个路径上就停下,这是人去查看一个成员将会拿到
535535
什么的方式。
536536

537+
**`mcpp pack` 报告的是这次请求**引入**的那些 artifact action。** 一条无论有没有人
538+
要格式都在场的 action —— 一个签名 stamp、一次 size budget —— 不是可分发物,点名它会
539+
是一个看起来像对的错答案。这个判据里没有任何一项是成员的性质:一个只打包一个具名程序、
540+
从不读暂存树的格式,与一个消费整个闭包的格式被同等识别。对一个谁都没为之提交的格式,
541+
会被点名拒绝。
542+
537543
**写了 `${mcpp.stage_dir}` 的 action 会自动获得一条对这棵树的 manifest 的依赖。**
538544
mcpp 会写出 `<暂存树>.stage-manifest` —— 一个兄弟文件,永不是成员,所以它不会跑进任何
539545
人的安装包里 —— 逐条列出每个已暂存文件的大小与相对路径。这条依赖由引擎添加,因为「用

modules/manifest/src/types.cppm

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -388,12 +388,19 @@ struct BuildAction {
388388
// Set by the engine, never by the build program: this action's command or
389389
// inputs named `${mcpp.stage_dir}`.
390390
//
391-
// It is what makes the distributable ATTRIBUTABLE. `mcpp pack --format
392-
// <name>` reports the file the pass produced, and the alternative -- taking
393-
// every artifact action's output -- would name a codesign stamp or a size
394-
// budget alongside it. It is also the flag the dispatch checks to refuse a
395-
// member that declared a format and then submitted nothing for it, which
396-
// would otherwise be a pack that succeeds and produces no package.
391+
// ITS ONE JOB IS THE IMPLICIT DEPENDENCY. An action that names the staged
392+
// tree gains an edge to that tree's manifest, so it is dirty when the
393+
// staged SET changes and not only when a link output does. The dependency
394+
// is implied by the use, so a member author cannot forget it.
395+
//
396+
// IT IS NOT HOW `mcpp pack --format <name>` DECIDES WHICH ACTION IS THE
397+
// DISTRIBUTABLE, and briefly was. Not every format consumes the closure: an
398+
// `.msi` built from ONE NAMED PROGRAM takes `${mcpp.target_file:<name>}`
399+
// and never looks at the tree -- which is the shape the guidance
400+
// recommends, after a bind path that resolved to nothing produced a valid,
401+
// empty, 52 KB installer. So the member that followed the guidance was the
402+
// member that check refused. The dispatch asks instead which artifact
403+
// actions the REQUEST INTRODUCED; see mcpp.pack.pipeline.
397404
bool consumesStageDir = false;
398405
std::vector<std::string> inputs; // absolute or package-relative
399406
std::vector<std::string> outputs; // ditto; declared, see INV-D

src/pack/pipeline.cppm

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import mcpp.build.ninja;
1717
import mcpp.build.plan;
1818
import mcpp.config;
1919
import mcpp.fetcher.progress;
20+
import mcpp.manifest;
2021
import mcpp.pack;
2122
import mcpp.pack.stage_tree;
2223
import mcpp.pack.strip;
@@ -318,18 +319,44 @@ export int build_and_pack(Options opts, bool modeFromUser,
318319
// host triple, say -- is the shape where two derivations of one value agree
319320
// on every machine the author has and disagree on one they do not.
320321
if (opts.format == mcpp::pack::Format::Dispatched) {
322+
// WHICH ARTIFACT ACTIONS THIS BUILD ALREADY HAD, before a format was
323+
// requested. The dispatch below reports what the REQUEST introduced,
324+
// and this is the other half of that subtraction.
325+
std::set<std::pair<std::string, std::string>> preexistingArtifacts;
326+
for (auto const& a : ctx->plan.actions)
327+
if (a.role == mcpp::manifest::BuildAction::Role::Artifact)
328+
preexistingArtifacts.emplace(a.packageName, a.id);
329+
321330
ov.pack_format = opts.formatName;
322331
ov.pack_stage_dir = plan->stagingRoot;
323332
auto distCtx = mcpp::build::prepare_build(false, false, {}, ov);
324333
if (!distCtx) { mcpp::ui::error(distCtx.error()); return 2; }
325334

326-
// WHICH ACTIONS ARE THE DISTRIBUTABLE. Only those that named
327-
// `${mcpp.stage_dir}`: a codesign stamp or a size budget is also an
328-
// artifact action, and reporting one as the package would be a wrong
329-
// answer that looks like a right one.
335+
// WHICH ACTIONS ARE THE DISTRIBUTABLE: the artifact actions the REQUEST
336+
// INTRODUCED. An action present in both passes existed before anyone
337+
// asked for a format -- a codesign stamp, a size budget -- and
338+
// reporting one as the package would be a wrong answer that looks like
339+
// a right one.
340+
//
341+
// THE FIRST VERSION ASKED A NARROWER QUESTION AND GOT IT WRONG. It
342+
// collected only actions naming `${mcpp.stage_dir}`, on the assumption
343+
// that a distributable consumes the staged closure. Not every format
344+
// does: an `.msi` built from ONE named program takes
345+
// `${mcpp.target_file:<name>}` and never looks at the tree, which is
346+
// the shape section 6 of the design record recommends -- "name the
347+
// input, do not harvest a directory", after a bind path that resolved
348+
// to nothing produced a valid, empty, 52 KB installer. So the member
349+
// that followed the guidance was the member the check refused, and the
350+
// workaround was to name the placeholder as an unused input purely to
351+
// satisfy it. Presence-in-this-pass is the property actually wanted,
352+
// and it needs nothing of the member.
353+
//
354+
// Identity is (package, id): an id is unique within the package that
355+
// declared it and nothing more.
330356
std::vector<std::string> distOutputs;
331357
for (auto const& a : distCtx->plan.actions) {
332-
if (!a.consumesStageDir) continue;
358+
if (a.role != mcpp::manifest::BuildAction::Role::Artifact) continue;
359+
if (preexistingArtifacts.contains({a.packageName, a.id})) continue;
333360
for (auto const& o : a.outputs) distOutputs.push_back(o);
334361
}
335362
// DECLARED AND THEN SUBMITTED NOTHING. The half of the contract a
@@ -340,9 +367,8 @@ export int build_and_pack(Options opts, bool modeFromUser,
340367
mcpp::ui::error(std::format(
341368
"no action claimed --format '{}'.\n"
342369
" A package declared it provides this format, and no build "
343-
"program submitted an\n"
344-
" artifact action referencing ${{mcpp.stage_dir}} when it was "
345-
"asked for.\n"
370+
"program submitted a new\n"
371+
" `role = \"artifact\"` action when it was asked for.\n"
346372
" The provider must gate on the request and not on anything "
347373
"else:\n"
348374
" mcpp::provides_pack_format(\"{}\"); "

tests/e2e/638_pack_format_dispatch.sh

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,4 +256,57 @@ set -e
256256
grep -q "no action claimed --format 'zap'" b7.log \
257257
|| { cat b7.log; echo "FAIL: the refusal does not name the unclaimed format"; exit 1; }
258258

259+
# ── 8. an artifact action that predates the request is not the package ─────
260+
# THE CRITERION IS "WHAT THE REQUEST INTRODUCED", and this is what distinguishes
261+
# it from "any artifact action". A codesign stamp or a size budget is also an
262+
# artifact action and is present whether or not a format was asked for; naming
263+
# one as the distributable would be a wrong answer that looks like a right one.
264+
#
265+
# It is also what an earlier revision got wrong from the other side: the check
266+
# collected only actions naming ${mcpp.stage_dir}, which refused a member that
267+
# packages ONE NAMED PROGRAM and never reads the tree -- the shape the design
268+
# record recommends, after a bind path that resolved to nothing produced a
269+
# valid, empty, 52 KB installer. So this fixture submits both shapes: an
270+
# ungated stamp that must be ignored, and a gated action that names no staged
271+
# tree at all and must still be reported.
272+
cd "$TMP"
273+
cp -r app twoshapes
274+
cd twoshapes
275+
cat > build.mcpp <<'EOF'
276+
import mcpp;
277+
#include <string>
278+
#include <string_view>
279+
int main() {
280+
mcpp::provides_pack_format("zap");
281+
const std::string root = mcpp::manifest_dir();
282+
283+
// Ungated: present in both passes, so it is not the distributable.
284+
const std::string stamp = std::string(mcpp::out_dir()) + "/size.stamp";
285+
mcpp::action s;
286+
s.id = "size-budget"; s.role = "artifact";
287+
s.arg((root + "/dist.sh").c_str()).arg("stamp").arg(root.c_str()).arg(stamp.c_str())
288+
.input("${mcpp.target_file:app}").output(stamp.c_str());
289+
s.submit();
290+
291+
if (std::string_view(mcpp::pack_format()) != "zap") return 0;
292+
// Gated, and it names NO staged tree: the program arrives through
293+
// ${mcpp.target_file:...} exactly as an MSI's one File row does.
294+
const std::string out = std::string(mcpp::out_dir()) + "/app.zap";
295+
mcpp::action a;
296+
a.id = "zap"; a.role = "artifact";
297+
a.arg((root + "/dist.sh").c_str()).arg("named").arg(root.c_str()).arg(out.c_str())
298+
.input("${mcpp.target_file:app}").output(out.c_str());
299+
a.submit();
300+
return 0;
301+
}
302+
EOF
303+
"$MCPP" pack --format zap > b8.log 2>&1 || { cat b8.log; echo "FAIL: a member that reads no staged tree was refused"; exit 1; }
304+
grep -q "app.zap" b8.log \
305+
|| { cat b8.log; echo "FAIL: the gated action was not reported as the package"; exit 1; }
306+
grep -q "size.stamp" b8.log \
307+
&& { cat b8.log; echo "FAIL: an action predating the request was reported as the package"; exit 1; }
308+
# Both files exist -- the stamp was built, it was simply not the answer.
309+
[ -n "$(find target -name 'size.stamp' 2>/dev/null)" ] \
310+
|| { echo "FAIL: the ungated artifact action did not run at all"; exit 1; }
311+
259312
echo "PASS: 638_pack_format_dispatch"

0 commit comments

Comments
 (0)