Add --depfile flag to inputs and multi-inputs tools.#2626
Add --depfile flag to inputs and multi-inputs tools.#2626digit-google wants to merge 1 commit intoninja-build:masterfrom
inputs and multi-inputs tools.#2626Conversation
|
Hi, I wonder if this loads dependencies from the DynDep file? |
digit-google
force-pushed
the
inputs-tool-with-depfile-deps
branch
2 times, most recently
from
57a1238 to
1697792
Compare
No, the code only loads inputs from the deps log, so 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
force-pushed
the
inputs-tool-with-depfile-deps
branch
from
1697792 to
c1c2d19
Compare
digit-google
force-pushed
the
inputs-tool-with-depfile-deps
branch
from
c1c2d19 to
857bfa1
Compare
| 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()); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I also don't see it including any of the depfile inputs, which may or may not be related.
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. Note in particular that rules are defined with and without `deps = gcc` as this triggers different code paths in Ninja in the deps loader. Fixes ninja-build#2618
digit-google
force-pushed
the
inputs-tool-with-depfile-deps
branch
from
857bfa1 to
bee2e39
Compare
|
Getting back to this PR after some time. I just pushed a fix for the As @jyn514 tested, it was not working correctly because the tools now need the The regression tests were not catching these because they used |
|
I tested your fixes and they work like a charm :) |
3 participants
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
inputsandmulti-inputsimplementation to support a new--depfileor-Dflag, 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