Patch 2207 resolve issue2682

Title resolve issue2682
Superseder Nosy List bf
Related Issues
Status review-in-progress Assigned To

Created on 2021-07-07.08:34:19 by bf, last changed 2023-03-18.20:24:10 by ganesh.

File name Status Uploaded Type Edit Remove
accept-issue2682.dpatch bf, 2021-07-07.08:34:18 application/x-darcs-patch
patch-preview.txt bf, 2021-07-07.08:34:18 text/x-darcs-patch
See mailing list archives for discussion on individual patches.
msg22905 (view) Author: bf Date: 2021-07-07.08:34:18
6 patches for repository http://darcs.net/screened:

patch f9ae44ccac2475fdcb09c7246bdcca6d2c7e46a1
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jul  5 09:45:10 CEST 2021
  * accept issue2682

patch d9f29cea4c3d0a09ec489d98c2b512f2ba671827
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Jul  6 18:03:06 CEST 2021
  * improve test for issue2682

patch c4191e928af9e5163d8dea62ffa13d3ff8f3efc2
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul  4 23:02:48 CEST 2021
  * re-add isConflicted method to class Conflict

patch 866ef8aac8f4cc67f91c0893f2ea0ea0c8134ae6
Author: Ben Franksen <ben.franksen@online.de>
Date:   Sun Jul  4 23:29:26 CEST 2021
  * add findConflicting to Darcs.Patch.Conflict

patch 8c843f0bdc7dabc0bacc61342cf6575e4e9fc391
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jul  5 13:27:51 CEST 2021
  * avoid use of the instance Named (Conflict p) for rebase unsuspend

  This is so that the fix for issue2682 does not break rebase unsuspend, but
  it also makes sense independently: when unsuspending patches we definitely
  don't want explicit dependencies to count as conflict resolutions.

patch 9411e9d8ccfb7e2c52d431c2cf2343018ec3b25b
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jul  5 09:54:17 CEST 2021
  * resolve issue2682: conflict not marked if tag pulled at the same time

  The fix is to consider a conflict as resolved only if all conflicting
  patches are (transitively) explicitly depended on by a single patch.
msg23146 (view) Author: ganesh Date: 2023-03-18.20:24:09
Looks good, couple of minor comments:

>   * add findConflicting to Darcs.Patch.Conflict

> +-- | Find all patches in the context that conflict with a given patch.
> +-- This works by commuting the patch and its dependencies backward until it
> +-- becomes unconflicted, then minimizing the trailing patches by re-commuting
> +-- them backward as long as that keeps the patch unconflicted.
> +-- Precondition: the context must contain all conflicting patches.

> +findConflicting
> +  :: forall p wX wY wZ
> +   . (Commute p, Conflict p, ShowPatch p)
> +  => RL p wX wY
> +  -> p wY wZ
> +  -> (RL p :> p :> RL p) wX wZ

It's not obvious to me from the signature which patches are the conflicting ones,
maybe add it to the doc comment?

> +-- | Like 'standardResolution' but it doesn't use the @instance (Named p)@
> +-- because the traling list of patches may contain "fake" conflictors.
> +rebaseResolution
> +  :: (RepoPatch p)

I wonder if it's worth tightening this signature to make it explicit that it
can't use the (Named p) instance?
Date User Action Args
2021-07-07 08:34:19bfcreate
2021-07-07 08:37:15bfsetstatus: needs-screening -> needs-review
2023-03-18 20:24:10ganeshsetstatus: needs-review -> review-in-progress
messages: + msg23146