darcs

Patch 2417 resolve issue2727: conflict resolution s... (and 3 more)

Title resolve issue2727: conflict resolution s... (and 3 more)
Superseder Nosy List bfrk
Related Issues
Status needs-review Assigned To
Milestone

Created on 2024-06-21.19:37:00 by bfrk, last changed 2024-06-27.19:12:52 by bfrk.

Files
File name Status Uploaded Type Edit Remove
_mostly_-fix-the-detrimental-side_effects-of-the-fix-for-issue2727.dpatch bfrk, 2024-06-27.19:12:51 application/x-darcs-patch
patch-preview.txt bfrk, 2024-06-21.19:36:58 text/x-darcs-patch
patch-preview.txt bfrk, 2024-06-27.19:12:50 text/x-darcs-patch
resolve-issue2727_-conflict-resolution-should-be-invariant-under-reordering.dpatch bfrk, 2024-06-21.19:36:58 application/x-darcs-patch
See mailing list archives for discussion on individual patches.
Messages
msg24030 (view) Author: bfrk Date: 2024-06-21.19:36:58
Finally! This took me *much* longer to finish than I expected.

4 patches for repository https://darcs.net/screened:

patch c4a890da8b461c287e924be82d1dea26eeac35a4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jun 21 20:23:17 CEST 2024
  * resolve issue2727: conflict resolution should be invariant under reordering

  This is almost, though not quite, a re-write of 'resolveConflicts' for Named
  patches. There were many things wrong with the old version. For instance,
  reliably determining the patches that a Named patch conflicts with is more
  involved than just a call to findConflicting. Also, lessons learned from the
  latest bug fixes to resolveConflicts for RepoPatchV3 have been applied here
  as well. Particularly, the question of which conflicts have been resolved at
  the Named patch layer via explicit dependencies can and should be answered
  for direct conflicts between two patches. Transitive conflicts do /not/ play
  any role here, they are only important to finally generate the conflicting
  alternatives, and that is (as before) done at the RepoPatch level. For
  clarity, I decided to split the algorithm into two passes, one which gathers
  the necessary information (conflicts, dependencies), and another one to
  separate patch contents into resolved (by explicit dependencies) and
  unresolved (by same). This makes it more verbose overall, and perhaps
  slightly less efficient. But it should be clearer now what is happening and
  why. It is also much easier to debug in this form, since we can trace
  intermediate results to locate where things start to go wrong.

patch 335821a41013031fa92037b583a91759cc8148bf
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jun 21 21:02:31 CEST 2024
  * a few local renamings in Darcs.Patch.Named for better clarity

patch 9277a3c136a7cfb4bf31acbaca68df36a7846db2
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jun 21 20:36:24 CEST 2024
  * rollback "disable propResolutionsOrderIndependent for Named patches"

  The property no longer fails for Named patches.

patch 5ef23cf7405e66b030efaa7fc773f1562b9f0334
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jun 21 20:38:16 CEST 2024
  * a number of new regression tests for issue2727

  These all came up during my attempts at fixing issue2727 and are based on
  failing QC test cases, which I manually reconstructed. Some of them (notably
  4, 5, 8, and 11) pass even before applying the final fix for issue2727 (but
  after the fixes for RepoPatchV3); they represent bugs I introduced on my way
  toward the final solution. I decided to include them anyway, in case we ever
  have to touch that code again.
Attachments
msg24045 (view) Author: bfrk Date: 2024-06-26.13:54:51
Unfortunately, after the fix for issue2727 the algorithm now has a higher probability to run into darcs-1 and darcs-2 bugs. This is why a number of tests now fail for those patch formats. In all cases, darcs crashes in function findConflicting. The probable reason is that we now call this function not only for conflicted patches in the original patch order, but again for each patch that this commutes out. I am not yet sure how to solve this problem.
msg24046 (view) Author: bfrk Date: 2024-06-26.13:57:57
Once again with proper formatting:

Unfortunately, after the fix for issue2727 the algorithm now has a 
higher probability to run into darcs-1 and darcs-2 bugs. This is why a 
number of tests now fail for those patch formats. In all cases, darcs 
crashes in function findConflicting. The probable reason is that we now 
call this function not only for conflicted patches in the original patch 
order, but again for each patch that this commutes out. I am not yet 
sure how to solve this problem.
msg24047 (view) Author: bfrk Date: 2024-06-27.19:12:51
1 patch for repository https://darcs.net/screened:

patch 66f91157f457dd80d754ed5f9775b950be9c0c57
Author: Ben Franksen <ben.franksen@online.de>
Date:   Wed Jun 26 16:43:39 CEST 2024
  * (mostly) fix the detrimental side-effects of the fix for issue2727

  This re-implements onlyRealConflicts by counting and comparing conflicts
  before and after commuting conflicting Named patches back to before the
  patch in question. We also handle the case where this commute fails (due to
  bugs in V1 or V2): instead of calling error i.e. crashing darcs, we prefer
  to create potentially inconsistent resolutions by assuming that the rest of
  the conflicting patches are real conflicts.

  Apart from the tests for issue2727 there remains just one failing test,
  where we run into the infamous "Non-exhaustive patterns in Just a2'o" bug in
  RepoPatchV2, namely tests/conflict-fight.sh. Skipping that one for darcs-2
  seems to be the only solution. Some of the tests for issue2727 that were
  previously skipped (for darcs-1 or darcs-2) now also pass for those formats.
Attachments
History
Date User Action Args
2024-06-21 19:37:00bfrkcreate
2024-06-21 19:38:00bfrksetstatus: needs-screening -> needs-review
2024-06-26 13:54:53bfrksetmessages: + msg24045
2024-06-26 13:57:58bfrksetmessages: + msg24046
2024-06-27 19:12:52bfrksetfiles: + patch-preview.txt, _mostly_-fix-the-detrimental-side_effects-of-the-fix-for-issue2727.dpatch
messages: + msg24047