Skip to content

Commit d4bac4f

Browse files
committed
fix(build): name artifacts from the target, not from the build host
target_output() spelled the suffix and library affixes from mcpp::platform::{exe_suffix,lib_prefix,static_lib_ext,shared_lib_ext} — host constants selected by #if defined(_WIN32)/__APPLE__. On a host build the host and target answers coincide, which is why it survived; they diverge the moment host != target. The consequence was not cosmetic. Cross-compiling Linux -> PE, ninja was told to produce `bin/foo` while mingw's GCC driver writes `bin/foo.exe`, so the declared output never existed and ninja reran the link edge on every single build. Incremental builds were effectively off for PE targets — the path CI exercises daily. Naming now comes from ArtifactNaming, resolved once per plan from the target triple. It is an (os, env) function, not an os one: x86_64-windows-gnu -> libfoo.a (GNU/mingw) x86_64-windows-msvc -> foo.lib (MSVC) A single _WIN32 branch cannot express that, which is why building a static library with mingw ON a Windows host produced `foo.lib` — a GNU archive wearing an MSVC name. That is a behaviour change for that configuration, and it fixes a name that was already wrong. An empty triple means "build for this machine", and only there is the host answer correct, so it is threaded in as the fallback rather than read directly. Host builds are therefore bit-for-bit unchanged. shared_library_link_flags gets the same treatment: whether a consumer links a full path (PE), uses @loader_path (Mach-O) or $ORIGIN (ELF) is a property of what we build FOR. Keying it on the host pointed it the wrong way under cross builds. Also refuses SharedLibrary on non-ELF targets. Every shared-library e2e declares `# requires: elf` and run_all.sh grants that only on Linux, so those paths have never been verified on PE or Mach-O — mingw's ld tolerates linking a .dll directly, MSVC's link.exe cannot, and neither has an import library because mcpp does not model one. A clear refusal beats emitting an artifact nothing has ever checked. Verified: - e2e 183 red before, green after - 53 unit tests pass - 08_shared_library, 64_shared_soname_runtime_alias, 55/57_*_shared_artifact, 102_mingw_cross_wine all pass — the soname alias edge is the one the design doc flagged as historically fragile Refs .agents/docs/2026-08-03-b3-target-aware-artifact-naming.md
1 parent d508551 commit d4bac4f

1 file changed

Lines changed: 102 additions & 28 deletions

File tree

src/build/plan.cppm

Lines changed: 102 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import mcpp.toolchain.cppfly;
1313
import mcpp.toolchain.detect;
1414
import mcpp.toolchain.dialect;
1515
import mcpp.toolchain.fingerprint;
16+
import mcpp.toolchain.triple;
1617
import mcpp.platform;
1718

1819
export namespace mcpp::build {
@@ -197,29 +198,52 @@ std::vector<std::string> dependency_name_candidates(
197198
return out;
198199
}
199200

200-
std::filesystem::path target_output(const mcpp::manifest::Target& t) {
201+
// The naming this MACHINE would use for its own binaries. Correct only for a
202+
// host-target build; passed to artifact_naming() as the fallback for an empty
203+
// triple, and never consulted directly when a target triple is present.
204+
mcpp::toolchain::triple::ArtifactNaming host_artifact_naming() {
205+
return {
206+
.exeSuffix = mcpp::platform::exe_suffix,
207+
.libPrefix = mcpp::platform::lib_prefix,
208+
.staticLibExt = mcpp::platform::static_lib_ext,
209+
.sharedLibExt = mcpp::platform::shared_lib_ext,
210+
.sharedNeedsImportLib = mcpp::platform::is_windows,
211+
};
212+
}
213+
214+
mcpp::toolchain::triple::ArtifactNaming naming_for(const mcpp::toolchain::Toolchain& tc) {
215+
auto t = mcpp::toolchain::triple::parse(tc.targetTriple);
216+
return mcpp::toolchain::triple::artifact_naming(
217+
t ? *t : mcpp::toolchain::triple::Triple{}, host_artifact_naming());
218+
}
219+
220+
// What the artifact is CALLED — a property of the target, not of this machine.
221+
// Reading the host constants here made ninja declare an output the compiler
222+
// never writes (Linux -> PE: declared `bin/foo`, produced `bin/foo.exe`), so
223+
// the link edge could never be satisfied and reran on every build.
224+
std::filesystem::path target_output(const mcpp::manifest::Target& t,
225+
const mcpp::toolchain::triple::ArtifactNaming& n) {
201226
if (t.kind == mcpp::manifest::Target::Library) {
202227
return std::filesystem::path("bin") /
203-
std::format("{}{}{}", mcpp::platform::lib_prefix, t.name,
204-
mcpp::platform::static_lib_ext);
228+
std::format("{}{}{}", n.libPrefix, t.name, n.staticLibExt);
205229
}
206230
if (t.kind == mcpp::manifest::Target::SharedLibrary) {
207231
return std::filesystem::path("bin") /
208-
std::format("{}{}{}", mcpp::platform::lib_prefix, t.name,
209-
mcpp::platform::shared_lib_ext);
232+
std::format("{}{}{}", n.libPrefix, t.name, n.sharedLibExt);
210233
}
211234
return std::filesystem::path("bin") /
212-
std::format("{}{}", t.name, mcpp::platform::exe_suffix);
235+
std::format("{}{}", t.name, n.exeSuffix);
213236
}
214237

215238
std::vector<std::filesystem::path> runtime_aliases_for_target(
216-
const mcpp::manifest::Target& t) {
239+
const mcpp::manifest::Target& t,
240+
const mcpp::toolchain::triple::ArtifactNaming& n) {
217241
std::vector<std::filesystem::path> aliases;
218242
if (t.kind != mcpp::manifest::Target::SharedLibrary || t.soname.empty()) {
219243
return aliases;
220244
}
221245

222-
auto output = target_output(t);
246+
auto output = target_output(t, n);
223247
if (t.soname != output.filename().string()) {
224248
aliases.push_back(output.parent_path() / t.soname);
225249
}
@@ -232,19 +256,30 @@ bool is_implementation_source(const std::filesystem::path& source) {
232256
|| ext == ".S" || ext == ".s" || ext == ".asm";
233257
}
234258

235-
std::vector<std::string> shared_library_link_flags(const mcpp::manifest::Target& t) {
259+
// How a CONSUMER links against a shared library. Also a target property: PE has
260+
// no rpath and wants an import library, Mach-O uses @loader_path, ELF uses
261+
// $ORIGIN. Keying this on the host pointed it the wrong way under cross builds.
262+
//
263+
// NOTE: shared libraries have never been verified end to end on PE or Mach-O —
264+
// every shared-library e2e declares `# requires: elf`, and that capability is
265+
// only granted on Linux. The PE branch here (linking the .dll path directly)
266+
// is therefore unproven: mingw's ld tolerates it, MSVC's link.exe cannot.
267+
// make_plan() rejects SharedLibrary targets on non-ELF targets rather than
268+
// emitting something unverifiable — see the guard there.
269+
std::vector<std::string> shared_library_link_flags(
270+
const mcpp::manifest::Target& t,
271+
const mcpp::toolchain::triple::ArtifactNaming& n,
272+
const mcpp::toolchain::triple::Triple& target) {
236273
std::vector<std::string> flags;
237-
if constexpr (mcpp::platform::is_windows) {
238-
flags.push_back(target_output(t).generic_string());
274+
const bool pe = n.sharedNeedsImportLib;
275+
const bool macho = target.empty() ? bool(mcpp::platform::is_macos)
276+
: target.os == "macos";
277+
if (pe) {
278+
flags.push_back(target_output(t, n).generic_string());
239279
} else {
240-
flags.push_back("-L" + target_output(t).parent_path().generic_string());
241-
if constexpr (mcpp::platform::supports_rpath) {
242-
if constexpr (mcpp::platform::is_macos) {
243-
flags.push_back("-Wl,-rpath,@loader_path");
244-
} else {
245-
flags.push_back("-Wl,-rpath,'$$ORIGIN'");
246-
}
247-
}
280+
flags.push_back("-L" + target_output(t, n).parent_path().generic_string());
281+
flags.push_back(macho ? "-Wl,-rpath,@loader_path"
282+
: "-Wl,-rpath,'$$ORIGIN'");
248283
flags.push_back("-l" + t.name);
249284
}
250285
return flags;
@@ -384,6 +419,45 @@ make_plan(const mcpp::manifest::Manifest& manifest,
384419
plan.manifest = manifest;
385420
plan.toolchain = tc;
386421
plan.fingerprint = fp;
422+
423+
// Artifact naming and shared-library link shape are properties of the
424+
// TARGET. Resolved once here from tc.targetTriple (empty = host target, in
425+
// which case the host constants ARE the right answer) and threaded down,
426+
// so nothing below reaches for mcpp::platform to describe an output.
427+
const auto targetTriple = [&] {
428+
auto t = mcpp::toolchain::triple::parse(tc.targetTriple);
429+
return t ? *t : mcpp::toolchain::triple::Triple{};
430+
}();
431+
const auto naming = naming_for(tc);
432+
433+
// Shared libraries have never been verified end to end on PE or Mach-O:
434+
// every shared-library e2e declares `# requires: elf`, and run_all.sh only
435+
// grants that capability on Linux. The non-ELF paths through
436+
// shared_library_link_flags are therefore unproven — mingw's ld tolerates
437+
// linking a .dll directly, MSVC's link.exe cannot, and neither has an
438+
// import library to link against because mcpp does not model one.
439+
//
440+
// Refusing is strictly better than emitting something unverifiable: a
441+
// branch that is neither tested nor willing to say no is the hardest kind
442+
// of debt, because it can be neither trusted nor deleted.
443+
if (!targetTriple.empty() && targetTriple.os != "linux") {
444+
for (auto const& t : manifest.targets) {
445+
if (t.kind != mcpp::manifest::Target::SharedLibrary) continue;
446+
return std::unexpected(std::format(
447+
"target '{}': shared libraries are only supported for Linux (ELF) "
448+
"targets today.\n"
449+
" target '{}' is kind=\"shared\"; build it as kind=\"lib\" "
450+
"(static) for this target,\n"
451+
" or build it for a linux target.\n"
452+
" note: PE consumers need an import library and Mach-O needs "
453+
"install-name handling;\n"
454+
" neither is modelled yet, so mcpp refuses rather than "
455+
"producing an artifact\n"
456+
" nothing has ever verified.",
457+
targetTriple.str(), t.name));
458+
}
459+
}
460+
387461
bool experimentalStd = false;
388462
if (auto stdCfg = mcpp::manifest::normalize_cpp_standard(manifest.package.standard)) {
389463
plan.cppStandard = stdCfg->canonical;
@@ -686,7 +760,7 @@ make_plan(const mcpp::manifest::Manifest& manifest,
686760
.packageIndex = i,
687761
.packageName = qname,
688762
.target = t,
689-
.output = target_output(t),
763+
.output = target_output(t, naming),
690764
});
691765
sharedTargetsByPackage[i].push_back(targetIndex);
692766
}
@@ -767,9 +841,9 @@ make_plan(const mcpp::manifest::Manifest& manifest,
767841
// (0.0.104-0.0.106). The failure surfaced far away, as
768842
// `libX11.so: undefined reference to xcb_connect` or a test
769843
// exiting 127.
770-
for (auto const& alias : runtime_aliases_for_target(dep.target))
844+
for (auto const& alias : runtime_aliases_for_target(dep.target, naming))
771845
lu.implicitInputs.push_back(alias);
772-
auto flags = shared_library_link_flags(dep.target);
846+
auto flags = shared_library_link_flags(dep.target, naming, targetTriple);
773847
lu.linkFlags.insert(lu.linkFlags.end(), flags.begin(), flags.end());
774848
}
775849
}
@@ -812,7 +886,7 @@ make_plan(const mcpp::manifest::Manifest& manifest,
812886
lu.kind = LinkUnit::SharedLibrary;
813887
lu.output = dep.output;
814888
lu.soname = dep.target.soname;
815-
lu.runtimeAliases = runtime_aliases_for_target(dep.target);
889+
lu.runtimeAliases = runtime_aliases_for_target(dep.target, naming);
816890
append_package_objects(lu, dep.packageName);
817891
append_direct_shared_deps(lu, dep.packageIndex);
818892
plan.linkUnits.push_back(std::move(lu));
@@ -833,19 +907,19 @@ make_plan(const mcpp::manifest::Manifest& manifest,
833907
lu.targetName = t.name;
834908
if (t.kind == mcpp::manifest::Target::Library) {
835909
lu.kind = LinkUnit::StaticLibrary;
836-
lu.output = target_output(t);
910+
lu.output = target_output(t, naming);
837911
} else if (t.kind == mcpp::manifest::Target::SharedLibrary) {
838912
lu.kind = LinkUnit::SharedLibrary;
839-
lu.output = target_output(t);
913+
lu.output = target_output(t, naming);
840914
lu.soname = t.soname;
841-
lu.runtimeAliases = runtime_aliases_for_target(t);
915+
lu.runtimeAliases = runtime_aliases_for_target(t, naming);
842916
} else if (t.kind == mcpp::manifest::Target::TestBinary) {
843917
lu.kind = LinkUnit::TestBinary;
844-
lu.output = target_output(t);
918+
lu.output = target_output(t, naming);
845919
if (!t.main.empty()) lu.entryMain = projectRoot / t.main;
846920
} else {
847921
lu.kind = LinkUnit::Binary;
848-
lu.output = target_output(t);
922+
lu.output = target_output(t, naming);
849923
if (!t.main.empty()) lu.entryMain = projectRoot / t.main;
850924
}
851925

0 commit comments

Comments
 (0)