Hi,

these 3 patches solve issue332 (ask to record if test fails).  Solving
this issue has required more work than expected because failing tests
used not to be signaled back to Record.lhs, throwing a generic
ExitFailure instead.

The first patch is used to allow accessing return values through
withSignalsBlocked and withGutsOf.  IMHO, it should be applied
regardless of what happens to the next two patches, but please review it
carefuly in case I got something wrong with the block/unblock couple.

The second patch gives more control on tests in finalizeRepositoryChanges.
Because of issue1988, Florent and I decided on IRC that the best
solution for now was to hide the changes behind an exception wrapper.
Therefore, this patch currently preserves backward-compatibility and
does not expose any new functionnality outside of Internal.hs.

The third patch effectively solves issue332.  Since issue1988 has not
been solved yet, I decided the cleanest way to go was to export the
modified finalizedRepositoryChanges' and use it in Record.lhs only.  The
long-term solution would be to expose the new version and use it
everywhere, but I do not feel confident enough to do that until
issue1988 is solved.  (I have done it for fun in my private repository,
but you really do not want to rely on something based on "grep" to
ensure no case has been forgotten).

I am not sure if the third patch should be included in darcs right now.
And I am not sure if we should keep the compatibility wrapper of the
second patch in the long-term (providing both interfaces) or completely
discard the old one.  But anyway, issue1988 should be fixed first.

This is my first substantial patch to darcs.  I would be glad to get any
feedback on style/effiency/etc.

Best,
              Gabriel

3 patches for repository http://darcs.net:

Thu Nov  4 12:45:45 CET 2010  Gabriel Kerneis <kerneis@pps.jussieu.fr>
  * Generalize withSignalsBlocked and withGutsOf

Thu Nov  4 13:49:38 CET 2010  Gabriel Kerneis <kerneis@pps.jussieu.fr>
  * Allow more control on tests run in finalizeRepositoryChanges
  
  Before this patch, tests run in finalizeRepositoryChanges either passed
  or killed darcs.  This blocked the resolution of issue332.
  
  This patch has two effects:
  - it adds a Bool parameter to finalizeRepositoryChanges to allow
    finalizing without running the tests (even if Test is present in the
    options of the repository),
  - it changes the return type of finalizeRepositoryChanges into IO(Either
    Int ()), where Int is the return code of the test in case it fails.
    The types of testAny, testRecorded and testTentative change
    accordingly.
  
  Note that currently, issue1988 prevents us from using IO(Either) safely.
  As a consequence, those changes are not yet exported. Backward-
  compatibility is preserved through the use of exceptionWrapper.  Once
  issue1988 is fixed, unwrapped finalizeRepositoryChanges' should replace
  finalizeRepositoryChanges.
  

Thu Nov  4 15:01:48 CET 2010  Gabriel Kerneis <kerneis@pps.jussieu.fr>
  * resolve issue332: ask to record if tests fail
  
  This patch is not as clean as possible, because of issue1988: it exports
  finalizeRepositoryChanges' whereas, in an ideal world, it should discard
  finalizeRepositoryChanges instead.  Since this cannot be done safely at
  the moment (ie. without the fear to miss some key compiler warnings), we
  shall live with this half-backed solution in the interim.

Attachments

• generalize-withsignalsblocked-and-withgutsof.dpatch
• unnamed

Note that the third patch breaks cabal test:
'issue1620-record-lies-about-leaving-logfile.sh' fails.

The attached patch fixes this.

Attachments

• fix-test-issue1620.patch

On Thu, Nov 04, 2010 at 20:20:17 +0100, Florent Becker wrote:
> Please send such amendments either as a new darcs recorded patch, or
> just resend your old patch amend-recorded.

Here it is.

Best,
              Gabriel

1 patch for repository http://darcs.net:

Thu Nov  4 21:13:09 CET 2010  Gabriel Kerneis <kerneis@pps.jussieu.fr>
  * resolve issue332: ask to record if tests fail
  
  This patch is not as clean as possible, because of issue1988: it exports
  finalizeRepositoryChanges' whereas, in an ideal world, it should discard
  finalizeRepositoryChanges instead.  Since this cannot be done safely at
  the moment (ie. without the fear to miss some key compiler warnings), we
  shall live with this half-backed solution in the interim.

Attachments

• resolve-issue332_-ask-to-record-if-tests-fail.dpatch
• unnamed

Do not accept this patch in its current form please, it is being
discussed on IRC/ML to find a cleaner solution.

A new version of this patch has been sent separately (based on a
completely different idea).