Re: [boost] Filesystem: remove_all should not follow symlinks

From: Jeremy Maitin-Shepard (jbms_at_[hidden])
Date: 2003-12-01 00:48:15

Walter Landry <wlandry_at_[hidden]> writes:

> Jeremy Maitin-Shepard <jbms_at_[hidden]> wrote:
>> Walter Landry <wlandry_at_[hidden]> writes:
>> 
>> > Jeremy Maitin-Shepard <jbms_at_[hidden]> wrote:
>> >> Walter Landry <wlandry_at_[hidden]> writes:
>> > The default should be to resolve symlinks, much like POSIX stat.
>> > Since it is mostly POSIX programmers who are going to have to deal
>> > with symlinks, it is best not to make defaults gratuitously different.
>> 
>> But other POSIX calls (and other functions in the boost filesystem
>> library) don't resolve symlinks, such as remove, rename.

> Actually, the boost filesystem library always calls stat() instead of
> lstat().  The only time it doesn't resolve symlinks is when it punts
> to ANSI C functions like remove and rename.

This is really just a matter of not recognizing symbolic links at all
though.

> Also, looking at the mail you just posted for hard link support, you
> decided to resolve symbolic links.  equivalent(), for example, needs
> to be modified to account for symbolic links.

That is true, I should have used lstat, although the code I posted was
basically just a `proof-of-concept,' and since the library
currently lacks support for symbolic links, there would be no way to
determine if the files to which the symlinks point are equivalent.
Consider though that a function like equivalent that takes two path
arguments does not work well with your suggested syntax of a bool
argument to specify if the symlink should be resolved: should there be
two bool arguments for equivalent, and in what order?

bool equivalent(path lhs, path rhs, bool deref_automatically_lhs =
true, deref_automatically_rhs = true);

equivalent(lhs, rhs, true, false);
Or, if you use constants:
equivalent(lhs, rhs, fs::dont_resolve, fs::resolve);

This definitely seems more awkward than:
equivalent(lhs, *rhs);

Or,
equivalent(lhs, resolve(rhs);

Or,
equivalent(lhs, dereference(rhs));

This same issue applies to the "link" function, or to the existing
"copy_file" function.  Note also that a "copy_file" that resolved
symbolic links automatically (by default) would be different from
rename, which we can agree should not.

>> I think it is clear that exists(ph) returning false if ph is a symlink
>> that points to a non-existent file will be confusing.

> That is what stat() returns.  For people dealing with filesystems with
> symbolic links, it isn't that confusing.

But stat does not have a name like "exists".

>> And if exists has different defaults from other query functions, it
>> will also be confusing.

> If we're just talking about query functions, then there can be no
> argument.  In Unix, stat() and lstat() give all of the information
> about files.

With two separate functions, stat and lstat, there are no `defaults.'
But that is a different type of interface from that in the
boost.filesystem library.

>> > As for the choice of names, we could make an enum or typedef with
>> > whatever name you like.  Then we could have something like
>> 
>> >   last_write_time(ph,dont_resolve)
>> 
>> This works, but the syntax still seems a lot more awkward than the
>> syntax I suggested.

> I think that my syntax is less awkward.  It even uses one less
> character (not that that really matters).

I suppose `awkward' in this case is a matter of preference (but see
above regarding `equivalent').  As far as the one less character, it
depends on which behavior is desired (perhaps depending on whether you
are looking to open a file, or looking to create an archive/backup
utility, or a utility that manipulates the filesystem.)

>> Also, I don't like the idea that the syntax is less uniform and each
>> of these query functions have to be modified to deal with symbolic
>> links.

> I'm not sure what you mean by "less uniform".

You are suggesting that query functions have different defaults from
the operation functions, as far as automatically resolving symbolic
links.  Furthermore, for two-path-arguments functions, a bool argument for
each path is certainly a more awkward interface.

> In any case, modifying these query functions is going to have to
> happen anyway.  Whether it is in the function or elsewhere.

They would have to be modified to use lstat, rather than stat, but other
than that, they would not need to deal with symbolic links.  If default
arguments are used instead, every function that could conceivably be
applied to either a symbolic link directly or to the path to which the
symbolic link points would need to include logic to deal with symbolic
links, and its signature would also need to be modified.

>> If the dereference function, when applied to a non-symbolic link, is
>> just the identity function, it wouldn't be particularly difficult to
>> use.

> It makes the whole function less transparent.  With the extra default
> argument, I can guess what is going on without even looking at the
> function definition.  With your syntax, I am tempted to look up the
> definition of dereference().

I don't think it is unreasonable to ask that users read all of the
boost.filesystem documentation, or at least read the documentation
regarding symbolic links (when added), before using them.

I do agree that operating on the pointed-to path is more common than
operating on the symbolic link directly, but IMO the value of added
convenience is outweighed by the awkwardness of the syntax of default
arguments, especially for two-path functions.

-- 
Jeremy Maitin-Shepard