[boost] FileSystem Review Results

From: William E. Kempf (williamkempf_at_[hidden])
Date: 2002-10-04 13:49:08

From: "William E. Kempf" <williamkempf_at_[hidden]>
> The formal review of the FileSystem library is officially over.  I'll
> tabulate the votes and summarize the comments in the next day or two.
> Thanks to all who reviewed the library.

Sorry it's taken so long.  My life is very hectic right now!

I'd like to announce that the FileSystem Library has been accepted into
Boost.  We received several useful evaluations and obtained 7 votes, all for
inclusion.  Congratulations Beman!

Here's a summary of the issues brought up during the review period:

* Problems with the iterator implementations were pointed out and need
corrected before official release.

* Problems with the implementation when doing "Unicode builds" on Win32
platforms were pointed and need to be corrected before release.  (Note this
has to do with the Win32 API using macros to "transparently" select actual
functions to be called based on whether or not Unicode is defined, and not
to do with supporting Unicode file names.)

* Path "joining" semantics were discussed and the syntax should be changed
to "/" and "/=" (from "<<" and "<<=").

* Issues with relative vs. absolute paths were discussed, and the general
concensus was to add a "root" concept (overly simplified... refer to the
reviews for full explanation).  Moderators Note:  This idea should actually
make it relatively simple to add concepts such as URLs to the path concept,
and is in general a very nice solution to a tricky portability issue.

* An issue with filesystem/fstream when using VC6 was pointed out, and
should probably be resolved (if possible) before official release.

* Several people pointed out that file_path() and directory_path() were
confusing.  Beman suggests that these be moved to a sub-namespace since they
should rarely be used.

* It was suggested that the documentation for generic_path(), file_path(),
directory_path(), leaf() and branch() need to be fleshed out some more.

* Several people suggested there needs to be a method to query and
manipulate the "current directory".  Beman agrees but thinks the usage needs
to be somehow discouraged.  As review moderator I suggest that this
functionality isn't strictly needed since the cases you'll need this are
almost universally cases in which you'll be dealing with non-portable
interfaces any way, so though inclusion is a worthy goal I don't think it
should be required for the official release.

* Extensions to allow for querying and manipulating attributes was
suggested.  This is another area where I think it's a worthy goal, but not
necessary for official release of the library.

* It was suggested that examples be given that implement cp, mv and ls, and
code was even provided.  This should be added to the official release.

* There was some discussion about rename() being present, but no move().  As
review moderator I'd like to point out that though a single method can be
made to handle both concepts, on some platforms rename() semantics are
restricted in several ways, such as not being able to rename() to an
existing file name, not being able to rename across certain boundaries, such
as file systems or even directories, etc.  So if the choice is made to have
only a single operation I'd suggest it be called move() instead, regardless
of what the C Library may have done, as this obeys the principle of least
surprise.  In any event, I don't believe this issue is a sticking point and
the group will accept any decision.

* It was suggested that there's a need for more fine grained exception
types.  As moderator I'd suggest that most cases retain the current design,
throwing only filesystem_error, while a few known cases be modified to throw
exceptions derived from this type to more accurately signify what caused the
error.  As other cases become known we can change them to throw more fine
grained exceptions as well.  It's probably not worth the effort of trying to
determine every possible case where finer grained exceptions *might* be
needed at this point.

* It was suggested that directory_iterator be made non-recursive by default,
with an overloaded constructor to control this behavior.  It's also noted
that the predicate functionality of directory_iterator needs more
documentation.

* It was suggested that trailing "/" be allowed in paths, as on many
platforms this would be expected behavior.

* Several people think that i18n path name support is needed.  I agree with
Beman that this is a very complex issue that no one knows how to handle
portably (and that it's not compatible with the standard libraries any way),
so initially we should make sure the library is functional and feature
complete before we tackle this issue.

Hope I didn't miss any other important comments.

Once again, congratulations Beman!

Bill Kempf
Review Moderator