Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implement native for windows #738

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kentookura
Copy link

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.

(* \\??\\ Is necessary with NtCreateFile. *)

@kentookura
Copy link
Author

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
Copy link
Collaborator

talex5 commented Jun 14, 2024

Thanks for working on this!

Currently the tests directory isn't run on Windows (I think due to realworldocaml/mdx#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 (realworldocaml/mdx#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
Copy link
Author

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants