darcs

Issue 547 weird rollback behavior

Title weird rollback behavior
Priority bug Status resolved
Milestone Resolved in
Superseder Nosy List darcs-devel, dmitry.kurochkin, kowey, markstos, ppessi, thorkilnaur, tommy
Assigned To
Topics Conflicts

Created on 2007-10-11.10:25:13 by ppessi, last changed 2009-08-27.14:07:27 by admin.

Files
File name Uploaded Type Edit Remove
exact-version ppessi, 2007-10-11.10:25:13 application/octet-stream
hidden-conflicts-bug.sh ppessi, 2007-10-26.11:17:14 application/x-shellscript
rollback-bug ppessi, 2007-10-11.10:25:13 application/octet-stream
Messages
msg2165 (view) Author: ppessi Date: 2007-10-11.10:25:13
Looks like rolling back two depending patches does not work as expected.

--Pekka

 $ darcs --version
1.0.9 (release)
$ sh rollback-bug
+ mkdir demo
+ cd demo
+ mkdir A
+ cd A
+ darcs init
+ cat
+ cat
+ cat
+ touch content
+ darcs add content
+ cp f1 content
+ darcs record -m p1 --all
Finished recording patch 'p1'
+ cp f2 content
+ darcs record -m p2 --all
Finished recording patch 'p2'
+ cp f3 content
+ darcs record -m p3 --all
Finished recording patch 'p3'
+ : Cannot rollback p2
+ darcs rollback --patch p2

Skipping depended-upon patch:
Thu Oct 11 13:10:44 EEST 2007  Pekka Pessi <Pekka.Pessi@nokia.com>
  * p2
Cancelling rollback since no patch was selected.
+ : Do it in B
+ darcs put ../B
Finished applying...

+ yes
+ darcs unpull --patch p3 --repo ../B

Thu Oct 11 13:10:44 EEST 2007  Pekka Pessi <Pekka.Pessi@nokia.com>
  * p3
Shall I unpull this patch? (1/?)  [ynWvpxqadjkc], or ? for help:
Finished unpulling.
+ yes
+ darcs rollback --patch p2 --repo ../B

Thu Oct 11 13:10:44 EEST 2007  Pekka Pessi <Pekka.Pessi@nokia.com>
  * p2
Shall I rollback this patch? [yNvpq], or ? for help:
Finished rolling back.
+ : Rollback p3
+ yes
+ darcs rollback --patch p3

Thu Oct 11 13:10:44 EEST 2007  Pekka Pessi <Pekka.Pessi@nokia.com>
  * p3
Shall I rollback this patch? [yNvpq], or ? for help:
Finished rolling back.
+ darcs revert --all
Finished reverting.
+ : We have UNDO p2 to pull
+ darcs pull ../B --dry-run
Pulling from "../B"...
Would pull the following changes:
Thu Oct 11 13:10:44 EEST 2007  Pekka Pessi <Pekka.Pessi@nokia.com>
  UNDO: p2

Making no changes:  this is a dry run.
+ : Pull it
+ darcs pull ../B --all
Pulling from "../B"...
Finished pulling and applying.
+ : Now we have rollback for p2 and p3
+ darcs changes
Thu Oct 11 13:10:44 EEST 2007  Pekka Pessi <Pekka.Pessi@nokia.com>
  UNDO: p2

Thu Oct 11 13:10:44 EEST 2007  Pekka Pessi <Pekka.Pessi@nokia.com>
  UNDO: p3

Thu Oct 11 13:10:44 EEST 2007  Pekka Pessi <Pekka.Pessi@nokia.com>
  * p3

Thu Oct 11 13:10:44 EEST 2007  Pekka Pessi <Pekka.Pessi@nokia.com>
  * p2

Thu Oct 11 13:10:44 EEST 2007  Pekka Pessi <Pekka.Pessi@nokia.com>
  * p1
+ : ..but
+ diff f1 content
4a5,6
> 12345
> 1234
Exit 1
Attachments
msg2166 (view) Author: droundy Date: 2007-10-11.15:51:02
On Thu, Oct 11, 2007 at 10:25:14AM -0000, Pekka Pessi wrote:
> Looks like rolling back two depending patches does not work as expected.

No.  Rollback isn't a very good command.  :(
-- 
David Roundy
Department of Physics
Oregon State University
msg2168 (view) Author: ppessi Date: 2007-10-17.02:09:09
2007/10/11, David Roundy <bugs@darcs.net>:
> On Thu, Oct 11, 2007 at 10:25:14AM -0000, Pekka Pessi wrote:
> > Looks like rolling back two depending patches does not work as expected.

> No.  Rollback isn't a very good command.  :(

Well, it seems to me that darcs isn't a very good command.

The problem has nothing to do with rollback command per se, but they
way the resulting patches are composed. The patch inverses can be
created with rollback or record, it does not matter. Any way the
patch-inverse pair hides conflicts.

Let me explain. If I have a patch P and its inverse P^{-1} , and then
a patch Q that conflicts with the patch P, darcs does not show any
conflict. The problem is not exactly alleviated by the fact that the
inverse can be composed, that is, if there is R and S so that (R S) =
P^{-1} or R S P = 1, the conflicting patch Q disappears.

Looks like innocent-looking whitespace changes and their accidental
reversals can eat my code. ;-(

I wonder if this is a well-known bug, I did not find anything that
fits exactly the bill from the bugs.darcs.net.
msg2174 (view) Author: kowey Date: 2007-10-24.12:42:42
issue436 might be related.
msg2176 (view) Author: ppessi Date: 2007-10-26.11:17:14
An another script demonstrating the problems is attached. It shows two hidden
conflicts, one involving patch d and its inverse, -d, and another involving
"null patch ring" with patches f, f2 and f0. 

The darcs changes --summary output on resulting repostories
/tmp/d1/R3../tmp/d1/R6 are surprising. The summary does not show the file
involved in hidden conflict.

I've also added some material on wiki and created a page called HiddenConflicts
dealing with the issue:

<http://wiki.darcs.net/DarcsWiki/HiddenConflicts>

The question is, what is the chosen strategy in fixing the bug #157 and friends
and how it could be dealt with in the mean time? Will the conflictors change the
behavior of hidden conflicts?

My concern is what happens if

1) Darcs is fixed so that it reports the now-hidden conflicts
2) Darcs is fixed so that null patch patch paths have null effect in conflict cases
3) Darcs is fixed in Correct Way, what ever that is

The case 1) is problematic, because there is possibly a large number of
repositories affected. Currently the resulting data corruption is unnoticed
unless it happens to have very significant effect, but Darcs reporting conflicts
in past versions would certainly get noticed and have effect on the users.

The case 2) is problematic. Instead of inevitable conflicts there would be
silent data resurrection (as it is now now have silently disappearing). Again,
the apparent repository state would change. This is not as bad bug as the case
1) in the presence of hidden conflicts. Apparently the current repository state
depends on ordering of the patches involved in the conflict, so had you to
reorder the patches, the result would be different in any case.

Now the option #3. My hunch is that it would need a version bump in repository
format and would either use merger 1.0 or conflictors or special tags beyond
which the patch semantics are changed.

Anyway, I'd like to have some brain time for the problem. While fix might be too
much to ask I'd like to have recommended ways to detect, avoid and recover from
the hidden conflicts.
Attachments
msg2420 (view) Author: markstos Date: 2008-01-11.04:07:42
I'm marking another rollback bug as a superceder. I believe David plans to
overhaul this command, and when that's done, many of the filed rollback bugs
will be addressed.
msg2556 (view) Author: markstos Date: 2008-01-18.02:57:28
ppessi,

I played with your "hidden-conflicts-bug.sh" tonight. Thanks for putting that
together!

However, I'm having trouble interpreting the output of the results. I'm used to
the standardized "ok/not ok" output of Perl test suites. 

I'm pasting the result of trying it with Darcs2 below, also using the darcs-2
format. I added some debugging output to see what was happening. Could
you review it and see if you still consider the bug present? 

If you do, "send" patch to the darcs-unstable repo with the failing test script
added to the "bugs" directory. 

Thanks!

   Mark

####3


darcs init
darcs add
Finished recording patch '0'
getting a
getting b
getting c
getting d
getting e
getting f
Finished pulling and applying.
Backing up ./text as ./text-darcs-backup0
We have conflicts in the following files:
./text
Finished pulling and applying.
Finished recording patch 'Resolve b+a'
No conflict in text:
x
Backing up ./text as ./text-darcs-backup0
We have conflicts in the following files:
./text
Finished pulling and applying.
Finished pulling and applying.
**** EXPECTING conflict: FAILED ***
Finished pulling and applying.
**** EXPECTING conflict: FAILED ***
Finished recording patch 'Resolve e+d+x+c+b+a'
No conflict in text:
f
Finished pulling and applying.
**** EXPECTING conflict: FAILED ***
/tmp/d1

Press ENTER or type command to continue
darcs init
darcs add
Finished recording patch '0'
getting a
getting b
getting c
getting d
getting e
getting f
Finished pulling and applying.
Backing up ./text as ./text-darcs-backup0
We have conflicts in the following files:
./text
Finished pulling and applying.
Finished recording patch 'Resolve b+a'
No conflict in text:
x
Backing up ./text as ./text-darcs-backup0
We have conflicts in the following files:
./text
Finished pulling and applying.
Finished pulling and applying.
**** EXPECTING conflict: FAILED ***
Finished pulling and applying.
**** EXPECTING conflict: FAILED ***
Finished recording patch 'Resolve e+d+x+c+b+a'
No conflict in text:
f
Finished pulling and applying.
**** EXPECTING conflict: FAILED ***
/tmp/d1
msg2562 (view) Author: ppessi Date: 2008-01-18.14:18:09
2008/1/18, Mark Stosberg <bugs@darcs.net>:
> I'm pasting the result of trying it with Darcs2 below, also using the darcs-2
> format. I added some debugging output to see what was happening. Could
> you review it and see if you still consider the bug present?

I ran the test script with darcs-unstable a couple of days ago and
noticed that it detects and marks conflicts correctly.

Utterly cool.
msg2564 (view) Author: markstos Date: 2008-01-18.15:02:09
Pekka Pessi wrote:
> 2008/1/18, Mark Stosberg <bugs@darcs.net>:
>> I'm pasting the result of trying it with Darcs2 below, also using the darcs-2
>> format. I added some debugging output to see what was happening. Could
>> you review it and see if you still consider the bug present?
> 
> I ran the test script with darcs-unstable a couple of days ago and
> noticed that it detects and marks conflicts correctly.
> 
> Utterly cool.

Great! It would still be nice to add your test as a regression test to 
"tests" if you don't mind preparing that.

Then we'll mark this bug as "resolved-in-unstable"

    Mark
msg2575 (view) Author: markstos Date: 2008-01-19.04:47:37
I just added a similar test script myself for this, so I'm marking this as
"resolved-in-unstable". See "rollback_conflict.pl" which I just sent to
darcs-devel if you are interested.
History
Date User Action Args
2007-10-11 10:25:14ppessicreate
2007-10-11 15:51:04droundysetstatus: unread -> unknown
messages: + msg2166
2007-10-17 02:09:10ppessisetmessages: + msg2168
2007-10-24 12:42:44koweysetmessages: + msg2174
2007-10-26 11:17:15ppessisettopic: + Conflicts
files: + hidden-conflicts-bug.sh
messages: + msg2176
2008-01-11 04:07:43markstossetnosy: + markstos
superseder: + Pulling rollbacks messes up dependencies
messages: + msg2420
2008-01-18 02:57:29markstossetnosy: droundy, tommy, beschmi, kowey, markstos, ppessi
superseder: - Pulling rollbacks messes up dependencies
messages: + msg2556
2008-01-18 14:18:11ppessisetnosy: droundy, tommy, beschmi, kowey, markstos, ppessi
messages: + msg2562
2008-01-18 15:02:10markstossetnosy: droundy, tommy, beschmi, kowey, markstos, ppessi
messages: + msg2564
2008-01-18 15:03:54markstossetstatus: unknown -> testing
nosy: droundy, tommy, beschmi, kowey, markstos, ppessi
2008-01-19 04:47:38markstossetstatus: testing -> resolved-in-unstable
nosy: droundy, tommy, beschmi, kowey, markstos, ppessi
messages: + msg2575
2008-09-04 21:31:29adminsetstatus: resolved-in-unstable -> resolved
nosy: + dagit
2009-08-06 17:51:20adminsetnosy: + jast, Serware, dmitry.kurochkin, darcs-devel, zooko, mornfall, simon, thorkilnaur, - droundy, ppessi
2009-08-06 20:54:12adminsetnosy: - beschmi
2009-08-10 22:09:00adminsetnosy: + ppessi, - darcs-devel, zooko, jast, Serware, mornfall
2009-08-11 00:02:41adminsetnosy: - dagit
2009-08-25 18:02:42adminsetnosy: + darcs-devel, - simon
2009-08-27 14:07:27adminsetnosy: tommy, kowey, markstos, darcs-devel, ppessi, thorkilnaur, dmitry.kurochkin