From 1f31f366bb8a52c39c793c023396c704233bcf28 Mon Sep 17 00:00:00 2001 From: Atila Neves Date: Thu, 15 Feb 2024 11:06:50 +0100 Subject: [PATCH 1/3] Fix rerun with missing source directories --- payload/reggae/backend/make.d | 11 ++++++--- payload/reggae/backend/ninja.d | 33 +++++++++++++++++++------- tests/it/runtime/issues.d | 43 ++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/payload/reggae/backend/make.d b/payload/reggae/backend/make.d index 1075d892..c8f0a371 100644 --- a/payload/reggae/backend/make.d +++ b/payload/reggae/backend/make.d @@ -75,8 +75,9 @@ struct Makefile { //includes rerunning reggae string output() @safe { - import std.array: join; + import std.array: join, replace; import std.range: chain; + import std.algorithm: sort, uniq, map; auto ret = simpleOutput; @@ -85,8 +86,12 @@ struct Makefile { } else { // add a dependency on the Makefile to reggae itself and the build description, // but only if not exporting a build - ret ~= fileName() ~ ": " ~ chain(options.reggaeFileDependencies, _srcDirs).join(" ") ~ "\n"; - ret ~= "\t" ~ options.rerunArgs.join(" ") ~ "\n"; + auto srcDirs = _srcDirs.sort.uniq; + const rerunLine = "\t" ~ options.rerunArgs.join(" ") ~ "\n"; + + ret ~= fileName() ~ ": " ~ chain(options.reggaeFileDependencies, srcDirs.save).join(" ") ~ "\n"; + ret ~= rerunLine; + ret ~= "\n" ~ srcDirs.save.map!(d => d ~ ":" ~ "\n" ~ rerunLine).join("\n"); } return replaceEnvVars(ret); diff --git a/payload/reggae/backend/ninja.d b/payload/reggae/backend/ninja.d index 999e6f82..3d2014ae 100644 --- a/payload/reggae/backend/ninja.d +++ b/payload/reggae/backend/ninja.d @@ -38,20 +38,33 @@ struct Ninja { //includes rerunning reggae const(NinjaEntry)[] allBuildEntries() @safe { - import std.array: array; - import std.algorithm: sort, uniq; - import std.range: chain; + import std.array: array, replace; + import std.algorithm: sort, uniq, map; + import std.range: chain, only; + auto srcDirs = _srcDirs.sort.uniq; const files = flattenEntriesInBuildLine( - chain(_options.reggaeFileDependencies, _srcDirs.sort.uniq).array + chain(_options.reggaeFileDependencies, srcDirs).array ); auto paramLines = _options.oldNinja ? [] : ["pool = console"]; const(NinjaEntry)[] rerunEntries() { // if exporting the build system, don't include rerunning reggae - return _options.export_ - ? [] - : [NinjaEntry("build build.ninja: _rerun | " ~ files, paramLines)]; + if(_options.export_) + return []; + auto rerun = NinjaEntry("build build.ninja: _rerun | " ~ files, paramLines); + // the reason this is needed is because source directories + // can be deleted or renamed. If they are, ninja will + // complain about the missing directories since they are a + // dependency of the build.ninja file for rerunning + // reggae. So we consider them phony targets with no + // dependencies to make that not happen and ninja will + // rerun reggae and change the list of source directories + // accordingly. + auto phonySrcDirs = srcDirs + .map!(a => NinjaEntry("build " ~ a.escapeBuildLine ~ ": phony")) + ; + return chain(rerun.only, phonySrcDirs).array; } const defaultOutputs = _build.defaultTargetsOutputs(_projectPath); @@ -364,7 +377,7 @@ private: import std.algorithm: map; import std.array: join, replace; return entries - .map!(e => e.replace(":", "$:").replace(" ", "$ ")) + .map!escapeBuildLine .join(" "); } @@ -392,6 +405,10 @@ private: } } +private string escapeBuildLine(in string line) @safe pure { + import std.array: replace; + return line.replace(":", "$:").replace(" ", "$ "); +} struct NinjaEntry { string mainLine; diff --git a/tests/it/runtime/issues.d b/tests/it/runtime/issues.d index 2353b18f..3dd08b96 100644 --- a/tests/it/runtime/issues.d +++ b/tests/it/runtime/issues.d @@ -481,3 +481,46 @@ unittest { shouldExist("dub_objs"); } } + +version(DigitalMars) { + static foreach(backend; ["ninja", "make"]) { + @("rerun.deleted.dir." ~ backend) + @Tags(backend) + unittest { + import std.file: rmdirRecurse; + + with(immutable ReggaeSandbox()) { + + writeFile( + "reggaefile.d", + q{ + import reggae; + alias lib = staticLibrary!( + "mylib", + Sources!("source") + ); + mixin build!lib; + } + ); + writeFile("source/maths/foo.d", "int twice (int i) { return i * 2; }"); + writeFile("source/maths/bar.d", "int thrice(int i) { return i * 3; }"); + writeFile("source/util/baz.d", "void baz() {}"); + + runReggae("-b", backend, "--verbose"); + mixin(backend).shouldExecuteOk; + + // delete the source directory and make sure reggae gets rerun + rmdirRecurse(inSandboxPath("source/util")); + + static if(backend == "ninja") + enum msg = "missing and no known rule"; + else static if(backend == "make") + enum msg = "No rule to make target"; + else + static assert(false, "unknown backend"); + + mixin(backend).shouldFailToExecute.shouldNotContain(msg); + } + } + } +} From 19f5cee1d9177d16a00b8331e9febd313b32b3da Mon Sep 17 00:00:00 2001 From: Atila Neves Date: Thu, 15 Feb 2024 16:59:38 +0100 Subject: [PATCH 2/3] Fix binary backend executable name/location --- src/reggae/reggae.d | 14 +++++++++++--- tests/it/runtime/regressions.d | 20 ++++++++++++++++++++ 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/reggae/reggae.d b/src/reggae/reggae.d index 9d5fbdeb..2a454462 100644 --- a/src/reggae/reggae.d +++ b/src/reggae/reggae.d @@ -290,7 +290,7 @@ private Binary buildReggaefileDub(O)( import std.path: buildPath; import std.algorithm: map, joiner; import std.range: chain, only; - import std.array: replace; + import std.array: replace, join; // calculates .dep so getReggaeFileDependenciesDlang works below calculateReggaeFileDeps(output, options); @@ -318,13 +318,21 @@ private Binary buildReggaefileDub(O)( const dubRecipeDir = hiddenDirAbsPath(options); const dubRecipePath = buildPath(dubRecipeDir, "dub.sdl"); + const linesIfBinary = [ + `targetPath ".."`, + `targetName "build"`, + ]; + const extraLines = options.backend == Backend.binary + ? linesIfBinary + : []; + writeIfDiffers( output, dubRecipePath, reggaeFileDubSdl.format( userSourceFilesForDubSdl, importPathsForDubSdl, - ), + ) ~ extraLines.join("\n"), ); writeIfDiffers( @@ -499,7 +507,7 @@ private string getBuildGenName(in Options options) @safe pure nothrow { import reggae.rules.common: exeExt; const baseName = options.backend == Backend.binary - ? buildPath("../build") + ? "../build" : "buildgen"; return baseName ~ exeExt; } diff --git a/tests/it/runtime/regressions.d b/tests/it/runtime/regressions.d index 1d05f780..60543add 100644 --- a/tests/it/runtime/regressions.d +++ b/tests/it/runtime/regressions.d @@ -106,3 +106,23 @@ unittest { ninja.shouldFailToExecute(testPath); } } + +@("buildgen.binary") +@Tags("binary") +unittest { + import reggae.rules.common: exeExt; + with(immutable ReggaeSandbox()) { + writeFile( + "reggaefile.d", + q{ + import reggae; + alias lib = staticLibrary!("mylib", Sources!("src")); + mixin build!lib; + } + ); + + writeFile("src/foo.d", "void foo() {}"); + runReggae("-b", "binary"); + shouldExist("build" ~ exeExt); + } +} From 461f6a0602aca393ec45ec3d29368699c1dd89e0 Mon Sep 17 00:00:00 2001 From: Martin Kinkelin Date: Mon, 19 Feb 2024 19:44:17 +0100 Subject: [PATCH 3/3] Pass through stdin/err/out streams when running buildgen executable For a live output, as that run can take a while. --- src/reggae/reggae.d | 9 ++++----- tests/it/runtime/issues.d | 4 +--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/reggae/reggae.d b/src/reggae/reggae.d index 2a454462..83f64920 100644 --- a/src/reggae/reggae.d +++ b/src/reggae/reggae.d @@ -189,6 +189,7 @@ private string[] getJsonOutputArgs(in Options options) @safe { private void createBuild(T)(auto ref T output, in Options options) { import reggae.io: log; + import std.process: spawnProcess, wait; enforce(options.reggaeFilePath.exists, text("Could not find ", options.reggaeFilePath)); @@ -204,12 +205,10 @@ private void createBuild(T)(auto ref T output, in Options options) { //actually run the build generator output.log("Running the created binary to generate the build"); - immutable retRunBuildgen = execute([buildPath(hiddenDirAbsPath(options), buildGenName)]); - enforce(retRunBuildgen.status == 0, - text("Couldn't execute the produced ", buildGenName, " binary:\n", retRunBuildgen.output)); + immutable buildgenStatus = spawnProcess([buildPath(hiddenDirAbsPath(options), buildGenName)]).wait(); + enforce(buildgenStatus == 0, + text("Executing the produced ", buildGenName, " binary failed")); output.log("Build generated"); - - if(retRunBuildgen.output.length) output.log(retRunBuildgen.output); } diff --git a/tests/it/runtime/issues.d b/tests/it/runtime/issues.d index 3dd08b96..219324a3 100644 --- a/tests/it/runtime/issues.d +++ b/tests/it/runtime/issues.d @@ -329,9 +329,7 @@ version(Posix) { ] ); runReggae("-b", "ninja").shouldThrowWithMessage( - "Couldn't execute the produced buildgen binary:\n" ~ - "Cannot have a custom rule with no $in or $out: " ~ - "use `phony` or explicit $in/$out instead.\n"); + "Executing the produced buildgen binary failed"); } } }