Skip to content

Commit 0b300fd

Browse files
committed
dist-apple: the iOS row takes the single-binary path without a staged tree, and resolves its icon directory
The floor check at the end of submit() unconditionally measured pack_stage_dir(), even when the layout step had already taken the single-binary (${mcpp.target_file:<target>}) path because pack_stage_dir() was empty -- the shape a host that cannot walk an iOS Mach-O's closure hands this member. Measuring an empty path produced 'the staged tree at holds only 0 bytes', a warning about a tree that was never asked for. The check now runs only when p.appdir is non-empty. options::icon named a project-relative path, but the actions that read it (ditto in the layout and icon steps) run with the build directory as their cwd, not the manifest directory -- so a relative icon directory or file validated at plan time and then failed inside the graph ('ditto: Cannot get the real path for source'). options::icon is now resolved against mcpp::manifest_dir() once, before validation, on both the iOS directory case and the macOS .icns case; a directory that still does not exist or holds no *.png is refused at plan time, naming options::icon and the resolved path. tests/ios-app-consumer/ios-icons/AppIcon60x60@2x.png is now a real 1x1 PNG (written with Python's zlib/struct) rather than placeholder text, so the icon path is exercised for real. check-ios-plan.sh gains three assertions: a non-empty fabricated stage names ${mcpp.stage_dir} with no staged-tree warning, an empty MCPP_PACK_STAGE_DIR takes the ${mcpp.target_file:...} path with no staged-tree warning, and a manifest directory with no ios-icons/ is refused at plan time naming options::icon and the resolved path.
1 parent e40ad09 commit 0b300fd

3 files changed

Lines changed: 117 additions & 28 deletions

File tree

dist/apple.cppm

Lines changed: 69 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -547,19 +547,47 @@ inline plan plan_for(options opt = {}) {
547547
const std::string executableName = stage.empty()
548548
? target : bundle_executable_name(launcher);
549549

550+
// `options::icon` IS RESOLVED AGAINST THE MANIFEST DIRECTORY HERE, ONCE,
551+
// BEFORE ANY VALIDATION OR ACTION ARGV USES IT -- ON BOTH ROWS.
552+
//
553+
// This program's own cwd is the manifest directory when mcpp runs it (the
554+
// usual case a project author sees, and why a bare relative path like
555+
// `"ios-icons"` validates below without complaint). But the ACTIONS this
556+
// member declares -- the `ditto` calls in the layout and icon steps -- are
557+
// graph edges ninja runs later, with the BUILD directory as their cwd, not
558+
// the manifest directory. A relative `options::icon` therefore reached
559+
// `ditto` as a path that does not exist from where `ditto` was standing:
560+
//
561+
// ditto ios-icons .../IosAppConsumer.app
562+
// ditto: Cannot get the real path for source 'ios-icons'
563+
//
564+
// failing inside the graph, on a host with no way to run this member
565+
// again to explain why. Resolving here, against `mcpp::manifest_dir()`,
566+
// makes every later use -- the validation immediately below and the
567+
// `icon.argv` this function builds further down -- see the same absolute
568+
// path regardless of which directory the thing reading it is standing in.
569+
// An already-absolute `options::icon` is left alone.
570+
if (!opt.icon.empty()) {
571+
std::filesystem::path iconPath(opt.icon);
572+
if (!iconPath.is_absolute())
573+
opt.icon = (std::filesystem::path(mcpp::manifest_dir()) / iconPath).string();
574+
}
575+
550576
// macOS: `options::icon` is a FILE. iOS: it is a DIRECTORY of flat PNGs
551577
// (see the `options::icon` comment and the header's icon paragraph) --
552578
// two different validations of the same field, because the two
553579
// platforms' icon conventions are not the same shape and this member
554-
// does not invent a third field to hold the distinction.
580+
// does not invent a third field to hold the distinction. Both refusals
581+
// name `options::icon` and the (now-resolved) path, so a project sees
582+
// exactly what this member read rather than a bare relative name it typed.
555583
std::vector<std::string> iosIconStems;
556584
if (!opt.icon.empty()) {
557585
if (isIos) {
558586
std::error_code ec;
559587
if (!std::filesystem::is_directory(opt.icon, ec)) {
560588
std::cerr << std::format(
561-
"mcpp.dist.apple: the iOS icon {} is not a directory. "
562-
"Set `options::icon` to a directory of flat PNGs "
589+
"mcpp.dist.apple: `options::icon` ({}) is not a "
590+
"directory. Set it to a directory of flat PNGs "
563591
"(one per size Apple's Home Screen and Settings need); "
564592
"this member lists their stems under `CFBundleIcons` "
565593
"and does not generate sizes itself.", opt.icon) << '\n';
@@ -574,13 +602,15 @@ inline plan plan_for(options opt = {}) {
574602
std::sort(iosIconStems.begin(), iosIconStems.end());
575603
if (iosIconStems.empty()) {
576604
std::cerr << std::format(
577-
"mcpp.dist.apple: the icon directory {} carries no "
605+
"mcpp.dist.apple: `options::icon` ({}) carries no "
578606
"*.png files.", opt.icon) << '\n';
579607
p.reason = "icon directory carries no PNGs";
580608
return p;
581609
}
582610
} else if (!is_file(opt.icon)) {
583-
std::cerr << std::format("mcpp.dist.apple: the icon {} was not found", opt.icon) << '\n';
611+
std::cerr << std::format(
612+
"mcpp.dist.apple: `options::icon` ({}) was not found",
613+
opt.icon) << '\n';
584614
p.reason = "icon not found";
585615
return p;
586616
}
@@ -763,7 +793,8 @@ inline bool submit(const plan& p) {
763793
a.submit();
764794
}
765795

766-
// A FLOOR ON THIS MEMBER'S OWN OUTPUT, ON THE SUCCESS PATH.
796+
// A FLOOR ON THIS MEMBER'S OWN OUTPUT, ON THE SUCCESS PATH -- AND ONLY
797+
// WHEN A STAGED TREE IS THE THING BEING MEASURED.
767798
//
768799
// The assembled bundle does not exist when this program runs -- ditto and
769800
// codesign have not been invoked yet, only declared -- so what this
@@ -774,24 +805,38 @@ inline bool submit(const plan& p) {
774805
// verify, or the bare-filename assumption `bundle_executable_name`
775806
// documents not holding for a non-default staging layout -- all of those
776807
// happen after this program has already exited.
777-
std::error_code ec;
778-
std::uintmax_t bytes = 0;
779-
for (auto const& e : std::filesystem::recursive_directory_iterator(p.appdir, ec)) {
780-
if (ec) break;
781-
if (e.is_regular_file(ec)) bytes += std::filesystem::file_size(e.path(), ec);
782-
}
783-
// Loose on purpose, matching `dist/appimage.cppm`'s own bound: this
784-
// exists to catch "nothing was staged", not to police a size budget.
785-
// Unlike that member, nothing is written INTO the staged tree here --
786-
// `Info.plist` and the icon live outside it until the layout and install
787-
// steps run -- so even a low bound is already suspicious.
788-
if (bytes < 4u * 1024u) {
789-
static char msg[512];
790-
std::snprintf(msg, sizeof msg,
791-
"mcpp.dist.apple: the staged tree at %s holds only %llu bytes, "
792-
"which is not a program; the .app will not launch anything",
793-
p.appdir.c_str(), static_cast<unsigned long long>(bytes));
794-
mcpp::warning(msg);
808+
//
809+
// `p.appdir` IS `pack_stage_dir()`, WHICH THE HEADER COMMENT ALREADY
810+
// DOCUMENTS AS OPTIONAL. When it is empty -- the common iOS case, since
811+
// the host running mcpp cannot always execute an iOS Mach-O to walk its
812+
// closure -- there is no tree to measure at all: the layout step above
813+
// already took the single-binary path (`${mcpp.target_file:<target>}`),
814+
// and `recursive_directory_iterator` on an empty path opens nothing,
815+
// leaving `bytes` at zero. Running the check anyway turned that "no tree
816+
// was ever asked for" into "the staged tree at holds only 0 bytes",
817+
// naming a path that is blank because none exists -- a warning about a
818+
// defect that was never present. So this floor applies only when a
819+
// staged tree exists to be measured.
820+
if (!p.appdir.empty()) {
821+
std::error_code ec;
822+
std::uintmax_t bytes = 0;
823+
for (auto const& e : std::filesystem::recursive_directory_iterator(p.appdir, ec)) {
824+
if (ec) break;
825+
if (e.is_regular_file(ec)) bytes += std::filesystem::file_size(e.path(), ec);
826+
}
827+
// Loose on purpose, matching `dist/appimage.cppm`'s own bound: this
828+
// exists to catch "nothing was staged", not to police a size budget.
829+
// Unlike that member, nothing is written INTO the staged tree here --
830+
// `Info.plist` and the icon live outside it until the layout and
831+
// install steps run -- so even a low bound is already suspicious.
832+
if (bytes < 4u * 1024u) {
833+
static char msg[512];
834+
std::snprintf(msg, sizeof msg,
835+
"mcpp.dist.apple: the staged tree at %s holds only %llu bytes, "
836+
"which is not a program; the .app will not launch anything",
837+
p.appdir.c_str(), static_cast<unsigned long long>(bytes));
838+
mcpp::warning(msg);
839+
}
795840
}
796841
return true;
797842
}

tests/ios-app-consumer/check-ios-plan.sh

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,29 @@ echo "ok: build.mcpp compiled to $BIN"
2626

2727
run_row() {
2828
# $1=label $2=MCPP_TARGET_OS $3=MCPP_TARGET_ENV(or empty) $4=out log path
29-
local label="$1" os="$2" envv="$3" log="$4"
29+
# $5=stage mode: "" (default) fabricates a non-empty staged tree, exactly
30+
# as every row did before this parameter existed; "EMPTY" instead sends
31+
# MCPP_PACK_STAGE_DIR="" -- the shape a host that cannot walk an iOS
32+
# Mach-O's closure actually hands this member.
33+
# $6=MCPP_MANIFEST_DIR override, empty by default (matches every row's
34+
# behaviour before this parameter existed: the fixture's own directory,
35+
# read through this script's cwd rather than through the variable).
36+
local label="$1" os="$2" envv="$3" log="$4" stage_mode="${5:-}" manifest="${6:-}"
3037
local stage out
31-
stage=$(mktemp -d) out=$(mktemp -d)
32-
mkdir -p "$stage/bin"
33-
head -c 5000 /dev/urandom > "$stage/bin/ios-app-consumer"
38+
if [ "$stage_mode" = "EMPTY" ]; then
39+
stage=""
40+
else
41+
stage=$(mktemp -d)
42+
mkdir -p "$stage/bin"
43+
head -c 5000 /dev/urandom > "$stage/bin/ios-app-consumer"
44+
fi
45+
out=$(mktemp -d)
3446
env -i PATH="$PATH" \
3547
MCPP_PACK_FORMAT=app \
3648
MCPP_TARGET_OS="$os" \
3749
MCPP_TARGET_ENV="$envv" \
3850
MCPP_PACK_STAGE_DIR="$stage" \
51+
MCPP_MANIFEST_DIR="$manifest" \
3952
MCPP_TARGET_MIN_PLATFORM_VERSION="$([ "$os" = ios ] && echo 18.0)" \
4053
MCPP_PKG_NAME=ios-app-consumer \
4154
MCPP_PKG_VERSION=0.3.0 \
@@ -67,8 +80,39 @@ grep -q 'LSMinimumSystemVersion' "$plist" && fail "the iOS plist carries the mac
6780
grep -q '<key>CFBundleIcons</key>' "$plist" || fail "no CFBundleIcons key for the directory icon" "$plist"
6881
grep -q 'AppIcon60x60@2x</string>' "$plist" || fail "the icon list is missing a stem" "$plist"
6982
grep -q 'AppIcon76x76@2x~ipad</string>' "$plist" || fail "the icon list is missing a stem" "$plist"
83+
# THE STAGED-TREE SIDE OF THE mcpp.stage_dir / target_file DECISION: this row
84+
# fabricates a non-empty MCPP_PACK_STAGE_DIR (see run_row), so the layout step
85+
# must name the tree, and the floor check in `submit()` must find enough
86+
# bytes in it to stay quiet.
87+
grep 'mcpp.dist.apple.layout' "$log" | grep -q '\${mcpp\.stage_dir}' \
88+
|| fail "the layout step did not name \${mcpp.stage_dir} for a non-empty staged tree" "$log"
89+
grep -q 'holds only' "$log" \
90+
&& fail "a non-empty staged tree still produced the 0-byte staged-tree warning" "$log"
7091
echo "ok: flat layout, no codesign, a named warning, and every iOS-only plist key"
7192

93+
echo "== iOS Simulator row, an empty pack_stage_dir (#622 B2 defect 1) =="
94+
run_row simnostage ios sim /tmp/ios-plan-sim-nostage.log EMPTY > /dev/null
95+
log=/tmp/ios-plan-sim-nostage.log
96+
grep -q 'mcpp.dist.apple.layout' "$log" || fail "no layout step planned with an empty stage dir" "$log"
97+
grep 'mcpp.dist.apple.layout' "$log" | grep -q '\${mcpp\.target_file:ios-app-consumer}' \
98+
|| fail "the layout step did not name \${mcpp.target_file:...} with an empty stage dir" "$log"
99+
grep 'mcpp.dist.apple.layout' "$log" | grep -q '\${mcpp\.stage_dir}' \
100+
&& fail "the layout step named \${mcpp.stage_dir} even though pack_stage_dir() was empty" "$log"
101+
grep -q 'holds only' "$log" \
102+
&& fail "an empty pack_stage_dir produced the misleading 0-byte staged-tree warning" "$log"
103+
echo "ok: an empty pack_stage_dir takes the single-binary path, named through \${mcpp.target_file:...}, with no staged-tree warning"
104+
105+
echo "== iOS Simulator row, a manifest directory with no ios-icons/ (#622 B2 defect 2) =="
106+
badmanifest=$(mktemp -d)
107+
run_row simbadicon ios sim /tmp/ios-plan-sim-badicon.log "" "$badmanifest" > /dev/null
108+
log=/tmp/ios-plan-sim-badicon.log
109+
grep -q 'options::icon' "$log" || fail "the missing-icon refusal did not name options::icon" "$log"
110+
grep -qF "$badmanifest/ios-icons" "$log" \
111+
|| fail "the missing-icon refusal did not resolve options::icon against MCPP_MANIFEST_DIR" "$log"
112+
grep -q 'mcpp.dist.apple.layout' "$log" \
113+
&& fail "a layout step was planned even though the icon directory was refused" "$log"
114+
echo "ok: a relative options::icon resolves against MCPP_MANIFEST_DIR, and a missing directory is refused at plan time, naming options::icon and the path"
115+
72116
echo "== iOS device row =="
73117
outdir=$(run_row device ios "" /tmp/ios-plan-device.log)
74118
log=/tmp/ios-plan-device.log
59 Bytes
Loading

0 commit comments

Comments
 (0)