[MERGE] bug #54172: revert handles new directories nicely

[John Arbash Meinel]

Aaron Bentley wrote:
> Hi all,
> 
> This bundle fixes a bug in revert that caused newly-added directories to
> be kept, when they should have been deleted.  The problem is that
> directories (and symlinks) don't have a hash, so they aren't inserted
> into the merge-modified list.  Now, entries that don't have hashes are
> also removed.
> 
> Aaron

...


So I've been thinking about this fix, and I'm wondering if it is
incomplete. Because I think there are a few ways that things can be
removed that shouldn't be. But I want to check with you, and see what
you think.

1) A renamed directory will be removed.

So you did a merge, and then renamed the directory to somewhere else.
That is technically a content change.

Now, how do we handle files? I just did a test, and I can see that we
remove them.

bzr init a; cd a; echo x > x; bzr add x; bzr commit -m x;
cd ..; bzr branch a b; echo y > a/y; bzr add a/y; bzr commit -m y a/y

At this point, you have branches a and b, where merging a into b will
create the file y.

If I do:
cd b
bzr merge ../a # y shows up
bzr revert # y disappears, good

bzr merge ../a
echo y >> y
bzr revert # y stays as an unversioned file. reasonable, no data loss

bzr merge ../a
bzr mv y z
bzr st
added:
  z
# Reasonable enough

bzr revert # z disappears, I guess

I suppose there isn't a content change of z, so this is okay. Especially
since the file would become unversioned anyway, we would lose the
associated file id, and thus wouldn't be able to track a rename.

So I'm okay with that. And I guess for directories, we can just be
consistent, that a revert nukes renames.


2) This is more important, and that is that we will always nuke symlinks.

In the above situation:

$ cd ../a
$ ln -s x z
$ bzr add
$ bzr commit -m z

$ cd ../b
$ bzr merge ../a
$ bzr st:
added:
  z@

$ ll z
.. z -> x

$ bzr revert
# z goes away

$ bzr merge ../a
$ rm z
$ ln -s z y
$ bzr revert
# z still goes away

Now there was a content change to 'z' but it still gets removed by the
revert step. I'm not hyper critical of this, but it seems risky. In our
other handling of symlinks, we tended to substitute the symlink target
in place of the sha1 sum. Could we put the target into the
'merge_modified' dictionary without really hosing anything?

It may be good to do this as part of a working tree format bump. For
now, I think I prefer removing the symlink than keeping it, because it
reduces clutter. But if we want to be more in a 'never lose anything',
then we should not remove symlinks until we can tell that their content
hasn't changed.

3) Directories with sub-files.
There is a bug here, that the conflict statement is wrong:
$ cd ../a
$ bzr mkdir s
$ echo t > s/t
$ bzr add
$ bzr commit -m 'st'

$ cd ../b
$ bzr merge ../a
$ bzr revert
# Both s and s/t go away, *good*

$ bzr merge ../a
$ echo u > s/u
$ bzr revert
Conflict adding files to s.  Not deleting.

The conflict shouldn't say we have a conflict adding files to a
directory that we are removing. Better to say something like directory
not empty. At this point we have an unversioned 's' and an unversioned
'u'. ('u' was never verisioned anyway.

Otherwise the behavior is okay.

So I guess my only real concern is that this change now deletes symlinks
that have been modified. And that Conflicts should give better error
messages.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060829/e518b5d8/attachment.pgp