Skip to content

Commit

Permalink
Simplify code to un-shadow the stdlib Unix module
Browse files Browse the repository at this point in the history
Summary:
Per feedback from Pradeep, the previous code to save the stdlib Unix module to a
temporary name, then restore the shadowed-by-Core module after was confusing.

This diff takes a different approach, which saves the original module to
CamlUnix, but does not then restore the "original" Unix module and just leaves
Core's override.

Reviewed By: connernilsen

Differential Revision: D50150099

fbshipit-source-id: 1af14175138658c2ef0841087758d606c3832950
  • Loading branch information
samwgoldman authored and facebook-github-bot committed Oct 11, 2023
1 parent fad827f commit 6c792f3
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 52 deletions.
8 changes: 4 additions & 4 deletions source/hack_parallel/hack_parallel/utils/hh_logger.ml
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@
(* TODO(T132410158) Add a module-level doc comment. *)


module Caml_unix = Unix
(* Core shadows/deprecates the stdlib Unix module. *)
module CamlUnix = Unix
open Core
module Unix = Caml_unix

let timestamp_string () =
let open Unix in
let open CamlUnix in
let tm = localtime (time ()) in
let year = tm.tm_year + 1900 in
Printf.sprintf "[%d-%02d-%02d %02d:%02d:%02d]"
Expand All @@ -39,7 +39,7 @@ let print fmt = Printf.ksprintf print_raw (fmt^^"\n")

let print_duration name t =
print_raw (name ^ ": ");
let t2 = Unix.gettimeofday() in
let t2 = CamlUnix.gettimeofday() in
Printf.eprintf "%f\n%!" (t2 -. t);
t2

Expand Down
9 changes: 4 additions & 5 deletions source/log.ml
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@
prefixing log output with an appropriate section name *)

(* Core shadows/deprecates the stdlib Unix module. *)
module Caml_unix = Unix
module CamlUnix = Unix
open Core
module Unix = Caml_unix

type section =
[ `Check
Expand Down Expand Up @@ -96,7 +95,7 @@ end
let is_enabled section = Hash_set.mem GlobalState.enabled (section_to_string section)

let format_tm fmt tm =
let open Unix in
let open CamlUnix in
let year = tm.tm_year + 1900 in
let month = tm.tm_mon + 1 in
Format.fprintf
Expand All @@ -113,7 +112,7 @@ let format_tm fmt tm =
let log ~section format =
let section = section_to_string section in
if Hash_set.mem GlobalState.enabled section then
let localtime = Unix.localtime (Unix.time ()) in
let localtime = CamlUnix.localtime (CamlUnix.time ()) in
Format.fprintf
Format.err_formatter
("%a %s " ^^ format ^^ "\n%!")
Expand All @@ -140,7 +139,7 @@ let log_unix_error ?(section = `Error) (error_kind, name, parameters) =
log
~section
"Unix error %s: %s(%s)"
(Unix.error_message error_kind [@alert "-deprecated"])
(CamlUnix.error_message error_kind [@alert "-deprecated"])
name
parameters

Expand Down
51 changes: 25 additions & 26 deletions source/pyrePath.ml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,8 @@
(* TODO(T132410158) Add a module-level doc comment. *)

(* Core shadows/deprecates the stdlib Unix module. *)
module Caml_unix = Unix
module CamlUnix = Unix
open Core
module Unix = Caml_unix

type path = string [@@deriving compare, show, sexp, hash]

Expand All @@ -29,7 +28,7 @@ let absolute = function

let create_absolute ?(follow_symbolic_links = false) path =
if follow_symbolic_links then
Absolute (Unix.realpath path)
Absolute (CamlUnix.realpath path)
else
Absolute path

Expand Down Expand Up @@ -59,7 +58,7 @@ let create_directory_recursively ?(permission = 0o777) path =
match do_create (Filename.dirname path) with
| Result.Error _ as error -> error
| Result.Ok () ->
Unix.mkdir path permission;
CamlUnix.mkdir path permission;
Result.Ok ())
| path when Caml.Sys.is_directory path -> Result.Ok ()
| path ->
Expand Down Expand Up @@ -107,9 +106,9 @@ let is_path_python_init path =


let rec stat_no_eintr_raw path =
try Some (Unix.stat path) with
| Unix.Unix_error (Unix.EINTR, _, _) -> stat_no_eintr_raw path
| Unix.Unix_error ((Unix.ENOENT | Unix.ENOTDIR), _, _) -> None
try Some (CamlUnix.stat path) with
| CamlUnix.Unix_error (CamlUnix.EINTR, _, _) -> stat_no_eintr_raw path
| CamlUnix.Unix_error ((CamlUnix.ENOENT | CamlUnix.ENOTDIR), _, _) -> None


let stat_no_eintr path = stat_no_eintr_raw (absolute path)
Expand All @@ -118,7 +117,7 @@ let file_exists path = Option.is_some (stat_no_eintr path)

let directory_exists path =
match stat_no_eintr path with
| Some { Unix.st_kind = Unix.S_DIR; _ } -> true
| Some { CamlUnix.st_kind = CamlUnix.S_DIR; _ } -> true
| _ -> false


Expand All @@ -129,7 +128,7 @@ let last path =

let follow_symbolic_link path =
try absolute path |> create_absolute ~follow_symbolic_links:true |> Option.some with
| Unix.Unix_error _ -> None
| CamlUnix.Unix_error _ -> None


(* Variant of Sys.readdir where names are sorted in alphabetical order *)
Expand All @@ -142,7 +141,7 @@ let read_directory_ordered_raw path =
let list ?(file_filter = fun _ -> true) ?(directory_filter = fun _ -> true) ~root () =
let rec list sofar path =
match stat_no_eintr_raw path with
| Some { Unix.st_kind = Unix.S_DIR; _ } ->
| Some { CamlUnix.st_kind = CamlUnix.S_DIR; _ } ->
if directory_filter path then (
match read_directory_ordered_raw path with
| entries ->
Expand Down Expand Up @@ -182,8 +181,8 @@ end
let search_upwards ~target ~target_type ~root =
let rec directory_has_target directory =
match target_type, stat_no_eintr_raw (directory ^/ target) with
| FileType.Directory, Some { Unix.st_kind = Unix.S_DIR; _ }
| FileType.File, Some { Unix.st_kind = Unix.S_REG; _ } ->
| FileType.Directory, Some { CamlUnix.st_kind = CamlUnix.S_DIR; _ }
| FileType.File, Some { CamlUnix.st_kind = CamlUnix.S_REG; _ } ->
Some (create_absolute directory)
| _ ->
let parent_directory = Filename.dirname directory in
Expand All @@ -196,34 +195,34 @@ let search_upwards ~target ~target_type ~root =
let unlink_if_exists path =
try Unix.unlink (absolute path) with
| Unix.Unix_error ((Unix.ENOENT | Unix.ENOTDIR), _, _) -> ()
try CamlUnix.unlink (absolute path) with
| CamlUnix.Unix_error ((CamlUnix.ENOENT | CamlUnix.ENOTDIR), _, _) -> ()
let remove_recursively path =
let rec do_remove path =
try
let stats = Unix.lstat path in
match stats.Unix.st_kind with
| Unix.S_DIR ->
let stats = CamlUnix.lstat path in
match stats.CamlUnix.st_kind with
| CamlUnix.S_DIR ->
let contents = Caml.Sys.readdir path in
List.iter (Array.to_list contents) ~f:(fun name ->
let name = Filename.concat path name in
do_remove name);
Unix.rmdir path
| Unix.S_LNK
| Unix.S_REG
| Unix.S_CHR
| Unix.S_BLK
| Unix.S_FIFO
| Unix.S_SOCK ->
Unix.unlink path
CamlUnix.rmdir path
| CamlUnix.S_LNK
| CamlUnix.S_REG
| CamlUnix.S_CHR
| CamlUnix.S_BLK
| CamlUnix.S_FIFO
| CamlUnix.S_SOCK ->
CamlUnix.unlink path
with
(* Path has been deleted out from under us - can ignore it. *)
| Sys_error message ->
Log.warning "Error occurred when removing %s recursively: %s" path message;
()
| Unix.Unix_error (Unix.ENOENT, _, _) -> ()
| CamlUnix.Unix_error (CamlUnix.ENOENT, _, _) -> ()
in
do_remove (absolute path)
Expand Down
6 changes: 3 additions & 3 deletions source/pyreProfiling.ml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@

(* Profiling logic for internal Pyre behaviour. *)

module Caml_unix = Unix
(* Core shadows/deprecates the stdlib Unix module. *)
module CamlUnix = Unix
open Core
module Unix = Caml_unix
module Worker = Hack_parallel.Std.Worker
module SharedMemory = Hack_parallel.Std.SharedMemory

Expand Down Expand Up @@ -61,7 +61,7 @@ module Event = struct

let create ?(timestamp = now_in_microseconds ()) ?(tags = []) ~event_type name =
let name = String.filter ~f:Char.is_print name in
let pid = Unix.getpid () in
let pid = CamlUnix.getpid () in
let worker_id = Worker.current_worker_id () in
{ name; worker_id; pid; event_type; timestamp; tags }
end
Expand Down
26 changes: 12 additions & 14 deletions source/service/memory.ml
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@

(* TODO(T132410158) Add a module-level doc comment. *)

module Caml_unix = Unix
(* Core shadows/deprecates the stdlib Unix module. *)
module CamlUnix = Unix
open Core
module Unix = Caml_unix
module Gc = Caml.Gc
module Set = Caml.Set
module SharedMemory = Hack_parallel.Std.SharedMemory
Expand Down Expand Up @@ -140,9 +140,9 @@ let prepare_saved_state_directory { Configuration.Analysis.log_directory; _ } =
let table_path = PyrePath.create_relative ~root ~relative:"table" in
let dependencies_path = PyrePath.create_relative ~root ~relative:"deps" in
let () =
try Unix.mkdir (PyrePath.absolute root) 0o777 with
try CamlUnix.mkdir (PyrePath.absolute root) 0o777 with
(* [mkdir] on MacOSX returns [EISDIR] instead of [EEXIST] if the directory already exists. *)
| Unix.Unix_error ((EEXIST | EISDIR), _, _) ->
| CamlUnix.Unix_error ((EEXIST | EISDIR), _, _) ->
PyrePath.unlink_if_exists table_path;
PyrePath.unlink_if_exists dependencies_path
| e -> raise e
Expand All @@ -151,23 +151,21 @@ let prepare_saved_state_directory { Configuration.Analysis.log_directory; _ } =


let rec waitpid_no_eintr flags pid =
try Unix.waitpid flags pid with
| Unix.Unix_error (Unix.EINTR, _, _) -> waitpid_no_eintr flags pid
try CamlUnix.waitpid flags pid with
| CamlUnix.Unix_error (EINTR, _, _) -> waitpid_no_eintr flags pid


let run_tar arguments =
let open Unix in
let in_read, in_write = Unix.pipe ~cloexec:true () in
let out_read, out_write = Unix.pipe ~cloexec:true () in
let err_read, err_write = Unix.pipe ~cloexec:true () in
let pid =
Unix.create_process "tar" (Array.of_list ("tar" :: arguments)) in_read out_write err_write
in
let open CamlUnix in
let in_read, in_write = pipe ~cloexec:true () in
let out_read, out_write = pipe ~cloexec:true () in
let err_read, err_write = pipe ~cloexec:true () in
let pid = create_process "tar" (Array.of_list ("tar" :: arguments)) in_read out_write err_write in
List.iter ~f:close [in_read; out_write; err_write];
let _, process_status = waitpid_no_eintr [] pid in
List.iter ~f:close [in_write; out_read; err_read];
match process_status with
| Unix.WEXITED 0 -> ()
| WEXITED 0 -> ()
| _ ->
raise
(TarError
Expand Down

0 comments on commit 6c792f3

Please sign in to comment.