darcs

Issue 2432 pull --reorder does not commute reordered patches

Title pull --reorder does not commute reordered patches
Priority Status resolved
Milestone 2.10.0 Resolved in 2.10.0
Superseder Nosy List alex.aegf, ganesh, gh
Assigned To alex.aegf
Topics

Created on 2015-02-01.23:12:54 by gh, last changed 2015-03-04.00:13:04 by noreply.

Messages
msg17972 (view) Author: gh Date: 2015-02-01.23:12:52
This happens since the inplementation of this function by
http://bugs.darcs.net/patch1174 . I'm sending a test script soon.
msg18226 (view) Author: ganesh Date: 2015-02-26.08:49:19
I think I know roughly what's going on here, and it's a good example of how we don't really track witnesses properly in 
repositories (Ben: another example of where we can't trust them!)

The problem is the tentativelyReplacePatches call made in tentativelyMergePatches_ (in Darcs.Repository.Merge). The 
precondition for the replace is that the patches passed in are already commuted to the end of the repo, so they can be 
used as the properly commuted ones to re-add to the end of the repo after removing the current versions from wherever  
they were before - the type is roughly Repository wR wU wT -> Patch wX wT -> IO ().

The problem in tentativelyMergePatches_ is that while 'r' and 'usi' have the right types at the start of the function, by 
the time we get to the replace call, we've written the recently pulled patches to the repository, but the witnesses 
haven't changed.

Will have to think about how best to fix this...
msg18248 (view) Author: noreply Date: 2015-03-04.00:13:02
The following patch sent by Ganesh Sittampalam <ganesh@earth.li> updated issue issue2432 with
status=resolved;resolvedin=2.10.0 HEAD

* resolve issue2432: use the merged version of the local patches 
Ignore-this: 3c2a8e1cc4d6c8b8040f683e6f2a3dc0

The previous code gave a false sense of security in its use of witnesses
on the Repository type, because those can be implicitly mutated by
operations on the repository but the Repository value with its old
witnesses is still available for use.

The bug was that tentativelyReplacePatches should be passed the replacement
patches, as it just adds them to the repository at the end after commuting
out the previous versions. So when doing pull/apply --reorder, we need
to pass in the local patches merged with the remote patches, rather than
in their original form.

I reworked the code so that the witnesses are correct, which provokes
a type error, and fixed the error by changing merge2FL to return both the
merged remote patches and the merged local patches so the latter can be
passed in to tentativelyReplacePatches.
History
Date User Action Args
2015-02-01 23:12:54ghcreate
2015-02-03 18:52:13ghsetassignedto: alex.aegf
2015-02-26 08:49:21ganeshsetstatus: unknown -> needs-diagnosis/design
nosy: + ganesh
messages: + msg18226
2015-03-04 00:13:04noreplysetstatus: needs-diagnosis/design -> resolved
messages: + msg18248
resolvedin: 2.10.0