From e7b1cc633bc8f5dbdb4f113456d20f1f11476662 Mon Sep 17 00:00:00 2001 From: William Baker Date: Mon, 19 Aug 2019 14:55:30 -0700 Subject: [PATCH 1/6] midx: add MIDX_PROGRESS flag Add the MIDX_PROGRESS flag and update the write|verify|expire|repack functions in midx.h to accept a flags parameter. The MIDX_PROGRESS flag indicates whether the caller of the function would like progress information to be displayed. This patch only changes the method prototypes and does not change the functionality. The functionality change will be handled by a later patch. Signed-off-by: William Baker --- builtin/multi-pack-index.c | 8 ++++---- builtin/repack.c | 2 +- midx.c | 8 ++++---- midx.h | 10 ++++++---- 4 files changed, 15 insertions(+), 13 deletions(-) diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c index b1ea1a6aa17724..e86b8cd17debd8 100644 --- a/builtin/multi-pack-index.c +++ b/builtin/multi-pack-index.c @@ -47,16 +47,16 @@ int cmd_multi_pack_index(int argc, const char **argv, trace2_cmd_mode(argv[0]); if (!strcmp(argv[0], "repack")) - return midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size); + return midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0); if (opts.batch_size) die(_("--batch-size option is only for 'repack' subcommand")); if (!strcmp(argv[0], "write")) - return write_midx_file(opts.object_dir); + return write_midx_file(opts.object_dir, 0); if (!strcmp(argv[0], "verify")) - return verify_midx_file(the_repository, opts.object_dir); + return verify_midx_file(the_repository, opts.object_dir, 0); if (!strcmp(argv[0], "expire")) - return expire_midx_packs(the_repository, opts.object_dir); + return expire_midx_packs(the_repository, opts.object_dir, 0); die(_("unrecognized subcommand: %s"), argv[0]); } diff --git a/builtin/repack.c b/builtin/repack.c index 094c2f8ea48cae..397081d568c62a 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -562,7 +562,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) remove_temporary_files(); if (git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0)) - write_midx_file(get_object_directory()); + write_midx_file(get_object_directory(), 0); string_list_clear(&names, 0); string_list_clear(&rollback, 0); diff --git a/midx.c b/midx.c index f29afc0d2daba3..f169a681dd34e9 100644 --- a/midx.c +++ b/midx.c @@ -1016,7 +1016,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * return result; } -int write_midx_file(const char *object_dir) +int write_midx_file(const char *object_dir, unsigned flags) { return write_midx_internal(object_dir, NULL, NULL); } @@ -1076,7 +1076,7 @@ static int compare_pair_pos_vs_id(const void *_a, const void *_b) display_progress(progress, _n); \ } while (0) -int verify_midx_file(struct repository *r, const char *object_dir) +int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags) { struct pair_pos_vs_id *pairs = NULL; uint32_t i; @@ -1183,7 +1183,7 @@ int verify_midx_file(struct repository *r, const char *object_dir) return verify_midx_error; } -int expire_midx_packs(struct repository *r, const char *object_dir) +int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags) { uint32_t i, *count, result = 0; struct string_list packs_to_drop = STRING_LIST_INIT_DUP; @@ -1315,7 +1315,7 @@ static int fill_included_packs_batch(struct repository *r, return 0; } -int midx_repack(struct repository *r, const char *object_dir, size_t batch_size) +int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags) { int result = 0; uint32_t i; diff --git a/midx.h b/midx.h index f0ae656b5d7676..e6fa356b5caaf6 100644 --- a/midx.h +++ b/midx.h @@ -37,6 +37,8 @@ struct multi_pack_index { char object_dir[FLEX_ARRAY]; }; +#define MIDX_PROGRESS (1 << 0) + struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local); int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id); int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result); @@ -47,11 +49,11 @@ int fill_midx_entry(struct repository *r, const struct object_id *oid, struct pa int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name); int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local); -int write_midx_file(const char *object_dir); +int write_midx_file(const char *object_dir, unsigned flags); void clear_midx_file(struct repository *r); -int verify_midx_file(struct repository *r, const char *object_dir); -int expire_midx_packs(struct repository *r, const char *object_dir); -int midx_repack(struct repository *r, const char *object_dir, size_t batch_size); +int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags); +int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags); +int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags); void close_midx(struct multi_pack_index *m); From 37768f58860d113e21730f2640ca773ebb54dff0 Mon Sep 17 00:00:00 2001 From: William Baker Date: Thu, 19 Sep 2019 13:08:37 -0700 Subject: [PATCH 2/6] midx: add progress to write_midx_file Add progress to write_midx_file. Progress is displayed when the MIDX_PROGRESS flag is set. Signed-off-by: William Baker --- midx.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/midx.c b/midx.c index f169a681dd34e9..006f36b5700883 100644 --- a/midx.c +++ b/midx.c @@ -448,6 +448,8 @@ struct pack_list { uint32_t nr; uint32_t alloc; struct multi_pack_index *m; + struct progress *progress; + unsigned pack_paths_checked; }; static void add_pack_to_midx(const char *full_path, size_t full_path_len, @@ -456,6 +458,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len, struct pack_list *packs = (struct pack_list *)data; if (ends_with(file_name, ".idx")) { + display_progress(packs->progress, ++packs->pack_paths_checked); if (packs->m && midx_contains_pack(packs->m, file_name)) return; @@ -785,7 +788,7 @@ static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_off } static int write_midx_internal(const char *object_dir, struct multi_pack_index *m, - struct string_list *packs_to_drop) + struct string_list *packs_to_drop, unsigned flags) { unsigned char cur_chunk, num_chunks = 0; char *midx_name; @@ -799,6 +802,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * uint64_t chunk_offsets[MIDX_MAX_CHUNKS + 1]; uint32_t nr_entries, num_large_offsets = 0; struct pack_midx_entry *entries = NULL; + struct progress *progress = NULL; int large_offsets_needed = 0; int pack_name_concat_len = 0; int dropped_packs = 0; @@ -832,8 +836,15 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * packs.nr++; } } + + packs.pack_paths_checked = 0; + if (flags & MIDX_PROGRESS) + packs.progress = start_progress(_("Adding packfiles to multi-pack-index"), 0); + else + packs.progress = NULL; for_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs); + stop_progress(&packs.progress); if (packs.m && packs.nr == packs.m->num_packs && !packs_to_drop) goto cleanup; @@ -958,6 +969,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * written += MIDX_CHUNKLOOKUP_WIDTH; } + if (flags & MIDX_PROGRESS) + progress = start_progress(_("Writing chunks to multi-pack-index"), + num_chunks); for (i = 0; i < num_chunks; i++) { if (written != chunk_offsets[i]) BUG("incorrect chunk offset (%"PRIu64" != %"PRIu64") for chunk id %"PRIx32, @@ -990,7 +1004,10 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * BUG("trying to write unknown chunk id %"PRIx32, chunk_ids[i]); } + + display_progress(progress, i + 1); } + stop_progress(&progress); if (written != chunk_offsets[num_chunks]) BUG("incorrect final offset %"PRIu64" != %"PRIu64, @@ -1018,7 +1035,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * int write_midx_file(const char *object_dir, unsigned flags) { - return write_midx_internal(object_dir, NULL, NULL); + return write_midx_internal(object_dir, NULL, NULL, flags); } void clear_midx_file(struct repository *r) @@ -1221,7 +1238,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla free(count); if (packs_to_drop.nr) - result = write_midx_internal(object_dir, m, &packs_to_drop); + result = write_midx_internal(object_dir, m, &packs_to_drop, flags); string_list_clear(&packs_to_drop, 0); return result; @@ -1370,7 +1387,7 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, goto cleanup; } - result = write_midx_internal(object_dir, m, NULL); + result = write_midx_internal(object_dir, m, NULL, flags); m = NULL; cleanup: From 4ac9527d984bffea22a399506500a18823b4bd53 Mon Sep 17 00:00:00 2001 From: William Baker Date: Thu, 19 Sep 2019 13:12:01 -0700 Subject: [PATCH 3/6] midx: add progress to expire_midx_packs Add progress to expire_midx_packs. Progress is displayed when the MIDX_PROGRESS flag is set. Signed-off-by: William Baker --- midx.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/midx.c b/midx.c index 006f36b5700883..7157259ca2d166 100644 --- a/midx.c +++ b/midx.c @@ -1205,18 +1205,29 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla uint32_t i, *count, result = 0; struct string_list packs_to_drop = STRING_LIST_INIT_DUP; struct multi_pack_index *m = load_multi_pack_index(object_dir, 1); + struct progress *progress = NULL; if (!m) return 0; count = xcalloc(m->num_packs, sizeof(uint32_t)); + + if (flags & MIDX_PROGRESS) + progress = start_progress(_("Counting referenced objects"), + m->num_objects); for (i = 0; i < m->num_objects; i++) { int pack_int_id = nth_midxed_pack_int_id(m, i); count[pack_int_id]++; + display_progress(progress, i + 1); } + stop_progress(&progress); + if (flags & MIDX_PROGRESS) + progress = start_progress(_("Finding and deleting unreferenced packfiles"), + m->num_packs); for (i = 0; i < m->num_packs; i++) { char *pack_name; + display_progress(progress, i + 1); if (count[i]) continue; @@ -1234,6 +1245,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla unlink_pack_path(pack_name, 0); free(pack_name); } + stop_progress(&progress); free(count); From 3468acf39ad402826878136f3c3ba7ab076ec0c3 Mon Sep 17 00:00:00 2001 From: William Baker Date: Thu, 19 Sep 2019 13:13:30 -0700 Subject: [PATCH 4/6] midx: honor the MIDX_PROGRESS flag in verify_midx_file Update verify_midx_file to only display progress when the MIDX_PROGRESS flag is set. Signed-off-by: William Baker --- midx.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/midx.c b/midx.c index 7157259ca2d166..35bdc5f72c9141 100644 --- a/midx.c +++ b/midx.c @@ -1097,15 +1097,16 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag { struct pair_pos_vs_id *pairs = NULL; uint32_t i; - struct progress *progress; + struct progress *progress = NULL; struct multi_pack_index *m = load_multi_pack_index(object_dir, 1); verify_midx_error = 0; if (!m) return 0; - progress = start_progress(_("Looking for referenced packfiles"), - m->num_packs); + if (flags & MIDX_PROGRESS) + progress = start_progress(_("Looking for referenced packfiles"), + m->num_packs); for (i = 0; i < m->num_packs; i++) { if (prepare_midx_pack(r, m, i)) midx_report("failed to load pack in position %d", i); @@ -1123,8 +1124,9 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag i, oid_fanout1, oid_fanout2, i + 1); } - progress = start_sparse_progress(_("Verifying OID order in MIDX"), - m->num_objects - 1); + if (flags & MIDX_PROGRESS) + progress = start_sparse_progress(_("Verifying OID order in multi-pack-index"), + m->num_objects - 1); for (i = 0; i < m->num_objects - 1; i++) { struct object_id oid1, oid2; @@ -1151,13 +1153,15 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag pairs[i].pack_int_id = nth_midxed_pack_int_id(m, i); } - progress = start_sparse_progress(_("Sorting objects by packfile"), - m->num_objects); + if (flags & MIDX_PROGRESS) + progress = start_sparse_progress(_("Sorting objects by packfile"), + m->num_objects); display_progress(progress, 0); /* TODO: Measure QSORT() progress */ QSORT(pairs, m->num_objects, compare_pair_pos_vs_id); stop_progress(&progress); - progress = start_sparse_progress(_("Verifying object offsets"), m->num_objects); + if (flags & MIDX_PROGRESS) + progress = start_sparse_progress(_("Verifying object offsets"), m->num_objects); for (i = 0; i < m->num_objects; i++) { struct object_id oid; struct pack_entry e; From d6ae4e1c54b50d33571ce0529da43ba1d41a630e Mon Sep 17 00:00:00 2001 From: William Baker Date: Thu, 19 Sep 2019 13:15:20 -0700 Subject: [PATCH 5/6] midx: honor the MIDX_PROGRESS flag in midx_repack Update midx_repack to only display progress when the MIDX_PROGRESS flag is set. Signed-off-by: William Baker --- midx.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/midx.c b/midx.c index 35bdc5f72c9141..b5f7f1c54a0aef 100644 --- a/midx.c +++ b/midx.c @@ -1373,6 +1373,12 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, strbuf_addstr(&base_name, object_dir); strbuf_addstr(&base_name, "/pack/pack"); argv_array_push(&cmd.args, base_name.buf); + + if (flags & MIDX_PROGRESS) + argv_array_push(&cmd.args, "--progress"); + else + argv_array_push(&cmd.args, "-q"); + strbuf_release(&base_name); cmd.git_cmd = 1; From ea4f869780fc95ccaf00fa9b95a0df3a53f88951 Mon Sep 17 00:00:00 2001 From: William Baker Date: Wed, 18 Sep 2019 16:06:30 -0700 Subject: [PATCH 6/6] multi-pack-index: add [--[no-]progress] option. Add the --[no-]progress option to git multi-pack-index. Pass the MIDX_PROGRESS flag to the subcommand functions when progress should be displayed by multi-pack-index. The progress feature was added to 'verify' in 144d703 ("multi-pack-index: report progress during 'verify'", 2018-09-13) but some subcommands were not updated to display progress, and the ability to opt-out was overlooked. Signed-off-by: William Baker --- Documentation/git-multi-pack-index.txt | 6 ++- builtin/multi-pack-index.c | 18 +++++-- t/t5319-multi-pack-index.sh | 69 ++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 6 deletions(-) diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt index 233b2b786271cc..d7a02cc6fa9280 100644 --- a/Documentation/git-multi-pack-index.txt +++ b/Documentation/git-multi-pack-index.txt @@ -9,7 +9,7 @@ git-multi-pack-index - Write and verify multi-pack-indexes SYNOPSIS -------- [verse] -'git multi-pack-index' [--object-dir=] +'git multi-pack-index' [--object-dir=] [--[no-]progress] DESCRIPTION ----------- @@ -23,6 +23,10 @@ OPTIONS `/packs/multi-pack-index` for the current MIDX file, and `/packs` for the pack-files to index. +--[no-]progress:: + Turn progress on/off explicitly. If neither is specified, progress is + shown if standard error is connected to a terminal. + The following subcommands are available: write:: diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c index e86b8cd17debd8..1730b21901fee8 100644 --- a/builtin/multi-pack-index.c +++ b/builtin/multi-pack-index.c @@ -6,21 +6,25 @@ #include "trace2.h" static char const * const builtin_multi_pack_index_usage[] = { - N_("git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=)"), + N_("git multi-pack-index [] (write|verify|expire|repack --batch-size=)"), NULL }; static struct opts_multi_pack_index { const char *object_dir; unsigned long batch_size; + int progress; } opts; int cmd_multi_pack_index(int argc, const char **argv, const char *prefix) { + unsigned flags = 0; + static struct option builtin_multi_pack_index_options[] = { OPT_FILENAME(0, "object-dir", &opts.object_dir, N_("object directory containing set of packfile and pack-index pairs")), + OPT_BOOL(0, "progress", &opts.progress, N_("force progress reporting")), OPT_MAGNITUDE(0, "batch-size", &opts.batch_size, N_("during repack, collect pack-files of smaller size into a batch that is larger than this size")), OPT_END(), @@ -28,12 +32,15 @@ int cmd_multi_pack_index(int argc, const char **argv, git_config(git_default_config, NULL); + opts.progress = isatty(2); argc = parse_options(argc, argv, prefix, builtin_multi_pack_index_options, builtin_multi_pack_index_usage, 0); if (!opts.object_dir) opts.object_dir = get_object_directory(); + if (opts.progress) + flags |= MIDX_PROGRESS; if (argc == 0) usage_with_options(builtin_multi_pack_index_usage, @@ -47,16 +54,17 @@ int cmd_multi_pack_index(int argc, const char **argv, trace2_cmd_mode(argv[0]); if (!strcmp(argv[0], "repack")) - return midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0); + return midx_repack(the_repository, opts.object_dir, + (size_t)opts.batch_size, flags); if (opts.batch_size) die(_("--batch-size option is only for 'repack' subcommand")); if (!strcmp(argv[0], "write")) - return write_midx_file(opts.object_dir, 0); + return write_midx_file(opts.object_dir, flags); if (!strcmp(argv[0], "verify")) - return verify_midx_file(the_repository, opts.object_dir, 0); + return verify_midx_file(the_repository, opts.object_dir, flags); if (!strcmp(argv[0], "expire")) - return expire_midx_packs(the_repository, opts.object_dir, 0); + return expire_midx_packs(the_repository, opts.object_dir, flags); die(_("unrecognized subcommand: %s"), argv[0]); } diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index c72ca0439993bb..cd2f87be6afe24 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -147,6 +147,21 @@ test_expect_success 'write midx with two packs' ' compare_results_with_midx "two packs" +test_expect_success 'write progress off for redirected stderr' ' + git multi-pack-index --object-dir=$objdir write 2>err && + test_line_count = 0 err +' + +test_expect_success 'write force progress on for stderr' ' + git multi-pack-index --object-dir=$objdir --progress write 2>err && + test_file_not_empty err +' + +test_expect_success 'write with the --no-progress option' ' + git multi-pack-index --object-dir=$objdir --no-progress write 2>err && + test_line_count = 0 err +' + test_expect_success 'add more packs' ' for j in $(test_seq 11 20) do @@ -169,6 +184,21 @@ test_expect_success 'verify multi-pack-index success' ' git multi-pack-index verify --object-dir=$objdir ' +test_expect_success 'verify progress off for redirected stderr' ' + git multi-pack-index verify --object-dir=$objdir 2>err && + test_line_count = 0 err +' + +test_expect_success 'verify force progress on for stderr' ' + git multi-pack-index verify --object-dir=$objdir --progress 2>err && + test_file_not_empty err +' + +test_expect_success 'verify with the --no-progress option' ' + git multi-pack-index verify --object-dir=$objdir --no-progress 2>err && + test_line_count = 0 err +' + # usage: corrupt_midx_and_verify corrupt_midx_and_verify() { POS=$1 && @@ -284,6 +314,21 @@ test_expect_success 'git-fsck incorrect offset' ' "git -c core.multipackindex=true fsck" ' +test_expect_success 'repack progress off for redirected stderr' ' + git multi-pack-index --object-dir=$objdir repack 2>err && + test_line_count = 0 err +' + +test_expect_success 'repack force progress on for stderr' ' + git multi-pack-index --object-dir=$objdir --progress repack 2>err && + test_file_not_empty err +' + +test_expect_success 'repack with the --no-progress option' ' + git multi-pack-index --object-dir=$objdir --no-progress repack 2>err && + test_line_count = 0 err +' + test_expect_success 'repack removes multi-pack-index' ' test_path_is_file $objdir/pack/multi-pack-index && GIT_TEST_MULTI_PACK_INDEX=0 git repack -adf && @@ -413,6 +458,30 @@ test_expect_success 'expire does not remove any packs' ' ) ' +test_expect_success 'expire progress off for redirected stderr' ' + ( + cd dup && + git multi-pack-index expire 2>err && + test_line_count = 0 err + ) +' + +test_expect_success 'expire force progress on for stderr' ' + ( + cd dup && + git multi-pack-index --progress expire 2>err && + test_file_not_empty err + ) +' + +test_expect_success 'expire with the --no-progress option' ' + ( + cd dup && + git multi-pack-index --no-progress expire 2>err && + test_line_count = 0 err + ) +' + test_expect_success 'expire removes unreferenced packs' ' ( cd dup &&