From efbc3aee08dfac70d426cca93cc5cfc0f14f8ee7 Mon Sep 17 00:00:00 2001 From: William Baker Date: Mon, 21 Oct 2019 18:39:58 +0000 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 Signed-off-by: Junio C Hamano --- 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 b1ea1a6aa1..e86b8cd17d 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 094c2f8ea4..397081d568 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 f29afc0d2d..f169a681dd 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 f0ae656b5d..e6fa356b5c 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 840cef0c70e4c664d814d09f304d4be9a63d11e4 Mon Sep 17 00:00:00 2001 From: William Baker Date: Mon, 21 Oct 2019 18:39:59 +0000 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 Signed-off-by: Junio C Hamano --- midx.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/midx.c b/midx.c index f169a681dd..716aeecefb 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; @@ -833,7 +837,14 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index * } } + 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 8dc18f8937faf542da785b28062731ddfbfee974 Mon Sep 17 00:00:00 2001 From: William Baker Date: Mon, 21 Oct 2019 18:40:00 +0000 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 Signed-off-by: Junio C Hamano --- midx.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/midx.c b/midx.c index 716aeecefb..cb2c602603 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 ad60096d1c82a6e05a01bb33c12cd1070bf01b4f Mon Sep 17 00:00:00 2001 From: William Baker Date: Mon, 21 Oct 2019 18:40:01 +0000 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 Signed-off-by: Junio C Hamano --- midx.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/midx.c b/midx.c index cb2c602603..1b22b2144e 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 64d80e7d52cc2663a44157fc3d49af576ea10192 Mon Sep 17 00:00:00 2001 From: William Baker Date: Mon, 21 Oct 2019 18:40:02 +0000 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 Signed-off-by: Junio C Hamano --- midx.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/midx.c b/midx.c index 1b22b2144e..37ec28623a 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 680cba2c2b1c8120c960faf80cf80a7636519be2 Mon Sep 17 00:00:00 2001 From: William Baker Date: Mon, 21 Oct 2019 18:40:03 +0000 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 Signed-off-by: Junio C Hamano --- 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 233b2b7862..642d9ac5b7 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 e86b8cd17d..5bf88cd2a8 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 c72ca04399..cd2f87be6a 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 &&