Skip to content

Commit 78e772c

Browse files
committed
DLL placement: the prepare stamps are inputs, and an earlier placement's copies resolve again from their directories
A prepare action rewrites files ninja does not know as outputs, so the DLL it replaced was placed one build late; the placement edge now takes the graph's prepare stamps as implicit inputs. A copy placed beside the program was then resolved in the program's directory before the runtime search directories and shadowed the newer version; the stamp records the names placed, and read_closure resolves those names outside the program's directory (notInFirstDir).
1 parent d8fb8e9 commit 78e772c

3 files changed

Lines changed: 53 additions & 9 deletions

File tree

‎src/build/ninja_backend.cppm‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2939,10 +2939,25 @@ std::string emit_ninja_string(const BuildPlan& plan) {
29392939
}
29402940

29412941
// The placement edge (see `place_dlls` above), after the link it reads.
2942+
//
2943+
// The stamps of the graph's `prepare` actions are implicit inputs.
2944+
// A `prepare` command rewrites files ninja does not know as outputs,
2945+
// so a DLL it replaces in a runtime search directory is newer than
2946+
// the depfile's record only after ninja has already decided what is
2947+
// dirty; without the stamps, the new DLL would be placed one build
2948+
// late. With them, the edge runs again in the build that ran the
2949+
// action, after it.
29422950
if (placeRuntimeDlls
29432951
&& (lu.kind == LinkUnit::Binary || lu.kind == LinkUnit::TestBinary)) {
29442952
const auto exe = escape_ninja_path(lu.output);
2945-
append("build " + exe + ".dlls: place_dlls " + exe + "\n");
2953+
std::string prepareStamps;
2954+
for (auto const& a : plan.actions)
2955+
if (a.role == mcpp::manifest::BuildAction::Role::Prepare)
2956+
for (auto const& o : a.outputs)
2957+
prepareStamps += " " + escape_ninja_path(o);
2958+
append("build " + exe + ".dlls: place_dlls " + exe
2959+
+ (prepareStamps.empty() ? std::string{} : " |" + prepareStamps)
2960+
+ "\n");
29462961
append("default " + exe + ".dlls\n\n");
29472962
}
29482963

‎src/cli/cmd_publish.cppm‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ export int cmd_publish(const mcpplibs::cmdline::ParsedArgs& parsed) {
4444
//
4545
// The depfile names every DLL placed, so ninja runs the edge again when one of
4646
// them changes in its directory; the stamp is the edge's only declared output,
47-
// because the DLL names are not known when the graph is written.
47+
// because the DLL names are not known when the graph is written, and it holds
48+
// the names placed, one per line, for the next run.
4849
export int cmd_place_dlls(const mcpplibs::cmdline::ParsedArgs& parsed) {
4950
const std::filesystem::path stamp{parsed.option_or_empty("output").value()};
5051
const std::filesystem::path depfile{parsed.option_or_empty("depfile").value()};
@@ -57,7 +58,16 @@ export int cmd_place_dlls(const mcpplibs::cmdline::ParsedArgs& parsed) {
5758
for (std::size_t i = 1; i < parsed.positional_count(); ++i)
5859
dirs.emplace_back(parsed.positional(i));
5960

60-
auto placed = mcpp::pack::place_runtime_dlls(program, dirs);
61+
// What the previous run placed, recorded in the stamp itself: those copies
62+
// are this edge's, not the program's, and resolve again from their
63+
// directories (see `place_runtime_dlls`).
64+
std::vector<std::string> placedBefore;
65+
{
66+
std::ifstream prev(stamp);
67+
for (std::string line; std::getline(prev, line);)
68+
if (!line.empty()) placedBefore.push_back(line);
69+
}
70+
auto placed = mcpp::pack::place_runtime_dlls(program, dirs, placedBefore);
6171
if (!placed) {
6272
std::println(stderr, "error: {}", placed.error().message);
6373
return 1;
@@ -89,6 +99,7 @@ export int cmd_place_dlls(const mcpplibs::cmdline::ParsedArgs& parsed) {
8999
}
90100
}
91101
std::ofstream st(stamp, std::ios::trunc);
102+
for (auto const& n : placed->names) st << n << '\n';
92103
if (!st) {
93104
std::println(stderr, "error: cannot write '{}'", stamp.string());
94105
return 1;

‎src/pack/pack.cppm‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,11 @@ struct ClosureReadInput {
333333
std::vector<std::filesystem::path> platformDirs; // Android
334334
std::vector<std::string> forceBundle; // Pe: overrides the system list
335335
std::string arch; // MachO: the slice of a fat file
336+
// Pe: names never resolved in `searchDirs.front()`. `place_runtime_dlls`
337+
// passes the copies an earlier placement put beside the program, so that
338+
// each resolves again where it came from instead of shadowing a newer
339+
// version there. `mcpp pack` leaves it empty.
340+
std::vector<std::string> notInFirstDir;
336341
};
337342

338343
// A name that resolved to a file, and where that file is staged: `dest` is
@@ -377,15 +382,21 @@ ClosureRead read_closure(const ClosureReadInput& in);
377382
// program's directory is published beside the program through the staging
378383
// primitive, which writes only when the bytes differ. `sources` lists what was
379384
// resolved, for the edge's depfile, so that a DLL replaced in its directory is
380-
// placed again on the next build; `notes` names each DLL that more than one
381-
// directory offers, with the one the search order chose.
385+
// placed again on the next build; `names` lists the DLLs placed, which the
386+
// caller records and passes back as `placedBefore` on the next run, so that a
387+
// copy this function put beside the program is resolved again from its
388+
// directory rather than taken for a file of the program's own; `notes` names
389+
// each DLL that more than one directory offers, with the one the search order
390+
// chose.
382391
struct RuntimeDllPlacement {
383392
std::vector<std::filesystem::path> sources;
393+
std::vector<std::string> names;
384394
std::vector<std::string> notes;
385395
};
386396
std::expected<RuntimeDllPlacement, Error>
387397
place_runtime_dlls(const std::filesystem::path& program,
388-
const std::vector<std::filesystem::path>& searchDirs);
398+
const std::vector<std::filesystem::path>& searchDirs,
399+
const std::vector<std::string>& placedBefore = {});
389400

390401
// Build a Plan from already-resolved inputs. Caller is expected to have
391402
// already run `mcpp build` (or equivalent) and pass the resulting
@@ -1351,7 +1362,8 @@ make_tarball(const std::filesystem::path& stagingRoot,
13511362

13521363
std::expected<RuntimeDllPlacement, Error>
13531364
place_runtime_dlls(const std::filesystem::path& program,
1354-
const std::vector<std::filesystem::path>& searchDirs)
1365+
const std::vector<std::filesystem::path>& searchDirs,
1366+
const std::vector<std::string>& placedBefore)
13551367
{
13561368
const auto programDir = program.parent_path();
13571369
auto same_dir = [](const std::filesystem::path& a, const std::filesystem::path& b) {
@@ -1366,6 +1378,7 @@ place_runtime_dlls(const std::filesystem::path& program,
13661378
in.searchDirs.push_back(programDir.empty() ? std::filesystem::path(".") : programDir);
13671379
for (auto const& d : searchDirs)
13681380
if (!same_dir(d, in.searchDirs.front())) in.searchDirs.push_back(d);
1381+
in.notInFirstDir = placedBefore;
13691382
const auto read = read_closure(in);
13701383

13711384
// The program itself is the one object the caller chose, so a program that
@@ -1385,6 +1398,7 @@ place_runtime_dlls(const std::filesystem::path& program,
13851398
"cannot place '{}' beside '{}': {}", m.source.string(),
13861399
program.filename().string(), staged.error().message)});
13871400
out.sources.push_back(m.source);
1401+
out.names.push_back(m.name);
13881402
// Search order decides between two directories that offer one name,
13891403
// and the decision is stated, because the other copy may be the one
13901404
// the author meant.
@@ -1467,8 +1481,12 @@ ClosureRead read_closure(const ClosureReadInput& in)
14671481
continue;
14681482
}
14691483
std::optional<std::filesystem::path> hit;
1470-
for (auto const& dir : in.searchDirs)
1471-
if (is_file(dir / name)) { hit = dir / name; break; }
1484+
const bool skipFirst = std::ranges::any_of(in.notInFirstDir,
1485+
[&](const std::string& n) { return lower(n) == lower(name); });
1486+
for (std::size_t d = 0; d < in.searchDirs.size(); ++d) {
1487+
if (d == 0 && skipFirst) continue;
1488+
if (is_file(in.searchDirs[d] / name)) { hit = in.searchDirs[d] / name; break; }
1489+
}
14721490
if (hit) {
14731491
out.members.push_back({name, *hit, std::filesystem::path(name)});
14741492
// Transitive: a staged library brings its own needs, and a

0 commit comments

Comments
 (0)