feat: regenerate build.ninja when dub.json or dub.selections.json change - #3134
Merged
Conversation
Added a regen rule plus a build edge for build.ninja so it rebuilds itself whenever dub.json or dub.selections.json changes. Only adds selections.json as a dependency if it actually exists, since packages with no deps don't have one. Test checks for the regen rule, the generator attribute, and the build edge in ninja-generator.sh. Verified it breaks without this and works with it. Signed-off-by: Hariprakash V <hariprakash4389285@gmail.com>
…egeneration on dub.json change
|
✅ PR OK, no changes in deprecations or warnings Total deprecations: 0 Total warnings: 0 Build statistics: statistics (-before, +after)
-executable size=5722856 bin/dub
-rough build time=53s
+executable size=5726952 bin/dub
+rough build time=54sFull build output |
atilaneves
requested changes
Jul 21, 2026
atilaneves
reviewed
Jul 22, 2026
atilaneves
reviewed
Jul 24, 2026
atilaneves
left a comment
atilaneves
left a comment
Contributor
There was a problem hiding this comment.
This still doesn't handle dub.sdl
atilaneves
reviewed
Jul 27, 2026
atilaneves
reviewed
Jul 28, 2026
The regen test only checked for the log message and exit status, which could pass even if build.ninja wasn't actually regenerated. Now it also mutates the recipe (dub.json/dub.sdl) to add an extra import path and checks that build.ninja actually reflects the change, per Atila's review. Also fixes lflags the same way src/regenInputs were already fixed: escaping paths so Windows drive-letter colons don't break ninja.
atilaneves
approved these changes
Jul 31, 2026
atilaneves
enabled auto-merge
July 31, 2026 11:03
Contributor
Author
|
Hi Atila, thanks for approving this one. Quick follow up on #3130 (makedeps) too. I asked on the 22nd whether it landed some other way or needs resubmitting, haven't heard back yet. I've got the commit ready to reapply on current master if you'd like a fresh PR, just want your go ahead first since #3129/#3130 already caused mixup once. No rush, just flagging since final GSoC submissions open Aug 17. |
thewilsonator
disabled auto-merge
August 1, 2026 08:09
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a regen rule and build.ninja edge so it rebuilds itself
automatically whenever dub.json changes (dub.selections.json too,
if present).
Independent of #3130 (makedeps-v3, still unmerged) - builds cleanly
on current upstream/master with subpkg (#3133) already merged.
Test verifies the actual behavior, not just config text: touches
dub.json, runs ninja, and asserts "Regenerating build.ninja" genuinely
appears in the output. Confirmed the test fails when the regen rule
is removed and passes when restored. Verified via fresh clone build.