darcs

Patch 2003 use unwind in rebase suspend

Title use unwind in rebase suspend
Superseder Nosy List ganesh
Related Issues
Status accepted Assigned To
Milestone

Created on 2020-02-23.22:20:11 by ganesh, last changed 2020-06-20.07:19:51 by bf.

Files
File name Status Uploaded Type Edit Remove
pEpkey.asc bf, 2020-02-24.08:57:11 application/pgp-keys
pEpkey.asc bf, 2020-02-24.13:09:08 application/pgp-keys
patch-preview.txt ganesh, 2020-02-23.22:20:10 text/x-darcs-patch
patch-preview.txt ganesh, 2020-03-05.06:58:53 text/x-darcs-patch
resolve-issue2634_-use-unwind-to-suspend-patches.dpatch ganesh, 2020-03-05.06:58:53 application/x-darcs-patch
tests_-always-use-prim-patches-for-generating_shrinking.dpatch dead ganesh, 2020-02-23.22:20:10 application/x-darcs-patch
unnamed ganesh, 2020-02-23.22:20:10 text/plain
unnamed ganesh, 2020-03-05.06:58:53 text/plain
See mailing list archives for discussion on individual patches.
Messages
msg21894 (view) Author: ganesh Date: 2020-02-23.22:20:10
The final patch in this bundle, building on patch1979, patch2001
and patch2002, shows that we can now suspend conflicts correctly
with unwind.

I don't want to screen this yet as it doesn't yet deal with
failed unwinds (albeit that they should be very rare).

10 patches for repository darcs-unstable@darcs.net:screened:

patch b043d8bcbbe106224a90148480f9735ab0bfefc3
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 10:58:34 GMT 2020
  * tests: always use prim patches for generating/shrinking
  
  We already didn't try to generate conflicted patches, and
  even shrinking unconflicted patches is actually unsound if
  there might be a conflict later in a sequence.
  
  Instead of needing partial functions on repo patches, it's
  better to express this invariant in the types by only storing
  prim patches, and generating the repo patches on the fly
  when actually using the test cases.
  

patch ba02c674facfc9dfc826adc1eb8509e07fe010c0
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 12:06:59 GMT 2020
  * tests: introduce infrastructure for merge checking
  
  Because V1 and V2 patches are known to be buggy, we
  sometimes need to exclude buggy merges when using them
  to test other properties.

patch b55922b9b19e27d3d42cf4c500d2c9e17e24e9d5
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 18:44:20 GMT 2020
  * tests: export V1Model(..)

patch 3dc6fb02adcabbba69bc02decb322fec09bab80a
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 18:48:06 GMT 2020
  * tests: move patch properties into D.T.P.P.Generic
  
  This means they can be used from D.T.P.RepoPatchV1
  

patch 6f7b4b6bd8785c0f4df3bf587c1ac1df1149d576
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 18:51:48 GMT 2020
  * tests: remove unused withStateShrinking

patch 4b0e6e70e1c73391b5ad787c88b47cf330ff6a29
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 18:52:48 GMT 2020
  * tests: generalise hasPrimConstruct, add usesV1Model

patch c26d508c17fc6157325cea574118adace10ba083
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 18:52:54 GMT 2020
  * tests: introduce method to identify V1 patch type

patch fa5773462abc1d2d4fc79429f43afe62574910e2
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 18:54:42 GMT 2020
  * tests: add withAllSequenceItems

patch ed48a37300e45d31ff9b98389340e59b20f8d725
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 18:57:30 GMT 2020
  * introduce concept of unwinding a conflict
  
  This provides a generic way to get at the underlying primitive
  patches in the conflict.

patch 2efa4a004c8edd7c49757de7a2f5ab6313f928d6
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 22:15:09 GMT 2020
  * resolve issue2634: use unwind to suspend patches
Attachments
msg21901 (view) Author: bf Date: 2020-02-24.08:57:11
> I don't want to screen this yet as it doesn't yet deal with
> failed unwinds (albeit that they should be very rare).

In principle, calling error would be okay here as I believe this
actually /is/ a bug. It's just that we know it's there but can't/won't
do anything about it. Perhaps we should throw a dedicated Exception
instead and explain the dilemma...
Attachments
msg21906 (view) Author: ganesh Date: 2020-02-24.12:18:07
On 24/02/2020 08:57, Ben Franksen wrote:
> 
> Ben Franksen <ben.franksen@online.de> added the comment:
> 
>> I don't want to screen this yet as it doesn't yet deal with
>> failed unwinds (albeit that they should be very rare).
> 
> In principle, calling error would be okay here as I believe this
> actually /is/ a bug. It's just that we know it's there but can't/won't
> do anything about it. Perhaps we should throw a dedicated Exception
> instead and explain the dilemma...

We discussed it here:

https://lists.osuosl.org/pipermail/darcs-devel/2020-January/020668.html

My current plan is just to inject the stuck fixups into the ToEdit
patch. The alternative of breaking the patch into pieces feels quite messy.
msg21909 (view) Author: bf Date: 2020-02-24.13:09:08
>> In principle, calling error would be okay here as I believe this
>> actually /is/ a bug. It's just that we know it's there but can't/won't
>> do anything about it. Perhaps we should throw a dedicated Exception
>> instead and explain the dilemma...
> 
> We discussed it here:
> 
> https://lists.osuosl.org/pipermail/darcs-devel/2020-January/020668.html

Yes, thanks for the reminder.

> My current plan is just to inject the stuck fixups into the ToEdit
> patch. The alternative of breaking the patch into pieces feels quite messy.

Fine. I was merely suggesting that a clean failure (i.e. just abort the
whole operation) would be a viable option, too, one that doesn't require
any special handling of this particular V1 bug.
Attachments
msg21913 (view) Author: ganesh Date: 2020-02-24.18:48:54
On 24/02/2020 13:09, Ben Franksen wrote:

>> My current plan is just to inject the stuck fixups into the ToEdit
>> patch. The alternative of breaking the patch into pieces feels quite messy.
> 
> Fine. I was merely suggesting that a clean failure (i.e. just abort the
> whole operation) would be a viable option, too, one that doesn't require
> any special handling of this particular V1 bug.

I've actually failed to write a shell test for this situation so far. I
tried to transcribe the example I found in D.T.P.E.Unwind, but on the
command-line it ends up failing first with an error from newUr or
reconcileUnwindings. So it seems quite likely that no-one will run into
it in practice. And if they do they could amend the conflict to turn it
into its effect, and then suspend.

So just aborting seems like a good option after all.

For the record, the cleanest idea I had for implementing error recovery
was to introduce a type class like PrimUnwindRecovery, with a method
something like

stuckPatch
  :: PrimUnwindRecovery prim => String -> prim wA wB -> prim wA wB

The default implementation would just be to call error with the first
argument. For rebase suspend we would arrange for prim to be some
wrapped type that would embed both the error message as a warning and
the actual prim in the result. It could then be post-processed to
generate the warning. But anyway, let's just do without it for now.

I'll screen this one as well soon unless there are objections.
msg21961 (view) Author: ganesh Date: 2020-03-05.06:58:53
Now that the dependencies are in screened, this is a clean
bundle containing the actual change to use unwind, plus
a couple of cleanups on top.

3 patches for repository /home/ganesh/darcs/screened:

patch 2efa4a004c8edd7c49757de7a2f5ab6313f928d6
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Sun Feb 23 22:15:09 GMT 2020
  * resolve issue2634: use unwind to suspend patches

patch 8034d885705bbdf4359b5b74bde149c81ce3feef
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Tue Mar  3 23:09:09 GMT 2020
  * move addToEditsToRebase to D.P.R.Suspended
  
  It should probably have been there to begin with

patch 9182fd25b5a0cb8bec08ed9145b7a7f992597425
Author: Ganesh Sittampalam <ganesh@earth.li>
Date:   Tue Mar  3 23:13:16 GMT 2020
  * improve abstraction of adding ToEdits
Attachments
History
Date User Action Args
2020-02-23 22:20:11ganeshcreate
2020-02-23 22:59:23ganeshsetstatus: needs-screening -> followup-in-progress
2020-02-24 08:57:11bfsetfiles: + pEpkey.asc
messages: + msg21901
2020-02-24 12:18:07ganeshsetmessages: + msg21906
2020-02-24 13:09:08bfsetfiles: + pEpkey.asc
messages: + msg21909
2020-02-24 18:48:55ganeshsetmessages: + msg21913
2020-02-24 18:49:06ganeshsetstatus: followup-in-progress -> needs-screening
2020-02-25 23:07:47ganeshsetstatus: needs-screening -> needs-review
2020-03-05 06:58:54ganeshsetfiles: + patch-preview.txt, resolve-issue2634_-use-unwind-to-suspend-patches.dpatch, unnamed
messages: + msg21961
2020-06-20 07:19:51bfsetstatus: needs-review -> accepted