WorkingTreeFormat4: bzr status README fails on windows

[Wouter van Heyst]

> bzr-0.14 get bzr.dev
> cd bzr.dev
> bzr st README

still ok
> bzr.dev upgrade
> bzr st README
bzr: ERROR: [Error 22] The directory name is invalid: u'C:/bzr/bzr.dev/README\\*.*'

[Alexander Belchenko (bialix)]

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

Wouter van Heyst пишет:
> Public bug reported:
>
>> bzr-0.14 get bzr.dev
>> cd bzr.dev
>> bzr st README
>
> still ok
>> bzr.dev upgrade
>> bzr st README
> bzr: ERROR: [Error 22] The directory name is invalid: u'C:/bzr/bzr.dev/README\\*.*'

After dirstate is landing, bzr.dev is unusable on win32.
The fixes coming soon, I hope.

[µ]
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFF8WUnzYr338mxwCURAkWUAJ96FgzcoCGjzOndQ7cuAf/UJSLTQACdGYsl
ceKoAunqiaHroTVvW9ZlflU=
=AXOg
-----END PGP SIGNATURE-----

[Wouter van Heyst (larstiq)]

With bzr st README, os.listdir() is getting called on a file and not a directory. On Windows that raises
  WindowsError(267, 'The directory name is invalid')

instead of the expected OSError. The attached patch is probably not the right way to catch it,
but it does work for me.

[John A Meinel (jameinel)]

I think the correct fix is to actually change the _walkdirs api, so that the first record returned is the record for 'top'. So if it is just a file, that is the only record that will be yielded. We can return it as a list with only one entry to be consistent with the rest of the yield.

That way rather than catching an exception, walkdirs just is doing an lstat to make sure it is always operating on directories.

It is easy enough to change the _walkdirs_utf8 code, since that is still considered a private api.

[Robert Collins (lifeless)]

On Sat, 2007-03-10 at 18:03 +0000, John A Meinel wrote:
> I think the correct fix is to actually change the _walkdirs api, so that
> the first record returned is the record for 'top'. So if it is just a
> file, that is the only record that will be yielded. We can return it as
> a list with only one entry to be consistent with the rest of the yield.
>
> That way rather than catching an exception, walkdirs just is doing an
> lstat to make sure it is always operating on directories.
>
> It is easy enough to change the _walkdirs_utf8 code, since that is still
> considered a private api.

We can, but I think its more tasteful to have a specific api for gather
data on a path, and one for the children of: see path_info in my
dirstate2 branch - sftp://bazaar.launchpad.bet/~bzr/bzr/dirstate2

-Rob

--
GPG key available at: <http://www.robertcollins.net/keys.txt>.

[John A Meinel (jameinel)]

This seems to me like it means all callers of _walkdirs will now need to check if it is really a directory first, before they call walkdirs. Which means they are doing the same work, just now it has to be done by every caller, rather than doing it inside one function.

The big reason (in my head) is that walkdirs is the one that touches the disk, and most callers don't.

If they already knew whether it was a file or directory (for some reason) it would be different. But all they can really know is whether they *expect* it to be a file or directory.

[Robert Collins (lifeless)]

On Sun, 2007-03-11 at 20:10 +0000, John A Meinel wrote:
> This seems to me like it means all callers of _walkdirs will now need to
> check if it is really a directory first, before they call walkdirs.
> Which means they are doing the same work, just now it has to be done by
> every caller, rather than doing it inside one function.
>
> The big reason (in my head) is that walkdirs is the one that touches the
> disk, and most callers don't.
>
> If they already knew whether it was a file or directory (for some
> reason) it would be different. But all they can really know is whether
> they *expect* it to be a file or directory.

Most callers will want details about a path X, and its children. So they
will call:
details = path_info(X)
if details[0] is kind_directory:
    for directory_info in walkdirs(X):
        ....

No special case exception handling required in the caller. And walkdirs
doesn't need to stat or check the path being looked, it can assume the
caller has done that, so nothing is being called or looked at twice.

-Rob

--
GPG key available at: <http://www.robertcollins.net/keys.txt>.

[John A Meinel (jameinel)]

I have this in the associated bugfix branch. It should be merged for 0.15