-
Notifications
You must be signed in to change notification settings - Fork 407
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
Avoid rewriting test files when calling cmake #3548
Avoid rewriting test files when calling cmake #3548
Conversation
Retest this please. |
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.
Please comment that you are writing a temporary intermediate file and calling configure_file
to avoid updating timestamps which would trigger unnecessary rebuilds on subsequent cmake runs.
See |
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 can approve without the suggested change - but curious what you think.
"#include <Test${Tag}_Category.hpp>\n" | ||
"#include <Test${Name}.hpp>\n" | ||
) | ||
configure_file(${dir}/dummy.cpp ${file}) |
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.
Would it be better to have a dummy.cpp.in with @test@, etc and skip the first file(WRITE
?
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.
Your last commit invalidates the comments. You must fix it.
I do not like your implementation of the changes suggested by @jjwilke. One now has to open test.cpp.in
to understand what is going on and nothing in the name of the variables indicates that they are being substituted in the file being configured.
My preference goes to revert.
03b5fee
to
8aedb69
Compare
Retest this please. |
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.
Yep, I'm not going fight on this one : )
Previously, we were changing the timestamp of the generated test files every time we called
cmake
resulting in unnecessary rebuilds.configure_file
only changes the files if they differ.