On Wed, Sep 23, 2009 at 4:30 AM, Stephen Leake <stephen_le...@stephe-leake.org> wrote: > Zack Weinberg <za...@panix.com> 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.
Ugh. Well, thanks for fixing it. > Side note: do_remove_recursive attempts to generate a nice error > message, but it doesn't come out anywhere that I can see. It's probably being eaten by Lua. This is a long-standing general bug which would not be hard to fix, but it's tedious and fiddly -- basically, all of the points where control transfers in or out of the Lua interpreter need to convert between C++ and Lua exceptions, rather than throwing them away. Some, but not all, Lua exceptions are supposed to be thrown away -- for instance, if a hook is not defined -- so it isn't purely mechanical, either. > I guess the only place raw pathname strings occur is inside > do_remove_recursive, as it fetches file names from the OS? I think that right now there is no other place that needs to read *and process* arbitrary OS file names. Many other places fetch file names from the OS but can refuse to operate on pathnames that would be invalid any_path objects. > Why is do_remove in platform.hh? The unix implementation requires C90 > 'remove'. Are we assuming C90 is not available on Windows? Yes (see Daniel's reply). The platform-independent code assumes C90 semantics. > But it would seem a better approach is to enhance any_path objects to > support all OS-supported filenames. That's how we keep people from checking in files that can't be checked out again on some other platform (e.g. the file named "\" that the skip_invalid_paths test creates) so we certainly do not want that for file_path and bookkeeping_path. I could see the argument for system_path. I think we would then want to templatify most of file_io.cc rather than continuing to have it manipulate bare any_path objects, so that each class of path got the appropriate checks. Closely related question: what the hell is going on with new_optimal_path? AFAICT all users of that function ought to be using system_path, full stop. > Does C90 not deal with this in a portable way? *hollow laughter* zw _______________________________________________ Monotone-devel mailing list Monotone-devel@nongnu.org http://lists.nongnu.org/mailman/listinfo/monotone-devel