Replace promote actions by diff actions + promote command

[ghost]

Following user feedback, this PR changes the way promotion works. It removes the promote and promote-if actions as well as the --promote flags. It adds the following:

Diff actions

This PR adds the following two actions:

• (diff a b): print a diff and fail if the contents of a and b are different
• (diff? a b): same thing but does nothing if a or b doesn't exist

Promote command

jbuilder promote is added and it does the following: for every (diff a b) or (diff? a b) action that failed in the previous run such that a is a source file and b is a generated file, promote b to a .

A command line flag --auto-promote is also added, which is the same as running jbuilder promote manually after jbuilder has exited.

Usage, for "regular" tests

(rule
 (with-stdout-to data.out (run ./test.exe)))

(alias
 ((name runtest)
  (action (diff data.expected data.out))))

Usage, for "quine" tests (generating .corrected files)

(alias
 ((name runtest)
  (action (progn
           (run ./texe.exe)
           (diff? test.ml test.ml.corrected)))))

Other changes

I updated all the tests in jbuilder to use this mechanism

[ghost]

/cc @avsm, @yminsky

[avsm]

I do like the diff? syntax over promote-if and the new behaviour looks sensible.

I'm still a bit confused about what build dependencies diff introduces. Does adding a diff clause cause any changes to build behaviour aside from promotion? It would be good to document that if so.

[rgrinberg]

Isn't this a bit dangerous? If optional is false, then it might try to read files that don't exist or were deleted.

[ghost]

That's not specific to diff.When executing an action, we always expect that all the dependencies are present.

[rgrinberg]

Please update the (include jbuild.inc) example as well. It's still using the old promote action.

[rgrinberg]

Here's another question, that's related to the feature rather than this specific PR. Suppose I'd like to check in generated .ml, .mli files from the ocamlyacc rules, how would I do that? I'd like to reuse the ocamlyacc stanza and perhaps have something like this:

(ocamlyacc (bar))

(alias
 ((name yacc)
  (deps (bar.ml bar.mli))
  (action
   (progn
    (diff bar.mli bar.mli)
    (diff bar.ml bar.ml)))))

But obviously this doesn't work since the diff stanza has to diff bar.ml in _build and in the source tree. Do I need to write my own custom ocamlyacc rule to make this work?

[ghost]

I'm still a bit confused about what build dependencies diff introduces. Does adding a diff clause cause any changes to build behaviour aside from promotion? It would be good to document that if so.

It's not specific to diff. Jbuilder always infer most dependencies and targets from the action itself. That why you can just write (rule (copy a b)). To execute (diff a b), it's clear that both a and b are needed, so jbuilder will add them as dependencies of the rule.

On the other hand, diff? does nothing when either file doesn't exist, so the files are not added as dependencies.

[ghost]

@rgrinberg, yes the case where we just want to commit a generate a file doesn't work well with this PR. We need something different for this case. I think that's ok, these are two different features in the end.

For instance, rule stanzas could have a promote field again:

(rule
 ((promote)
  (action (run ocamlyacc ...))))

or we could provide a promote stanza meaning that the files are copied from the build directory to the source tree instead of the opposite:

(ocamlyacc (bar))

(promote (bar.mly))

But we must consider the following:

• in release mode, we might want to ignore the rule and promotion to speed up things and cut dependencies
• we might want a bootstrap mode, i.e. where building the generated file requires the generated file itself. In such case we want to ignore the rule and promotion at first, and when the file is requested, we first use the version in the source tree and then trigger the rule and check that the result is the same

We should discuss all that in a different PR.

[rgrinberg]

Sure, we can discuss this later. I prefer the separate promote stanza out of those options. Because it lets you promote targets from rules that you didn't write yourself.

[ghost]

I improved a bit the code and updated the documentation

[bobot]

Is source tree the right word? a could be in the build tree and that would still work.

[ghost]

Maybe "source file" versus "generated file"?

[bobot]

a could indeed be source file even if it is not very useful. You don't like build tree?

[ghost]

I don't understand the difference. All files always end up in the build tree. What is important is whether the files are generated by an explicit rule or where copied as it from the source tree.