[darcs-devel] [patch1871] slightly extend and cleanup test for iss... (and 5 more)

[Ben Franksen]

Ben Franksen <ben.franksen at online.de> added the comment:

>> I think the above considerations clearly show that there is /no/
>> consistent solution. This means we must /fail/ with an error message
>> saying that pulling a patch that conflicts with pending changes is not
>> supported; to make the pull succeed, the user can either revert or
>> record the conflicting pending changes.
> 
> So it's ok to conflict with unrecorded, but not with pending? That feels
> awkward to me, but I'm not sure there are any good solutions so we'll
> have to compromise somewhere.

Ah, no. I meant pending +>+ working. Sorry for being unclear. We throw
them together into a single anonymous Named patch during merge. The
wording also makes sense as these really are all "pending changes" i.e.
unrecorded changes. I'll try to be precise and say "unrecorded" from now on.

> Just thinking out loud now, when darcs puts something in pending, is it
> always also present in unrecorded?

By definition, yes. D.R.State.unrecordedChanges reads the pending patch,
applies it to the recorded state, then calculates the diff between that
and the working state. Then the two sequences are appended (but there
are other calls that return them separately).

BTW. Apart form the single use of addPendingDiffToPending (what a name!)
in the move command, we usually add things to pending by making a patch
that applies to the working state, then commute it backwards so we can
add it to the end of the pending patch. Oh, I forgot: we also have
tentativelyRemoveFromPending and tentativelyRemoveFromPW. these are
there to adapt pending when we amend, record, obliterate etc. They add
things to pending from the left. For those commands we must manually
ensure we have a consistent working state.

> We certainly can't make the stronger
> claim that pending is always a subset of unrecorded because the user can
> always break that invariant.

I think the way we read working simply makes it so. You can of course
make changes to the working state that annihilate parts of pending. This
just means our list of unrecorded changes gets longer.

> If so, then maybe the starting point is to think about what should land
> in unrecorded when there's a conflict with a patch we're pulling, and
> then try to deduce what that implies for pending.

This is exactly what I tried to do with my analysis. Replace "pending"
with "unrecorded changes" in my previous message and re-read.

The point is that we have no way to represent a conflict with unrecorded
changes. Applying the conflict markup is not a good solution. First of
all, we can only mark hunk/hunk conflicts. What about other conflicts?
Next, there is no way to restore the conflict markup if we change the
working state. This is the same problem I have with rebase unsuspend: a
simple revert -a eradicates all the information about the conflict. We
should not do something automatic that is neither sound nor safe.
Telling the user that they must either record or revert before the merge
can succeed is the only sensible solution.

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch1871>
__________________________________