-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix full dependencies that should be order only in ninja backend #13377
base: master
Are you sure you want to change the base?
Fix full dependencies that should be order only in ninja backend #13377
Conversation
3897d7d
to
4013ad3
Compare
4013ad3
to
b97501d
Compare
b97501d
to
3e24341
Compare
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 think the third commit message could be a little more explicit. Like "refactored by adding a add_order_deps
function", or something like that.
header dependency are usually order-only, but this actually provides a full dependency.
Due to the confusing naming of the parameters, a number of arguments were incorrectly added as full dependencies when they should be order only.
3e24341
to
c6963bb
Compare
@bruchar1 done and done. Thanks for the review :) |
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.
LGTM! Should it be added to the next point release?
I don't think so. This change potentially makes re-compiles faster when using generated headers, but it doesn't affect the correctness of the build |
The current terminology is odd, it calls them "header deps" and "order deps", but headers are only order deps, and order deps are full dependencies. This renames and fixes the kind of dependencies being used.