Thread (191 messages) flat view 191 messages, 12 authors, 2022-02-03
STALE1684d

Revision v5 of 6 in this series.

Revisions (6)
  1. v1 [diff vs current]
  2. v2 [diff vs current]
  3. v3 [diff vs current]
  4. v4 [diff vs current]
  5. v5 current
  6. v6 [diff vs current]

[PATCH v5 0/8] progress: assert "global_progress" + test fixes / cleanup

From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-11-01 19:09:21

This is a commit-message only update of v4[1]. It was pointed out on
v4 that linked-to POC code discussed in the commit message wasn't safe
for running inside a signal handler.

That's correct, but achieving the same goals would be possible while
retaining signal safety on some platforms, so update the commit
message discussion to note that.

In any case that part of the commit message is discussing future
side-benefits of the change, the change as it is isn't doing anything
novel with signal handlers in the progress.c code.

1. https://lore.kernel.org/git/cover-v4-0.8-00000000000-20211025T111915Z-avarab@gmail.com/ (local)
2. https://lore.kernel.org/git/c65c0a54-b66f-a043-01d9-192f9ca1efde@kdbg.org/ (local)

Ævar Arnfjörð Bjarmason (8):
  leak tests: fix a memory leaks in "test-progress" helper
  progress.c test helper: add missing braces
  progress.c tests: make start/stop commands on stdin
  progress.c tests: test some invalid usage
  progress.c: add temporary variable from progress struct
  pack-bitmap-write.c: don't return without stop_progress()
  various *.c: use isatty(1|2), not isatty(STDIN_FILENO|STDERR_FILENO)
  progress.c: add & assert a "global_progress" variable

 builtin/bisect--helper.c    |   2 +-
 builtin/bundle.c            |   2 +-
 compat/mingw.c              |   2 +-
 pack-bitmap-write.c         |   6 +--
 progress.c                  |  23 +++++++-
 t/helper/test-progress.c    |  52 +++++++++++++-----
 t/t0500-progress-display.sh | 105 ++++++++++++++++++++++++++++--------
 7 files changed, 150 insertions(+), 42 deletions(-)

Range-diff against v4:
1:  a3bd032d1eb = 1:  0c0e5bfc656 leak tests: fix a memory leaks in "test-progress" helper
2:  e441cfea7c5 = 2:  0ae86e40e16 progress.c test helper: add missing braces
3:  1c5f9bdfe6d = 3:  767aa010026 progress.c tests: make start/stop commands on stdin
4:  474ce31f9d2 = 4:  9bcc8cfc9d2 progress.c tests: test some invalid usage
5:  ff039742148 = 5:  3a95613cd04 progress.c: add temporary variable from progress struct
6:  3dfe31decff = 6:  0300b802dcb pack-bitmap-write.c: don't return without stop_progress()
7:  8a18eb40fae = 7:  4795d4835b0 various *.c: use isatty(1|2), not isatty(STDIN_FILENO|STDERR_FILENO)
8:  06124e8ac5e ! 8:  e58f4a0c1b9 progress.c: add & assert a "global_progress" variable
    @@ Commit message
     
         It will also establish scaffolding to address current fundamental
         limitations in the progress output: The current output must be
    -    "driven" by calls to the likes of display_progress(). Once we have a
    -    global current progress object we'll be able to update that object via
    -    SIGALRM, this will cover cases where we're busy, but either haven't
    -    invoked our first display_progress() yet, or the time between
    -    display_progress() is too long. See [3] for early code to do that.
    +    "driven" by calls to the likes of display_progress().
    +
    +    Once we have a global current progress object we'll be able to update
    +    that object via SIGALRM, this will cover cases where we're busy, but
    +    either haven't invoked our first display_progress() yet, or the time
    +    between display_progress() is too long. See [3] for early code to do
    +    that.
    +
    +    The linked code in [3] is WIP and not signal-safe since among other
    +    things it calls sprintf() from within a signal handler, see e.g. "man
    +    7 signal-safety". But on some platforms a real implementation of it
    +    would be able to write() out a prepared-formatted progress update from
    +    within a signal handler. That would be sufficient to e.g. show that
    +    we're "stalled", or to display something like a simple pre-formatted
    +    "spinner".
     
         It's conceivable that this change will hit the BUG() condition in some
         scenario that we don't currently have tests for, this would be very
-- 
2.33.1.1570.g069344fdd45
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help