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

Support chown in eio_posix and eio_linux #781

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix eio_windows
patricoferris committed Nov 20, 2024
commit 9808d1310dd205baa69dccfae2f3e21585d1a8d7
4 changes: 3 additions & 1 deletion lib_eio_windows/low_level.ml
Original file line number Diff line number Diff line change
@@ -140,7 +140,9 @@ let read_link ?dirfd path =

let chown ?dirfd ~follow:_ ~uid ~gid path =
in_worker_thread @@ fun () ->
Eio_unix.Private.chown ~flags:0 ~uid ~gid dirfd path
match dirfd with
| None -> failwith "Chown is unsupported on Windows"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could just use Unix.chown here.

Copy link
Collaborator Author

@patricoferris patricoferris Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I'm wondering whether chown should be in the cross-platform API at all.

Unix.chown isn't implemented on Windows:
https://github.com/ocaml/ocaml/blob/c484c6932fa2eae03ba0f5a7dbdb26e3eee65eb0/otherlibs/unix/unix_win32.ml#L450 -- it's not implemented in python's os.chown either?

Do we have a rule for what makes it into the cross-platform API and what doesn't ? I mean we already have File.Unix_perm.t.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's OK to keep it. We might want a proper operation-not-supported error at some point.

| Some dirfd -> Eio_unix.Private.chown ~flags:0 ~uid ~gid dirfd path

external eio_readv : Unix.file_descr -> Cstruct.t array -> int = "caml_eio_windows_readv"