Skip to content

Commit 90d6814

Browse files
authored
CA-423576: fix cli_progress_bar crashes (#6892)
cli_progress_bar is used by `xe --progress`, and I've reused it in my test code in #6858. However >90% of my test runs failed on various machines due to a `String.blit` exception from `cli_progress_bar`. There are 2 possible reasons, not sure which one caused the failure, but I've fixed both, and now I have a lot more green tests (and the failures are due to actual bugs in the product, not bugs in the progress bar): * if the ETA printed would be >99h (even just temporarily) then we'd overflow the buffer's size and raise an exception. `%02d` means at least 2 digits, not at most! * if time goes backwards then we'd get a negative ETA and try to print a `-` and overflow the buffer size again and raise an exception. Replaced it with monotonic time This also contains an improvement I've made on the other PR to print total time in `ms` (to avoid having to solve rebase conflicts twice in the 2 PRs). This avoids printing awkward looking lines like Total time 00:00:00, when it actually took 0.9s maybe.
2 parents 2a7acb5 + b9cd1e5 commit 90d6814

2 files changed

Lines changed: 27 additions & 6 deletions

File tree

ocaml/xapi-cli-server/cli_progress_bar.ml

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ module Make (T : Floatable) = struct
2525
; width: int
2626
; line: bytes
2727
; mutable spin_index: int
28-
; start_time: float
28+
; start_time: Mtime_clock.counter
2929
; mutable summarised: bool
3030
}
3131

@@ -44,7 +44,7 @@ module Make (T : Floatable) = struct
4444
String.blit prefix_s 0 line 0 prefix ;
4545
String.blit suffix_s 0 line (width - suffix - 1) suffix ;
4646
let spin_index = 0 in
47-
let start_time = Unix.gettimeofday () in
47+
let start_time = Mtime_clock.counter () in
4848
{
4949
max_value
5050
; current_value
@@ -55,6 +55,9 @@ module Make (T : Floatable) = struct
5555
; summarised= false
5656
}
5757

58+
let elapsed t =
59+
1e-9 *. (Mtime_clock.count t.start_time |> Mtime.Span.to_float_ns)
60+
5861
let percent t =
5962
int_of_float T.(to_float t.current_value /. to_float t.max_value *. 100.)
6063

@@ -70,10 +73,20 @@ module Make (T : Floatable) = struct
7073
let h = secs / 3600 in
7174
let m = secs mod 3600 / 60 in
7275
let s = secs mod 60 in
73-
Printf.sprintf "%02d:%02d:%02d" h m s
76+
let str = Printf.sprintf "%02d:%02d:%02d" h m s in
77+
if String.length str > 8 then
78+
(* negative or > 99 hours *)
79+
let str = Printf.sprintf "%05d:%02d" h m in
80+
if String.length str > 8 then
81+
(* still too long, >11 years *)
82+
"++:++:++"
83+
else
84+
str
85+
else
86+
str
7487

7588
let eta t =
76-
let time_so_far = Unix.gettimeofday () -. t.start_time in
89+
let time_so_far = elapsed t in
7790
let total_time =
7891
T.(to_float t.max_value /. to_float t.current_value) *. time_so_far
7992
in
@@ -108,8 +121,8 @@ module Make (T : Floatable) = struct
108121
let summarise t =
109122
if not t.summarised then (
110123
t.summarised <- true ;
111-
Printf.sprintf "Total time: %s\n"
112-
(hms (int_of_float (Unix.gettimeofday () -. t.start_time)))
124+
Format.asprintf "Total time: %a@." Mtime.Span.pp
125+
(Mtime_clock.count t.start_time)
113126
) else
114127
""
115128
end

ocaml/xapi-cli-server/dune

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,16 @@
88
(run %{gen} utils --filter-internal --filter closed)))
99
)
1010

11+
(library
12+
(name cli_progress_bar)
13+
(modules cli_progress_bar)
14+
(libraries mtime mtime.clock.os)
15+
)
16+
1117
(library
1218
(name xapi_cli_server)
1319
(modes best)
20+
(modules (:standard \ cli_progress_bar))
1421
(libraries
1522
astring
1623
base64
@@ -37,6 +44,7 @@
3744
xapi-client
3845
xapi-cli-protocol
3946
xapi_aux
47+
cli_progress_bar
4048
clock
4149
xapi-stdext-pervasives
4250
xapi-stdext-std

0 commit comments

Comments
 (0)