dyndep support

[Duncaen]

rule untar
  command = tar xf $in && touch $out
rule scantar
  command = printf "ninja_dyndep_version = 1\nbuild %s | %s: dyndep\n  restat = true\n" "$stamp" $
  "$$(tar tf $in | tr "\n" " ")" >$out.tmp && mv $out.tmp $out
rule cat
  command = cat $in >$out.tmp && mv $out.tmp $out
build foo.tar.dd: scantar foo.tar
  stamp = foo.tar.stamp
build foo.tar.stamp: untar foo.tar || foo.tar.dd
  dyndep = foo.tar.dd

samurai$ tar tf test/foo.tar
foo
bar

samurai$ ./samu -C test/ -d explain
samu: explain foo.tar.dd: missing
samu: explain foo.tar.stamp: missing
[1/2] printf "ninja_dyndep_version = 1\nbuild %s | %s: dyndep\n  restat = true\n" "foo.tar.stamp" "$(tar tf foo.tar | tr "\n" " ")" >foo.tar.dd.tmp && mv foo.tar.dd.tmp foo.tar.dd
samu: loading dyndep file: 'foo.tar.dd'
[2/2] tar xf foo.tar && touch foo.tar.stamp

samurai$ ./samu -C test/ -d explain
samu: loading dyndep file: 'foo.tar.dd'
samu: nothing to do

samurai$ rm test/foo
samurai$ ./samu -C test/ -d explain
samu: loading dyndep file: 'foo.tar.dd'
samu: explain foo: missing
samu: explain foo.tar.stamp: output of generating action is dirty
samu: explain bar: output of generating action is dirty
[1/1] tar xf foo.tar && touch foo.tar.stamp

samurai$ ./samu -C test/ -d explain -t clean
samu: loading dyndep file: 'foo.tar.dd'
remove foo.tar.stamp
remove foo
remove bar
remove foo.tar.dd

[Duncaen]

One big open issue is that it currently does not recompute the newly added dependencies when they are loaded after the dyndep node is done.

I'm not really sure how to add this without maybe splitting out parts of the buildadd function into i.e. buildcheckdeps.

I'm also not sure if the log change to always add nodes is good.

[michaelforney]

Thank you so much for working on this! This is an important feature for samurai to keep up with current ninja, and I'm not sure when I would've gotten around to implementing it.

My first impression after skimming these changes is that this looks great. I really appreciate you taking the care to match the existing style and design of samurai. I haven't read up on the details of dyndeps yet, so I'll do that and get back to you with a more thorough review.

[michaelforney]

I think this should just take the file path directly.

[Duncaen]

Ok nice, I opened the PR early to get some initial feedback and to avoid duplicating work.

I'm going to fix some outstanding issues:

• recompute nblock/nprune after loading the dyndep.
• some cases like -t clean should be able to delete broken dyndep files and not call fatal if the file can't be parsed.
• write down some details not clear form the manual.
• come up with more extensive tests and maybe use some ninja files on github for testing.
• probably a few more after more testing

[Duncaen]

• some cases like -t clean should be able to delete broken dyndep files and not call fatal if the file can't be parsed.

This would either require to add error return values and a lot of ifs to the current scanner which makes everything more complicated.
Would using setjmp be acceptable here so that instead of calling fatal directly, it could be handled whether it should be fatal or not by setting a jump buffer.

Current issues:

• ntotal is not updated correctly.
• need to rewrite checkversion to compare the numbers: comparing 1.5.0 to 1.10.0 fails.
• cmake tests for cleandead, recompact and restat tools when building fortran modules with dyndep support; while testing I've just added stubs that are always successful.

[michaelforney]

• some cases like -t clean should be able to delete broken dyndep files and not call fatal if the file can't be parsed.

This would either require to add error return values and a lot of ifs to the current scanner which makes everything more complicated.
Would using setjmp be acceptable here so that instead of calling fatal directly, it could be handled whether it should be fatal or not by setting a jump buffer.

Hmm... yes, I see the problem.

I think I'd like to make the scanner be able to report errors instead of using setjmp. Yes, it would add a bit of complexity to the parser, but I don't think it would be too bad.

Current issues:

* `ntotal` is not updated correctly.

* need to rewrite `checkversion` to compare the numbers: comparing `1.5.0` to `1.10.0` fails.

Looks like these two are resolved now, is that right?

* cmake tests for `cleandead`, `recompact` and `restat` tools when building fortran modules with dyndep support; while testing I've just added stubs that are always successful.

Do you know why it needs those subtools?

[michaelforney]

Thanks for factoring this into smaller changes. It makes it a lot easier to review.

But, could you expand on the commit descriptions a bit? Explaining the problems it solves, or how it will be used in future commit, etc.

In particular, I don't quite understand "build: edgedone: prune all output nodes if one is being pruned" and "log: create nodes if they don't exist so dyndeps can use logmtime".

The pruning stuff is probably the trickiest part of samurai, so I want to make sure we don't accidentally introduce any regressions.

[michaelforney]

I wonder if we could reuse even more code here by passing input and output nodes as parameters to a helper function

[michaelforney]

Could you explain why we would otherwise queue edges multiple times with dyndeps?

[michaelforney]

I think this might break manifest rebuilds. We currently use the dirty flag to check if we actually needed to rebuild it (see #31).

[michaelforney]

Just checking, does ninja treat set-but-empty generator and restat as false?

[orbea]

@Duncaen & @michaelforney Any news on the status of this feature?