Avoid rerunning mkvmi repeatedly for no reason #875
Conversation
Previously, defatom/mkvmi avoided updating the output file (and its mtime) if the newly generated data is identical to the existing contents of the file. This caused problems if the input files have changed, but in a way that doesn't change the output data. In that case, defatom/mkvmi is called on every build - the output is considered outdated as its mtime is older than that of the inputs, but running defatom/mkvmi doesn't update the mtime, so on the next build it's still considered outdated. This would repeat until the inputs change in a way that actually changes the output. To fix this, defatom/mkvmi now unconditionally updates the output file. This fixes the problem above, at the cost of sometimes causing an unnecessary rebuild of things that depend on the outputs (but this will only happen once per change, not on every build as with the previous problem).
|
I'm not sure this is a good or a bad step. mkvmi is a quick step while an unneeded build of the whole is a lot slower. I wonder whether there is a more elegant way to solve this? It should be a known problem ... |
|
That's true, and I can understand it you want to leave it the way it is right now because it's quicker for active development. mkvmi runs fast enough that it doesn't really matter if it runs more often than needed. I don't know if there is any way to handle this properly, i. e. mark the output as updated without also causing a rebuild of everything that depends on it. I feel like this is impossible with the simple make-style model where only the mtime matters - either you update the output mtime, which makes all dependents of the output outdated, or you don't update the mtime, which leaves the output as outdated. Maybe there's some advanced CMake feature that can help with this, but if there is, I don't know about it... |
|
There has been lots of discussion on this around ninja. See e.g. ninja-build/ninja#1459. I propose to keep things as is for now. Not that for development of the Prolog libraries there is no need to build the system. Just use the binary from |
This is a pretty minor thing, but I noticed that every time I run
cmake --build, it regenerates vmi-metadata.h for no apparent reason. The CMake scripts say that it depends only on pl-vmi.c and the mkvmi utility itself, neither of which should change on every build. So I debugged this further usingmake -dand found this in the output:As it turns out,
mkvmitries to be smart - if the generated file already exists with identical contents, it doesn't overwrite the file to avoid touching the mtime. Unfortunately this causes problems here -mkvmiis being called because the output file is outdated, but then it doesn't actually update the output file's mtime, so on the next build the file is still considered outdated andmkvmiis called again, etc.This PR removes the extra check and simply rewrites the output file every time, which fixes this problem. The downside is that now every time that pl-vmi.c changes, all dependents of vmi-metadata.h (i. e. most of the SWI-Prolog core) are rebuilt, even if the contents of vmi-metadata.h haven't actually changed.
The same problem applies to defatom, but it's less of an issue there, because changing the ATOMS input file will almost always cause a real change to the generated headers.