ocaml-multicore / eio

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

implement native for windows #738

Open kentookura opened 5 months ago

kentookura commented 5 months ago

The implementation is copied from lib_eio_posix/fs.ml, simply replacing a forward slash.

I added the test case to the windows directory since the cross-platform test has some hard-coded forward slashes.

The output of the test reads

+<fs> -> Some .
+<fs:\\> -> Some \
+<fs:\\etc\\hosts> -> Some \etc\hosts
+<fs:.> -> Some .
+<fs:foo\\bar> -> Some .\foo\bar
+<cwd> -> Some .
+<cwd:..> -> Some .\..
+<native-sub> -> Some
+                  \??\C:\Users\asdf\eio\_build\default\lib_eio_windows\test\native-sub\
+<native-sub:foo.txt> -> Some
+                          \??\C:\Users\asdf\eio\_build\default\lib_eio_windows\test\native-sub\foo.txt
+<native-sub:.> -> Some
+                    \??\C:\Users\asdf\eio\_build\default\lib_eio_windows\test\native-sub\.
+<native-sub:..> -> Some
+                     \??\C:\Users\asdf\eio\_build\default\lib_eio_windows\test\native-sub\..
+<native-sub:\\etc\\passwd> -> Some \etc\passwd

I don't quite understand what \??\ is all about.

https://github.com/ocaml-multicore/eio/blob/77d881014d0abb3246dda6f7af8178e86f05061a/lib_eio_windows/fs.ml#L60

kentookura commented 5 months ago

Hacking.md states that the cross-platform tests "are run against whichever backend Eio_main.run selects, and therefore must get the same result for all backends", so I am a bit confused about how to correctly set up the new tests, as native was previously not available for windows, and the tests for native assume forward slashes.

talex5 commented 5 months ago

Thanks for working on this!

Currently the tests directory isn't run on Windows (I think due to https://github.com/realworldocaml/mdx/issues/295, though it's unclear what the problem is). native will indeed give different results on Windows, but we can use the new os_type feature added by @polytypic (https://github.com/realworldocaml/mdx/pull/433) to skip the tests when they don't apply.

Windows supports forward and backward slashes in paths, and the idea is that an Eio.Path.t always uses forward slashes internally. So probably native needs to convert all of them to backward slashes for display.

I have no idea what the \\??\\ thing is about - @patricoferris wrote that code I think.

kentookura commented 4 months ago

Alright, so commenting out the line int test/dune that disables the mdx tests on windows causes 100% CPU and memory usage for 10+ minutes with no end in sight.

patricoferris commented 1 month ago

I have no idea what the \??\ thing is about - @patricoferris wrote that code I think.

I think we can definitely improve on the Windows code and file path handling !

My understanding (helped plenty after speaking with @dra27) is these are NT Object Manager paths. Windows, to the normal user, has no real notion of "root" (/ on Unix-y systems). In that case, what exactly does Eio.Stdenv.fs env give to Window's users?

In order to use lower-level system functions like NtCreate... (to provide an openat-style interface) we have to use these NT paths.