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
Show file tree
Hide file tree
Changes from 5 commits
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
1 change: 1 addition & 0 deletions lib_eio/fs.ml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ module Pi = struct
val rename : t -> path -> _ dir -> path -> unit
val read_link : t -> path -> string
val symlink : link_to:path -> t -> path -> unit
val chown : follow:bool -> uid:int64 -> gid:int64 -> t -> path -> unit
val pp : t Fmt.t
val native : t -> string -> string option
end
Expand Down
8 changes: 8 additions & 0 deletions lib_eio/path.ml
Original file line number Diff line number Diff line change
Expand Up @@ -238,3 +238,11 @@ let read_link t =
with Exn.Io _ as ex ->
let bt = Printexc.get_raw_backtrace () in
Exn.reraise_with_context ex bt "reading target of symlink %a" pp t

let chown ~follow ~uid ~gid t =
let (Resource.T (dir, ops), path) = t in
let module X = (val (Resource.get ops Fs.Pi.Dir)) in
try X.chown ~follow ~uid ~gid dir path
with Exn.Io _ as ex ->
let bt = Printexc.get_raw_backtrace () in
Exn.reraise_with_context ex bt "chowing file %a" pp t
patricoferris marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions lib_eio/path.mli
Original file line number Diff line number Diff line change
Expand Up @@ -217,3 +217,6 @@ val symlink : link_to:string -> _ t -> unit
{[
Eio.Path.symlink (dir / "current") ~link_to:"version-1.0"
]} *)

val chown : follow:bool -> uid:int64 -> gid:int64 -> _ t -> unit
(** [chown ~follow ~uid ~gid t] changes the ownership of [t] to be [uid, gid]. *)
3 changes: 3 additions & 0 deletions lib_eio/unix/eio_unix.mli
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ module Private : sig

val read_link : Fd.t option -> string -> string
val read_link_unix : Unix.file_descr option -> string -> string

val chown : flags:int -> uid:int64 -> gid:int64 -> Fd.t -> string -> unit
val chown_unix : flags:int -> uid:int64 -> gid:int64 -> Unix.file_descr -> string -> unit
patricoferris marked this conversation as resolved.
Show resolved Hide resolved
end

module Pi = Pi
1 change: 1 addition & 0 deletions lib_eio/unix/primitives.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,5 @@ CAMLprim value eio_unix_fork_fchdir(value);
CAMLprim value eio_unix_fork_dups(value);
CAMLprim value eio_unix_cap_enter(value);
CAMLprim value eio_unix_readlinkat(value, value, value);
CAMLprim value eio_unix_fchownat(value, value, value, value, value);
CAMLprim value eio_unix_is_blocking(value);
8 changes: 8 additions & 0 deletions lib_eio/unix/private.ml
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,11 @@ let read_link_unix fd path =
aux 1024

let read_link fd path = Fd.use_exn_opt "readlink" fd (fun fd -> read_link_unix fd path)

external eio_fchownat : Unix.file_descr -> string -> int64 -> int64 -> int -> unit = "eio_unix_fchownat"

let chown_unix ~flags ~uid ~gid fd path =
eio_fchownat fd path uid gid flags

let chown ~flags ~uid ~gid fd path =
Fd.use_exn "chown" fd (fun fd -> chown_unix ~uid ~gid ~flags fd path)
21 changes: 21 additions & 0 deletions lib_eio/unix/stubs.c
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,24 @@ CAMLprim value eio_unix_readlinkat(value v_fd, value v_path, value v_cs) {
CAMLreturn(Val_int(ret));
#endif
}

CAMLprim value eio_unix_fchownat(value v_fd, value v_path, value v_uid, value v_gid, value v_flags) {
#ifdef _WIN32
caml_unix_error(EOPNOTSUPP, "fchownat not supported on Windows", v_path);
#else
CAMLparam3(v_path, v_uid, v_gid);
char *path;
int fd = Int_val(v_fd);
int ret;
caml_unix_check_path(v_path, "fchownat");
path = caml_stat_strdup(String_val(v_path));
caml_enter_blocking_section();
ret = fchownat(fd, path, Int64_val(v_uid), Int64_val(v_gid), Int_val(v_flags));
patricoferris marked this conversation as resolved.
Show resolved Hide resolved
caml_leave_blocking_section();
caml_stat_free_preserving_errno(path);
if (ret == -1)
caml_uerror("fchownat", v_path);
CAMLreturn(Val_unit);
#endif
}

3 changes: 3 additions & 0 deletions lib_eio_linux/eio_linux.ml
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,9 @@ end = struct

let read_link t path = Low_level.read_link t.fd path

let chown ~follow ~uid ~gid t path =
Low_level.chown ~follow ~uid ~gid t.fd path

let close t =
match t.fd with
| FD x -> Fd.close x
Expand Down
9 changes: 9 additions & 0 deletions lib_eio_linux/low_level.ml
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,15 @@ let read_link fd path =
Eio_unix.run_in_systhread ~label:"read_link" (fun () -> Eio_unix.Private.read_link (Some parent) leaf)
with Unix.Unix_error (code, name, arg) -> raise @@ Err.wrap_fs code name arg

let chown ~follow ~uid ~gid fd path =
let module At = Uring.Linkat_flags in
let follow = if follow then At.empty_path else At.(empty_path + symlink_follow) in
patricoferris marked this conversation as resolved.
Show resolved Hide resolved
let flags = (follow :> int) in
try
with_parent_dir_fd fd path @@ fun parent leaf ->
Eio_unix.run_in_systhread ~label:"chown" (fun () -> Eio_unix.Private.chown ~flags ~uid ~gid parent leaf)
with Unix.Unix_error (code, name, arg) -> raise @@ Err.wrap_fs code name arg

(* https://www.iana.org/assignments/protocol-numbers/protocol-numbers.xhtml *)
let getaddrinfo ~service node =
let to_eio_sockaddr_t {Unix.ai_family; ai_addr; ai_socktype; ai_protocol; _ } =
Expand Down
6 changes: 6 additions & 0 deletions lib_eio_linux/low_level.mli
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ val rename : dir_fd -> string -> dir_fd -> string -> unit
val symlink : link_to:string -> dir_fd -> string -> unit
(** [symlink ~link_to dir path] creates a new symlink at [dir / path] pointing to [link_to]. *)

val chown : follow:bool -> uid:int64 -> gid:int64 -> dir_fd -> string -> unit
(** [chown ~follow ~uid ~gid dir path] changes the ownership of [dir / path] to [uid, gid].

If [follow = true] and [dir / path] is a symlink, then the ownership of the {e target} is
changed. If it is [false] then the ownership of the symlink itself is changed. *)

val pipe : sw:Switch.t -> fd * fd
(** [pipe ~sw] returns a pair [r, w] with the readable and writeable ends of a new pipe. *)

Expand Down
3 changes: 3 additions & 0 deletions lib_eio_posix/fs.ml
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ end = struct
let symlink ~link_to t path =
Err.run (Low_level.symlink ~link_to t.fd) path

let chown ~follow ~uid ~gid t path =
Err.run (Low_level.chown ~follow ~uid ~gid t.fd) path

let open_dir t ~sw path =
let flags = Low_level.Open_flags.(rdonly + directory +? path) in
let fd = Err.run (Low_level.openat ~sw ~mode:0 t.fd path) flags in
Expand Down
7 changes: 7 additions & 0 deletions lib_eio_posix/low_level.ml
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,13 @@ let read_link dirfd path =
Resolve.with_parent "read_link" dirfd path @@ fun dirfd path ->
Eio_unix.Private.read_link_unix dirfd path

let chown ~follow ~uid ~gid dirfd path =
let flags = if follow then 0 else Config.at_symlink_nofollow in
in_worker_thread "chown" @@ fun () ->
Resolve.with_parent "chown" dirfd path @@ fun dirfd path ->
let dirfd = Option.value dirfd ~default:at_fdcwd in
Eio_unix.Private.chown_unix ~flags ~uid ~gid dirfd path

type stat
external create_stat : unit -> stat = "caml_eio_posix_make_stat"
external eio_fstatat : stat -> Unix.file_descr -> string -> int -> unit = "caml_eio_posix_fstatat"
Expand Down
4 changes: 4 additions & 0 deletions lib_eio_posix/low_level.mli
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ val symlink : link_to:string -> dir_fd -> string -> unit
(** [symlink ~link_to dir path] will create a new symlink at [dir / path]
linking to [link_to]. *)

val chown : follow:bool -> uid:int64 -> gid:int64 -> dir_fd -> string -> unit
(** [chown ~follow ~uid ~gid dir path] will change the ownership of [dir / path]
to [uid, gid]. *)

val readdir : dir_fd -> string -> string array

val readv : fd -> Cstruct.t array -> int
Expand Down
4 changes: 4 additions & 0 deletions lib_eio_windows/fs.ml
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,10 @@ end = struct
with_parent_dir t path @@ fun dirfd path ->
Err.run (Low_level.read_link ?dirfd) path

let chown ~follow ~uid ~gid t path =
with_parent_dir t path @@ fun dirfd path ->
Err.run (fun () -> Low_level.chown ?dirfd ~follow ~uid ~gid path) ()

let rename t old_path new_dir new_path =
match Handler.as_posix_dir new_dir with
| None -> invalid_arg "Target is not an eio_windows directory!"
Expand Down
6 changes: 6 additions & 0 deletions lib_eio_windows/low_level.ml
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,12 @@ let read_link ?dirfd path =
in_worker_thread @@ fun () ->
Eio_unix.Private.read_link dirfd path

let chown ?dirfd ~follow:_ ~uid ~gid path =
in_worker_thread @@ fun () ->
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"

external eio_preadv : Unix.file_descr -> Cstruct.t array -> Optint.Int63.t -> int = "caml_eio_windows_preadv"
Expand Down
1 change: 1 addition & 0 deletions lib_eio_windows/low_level.mli
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ val lstat : string -> Unix.LargeFile.stats

val realpath : string -> string
val read_link : ?dirfd:fd -> string -> string
val chown : ?dirfd:fd -> follow:bool -> uid:int64 -> gid:int64 -> string -> unit

val mkdir : ?dirfd:fd -> ?nofollow:bool -> mode:int -> string -> unit
val unlink : ?dirfd:fd -> dir:bool -> string -> unit
Expand Down
56 changes: 56 additions & 0 deletions tests/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,11 @@ let try_symlink ~link_to path =
match Path.symlink ~link_to path with
| s -> traceln "symlink %a -> %S" Path.pp path link_to
| exception ex -> traceln "@[<h>%a@]" Eio.Exn.pp ex

let try_chown ~follow ~uid ~gid path =
match Path.chown ~follow ~uid ~gid path with
| s -> traceln "chown %a" Path.pp path
| exception ex -> traceln "@[<h>%a@]" Eio.Exn.pp ex
```

# Basic test cases
Expand Down Expand Up @@ -905,6 +910,57 @@ Reading at the end of a file:
- : unit = ()
```

# Changing Ownership

<!-- This test is written a little obscurely as we only want to
run it on macOS, but MDX only supports os_type (Unix, Win, Cygwin) -->
```ocaml
# run @@ fun env ->
let cwd = Eio.Stdenv.cwd env in
let everyone = try Some (Unix.getgrnam "everyone") with Not_found -> None in
let stat_uid_gid path =
let stat = Eio.Path.stat ~follow:false path in
stat.Eio.File.Stat.uid, stat.gid
in
match everyone with
| None -> ()
| Some entry ->
let everybody = Int64.of_int entry.gr_gid in

let path = cwd / "owner.txt" in
let link = cwd / "link.txt" in
let symlink = cwd / "symlink.txt" in

(* Normal chown to everyone group *)
Path.with_open_out path ~create:(`Or_truncate 0o644) ignore;
let uid_before, _gid_before = stat_uid_gid path in
Eio.Path.chown ~follow:false ~uid:uid_before ~gid:everybody path;
let uid_after, gid_after = stat_uid_gid path in
assert (uid_before = uid_after);
assert (everybody = gid_after);

(* Chowning through a symlink *)
Path.with_open_out link ~create:(`Or_truncate 0o644) ignore;
Eio.Path.symlink ~link_to:"link.txt" symlink;
let luid1, lgid1 = stat_uid_gid link in
let suid1, sgid1 = stat_uid_gid symlink in

(* Only chown the symlink, not the target *)
Eio.Path.chown ~follow:false ~uid:uid_before ~gid:everybody symlink;
let luid2, lgid2 = stat_uid_gid link in
let suid2, sgid2 = stat_uid_gid symlink in
assert (luid1 = luid2 && lgid1 = lgid2 && suid1 = suid2 && Int64.to_int sgid2 = entry.gr_gid);

(* Now chown the target too *)
Eio.Path.chown ~follow:true ~uid:uid_before ~gid:everybody symlink;
let luid3, lgid3 = stat_uid_gid link in
let suid3, sgid3 = stat_uid_gid symlink in
assert (suid3 = luid3);
assert (Int64.to_int lgid3 = entry.gr_gid);
assert (Int64.to_int sgid3 = entry.gr_gid);;
- : unit = ()
```

# Cancelling while readable

Ensure reads can be cancelled promptly, even if there is no need to wait:
Expand Down
Loading