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.

Sign up for GitHub

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

irmin-pack: use supplied ppf for integrity check #2229

Merged
merged 1 commit into from
Mar 29, 2023
Merged

irmin-pack: use supplied ppf for integrity check #2229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 19 additions & 17 deletions src/irmin-pack/unix/checks.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,17 @@ let path =

let deprecated_info = (Cmdliner.Term.info [@alert "-deprecated"])

let ppf_or_null ppf =
let null =
match Sys.os_type with
| "Unix" | "Cygwin" -> "/dev/null"
| "Win32" -> "NUL"
| _ -> invalid_arg "invalid os type"
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps we can use https://ocaml.org/p/ocaml-variants/4.12.0%2Bdomains/doc/Stdlib/Format/index.html#val-make_formatter to define a generic "null" formatter that doesn't depend on the OS?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, yeah I can look into it. This code is just copy-pasta of what already existed. 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

haha yeah I saw that but it still looks icky :P

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe using Logs directly could be what you want ? (and use the associated cli to determine where to print)

Copy link
Member Author

Choose a reason for hiding this comment

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

Since this isn't the goal of this current PR, I opened an issue for future enhancement. #2231

in
match ppf with
| Some p -> p
| None -> open_out null |> Format.formatter_of_out_channel

module Make (Store : Store) = struct
module Hash = Store.Hash
module Index = Pack_index.Make (Hash)
Expand Down Expand Up @@ -177,11 +188,12 @@ module Make (Store : Store) = struct
Conf.init ~readonly:false ~fresh:false ~no_migrate:true ~indexing_strategy
root

let handle_result ?name res =
let handle_result ?ppf ?name res =
let ppf = ppf_or_null ppf in
let name = match name with Some x -> x ^ ": " | None -> "" in
match res with
| Ok (`Fixed n) -> Printf.printf "%sOk -- fixed %d\n%!" name n
| Ok `No_error -> Printf.printf "%sOk\n%!" name
| Ok (`Fixed n) -> Fmt.pf ppf "%sOk -- fixed %d\n%!" name n
| Ok `No_error -> Fmt.pf ppf "%sOk\n%!" name
| Error (`Cannot_fix x) ->
Printf.eprintf "%sError -- cannot fix: %s\n%!" name x
| Error (`Corrupted x) ->
Expand All @@ -203,7 +215,7 @@ module Make (Store : Store) = struct
in
let* result = Store.integrity_check ?ppf ~auto_repair ~heads repo in
let+ () = Store.Repo.close repo in
handle_result ?name:None result
handle_result ?ppf ?name:None result

let heads =
let open Cmdliner.Arg in
Expand Down Expand Up @@ -377,18 +389,8 @@ module Integrity_checks
and type Schema.Hash.t = XKey.hash)
(Index : Pack_index.S) =
struct
let null =
match Sys.os_type with
| "Unix" | "Cygwin" -> "/dev/null"
| "Win32" -> "NUL"
| _ -> invalid_arg "invalid os type"

let set_ppf = function
| Some p -> p
| None -> open_out null |> Format.formatter_of_out_channel

let check_always ?ppf ~auto_repair ~check index =
let ppf = set_ppf ppf in
let ppf = ppf_or_null ppf in
Fmt.pf ppf "Running the integrity_check.\n%!";
let nb_absent = ref 0 in
let nb_corrupted = ref 0 in
Expand Down Expand Up @@ -437,7 +439,7 @@ struct
result

let check_minimal ?ppf ~pred ~iter ~check ~recompute_hash t =
let ppf = set_ppf ppf in
let ppf = ppf_or_null ppf in
Fmt.pf ppf "Running the integrity_check.\n%!";
let errors = ref [] in
let counter, (progress_contents, progress_nodes, progress_commits) =
Expand Down Expand Up @@ -535,7 +537,7 @@ struct
!errors

let check_inodes ?ppf ~iter ~pred ~check t =
let ppf = set_ppf ppf in
let ppf = ppf_or_null ppf in
Fmt.pf ppf "Check integrity for inodes.\n%!";
let counter, (_, progress_nodes, progress_commits) =
Utils.Object_counter.start ppf
Expand Down
1 change: 1 addition & 0 deletions src/irmin-pack/unix/checks_intf.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ module type S = sig
unit Lwt.t

val handle_result :
?ppf:Format.formatter ->
?name:string ->
( [< `Fixed of int | `No_error ],
[< `Cannot_fix of string | `Corrupted of int ] )
Expand Down