[MERGE] Fix for bug 183831

[Geoff Bache]

> They probably should not.  I'd be happy with a solution that addressed
> all commands.  The reason I've opposed fixing this in osutils is because
> that may affect other operations that just commands.  bzrlib, being a
> library, should not be sensitive to the cwd.  It's easy to imagine using
> relpath for remote paths or URL fragments, for example.
>

Aha, I haven't been aware of this up until now, I had no idea this
code was reused in different products entirely. Finally I see why you
don't want to change osutils.

>> which
>> is wrong in a lot of cases. What "log" does I can't really fathom,
>> but it seems to work best of the three implementations from
>> experimenting (I haven't succeeded in breaking it, whereas breaking
>> the other two is easy). Maybe finding its symlink code would be a good
>> idea, but I've failed to do that unfortunately. Can you help?
>
> Sure.  The main code is in _get_info_for_log_files.  But the reason why
> it is bullet-proof (at least for the first argument) is that it uses the
> path returned by bzrdir.BzrDir.open_containing_tree_or_branch().
> internal_tree_files and tree_files_for_add, are both ignoring the path
> returned by WorkingTree.open_containing, supplying their own, and
> getting it wrong.
>

OK, I'm trying to make use of the returned path instead. But doing so
seems to have knock-on effects...

>> It seems wrong to me to just exacerbate this situation by inventing
>> some more complicated symlink rules of my own that apply only to
>> "add".
>
> Believe me, I'd be much happier if tree_files_for_add didn't exist.  I
> don't know why it does, but I suspect there is a better alternative to
> all this copy-and-paste code.
>
> I see a big difference in view handling:
> - - it considers the view when files are explicitly supplied
> - - it considers whether files are *contained* by view files , rather than
>  whether they are, themselves, view files.
> - - it raises an exception when files are outside the view instead of
>  ignoring them.
>

Did you miss safe_relpath_files? This is called by internal_tree_files
and seems to have roughly equivalent view handling to
tree_files_for_add.

> Other differences:
> - - no symlink traversal

which is what we're trying to fix...

> - - does not return tree-relative paths

This seems weird, but is very hard to change because of the large
amounts of unit tests that assume mutabletree.smart_add takes an
absolute path. I've got most of them working but am stuck on
bzrlib.tests.workingtree_implementations.test_smart_add.TestSmartAddTree.test_inaccessible_implicit
and nearly equivalent friend: although it's very unclear to me what
the point of this test would be if smart_add took a relative path like
everything else. It's based on monkey-patching osutils which seems a
strange thing to do (?)

Regards,
Geoff