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

MSVC compiler support with Win32 pread/pwrite #383

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

Conversation

jonahbeckford
Copy link

This PR defines ssize_t which helps get the MSVC compiler working. But there is no pread or pwrite on Windows so more PRs are needed.

(I was trying to see if irmin-unix would work on Windows, but there are too many other meaty issues like the lack of pread/pwrite).

@tomjridge
Copy link
Contributor

tomjridge commented Mar 11, 2022

My guess is that, since we are single threaded, we can replace a pread/pwrite with a seek+read/write (assuming the code is not in lwt). For Lwt, I assume that they have a separate implementation of pread/pwrite that works on Windows as well. This seems to be the case after reading https://ocsigen.org/lwt/latest/api/Lwt_unix

I guess the problem is then with code like "append" that assumes the seek pointer is always at the end of the file. But this could be enforced just by calling seek before appending data.

@tomjridge
Copy link
Contributor

If we really wanted to support Windows, perhaps another option is moving IO functions to a library like luv or some other cross-platform IO library.

@jonahbeckford
Copy link
Author

Sigh ... I don't know why I didn't see your comments earlier. I had gone ahead and implemented+debugged a Windows port of pread/pwrite implementations. Didn't see any problems with either pread or pwrite during my debugging sessions.

Testing

I tried the tests (I can't compile crowbar so the fuzzing tests were not run) ... a couple tests fail. The semantics of Windows file removal and renaming are too different compared to Linux (ex. can unlink file while it is open on Linux; definitely can't do that on Windows).


Command [18] exited with code 2:
$ (cd _build/default/test/cli && ./generate.exe)
Fatal error: exception Unix.Unix_error(Unix.EACCES, "unlink", "data/random\\index\\log_async")
Raised by primitive operation at Index_unix.IO.clear in file "src/unix/index_unix.ml", line 213, characters 6-24
Called from Index.Make_private.v_no_cache in file "src/index.ml", line 569, characters 22-59
Called from Index.Make_private.v.new_instance in file "src/index.ml", line 628, characters 8-97
Called from Dune__exe__Generate.random in file "test/cli/generate.ml", line 15, characters 14-61
Called from Dune__exe__Generate in file "test/cli/generate.ml", line 22, characters 9-18

I added some debug lines for this one ... line numbers may be a bit off

Running[56]: (cd _build/default/test/unix && ./main.exe)
(point1) Renaming data/random\index\merge to data/random\index\data
Thread 1 killed on uncaught exception Unix.Unix_error(1, "rename", "data/random\index\merge")
Raised by primitive operation at Index_unix.IO.rename in file "src/unix/index_unix.ml", line 65, characters 4-33
...
(point1) Renaming data/random\index\merge to data/random\index\data
Thread 9 killed on uncaught exception Unix.Unix_error(1, "rename", "data/random\index\merge")
Raised by primitive operation at Index_unix.IO.rename in file "src/unix/index_unix.ml", line 65, characters 4-33
Called from Index.Make_private.unsafe_perform_merge.(fun) in file "src/index.ml", line 842, characters 14-48
Called from Stdlib__fun.protect in file "fun.ml", line 33, characters 8-15
Re-raised at Stdlib__fun.protect in file "fun.ml", line 38, characters 6-52
Called from Index.Make_private.unsafe_perform_merge in file "src/index.ml", line 840, characters 10-1023
Called from Stdlib__fun.protect in file "fun.ml", line 33, characters 8-15
Re-raised at Stdlib__fun.protect in file "fun.ml", line 38, characters 6-52
Called from Index.Make_private.merge'.go in file "src/index.ml", line 910, characters 8-142
Called from Index_unix.Thread.async.protected_f in file "src/unix/index_unix.ml", line 398, characters 29-34
Re-raised at Index_unix.Thread.async.protected_f in file "src/unix/index_unix.ml", line 401, characters 8-17
Called from Thread.create.(fun) in file "thread.ml", line 41, characters 8-14

...
+195706us [DEBUG] index_unix [_tests\unix.main\index_6\index\merge] flushing 200 bytes
Thread 19 killed on uncaught exception Unix.Unix_error(1, "rename", "_tests\unix.main\index_6\index\merge")
Raised by primitive operation at Index_unix.IO.rename in file "src/unix/index_unix.ml", line 63, characters 4-33
...
+420086us [DEBUG] index_unix [_tests\unix.main\index_30\index\merge] flushing 200 bytes
Thread 37 killed on uncaught exception Unix.Unix_error(1, "rename", "_tests\unix.main\index_30\index\merge")
Raised by primitive operation at Index_unix.IO.rename in file "src/unix/index_unix.ml", line 63, characters 4-33
Called from Index.Make_private.unsafe_perform_merge.(fun) in file "src/index.ml", line 842, characters 14-48
Called from Stdlib__fun.protect in file "fun.ml", line 33, characters 8-15
Re-raised at Stdlib__fun.protect in file "fun.ml", line 38, characters 6-52
Called from Index.Make_private.unsafe_perform_merge in file "src/index.ml", line 840, characters 10-1023
Called from Stdlib__fun.protect in file "fun.ml", line 33, characters 8-15
Re-raised at Stdlib__fun.protect in file "fun.ml", line 38, characters 6-52
Called from Index.Make_private.merge'.go in file "src/index.ml", line 910, characters 8-142
Called from Index_unix.Thread.async.protected_f in file "src/unix/index_unix.ml", line 392, characters 29-34
Re-raised at Index_unix.Thread.async.protected_f in file "src/unix/index_unix.ml", line 395, characters 8-17
Called from Thread.create.(fun) in file "thread.ml", line 41, characters 8-14
...
+438578us [DEBUG] index_unix [_tests\unix.main\index_31\index\merge] flushing 40 bytes
Thread 38 killed on uncaught exception Unix.Unix_error(1, "rename", "_tests\unix.main\index_31\index\merge")
Raised by primitive operation at Index_unix.IO.rename in file "src/unix/index_unix.ml", line 63, characters 4-33
Called from Index.Make_private.unsafe_perform_merge.(fun) in file "src/index.ml", line 842, characters 14-48
Called from Stdlib__fun.protect in file "fun.ml", line 33, characters 8-15
Re-raised at Stdlib__fun.protect in file "fun.ml", line 38, characters 6-52
Called from Index.Make_private.unsafe_perform_merge in file "src/index.ml", line 840, characters 10-1023
Called from Stdlib__fun.protect in file "fun.ml", line 33, characters 8-15
Re-raised at Stdlib__fun.protect in file "fun.ml", line 38, characters 6-52
Called from Index.Make_private.merge'.go in file "src/index.ml", line 910, characters 8-142
Called from Index_unix.Thread.async.protected_f in file "src/unix/index_unix.ml", line 392, characters 29-34
Re-raised at Index_unix.Thread.async.protected_f in file "src/unix/index_unix.ml", line 395, characters 8-17
Called from Thread.create.(fun) in file "thread.ml", line 41, characters 8-14

@jonahbeckford jonahbeckford changed the title Add ssize_t typedef for MSVC MSVC compiler support with Win32 pread/pwrite Mar 12, 2022
@jonahbeckford jonahbeckford force-pushed the feature-msvc-ssize_t branch from fdb312e to 00c00ce Compare June 1, 2022 23:58
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.

2 participants