[MERGE] fix blackbox failures when run from windows executable

[John Arbash Meinel]

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

Andrew Bennetts wrote:
> Mark Hammond wrote:
> [...]
>>> Transport.local_abspath is supposed to be the absolute path on the
>>> local filesystem.  I'm a little surprised that local_abspath is 
>>> different to the realpath.  Is it just a \ vs. / issue?
>> Exactly - / vs \.  Maybe a path.replace("/", os.path.sep) would be better?
> 
> I think I'd mildly prefer that.  That at least makes it clear that there's no
> fundamental change to the path being represented, just a cosmetic change of
> representation.

I think you are fixing the wrong half of it. I would rather you change:

self.assertEqual(os.path.realpath('blah', 'blah', 'blah'),
		 ...local_abspath('blah'))

to be

self.assertEqual(osutils.realpath('blah', 'blah', 'blah'),
		 ...local_abspath('blah'))

osutils.realpath() should handle the .replace('\\', '/') for you.

I think it is *correct* to assert that local_abspath() returns '/'
separated paths. Which is what osutils.realpath() will do.


> 
>>> In fact, the docstring for Transport.local_abspath says:
>>>
>>>         This function is quite expensive: it calls realpath which
>>> resolves symlinks.
>>>
>>> So I think perhaps the right fix might be to fix
>>> Transport.local_abspath (i.e.
>>> urlutils._win32_local_path_from_url) rather than these tests?
>> I'm inclined to agree with you about that - the function name implies native
>> seps should be used - however I brought this up with Robert on IRC and he
>> argued that this was still an 'internal path string', and as it would work
>> with all os functions, it was not a problem, and steered me towards that test
>> fix.
>>
>> My opinion is that if this function intends to serve as an api for getting a
>> "native" path that applications may call, it should use native seps - but if
>> it is indeed considered internal, I see no problem with it staying as it is.
> 
> Hmm, I think Robert's right that that's for internal use.  So in that case the
> only change to Transport.local_abspath we should make is clarifying the
> docstring, because at the moment it implies that it will produce the something
> equivalent to os.path.realpath's output.
> 
> -Andrew.
> 

So... local_abspath() is meant to be used because Transports work only
in terms of URLs, and sometimes we need to open something up on disk,
and doing open('file:////') doesn't work.
The only callers are in workingtree.py and workingtree_4.py

One could argue about it returning a "C:\Path" rather than "C:/Path",
but ATM I prefer it returning "C:/Path" to be consistent with all other
paths inside bzrlib.

John
=:->

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

iEYEARECAAYFAkigwyEACgkQJdeBCYSNAANqvgCgtAH/LfkARG7QspAoYa1bJE/f
bW0AmwagwoB/2PyHrRv8/Gy0ltESbWvK
=NpFD
-----END PGP SIGNATURE-----