Avoid --keep-going message

[nomeata]

Hi Neil,

my build system usually uses --keep-going, because I expect there to be a number of build failures (finding and fixing them is the main purpose of the tool).

The message printed upon the first error is confusing to the user. Can it somehow be disabled?

Here is an example:

import Development.Shake
main = shakeArgs shakeOptions $ do
    want ["foo1","foo2"]
    "foo1" %> \out -> do
        need ["bar1"]
        putNormal out
    "foo2" %> \out -> do
        need ["bar2"]
        putNormal out
    "bar1" %> \out -> do
        writeFileLines out []
        putNormal out
    "bar2" %> \out -> do
        writeFileLines out []
        putNormal out

Currently, it prints:

$ ./staunch-bug --keep-going
bar2
foo2
Error when running Shake build system:
* foo2
Error, rule "foo2" failed to build file:
  foo2
Continuing due to staunch mode, this error will be repeated later
bar1
foo1
Error when running Shake build system:
* foo2
Error, rule "foo2" failed to build file:
  foo2

Note that the error on foo2 is reported twice, but no other error is.

I would rather like to see either

$ ./staunch-bug --keep-going
bar2
foo2
bar1
foo1

(and I would extend my rules to give better error messages), or

$ ./staunch-bug --keep-going
bar2
foo2
Error when running Shake build system:
* foo2
Error, rule "foo2" failed to build file:
  foo2
bar1
foo1
Error when running Shake build system:
* foo1
Error, rule "foo1" failed to build file:
  foo1

[ndmitchell]

The idea behind giving the first error message is that you want to see it as soon as you can. The idea behind giving it again at the end is because you might have missed it if you have done a lot of things since then. So is showing the same error twice problematic on its own? I could certainly imagine that when building in staunch I show all error messages, that seems consistent and useful, but you'd still get the first error twice - is that a problem?

[nomeata]

Given that I use Shake as a library, for a use case where failures are not that uncommon (the purpose of the tool is to find them), I would rather have shake not produce any of these, and rather handle the errors in my Action.

And if Shakes gives an error message, I think it should print all of them. With multiple threads, it is confusing if one run reports that message, and another run right afterwards another message.

[ndmitchell]

Maybe I could add a display error function to the ShakeOptions, then you could override how you want the options to appear.

[nomeata]

Possibly, although shake’s displaying of errors is fine, it really is the non-uniform handling of the first error compared to the others that is strange.

Also, since I report failures with fail with a helpful message (I could not find an explicit failAction :: String -> Action () method), but with --keep-going the message to fail would be swallowed, so I changed it to putNormal followed by fail "".

How about this: In --keep-going, all errors are reported in the usual manner, and without mention that --keep-going is enabled (the users sees that you keep going, and probably has specified it explicit). At the end, instead of repeating one message, which in the presence of -j4 is actually random, print a summary like # There were 23 failures.

[ndmitchell]

So displaying all errors, rather than just the first, seems like a good idea. And including how many errors there have already been seems sensible. I'll do both of those.

To avoid the final display, you can actually already do that. The final display is merely an uncaught exception which I rethrow, so you can catch that, and throw it away - Shake isn't really displaying it anyway. That way, you can implemented the There were 23 errors instead. I do find it useful to reprint the first error, as often the user missed it in the scrollback.

I still like the idea of saying why you are still going even in the presence of exceptions. My users at $WORK certainly do not know they specified --keep-going, since there's a lot of cargo culted flags flying around. Let's keep that in for now, and see how the other changes make it look.

[ndmitchell]

BTW, for failAction that was merely a proposed hypothetical function I could introduce.

[ndmitchell]

So testing with two phony rules, both of which fail, gives me:

> :main errors fail1 fail2 --keep-going
## BUILD errors fail1 fail2 --keep-going
Error when running Shake build system:
* fail2
die2
Continuing due to staunch mode
Error when running Shake build system:
* fail1
die1
Continuing due to staunch mode
Error when running Shake build system:
* fail2
die2

So it prints all the failures (both die1 and die2), then rethrows the first one as an exception, which you could catch/hide if you wanted. With a try around shakeArgs you can get almost everything you are after. Or are you only seeing the first error? Note I changed that in shake-0.15.3 with bug #254.

Thinking further, I wonder if failure and staunch mode is the right thing for your project. Your goal seems to be to determine the success/failure of a set of thingies. In that case, isn't failure a "good" result? Then you could just write a file containing the success/failure for each result, and afterwards summarize the failures. That way failure would be a real error in the build system (you failed at a stage that you don't think should fail, e.g. downloading) rather than the default.

[nomeata]

Thanks!

It’s not that I want these build failure, it is not a “good” result. It just happens to be something that happens often enough so that I want to see all of them instead of one of them at a time. But I still would want, for example, the build system not even attempt to build stuff that depends on a failed artefact.

[ndmitchell]

OK, given your description, your current approach seems quite reasonable - I guess the dependency fail means you stop building is the key thing that means you should be using exceptions here.

[ndmitchell]

I did something at work, and didn't see multiple error messages, even though I really wanted them - so maybe my multiple errors thing isn't as robust as it could be, or fails in some more interesting/real circumstances.

[ndmitchell]

So adding:

phony "fail12" $ need ["fail1","fail2"]

Now I only get on error message. I guess that's what you are seeing.