From a318cd3518a10341eab16f1734a89e99ab02651c Mon Sep 17 00:00:00 2001 From: Hariprakash V Date: Thu, 25 Jun 2026 11:19:58 +0530 Subject: [PATCH 1/2] fix: emit -J flags for stringImportPaths and add behavioral makedeps test --- source/dub/generators/ninja.d | 6 +++++- test/ninja-generator.sh | 24 ++++++++++++++++++++++++ test/ninja-generator/dub.json | 3 ++- test/ninja-generator/source/app.d | 4 +++- test/ninja-generator/views/data.txt | 1 + 5 files changed, 35 insertions(+), 3 deletions(-) create mode 100644 test/ninja-generator/views/data.txt diff --git a/source/dub/generators/ninja.d b/source/dub/generators/ninja.d index 79b8a756b5..02b44a36d7 100644 --- a/source/dub/generators/ninja.d +++ b/source/dub/generators/ninja.d @@ -37,7 +37,9 @@ class NinjaGenerator : ProjectGenerator f.writeln("ar = ar"); f.writeln(); f.writeln("rule dc"); - f.writeln(" command = ", compiler, " $flags -c $in -of=$out"); + f.writeln(" command = ", compiler, " $flags -c $in -of=$out -makedeps=$out.dep"); + f.writeln(" depfile = $out.dep"); + f.writeln(" deps = gcc"); f.writeln(" description = Compiling $in"); f.writeln(); f.writeln("rule link"); @@ -70,12 +72,14 @@ class NinjaGenerator : ProjectGenerator auto bs = info.buildSettings; auto importFlags = bs.importPaths.map!(p => "-I" ~ p).join(" "); + auto strImportFlags = bs.stringImportPaths.map!(p => "-J" ~ p).join(" "); auto versionFlags = bs.versions.map!(v => versionFlag(cname) ~ v).join(" "); auto debugFlags = bs.debugVersions.map!(v => debugFlag(cname) ~ v).join(" "); auto extraFlags = bs.dflags.join(" "); string[] parts; if (importFlags.length) parts ~= importFlags; + if (strImportFlags.length) parts ~= strImportFlags; if (versionFlags.length) parts ~= versionFlags; if (debugFlags.length) parts ~= debugFlags; if (extraFlags.length) parts ~= extraFlags; diff --git a/test/ninja-generator.sh b/test/ninja-generator.sh index a75785d93e..95338fa5e8 100755 --- a/test/ninja-generator.sh +++ b/test/ninja-generator.sh @@ -19,4 +19,28 @@ if ! grep -q "rule link" build.ninja; then die $LINENO 'build.ninja missing link rule!' fi +ninja -t clean +if ! ninja 2>&1; then + die $LINENO 'initial ninja build failed!' +fi + +output1=$(./ninja-generator) +if [ "$output1" != "hello" ]; then + die $LINENO "expected program output 'hello', got '$output1'" +fi + +echo -n "world" > views/data.txt + +if ! ninja 2>&1; then + die $LINENO 'rebuild after data.txt change failed!' +fi + +output2=$(./ninja-generator) +if [ "$output2" != "world" ]; then + die $LINENO "expected program output 'world' after data.txt change, got '$output2'" +fi + +echo -n "hello" > views/data.txt + +ninja -t clean rm -f build.ninja diff --git a/test/ninja-generator/dub.json b/test/ninja-generator/dub.json index 136c900a36..5ef941e8af 100644 --- a/test/ninja-generator/dub.json +++ b/test/ninja-generator/dub.json @@ -1,4 +1,5 @@ { "name": "ninja-generator", - "targetType": "executable" + "targetType": "executable", + "stringImportPaths": ["views"] } diff --git a/test/ninja-generator/source/app.d b/test/ninja-generator/source/app.d index ab73b3a234..123510fdba 100644 --- a/test/ninja-generator/source/app.d +++ b/test/ninja-generator/source/app.d @@ -1 +1,3 @@ -void main() {} +import std.stdio; +enum s = import("data.txt"); +void main() { write(s); } diff --git a/test/ninja-generator/views/data.txt b/test/ninja-generator/views/data.txt new file mode 100644 index 0000000000..b6fc4c620b --- /dev/null +++ b/test/ninja-generator/views/data.txt @@ -0,0 +1 @@ +hello \ No newline at end of file From 5ca165dbb9c9fef5cc131eea22059f9a79246ff7 Mon Sep 17 00:00:00 2001 From: Hariprakash V Date: Mon, 3 Aug 2026 16:17:04 +0530 Subject: [PATCH 2/2] ninja: escape import/string-import paths, add real makedeps test -I and -J flags weren't escaped through escapeNinjaPath(), same Windows colon issue already fixed for src/regenInputs/lflags. Replaced the bash rebuild check in ninja-generator.sh with a proper ninja-makedeps.script.d test that proves the depfile tracks transitive imports, not just that the flag exists. Verified by breaking makedeps emission and confirming the test catches it. --- source/dub/generators/ninja.d | 4 +- test/ninja-generator.sh | 24 ---------- test/ninja-generator/dub.json | 3 +- test/ninja-generator/source/app.d | 4 +- test/ninja-generator/views/data.txt | 1 - test/ninja-makedeps.script.d | 73 +++++++++++++++++++++++++++++ test/ninja-makedeps/dub.json | 4 ++ test/ninja-makedeps/source/app.d | 2 + test/ninja-makedeps/source/helper.d | 1 + 9 files changed, 84 insertions(+), 32 deletions(-) delete mode 100644 test/ninja-generator/views/data.txt create mode 100644 test/ninja-makedeps.script.d create mode 100644 test/ninja-makedeps/dub.json create mode 100644 test/ninja-makedeps/source/app.d create mode 100644 test/ninja-makedeps/source/helper.d diff --git a/source/dub/generators/ninja.d b/source/dub/generators/ninja.d index 02b44a36d7..12588239ab 100644 --- a/source/dub/generators/ninja.d +++ b/source/dub/generators/ninja.d @@ -71,8 +71,8 @@ class NinjaGenerator : ProjectGenerator { auto bs = info.buildSettings; - auto importFlags = bs.importPaths.map!(p => "-I" ~ p).join(" "); - auto strImportFlags = bs.stringImportPaths.map!(p => "-J" ~ p).join(" "); + auto importFlags = bs.importPaths.map!(p => "-I" ~ escapeNinjaPath(p)).join(" "); + auto strImportFlags = bs.stringImportPaths.map!(p => "-J" ~ escapeNinjaPath(p)).join(" "); auto versionFlags = bs.versions.map!(v => versionFlag(cname) ~ v).join(" "); auto debugFlags = bs.debugVersions.map!(v => debugFlag(cname) ~ v).join(" "); auto extraFlags = bs.dflags.join(" "); diff --git a/test/ninja-generator.sh b/test/ninja-generator.sh index 95338fa5e8..a75785d93e 100755 --- a/test/ninja-generator.sh +++ b/test/ninja-generator.sh @@ -19,28 +19,4 @@ if ! grep -q "rule link" build.ninja; then die $LINENO 'build.ninja missing link rule!' fi -ninja -t clean -if ! ninja 2>&1; then - die $LINENO 'initial ninja build failed!' -fi - -output1=$(./ninja-generator) -if [ "$output1" != "hello" ]; then - die $LINENO "expected program output 'hello', got '$output1'" -fi - -echo -n "world" > views/data.txt - -if ! ninja 2>&1; then - die $LINENO 'rebuild after data.txt change failed!' -fi - -output2=$(./ninja-generator) -if [ "$output2" != "world" ]; then - die $LINENO "expected program output 'world' after data.txt change, got '$output2'" -fi - -echo -n "hello" > views/data.txt - -ninja -t clean rm -f build.ninja diff --git a/test/ninja-generator/dub.json b/test/ninja-generator/dub.json index 5ef941e8af..136c900a36 100644 --- a/test/ninja-generator/dub.json +++ b/test/ninja-generator/dub.json @@ -1,5 +1,4 @@ { "name": "ninja-generator", - "targetType": "executable", - "stringImportPaths": ["views"] + "targetType": "executable" } diff --git a/test/ninja-generator/source/app.d b/test/ninja-generator/source/app.d index 123510fdba..ab73b3a234 100644 --- a/test/ninja-generator/source/app.d +++ b/test/ninja-generator/source/app.d @@ -1,3 +1 @@ -import std.stdio; -enum s = import("data.txt"); -void main() { write(s); } +void main() {} diff --git a/test/ninja-generator/views/data.txt b/test/ninja-generator/views/data.txt deleted file mode 100644 index b6fc4c620b..0000000000 --- a/test/ninja-generator/views/data.txt +++ /dev/null @@ -1 +0,0 @@ -hello \ No newline at end of file diff --git a/test/ninja-makedeps.script.d b/test/ninja-makedeps.script.d new file mode 100644 index 0000000000..737b78c8f3 --- /dev/null +++ b/test/ninja-makedeps.script.d @@ -0,0 +1,73 @@ +/+ dub.sdl: + name "ninja-makedeps-regen" + dependency "common" path="./common" + +/ + +module ninja_makedeps_regen; + +import std.process : environment, execute, Config; +import std.path : buildPath, dirName; +import std.file : readText, write, timeLastModified, exists; +import std.algorithm : canFind; +import core.thread : Thread; +import core.time : seconds; + +import common; + +int main() +{ + const dub = environment.get("DUB", buildPath(__FILE_FULL_PATH__.dirName.dirName, "bin", "dub")); + const dc = environment.get("DC", "dmd"); + const curr_dir = environment.get("CURR_DIR", buildPath(__FILE_FULL_PATH__.dirName)); + const projDir = buildPath(curr_dir, "ninja-makedeps"); + + if (execute([dub, "generate", "ninja", "--compiler", dc], null, Config.none, size_t.max, projDir).status) + die("dub generate ninja failed"); + + const buildNinjaPath = buildPath(projDir, "build.ninja"); + if (!readText(buildNinjaPath).canFind("-makedeps")) + die("build.ninja missing -makedeps flag on dc rule"); + if (!readText(buildNinjaPath).canFind("depfile =")) + die("build.ninja missing depfile directive on dc rule"); + + execute(["ninja", "-t", "clean"], null, Config.none, size_t.max, projDir); + if (execute(["ninja"], null, Config.none, size_t.max, projDir).status) + die("initial ninja build failed"); + + // Locate the actual app.o produced, since the generator encodes the full + // source path into the object filename. + import std.file : dirEntries, SpanMode; + string findObj(string moduleName) + { + foreach (entry; dirEntries(projDir, SpanMode.shallow)) + if (entry.name.canFind("_" ~ moduleName ~ ".o")) + return entry.name; + return ""; + } + + const objPath = findObj("app"); + if (!objPath.length || !exists(objPath)) + die("could not locate app.o after initial build"); + + const mtimeBefore = timeLastModified(objPath); + + const helperPath = buildPath(projDir, "source", "helper.d"); + const origHelper = readText(helperPath); + + Thread.sleep(1.seconds); + write(helperPath, origHelper ~ "\n// touched\n"); + scope(exit) write(helperPath, origHelper); + + if (execute(["ninja"], null, Config.none, size_t.max, projDir).status) + die("rebuild after touching helper.d failed"); + + const mtimeAfter = timeLastModified(objPath); + + if (mtimeAfter <= mtimeBefore) + die("app.o was not recompiled after helper.d changed -- depfile is not tracking transitive imports"); + + execute(["ninja", "-t", "clean"], null, Config.none, size_t.max, projDir); + + log("PASS"); + return 0; +} diff --git a/test/ninja-makedeps/dub.json b/test/ninja-makedeps/dub.json new file mode 100644 index 0000000000..7d9b84f35a --- /dev/null +++ b/test/ninja-makedeps/dub.json @@ -0,0 +1,4 @@ +{ + "name": "ninja-makedeps", + "targetType": "executable" +} diff --git a/test/ninja-makedeps/source/app.d b/test/ninja-makedeps/source/app.d new file mode 100644 index 0000000000..c42ee2a33b --- /dev/null +++ b/test/ninja-makedeps/source/app.d @@ -0,0 +1,2 @@ +import helper; +void main() { helperFunc(); } diff --git a/test/ninja-makedeps/source/helper.d b/test/ninja-makedeps/source/helper.d new file mode 100644 index 0000000000..d76364d358 --- /dev/null +++ b/test/ninja-makedeps/source/helper.d @@ -0,0 +1 @@ +void helperFunc() {}