[darcs-devel] darcs patch: Fall back to the inefficient, but safe, ... (and 4 more)

[David Roundy]

Hi Ian,

This looks good! (And it's a relief to have it done.)  But I'm wondering
how we ought to define mDoes{Directory,File}Exist.  Mightn't it be better
to make sure that the object really is a directory, not a symlink?

I can see that the behavior you've implemented is sort of nice in that it
allows users to make _darcs/prefs a symlink if they wish, but I'm a bit
afraid that this choice will come back to bite us later, if we base a
decision on how to treat a file (as file or directory) based on these
functions.  :(

Another alternative would be to do something like

data StupidObjectState
      = IsFile | IsDirectory | DoesntExist | IsSymlink StupidObjectState
mIdentifyStupidObjectState :: FileName -> m StupidObjectState

which would secretly be a call to lstat (and perhaps also stat, if lstat
doesn't tell us about the object pointed referenced by a symlink?).  From
this primitive we could then implement either variety of DoesExist
(e.g. doesFileExist vs doesFileReallyExist).  But this would be a tad more
complex.  Presumably if we eventually supported them, IsSymlink would refer
to "shortcuts" on Windows.

mDoesFileExist f = do st <- mIdentifyStupidObjectState
                      case st of
                        IsFile -> True
                        IsSymlink IsFile -> True
                        _ -> False

Anyhow, just some thoughts.  I'm not married to this idea, but wanted to
make sure you considered the implications of treating symlinks to
directories as directories.  Ideally, I wish that the IO instance of
ReadableDirectory and WriteableDirectory would be effectively chrooted,
which would give us a considerable safety barrier.  We don't currently have
that feature, since we never check that the FileName arguments are relative
to the current directory.  But if we were to add this feature (which would
be great, provided it didn't have a noticeable cost in efficiency), we'd
also need to be careful about how we treat symlinks, since we wouldn't want
people to be able to "escape" the chroot simply by putting a symlink inside
it.

Note that the API for changing directories is designed with this sort of
chrootish behavior in mind.  Also, of course, the Slurpy and other
instances implicitely have this behavior (you can't cd .. out of the one
you're in).  Making the IO monad also support this behavior would
definitely be comforting to me.
-- 
David Roundy
http://www.darcs.net