Support chown in eio_posix and eio_linux#781
Conversation
patricoferris
force-pushed
the
fchownat
branch
from
dbe9a44 to
9808d13
Compare
talex5
left a comment
talex5
left a comment
There was a problem hiding this comment.
Thanks!
Rebasing on main should fix the OpenBSD CI error.
| let chown ?dirfd ~follow:_ ~uid ~gid path = | ||
| in_worker_thread @@ fun () -> | ||
| match dirfd with | ||
| | None -> failwith "Chown is unsupported on Windows" |
There was a problem hiding this comment.
Could just use Unix.chown here.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I think it's OK to keep it. We might want a proper operation-not-supported error at some point.
patricoferris
force-pushed
the
fchownat
branch
from
c1919aa to
4dd2863
Compare
|
BTW, I was planning to make a new Eio release tomorrow. Do you want me to delay that to get this in, or should I go ahead and merge this later? |
|
Oops sorry -- cut the release, no rush on this :)) |
talex5
left a comment
talex5
left a comment
There was a problem hiding this comment.
I squashed and rebased this. Just wondering if the uid and gid should be optional.
| val chown : follow:bool -> uid:int64 -> gid:int64 -> _ t -> unit | ||
| (** [chown ~follow ~uid ~gid t] changes the ownership of [t] to be [uid, gid]. *) |
There was a problem hiding this comment.
Need to document follow. Also, fchown says "If the owner or group is specified as -1, then that ID is not changed". So maybe:
| val chown : follow:bool -> uid:int64 -> gid:int64 -> _ t -> unit | |
| (** [chown ~follow ~uid ~gid t] changes the ownership of [t] to be [uid, gid]. *) | |
| val chown : follow:bool -> ?uid:int64 -> ?gid:int64 -> _ t -> unit | |
| (** [chown ~follow ~uid ~gid t] changes the ownership of [t] to be [uid, gid]. | |
| [uid] or [gid] can be [None] to leave the current value unchanged. | |
| @param follow If [t] is a symbolic link, change the permission of its target. *) |
There was a problem hiding this comment.
Having a means to update only one of them seems sensible to me, thanks for taking a look!
2 participants
This PR adds support to Eio for changing the ownership of a path using
fchownat.