[MERGE] Fix bug #173944 - adding files below symlink causes error

[John Arbash Meinel]

John Arbash Meinel has voted resubmit.
Status is now: Resubmit
Comment:
+            if kind == 'symlink':
+                # Don't add files below a symlink'd directory
+                raise StopIteration, []

^- I don't think we support this notation. I think it should be:

StopIteration([])

Also, is StopIteration the right thing to be raising here? It 
specifically means that a generator/iterator is empty, and you should 
stop your loop.

I *think* you want to use a custom exception. Generally you can define 
one in bzrlib/errors.py, add a test for its formatting in 
bzrlib/tests/test_errors.py

And then do something like:

   raise MyCustomError([])


...

          if inv.has_filename(path.raw_path):
+            if kind == 'symlink':
+                # Don't add files below a symlink'd directory
+                raise StopIteration, []
              return []

^- I may be misunderstanding the code flow, but I don't see the benefit 
here.

Specifically in the outer function you do:

try:
   files = _add_one...
except ...:
   files = ...

And so in the one code path, you just return [] which sets files to []. 
And in the other, you raise an exception. Which sets files = [].

Isn't it better to just always return [] right away? And then you don't 
need to write a custom exception.

Also, how are you making it work. If you do:

bzr add symlink/foo.txt

does it actually end up doing

bzr add real_dir/foo.txt
?

For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C1214763998.6352.5.camel%40will-desktop%3E