Patch 2304 fix a (serious) bug in permutationsRL (and 1 more)

Title fix a (serious) bug in permutationsRL (and 1 more)
Superseder Nosy List bfrk
Related Issues
Status accepted Assigned To

Created on 2023-06-05.19:14:57 by bfrk, last changed 2023-07-29.10:54:36 by ganesh.

File name Status Uploaded Type Edit Remove
fix-a-_serious_-bug-in-permutationsrl.dpatch bfrk, 2023-06-05.19:14:53 application/x-darcs-patch
patch-preview.txt bfrk, 2023-06-05.19:14:52 text/x-darcs-patch
See mailing list archives for discussion on individual patches.
msg23314 (view) Author: bfrk Date: 2023-06-05.19:14:53
Will self-accept. Note that propResolutionsOrderIndependent now fails with
larger values for -q (you typically need -q=10000 to find the more subtle
bugs). I will send a fix separately, since it depends on semantic changes
that may require discussion.

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

patch f77ba444a977df49684edbac69f473caf6876757
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Jun  3 20:08:45 CEST 2023
  * fix a (serious) bug in permutationsRL

  It always returned an empty list. Thankfully it wasn't used in darcs proper.
  However, two RepoPatch tests are affected (propResolutionsOrderIndependent,
  propResolutionsDontConflict) with the result that they never tested anything!

patch 946395e709aa45208f103f23dcc433e81461bd52
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sat Jun  3 21:31:20 CEST 2023
  * fix propConsistentReorderings and propResolutionsOrderIndependent

  After fixing permutationsRL it became obvious that propConsistentReorderings
  is completely wrong: it succeeded when it should have failed and vice versa.
  To guard against regressions, we now call error if permutationsRL returns an
  empty list and reject the test case if it is a singleton.

  In propResolutionsOrderIndependent we now compare the resolutions before
  mangling. This more general and also more reliable, since mangling can fail
  if not all prims involved in the resolution are hunks. Note that we must use
  order-independent equality for the alternatives.
msg23403 (view) Author: ganesh Date: 2023-06-24.20:20:19
Follow up is http://bugs.darcs.net/patch2305, right?

I'm somewhat inclined to merge the two together to avoid temporarily
leaving us with known failing tests, even if they are very unlikely to
fail with the default -q.
msg23441 (view) Author: bfrk Date: 2023-06-27.07:02:26
Yes (should have left a note here). With "merge" you mean not accept 
this one until we are done with patch2305? Or should I re-send the 
latter to this ticket?
msg23465 (view) Author: ganesh Date: 2023-07-01.15:20:29
> With "merge" you mean not accept  this one until we are done with 
> patch2305?

Yes, this. If we just leave it as review-in-progress for now it'll
be a signal (at least to me) not to push it to reviewed.
msg23473 (view) Author: bfrk Date: 2023-07-01.19:46:02
I'm happy with that.
Date User Action Args
2023-06-05 19:14:57bfrkcreate
2023-06-05 20:50:54bfrksetstatus: needs-screening -> accepted-pending-tests
2023-06-24 20:20:19ganeshsetstatus: accepted-pending-tests -> review-in-progress
messages: + msg23403
2023-06-27 07:02:26bfrksetmessages: + msg23441
2023-07-01 15:20:29ganeshsetmessages: + msg23465
2023-07-01 19:46:02bfrksetmessages: + msg23473
2023-07-16 20:15:59ganeshsetstatus: review-in-progress -> accepted-pending-tests
2023-07-29 10:54:36ganeshsetstatus: accepted-pending-tests -> accepted