Fix Makefile arginfo files generation rule #20891
Conversation
If the mtime of the stub file is more recent than arginfo's one, but the content is unchanged, the build script will launch gen_stub.php without any effect, and attempt to do so over and over. By touching the arginfo file, we ensure that no more attempts are made unless the stub file is modified again.
|
What's the right target for this? I have more similar fixes to the build process coming up. |
8.4 looks right. |
| if test ! -z "$(PHP)"; then \ | ||
| echo Parse $< to generate $@;\ | ||
| $(PHP) $(top_srcdir)/build/gen_stub.php $<; \ | ||
| touch $@; \ |
There was a problem hiding this comment.
Should the touch() rather be included in gen_stub.php to make it self-contained?
There was a problem hiding this comment.
I thought about it. My reasoning is that updating the timestamp is strictly makefile business, because that's how makefile works. If you think about the gen_stub.php script, when you invoke it manually, has no business updating the timestamp in case of a no-op, it doesn't care nor it shouldn't. Hence, I figured it belongs to the makefile.
There was a problem hiding this comment.
I disagree. When I run gen_stub.php, I expect it to generate the resulting artifacts. The hash check is an optimization / implementation detail to me, except it's currently a leaky optimization, since it is observable. Moving the touch() into gen_stub.php would be the right thing.
Also: What would happen if gen_stub.php fails for some reason? Would the touch still be executed? Is the shell running with -e?
There was a problem hiding this comment.
Also: What would happen if
gen_stub.phpfails for some reason? Would thetouchstill be executed? Is the shell running with-e?
Good point, I assumed it does and the execution would stop in case of failure, but now that you point it out... I'll test it anyway and then update the PR with the touch inside the script.
If the mtime of the stub file is more recent than arginfo's one, but the content is unchanged, the build script will launch gen_stub.php without any effect, and attempt to do so over and over.
By touching the arginfo file, we ensure that no more attempts are made unless the stub file is modified again.