Patch 2414 fix/generalize propResolutionsOrderIndep... (and 2 more)

Title fix/generalize propResolutionsOrderIndep... (and 2 more)
Superseder Nosy List bfrk
Related Issues
Status needs-review Assigned To

Created on 2024-06-17.17:07:42 by bfrk, last changed 2024-06-21.19:18:24 by bfrk.

File name Status Uploaded Type Edit Remove
fix-another-bug-in-repopatchv3-conflict-resolution.dpatch bfrk, 2024-06-21.08:01:29 application/x-darcs-patch
fix_generalize-propresolutionsorderindependent.dpatch bfrk, 2024-06-17.17:07:41 application/x-darcs-patch
minor-changes-to-a-regression-test-for-issue2727.dpatch bfrk, 2024-06-21.19:18:23 application/x-darcs-patch
patch-preview.txt bfrk, 2024-06-17.17:07:41 text/x-darcs-patch
patch-preview.txt bfrk, 2024-06-21.08:01:29 text/x-darcs-patch
patch-preview.txt bfrk, 2024-06-21.19:18:22 text/x-darcs-patch
See mailing list archives for discussion on individual patches.
msg24020 (view) Author: bfrk Date: 2024-06-17.17:07:41
This is the first half of fixing issue2727. It fixes only errors in the
underlying RepoPatchV3 conflict resolution, but the generalized test for
propResolutionsOrderIndependent will also catch more bugs for Named patches.

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

patch d0757eecafa1cc633587bbe572dae8c412dfca26
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jun 16 18:34:08 CEST 2024
  * fix/generalize propResolutionsOrderIndependent

  It now tests the property (as it should) on a pair of mergeable RLs, whereas
  before we fixed the context (the first sequence) to be empty.

  The generator for the pair requires a wrapper type so we can generate a
  split point for the sequence i.e. a random number between 0 and the length
  of the sequence. This wrapper then needs various instances which are mostly
  trivial but nevertheless tedious to implement.

  Note that the simpler method of directly generating a pair of
  MergeableSequences is /not/ general enough, and indeed does not catch the
  kind of errors we want to guard against. This is because it confines
  conflicts to each sequence, whereas we need them to spread over both sides
  of the split.

patch d38f51fd0604df5b79bbd943eec6137a3d5cc52e
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jun 17 10:15:46 CEST 2024
  * bugfix in Darcs.Patch.V3.Resolution.findComponents

  The error causes propResolutionsOrderIndependent to fail and also generates
  more conflict markup than wanted, in in the case of a non-empty context. the
  point is that we should /not/ add conflicts contained in the patch under
  consideration to our todo set if we have exhausted the patches in the
  trailing "interesting" segment and the patch under consideration is in the
  "context". We regard those as uninteresting, either because they are not
  "new" conflicts e.g. when pulling, or because we consider conflicts
  contained in them as already resolved e.g. due to explicit dependencies.

patch 5e21c4bdb15498be6227f06a9fdf8723b3736be2
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jun 17 18:42:07 CEST 2024
  * two regression tests related to issue2727

  These contain no --add-deps and thus guard against errors in the RepoPatch
  implementations of resolveConflicts. One of them fails for darcs-1 patches
  and is therefore skipped (I can't be bothered to even look at the
  RepoPatchV1 code anymore, much less make any changes to it).
msg24021 (view) Author: bfrk Date: 2024-06-17.17:36:08
This particular property succeeds for me with with -q=300000 (only naked RepoPatchV3 yet, not for Named, working on that).
msg24026 (view) Author: bfrk Date: 2024-06-21.08:01:29
This is still not about the (multiple) problems in the implementation for
Named patches. (I do have fixes for those too by now, or so I believe, but
have to clean up my patches first. Will send them to a separate ticket.)

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

patch eb177d01ccc59fb7ccb175a82908ba26cbfebaaa
Author: Ben Franksen <ben.franksen@online.de>
Date:   Thu Jun 20 00:24:33 CEST 2024
  * fix another bug in RepoPatchV3 conflict resolution

  The error this patch fixes should have been obvious, given the motivating
  example scenarios discussed in the doc comment to 'findComponents'. Indeed,
  I concluded there that "it would be wrong to first join conflict sets and
  then use those to join components". (Note that this remark is about
  /resolved/ conflicts.) Yet, I failed to consistently follow that reasoning
  in the code, and at one point lumped together the patch and the ones it
  conflicts with into a single set. The correct view is that a single "direct
  conflict" is an edge in the conflict graph and the corresponding "conflict
  set" always contains exactly two patches.

patch e57dd34d1fdc3faae4e47c53e065c5839056eb7a
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jun 21 08:16:30 CEST 2024
  * regression test for issue2727, regarding RepoPatchV3 resolution

  It originated as a failing QC test for Named RepoPatchV3 with -q=100000. As
  with the previous regression tests for that issue, it turned out that
  explicit dependencies play no role at all here and the inputs to the
  instance for RepoPatchV3 were in fact identical up to commutation.
msg24029 (view) Author: bfrk Date: 2024-06-21.19:18:23
1 patch for repository https://darcs.net/screened:

patch ebf5fd64aa48c264d60f14967aa3193850c802e1
Author: Ben Franksen <ben.franksen@online.de>
Date:   Fri Jun 21 20:15:48 CEST 2024
  * minor changes to a regression test for issue2727
Date User Action Args
2024-06-17 17:07:42bfrkcreate
2024-06-17 17:32:22bfrksetstatus: needs-screening -> needs-review
2024-06-17 17:36:09bfrksetmessages: + msg24021
2024-06-21 08:01:29bfrksetfiles: + patch-preview.txt, fix-another-bug-in-repopatchv3-conflict-resolution.dpatch
messages: + msg24026
2024-06-21 19:18:24bfrksetfiles: + patch-preview.txt, minor-changes-to-a-regression-test-for-issue2727.dpatch
messages: + msg24029