ocaml-multicore / eio

Effects-based direct-style IO for multicore OCaml
Other
559 stars 72 forks source link

Improve handling of Windows paths #762

Open talex5 opened 1 month ago

talex5 commented 1 month ago

In https://github.com/ocaml-multicore/eio/issues/124, @samwgoldman said:

For Flow, we have the need to share paths between platforms. These are relative paths, using the / path separator. We only support Windows systems that transparently support the / path separator, so we can just pass these paths into Windows APIs without transforming them.

However, Eio also needs to support absolute paths on Windows and it seems this doesn't work (see https://discuss.ocaml.org/t/how-to-specify-a-full-windows-path-in-eio/14880). It would be good to investigate why.

@dra27 commented:

I must confess that introducing yet another notation for a Windows path doesn’t look at all sensible to me! /C:/path is simply invalid, and if that’s ever printed (error messages, exceptions, etc.) it kinda just looks like Eio doesn’t know what it’s doing.

As mentioned in the original issue:

The Fpath library encodes lots of knowledge about Windows paths. It probably can't be used directly because its paths behave differently depending on the host platform (preventing e.g. a program running on Linux from creating paths that are to be used on Windows). The path rules should probably be per-filesystem instead.

I think we should give up on a uniform representation of paths and instead move some operations (e.g. split) to Fs.Pi.DIR, so that each file-system can provide its own implementation.

samwgoldman commented 1 month ago

Apologies for the misleading feedback I shared. Absolute paths never came up in Flow (in practice, all paths are relative to the process's working directory) and I wasn't aware that Windows APIs don't support / for absolute paths.

talex5 commented 1 month ago

@samwgoldman not misleading at all; it was very useful to know we could ship 1.0 like that. I just tagged you in case you objected to a change of direction, but I think it won't affect you either way.

dra27 commented 1 month ago

Various thoughts, partly following up a chat with @patricoferris last week: it sounds as though it's worth separating what EIO needs internally from paths vs what the EIO user sees/provides. From that:

talex5 commented 3 weeks ago

I think rather than trying to identify a concept of a root directory, it's better instead to let go of the notion entirely

Indeed, and Eio doesn't actually require this anyway. The directory fs refers to the current directory. It's just that it allows access to the rest of the file-system from there (so e.g. fs / ".." can be used to refer to the parent directory). Possibly it should have been called cwd_unconfined or something.

the representation allows C:\nasty path "with redundant quoting" with ";" and spaces\bin;C:\Windows\system32 to be internally treated as two entries C:\nasty path with redundant quoting with ; and spaces\bin (that's the actual directory name)

I don't quite understand this. Does this mean that Windows doesn't allow quotes in names, but will ignore them if given?