[Sml-basis-discuss] [MLton-user] Re: MLton OS.Path under Windows [MLton-user Digest, Vol 27, Issue 6]

[Andreas Rossberg]

On Nov 14, 2008, at 14.08h, John Reppy wrote:
>
> On Nov 14, 2008, at 12:54 PM, Andreas Rossberg wrote:
>
>> On Nov 14, 2008, at 10.01h, John Reppy wrote:
>>
>>> Looking at the OS.Path specification, there is clearly an
>>> inconsistency in the definition of
>>> absolute paths on Windows.  The confusion arises because Windows
>>> has the notion of a path
>>> that is absolute relative to the current volume (but not strictly
>>> absolute, since changing
>>> the volume changes its meaning).  I'm not sure what the correct fix
>>> is.  The path of least
>>> resistance might be to change the specification of toString to read
>>>
>>> 	The exception Path is raised if vol <> "" and validVolume{isAbs,
>>> vol} is false.
>>
>> That would still allow fromString to deliver results that are not
>> considered "valid". For paths that are perfectly sensible.
>
> I'm not sure what you are driving at?  Can you give some examples?

Well, fromString "\\bar" = {isAbs=true, vol=false, arcs=["bar"]}, but  
validVolume{isAbs=true, vol=""} = false. Likewise for the root path "\ 
\". These certainly are meaningful paths, so it's weird that the  
validVolume function somehow considers them invalid.


>> Since we are at it, as I mentioned earlier (on the MLton list) I
>> have a number of other issues with inconsistencies in OS.Path. If
>> you are interested, I'd be happy to post them.
>
> Please do so.

Here is my list. Note that about half of the problems can easily be  
resolved by interpreting "/" as containing no arcs. (I would guess  
that the motivation for letting it have an empty arc was the desire  
to treat the root separator like any other arc separator (in  
particular, a trailing separator indicates an empty arc).  
Unfortunately, the root separator is quite different in nature, and  
behaves more like a volume.)

Best,
- Andreas


Empty arcs:

- The path "/" is treated as having an empty arc. This introduces a  
special
   case of a valid initial empty arc, and consequently, a number of  
issues
   with canonical paths and invertibility of string conversion (see  
below).
   It also arguably is inconsistent with the treatment of "", which  
has no arcs.

   Proposal: Define "/" to have no arcs. Consider initial empty arcs
   illegal under all circumstances.

- Related, what is #arcs(fromString "A:"), e.g. on Windows, [] or [""]?

   Proposal: Following the previous point, choose [].


Canonical paths:

- The definition of canonical paths rules out empty arcs, but then lists
   "/" as a canonical path (which supposedly has an empty arc).

   Proposal: Resolved by interpreting "/" to have no arcs.

- On the other hand, the spec as is actually makes "" a canonical path
   (it contains no arcs). This does not seem to be intended, as  
witnessed
   by the examples and the definition of mkCanonical.

   Proposal: Explicitly rule out "" from canonical paths.

- Since "/" and "/." denote the same path, not both should be canonical.
   (That also is inconsistent with defining "." as canonical, but not  
"")

   Proposal: Only allow "." in canonical paths that otherwise empty.
   In particular, don't treat "/." as canonical.
   [All systems I tried actually treat "/." as non-canonical]


String conversion:

- toString o fromString can only be the identity up to the choice of arc
   separators (e.g. on Windows, where both #"/" and #"\\" are allowed).

   Proposal: Weaken the respective statement.

- fromString o toString is not the identity for absolute paths with  
no arcs, e.g.:
       {isAbs=true, vol="", arcs=[]} -> {isAbs=true, vol="", arcs=[""]}

   Proposal: Resolved by interpreting "/" to have no arcs.

- validVolume on Windows allows empty volumes only for relative paths.
   This treats basic paths like "\\d\\e" or even "\\" as "invalid", and
   furthermore causes violation of the requirement for toString o  
fromString
   to be the identity.

   Proposal: Allow empty volumes unconditionally on Windows.


JoinDirFile:

- Is defined as just "extending" a path with an arc, but this  
violates the
   stated principle that canonical paths should be preserved (namely  
in the
   cases of "." and "/.").

   Proposal: Specify that for dir=".", file is returned. The case  
"/." is
   dealt with by not considering it canonical (see above).
   [All systems I tried construct non-canonical paths in these cases.]

- Since "/" is supposed to have an empty arc, the spec as stated would
   imply joinDirFile{dir="/", file="a"} = "//a", which is non-sensical.

   Proposal: Resolved by interpreting "/" to have no arcs.

- The description also does not cover the case where dir has no arcs
   and the file is the empty arc (the logical result is not  
representable
   as a string, since it would have an initial empty arc).

   Proposal: In order to behave like the inverse of splitDirFile,  
specify
   that the special case of joining a dir without arcs with the empty  
arc
   returns the dir.


Absolute/relative paths:

- According to the spec, mkRelative{path="/a", relativeTo="/"} has to
   deliver "../a", because "/" is canonical but defined to contain an  
arc.

   Proposal: Resolved by interpreting "/" to have no arcs.

- It would be *very* useful if mkRelative worked for relative paths,  
too.

   Proposal: Instead of raising Path, specify that if both arguments are
   relative paths, resolution proceeds analogously to two absolute  
paths,
   essentially treating "." as the root. Here are examples:

   path     relativeTo  mkRelative{path, relativeTo}
   "a"      ""          "a"
   "a"      "."         "a"
   "a"      "a"         "."
   "a/b/c"  "a"         "b/c"
   "a"      "b/c"       "../../a"
   "a"      "./a"       "."
   "a/"     ""          "a/"

- The spec of mkRelative is inadequate for case-insensitive file  
systems,
   because relativeTo is brought into canonical form, but not path.

   Proposal: At least make clear that "common" prefix has to be  
interpreted
   up to equivalence classes of file names on a given system. Or simply
   require path to be made canonical, too.

- Similary, the sample implementation for concat suggests that  
volumes must
   be equal even with respect to case. Again, this is inadequate for
   case-insensitive file systems.

   Proposal: Add a respective comment.

- Typos
   mkAbsolute:
     s/mkCanonical (concat (abs, p))/mkCanonical (concat(relativeTo,  
path))/
   mkRelative:
     s/but have different roots/but have different volumes/
   concat:
     s/A implementation/An implementation/