monotone-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Monotone-devel] heads up: file system changes


From: Stephen Leake
Subject: Re: [Monotone-devel] heads up: file system changes
Date: Wed, 23 Sep 2009 07:30:40 -0400
User-agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (windows-nt)

Zack Weinberg <address@hidden> writes:

> On Tue, Sep 22, 2009 at 1:55 AM, Stephen Leake >
>> I'll look at it more later, but I suspect the simplest fix is to just
>> move the original do_remove_recursive into win32/fs.cc.
>
> Yah, or you should be able to copy the unix version, which isn't very
> unix specific.  You might need more make_accessible calls though.

It turns out the fix to your code was simple; SHFileOperationA does not
like '/' directory separators, and it returns non-zero when the path
doesn't exist.

Now the tests are running again.

Side note: do_remove_recursive attempts to generate a nice error
message, but it doesn't come out anywhere that I can see.

>> What is the rationale for making this platform-specific? It would help
>> if that rationale was documented in the code.
>
> The immediate reason is, we need do_remove_recursive to work on raw
> pathname strings so it can delete things that can't be represented as
> pathname objects.  The only place to put code that works on raw
> pathname strings is {unix,win32}/fs.cc.

I guess the only place raw pathname strings occur is inside
do_remove_recursive, as it fetches file names from the OS?

Another rationale is that delegating file operations to the OS is
likely to be more efficient.

Why is do_remove in platform.hh? The unix implementation requires C90
'remove'. Are we assuming C90 is not available on Windows? I assume
MinGW provides that (but maybe it doesn't?); do we really care about
any other compiler/runtime?

> Notionally, in the future, we might want to switch everything in
> win32/fs.cc over to the Unicode APIs, and do_remove_recursive would be
> no exception, but I surely am not doing that work.

Ok; I added a comment about this.

But it would seem a better approach is to enhance any_path objects to
support all OS-supported filenames. Does that mean it has to be UTF-8?
I guess that might also require fs.cc to use the Unicode APIs. 

Does C90 not deal with this in a portable way?

-- 
-- Stephe




reply via email to

[Prev in Thread] Current Thread [Next in Thread]