Skip to content

Conversation

digit-google
Copy link
Contributor

This forces the load of depfile entries from either the Ninja log or explicit depfiles to be performed when visiting the graph, thus making them visible in the result.

  • graph.h, graph.cc: Add optional ImplicitDepLoader* argument to InputsCollector constructor, and if provided, use it to load deps during the visit if needed.

  • ninja.cc: Modify inputs and multi-inputs implementation to support a new --depfile or -D flag, and create an ImplicitDepLoader instance when it is used to initialize the InputsCollector instances.

    NOTE: Using std::make_unique<> fails on our MacOS CI builder, which still uses -std=gnu+11 for some unknown reason, so this uses std::unique_ptr<T>::reset(new T(...)) instead :-/

  • output_test.py: Add regression tests + fix minor typing issues.

Fixes #2618

@hehaoqian
Copy link

Hi, I wonder if this loads dependencies from the DynDep file?
If it does, the flag of --depfile may be inappropriate

@digit-google digit-google force-pushed the inputs-tool-with-depfile-deps branch 2 times, most recently from 57a1238 to 1697792 Compare July 7, 2025 12:36
@digit-google
Copy link
Contributor Author

Hi, I wonder if this loads dependencies from the DynDep file? If it does, the flag of --depfile may be inappropriate

No, the code only loads inputs from the deps log, so --depfile is an appropriate name (also this is what the original feature request asked for).

For dyndep files, the changes to the graph that they involve are a bit more aggressive, and I admit I am not sure at this point that the current collection algorithm would be correct when they are loaded. So probably better left for another PR.

@digit-google digit-google force-pushed the inputs-tool-with-depfile-deps branch from 1697792 to c1c2d19 Compare August 19, 2025 07:44
This forces the load of depfile entries from either the Ninja log
or explicit depfiles to be performed when visiting the graph, thus
making them visible in the result.

- graph.h, graph.cc: Add optional ImplicitDepLoader* argument to
  InputsCollector constructor, and if provided, use it to load deps
  during the visit if needed.

- ninja.cc: Modify `inputs` and `multi-inputs` implementation to
  support a new `--depfile` flag, and create an ImplicitDepLoader
  instance when it is used to initialize the InputsCollector
  instances.

- output_test.py: Add regression tests + fix minor typing issues.

Fixes ninja-build#2618
@digit-google digit-google force-pushed the inputs-tool-with-depfile-deps branch from c1c2d19 to 857bfa1 Compare September 26, 2025 09:12
if (!implicit_dep_loader_->LoadDeps(edge, &err)) {
// Print the error as a warning on stderr when an error occurred during
// the load.
Warning("%s", err.c_str());
Copy link

Choose a reason for hiding this comment

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

FYI I tried out this branch just now and got the following output on ninja itself:

(-zsh@hephaestus.local) ~/src/ninja (detached HEAD)
; ./ninja -t inputs --depfile
ninja: warning:
...
ninja: warning:
ninja: warning:
ninja: warning:
ninja: warning:
build/browse.o

Probably there is a bug here somewhere? I would at least expect err to be non-empty.

Copy link

Choose a reason for hiding this comment

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

I also don't see it including any of the depfile inputs, which may or may not be related.

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.

Feature Request: Include inputs from depfile in ninja -t inputs

3 participants