[PATCH v2 0/1] commit-graph: add --[no-]progress to write and verify

["Garima Singh via GitGitGadget" <gitgitgadget@gmail.com>]

Hey Git contributors! 

My name is Garima Singh and I work at Microsoft. I recently started working
closely with the Microsoft team contributing to the git client ecosystem. I
am very glad to have the opportunity to work with this community. I am new
to the world of git client development but I did work on the Git service
offering of Azure Developer Services for a few years. I am sure I will get
to learn a lot from all of you. 

Dr. Derrick Stolee helped me pick out my first task (Thanks Stolee!) He
mentioned an issue in the commit-graph builtin where git did not support
opting in and out of the progress output. This was bloating up the stderr
logs in VFS for Git. The progress feature was introduced in 7b0f229222
("commit-graph write: add progress output", 2018-09-17) but the ability to
opt-out was overlooked. This patch adds the --no-progress option so that
callers can control the amount of logging they receive. 

Looking forward to your review. Cheers! Garima Singh

CC: stolee@gmail.com, avarab@gmail.com, garimasigit@gmail.com

Garima Singh (1):
  commit-graph: add --[no-]progress to write and verify.

 Documentation/git-commit-graph.txt |  7 ++++--
 builtin/commit-graph.c             | 21 ++++++++++++-----
 commit-graph.c                     |  6 +++--
 t/t5318-commit-graph.sh            | 36 ++++++++++++++++++++++++++++++
 t/t5324-split-commit-graph.sh      |  2 +-
 5 files changed, 61 insertions(+), 11 deletions(-)


base-commit: 745f6812895b31c02b29bdfe4ae8e5498f776c26
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-315%2Fgarimasi514%2FcoreGit-commit-graph-progress-toggle-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-315/garimasi514/coreGit-commit-graph-progress-toggle-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/315

Range-diff vs v1:

 1:  da89f7dadb ! 1:  47cc99bd15 commit-graph: add --[no-]progress to write and verify.
     @@ -17,16 +17,19 @@
       [verse]
       'git commit-graph read' [--object-dir <dir>]
      -'git commit-graph verify' [--object-dir <dir>] [--shallow]
     +-'git commit-graph write' <options> [--object-dir <dir>]
      +'git commit-graph verify' [--object-dir <dir>] [--shallow] [--[no-]progress]
     - 'git commit-graph write' <options> [--object-dir <dir>]
     ++'git commit-graph write' <options> [--object-dir <dir>] [--[no-]progress]
       
       
     + DESCRIPTION
      @@
       	commit-graph file is expected to be in the `<dir>/info` directory and
       	the packfiles are expected to be in `<dir>/pack`.
       
      +--[no-]progress::
     -+	Toggle whether to show progress or not.
     ++	Turn progress on/off explicitly. If neither is specified, progress is 
     ++	shown if standard error is connected to a terminal.
       
       COMMANDS
       --------
     @@ -35,11 +38,6 @@
       --- a/builtin/commit-graph.c
       +++ b/builtin/commit-graph.c
      @@
     - #include "repository.h"
     - #include "commit-graph.h"
     - #include "object-store.h"
     -+#include "unistd.h"
     - 
       static char const * const builtin_commit_graph_usage[] = {
       	N_("git commit-graph [--object-dir <objdir>]"),
       	N_("git commit-graph read [--object-dir <objdir>]"),
     @@ -74,15 +72,6 @@
       
       static int graph_verify(int argc, const char **argv)
      @@
     - 	int fd;
     - 	struct stat st;
     - 	int flags = 0;
     --
     -+	int defaultProgressState = isatty(2);
     -+	
     - 	static struct option builtin_commit_graph_verify_options[] = {
     - 		OPT_STRING(0, "object-dir", &opts.obj_dir,
     - 			   N_("dir"),
       			   N_("The object directory to store the graph")),
       		OPT_BOOL(0, "shallow", &opts.shallow,
       			 N_("if the commit-graph is split, only verify the tip file")),
     @@ -90,8 +79,7 @@
       		OPT_END(),
       	};
       
     -+	opts.progress = defaultProgressState;
     -+	
     ++	opts.progress = isatty(2);
       	argc = parse_options(argc, argv, NULL,
       			     builtin_commit_graph_verify_options,
       			     builtin_commit_graph_verify_usage, 0);
     @@ -101,7 +89,7 @@
       		flags |= COMMIT_GRAPH_VERIFY_SHALLOW;
      -
      +	if (opts.progress)
     -+		flags |= COMMIT_GRAPH_PROGRESS;
     ++		flags |= COMMIT_GRAPH_WRITE_PROGRESS;
      +	
       	graph_name = get_commit_graph_filename(opts.obj_dir);
       	open_ok = open_commit_graph(graph_name, &fd, &st);
     @@ -110,14 +98,11 @@
       	struct string_list *commit_hex = NULL;
       	struct string_list lines;
       	int result = 0;
     --	unsigned int flags = COMMIT_GRAPH_PROGRESS;
     --
     -+	unsigned int flags = 0;
     -+	int defaultProgressState = isatty(2);
     -+	
     +-	enum commit_graph_write_flags flags = COMMIT_GRAPH_WRITE_PROGRESS;
     ++	enum commit_graph_write_flags flags = 0;
     + 
       	static struct option builtin_commit_graph_write_options[] = {
       		OPT_STRING(0, "object-dir", &opts.obj_dir,
     - 			N_("dir"),
      @@
       			N_("start walk at commits listed by stdin")),
       		OPT_BOOL(0, "append", &opts.append,
     @@ -130,16 +115,16 @@
       		OPT_END(),
       	};
       
     -+	opts.progress = defaultProgressState;
     ++	opts.progress = isatty(2);
       	split_opts.size_multiple = 2;
       	split_opts.max_commits = 0;
       	split_opts.expire_time = 0;
      @@
     - 		flags |= COMMIT_GRAPH_APPEND;
     + 		flags |= COMMIT_GRAPH_WRITE_APPEND;
       	if (opts.split)
     - 		flags |= COMMIT_GRAPH_SPLIT;
     + 		flags |= COMMIT_GRAPH_WRITE_SPLIT;
      +	if (opts.progress)
     -+		flags |= COMMIT_GRAPH_PROGRESS;
     ++		flags |= COMMIT_GRAPH_WRITE_PROGRESS;
       
       	read_replace_refs = 0;
       
     @@ -153,20 +138,13 @@
       
      -	progress = start_progress(_("Verifying commits in commit graph"),
      -				  g->num_commits);
     -+	if (flags & COMMIT_GRAPH_PROGRESS)
     ++	if (flags & COMMIT_GRAPH_WRITE_PROGRESS)
      +		progress = start_progress(_("Verifying commits in commit graph"),
      +					g->num_commits);
      +
       	for (i = 0; i < g->num_commits; i++) {
       		struct commit *graph_commit, *odb_commit;
       		struct commit_list *graph_parents, *odb_parents;
     - 		uint32_t max_generation = 0;
     - 
     - 		display_progress(progress, i + 1);
     -+
     - 		hashcpy(cur_oid.hash, g->chunk_oid_lookup + g->hash_len * i);
     - 
     - 		graph_commit = lookup_commit(r, &cur_oid);
      
       diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
       --- a/t/t5318-commit-graph.sh
     @@ -175,7 +153,7 @@
       	git repack
       '
       
     -+test_expect_success 'commit-graph write progress off by default for stderr' '
     ++test_expect_success 'commit-graph write progress off for redirected stderr' '
      +	cd "$TRASH_DIRECTORY/full" &&
      +	git commit-graph write 2>err &&
      +	test_line_count = 0 err
     @@ -193,7 +171,7 @@
      +	test_line_count = 0 err
      +'
      +
     -+test_expect_success 'commit-graph verify progress off by default for stderr' '
     ++test_expect_success 'commit-graph verify progress off for redirected stderr' '
      +	cd "$TRASH_DIRECTORY/full" &&
      +	git commit-graph verify 2>err &&
      +	test_line_count = 0 err

-- 
gitgitgadget