[MERGE] Fix for bug 183831

[Aaron Bentley]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Geoff Bache wrote:
> On Thu, Apr 2, 2009 at 11:33 PM, Aaron Bentley <aaron at aaronbentley.com> wrote:
>> Geoff Bache wrote:
>>> The situation I'm trying to fix is far simpler and (I would guess) far
>>> more common and is nothing to do with how Bazaar version-controls
>>> symbolic links. In this case the symbolic link is just part of the
>>> full path to the working tree (usually because of disk volume mounting
>>> in my case).
>> That is not the case that this bug discusses.  This bug discusses the
>> unwanted traversal of symlinks.  When "bzr add"ing a symlink, you do not
>> want bzr to follow the symlink and add the referenced file.  You want to
>> add the symlink itself.
>>
>> Since you weren't trying to solve this bug, it was hard to see how your
>> solution could make sense.  In fact, traversing symlinks
>> inappropriately is the cause of bug 183831.
> 
> Now I'm even more confused. As far as I can see bug 183831 (the bug
> *I'm* trying to fix) is caused by failing to traverse an external
> symlink to correctly find the relative path to the repository.

Not quite.  bug 183831 is caused by *inconsistently* traversing a
symlink.  The bug is titled "bzr add <softlink> follows the link", so I
assumed the solution was for "bzr add <softlink>" to stop following the
link.

The example does not provide enough steps to reproduce the bug.  A
fuller example, as I now understand it, would be:
$ bzr init foo
Created a standalone tree (format: pack-0.92)

$ ln -s foo bar
$ bzr add bar
bzr: ERROR: bzrlib.errors.PathNotChild: Path "/home/abentley/bar" is not
a child of path "/home/abentley/foo"

I had misunderstood the nature of the complaint.  I that bar was being
traversed when determining the path to add.  Instead, it was being
traversed when the working tree was opened.  That misunderstanding is
probably why my suggestions didn't make any sense to you.

Still, there are two potential solutions to this problem:
1. Do not traverse the symlink at all.
2. Always traverse the symlink.

I think that 1 is a better kind of solution, because it avoids throwing
information away.  But even if we were to pursue 1, I don't think that
changing the behaviour of osutils.relpath is the right approach, because
it will change the behavior in other places, and it's not at all clear
that this is correct.

Here's an example of the problem with your approach:
$ bzr init foo
Created a standalone tree (format: pack-0.92)

$ ln -s . foo/bar
$ bzr add foo/bar --no-recurse
$ bzr status foo
?   bar@

Or, if we permit recursion,
$ bzr init foo
Created a standalone tree (format: pack-0.92)

$ ln -s . foo/bar
$ touch foo/baz
$ ~/bzr/bzr.ab.integration/bzr add foo/bar
adding bar
adding baz

This is especially bad if you do revert:
$ bzr revert foo/bar
- -   /

- -   baz
Conflict: can't delete  because it is not empty.  Not deleting.

This basically causes situations where you can't add a symlink at all--
either you add too much or too little.  And I think that's worse than
the bug you are trying to fix.

However, I'd support traversing the symlink in tree_files_for_add.  And
I'd be really happy with changing WorkingTree so that it doesn't
traverse the symlink in the first place.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAknjrEYACgkQ0F+nu1YWqI0x+wCfdtnR1yYREBF5uvm9Kl39FU7m
KL4AnjVZBLNNsoMw90FGS7jGcWqFtxx1
=P+Pu
-----END PGP SIGNATURE-----