CA-423576: fix cli_progress_bar crashes#6892
Merged
edwintorok merged 4 commits intoxapi-project:masterfrom Feb 5, 2026
Merged
Conversation
last-genius
approved these changes
Feb 5, 2026
This will be reused by some test code. Signed-off-by: Edwin Török <edwin.torok@citrix.com>
Got some `String.blit` exceptions, which could've happened if the calculated time was negative, and thus exceeded 10 digits when printed with `eta`. Use monotonic time instead, so that we don't crash when the clock is adjusted. Signed-off-by: Edwin Török <edwin.torok@citrix.com>
The ETA is copied into a preallocated bytes buffer of fixed length. When the ETA exceeds 99h then drop the seconds and report just `hh:mm`. If it still doesn't fit then replace it with a static string '++:++:++' (this would only happen if the ETA is >11 years, although it could happen if an operation became stuck. The operation could still recover and then the ETA will print a number). This could also happen if a task makes near 0 progress by the time we attempt to print progress, which would result in a near infinite ETA. Signed-off-by: Edwin Török <edwin.torok@citrix.com>
Currently total time is printed as `hh:mm:ss`, and that will be 00:00:00 if an operation takes <1s. But that is confusing when an operation takes 0.9s for example. Using Mtime.Span.pp instead, which prints values at an appropriate scale based on their magnitude. Signed-off-by: Edwin Török <edwin.torok@citrix.com>
edwintorok
force-pushed
the
private/edvint/progress-fix
branch
from
793d5f4 to
b9cd1e5
Compare
psafont
approved these changes
Feb 5, 2026
Member
psafont
left a comment
psafont
left a comment
There was a problem hiding this comment.
I'm surprised by the amount of custom code to print the time, there's possibility for further enhancements here
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.blitexception fromcli_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):
%02dmeans at least 2 digits, not at most!-and overflow the buffer size again and raise an exception. Replaced it with monotonic timeThis 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.