mtime set to 0 in .ninja_log for outputs of rule with restat = 1 whose sole dependency is a phony target

[sdefresne]

This was found when making a change in gn to use phony targets (https://gn-review.git.corp.google.com/c/gn/+/16920) for Chromium.

If a target use a custom rule, has restat = 1, does not modify its output, and only has phony target as outputs, then ninja will record a mtime of 0 for that target if it decides to rebuild it as part of an incremental build. From that point, the build will always be considered as dirty because the recorded mtime (0) will always be older than the mtime of the phony target.

Sample reproduction:

$ mkdir repro && cd repro
$ touch a
$ cat > build.ninja <<EOF
rule create_b
  command = test -f b || touch b
  restat = 1

build s: phony a
build b: create_b | s
EOF
$ ninja b -d explain
ninja explain: output b doesn't exist
[1/1] test -f b || touch b
$ ninja b -d explain
ninja: no work to do.
$ touch a
$ ninja b -d explain
ninja explain: restat of output b older than most recent input s (1711640092851462338 vs 1711640098788860000)
[1/1] test -f b || touch b
$ ninja b -d explain
ninja explain: restat of output b older than most recent input s (0 vs 1711640098788860000)
[1/1] test -f b || touch b
$ cat .ninja_log 
# ninja log v5
0	10	1711640092851462338	b	142acf958f60ee36
1	7	0	b	142acf958f60ee36
1	7	0	b	142acf958f60ee36

As can be seen, the mtime for b is stored as 0 after the target is rebuild due to the modification of a.

I think this is because the following code in Builder::FinishCommand(...):

  // Restat the edge outputs
  TimeStamp output_mtime = 0;
  bool restat = edge->GetBindingBool("restat");
  if (!config_.dry_run) {
    bool node_cleaned = false;
    ...

    if (node_cleaned) {
      TimeStamp restat_mtime = 0;
      // If any output was cleaned, find the most recent mtime of any
      // (existing) non-order-only input or the depfile.
      for (vector<Node*>::iterator i = edge->inputs_.begin();
           i != edge->inputs_.end() - edge->order_only_deps_; ++i) {
        // here (*i)->path() is "s" which does not exist, so
        // Stat() returns 0.
        TimeStamp input_mtime = disk_interface_->Stat((*i)->path(), err);
        ...
      }
      
      ...

      output_mtime = restat_mtime;
    }
  }

[sdefresne]

So it looks like it reproduce with origin/release but not with origin/master.

After looking at the changes between those two branches, it looks like this was fixed by a2b5e6d.