Improve incremental build: Make outputs modified outside of the build system considered dirty

[Dragnalith]

(The previous PR #1945 was closed automatically because of a branch renaming on my side, so I am re-opening a new one)

This pull request is a fix for #1946.

The Problem

The general problem I am trying to solve is improving the correctness of the incremental build. I think we can say the incremental build is correct when the result of running ninja is the same as doing a full build (enforcing all rule command to be executed again). Today there are at least two scenarios where this is not the case:

• If you modified an output (manually or by mistake), ninja will not rebuilt. (this is the problem addressed by this PR)
• If you rule command create files that cannot be predicted, i.e dynamic outputs, ninja will not be aware about those file and won't re-run the rule command if those files are modified or deleted. (this problem is addressed in the PR Improve incremental build: make ninja handle dynamic outputs #1953 )

Example In Practice

• You have an action registered as order-only dependency which generate some C++ header.
• You include one of generated file for the time to one of your .cpp, so ninja does not consider it yet as implicit dependency.
• You open the generated file with your favorite text editor, and by mistyping on your keyboard you modify the generated header to an invalid C++ state.
• As consequence, ninja will not try to re-generate the header, because it is not yet a dependency of the build, and ninja will fail the build because the header is not valid C++.

My Solution

Today if an output of the build is modified from outside of the build system (manually or by another process) ninja will not consider it to be dirty.

My modification considered an output is dirty if its mtime is more recent than the one in the build log. The mtime of the output can be older than the one on the build log in the case its build edge is mark with 'restat = 1'. In that case if the rule command did not modified the output, its build log entry will inherit the mtime of its most recent inputs or depfile.

Also, since in some scenario like when you consciously modified outputs for quick testing you may not want ninja to override your change, my implementation makes it optional. For backward compatibility ninja keep the same default behavior, but if run with -m flag, it will considered modified outputs as dirty

[bradking]

Cc: @jdrouhard

[jdrouhard]

Few thoughts:

1. This is incompatible with Provide resiliency against inputs changing during the build #1943 as it stands. That PR has been reviewed and is just waiting on being merged. If I understand what this PR is trying to solve, I think Provide resiliency against inputs changing during the build #1943 does it as well. It changes how the mtime stored in the log is calculated and used, and should be resilient to dependencies (even undiscovered ones) being changed while running a build. It doesn't fix changing leaf node outputs after a run has completed and expecting them to rebuild, however.
2. If we want to make ninja more generally consider outputs that have changed since the previous run as dirty, I think that merits broader discussion. Build systems generally only care about whether the inputs that produce the outputs have changed, not whether outputs themselves might be different. I'm not directly opposed to the idea, but I think we'd need a build log version bump and we'd need to restat every output when commands have finished (not just generator and restat rules) and store that mtime separately (and use it only to compare the output's mtime on subsequent builds).
3. I don't think adding a command line option is appropriate here. ninja should either always do this or not.

[Dragnalith]

@jdrouhard

I have looked at #1943 and I understand what it solves and how it is incompatible with my PR.

Actually it is not an implementation incompatibility, it is a ninja specification incompatibility: is the purpose of a ninja build to give the same result as a full rebuild?

If the answer is yes, the problem addressed by #1943 (i.e what happen if inputs change after starting executing a rule command but before the output has been written) should lead to a ninja build failure. The current solution of #1943 is to succeed the build, but consider the edge dirty, so it will re-execute next time ninja is called.

I understand it may exist reason that in practice we do not want ninja to tend to a correct incremental build.

[Dragnalith]

@jdrouhard I think the only to make both approaches compatible will be to add a new column the build log to store the actual mtime of the output. There would be two mtimes: record_mtime and actual_mtime. record_mtime would be used as it is today, to know if an output should be rebuilt, and actual_mtime would let ninja know if the output has been modified since the last time it has been logged.