[darcs-devel] Addressing "darcs rebase --unsuspend should stop at first conflict"

Ganesh Sittampalam ganesh at earth.li
Tue Jan 9 07:11:41 UTC 2018


Hi,

Sorry for being slow to reply.

I think if you literally want to get rebase --unsuspend to stop at the
first conflict, the way to do it is to mirror the existing
"--skip-conflicts" option, which works by filtering the patches that are
even offered to the user:

    offer :> dontoffer <-
        return $
            case O.conflictsYes ? opts of
              Nothing -> partitionUnconflicted inRange -- skip conflicts
              Just _ -> inRange :> NilRL

I'm not sure what the option should be called, as
--stop-at-first-conflict is a bit verbose, but otherwise it seems like
quite a natural addition to darcs, that could also be applied to other
commands that accept --skip-conflicts (I think pull and apply).

That said, this might not be the ideal workflow/UI, so some comments on
your ideas/questions inline:

On 27/12/2017 16:13, Dan wrote:
> I'm interested in addressing this ticket http://bugs.darcs.net/issue2552
> "darcs rebase --unsuspend should stop at first conflict". I've done a
> bit of research into the codebase and am looking for verification of my
> understanding and tips on further approaches.
> 
> So far, I've been digging around in the unsuspendCmd function in
> Darcs.UI.Commands.Rebase. On line 373, there is this section:
> 
>     have_conflicts <- announceMergeConflicts "unsuspend"
>         (allowConflicts opts) (externalMerge ? opts) standard_resolved_p
>     Sealed (resolved_p  :: FL (PrimOf p) wA wB) <-
>         case (externalMerge ? opts, have_conflicts) of
>             (NoExternalMerge, _) ->
>                 case O.conflictsYes ? opts of
>                     Just O.YesAllowConflicts -> return $ seal NilFL --
> i.e. don't mark them
>                     _ -> return $ seal standard_resolved_p
>             (_, False) -> return $ seal standard_resolved_p
>             (YesExternalMerge _, True) ->
>                 error "external resolution for unsuspend not implemented
> yet"
> 
> I think at that point we know about any conflicts. Additionally from my
> reading it appears that the externalMerge resolution strategy is capable
> of pausing at conflicting patches while it calls an external merge tool
> to handle the conflicts. This isn't currently implemented for unsuspend,
> but appears to implemented for other commands using applyPatches by way
> of standardApplyPatches by way of tentativelyMergePatches.

I've never really used external merge tools, but I thought the external
merge tool normally gets called once in a single darcs invocation, i.e.
once for all conflicts, rather than once per conflict? That's the
'externalResolution' call in
Darcs.Repository.Merge.tentativelyMergePatches_.

> It looks like the doAdd helper function is responsible for actually
> adding the unsuspended patches to the repository, although it doesn't
> seem to do anything specific to patches with conflicts.
> 
> For an approach, I'm thinking of two things (in no particular order):
> 
> 1. I can try to implement the external merge tool option for rebase
> unsuspend and work its method of pausing at conflicts into the internal
> merge tool logic.
> 2. I can try adding some specific logic to check if a patch being added
> in "doAdd" has conflicts and pause there to allow users to fix the
> conflicts.
>
> As considerations for how this should work. If I was using this, I think
> I would like to be able to handle conflicts in one of two ways. The
> first way would be to resolve the conflicts and amend them into the
> patch being unsuspended. The second way would be to resolve the
> conflicts and work them into one (or more) new patches. I think a
> solution should include these two options.

Adding external merge tool support to rebase would be good to fill in a
gap, but I think it'd be a bigger change if it were to pause at *each*
conflict individually in a single 'rebase unsuspend' invocation.

I think the current intended workflow is that you unsuspend each
conflict in a separate darcs invocation, unless you want to deal with
multiple sets of conflicts at once.

> Questions:
> 
> Why doesn't unsuspendCmd use applyPatches to apply the unsuspended patches?


I'm not 100% sure without trying to make it do it, but think it's
because unsuspend isn't merging two streams of unrelated patches in the
same way as pull/apply, so when
Darcs.UI.ApplyPatches.standardApplyPatches ultimately calls
'tentativelyMergePatches', that's inappropriate for unsuspend. Although
unsuspend is generating conflicts in the same way as a merge, it comes
from the rebase fixup patches which are a bit different.


> Why doesn't the doAdd helper "(repository'', renames) <- runHijackT
> IgnoreHijack $ doAdd repository' ps_to_unsuspend" call mention anything
> about the resolved patches? It only seems to use the patches that were
> selected for unsuspending.

What do you mean by 'resolved' patches here? doAdd is exactly for adding
the selected patches to the repository.

Cheers,

Ganesh


More information about the darcs-devel mailing list