fix: give a project file its args only for the CrafterBuildProject call
ProjectArgs() read args that LoadProject recorded when it pushed the
frame, so they were there for everything that ran on that frame, not
only for CrafterBuildProject. Two things went wrong with that.
The pragma pre-pass runs on the frame too, so both of its specs needed
a hand-written `.args = {}` to keep the consumer's args from reaching
host tooling, and anything else built there would have needed one as
well. And dlopen runs a project file's static initialisers only on the
first load of that file: a later load gets the same mapping back. A
spec made at namespace scope therefore kept the first load's args for
every later load of the file, a silently wrong variant.
The frame now gets the args just before CrafterBuildProject is called.
The pre-pass and the static initialisers see none, on every load, so
the two opt-outs go. The dependencies the pre-pass resolved move the
other way, to just before dlopen: they are the same on every load, and
ProjectDependency() called from a static initialiser used to find none
and terminate the process.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
5cf295e1d4
commit
060ab4380f
2 changed files with 40 additions and 29 deletions
|
|
@ -413,9 +413,10 @@ namespace {
|
||||||
|
|
||||||
struct ProjectFileFrame {
|
struct ProjectFileFrame {
|
||||||
fs::path project;
|
fs::path project;
|
||||||
// What LoadProject was given, for ProjectArgs(). A view is enough:
|
// What LoadProject was given, for ProjectArgs(), and only once
|
||||||
// the caller's storage outlives LoadProject, and the frame is popped
|
// CrafterBuildProject is about to be called (see LoadProject). A view
|
||||||
// before LoadProject returns.
|
// is enough: the caller's storage outlives LoadProject, and the frame
|
||||||
|
// is popped before LoadProject returns.
|
||||||
std::span<const std::string_view> args;
|
std::span<const std::string_view> args;
|
||||||
std::vector<Configuration*> dependencies;
|
std::vector<Configuration*> dependencies;
|
||||||
};
|
};
|
||||||
|
|
@ -430,10 +431,7 @@ namespace {
|
||||||
// already on the stack is a cycle, which used to recurse forever.
|
// already on the stack is a cycle, which used to recurse forever.
|
||||||
class ProjectFileGuard {
|
class ProjectFileGuard {
|
||||||
public:
|
public:
|
||||||
// The args are recorded with the frame rather than just before
|
explicit ProjectFileGuard(const fs::path& absProject) {
|
||||||
// CrafterBuildProject runs, so a spec built while dlopen runs the
|
|
||||||
// project file's static initialisers sees them too.
|
|
||||||
ProjectFileGuard(const fs::path& absProject, std::span<const std::string_view> args) {
|
|
||||||
for (const ProjectFileFrame& frame : LoadingFrames) {
|
for (const ProjectFileFrame& frame : LoadingFrames) {
|
||||||
if (frame.project != absProject) continue;
|
if (frame.project != absProject) continue;
|
||||||
std::string chain;
|
std::string chain;
|
||||||
|
|
@ -441,7 +439,7 @@ namespace {
|
||||||
throw std::runtime_error(std::format("circular project dependency: {}{}", chain, absProject.string()));
|
throw std::runtime_error(std::format("circular project dependency: {}{}", chain, absProject.string()));
|
||||||
}
|
}
|
||||||
index_ = LoadingFrames.size();
|
index_ = LoadingFrames.size();
|
||||||
LoadingFrames.push_back({ absProject, args, {} });
|
LoadingFrames.push_back({ absProject, {}, {} });
|
||||||
}
|
}
|
||||||
~ProjectFileGuard() { LoadingFrames.pop_back(); }
|
~ProjectFileGuard() { LoadingFrames.pop_back(); }
|
||||||
ProjectFileGuard(const ProjectFileGuard&) = delete;
|
ProjectFileGuard(const ProjectFileGuard&) = delete;
|
||||||
|
|
@ -503,16 +501,15 @@ namespace {
|
||||||
std::set<std::string> libs;
|
std::set<std::string> libs;
|
||||||
|
|
||||||
for (const PragmaDependency& pragma : pragmas) {
|
for (const PragmaDependency& pragma : pragmas) {
|
||||||
// Host tooling is loaded with none of the consumer's args, so both
|
// Neither spec sets args, and the tool still gets none of the
|
||||||
// specs pass `.args = {}` instead of defaulting to them. The tool
|
// consumer's: this pre-pass runs before LoadProject hands the
|
||||||
// is built for the host whatever the consumer targets: given the
|
// frame its args, so ProjectArgs() is empty. Given the consumer's
|
||||||
// consumer's --target=, its project.cpp would pick sources and
|
// --target=, the tool's project.cpp would pick sources and flags
|
||||||
// flags for the wrong triple before ForceHostTooling resets the
|
// for the wrong triple, and its flags would split the tool's
|
||||||
// fields, and the consumer's own flags would hash it apart from
|
// variant from the one every other consumer shares.
|
||||||
// the build every other consumer of the tool shares.
|
|
||||||
Configuration* cfg = nullptr;
|
Configuration* cfg = nullptr;
|
||||||
if (IsGitSource(pragma.source)) {
|
if (IsGitSource(pragma.source)) {
|
||||||
GitProjectSpec spec{ .source = { pragma.source, pragma.branch, pragma.commit }, .args = {} };
|
GitProjectSpec spec{ .source = { pragma.source, pragma.branch, pragma.commit } };
|
||||||
if (!pragma.project.empty()) spec.projectFile = pragma.project;
|
if (!pragma.project.empty()) spec.projectFile = pragma.project;
|
||||||
cfg = GitProject(spec);
|
cfg = GitProject(spec);
|
||||||
} else {
|
} else {
|
||||||
|
|
@ -526,7 +523,7 @@ namespace {
|
||||||
PragmaError(absProject, pragma.line, "'project' only applies to a directory source");
|
PragmaError(absProject, pragma.line, "'project' only applies to a directory source");
|
||||||
}
|
}
|
||||||
if (!fs::exists(local)) PragmaError(absProject, pragma.line, std::format("no such project file: {}", local.string()));
|
if (!fs::exists(local)) PragmaError(absProject, pragma.line, std::format("no such project file: {}", local.string()));
|
||||||
cfg = LocalProject({ .projectFile = local, .args = {} });
|
cfg = LocalProject({ .projectFile = local });
|
||||||
}
|
}
|
||||||
if (std::ranges::contains(out.configs, cfg)) continue;
|
if (std::ranges::contains(out.configs, cfg)) continue;
|
||||||
|
|
||||||
|
|
@ -902,7 +899,7 @@ namespace {
|
||||||
Configuration Crafter::LoadProject(const fs::path& projectFile, std::span<const std::string_view> args) {
|
Configuration Crafter::LoadProject(const fs::path& projectFile, std::span<const std::string_view> args) {
|
||||||
RequireToolchain();
|
RequireToolchain();
|
||||||
fs::path absProject = fs::canonical(projectFile);
|
fs::path absProject = fs::canonical(projectFile);
|
||||||
ProjectFileGuard guard(absProject, args);
|
ProjectFileGuard guard(absProject);
|
||||||
fs::path buildDir = absProject.parent_path() / "build";
|
fs::path buildDir = absProject.parent_path() / "build";
|
||||||
if (!fs::exists(buildDir)) {
|
if (!fs::exists(buildDir)) {
|
||||||
fs::create_directories(buildDir);
|
fs::create_directories(buildDir);
|
||||||
|
|
@ -974,6 +971,9 @@ Configuration Crafter::LoadProject(const fs::path& projectFile, std::span<const
|
||||||
WriteStamp(StampPath(soPath), projectStamp);
|
WriteStamp(StampPath(soPath), projectStamp);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Before dlopen, so the file's static initialisers can already call
|
||||||
|
// ProjectDependency(): its dependencies are the same on every load.
|
||||||
|
guard.Frame().dependencies = std::move(preDeps.configs);
|
||||||
void* handle = dlopen(soPath.c_str(), RTLD_NOW | RTLD_GLOBAL);
|
void* handle = dlopen(soPath.c_str(), RTLD_NOW | RTLD_GLOBAL);
|
||||||
if (!handle) {
|
if (!handle) {
|
||||||
throw std::runtime_error(std::format("Failed to load project {}: {}", soPath.string(), dlerror()));
|
throw std::runtime_error(std::format("Failed to load project {}: {}", soPath.string(), dlerror()));
|
||||||
|
|
@ -986,6 +986,12 @@ Configuration Crafter::LoadProject(const fs::path& projectFile, std::span<const
|
||||||
throw std::runtime_error(std::format("CrafterBuildProject not found in {}: {}", soPath.string(), err));
|
throw std::runtime_error(std::format("CrafterBuildProject not found in {}: {}", soPath.string(), err));
|
||||||
}
|
}
|
||||||
|
|
||||||
guard.Frame().dependencies = std::move(preDeps.configs);
|
// The args only for the call itself, unlike the dependencies. A static
|
||||||
|
// initialiser runs once per process, on the first load of the file: a
|
||||||
|
// later load gets the same mapping back from dlopen, so a spec built at
|
||||||
|
// namespace scope kept the first load's args for every load after it.
|
||||||
|
// Now it gets none, whichever load ran it. The pragma pre-pass above
|
||||||
|
// also ran on this frame and relies on the args not being there yet.
|
||||||
|
guard.Frame().args = args;
|
||||||
return fn(args);
|
return fn(args);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -71,16 +71,21 @@ export namespace Crafter {
|
||||||
// clone can still differ in includeDirs, libDirs and libs.
|
// clone can still differ in includeDirs, libDirs and libs.
|
||||||
ExternalBuildResult ExternalOutputs(const ExternalDependency& dep, std::string_view target);
|
ExternalBuildResult ExternalOutputs(const ExternalDependency& dep, std::string_view target);
|
||||||
|
|
||||||
// The args LoadProject was given for the project file currently running
|
// The args the project file now running its CrafterBuildProject was
|
||||||
// its CrafterBuildProject: what that function receives, standard flags
|
// loaded with: everything that function receives, --target=, --debug and
|
||||||
// included (cfg.projectArgs is only the part ApplyStandardArgs left
|
// the other standard flags included. Not cfg.projectArgs, which holds
|
||||||
// over). Empty when no project file is running, and on any thread but
|
// only what ApplyStandardArgs left uninterpreted: `.args =
|
||||||
// the one loading it, because it reads the stack LoadProject keeps per
|
// cfg.projectArgs` would drop the target and the build type. Empty
|
||||||
// thread, as IsTopLevelProject does. The specs below default their args
|
// everywhere else, because it reads the stack LoadProject keeps per
|
||||||
// to it. Defined beside LoadProject, but declared here so those defaults
|
// thread, as IsTopLevelProject does: outside a project file, on any
|
||||||
// can see it: importing :Platform into this partition would make the
|
// thread but the one running it, once CrafterBuildProject has returned
|
||||||
// exported `struct Configuration;` above a redeclaration of :Platform's
|
// (a script runs later), and before it is called, which covers the
|
||||||
// unexported one, which clang rejects.
|
// file's static initialisers and its `#pragma crafter dependency`
|
||||||
|
// pre-pass. The specs below default their args to it. Defined beside
|
||||||
|
// LoadProject, but declared here so those defaults can see it: importing
|
||||||
|
// :Platform into this partition would make the exported `struct
|
||||||
|
// Configuration;` above a redeclaration of :Platform's unexported one,
|
||||||
|
// which clang rejects.
|
||||||
std::vector<std::string> ProjectArgs();
|
std::vector<std::string> ProjectArgs();
|
||||||
|
|
||||||
// Specification for a sibling crafter-build project to fetch and depend on.
|
// Specification for a sibling crafter-build project to fetch and depend on.
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue