Skip to content

Restore "explain" output when a dyndep file is loaded#2753

Open
bradking wants to merge 4 commits intoninja-build:masterfrom
bradking:explain-dyndep
Open

Restore "explain" output when a dyndep file is loaded#2753
bradking wants to merge 4 commits intoninja-build:masterfrom
bradking:explain-dyndep

Conversation

@bradking
Copy link
Copy Markdown
Contributor

@bradking bradking commented Mar 24, 2026

Since #2067 the "loading dyndep file '...'" explanation has not been printed. Instead the explanation has been recorded for the dyndep file's node after it has been brought up-to-date, and therefore never printed.

This explanation is generated just before its operation runs, so we can print it immediately instead of buffering it.

Fixes: #2758

@bradking
Copy link
Copy Markdown
Contributor Author

@jhasse this fixes a regression in ninja -d explain output with dyndep bindings that I noticed while working on #2749.

Copy link
Copy Markdown
Contributor

@digit-google digit-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you provide a reproduction case that better explains the issue, as well as a regression test for this?

@bradking bradking changed the title explain: restore output when a dyndep file is loaded Restore "explain" output when a dyndep file is loaded Mar 27, 2026
@bradking
Copy link
Copy Markdown
Contributor Author

a reproduction case...a regression test for this?

I opened #2758 to document the problematic case, and added a corresponding test to misc/output_test.py.

While working on this I also noticed #2759, but that behavior has been present since dyndep was introduced and is not a regression. I've left that as a FIXME comment in the test here, and will defer a fix to a later PR.

@bradking bradking requested a review from digit-google March 27, 2026 00:23
Copy link
Copy Markdown
Contributor

@digit-google digit-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

bradking added 3 commits April 3, 2026 09:16
It makes more sense to live in the latter.

Suggested-by: David 'Digit' Turner <digit+github@google.com>
This was regressed by commit 8e6c741 ("explain" debug prints just
before each command is run, 2022-01-06, v1.13.0~1^2~56^2~2).  Add a
test demonstrating the problems, called out by FIXME comments.

Issue: ninja-build#2758, ninja-build#2759
Since commit 8e6c741 ("explain" debug prints just before each command
is run, 2022-01-06, v1.13.0~1^2~56^2~2) the "loading dyndep file '...'"
explanation has not been printed.  Instead the explanation has been
recorded for the dyndep file's node *after* it has been brought
up-to-date, and therefore never printed.

This explanation is generated just before its operation runs, so we can
print it immediately instead of buffering it.

Fixes: ninja-build#2758
Copy link
Copy Markdown
Contributor

@digit-google digit-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ninja no longer explains when dyndep files are loaded

3 participants