[MERGE] Fix for bug 183831

[Aaron Bentley]

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

Geoff Bache wrote:
>>> I haven't added any unit
>>> tests because I don't know how meaningful they would be.
>> A unit test should always be added, unless you're working on a command
>> class itself.  Many things use osutils.relpath, so it's not appropriate
>> to only test "add".
> 
> OK. But I'm unsure of what you mean by a unit test in this case. I can
> see 2 possible routes:
> 1) Create symlinks for real in the file system.

I recommend this path.

> But many unit test suites ban touching the file system

We do not.  The vast majority of our unit tests touch the file system.

>: if everyone does this the unit
> tests will be just as slow as the blackbox tests.

Available evidence suggests otherwise :-)

> To me it would seem useful if
> --no-plugins was the default for
> "bzr selftest", as it seems like there is plenty of scope for this
> kind of thing to happen otherwise.

bzr itself includes plugins, so a complete run of the test suite must
include plugins.  We also want to encourage plugin authors to keep their
tests passing.

> Or at least for this issue to be
> mentioned in the Guide to Testing Bazaar to warn the unsuspecting
> newbie that they probably want to run with that flag if they want
> their tests to work.

I don't understand why failing plugin tests would prevent the newbie's
tests from working.

>>> It could be advanced that this change might slow things down: after
>>> all it adds complexity to quite a low-level function, and
>>> os.path.samefile will hit the file system whereas the old function was
>>> purely string manipulation.
>> What would really help here is an explanation of why calling
>> os.path.samefile is better than string manipulation.
> 
> I don't really follow you.
> I don't know any way to check for  the existence of symbolic links by string
> manipulation...

Okay, but I don't understand why you need to check for the existence of
symbolic links during relpath in order to avoid add traversing a
symlink.  For example, if you look at the comments in 124859, you'll see
my "terminal path element" suggestion for dealing with this situation.
I'd like to hear what you think of it.

> Isn't the fact that it fixes this issue enough for it to be "better"...?

When you're talking about changing the behaviour of a low-level function
in order to achieve a high-level effect, no it's not.

>> Time to run "bzr branches" on bzr.dev (best of 3)
> ...
> What is "bzr branches"? It's not a command my system recognises.

It's part of the bzrtools plugin.

> 3 samples doesn't sound very
> much to me for something that only takes 2 seconds anyway.

Best of 3 is our default practice for benchmarking.

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

iEYEARECAAYFAknUuVcACgkQ0F+nu1YWqI3lkgCdEVHU9kAuhI4HweeoTWPivhh3
MtUAn3O8hHKeOQK15WscuutWjY6cCAmS
=3h/E
-----END PGP SIGNATURE-----