[darcs-devel] FullUnwind rebase behavior

[Ganesh Sittampalam]

Hi,

On 07/05/2020 09:00, Ben Franksen wrote:
> Am 05.05.20 um 22:49 schrieb Ganesh Sittampalam:
>> Suppose we have A;B suspended where A is a (previously) conflicted patch
>> and B is its resolution.
>>
>> Before, if darcs didn't crash, then if we unsuspended A, it would just
>> be replaced by its effect and maybe some conflict markers. If we
>> unsuspended A and B together, or unsuspended A, then reverted the
>> conflict markers and unsuspended B, we'd end up with the effect of A
>> followed by the effect of the conflict resolution, which is exactly what
>> we'd want.
>>
>> Now, the suspended patches will be <fixups_before>;A;<fixups_after>;B.
>> If we unsuspend A and B, we'll end up with conflicts between A and
>> fixups_before, and then conflicts between B and fixups_after. That'll be
>> rather a big mess.
> 
> It's worse. Even if there is no conflict when unsuspending, the conflict
> resolution may just get lost. Here is a simple test:

I've done a bit of playing now and I'm not sure the situation is totally
awful (though it's still not good!)

Your experiment is somewhat complicated by the fact that file existence
conflicts are already handled badly by darcs. If I try one where the
conflict is in file contents, the behaviour when I unsuspend one at a
time doesn't seem too bad. The behaviour if I unsuspend both
conflict+resolution together is really bad though.

The attached test case demonstrates that if you suspend a conflict + its
resolution, then first unsuspend the conflict and revert the conflict
markers, then unsuspend the resolution, you get reasonable behaviour:

- conflict patch unsuspends to reverting the thing it conflicted with
(i.e. the effect of the conflict)
- resolution unsuspends to applying the resolution on top of the base
state as you'd expect

I was also able to get something vaguely reasonable by using "rebase
inject" to push the fixups into the main patches, but that's hardly a
good user experience. Your idea of a squash operation seems plausible
but means people have to do more work "inside" the rebase state which
may pose some UI challenges.

More generally, I think it would be really useful for us to formulate
some generic properties of rebase that we can test with property-based
tests (QuickCheck or whatever). That'll require a bit of infrastructure
in the test harness but it seems like a reasonable investment.

One example of such a property would be that "suspend then unsuspend" is
a no-op, though we'd have to be precise about what that means in terms
of the working directory/conflict markers.

You commented that you have no idea what the force-commute operation
does. I don't really have much more of a description than a high-level
statement that it works via invert then merge then invert, but I think
in practice it is doing something approximating the right thing when a
single unwound patch is unsuspended: the fixups after the patch end up
being cancelled by the fixups resulting from the force-commute. That
makes me wonder why it doesn't work when unsuspending multiple patches.

Ganesh
-------------- next part --------------
#!/usr/bin/env bash

. lib

rm -rf R S

darcs init R

cd R
echo 'initial content' > f
darcs rec -lam "initial content"
cd ..

darcs clone R S

cd R
echo 'content1' > f
darcs rec -am "content1"
cd ..

cd S
echo 'content2' > f
darcs rec -am "content2"
cd ..

cd R
darcs pull --allow-conflicts -a ../S
echo 'content12' > f
darcs rec -am "content12 (resolution)"

echo yyd | darcs rebase suspend

echo yd | darcs rebase unsuspend
darcs rev -a
echo yd | darcs rebase unsuspend

cat > f.expected << END
content12
END

diff -u f.expected f