-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Add --depfile flag to inputs
and multi-inputs
tools.
#2626
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add --depfile flag to inputs
and multi-inputs
tools.
#2626
Conversation
Hi, I wonder if this loads dependencies from the DynDep file? |
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. |
1697792
to
c1c2d19
Compare
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
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.
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.
There was a problem hiding this comment.
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.
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
andmulti-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