Skip to content

Commit d44d61c

Browse files
committed
The staged cache is placed by its own ninja pass (ninja 1.12.1 use-after-free, ninja-build/ninja#2662); pack reads runtime files against the product directory; a program shipped through artifacts is linked with its package's intermediates and placed in each member's product directory; a file never takes a product directory's path
1 parent dab99e2 commit d44d61c

4 files changed

Lines changed: 103 additions & 8 deletions

File tree

‎src/build/ninja_backend.cppm‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1141,6 +1141,11 @@ std::string filter_ninja_output(std::string_view output,
11411141
return filtered;
11421142
}
11431143

1144+
// The aggregate target for everything staged out of the global cache. Named
1145+
// with a leading underscore so it cannot collide with a module or target name
1146+
// (both of which are identifiers or paths).
1147+
constexpr std::string_view kStagedCacheGoal = "_mcpp_staged_cache";
1148+
11441149
std::string emit_ninja_string(const BuildPlan& plan) {
11451150
return emit_ninja_string(plan, nullptr);
11461151
}
@@ -2205,7 +2210,7 @@ std::string emit_ninja_string(const BuildPlan& plan, std::string* placements) {
22052210
// Aggregate target for everything staged out of the global cache. Named
22062211
// with a leading underscore so it cannot collide with a module or target
22072212
// name (both of which are identifiers or paths).
2208-
constexpr std::string_view kStagedCachePhony = "_mcpp_staged_cache";
2213+
constexpr std::string_view kStagedCachePhony = kStagedCacheGoal;
22092214

22102215
auto bmi_path = [&traits](std::string_view name) {
22112216
std::string s(traits.bmiDir);
@@ -4051,6 +4056,26 @@ std::expected<BuildResult, BuildError> NinjaBackend::build(const BuildPlan& plan
40514056
for (auto& ev : plan.toolchain.envOverrides)
40524057
nenv.emplace_back(ev.key, ev.value);
40534058

4059+
// THE STAGED CACHE IS PLACED BY A PASS OF ITS OWN, before the graph that
4060+
// reads it. ninja 1.12.1 loads a dyndep file whose producer is up to date
4061+
// as soon as that producer's order-only inputs are ready, from inside the
4062+
// completion of the node that readied them; when the dyndep file names that
4063+
// same node as an input, loading it grows the list the completion is
4064+
// iterating, a use-after-free (ninja-build/ninja#2662, unfixed upstream).
4065+
// The scans wait on `_mcpp_staged_cache` and the importers' dyndep files
4066+
// name the staged BMIs, so re-staging a dependency in a directory whose
4067+
// scans are current -- a dependency upgraded with its build served from the
4068+
// cache -- crashed ninja (e2e 196). After this pass the staged nodes are
4069+
// current, and the second pass loads those dyndep files when it starts.
4070+
if (manifest.find("\nbuild " + std::string(kStagedCacheGoal) + " : phony") != std::string::npos) {
4071+
std::vector<std::string> pre{ninjaProgram, "--quiet", "-C", plan.outputDir.string(),
4072+
std::string(kStagedCacheGoal)};
4073+
(void)mcpp::platform::process::capture_exec_deadline(pre, nenv,
4074+
std::chrono::milliseconds(static_cast<long long>(opts.buildTimeoutSecs) * 1000),
4075+
nullptr);
4076+
stage("ninja-staged-cache");
4077+
}
4078+
40544079
bool buildTimedOut = false;
40554080
auto cap = mcpp::platform::process::capture_exec_deadline(
40564081
nargv, nenv,

‎src/build/plan.cppm‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2701,6 +2701,12 @@ make_plan(const mcpp::manifest::Manifest& manifest,
27012701
lu.dependencyOwned = true;
27022702
lu.artifactOf = owner;
27032703
lu.output = target_output(r.target, naming);
2704+
// In a workspace plan `bin/` holds products only: the workspace's own
2705+
// package's, and each member's directory. The program is linked with
2706+
// its package's intermediate files and placed beside each member
2707+
// program that ships it (step 6, §15 of the 2026-09-29 design).
2708+
if (manifest.package.virtualRoot)
2709+
lu.output = std::filesystem::path("obj") / sanitize(owner) / lu.output.filename();
27042710
lu.windowsSubsystem = r.target.windowsSubsystem;
27052711
lu.windowsEntry = r.target.windowsEntry;
27062712
lu.loaderTagFlag = loader_tag_flag(lu.kind);
@@ -3005,10 +3011,46 @@ make_plan(const mcpp::manifest::Manifest& manifest,
30053011
}
30063012
units.push_back(std::move(lu));
30073013
}
3014+
// A program the closure ships through `artifacts` (mcpp#711) is linked
3015+
// once, at `bin/`, and placed beside the member's programs, where a
3016+
// program that launches it looks for it.
3017+
for (auto const& r : artifactRequests) {
3018+
const bool requested = std::ranges::any_of(artifactEdges, [&](auto const& e) {
3019+
return closureIdx.contains(e.first) && e.second == r.packageIndex;
3020+
});
3021+
if (!requested) continue;
3022+
std::filesystem::path out;
3023+
for (auto const& u : plan.linkUnits)
3024+
if (u.artifactOf == qualified_package_name(packages[r.packageIndex].manifest)
3025+
&& u.targetName == r.target.name) { out = u.output; break; }
3026+
if (out.empty()) continue;
3027+
const bool seen = std::ranges::any_of(group.placements,
3028+
[&](auto const& pl) { return pl.source == out; });
3029+
if (!seen && out.parent_path() != productDir)
3030+
group.placements.push_back({out, place(out)});
3031+
}
30083032
plan.linkGroups.push_back(std::move(group));
30093033
for (auto& u : units) plan.linkUnits.push_back(std::move(u));
30103034
}
30113035

3036+
// A product directory is a directory: nothing this plan writes may take
3037+
// its path as a file (a program of the workspace's own package named as a
3038+
// member, say).
3039+
for (auto const& g : plan.linkGroups) {
3040+
if (g.productDir == std::filesystem::path("bin")) continue;
3041+
for (auto const& u : plan.linkUnits) {
3042+
bool clash = u.output == g.productDir;
3043+
for (auto const& a : u.runtimeAliases) clash = clash || a == g.productDir;
3044+
if (clash)
3045+
return std::unexpected(std::format(
3046+
"target '{}' would be written to '{}', which is the product "
3047+
"directory of workspace member '{}'.\n"
3048+
" Rename the target, or give the member a namespace so "
3049+
"that its directory is qualified.",
3050+
u.targetName, u.output.generic_string(), g.member));
3051+
}
3052+
}
3053+
30123054
// The single derivation. Deliberately at the END of make_plan, after every
30133055
// producer of a link unit has run: a dependency resolved to the shared
30143056
// form arrives as an ordinary SharedLibrary unit, so this one predicate

‎src/pack/pipeline.cppm‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -500,20 +500,26 @@ export PackOutcome build_and_pack(Options opts, bool modeFromUser,
500500
// package that the contract says the host serves.
501501
namespace rp = mcpp::build::runtime_placement;
502502
const bool msvcAbi = mcpp::toolchain::is_msvc_target(ctx->plan.toolchain);
503+
// Relative to the program's directory: `bin`, or a workspace member's
504+
// product directory (§15 of the 2026-09-29 workspace design).
505+
const auto& productDir = ctx->plan.productDir;
503506
for (auto const& d : flags.runtimeDeploy) {
504-
if (msvcAbi && d.dest.parent_path() == "bin"
507+
if (msvcAbi && d.dest.parent_path() == productDir
505508
&& rp::is_msvc_crt_name(d.dest.filename().string()))
506509
continue;
507-
opts.runtimeFiles.push_back(d.dest.lexically_relative("bin"));
510+
opts.runtimeFiles.push_back(d.dest.lexically_relative(productDir));
508511
}
509512
if (msvcAbi && !opts.carryToolchainRuntime && flags.runtimeCrtPolicy != "static")
510513
for (auto n : rp::kMsvcCrtNames) opts.hostProvidedLibs.emplace_back(n);
511514
// A dependency's program the manifest ships with this one (mcpp#711,
512515
// `artifacts = [...]`) is linked into `bin/` beside the executable, so
513516
// it is staged the way a deployed file is.
517+
// In a workspace member's product directory it is the placed copy.
514518
for (auto const& u : ctx->plan.linkUnits)
515519
if (!u.artifactOf.empty())
516-
opts.runtimeFiles.push_back(u.output.lexically_relative("bin"));
520+
opts.runtimeFiles.push_back(productDir == "bin"
521+
? u.output.lexically_relative("bin")
522+
: std::filesystem::path(u.output.filename()));
517523
// #634 A3: the Android row reads its closure against the directories
518524
// its link declared -- a prebuilt library named through `[runtime]
519525
// link_library_dirs` is a file the link used and the device does not

‎tests/e2e/833_a_workspace_is_one_graph_per_configuration.sh‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
# G6 two members with one package name in two namespaces get qualified
1717
# product directories;
1818
# G7 no member directory receives a build directory, and `clean --stale`
19-
# removes the ones members held before, keeping `target/.build-mcpp/`.
19+
# removes the ones members held before, keeping `target/.build-mcpp/`;
20+
# G8 a program a member ships through `artifacts` is beside that member's
21+
# program, in its product directory.
2022
set -e
2123

2224
TMP=$(mktemp -d)
@@ -29,10 +31,10 @@ case "$(uname -s)" in MINGW*|MSYS*|CYGWIN*) EXE=".exe" ;; esac
2931

3032
cat > mcpp.toml <<'EOF'
3133
[workspace]
32-
members = ["core", "cli", "gui", "ns1/tool", "ns2/tool", "modern"]
34+
members = ["core", "cli", "gui", "ns1/tool", "ns2/tool", "modern", "helper"]
3335
EOF
3436

35-
mkdir -p core/src cli/src gui/src ns1/tool/src ns2/tool/src modern/src
37+
mkdir -p core/src cli/src gui/src ns1/tool/src ns2/tool/src modern/src helper/src
3638
cat > core/mcpp.toml <<'EOF'
3739
[package]
3840
name = "core"
@@ -59,6 +61,22 @@ EOF
5961
done
6062
printf '#include <cstdio>\nimport shared_core;\nint main() { std::printf("%%d\\n", core_v() + 1); return 0; }\n' > cli/src/main.cpp
6163
printf '#include <cstdio>\nimport shared_core;\nint main() { std::printf("%%d\\n", core_v() + 2); return 0; }\n' > gui/src/main.cpp
64+
cat >> gui/mcpp.toml <<'EOF'
65+
66+
[dependencies.helper]
67+
path = "../helper"
68+
artifacts = ["helper"]
69+
EOF
70+
cat > helper/mcpp.toml <<'EOF'
71+
[package]
72+
name = "helper"
73+
version = "0.1.0"
74+
75+
[targets.helper]
76+
kind = "bin"
77+
main = "src/main.cpp"
78+
EOF
79+
printf 'int main() { return 0; }\n' > helper/src/main.cpp
6280

6381
for ns in ns1 ns2; do
6482
cat > $ns/tool/mcpp.toml <<EOF
@@ -108,6 +126,10 @@ other=$(echo "$dirs" | grep -v "^$main_dir$")
108126
[ -f "$main_dir/bin/ns1.tool/tool$EXE" ] || fail "G6: bin/ns1.tool/tool missing" ws.log
109127
[ -f "$main_dir/bin/ns2.tool/tool$EXE" ] || fail "G6: bin/ns2.tool/tool missing" ws.log
110128

129+
# G8: the program gui ships is beside gui's program.
130+
[ -f "$main_dir/bin/gui/helper$EXE" ] || fail "G8: bin/gui/helper is missing" ws.log
131+
[ -f "$main_dir/bin/helper/helper$EXE" ] || fail "G8: bin/helper/helper is missing" ws.log
132+
111133
# G1: the core module is compiled once, in the one graph.
112134
log="$main_dir/.ninja_log"
113135
n=$(grep -c 'core\.m\.o' "$log" || true)
@@ -136,7 +158,7 @@ fi
136158
[ -d "$main_dir" ] || fail "G3: the flag edit moved the build directory"
137159

138160
# G7: no member directory holds a build directory; the old ones are stale.
139-
for m in core cli gui ns1/tool ns2/tool modern; do
161+
for m in core cli gui ns1/tool ns2/tool modern helper; do
140162
[ ! -d "$m/target/$(basename "$(dirname "$main_dir")")" ] \
141163
|| fail "G7: $m received a build directory"
142164
done

0 commit comments

Comments
 (0)