mbox series

[0/2] enable progress traces even when quiet

Message ID 20200710014242.1088216-1-emilyshaffer@google.com (mailing list archive)
Headers show
Series enable progress traces even when quiet | expand

Message

Emily Shaffer July 10, 2020, 1:42 a.m. UTC
This is a simple but large-scale change, because it changes an API and
fixes all callers. The bulk of the change is in the first patch,
including justification in the commit message; the second patch is only
tidying I noticed while fixing compiler errors from the former. If that
churn is unwanted I'm happy to drop it.

I wanted to do this change because we received reports that 'git clone'
was only logging traces sometimes, not all the time. An alternative
approach would have been to move the 'if (verbose)' wrappers to surround
calls to 'display_progress()' - but this would have worked in
builtin/index-pack.c (where the reported bug was happening) only because
of that code's use of static global flags. Placing the onus on the
caller to ensure that trace collection occurs, even in quiet mode, is
flimsy and error-prone. Adding another flag to 'struct progress' ensures
that trace collection is consistent regardless of how the caller
organizes their code.

In a couple of cases, this does result in some additional work
calculating titles and object counts where previously there wasn't;
however, since those values will be used by the trace logger, I don't
consider it a waste of resources.

Finally, 'git rev-list' has one weird case where it takes the progress
title as an argument (i.e. 'git rev-list --progress "Working hard" ...')
and also null-checks that string to decide whether to calle
'start_progress()' - in this case, I added a placeholder text for the
traces to show. Bikeshedding welcome here.

 - Emily

Emily Shaffer (2):
  progress: create progress struct in 'verbose' mode
  progress: remove redundant null-checking

 builtin/blame.c             |   4 +-
 builtin/commit-graph.c      |   8 +-
 builtin/fsck.c              |  25 +++----
 builtin/index-pack.c        |  16 ++--
 builtin/log.c               |   4 +-
 builtin/pack-objects.c      |  18 ++---
 builtin/prune.c             |   4 +-
 builtin/rev-list.c          |   9 ++-
 builtin/unpack-objects.c    |   3 +-
 commit-graph.c              | 143 +++++++++++++++++-------------------
 delta-islands.c             |   4 +-
 diffcore-rename.c           |   9 +--
 entry.c                     |   2 +-
 midx.c                      |  43 +++++------
 pack-bitmap-write.c         |   8 +-
 pack-bitmap.c               |   5 +-
 preload-index.c             |   7 +-
 progress.c                  |  27 ++++---
 progress.h                  |  10 ++-
 prune-packed.c              |   4 +-
 read-cache.c                |  15 ++--
 t/helper/test-progress.c    |   6 +-
 t/t0500-progress-display.sh |  27 +++++++
 unpack-trees.c              |  14 ++--
 walker.c                    |   4 +-
 25 files changed, 217 insertions(+), 202 deletions(-)