[darcs-users] Data loss bug when renaming and replacing a non standard token?

Dan Pascu dan at ag-projects.com
Mon Mar 23 10:45:00 UTC 2009


On Monday 23 March 2009, Eric Kow wrote:
> On Sun, Mar 22, 2009 at 22:55:37 +0200, Dan Pascu wrote:
> > It seems that this happens if you do not have an external-merge
> > option defined in ~/.darcs/defaults or you do not specify one on the
> > command line, but use --mark-conflicts (or don't use any
> > ---xxx-conflicts).
>
> So we can think of --mark-conflicts as being yet another merge tool
> like one you would supply with external merge (only that it is
> understands darcs things like replace and move patches, whereas the
> external merge tool does not)

Sort of. I would expect --mark-conflicts to work like the cvs merge works. 
If the conflict cannot be resolved automatically, --mark-conflicts should 
add both versions in the working copy like:

vvvvvvvvvvvvvvvvv
one version
-----------------
the other version
^^^^^^^^^^^^^^^^^

and let the user sort it out before recording the conflit resolution 
patch.

>
> It's funny because your result is:
>
> allow-conflicts : leaves a replace patch in unrecord
> mark-conflicts  : nothing new
>
> Now with external merge (sans saving)...
>
> > $ cat testdata
> > Given %r/I have a file%r/ do
> > end
> > $ darcs whatsnew
> > replace ./testdata [%r/] / %r/
>
> This isn't surprising because it's just what allow-conflicts does.

I would expect that if I use an external merger and I do not save anything 
from it, it should add the 2 versions like shown above, letting the user 
sort it out later manually in an editor. In other words, it should work 
like --mark-conflicts proposed above.

>
> > 2.2 If I save the merged result from kdiff3, then I get the following
> > result:
> >
> > hunk ./testdata 1
> > -Given %r/I have a file%r/ do
> > +Given /I have a file/ do
> > +end
> > +Given %r/something else/ do
> > addfile ./testdata.orig
> > hunk ./testdata.orig 1
> > +Given %r/I have a file%r/ do
> > +end
>
> No surprise there, as far as I can tell.  In fact, this is the result
> we want, right?

That is correct, except the unreplaced %r/ (but I guess that's another 
bug). Also note that only version 1.0.9 did the right thing. 2.2.0 added 
the extra testdata.orig to the repository. So the patch you quoted above 
is wrong starting from addfile ./testdata.orig

>
> > 1. Why --allow-conflicts has a reversed replaced applied to working
> > copy?
>
> Right.  Shouldn't allow-conflicts just nullify both patches?

I'm not sure what --allow-conflicts should do. What it does, is unexpected 
as it doesn't put either of the entries, it leaves a third version in the 
file which was not present in any of the versions.

Maybe --allow-conflicts should not exist and we should only 
have --dont-allow-conflicts that works like now and --mark-conflicts that 
will leaves both versions in the file if no external merge tool is used 
or the external merge tool gives an error or the user doesn't save from 
it. Otherwise, with an external merge tool, it should put whatever the 
external merge tool saved.

In the end, what is the purpose of --allow-conflicts? What is the expected 
result and how can that be useful?

> It's almost as if --allow-conflicts and --mark-conflicts have the
> reverse behaviour

I do not expect any of the --xxx-conflicts to discard one version and only 
keep the other in the file, even less to discard both and keep a third 
one that didn't exist anywhere. For this reason I see no reason for both 
the allow and mark options as I don't see how they can do different 
meaningful things. IMO a --mark and a --dont-allow would suffice 
if --mark-conflicts would work by adding both versions in the file as 
shown above when no external merge is performed.

>
> > 2. Why both --allow-conflicts and --mark-conflicts have removed the
> > unrecorded lines.
> >
> > With --external-merge defined there are more questions though:
> >
> > 1. If external-merge is defined in defaults, --xxx-conflicts are
> > completely ignored. I believe the external merger should not be used
> > if I explicitly indicate on the command line --dont-allow-conflicts.
> > I'm not sure what should happen with --allow-conflicts.
>
> That sounds like a bug for which we ought to have a regression test.
>
> > 2. Even though there was a replace patch that changed '%r/' to '/',
> > in the merged result, the lines that were added but not yet recorded
> > in repo2, still contain %r/. I was under the impression that a
> > replace patch is useful exactly for these situations as it will
> > combine with the changes that may have still added the old token on
> > the receiving side and change those entries to the new token as well.
> > But here we see that the newly added lines kept the original token
> > and were not affected by the replace patch. Is this expected
> > behavior, and if so what is the usefulness of a replace patch if it
> > doesn't do more than a find/replace in the editor does? At least the
> > documentation explained this expected behavior for the replace patch
> > and use it a a reason why the replace patch is better than a
> > find/replace in the editor.
>
> Not having had a chance to think too carefully about this, I think this
> is a consequence of darcs's behaviour of nullifying both sides of the
> conflict first (and only adding stuff on top of it later in
> mark-conflicts)

My guess is that darcs doesn't handle the last two lines added locally in 
the given example, to get a chance to replace the token in them. This 
assumption is supported by the fact that if no external merger tool is 
used, those lines are missing from the file when darcs finishes with 
merging. My guess is that the external merger tool is the one to bring 
them back, because it sees them in one of the copies and the default for 
kdiff3 (at least) is to include the versions from both copies before 
giving control to the user to make whatever changes he wants.

-- 
Dan


More information about the darcs-users mailing list