{"thread":{"id":"51830","subject":"[PATCH 0/1] multi-pack-index: add --no-progress","startedAt":"2019-09-11T15:37:20Z","lastAt":"2019-10-21T18:40:17Z","messageCount":58,"participants":["William Baker via GitGitGadget","Derrick Stolee","Junio C Hamano","William Baker","Philip Oakley","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"382166","messageId":"pull.337.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":null,"subject":"[PATCH 0/1] multi-pack-index: add --no-progress","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-11T15:37:14Z","receivedAt":"2019-09-11T15:37:20Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"Hello Git contributors!\n\nMy name is William Baker and I work at Microsoft. Over the past few years\nI've worked closely with the Microsoft team contributing to the git\necosystem and I'm excited to start working with the community.\n\nDerrick Stolee helped me pick out my first task, and it's to add support for\nopting out of the multi-pack-index progress output. The progress feature was\nintroduced in 144d7033 (\"multi-pack-index: report progress during 'verify'\",\n2018-09-13) but it did not include the ability to opt-out. This patch adds\nthe --[no-]progress option to allow callers control whether progress is\nreported.\n\nThis path is very similar to Garima Singh's series for progress reporting in\nthe commit-graph [1].\n\n[1] https://public-inbox.org/git/pull.315.git.gitgitgadget@gmail.com/\n\nI'm looking forward to your review. Thanks! William Baker\n\nWilliam Baker (1):\n multi-pack-index: add --no-progress Add --no-progress option to git\n multi-pack-index. The progress feature was added in 144d703\n (\"multi-pack-index: report progress during 'verify'\", 2018-09-13)\n but the ability to opt-out was overlooked.\n\n Documentation/git-multi-pack-index.txt | 6 +++++-\n builtin/multi-pack-index.c | 14 +++++++++---\n midx.c | 30 +++++++++++++++++---------\n midx.h | 6 ++++--\n t/t5319-multi-pack-index.sh | 30 ++++++++++++++++++++++++++\n 5 files changed, 70 insertions(+), 16 deletions(-)\n\n\nbase-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-337%2Fwilbaker%2Fmulti-pack-index-progress-toggle-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-337/wilbaker/multi-pack-index-progress-toggle-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/337\n-- \ngitgitgadget\n"},{"id":"382167","messageId":"0821a8073a48067ecd9ce08226656fa04d803f6b.1568216234.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.git.gitgitgadget@gmail.com","subject":"[PATCH 1/1] multi-pack-index: add --no-progress Add --no-progress option to git multi-pack-index. The progress feature was added in 144d703 (\"multi-pack-index: report progress during 'verify'\", 2018-09-13) but the ability to opt-out was overlooked.","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-11T15:37:15Z","receivedAt":"2019-09-11T15:37:21Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nSigned-off-by: William Baker \n---\n Documentation/git-multi-pack-index.txt | 6 +++++-\n builtin/multi-pack-index.c | 14 +++++++++---\n midx.c | 30 +++++++++++++++++---------\n midx.h | 6 ++++--\n t/t5319-multi-pack-index.sh | 30 ++++++++++++++++++++++++++\n 5 files changed, 70 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex 233b2b7862..19a5a42dc0 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -9,7 +9,7 @@ git-multi-pack-index - Write and verify multi-pack-indexes\n SYNOPSIS\n --------\n [verse]\n-'git multi-pack-index' [--object-dir=] \n+'git multi-pack-index' [--object-dir=] [--[no-]progress]\n \n DESCRIPTION\n -----------\n@@ -23,6 +23,10 @@ OPTIONS\n \t`/packs/multi-pack-index` for the current MIDX file, and\n \t`/packs` for the pack-files to index.\n \n+--[no-]progress::\n+\tTurn progress on/off explicitly. If neither is specified, progress is \n+\tshown if standard error is connected to a terminal.\n+\n The following subcommands are available:\n \n write::\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex b1ea1a6aa1..f8b2a74179 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -6,34 +6,41 @@\n #include \"trace2.h\"\n \n static char const * const builtin_multi_pack_index_usage[] = {\n-\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=)\"),\n+\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=) [--[no-]progress]\"),\n \tNULL\n };\n \n static struct opts_multi_pack_index {\n \tconst char *object_dir;\n \tunsigned long batch_size;\n+\tint progress;\n } opts;\n \n int cmd_multi_pack_index(int argc, const char **argv,\n \t\t\t const char *prefix)\n {\n+\tunsigned flags = 0;\n+\n \tstatic struct option builtin_multi_pack_index_options[] = {\n \t\tOPT_FILENAME(0, \"object-dir\", &opts.object_dir,\n \t\t N_(\"object directory containing set of packfile and pack-index pairs\")),\n \t\tOPT_MAGNITUDE(0, \"batch-size\", &opts.batch_size,\n \t\t N_(\"during repack, collect pack-files of smaller size into a batch that is larger than this size\")),\n+\t\tOPT_BOOL(0, \"progress\", &opts.progress, N_(\"force progress reporting\")),\n \t\tOPT_END(),\n \t};\n \n \tgit_config(git_default_config, NULL);\n \n+\topts.progress = isatty(2);\n \targc = parse_options(argc, argv, prefix,\n \t\t\t builtin_multi_pack_index_options,\n \t\t\t builtin_multi_pack_index_usage, 0);\n \n \tif (!opts.object_dir)\n \t\topts.object_dir = get_object_directory();\n+\tif (opts.progress)\n+\t\tflags |= MIDX_PROGRESS;\n \n \tif (argc == 0)\n \t\tusage_with_options(builtin_multi_pack_index_usage,\n@@ -47,14 +54,15 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \ttrace2_cmd_mode(argv[0]);\n \n \tif (!strcmp(argv[0], \"repack\"))\n-\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size);\n+\t\treturn midx_repack(the_repository, opts.object_dir, \n+\t\t\t(size_t)opts.batch_size, flags);\n \tif (opts.batch_size)\n \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n \n \tif (!strcmp(argv[0], \"write\"))\n \t\treturn write_midx_file(opts.object_dir);\n \tif (!strcmp(argv[0], \"verify\"))\n-\t\treturn verify_midx_file(the_repository, opts.object_dir);\n+\t\treturn verify_midx_file(the_repository, opts.object_dir, flags);\n \tif (!strcmp(argv[0], \"expire\"))\n \t\treturn expire_midx_packs(the_repository, opts.object_dir);\n \ndiff --git a/midx.c b/midx.c\nindex d649644420..b1a9ad9e5b 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1077,19 +1077,20 @@ static int compare_pair_pos_vs_id(const void *_a, const void *_b)\n \t\t\tdisplay_progress(progress, _n); \\\n \t} while (0)\n \n-int verify_midx_file(struct repository *r, const char *object_dir)\n+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags)\n {\n \tstruct pair_pos_vs_id *pairs = NULL;\n \tuint32_t i;\n-\tstruct progress *progress;\n+\tstruct progress *progress = NULL;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n \tverify_midx_error = 0;\n \n \tif (!m)\n \t\treturn 0;\n \n-\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n-\t\t\t\t m->num_packs);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n+\t\t\t\t \t m->num_packs);\n \tfor (i = 0; i < m->num_packs; i++) {\n \t\tif (prepare_midx_pack(r, m, i))\n \t\t\tmidx_report(\"failed to load pack in position %d\", i);\n@@ -1107,8 +1108,9 @@ int verify_midx_file(struct repository *r, const char *object_dir)\n \t\t\t\t i, oid_fanout1, oid_fanout2, i + 1);\n \t}\n \n-\tprogress = start_sparse_progress(_(\"Verifying OID order in MIDX\"),\n-\t\t\t\t\t m->num_objects - 1);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Verifying OID order in MIDX\"),\n+\t\t\t\t\t\t m->num_objects - 1);\n \tfor (i = 0; i < m->num_objects - 1; i++) {\n \t\tstruct object_id oid1, oid2;\n \n@@ -1135,13 +1137,15 @@ int verify_midx_file(struct repository *r, const char *object_dir)\n \t\tpairs[i].pack_int_id = nth_midxed_pack_int_id(m, i);\n \t}\n \n-\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n-\t\t\t\t\t m->num_objects);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n+\t\t\t\t\t\t m->num_objects);\n \tdisplay_progress(progress, 0); /* TODO: Measure QSORT() progress */\n \tQSORT(pairs, m->num_objects, compare_pair_pos_vs_id);\n \tstop_progress(&progress);\n \n-\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n \tfor (i = 0; i < m->num_objects; i++) {\n \t\tstruct object_id oid;\n \t\tstruct pack_entry e;\n@@ -1316,7 +1320,7 @@ static int fill_included_packs_batch(struct repository *r,\n \treturn 0;\n }\n \n-int midx_repack(struct repository *r, const char *object_dir, size_t batch_size)\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags)\n {\n \tint result = 0;\n \tuint32_t i;\n@@ -1341,6 +1345,12 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size)\n \tstrbuf_addstr(&base_name, object_dir);\n \tstrbuf_addstr(&base_name, \"/pack/pack\");\n \targv_array_push(&cmd.args, base_name.buf);\n+\n+\tif (flags & MIDX_PROGRESS)\n+\t\targv_array_push(&cmd.args, \"--progress\");\n+\telse\n+\t\targv_array_push(&cmd.args, \"-q\");\n+\n \tstrbuf_release(&base_name);\n \n \tcmd.git_cmd = 1;\ndiff --git a/midx.h b/midx.h\nindex f0ae656b5d..88abc85bc3 100644\n--- a/midx.h\n+++ b/midx.h\n@@ -37,6 +37,8 @@ struct multi_pack_index {\n \tchar object_dir[FLEX_ARRAY];\n };\n \n+#define MIDX_PROGRESS (1 << 0)\n+\n struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local);\n int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id);\n int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result);\n@@ -49,9 +51,9 @@ int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, i\n \n int write_midx_file(const char *object_dir);\n void clear_midx_file(struct repository *r);\n-int verify_midx_file(struct repository *r, const char *object_dir);\n+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags);\n int expire_midx_packs(struct repository *r, const char *object_dir);\n-int midx_repack(struct repository *r, const char *object_dir, size_t batch_size);\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags);\n \n void close_midx(struct multi_pack_index *m);\n \ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex c72ca04399..cdc09b85f1 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -169,6 +169,21 @@ test_expect_success 'verify multi-pack-index success' '\n \tgit multi-pack-index verify --object-dir=$objdir\n '\n \n+test_expect_success 'verify progress off for redirected stderr' '\n+\tgit multi-pack-index verify --object-dir=$objdir 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'verify force progress on for stderr' '\n+\tgit multi-pack-index verify --object-dir=$objdir --progress 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'verify with the --no-progress option' '\n+\tgit multi-pack-index verify --object-dir=$objdir --no-progress 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n # usage: corrupt_midx_and_verify \n corrupt_midx_and_verify() {\n \tPOS=$1 &&\n@@ -284,6 +299,21 @@ test_expect_success 'git-fsck incorrect offset' '\n \t\t\"git -c core.multipackindex=true fsck\"\n '\n \n+test_expect_success 'repack progress off for redirected stderr' '\n+\tgit multi-pack-index repack --object-dir=$objdir 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'repack force progress on for stderr' '\n+\tgit multi-pack-index repack --object-dir=$objdir --progress 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'repack with the --no-progress option' '\n+\tgit multi-pack-index repack --object-dir=$objdir --no-progress 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tGIT_TEST_MULTI_PACK_INDEX=0 git repack -adf &&\n-- \ngitgitgadget\n"},{"id":"382183","messageId":"0dce74c7-4df8-6c6d-2a3a-2a23c1e89b42@gmail.com","threadId":"51830","inReplyTo":"pull.337.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/1] multi-pack-index: add --no-progress","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-09-11T20:30:09Z","receivedAt":"2019-09-11T20:30:13Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/11/2019 11:37 AM, William Baker via GitGitGadget wrote:\n> Hello Git contributors!\n> \n> My name is William Baker and I work at Microsoft. Over the past few years\n> I've worked closely with the Microsoft team contributing to the git\n> ecosystem and I'm excited to start working with the community.\n\nWilliam is being modest. He joined our team from the Windows org due to\nhis extremely impactful work in the VFS for Git client, especially around\nthe file system virtualization areas. While he started with the filesystem,\nhe also delivered important features around managing the very large object\nstore, so he is not new to some of Git's deepest internals at scale.\n\nThanks,\n-Stolee \n"},{"id":"382255","messageId":"xmqqlfutfewv.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"0821a8073a48067ecd9ce08226656fa04d803f6b.1568216234.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] multi-pack-index: add --no-progress Add --no-progress option to git multi-pack-index. The progress feature was added in 144d703 (\"multi-pack-index: report progress during 'verify'\", 2018-09-13) but the ability to opt-out was overlooked.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-12T20:17:04Z","receivedAt":"2019-09-12T20:17:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"William Baker via GitGitGadget\" writes:\n\n> [verse]\n> -'git multi-pack-index' [--object-dir=] \n> +'git multi-pack-index' [--object-dir=] [--[no-]progress]\n\nI am wondering what the reasoning behind having this new one *after*\nthe subcommand while the existing one *before* is. Isn't the\n--[no-]progress option supported by all subcommands of the\nmulti-pack-index command, just like the --object-dir= option\nis?\n\nIf there is no reason that explains why one must come before and the\nother must come after the subcommand, we are then adding another\nthing the end-users or scriptors need to memorize, which is not\nideal.\n\n> +--[no-]progress::\n> +\tTurn progress on/off explicitly. If neither is specified, progress is \n> +\tshown if standard error is connected to a terminal.\n\nSounds sensible.\n\n> diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\n> index b1ea1a6aa1..f8b2a74179 100644\n> --- a/builtin/multi-pack-index.c\n> +++ b/builtin/multi-pack-index.c\n> @@ -6,34 +6,41 @@\n> #include \"trace2.h\"\n> \n> static char const * const builtin_multi_pack_index_usage[] = {\n> -\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=)\"),\n> +\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=) [--[no-]progress]\"),\n> \tNULL\n> };\n\nThe same comment as the SYNOPSIS part.\n\n> static struct opts_multi_pack_index {\n> \tconst char *object_dir;\n> \tunsigned long batch_size;\n> +\tint progress;\n> } opts;\n> \n> int cmd_multi_pack_index(int argc, const char **argv,\n> \t\t\t const char *prefix)\n> {\n> +\tunsigned flags = 0;\n> +\n> \tstatic struct option builtin_multi_pack_index_options[] = {\n> \t\tOPT_FILENAME(0, \"object-dir\", &opts.object_dir,\n> \t\t N_(\"object directory containing set of packfile and pack-index pairs\")),\n> \t\tOPT_MAGNITUDE(0, \"batch-size\", &opts.batch_size,\n> \t\t N_(\"during repack, collect pack-files of smaller size into a batch that is larger than this size\")),\n> +\t\tOPT_BOOL(0, \"progress\", &opts.progress, N_(\"force progress reporting\")),\n> \t\tOPT_END(),\n> \t};\n\nSeeing that all the options that made me curious (see above) are\ndefined here for all subcommands, I suspect that \"technically\",\nall options must come before the (side note: I know\nparse-options may reorder commands after options, but in the\ndocumentation and usage, we strongly discourage users from relying\non the reordering and always show \"global --options, subcommand, and\nthen subcommand --options\" order). I also see in the code that\nhandles opts.batch_size that there is a workaround for this inverted\ncode structure to make sure subcommands other than repack does not\nallow --batch-size option specified.\n\nUnless and until we get rid of the \"git multi-pack-index\" as a\nseparate command (side note: when it happens, we;'d call the\nunderlying midx API functions directly from appropriate places in\nthe codepaths like \"gc\"), we probably would want to correct the use\nof parse_options() API in the implementation of this command before\nadding any new option or subcommand.\n\n> @@ -47,14 +54,15 @@ int cmd_multi_pack_index(int argc, const char **argv,\n> \ttrace2_cmd_mode(argv[0]);\n> \n> \tif (!strcmp(argv[0], \"repack\"))\n> -\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size);\n> +\t\treturn midx_repack(the_repository, opts.object_dir, \n> +\t\t\t(size_t)opts.batch_size, flags);\n> \tif (opts.batch_size)\n> \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n> \n> \tif (!strcmp(argv[0], \"write\"))\n> \t\treturn write_midx_file(opts.object_dir);\n> \tif (!strcmp(argv[0], \"verify\"))\n> -\t\treturn verify_midx_file(the_repository, opts.object_dir);\n> +\t\treturn verify_midx_file(the_repository, opts.object_dir, flags);\n> \tif (!strcmp(argv[0], \"expire\"))\n> \t\treturn expire_midx_packs(the_repository, opts.object_dir);\n\nWe can see that the new option only affects \"verify\", even though\nthe SYNOPSIS and usage text pretends that everybody understands and\nreacts to it. Shouldn't it be documented just like how --batch-size\nis documented that it is understood only by \"repack\"?\n\nIf the mid-term aspiration of this patch is to later enhance other\nsubcommands to also understand the progress output or verbosity\noption (and if the excuse given as a response to the above analysis\nis \"this is just a first step, more will come later\"), the instead\nof adding a \"unsigned flag\" local variable to the function, it would\nprobably make much more sense to\n\n (1) make struct opts_multi_pack_index as a part of the public API\n between cmd_multi_pack_index() and midx.c and document it in\n midx.h;\n\n (2) instead of passing opts.object_dir to existing command\n implementations, pass &opts, the pointer to the whole\n structure;\n\n (3) add a new field \"unsigned progress\" to the structure, and teach\n the command line parser to flip it upon seeing \"--[no-]progress\".\n"},{"id":"382318","messageId":"24c1a9aa-c83b-a984-8821-ecc51a4bc0e2@gmail.com","threadId":"51830","inReplyTo":"xmqqlfutfewv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] multi-pack-index: add --no-progress Add --no-progress option to git multi-pack-index. The progress feature was added in 144d703 (\"multi-pack-index: report progress during 'verify'\", 2018-09-13) but the ability to opt-out was overlooked.","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-09-13T18:45:29Z","receivedAt":"2019-09-13T18:45:34Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"Hi Junio,\n\nThanks for the review!\n\nOn 9/12/19 1:17 PM, Junio C Hamano wrote:\n\n>> +'git multi-pack-index' [--object-dir=] [--[no-]progress]\n> \n> I am wondering what the reasoning behind having this new one *after*\n> the subcommand while the existing one *before* is. Isn't the\n> --[no-]progress option supported by all subcommands of the\n> multi-pack-index command, just like the --object-dir= option\n> is?\n> \n> always show \"global --options, subcommand, and then subcommand --options\" order).\n\nThanks for calling this out. I didn't have a specific reason for making this\noption appear after the subcommand. I tried looking at other commands as\nexamples and I missed that there's a specific ordering based on the type\nof the option. I will clean this up in the next patch. \n\n> I also see in the code that\n> handles opts.batch_size that there is a workaround for this inverted\n> code structure to make sure subcommands other than repack does not\n> allow --batch-size option specified.\n\n> we probably would want to correct the use\n> of parse_options() API in the implementation of this command before\n> adding any new option or subcommand.\n\nTo confirm I understand, is the recommendation that\ncmd_multi_pack_index be updated to only parse \"batch-size\" for the repack\nsubcommand (i.e. use PARSE_OPT_STOP_AT_NON_OPTION to parse all of the common\noptions, and then only parse \"batch-size\" when the repack subcommand is running)?\n\n\n>> @@ -47,14 +54,15 @@ int cmd_multi_pack_index(int argc, const char **argv,\n>> \ttrace2_cmd_mode(argv[0]);\n>> \n>> \tif (!strcmp(argv[0], \"repack\"))\n>> -\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size);\n>> +\t\treturn midx_repack(the_repository, opts.object_dir, \n>> +\t\t\t(size_t)opts.batch_size, flags);\n>> \tif (opts.batch_size)\n>> \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n>> \n>> \tif (!strcmp(argv[0], \"write\"))\n>> \t\treturn write_midx_file(opts.object_dir);\n>> \tif (!strcmp(argv[0], \"verify\"))\n>> -\t\treturn verify_midx_file(the_repository, opts.object_dir);\n>> +\t\treturn verify_midx_file(the_repository, opts.object_dir, flags);\n>> \tif (!strcmp(argv[0], \"expire\"))\n>> \t\treturn expire_midx_packs(the_repository, opts.object_dir);\n> \n\n> We can see that the new option only affects \"verify\", even though\n> the SYNOPSIS and usage text pretends that everybody understands and\n> reacts to it. Shouldn't it be documented just like how --batch-size\n> is documented that it is understood only by \"repack\"?\n> \n> If the mid-term aspiration of this patch is to later enhance other\n> subcommands to also understand the progress output or verbosity\n> option (and if the excuse given as a response to the above analysis\n> is \"this is just a first step, more will come later\")\n\nYep this was my thinking. Today \"repack\" and \"verify\" are the only subcommands\nthat have any progress output but as the other subcommands learn how to provide\nprogress the [--[no-]progress] option can be used to control it. \n\n> instead of adding a \"unsigned flag\" local variable to the function, it would\n> probably make much more sense to\n> \n> (1) make struct opts_multi_pack_index as a part of the public API\n> between cmd_multi_pack_index() and midx.c and document it in\n> midx.h;\n> \n> (2) instead of passing opts.object_dir to existing command\n> implementations, pass &opts, the pointer to the whole\n> structure;\n> \n> (3) add a new field \"unsigned progress\" to the structure, and teach\n> the command line parser to flip it upon seeing \"--[no-]progress\".\n> \n\nThanks for this suggestion I'll use this approach in the next patch.\n\nOne small point to clarify, in the current struct I'm using an int for \n\"progress\" because OPT_BOOL expects an int*. Do you have any concerns with\nkeeping \"progress\" as an int in the public struct, or would you rather \ncmd_multi_pack_index parse an int internally and use it to populate the\npublic struct?\n\nThanks!\nWilliam\n"},{"id":"382332","messageId":"xmqqo8zoc57y.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"24c1a9aa-c83b-a984-8821-ecc51a4bc0e2@gmail.com","subject":"Re: [PATCH 1/1] multi-pack-index: add --no-progress Add --no-progress option to git multi-pack-index. The progress feature was added in 144d703 (\"multi-pack-index: report progress during 'verify'\", 2018-09-13) but the ability to opt-out was overlooked.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-13T20:26:57Z","receivedAt":"2019-09-13T20:27:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"William Baker writes:\n\n>> I also see in the code that\n>> handles opts.batch_size that there is a workaround for this inverted\n>> code structure to make sure subcommands other than repack does not\n>> allow --batch-size option specified.\n>\n>> we probably would want to correct the use\n>> of parse_options() API in the implementation of this command before\n>> adding any new option or subcommand.\n>\n> To confirm I understand, is the recommendation that\n> cmd_multi_pack_index be updated to only parse \"batch-size\" for the repack\n> subcommand (i.e. use PARSE_OPT_STOP_AT_NON_OPTION to parse all of the common\n> options, and then only parse \"batch-size\" when the repack subcommand is running)?\n\nCompare the ways how dispatching and command line option parsing of\nsubcommands in \"multi-pack-index\" and \"commit-graph\" are\nimplemented. When a command (e.g. \"commit-graph\") takes common\noptions and also has subcommands (e.g. \"read\" and \"write\") that take\ndifferent set of options, there is a common options parser in the\nprimary entry point (e.g. \"cmd_commit_graph()\"), and after\ndispatching to a chosen subcommand, the implementation of each\nsubcommand (e.g. \"graph_read()\" and \"graph_write()\") parses its own\noptions. That's bog-standard way.\n\ncmd_multi_pack_index() \"cheats\" and does not implement proper\nsubcommand dispatch (it directly makes underlying API calls\ninstead). Started as an isolated experimental command whose\nexistence as a standalone command is solely because it was easier to\nexperiment with (as opposed to being a plumbing command to be used\nby scripters), it probably was an acceptable trade-off to leave the\ncode in this shape. In the longer run, I suspect we'd rather want\nto get rid of \"git multi-pack-index\" as a standalone command and\ninstead make \"git gc\" and other commands make direct calls to the\ninternal machinery of midx code from strategic places. So in that\nsense, I am not sure if I should \"recommend\" fixing the way the\nsubcommand dispatching works in this command, or just accept to keep\nthe ugly technical debt and let it limp along...\n\n>> subcommands to also understand the progress output or verbosity\n>> option (and if the excuse given as a response to the above analysis\n>> is \"this is just a first step, more will come later\")\n>\n> Yep this was my thinking. Today \"repack\" and \"verify\" are the only subcommands\n> that have any progress output but as the other subcommands learn how to provide\n> progress the [--[no-]progress] option can be used to control it. \n>\n>> instead of adding a \"unsigned flag\" local variable to the function, it would\n>> probably make much more sense to\n>> \n>> (1) make struct opts_multi_pack_index as a part of the public API\n>> between cmd_multi_pack_index() and midx.c and document it in\n>> midx.h;\n>> \n>> (2) instead of passing opts.object_dir to existing command\n>> implementations, pass &opts, the pointer to the whole\n>> structure;\n>> \n>> (3) add a new field \"unsigned progress\" to the structure, and teach\n>> the command line parser to flip it upon seeing \"--[no-]progress\".\n>> \n>\n> Thanks for this suggestion I'll use this approach in the next patch.\n\n...but it appears to me that you are more enthused than I am in\nimproving this code, so I probably should actually recommend fixing\nthe main entry point function of this command to imitate the way\n\"commit-graph\" implements subcommands and their own set of options\n;-)\n\n"},{"id":"382449","messageId":"ec5e29f4-9aa1-b179-c71e-2693e4247b01@gmail.com","threadId":"51830","inReplyTo":"xmqqo8zoc57y.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/1] multi-pack-index: add --no-progress Add --no-progress option to git multi-pack-index. The progress feature was added in 144d703 (\"multi-pack-index: report progress during 'verify'\", 2018-09-13) but the ability to opt-out was overlooked.","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-09-16T19:49:24Z","receivedAt":"2019-09-16T19:49:28Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 9/13/19 1:26 PM, Junio C Hamano wrote:\n\n> Compare the ways how dispatching and command line option parsing of\n> subcommands in \"multi-pack-index\" and \"commit-graph\" are\n> implemented. When a command (e.g. \"commit-graph\") takes common\n> options and also has subcommands (e.g. \"read\" and \"write\") that take\n> different set of options, there is a common options parser in the\n> primary entry point (e.g. \"cmd_commit_graph()\"), and after\n> dispatching to a chosen subcommand, the implementation of each\n> subcommand (e.g. \"graph_read()\" and \"graph_write()\") parses its own\n> options. That's bog-standard way.\n\nThanks for the pointer to \"commit-graph\", looking through that code\ncleared up any questions I had.\n\nAfter taking another look through the \"multi-pack-index\" code my plan\nis to update all of the subcommands to understand [--[no-]progress].\nI gave the public struct in midx.h approach a try, but after seeing\nhow that looks I think it would be cleaner to update \"write\" and \"expire\"\nto display progress and have an explicit parameter.\n\n> Started as an isolated experimental command whose\n> existence as a standalone command is solely because it was easier to\n> experiment with (as opposed to being a plumbing command to be used\n> by scripters), it probably was an acceptable trade-off to leave the\n> code in this shape. In the longer run, I suspect we'd rather want\n> to get rid of \"git multi-pack-index\" as a standalone command and\n> instead make \"git gc\" and other commands make direct calls to the\n> internal machinery of midx code from strategic places. So in that\n> sense, I am not sure if I should \"recommend\" fixing the way the\n> subcommand dispatching works in this command, or just accept to keep\n> the ugly technical debt and let it limp along...\n\nThanks for the background here, it helped with understanding why the\nmulti-pack-index parsing is different than the other commands.\n\nMy plan is to include 3 commits in the next (v2) patch series:\n\n1. Adding the new parameters to midx.h/c to control progress\n2. Update write/expire to display progress\n3. Update the multi-pack-index.c builtin to parse the [--[no-]progress]\n option and update the tests.\n\nI wasn't thinking that I would adjust the the subcommand dispatching in \nmulti-pack-index.c in this patch series. By updating all of the subcommands\nto support [--[no-]progress] I should be able to keep the changes to \nmulti-pack-index.c quite small. If you see any potential issues with this\napproach please let me know.\n\nThanks,\nWilliam\n"},{"id":"382673","messageId":"pull.337.v2.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.git.gitgitgadget@gmail.com","subject":"[PATCH v2 0/6] multi-pack-index: add --no-progress","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-20T16:53:47Z","receivedAt":"2019-09-20T16:53:52Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"Hello Git contributors!\n\nMy name is William Baker and I work at Microsoft. Over the past few years\nI've worked closely with the Microsoft team contributing to the git\necosystem and I'm excited to start working with the community.\n\nDerrick Stolee helped me pick out my first task, and it's to add support for\nopting out of the multi-pack-index progress output. The progress feature was\nintroduced in 144d7033 (\"multi-pack-index: report progress during 'verify'\",\n2018-09-13) but it did not include the ability to opt-out. This patch adds\nthe --[no-]progress option to allow callers control whether progress is\nreported.\n\nThis path is very similar to Garima Singh's series for progress reporting in\nthe commit-graph [1].\n\n[1] https://public-inbox.org/git/pull.315.git.gitgitgadget@gmail.com/\n\nI'm looking forward to your review. Thanks! William Baker\n\nWilliam Baker (6):\n midx: add MIDX_PROGRESS flag Add the MIDX_PROGRESS flag and update the\n write|verify|expire|repack functions in midx.h to accept a flags\n parameter. The MIDX_PROGRESS flag indicates whether the caller of\n the function would like progress information to be displayed. This\n patch only changes the method prototypes and does not change the\n functionality. The functionality change will be handled by a later\n patch.\n midx: add progress to write_midx_file Add progress to write_midx_file.\n Progress is displayed when the MIDX_PROGRESS flag is set.\n midx: add progress to expire_midx_packs Add progress to\n expire_midx_packs. Progress is displayed when the MIDX_PROGRESS\n flag is set.\n midx: honor the MIDX_PROGRESS flag in verify_midx_file Update\n verify_midx_file to only display progress when the MIDX_PROGRESS\n flag is set.\n midx: honor the MIDX_PROGRESS flag in midx_repack Update midx_repack\n to only display progress when the MIDX_PROGRESS flag is set.\n multi-pack-index: add [--[no-]progress] option. Add the\n --[no-]progress option to git multi-pack-index. Pass the\n MIDX_PROGRESS flag to the subcommand functions when progress should\n be displayed by multi-pack-index. The progress feature was added to\n 'verify' in 144d703 (\"multi-pack-index: report progress during\n 'verify'\", 2018-09-13) but some subcommands were not updated to\n display progress, and the ability to opt-out was overlooked.\n\n Documentation/git-multi-pack-index.txt | 6 ++-\n builtin/multi-pack-index.c | 18 +++++--\n builtin/repack.c | 2 +-\n midx.c | 71 ++++++++++++++++++++------\n midx.h | 10 ++--\n t/t5319-multi-pack-index.sh | 69 +++++++++++++++++++++++++\n 6 files changed, 149 insertions(+), 27 deletions(-)\n\n\nbase-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-337%2Fwilbaker%2Fmulti-pack-index-progress-toggle-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-337/wilbaker/multi-pack-index-progress-toggle-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/337\n\nRange-diff vs v1:\n\n -: ---------- > 1: 6badd9ceaf 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.\n -: ---------- > 2: 3bc8677ea7 midx: add progress to write_midx_file Add progress to write_midx_file. Progress is displayed when the MIDX_PROGRESS flag is set.\n -: ---------- > 3: 3374574001 midx: add progress to expire_midx_packs Add progress to expire_midx_packs. Progress is displayed when the MIDX_PROGRESS flag is set.\n -: ---------- > 4: 29d03771c0 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.\n -: ---------- > 5: 57f6742f09 midx: honor the MIDX_PROGRESS flag in midx_repack Update midx_repack to only display progress when the MIDX_PROGRESS flag is set.\n 1: 0821a8073a ! 6: 5b933ab744 multi-pack-index: add --no-progress Add --no-progress option to git multi-pack-index. The progress feature was added in 144d703 (\"multi-pack-index: report progress during 'verify'\", 2018-09-13) but the ability to opt-out was overlooked.\n @@ -1,10 +1,13 @@\n Author: William Baker \n \n - multi-pack-index: add --no-progress\n - Add --no-progress option to git multi-pack-index.\n - The progress feature was added in 144d703\n + multi-pack-index: add [--[no-]progress] option.\n + Add the --[no-]progress option to git multi-pack-index.\n + Pass the MIDX_PROGRESS flag to the subcommand functions\n + when progress should be displayed by multi-pack-index.\n + The progress feature was added to 'verify' in 144d703\n (\"multi-pack-index: report progress during 'verify'\", 2018-09-13)\n - but the ability to opt-out was overlooked.\n + but some subcommands were not updated to display progress, and\n + the ability to opt-out was overlooked.\n \n Signed-off-by: William Baker \n \n @@ -16,7 +19,7 @@\n --------\n [verse]\n -'git multi-pack-index' [--object-dir=] \n -+'git multi-pack-index' [--object-dir=] [--[no-]progress]\n ++'git multi-pack-index' [--object-dir=] [--[no-]progress] \n \n DESCRIPTION\n -----------\n @@ -40,7 +43,7 @@\n \n static char const * const builtin_multi_pack_index_usage[] = {\n -\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=)\"),\n -+\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=) [--[no-]progress]\"),\n ++\tN_(\"git multi-pack-index [] (write|verify|expire|repack --batch-size=)\"),\n \tNULL\n };\n \n @@ -58,11 +61,11 @@\n \tstatic struct option builtin_multi_pack_index_options[] = {\n \t\tOPT_FILENAME(0, \"object-dir\", &opts.object_dir,\n \t\t N_(\"object directory containing set of packfile and pack-index pairs\")),\n ++\t\tOPT_BOOL(0, \"progress\", &opts.progress, N_(\"force progress reporting\")),\n \t\tOPT_MAGNITUDE(0, \"batch-size\", &opts.batch_size,\n \t\t N_(\"during repack, collect pack-files of smaller size into a batch that is larger than this size\")),\n -+\t\tOPT_BOOL(0, \"progress\", &opts.progress, N_(\"force progress reporting\")),\n \t\tOPT_END(),\n - \t};\n +@@\n \n \tgit_config(git_default_config, NULL);\n \n @@ -82,131 +85,50 @@\n \ttrace2_cmd_mode(argv[0]);\n \n \tif (!strcmp(argv[0], \"repack\"))\n --\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size);\n +-\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0);\n +\t\treturn midx_repack(the_repository, opts.object_dir, \n +\t\t\t(size_t)opts.batch_size, flags);\n \tif (opts.batch_size)\n \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n \n \tif (!strcmp(argv[0], \"write\"))\n - \t\treturn write_midx_file(opts.object_dir);\n +-\t\treturn write_midx_file(opts.object_dir, 0);\n ++\t\treturn write_midx_file(opts.object_dir, flags);\n \tif (!strcmp(argv[0], \"verify\"))\n --\t\treturn verify_midx_file(the_repository, opts.object_dir);\n +-\t\treturn verify_midx_file(the_repository, opts.object_dir, 0);\n +\t\treturn verify_midx_file(the_repository, opts.object_dir, flags);\n \tif (!strcmp(argv[0], \"expire\"))\n - \t\treturn expire_midx_packs(the_repository, opts.object_dir);\n +-\t\treturn expire_midx_packs(the_repository, opts.object_dir, 0);\n ++\t\treturn expire_midx_packs(the_repository, opts.object_dir, flags);\n \n + \tdie(_(\"unrecognized subcommand: %s\"), argv[0]);\n + }\n \n - diff --git a/midx.c b/midx.c\n - --- a/midx.c\n - +++ b/midx.c\n -@@\n - \t\t\tdisplay_progress(progress, _n); \\\n - \t} while (0)\n - \n --int verify_midx_file(struct repository *r, const char *object_dir)\n -+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags)\n - {\n - \tstruct pair_pos_vs_id *pairs = NULL;\n - \tuint32_t i;\n --\tstruct progress *progress;\n -+\tstruct progress *progress = NULL;\n - \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n - \tverify_midx_error = 0;\n - \n - \tif (!m)\n - \t\treturn 0;\n - \n --\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n --\t\t\t\t m->num_packs);\n -+\tif (flags & MIDX_PROGRESS)\n -+\t\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n -+\t\t\t\t \t m->num_packs);\n - \tfor (i = 0; i < m->num_packs; i++) {\n - \t\tif (prepare_midx_pack(r, m, i))\n - \t\t\tmidx_report(\"failed to load pack in position %d\", i);\n + diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n + --- a/t/t5319-multi-pack-index.sh\n + +++ b/t/t5319-multi-pack-index.sh\n @@\n - \t\t\t\t i, oid_fanout1, oid_fanout2, i + 1);\n - \t}\n - \n --\tprogress = start_sparse_progress(_(\"Verifying OID order in MIDX\"),\n --\t\t\t\t\t m->num_objects - 1);\n -+\tif (flags & MIDX_PROGRESS)\n -+\t\tprogress = start_sparse_progress(_(\"Verifying OID order in MIDX\"),\n -+\t\t\t\t\t\t m->num_objects - 1);\n - \tfor (i = 0; i < m->num_objects - 1; i++) {\n - \t\tstruct object_id oid1, oid2;\n \n -@@\n - \t\tpairs[i].pack_int_id = nth_midxed_pack_int_id(m, i);\n - \t}\n - \n --\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n --\t\t\t\t\t m->num_objects);\n -+\tif (flags & MIDX_PROGRESS)\n -+\t\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n -+\t\t\t\t\t\t m->num_objects);\n - \tdisplay_progress(progress, 0); /* TODO: Measure QSORT() progress */\n - \tQSORT(pairs, m->num_objects, compare_pair_pos_vs_id);\n - \tstop_progress(&progress);\n - \n --\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n -+\tif (flags & MIDX_PROGRESS)\n -+\t\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n - \tfor (i = 0; i < m->num_objects; i++) {\n - \t\tstruct object_id oid;\n - \t\tstruct pack_entry e;\n -@@\n - \treturn 0;\n - }\n + compare_results_with_midx \"two packs\"\n \n --int midx_repack(struct repository *r, const char *object_dir, size_t batch_size)\n -+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags)\n - {\n - \tint result = 0;\n - \tuint32_t i;\n -@@\n - \tstrbuf_addstr(&base_name, object_dir);\n - \tstrbuf_addstr(&base_name, \"/pack/pack\");\n - \targv_array_push(&cmd.args, base_name.buf);\n ++test_expect_success 'write progress off for redirected stderr' '\n ++\tgit multi-pack-index --object-dir=$objdir write 2>err &&\n ++\ttest_line_count = 0 err\n ++'\n +\n -+\tif (flags & MIDX_PROGRESS)\n -+\t\targv_array_push(&cmd.args, \"--progress\");\n -+\telse\n -+\t\targv_array_push(&cmd.args, \"-q\");\n ++test_expect_success 'write force progress on for stderr' '\n ++\tgit multi-pack-index --object-dir=$objdir --progress write 2>err &&\n ++\ttest_file_not_empty err\n ++'\n +\n - \tstrbuf_release(&base_name);\n - \n - \tcmd.git_cmd = 1;\n -\n - diff --git a/midx.h b/midx.h\n - --- a/midx.h\n - +++ b/midx.h\n -@@\n - \tchar object_dir[FLEX_ARRAY];\n - };\n - \n -+#define MIDX_PROGRESS (1 << 0)\n ++test_expect_success 'write with the --no-progress option' '\n ++\tgit multi-pack-index --object-dir=$objdir --no-progress write 2>err &&\n ++\ttest_line_count = 0 err\n ++'\n +\n - struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local);\n - int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id);\n - int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result);\n -@@\n - \n - int write_midx_file(const char *object_dir);\n - void clear_midx_file(struct repository *r);\n --int verify_midx_file(struct repository *r, const char *object_dir);\n -+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags);\n - int expire_midx_packs(struct repository *r, const char *object_dir);\n --int midx_repack(struct repository *r, const char *object_dir, size_t batch_size);\n -+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags);\n - \n - void close_midx(struct multi_pack_index *m);\n - \n -\n - diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n - --- a/t/t5319-multi-pack-index.sh\n - +++ b/t/t5319-multi-pack-index.sh\n + test_expect_success 'add more packs' '\n + \tfor j in $(test_seq 11 20)\n + \tdo\n @@\n \tgit multi-pack-index verify --object-dir=$objdir\n '\n @@ -234,20 +156,51 @@\n '\n \n +test_expect_success 'repack progress off for redirected stderr' '\n -+\tgit multi-pack-index repack --object-dir=$objdir 2>err &&\n ++\tgit multi-pack-index --object-dir=$objdir repack 2>err &&\n +\ttest_line_count = 0 err\n +'\n +\n +test_expect_success 'repack force progress on for stderr' '\n -+\tgit multi-pack-index repack --object-dir=$objdir --progress 2>err &&\n ++\tgit multi-pack-index --object-dir=$objdir --progress repack 2>err &&\n +\ttest_file_not_empty err\n +'\n +\n +test_expect_success 'repack with the --no-progress option' '\n -+\tgit multi-pack-index repack --object-dir=$objdir --no-progress 2>err &&\n ++\tgit multi-pack-index --object-dir=$objdir --no-progress repack 2>err &&\n +\ttest_line_count = 0 err\n +'\n +\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tGIT_TEST_MULTI_PACK_INDEX=0 git repack -adf &&\n +@@\n + \t)\n + '\n + \n ++test_expect_success 'expire progress off for redirected stderr' '\n ++\t(\n ++\t\tcd dup &&\n ++\t\tgit multi-pack-index expire 2>err &&\n ++\t\ttest_line_count = 0 err\n ++\t)\n ++'\n ++\n ++test_expect_success 'expire force progress on for stderr' '\n ++\t(\n ++\t\tcd dup &&\n ++\t\tgit multi-pack-index --progress expire 2>err &&\n ++\t\ttest_file_not_empty err\n ++\t)\n ++'\n ++\n ++test_expect_success 'expire with the --no-progress option' '\n ++\t(\n ++\t\tcd dup &&\n ++\t\tgit multi-pack-index --no-progress expire 2>err &&\n ++\t\ttest_line_count = 0 err\n ++\t)\n ++'\n ++\n + test_expect_success 'expire removes unreferenced packs' '\n + \t(\n + \t\tcd dup &&\n\n-- \ngitgitgadget\n"},{"id":"382674","messageId":"6badd9ceaf4851b2984e78a5cfd0cb8ec0c810f5.1568998427.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 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.","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-20T16:53:48Z","receivedAt":"2019-09-20T16:53:53Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nSigned-off-by: William Baker \n---\n builtin/multi-pack-index.c | 8 ++++----\n builtin/repack.c | 2 +-\n midx.c | 8 ++++----\n midx.h | 10 ++++++----\n 4 files changed, 15 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex b1ea1a6aa1..e86b8cd17d 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -47,16 +47,16 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \ttrace2_cmd_mode(argv[0]);\n \n \tif (!strcmp(argv[0], \"repack\"))\n-\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size);\n+\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0);\n \tif (opts.batch_size)\n \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n \n \tif (!strcmp(argv[0], \"write\"))\n-\t\treturn write_midx_file(opts.object_dir);\n+\t\treturn write_midx_file(opts.object_dir, 0);\n \tif (!strcmp(argv[0], \"verify\"))\n-\t\treturn verify_midx_file(the_repository, opts.object_dir);\n+\t\treturn verify_midx_file(the_repository, opts.object_dir, 0);\n \tif (!strcmp(argv[0], \"expire\"))\n-\t\treturn expire_midx_packs(the_repository, opts.object_dir);\n+\t\treturn expire_midx_packs(the_repository, opts.object_dir, 0);\n \n \tdie(_(\"unrecognized subcommand: %s\"), argv[0]);\n }\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 632c0c0a79..5b3623337f 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -561,7 +561,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \tremove_temporary_files();\n \n \tif (git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0))\n-\t\twrite_midx_file(get_object_directory());\n+\t\twrite_midx_file(get_object_directory(), 0);\n \n \tstring_list_clear(&names, 0);\n \tstring_list_clear(&rollback, 0);\ndiff --git a/midx.c b/midx.c\nindex d649644420..b2673f52e8 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1017,7 +1017,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \treturn result;\n }\n \n-int write_midx_file(const char *object_dir)\n+int write_midx_file(const char *object_dir, unsigned flags)\n {\n \treturn write_midx_internal(object_dir, NULL, NULL);\n }\n@@ -1077,7 +1077,7 @@ static int compare_pair_pos_vs_id(const void *_a, const void *_b)\n \t\t\tdisplay_progress(progress, _n); \\\n \t} while (0)\n \n-int verify_midx_file(struct repository *r, const char *object_dir)\n+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags)\n {\n \tstruct pair_pos_vs_id *pairs = NULL;\n \tuint32_t i;\n@@ -1184,7 +1184,7 @@ int verify_midx_file(struct repository *r, const char *object_dir)\n \treturn verify_midx_error;\n }\n \n-int expire_midx_packs(struct repository *r, const char *object_dir)\n+int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags)\n {\n \tuint32_t i, *count, result = 0;\n \tstruct string_list packs_to_drop = STRING_LIST_INIT_DUP;\n@@ -1316,7 +1316,7 @@ static int fill_included_packs_batch(struct repository *r,\n \treturn 0;\n }\n \n-int midx_repack(struct repository *r, const char *object_dir, size_t batch_size)\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags)\n {\n \tint result = 0;\n \tuint32_t i;\ndiff --git a/midx.h b/midx.h\nindex f0ae656b5d..e6fa356b5c 100644\n--- a/midx.h\n+++ b/midx.h\n@@ -37,6 +37,8 @@ struct multi_pack_index {\n \tchar object_dir[FLEX_ARRAY];\n };\n \n+#define MIDX_PROGRESS (1 << 0)\n+\n struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local);\n int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id);\n int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result);\n@@ -47,11 +49,11 @@ int fill_midx_entry(struct repository *r, const struct object_id *oid, struct pa\n int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name);\n int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local);\n \n-int write_midx_file(const char *object_dir);\n+int write_midx_file(const char *object_dir, unsigned flags);\n void clear_midx_file(struct repository *r);\n-int verify_midx_file(struct repository *r, const char *object_dir);\n-int expire_midx_packs(struct repository *r, const char *object_dir);\n-int midx_repack(struct repository *r, const char *object_dir, size_t batch_size);\n+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags);\n+int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags);\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags);\n \n void close_midx(struct multi_pack_index *m);\n \n-- \ngitgitgadget\n\n"},{"id":"382675","messageId":"57f6742f09820964453bb325023d44d86bca05a2.1568998427.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 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.","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-20T16:53:51Z","receivedAt":"2019-09-20T16:53:55Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nSigned-off-by: William Baker \n---\n midx.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex dc7c5f43f8..106ccc4ab2 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1374,6 +1374,12 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,\n \tstrbuf_addstr(&base_name, object_dir);\n \tstrbuf_addstr(&base_name, \"/pack/pack\");\n \targv_array_push(&cmd.args, base_name.buf);\n+\n+\tif (flags & MIDX_PROGRESS)\n+\t\targv_array_push(&cmd.args, \"--progress\");\n+\telse\n+\t\targv_array_push(&cmd.args, \"-q\");\n+\n \tstrbuf_release(&base_name);\n \n \tcmd.git_cmd = 1;\n-- \ngitgitgadget\n\n"},{"id":"382676","messageId":"29d03771c00ee4d18a7c6455edae5ebb75043c62.1568998427.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 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.","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-20T16:53:51Z","receivedAt":"2019-09-20T16:53:56Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nSigned-off-by: William Baker \n---\n midx.c | 20 ++++++++++++--------\n 1 file changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex 3a0e161aea..dc7c5f43f8 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1098,15 +1098,16 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n {\n \tstruct pair_pos_vs_id *pairs = NULL;\n \tuint32_t i;\n-\tstruct progress *progress;\n+\tstruct progress *progress = NULL;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n \tverify_midx_error = 0;\n \n \tif (!m)\n \t\treturn 0;\n \n-\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n-\t\t\t\t m->num_packs);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n+\t\t\t\t \t m->num_packs);\n \tfor (i = 0; i < m->num_packs; i++) {\n \t\tif (prepare_midx_pack(r, m, i))\n \t\t\tmidx_report(\"failed to load pack in position %d\", i);\n@@ -1124,8 +1125,9 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n \t\t\t\t i, oid_fanout1, oid_fanout2, i + 1);\n \t}\n \n-\tprogress = start_sparse_progress(_(\"Verifying OID order in MIDX\"),\n-\t\t\t\t\t m->num_objects - 1);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Verifying OID order in multi-pack-index\"),\n+\t\t\t\t\t\t m->num_objects - 1);\n \tfor (i = 0; i < m->num_objects - 1; i++) {\n \t\tstruct object_id oid1, oid2;\n \n@@ -1152,13 +1154,15 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n \t\tpairs[i].pack_int_id = nth_midxed_pack_int_id(m, i);\n \t}\n \n-\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n-\t\t\t\t\t m->num_objects);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n+\t\t\t\t\t\t m->num_objects);\n \tdisplay_progress(progress, 0); /* TODO: Measure QSORT() progress */\n \tQSORT(pairs, m->num_objects, compare_pair_pos_vs_id);\n \tstop_progress(&progress);\n \n-\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n \tfor (i = 0; i < m->num_objects; i++) {\n \t\tstruct object_id oid;\n \t\tstruct pack_entry e;\n-- \ngitgitgadget\n\n"},{"id":"382677","messageId":"3bc8677ea7655a3706914f9753c0a3b79dbf7e1f.1568998427.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 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.","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-20T16:53:49Z","receivedAt":"2019-09-20T16:53:57Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nSigned-off-by: William Baker \n---\n midx.c | 25 +++++++++++++++++++++----\n 1 file changed, 21 insertions(+), 4 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex b2673f52e8..54e4e93b2b 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -449,6 +449,8 @@ struct pack_list {\n \tuint32_t nr;\n \tuint32_t alloc;\n \tstruct multi_pack_index *m;\n+\tstruct progress *progress;\n+\tuint32_t pack_paths_checked;\n };\n \n static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n@@ -457,6 +459,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n \tstruct pack_list *packs = (struct pack_list *)data;\n \n \tif (ends_with(file_name, \".idx\")) {\n+\t\tdisplay_progress(packs->progress, ++packs->pack_paths_checked);\n \t\tif (packs->m && midx_contains_pack(packs->m, file_name))\n \t\t\treturn;\n \n@@ -786,7 +789,7 @@ static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_off\n }\n \n static int write_midx_internal(const char *object_dir, struct multi_pack_index *m,\n-\t\t\t struct string_list *packs_to_drop)\n+\t\t\t struct string_list *packs_to_drop, unsigned flags)\n {\n \tunsigned char cur_chunk, num_chunks = 0;\n \tchar *midx_name;\n@@ -800,6 +803,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \tuint64_t chunk_offsets[MIDX_MAX_CHUNKS + 1];\n \tuint32_t nr_entries, num_large_offsets = 0;\n \tstruct pack_midx_entry *entries = NULL;\n+\tstruct progress *progress = NULL;\n \tint large_offsets_needed = 0;\n \tint pack_name_concat_len = 0;\n \tint dropped_packs = 0;\n@@ -833,8 +837,15 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \t\t\tpacks.nr++;\n \t\t}\n \t}\n+\t\n+\tpacks.pack_paths_checked = 0;\n+\tif (flags & MIDX_PROGRESS)\n+\t\tpacks.progress = start_progress(_(\"Adding packfiles to multi-pack-index\"), 0);\n+\telse\n+\t\tpacks.progress = NULL;\n \n \tfor_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs);\n+\tstop_progress(&packs.progress);\n \n \tif (packs.m && packs.nr == packs.m->num_packs && !packs_to_drop)\n \t\tgoto cleanup;\n@@ -959,6 +970,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \t\twritten += MIDX_CHUNKLOOKUP_WIDTH;\n \t}\n \n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Writing chunks to multi-pack-index\"),\n+\t\t\t\t\t num_chunks);\n \tfor (i = 0; i < num_chunks; i++) {\n \t\tif (written != chunk_offsets[i])\n \t\t\tBUG(\"incorrect chunk offset (%\"PRIu64\" != %\"PRIu64\") for chunk id %\"PRIx32,\n@@ -991,7 +1005,10 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \t\t\t\tBUG(\"trying to write unknown chunk id %\"PRIx32,\n \t\t\t\t chunk_ids[i]);\n \t\t}\n+\n+\t\tdisplay_progress(progress, i + 1);\n \t}\n+\tstop_progress(&progress);\n \n \tif (written != chunk_offsets[num_chunks])\n \t\tBUG(\"incorrect final offset %\"PRIu64\" != %\"PRIu64,\n@@ -1019,7 +1036,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \n int write_midx_file(const char *object_dir, unsigned flags)\n {\n-\treturn write_midx_internal(object_dir, NULL, NULL);\n+\treturn write_midx_internal(object_dir, NULL, NULL, flags);\n }\n \n void clear_midx_file(struct repository *r)\n@@ -1222,7 +1239,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla\n \tfree(count);\n \n \tif (packs_to_drop.nr)\n-\t\tresult = write_midx_internal(object_dir, m, &packs_to_drop);\n+\t\tresult = write_midx_internal(object_dir, m, &packs_to_drop, flags);\n \n \tstring_list_clear(&packs_to_drop, 0);\n \treturn result;\n@@ -1371,7 +1388,7 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,\n \t\tgoto cleanup;\n \t}\n \n-\tresult = write_midx_internal(object_dir, m, NULL);\n+\tresult = write_midx_internal(object_dir, m, NULL, flags);\n \tm = NULL;\n \n cleanup:\n-- \ngitgitgadget\n\n"},{"id":"382678","messageId":"3374574001cf188d9d17ac3aeed1139b82330284.1568998427.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 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.","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-20T16:53:50Z","receivedAt":"2019-09-20T16:53:58Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nSigned-off-by: William Baker \n---\n midx.c | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex 54e4e93b2b..3a0e161aea 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1206,18 +1206,29 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla\n \tuint32_t i, *count, result = 0;\n \tstruct string_list packs_to_drop = STRING_LIST_INIT_DUP;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n+\tstruct progress *progress = NULL;\n \n \tif (!m)\n \t\treturn 0;\n \n \tcount = xcalloc(m->num_packs, sizeof(uint32_t));\n+\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Counting referenced objects\"), \n+\t\t\t\t\t m->num_objects);\n \tfor (i = 0; i < m->num_objects; i++) {\n \t\tint pack_int_id = nth_midxed_pack_int_id(m, i);\n \t\tcount[pack_int_id]++;\n+\t\tdisplay_progress(progress, i + 1);\n \t}\n+\tstop_progress(&progress);\n \n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Finding and deleting unreferenced packfiles\"),\n+\t\t\t\t\t m->num_packs);\n \tfor (i = 0; i < m->num_packs; i++) {\n \t\tchar *pack_name;\n+\t\tdisplay_progress(progress, i + 1);\n \n \t\tif (count[i])\n \t\t\tcontinue;\n@@ -1235,6 +1246,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla\n \t\tunlink_pack_path(pack_name, 0);\n \t\tfree(pack_name);\n \t}\n+\tstop_progress(&progress);\n \n \tfree(count);\n \n-- \ngitgitgadget\n\n"},{"id":"382679","messageId":"5b933ab74424d768e99d8e810ad24b62132dbd02.1568998427.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v2 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.","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-09-20T16:53:52Z","receivedAt":"2019-09-20T16:54:00Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nSigned-off-by: William Baker \n---\n Documentation/git-multi-pack-index.txt | 6 ++-\n builtin/multi-pack-index.c | 18 +++++--\n t/t5319-multi-pack-index.sh | 69 ++++++++++++++++++++++++++\n 3 files changed, 87 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex 233b2b7862..d7a02cc6fa 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -9,7 +9,7 @@ git-multi-pack-index - Write and verify multi-pack-indexes\n SYNOPSIS\n --------\n [verse]\n-'git multi-pack-index' [--object-dir=] \n+'git multi-pack-index' [--object-dir=] [--[no-]progress] \n \n DESCRIPTION\n -----------\n@@ -23,6 +23,10 @@ OPTIONS\n \t`/packs/multi-pack-index` for the current MIDX file, and\n \t`/packs` for the pack-files to index.\n \n+--[no-]progress::\n+\tTurn progress on/off explicitly. If neither is specified, progress is \n+\tshown if standard error is connected to a terminal.\n+\n The following subcommands are available:\n \n write::\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex e86b8cd17d..1730b21901 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -6,21 +6,25 @@\n #include \"trace2.h\"\n \n static char const * const builtin_multi_pack_index_usage[] = {\n-\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=)\"),\n+\tN_(\"git multi-pack-index [] (write|verify|expire|repack --batch-size=)\"),\n \tNULL\n };\n \n static struct opts_multi_pack_index {\n \tconst char *object_dir;\n \tunsigned long batch_size;\n+\tint progress;\n } opts;\n \n int cmd_multi_pack_index(int argc, const char **argv,\n \t\t\t const char *prefix)\n {\n+\tunsigned flags = 0;\n+\n \tstatic struct option builtin_multi_pack_index_options[] = {\n \t\tOPT_FILENAME(0, \"object-dir\", &opts.object_dir,\n \t\t N_(\"object directory containing set of packfile and pack-index pairs\")),\n+\t\tOPT_BOOL(0, \"progress\", &opts.progress, N_(\"force progress reporting\")),\n \t\tOPT_MAGNITUDE(0, \"batch-size\", &opts.batch_size,\n \t\t N_(\"during repack, collect pack-files of smaller size into a batch that is larger than this size\")),\n \t\tOPT_END(),\n@@ -28,12 +32,15 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \n \tgit_config(git_default_config, NULL);\n \n+\topts.progress = isatty(2);\n \targc = parse_options(argc, argv, prefix,\n \t\t\t builtin_multi_pack_index_options,\n \t\t\t builtin_multi_pack_index_usage, 0);\n \n \tif (!opts.object_dir)\n \t\topts.object_dir = get_object_directory();\n+\tif (opts.progress)\n+\t\tflags |= MIDX_PROGRESS;\n \n \tif (argc == 0)\n \t\tusage_with_options(builtin_multi_pack_index_usage,\n@@ -47,16 +54,17 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \ttrace2_cmd_mode(argv[0]);\n \n \tif (!strcmp(argv[0], \"repack\"))\n-\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0);\n+\t\treturn midx_repack(the_repository, opts.object_dir, \n+\t\t\t(size_t)opts.batch_size, flags);\n \tif (opts.batch_size)\n \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n \n \tif (!strcmp(argv[0], \"write\"))\n-\t\treturn write_midx_file(opts.object_dir, 0);\n+\t\treturn write_midx_file(opts.object_dir, flags);\n \tif (!strcmp(argv[0], \"verify\"))\n-\t\treturn verify_midx_file(the_repository, opts.object_dir, 0);\n+\t\treturn verify_midx_file(the_repository, opts.object_dir, flags);\n \tif (!strcmp(argv[0], \"expire\"))\n-\t\treturn expire_midx_packs(the_repository, opts.object_dir, 0);\n+\t\treturn expire_midx_packs(the_repository, opts.object_dir, flags);\n \n \tdie(_(\"unrecognized subcommand: %s\"), argv[0]);\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex c72ca04399..cd2f87be6a 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -147,6 +147,21 @@ test_expect_success 'write midx with two packs' '\n \n compare_results_with_midx \"two packs\"\n \n+test_expect_success 'write progress off for redirected stderr' '\n+\tgit multi-pack-index --object-dir=$objdir write 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'write force progress on for stderr' '\n+\tgit multi-pack-index --object-dir=$objdir --progress write 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'write with the --no-progress option' '\n+\tgit multi-pack-index --object-dir=$objdir --no-progress write 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n test_expect_success 'add more packs' '\n \tfor j in $(test_seq 11 20)\n \tdo\n@@ -169,6 +184,21 @@ test_expect_success 'verify multi-pack-index success' '\n \tgit multi-pack-index verify --object-dir=$objdir\n '\n \n+test_expect_success 'verify progress off for redirected stderr' '\n+\tgit multi-pack-index verify --object-dir=$objdir 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'verify force progress on for stderr' '\n+\tgit multi-pack-index verify --object-dir=$objdir --progress 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'verify with the --no-progress option' '\n+\tgit multi-pack-index verify --object-dir=$objdir --no-progress 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n # usage: corrupt_midx_and_verify \n corrupt_midx_and_verify() {\n \tPOS=$1 &&\n@@ -284,6 +314,21 @@ test_expect_success 'git-fsck incorrect offset' '\n \t\t\"git -c core.multipackindex=true fsck\"\n '\n \n+test_expect_success 'repack progress off for redirected stderr' '\n+\tgit multi-pack-index --object-dir=$objdir repack 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'repack force progress on for stderr' '\n+\tgit multi-pack-index --object-dir=$objdir --progress repack 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'repack with the --no-progress option' '\n+\tgit multi-pack-index --object-dir=$objdir --no-progress repack 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tGIT_TEST_MULTI_PACK_INDEX=0 git repack -adf &&\n@@ -413,6 +458,30 @@ test_expect_success 'expire does not remove any packs' '\n \t)\n '\n \n+test_expect_success 'expire progress off for redirected stderr' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit multi-pack-index expire 2>err &&\n+\t\ttest_line_count = 0 err\n+\t)\n+'\n+\n+test_expect_success 'expire force progress on for stderr' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit multi-pack-index --progress expire 2>err &&\n+\t\ttest_file_not_empty err\n+\t)\n+'\n+\n+test_expect_success 'expire with the --no-progress option' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit multi-pack-index --no-progress expire 2>err &&\n+\t\ttest_line_count = 0 err\n+\t)\n+'\n+\n test_expect_success 'expire removes unreferenced packs' '\n \t(\n \t\tcd dup &&\n-- \ngitgitgadget\n"},{"id":"382700","messageId":"xmqqv9tmu4jz.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"pull.337.v2.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/6] multi-pack-index: add --no-progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-20T19:54:24Z","receivedAt":"2019-09-20T19:54:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"William Baker via GitGitGadget\" writes:\n\n> William Baker (6):\n> midx: add MIDX_PROGRESS flag Add the MIDX_PROGRESS flag and update the\n> write|verify|expire|repack functions in midx.h to accept a flags\n> parameter. The MIDX_PROGRESS flag indicates whether the caller of\n> the function would like progress information to be displayed. This\n> patch only changes the method prototypes and does not change the\n> functionality. The functionality change will be handled by a later\n> patch.\n> midx: add progress to write_midx_file Add progress to write_midx_file.\n> Progress is displayed when the MIDX_PROGRESS flag is set.\n> midx: add progress to expire_midx_packs Add progress to\n> expire_midx_packs. Progress is displayed when the MIDX_PROGRESS\n> flag is set.\n> midx: honor the MIDX_PROGRESS flag in verify_midx_file Update\n> verify_midx_file to only display progress when the MIDX_PROGRESS\n> flag is set.\n> midx: honor the MIDX_PROGRESS flag in midx_repack Update midx_repack\n> to only display progress when the MIDX_PROGRESS flag is set.\n> multi-pack-index: add [--[no-]progress] option. Add the\n> --[no-]progress option to git multi-pack-index. Pass the\n> MIDX_PROGRESS flag to the subcommand functions when progress should\n> be displayed by multi-pack-index. The progress feature was added to\n> 'verify' in 144d703 (\"multi-pack-index: report progress during\n> 'verify'\", 2018-09-13) but some subcommands were not updated to\n> display progress, and the ability to opt-out was overlooked.\n\nDo all of these commits have overly long title with no body recorded\nin the commit objects, or is this just a mail sending program\nscrewing up?\n\n"},{"id":"382701","messageId":"xmqqr24au480.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"6badd9ceaf4851b2984e78a5cfd0cb8ec0c810f5.1568998427.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 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.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-20T20:01:35Z","receivedAt":"2019-09-20T20:01:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"William Baker via GitGitGadget\" writes:\n\n> From: William Baker \n>\n> Signed-off-by: William Baker \n> ---\n> builtin/multi-pack-index.c | 8 ++++----\n> builtin/repack.c | 2 +-\n> midx.c | 8 ++++----\n> midx.h | 10 ++++++----\n> 4 files changed, 15 insertions(+), 13 deletions(-)\n>\n> diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\n> index b1ea1a6aa1..e86b8cd17d 100644\n> --- a/builtin/multi-pack-index.c\n> +++ b/builtin/multi-pack-index.c\n> @@ -47,16 +47,16 @@ int cmd_multi_pack_index(int argc, const char **argv,\n> \ttrace2_cmd_mode(argv[0]);\n> \n> \tif (!strcmp(argv[0], \"repack\"))\n> -\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size);\n> +\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0);\n> \tif (opts.batch_size)\n> \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n> \n> \tif (!strcmp(argv[0], \"write\"))\n> -\t\treturn write_midx_file(opts.object_dir);\n> +\t\treturn write_midx_file(opts.object_dir, 0);\n> \tif (!strcmp(argv[0], \"verify\"))\n> -\t\treturn verify_midx_file(the_repository, opts.object_dir);\n> +\t\treturn verify_midx_file(the_repository, opts.object_dir, 0);\n> \tif (!strcmp(argv[0], \"expire\"))\n> -\t\treturn expire_midx_packs(the_repository, opts.object_dir);\n> +\t\treturn expire_midx_packs(the_repository, opts.object_dir, 0);\n\nHmm, I actually would have expected to see a new .progress field in\nthe opts structure and these lower-level functions would start\ntaking a pointer to the whole opts structure, instead of just a\npointer to a single member opts.object_dir. That way, we can later\nextend and enrich the interface between this caller and the\nunderlying functions without much patch noise in the dispatch layer\n(i.e. here).\n\nThis step is meant to be just preparing by extending the interface\nand passing the new argument throughout the callchain, I believe,\nand it looks correctly done, assuming that we are OK to take this\n\"add a separate 'progress' parameter, when we need more parameters\nlater, the interface will gain more and more parameters\" approach.\n"},{"id":"382702","messageId":"xmqqlfuiu3sv.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"3bc8677ea7655a3706914f9753c0a3b79dbf7e1f.1568998427.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 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.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-20T20:10:40Z","receivedAt":"2019-09-20T20:10:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"William Baker via GitGitGadget\" writes:\n\n> diff --git a/midx.c b/midx.c\n> index b2673f52e8..54e4e93b2b 100644\n> --- a/midx.c\n> +++ b/midx.c\n> @@ -449,6 +449,8 @@ struct pack_list {\n> \tuint32_t nr;\n> \tuint32_t alloc;\n> \tstruct multi_pack_index *m;\n> +\tstruct progress *progress;\n> +\tuint32_t pack_paths_checked;\n> };\n\nWhat is the reason behind u32 here? Do we want to be consistently\nlimited to 4G on all platforms?\n\nI have a feeling that we do not care too deeply all that much for\nthe purpose of progress output, in which case, just an ordinary\n\"unsigned\" may be much less misleading.\n\nFWIW, the function that uses this field is display_progress(), which\ntakes uint64_t there.\n\n> @@ -457,6 +459,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n> \tstruct pack_list *packs = (struct pack_list *)data;\n> \n> \tif (ends_with(file_name, \".idx\")) {\n> +\t\tdisplay_progress(packs->progress, ++packs->pack_paths_checked);\n\nOK, \"git grep display_progress\" tells me that we are expected to\npass the value of the counter that is already incremented, so this\nis also in line with that practice.\n\n> +\tif (flags & MIDX_PROGRESS)\n> +\t\tpacks.progress = start_progress(_(\"Adding packfiles to multi-pack-index\"), 0);\n> +\telse\n> +\t\tpacks.progress = NULL;\n>\n> \tfor_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs);\n> +\tstop_progress(&packs.progress);\n\nOK.\n\n> +\tif (flags & MIDX_PROGRESS)\n> +\t\tprogress = start_progress(_(\"Writing chunks to multi-pack-index\"),\n> +\t\t\t\t\t num_chunks);\n> ...\n> +\n> +\t\tdisplay_progress(progress, i + 1);\n> \t}\n> +\tstop_progress(&progress);\n> \n\nOK.\n"},{"id":"382703","messageId":"xmqqh856u3q0.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"57f6742f09820964453bb325023d44d86bca05a2.1568998427.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 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.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-20T20:12:23Z","receivedAt":"2019-09-20T20:12:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steps 4 & 5 look OK.\n"},{"id":"382704","messageId":"xmqqd0fuu3ic.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"xmqqr24au480.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 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.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-20T20:16:59Z","receivedAt":"2019-09-20T20:17:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> This step is meant to be just preparing by extending the interface\n> and passing the new argument throughout the callchain, I believe,\n> and it looks correctly done, assuming that we are OK to take this\n> \"add a separate 'progress' parameter, when we need more parameters\n> later, the interface will gain more and more parameters\" approach.\n\nAfter re-reading the remainder of the series, I think the structure\nthis patch series takes (i.e. adding a new parameter to these callees)\nis better than the alternative (i.e. making sure everybody takes the\npointer to the opts structure).\n\nThanks. I couldn't review the proposed log messages of these\ncommits, which were unreadable, at all, though.\n"},{"id":"382706","messageId":"1c94eaf5-fd62-cca3-2aeb-37512e8e41e6@iee.email","threadId":"51830","inReplyTo":"pull.337.v2.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 0/6] multi-pack-index: add --no-progress","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2019-09-20T20:23:53Z","receivedAt":"2019-09-20T20:27:14Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi William, welcome.\n\nOn 20/09/2019 17:53, William Baker via GitGitGadget wrote:\n> Hello Git contributors!\n>\n> My name is William Baker and I work at Microsoft. Over the past few years\n> I've worked closely with the Microsoft team contributing to the git\n> ecosystem and I'm excited to start working with the community.\n>\n> Derrick Stolee helped me pick out my first task, and it's to add support for\n> opting out of the multi-pack-index progress output. The progress feature was\n> introduced in 144d7033 (\"multi-pack-index: report progress during 'verify'\",\n> 2018-09-13) but it did not include the ability to opt-out. This patch adds\n> the --[no-]progress option to allow callers control whether progress is\n> reported.\n>\n> This path is very similar to Garima Singh's series for progress reporting in\n> the commit-graph [1].\n>\n> [1] https://public-inbox.org/git/pull.315.git.gitgitgadget@gmail.com/\n>\n> I'm looking forward to your review. Thanks! William Baker\n>\n> William Baker (6):\n> midx: add MIDX_PROGRESS flag Add the MIDX_PROGRESS flag and update the\n> write|verify|expire|repack functions in midx.h to accept a flags\n> parameter. The MIDX_PROGRESS flag indicates whether the caller of\n> the function would like progress information to be displayed. This\n> patch only changes the method prototypes and does not change the\n> functionality. The functionality change will be handled by a later\n> patch.\n> midx: add progress to write_midx_file Add progress to write_midx_file.\n> Progress is displayed when the MIDX_PROGRESS flag is set.\n> midx: add progress to expire_midx_packs Add progress to\n> expire_midx_packs. Progress is displayed when the MIDX_PROGRESS\n> flag is set.\n> midx: honor the MIDX_PROGRESS flag in verify_midx_file Update\n> verify_midx_file to only display progress when the MIDX_PROGRESS\n> flag is set.\n> midx: honor the MIDX_PROGRESS flag in midx_repack Update midx_repack\n> to only display progress when the MIDX_PROGRESS flag is set.\n> multi-pack-index: add [--[no-]progress] option. Add the\n> --[no-]progress option to git multi-pack-index. Pass the\n> MIDX_PROGRESS flag to the subcommand functions when progress should\n> be displayed by multi-pack-index. The progress feature was added to\n> 'verify' in 144d703 (\"multi-pack-index: report progress during\n> 'verify'\", 2018-09-13) but some subcommands were not updated to\n> display progress, and the ability to opt-out was overlooked.\nThese subject lines are a bit long. Has there been some white space \ndamage during preparation, such that the end of line, and following \nblank line, for a short subject line been compromised?\n\nNormally the 'git log --oneline' should fit within the line, so ~50 chars.\n\nPhilip\n\n> Documentation/git-multi-pack-index.txt | 6 ++-\n> builtin/multi-pack-index.c | 18 +++++--\n> builtin/repack.c | 2 +-\n> midx.c | 71 ++++++++++++++++++++------\n> midx.h | 10 ++--\n> t/t5319-multi-pack-index.sh | 69 +++++++++++++++++++++++++\n> 6 files changed, 149 insertions(+), 27 deletions(-)\n>\n>\n> base-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-337%2Fwilbaker%2Fmulti-pack-index-progress-toggle-v2\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-337/wilbaker/multi-pack-index-progress-toggle-v2\n> Pull-Request: https://github.com/gitgitgadget/git/pull/337\n>\n> Range-diff vs v1:\n>\n> -: ---------- > 1: 6badd9ceaf 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.\n> -: ---------- > 2: 3bc8677ea7 midx: add progress to write_midx_file Add progress to write_midx_file. Progress is displayed when the MIDX_PROGRESS flag is set.\n> -: ---------- > 3: 3374574001 midx: add progress to expire_midx_packs Add progress to expire_midx_packs. Progress is displayed when the MIDX_PROGRESS flag is set.\n> -: ---------- > 4: 29d03771c0 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.\n> -: ---------- > 5: 57f6742f09 midx: honor the MIDX_PROGRESS flag in midx_repack Update midx_repack to only display progress when the MIDX_PROGRESS flag is set.\n> 1: 0821a8073a ! 6: 5b933ab744 multi-pack-index: add --no-progress Add --no-progress option to git multi-pack-index. The progress feature was added in 144d703 (\"multi-pack-index: report progress during 'verify'\", 2018-09-13) but the ability to opt-out was overlooked.\n> @@ -1,10 +1,13 @@\n> Author: William Baker \n> \n> - multi-pack-index: add --no-progress\n> - Add --no-progress option to git multi-pack-index.\n> - The progress feature was added in 144d703\n> + multi-pack-index: add [--[no-]progress] option.\n> + Add the --[no-]progress option to git multi-pack-index.\n> + Pass the MIDX_PROGRESS flag to the subcommand functions\n> + when progress should be displayed by multi-pack-index.\n> + The progress feature was added to 'verify' in 144d703\n> (\"multi-pack-index: report progress during 'verify'\", 2018-09-13)\n> - but the ability to opt-out was overlooked.\n> + but some subcommands were not updated to display progress, and\n> + the ability to opt-out was overlooked.\n> \n> Signed-off-by: William Baker \n> \n> @@ -16,7 +19,7 @@\n> --------\n> [verse]\n> -'git multi-pack-index' [--object-dir=] \n> -+'git multi-pack-index' [--object-dir=] [--[no-]progress]\n> ++'git multi-pack-index' [--object-dir=] [--[no-]progress] \n> \n> DESCRIPTION\n> -----------\n> @@ -40,7 +43,7 @@\n> \n> static char const * const builtin_multi_pack_index_usage[] = {\n> -\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=)\"),\n> -+\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=) [--[no-]progress]\"),\n> ++\tN_(\"git multi-pack-index [] (write|verify|expire|repack --batch-size=)\"),\n> \tNULL\n> };\n> \n> @@ -58,11 +61,11 @@\n> \tstatic struct option builtin_multi_pack_index_options[] = {\n> \t\tOPT_FILENAME(0, \"object-dir\", &opts.object_dir,\n> \t\t N_(\"object directory containing set of packfile and pack-index pairs\")),\n> ++\t\tOPT_BOOL(0, \"progress\", &opts.progress, N_(\"force progress reporting\")),\n> \t\tOPT_MAGNITUDE(0, \"batch-size\", &opts.batch_size,\n> \t\t N_(\"during repack, collect pack-files of smaller size into a batch that is larger than this size\")),\n> -+\t\tOPT_BOOL(0, \"progress\", &opts.progress, N_(\"force progress reporting\")),\n> \t\tOPT_END(),\n> - \t};\n> +@@\n> \n> \tgit_config(git_default_config, NULL);\n> \n> @@ -82,131 +85,50 @@\n> \ttrace2_cmd_mode(argv[0]);\n> \n> \tif (!strcmp(argv[0], \"repack\"))\n> --\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size);\n> +-\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0);\n> +\t\treturn midx_repack(the_repository, opts.object_dir,\n> +\t\t\t(size_t)opts.batch_size, flags);\n> \tif (opts.batch_size)\n> \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n> \n> \tif (!strcmp(argv[0], \"write\"))\n> - \t\treturn write_midx_file(opts.object_dir);\n> +-\t\treturn write_midx_file(opts.object_dir, 0);\n> ++\t\treturn write_midx_file(opts.object_dir, flags);\n> \tif (!strcmp(argv[0], \"verify\"))\n> --\t\treturn verify_midx_file(the_repository, opts.object_dir);\n> +-\t\treturn verify_midx_file(the_repository, opts.object_dir, 0);\n> +\t\treturn verify_midx_file(the_repository, opts.object_dir, flags);\n> \tif (!strcmp(argv[0], \"expire\"))\n> - \t\treturn expire_midx_packs(the_repository, opts.object_dir);\n> +-\t\treturn expire_midx_packs(the_repository, opts.object_dir, 0);\n> ++\t\treturn expire_midx_packs(the_repository, opts.object_dir, flags);\n> \n> + \tdie(_(\"unrecognized subcommand: %s\"), argv[0]);\n> + }\n> \n> - diff --git a/midx.c b/midx.c\n> - --- a/midx.c\n> - +++ b/midx.c\n> -@@\n> - \t\t\tdisplay_progress(progress, _n); \\\n> - \t} while (0)\n> -\n> --int verify_midx_file(struct repository *r, const char *object_dir)\n> -+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags)\n> - {\n> - \tstruct pair_pos_vs_id *pairs = NULL;\n> - \tuint32_t i;\n> --\tstruct progress *progress;\n> -+\tstruct progress *progress = NULL;\n> - \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n> - \tverify_midx_error = 0;\n> -\n> - \tif (!m)\n> - \t\treturn 0;\n> -\n> --\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n> --\t\t\t\t m->num_packs);\n> -+\tif (flags & MIDX_PROGRESS)\n> -+\t\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n> -+\t\t\t\t \t m->num_packs);\n> - \tfor (i = 0; i < m->num_packs; i++) {\n> - \t\tif (prepare_midx_pack(r, m, i))\n> - \t\t\tmidx_report(\"failed to load pack in position %d\", i);\n> + diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n> + --- a/t/t5319-multi-pack-index.sh\n> + +++ b/t/t5319-multi-pack-index.sh\n> @@\n> - \t\t\t\t i, oid_fanout1, oid_fanout2, i + 1);\n> - \t}\n> -\n> --\tprogress = start_sparse_progress(_(\"Verifying OID order in MIDX\"),\n> --\t\t\t\t\t m->num_objects - 1);\n> -+\tif (flags & MIDX_PROGRESS)\n> -+\t\tprogress = start_sparse_progress(_(\"Verifying OID order in MIDX\"),\n> -+\t\t\t\t\t\t m->num_objects - 1);\n> - \tfor (i = 0; i < m->num_objects - 1; i++) {\n> - \t\tstruct object_id oid1, oid2;\n> \n> -@@\n> - \t\tpairs[i].pack_int_id = nth_midxed_pack_int_id(m, i);\n> - \t}\n> -\n> --\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n> --\t\t\t\t\t m->num_objects);\n> -+\tif (flags & MIDX_PROGRESS)\n> -+\t\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n> -+\t\t\t\t\t\t m->num_objects);\n> - \tdisplay_progress(progress, 0); /* TODO: Measure QSORT() progress */\n> - \tQSORT(pairs, m->num_objects, compare_pair_pos_vs_id);\n> - \tstop_progress(&progress);\n> -\n> --\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n> -+\tif (flags & MIDX_PROGRESS)\n> -+\t\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n> - \tfor (i = 0; i < m->num_objects; i++) {\n> - \t\tstruct object_id oid;\n> - \t\tstruct pack_entry e;\n> -@@\n> - \treturn 0;\n> - }\n> + compare_results_with_midx \"two packs\"\n> \n> --int midx_repack(struct repository *r, const char *object_dir, size_t batch_size)\n> -+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags)\n> - {\n> - \tint result = 0;\n> - \tuint32_t i;\n> -@@\n> - \tstrbuf_addstr(&base_name, object_dir);\n> - \tstrbuf_addstr(&base_name, \"/pack/pack\");\n> - \targv_array_push(&cmd.args, base_name.buf);\n> ++test_expect_success 'write progress off for redirected stderr' '\n> ++\tgit multi-pack-index --object-dir=$objdir write 2>err &&\n> ++\ttest_line_count = 0 err\n> ++'\n> +\n> -+\tif (flags & MIDX_PROGRESS)\n> -+\t\targv_array_push(&cmd.args, \"--progress\");\n> -+\telse\n> -+\t\targv_array_push(&cmd.args, \"-q\");\n> ++test_expect_success 'write force progress on for stderr' '\n> ++\tgit multi-pack-index --object-dir=$objdir --progress write 2>err &&\n> ++\ttest_file_not_empty err\n> ++'\n> +\n> - \tstrbuf_release(&base_name);\n> -\n> - \tcmd.git_cmd = 1;\n> -\n> - diff --git a/midx.h b/midx.h\n> - --- a/midx.h\n> - +++ b/midx.h\n> -@@\n> - \tchar object_dir[FLEX_ARRAY];\n> - };\n> -\n> -+#define MIDX_PROGRESS (1 << 0)\n> ++test_expect_success 'write with the --no-progress option' '\n> ++\tgit multi-pack-index --object-dir=$objdir --no-progress write 2>err &&\n> ++\ttest_line_count = 0 err\n> ++'\n> +\n> - struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local);\n> - int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id);\n> - int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result);\n> -@@\n> -\n> - int write_midx_file(const char *object_dir);\n> - void clear_midx_file(struct repository *r);\n> --int verify_midx_file(struct repository *r, const char *object_dir);\n> -+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags);\n> - int expire_midx_packs(struct repository *r, const char *object_dir);\n> --int midx_repack(struct repository *r, const char *object_dir, size_t batch_size);\n> -+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags);\n> -\n> - void close_midx(struct multi_pack_index *m);\n> -\n> -\n> - diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\n> - --- a/t/t5319-multi-pack-index.sh\n> - +++ b/t/t5319-multi-pack-index.sh\n> + test_expect_success 'add more packs' '\n> + \tfor j in $(test_seq 11 20)\n> + \tdo\n> @@\n> \tgit multi-pack-index verify --object-dir=$objdir\n> '\n> @@ -234,20 +156,51 @@\n> '\n> \n> +test_expect_success 'repack progress off for redirected stderr' '\n> -+\tgit multi-pack-index repack --object-dir=$objdir 2>err &&\n> ++\tgit multi-pack-index --object-dir=$objdir repack 2>err &&\n> +\ttest_line_count = 0 err\n> +'\n> +\n> +test_expect_success 'repack force progress on for stderr' '\n> -+\tgit multi-pack-index repack --object-dir=$objdir --progress 2>err &&\n> ++\tgit multi-pack-index --object-dir=$objdir --progress repack 2>err &&\n> +\ttest_file_not_empty err\n> +'\n> +\n> +test_expect_success 'repack with the --no-progress option' '\n> -+\tgit multi-pack-index repack --object-dir=$objdir --no-progress 2>err &&\n> ++\tgit multi-pack-index --object-dir=$objdir --no-progress repack 2>err &&\n> +\ttest_line_count = 0 err\n> +'\n> +\n> test_expect_success 'repack removes multi-pack-index' '\n> \ttest_path_is_file $objdir/pack/multi-pack-index &&\n> \tGIT_TEST_MULTI_PACK_INDEX=0 git repack -adf &&\n> +@@\n> + \t)\n> + '\n> +\n> ++test_expect_success 'expire progress off for redirected stderr' '\n> ++\t(\n> ++\t\tcd dup &&\n> ++\t\tgit multi-pack-index expire 2>err &&\n> ++\t\ttest_line_count = 0 err\n> ++\t)\n> ++'\n> ++\n> ++test_expect_success 'expire force progress on for stderr' '\n> ++\t(\n> ++\t\tcd dup &&\n> ++\t\tgit multi-pack-index --progress expire 2>err &&\n> ++\t\ttest_file_not_empty err\n> ++\t)\n> ++'\n> ++\n> ++test_expect_success 'expire with the --no-progress option' '\n> ++\t(\n> ++\t\tcd dup &&\n> ++\t\tgit multi-pack-index --no-progress expire 2>err &&\n> ++\t\ttest_line_count = 0 err\n> ++\t)\n> ++'\n> ++\n> + test_expect_success 'expire removes unreferenced packs' '\n> + \t(\n> + \t\tcd dup &&\n>\n\n"},{"id":"382707","messageId":"6b33f29b-d211-1548-05ec-c0863685b8a0@gmail.com","threadId":"51830","inReplyTo":"xmqqv9tmu4jz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 0/6] multi-pack-index: add --no-progress","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-09-20T20:33:48Z","receivedAt":"2019-09-20T20:33:51Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 9/20/19 12:54 PM, Junio C Hamano wrote:\n> \n> Do all of these commits have overly long title with no body recorded\n> in the commit objects, or is this just a mail sending program\n> screwing up?\n> \n\nSorry about that! With some help from Stolee I found the issue was that\nI was missing a second line break before the body of the commit.\n\n-William \n"},{"id":"382709","messageId":"247026fb-0429-bc0e-3ef3-9f2553dfebd7@gmail.com","threadId":"51830","inReplyTo":"1c94eaf5-fd62-cca3-2aeb-37512e8e41e6@iee.email","subject":"Re: [PATCH v2 0/6] multi-pack-index: add --no-progress","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-09-20T20:39:48Z","receivedAt":"2019-09-20T20:39:51Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 9/20/19 1:23 PM, Philip Oakley wrote:\n\n> These subject lines are a bit long. Has there been some white space damage during preparation, such that the end of line, and following blank line, for a short subject line been compromised?\n> \n> Normally the 'git log --oneline' should fit within the line, so ~50 chars.\n> \n> Philip\n> \n\nThanks for the tip regarding 'git log --oneline'! I missed that\nI need a second end of line before the body of the commit\nmessage and I will be sure to include them in future commits.\n\n-William\n"},{"id":"382722","messageId":"20190921121104.GA6787@szeder.dev","threadId":"51830","inReplyTo":"6badd9ceaf4851b2984e78a5cfd0cb8ec0c810f5.1568998427.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-09-21T12:11:04Z","receivedAt":"2019-09-21T12:13:23Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Sep 20, 2019 at 09:53:48AM -0700, William Baker via GitGitGadget wrote:\n> diff --git a/midx.h b/midx.h\n> index f0ae656b5d..e6fa356b5c 100644\n> --- a/midx.h\n> +++ b/midx.h\n> @@ -37,6 +37,8 @@ struct multi_pack_index {\n> \tchar object_dir[FLEX_ARRAY];\n> };\n> \n> +#define MIDX_PROGRESS (1 << 0)\n\nPlease consider using an enum.\n\nA preprocessor constant is just that: a number. It is shown as a\nnumber while debugging, and there is no clear indication what related\nvalues belong in the same group (apart from the prefix of the macro\nname), or what possible values an 'unsigned flags' variable might have\n(though in this particular case there is only a single possible\nvalue...). \n\nAn enum, however, is much friendlier to humans when debugging, because\neven 'gdb' shows the value using the symbolic names, e.g.:\n\n write_commit_graph_reachable (obj_dir=0x9ab870 \".git/objects\", \n flags=(COMMIT_GRAPH_WRITE_APPEND | COMMIT_GRAPH_WRITE_PROGRESS), \n split_opts=0x9670e0 ) at commit-graph.c:1141\n\nand it's quite clear what values a variable of type 'enum\ncommit_graph_write_flags' can have.\n\n"},{"id":"382797","messageId":"373fc8e9-2ef3-d501-b6e0-b953e047ac78@gmail.com","threadId":"51830","inReplyTo":"xmqqlfuiu3sv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 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.","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-09-23T21:12:54Z","receivedAt":"2019-09-23T21:12:58Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 9/20/19 1:10 PM, Junio C Hamano wrote:\n>> diff --git a/midx.c b/midx.c\n>> index b2673f52e8..54e4e93b2b 100644\n>> --- a/midx.c\n>> +++ b/midx.c\n>> @@ -449,6 +449,8 @@ struct pack_list {\n>> \tuint32_t nr;\n>> \tuint32_t alloc;\n>> \tstruct multi_pack_index *m;\n>> +\tstruct progress *progress;\n>> +\tuint32_t pack_paths_checked;\n>> };\n> \n> What is the reason behind u32 here? Do we want to be consistently\n> limited to 4G on all platforms?\n> \n> I have a feeling that we do not care too deeply all that much for\n> the purpose of progress output, in which case, just an ordinary\n> \"unsigned\" may be much less misleading.\n\nI went with u32 here to match the data type used to track how many\nentries are in the pack_list ('nr' is a u32).\n\nI could switch to this to an unsigned but we would run the (extremely\nunlikely) risk of having more than 65k packs on a system where\nunsigned is 16 bits.\n\n> FWIW, the function that uses this field is display_progress(), which\n> takes uint64_t there.\n\nThanks for pointing this out. Given that display_progress() expects a\nu64 using that type here for 'pack_paths_checked' could help make things\nmore straightforward.\n\n-William\n"},{"id":"382802","messageId":"b5ce3e27-3401-9726-18dc-358f69f1b996@gmail.com","threadId":"51830","inReplyTo":"20190921121104.GA6787@szeder.dev","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-09-23T21:55:59Z","receivedAt":"2019-09-23T21:56:02Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 9/21/19 5:11 AM, SZEDER Gábor wrote:\n>> \n>> +#define MIDX_PROGRESS (1 << 0)\n> \n> Please consider using an enum.\n> \n> A preprocessor constant is just that: a number. It is shown as a\n> number while debugging, and there is no clear indication what related\n> values belong in the same group (apart from the prefix of the macro\n> name), or what possible values an 'unsigned flags' variable might have\n> (though in this particular case there is only a single possible\n> value...). \n> \n> An enum, however, is much friendlier to humans when debugging, because\n> even 'gdb' shows the value using the symbolic names, e.g.:\n\nThanks for this suggestion. I agree on the benefits of using an enum\nhere and will make the switch in my next set of changes.\n\n-William\n"},{"id":"383077","messageId":"xmqqlfu9krzv.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"20190921121104.GA6787@szeder.dev","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-28T03:40:52Z","receivedAt":"2019-09-28T03:41:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n> On Fri, Sep 20, 2019 at 09:53:48AM -0700, William Baker via GitGitGadget wrote:\n>> diff --git a/midx.h b/midx.h\n>> index f0ae656b5d..e6fa356b5c 100644\n>> --- a/midx.h\n>> +++ b/midx.h\n>> @@ -37,6 +37,8 @@ struct multi_pack_index {\n>> \tchar object_dir[FLEX_ARRAY];\n>> };\n>> \n>> +#define MIDX_PROGRESS (1 << 0)\n>\n> Please consider using an enum.\n\nIf they are used by assiging one of their values, definitely a good\nidea to use an enum. Are debuggers clever enough that they can\ntell, when they see something like this:\n\n\tenum gress {\n\t\tPROGRESS = 1,\n\t\tREGRESS = 2,\n\t};\n\n\tvoid func(enum gress v);\n\n\t...\n\n void caller(void)\n\t{\n\t\tfunc(PROGRESS | REGRESS);\n\t\tfunc(PROGRESS + REGRESS);\n\t\tfunc(PROGRESS * 3);\n\t}\n\nhow caller came about to give 3?\n"},{"id":"383078","messageId":"xmqqftkhkrl6.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"373fc8e9-2ef3-d501-b6e0-b953e047ac78@gmail.com","subject":"Re: [PATCH v2 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.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-09-28T03:49:41Z","receivedAt":"2019-09-28T03:49:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"William Baker writes:\n\n> On 9/20/19 1:10 PM, Junio C Hamano wrote:\n>>> diff --git a/midx.c b/midx.c\n>>> index b2673f52e8..54e4e93b2b 100644\n>>> --- a/midx.c\n>>> +++ b/midx.c\n>>> @@ -449,6 +449,8 @@ struct pack_list {\n>>> \tuint32_t nr;\n>>> \tuint32_t alloc;\n>>> \tstruct multi_pack_index *m;\n>>> +\tstruct progress *progress;\n>>> +\tuint32_t pack_paths_checked;\n>>> };\n>> \n>> What is the reason behind u32 here? Do we want to be consistently\n>> limited to 4G on all platforms?\n>> \n>> I have a feeling that we do not care too deeply all that much for\n>> the purpose of progress output, in which case, just an ordinary\n>> \"unsigned\" may be much less misleading.\n>\n> I went with u32 here to match the data type used to track how many\n> entries are in the pack_list ('nr' is a u32).\n\nThat kind of parallel is valid when you could compare nr with this\nnew thing (or put it differently, the existing nr and this new thing\ncounts the same). Are they both about the number of packs?\n\n> I could switch to this to an unsigned but we would run the (extremely\n> unlikely) risk of having more than 65k packs on a system where\n> unsigned is 16 bits.\n\nWhy? If an arch is small enough that the natural integer size is 16-bit,\nthen limiting the total number of packs to 65k sound entirely\nsensible.\n\nThe only reason why you'd want fixed (across platforms and\narchitectures) type is when you want to make sure that a file\nstoring the literal values taken from these fields are portable and\neverybody is limited the same way. If a platform's natural integer\nis 64-bit, by artificially limiting the size of this field to u32,\nyou are making disservice to the platform users, and more\nimportantly, you are wasting developers' time by forcing them to\nwonder if there is a reason behind the choice of u32 (does it really\nneed to be able to store up to 4G, even on a smaller machines? Is\nit necessary to refuse to store more than 4G? What are the\nreasons?), like me wondering about these questions and writing them\ndown here.\n\nSo, unless there is a reason why this must be of fixed type, I'd say\njust an unsigned would be the most reasonable choice.\n"},{"id":"383160","messageId":"07d054d2-c37e-5a8a-9550-c1d90bb80af7@gmail.com","threadId":"51830","inReplyTo":"xmqqftkhkrl6.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 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.","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-09-30T16:36:20Z","receivedAt":"2019-09-30T16:36:23Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 9/27/19 8:49 PM, Junio C Hamano wrote:\n>>>> diff --git a/midx.c b/midx.c\n>>>> index b2673f52e8..54e4e93b2b 100644\n>>>> --- a/midx.c\n>>>> +++ b/midx.c\n>>>> @@ -449,6 +449,8 @@ struct pack_list {\n>>>> \tuint32_t nr;\n>>>> \tuint32_t alloc;\n>>>> \tstruct multi_pack_index *m;\n>>>> +\tstruct progress *progress;\n>>>> +\tuint32_t pack_paths_checked;\n>>>> };\n>>\n>> I went with u32 here to match the data type used to track how many\n>> entries are in the pack_list ('nr' is a u32).\n> \n> That kind of parallel is valid when you could compare nr with this\n> new thing (or put it differently, the existing nr and this new thing\n> counts the same). Are they both about the number of packs?\n> \n\nBoth 'nr' and 'pack_paths_checked' are about the number of packs. \n'nr' tracks the number of packs in the multi-pack-index and it grows\nas add_pack_to_midx() finds new packs to add. 'pack_paths_checked' is\nthe number of pack \".idx\" files that have been checked by add_pack_to_midx().\n\n>> I could switch to this to an unsigned but we would run the (extremely\n>> unlikely) risk of having more than 65k packs on a system where\n>> unsigned is 16 bits.\n> \n> Why? If an arch is small enough that the natural integer size is 16-bit,\n> then limiting the total number of packs to 65k sound entirely\n> sensible.> The only reason why you'd want fixed (across platforms and\n> architectures) type is when you want to make sure that a file\n> storing the literal values taken from these fields are portable and\n> everybody is limited the same way. If a platform's natural integer\n> is 64-bit, by artificially limiting the size of this field to u32,\n> you are making disservice to the platform users, and more\n> importantly, you are wasting developers' time by forcing them to\n> wonder if there is a reason behind the choice of u32 (does it really\n> need to be able to store up to 4G, even on a smaller machines? Is\n> it necessary to refuse to store more than 4G? What are the\n> reasons?), like me wondering about these questions and writing them\n> down here.\n> \n> So, unless there is a reason why this must be of fixed type, I'd say\n> just an unsigned would be the most reasonable choice.\n> \n\nI agree that it's best to avoid using a fixed type here unless there's\na reason that it must be. Do you think that both 'nr' and\n'pack_paths_checked' being about the number of packs is a strong enough\nreason to use u32 for 'pack_paths_checked'? If not, I will update\n'pack_paths_checked' in the next path series to be an unsigned int.\n\nThanks,\nWilliam\n\n\n\n"},{"id":"383162","messageId":"2de6b236-7bd8-256b-7d8f-911d63a47498@gmail.com","threadId":"51830","inReplyTo":"xmqqlfu9krzv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-09-30T17:01:55Z","receivedAt":"2019-09-30T17:01:58Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 9/27/19 8:40 PM, Junio C Hamano wrote:\n> SZEDER Gábor writes:\n> \n>>> +#define MIDX_PROGRESS (1 << 0)\n>>\n>> Please consider using an enum.\n> \n> If they are used by assiging one of their values, definitely a good\n> idea to use an enum. Are debuggers clever enough that they can\n> tell, when they see something like this:\n> \n> \tenum gress {\n> \t\tPROGRESS = 1,\n> \t\tREGRESS = 2,\n> \t};\n> \n> \tvoid func(enum gress v);\n> \n> \t...\n> \n> void caller(void)\n> \t{\n> \t\tfunc(PROGRESS | REGRESS);\n> \t\tfunc(PROGRESS + REGRESS);\n> \t\tfunc(PROGRESS * 3);\n> \t}\n> \n> how caller came about to give 3?\n> \n\nMy debugger was not smart enough to figure out what flags were combined\nto give the value of 3 in this example. \n\nI saw that the code base is currently a mix of #define and enums when it\ncomes to flags (e.g. dir_struct.flags and rebase_options.flags are both\nenums) and so using one here would not be something new stylistically.\n\nAlthough my debugger might not be the smartest, I haven't noticed any\ndownsides to switching this to an enum.\n\nThanks,\nWilliam\n"},{"id":"383242","messageId":"xmqqwodng10v.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"2de6b236-7bd8-256b-7d8f-911d63a47498@gmail.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-02T05:38:24Z","receivedAt":"2019-10-02T05:38:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"William Baker writes:\n\n> I saw that the code base is currently a mix of #define and enums when it\n> comes to flags (e.g. dir_struct.flags and rebase_options.flags are both\n> enums) and so using one here would not be something new stylistically.\n\nYes. But it is a different matter to spread a bad practice to\nparts of the code that haven't been contaminated by it.\n"},{"id":"383244","messageId":"xmqqsgobg0rv.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"2de6b236-7bd8-256b-7d8f-911d63a47498@gmail.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-02T05:43:48Z","receivedAt":"2019-10-02T05:43:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"William Baker writes:\n\n> Although my debugger might not be the smartest, I haven't noticed any\n> downsides to switching this to an enum.\n\nWell, if you write\n\n\tenum { BIT_0 = 1, BIT_1 = 2, BIT_3 = 4 } var;\n\nit's pretty much a promise that the normal value for the var is one\nof these listed values to your readers. But bit flags are meant to\nbe used combined (after all, they are cheaper alternative for 1-bit\nwide bitfields in a structure), so it is misleading to use enum as\nsuch.\n"},{"id":"383255","messageId":"xmqqlfu3eigp.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"xmqqsgobg0rv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-02T07:04:38Z","receivedAt":"2019-10-02T07:04:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> William Baker writes:\n>\n>> Although my debugger might not be the smartest, I haven't noticed any\n>> downsides to switching this to an enum.\n>\n> Well, if you write\n>\n> \tenum { BIT_0 = 1, BIT_1 = 2, BIT_3 = 4 } var;\n>\n> it's pretty much a promise that the normal value for the var is one\n> of these listed values to your readers.\n\n... that is why compilers give a warning when you write\n\n\tswitch (var) {\n\tcase ...:\n\t...\n\t}\n\nand do not have case arms for all the declared enum values without\nhaving the 'default' arm.\n"},{"id":"383282","messageId":"8ec5513c-fcfa-bc55-5d73-6aa6528ac4dc@gmail.com","threadId":"51830","inReplyTo":"xmqqlfu3eigp.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-10-02T15:56:45Z","receivedAt":"2019-10-02T15:56:49Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 10/2/19 12:04 AM, Junio C Hamano wrote>>\n>> Well, if you write\n>>\n>> \tenum { BIT_0 = 1, BIT_1 = 2, BIT_3 = 4 } var;\n>>\n>> it's pretty much a promise that the normal value for the var is one\n>> of these listed values to your readers.\n> \n> ... that is why compilers give a warning when you write\n> \n> \tswitch (var) {\n> \tcase ...:\n> \t...\n> \t}\n> \n> and do not have case arms for all the declared enum values without\n> having the 'default' arm.\n> \n\nThanks for all of the details and feedback here. I can leave the\nflag as-is (and not switch to enum).\n\nThanks,\nWilliam\n"},{"id":"383334","messageId":"pull.337.v3.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v2.git.gitgitgadget@gmail.com","subject":"[PATCH v3 0/6] multi-pack-index: add --no-progress","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-03T17:53:31Z","receivedAt":"2019-10-03T17:53:36Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"Hello Git contributors,\n\nThis is the third iteration of changes for adding support for\n--[no-]progress to multi-pack-index, and it includes the following updates\nfor the feedback I received on v2:\n\n * Fixed commit message formatting\n * Updated 'pack_paths_checked' from u32 to unsigned\n\nThanks, William\n\nWilliam Baker (6):\n midx: add MIDX_PROGRESS flag\n midx: add progress to write_midx_file\n midx: add progress to expire_midx_packs\n midx: honor the MIDX_PROGRESS flag in verify_midx_file\n midx: honor the MIDX_PROGRESS flag in midx_repack\n multi-pack-index: add [--[no-]progress] option.\n\n Documentation/git-multi-pack-index.txt | 6 ++-\n builtin/multi-pack-index.c | 18 +++++--\n builtin/repack.c | 2 +-\n midx.c | 71 ++++++++++++++++++++------\n midx.h | 10 ++--\n t/t5319-multi-pack-index.sh | 69 +++++++++++++++++++++++++\n 6 files changed, 149 insertions(+), 27 deletions(-)\n\n\nbase-commit: 5fa0f5238b0cd46cfe7f6fa76c3f526ea98148d9\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-337%2Fwilbaker%2Fmulti-pack-index-progress-toggle-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-337/wilbaker/multi-pack-index-progress-toggle-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/337\n\nRange-diff vs v2:\n\n 1: 6badd9ceaf ! 1: cd041107de 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.\n @@ -1,6 +1,7 @@\n Author: William Baker \n \n midx: add MIDX_PROGRESS flag\n +\n Add the MIDX_PROGRESS flag and update the\n write|verify|expire|repack functions in midx.h\n to accept a flags parameter. The MIDX_PROGRESS\n 2: 3bc8677ea7 ! 2: 0117f55c9d midx: add progress to write_midx_file Add progress to write_midx_file. Progress is displayed when the MIDX_PROGRESS flag is set.\n @@ -1,6 +1,7 @@\n Author: William Baker \n \n midx: add progress to write_midx_file\n +\n Add progress to write_midx_file. Progress is displayed\n when the MIDX_PROGRESS flag is set.\n \n @@ -14,7 +15,7 @@\n \tuint32_t alloc;\n \tstruct multi_pack_index *m;\n +\tstruct progress *progress;\n -+\tuint32_t pack_paths_checked;\n ++\tunsigned pack_paths_checked;\n };\n \n static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n 3: 3374574001 ! 3: d741dc60bc midx: add progress to expire_midx_packs Add progress to expire_midx_packs. Progress is displayed when the MIDX_PROGRESS flag is set.\n @@ -1,6 +1,7 @@\n Author: William Baker \n \n midx: add progress to expire_midx_packs\n +\n Add progress to expire_midx_packs. Progress is\n displayed when the MIDX_PROGRESS flag is set.\n \n 4: 29d03771c0 ! 4: f208c09639 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.\n @@ -1,6 +1,7 @@\n Author: William Baker \n \n midx: honor the MIDX_PROGRESS flag in verify_midx_file\n +\n Update verify_midx_file to only display progress when\n the MIDX_PROGRESS flag is set.\n \n 5: 57f6742f09 ! 5: 7566090769 midx: honor the MIDX_PROGRESS flag in midx_repack Update midx_repack to only display progress when the MIDX_PROGRESS flag is set.\n @@ -1,6 +1,7 @@\n Author: William Baker \n \n midx: honor the MIDX_PROGRESS flag in midx_repack\n +\n Update midx_repack to only display progress when\n the MIDX_PROGRESS flag is set.\n \n 6: 5b933ab744 ! 6: a3c50948d9 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.\n @@ -1,6 +1,7 @@\n Author: William Baker \n \n multi-pack-index: add [--[no-]progress] option.\n +\n Add the --[no-]progress option to git multi-pack-index.\n Pass the MIDX_PROGRESS flag to the subcommand functions\n when progress should be displayed by multi-pack-index.\n\n-- \ngitgitgadget\n"},{"id":"383335","messageId":"cd041107defd0032043737c060a0aba1326d6ac2.1570125210.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 1/6] midx: add MIDX_PROGRESS flag","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-03T17:53:31Z","receivedAt":"2019-10-03T17:53:37Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nAdd the MIDX_PROGRESS flag and update the\nwrite|verify|expire|repack functions in midx.h\nto accept a flags parameter. The MIDX_PROGRESS\nflag indicates whether the caller of the function\nwould like progress information to be displayed.\nThis patch only changes the method prototypes\nand does not change the functionality. The\nfunctionality change will be handled by a later patch.\n\nSigned-off-by: William Baker \n---\n builtin/multi-pack-index.c | 8 ++++----\n builtin/repack.c | 2 +-\n midx.c | 8 ++++----\n midx.h | 10 ++++++----\n 4 files changed, 15 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex b1ea1a6aa1..e86b8cd17d 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -47,16 +47,16 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \ttrace2_cmd_mode(argv[0]);\n \n \tif (!strcmp(argv[0], \"repack\"))\n-\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size);\n+\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0);\n \tif (opts.batch_size)\n \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n \n \tif (!strcmp(argv[0], \"write\"))\n-\t\treturn write_midx_file(opts.object_dir);\n+\t\treturn write_midx_file(opts.object_dir, 0);\n \tif (!strcmp(argv[0], \"verify\"))\n-\t\treturn verify_midx_file(the_repository, opts.object_dir);\n+\t\treturn verify_midx_file(the_repository, opts.object_dir, 0);\n \tif (!strcmp(argv[0], \"expire\"))\n-\t\treturn expire_midx_packs(the_repository, opts.object_dir);\n+\t\treturn expire_midx_packs(the_repository, opts.object_dir, 0);\n \n \tdie(_(\"unrecognized subcommand: %s\"), argv[0]);\n }\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 632c0c0a79..5b3623337f 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -561,7 +561,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \tremove_temporary_files();\n \n \tif (git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0))\n-\t\twrite_midx_file(get_object_directory());\n+\t\twrite_midx_file(get_object_directory(), 0);\n \n \tstring_list_clear(&names, 0);\n \tstring_list_clear(&rollback, 0);\ndiff --git a/midx.c b/midx.c\nindex d649644420..b2673f52e8 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1017,7 +1017,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \treturn result;\n }\n \n-int write_midx_file(const char *object_dir)\n+int write_midx_file(const char *object_dir, unsigned flags)\n {\n \treturn write_midx_internal(object_dir, NULL, NULL);\n }\n@@ -1077,7 +1077,7 @@ static int compare_pair_pos_vs_id(const void *_a, const void *_b)\n \t\t\tdisplay_progress(progress, _n); \\\n \t} while (0)\n \n-int verify_midx_file(struct repository *r, const char *object_dir)\n+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags)\n {\n \tstruct pair_pos_vs_id *pairs = NULL;\n \tuint32_t i;\n@@ -1184,7 +1184,7 @@ int verify_midx_file(struct repository *r, const char *object_dir)\n \treturn verify_midx_error;\n }\n \n-int expire_midx_packs(struct repository *r, const char *object_dir)\n+int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags)\n {\n \tuint32_t i, *count, result = 0;\n \tstruct string_list packs_to_drop = STRING_LIST_INIT_DUP;\n@@ -1316,7 +1316,7 @@ static int fill_included_packs_batch(struct repository *r,\n \treturn 0;\n }\n \n-int midx_repack(struct repository *r, const char *object_dir, size_t batch_size)\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags)\n {\n \tint result = 0;\n \tuint32_t i;\ndiff --git a/midx.h b/midx.h\nindex f0ae656b5d..e6fa356b5c 100644\n--- a/midx.h\n+++ b/midx.h\n@@ -37,6 +37,8 @@ struct multi_pack_index {\n \tchar object_dir[FLEX_ARRAY];\n };\n \n+#define MIDX_PROGRESS (1 << 0)\n+\n struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local);\n int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id);\n int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result);\n@@ -47,11 +49,11 @@ int fill_midx_entry(struct repository *r, const struct object_id *oid, struct pa\n int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name);\n int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local);\n \n-int write_midx_file(const char *object_dir);\n+int write_midx_file(const char *object_dir, unsigned flags);\n void clear_midx_file(struct repository *r);\n-int verify_midx_file(struct repository *r, const char *object_dir);\n-int expire_midx_packs(struct repository *r, const char *object_dir);\n-int midx_repack(struct repository *r, const char *object_dir, size_t batch_size);\n+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags);\n+int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags);\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags);\n \n void close_midx(struct multi_pack_index *m);\n \n-- \ngitgitgadget\n\n"},{"id":"383337","messageId":"d741dc60bcd05093894e3a258fc95e9b26592d12.1570125210.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 3/6] midx: add progress to expire_midx_packs","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-03T17:53:33Z","receivedAt":"2019-10-03T17:53:37Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nAdd progress to expire_midx_packs. Progress is\ndisplayed when the MIDX_PROGRESS flag is set.\n\nSigned-off-by: William Baker \n---\n midx.c | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex 0808a40dd4..f14bebb092 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1206,18 +1206,29 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla\n \tuint32_t i, *count, result = 0;\n \tstruct string_list packs_to_drop = STRING_LIST_INIT_DUP;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n+\tstruct progress *progress = NULL;\n \n \tif (!m)\n \t\treturn 0;\n \n \tcount = xcalloc(m->num_packs, sizeof(uint32_t));\n+\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Counting referenced objects\"), \n+\t\t\t\t\t m->num_objects);\n \tfor (i = 0; i < m->num_objects; i++) {\n \t\tint pack_int_id = nth_midxed_pack_int_id(m, i);\n \t\tcount[pack_int_id]++;\n+\t\tdisplay_progress(progress, i + 1);\n \t}\n+\tstop_progress(&progress);\n \n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Finding and deleting unreferenced packfiles\"),\n+\t\t\t\t\t m->num_packs);\n \tfor (i = 0; i < m->num_packs; i++) {\n \t\tchar *pack_name;\n+\t\tdisplay_progress(progress, i + 1);\n \n \t\tif (count[i])\n \t\t\tcontinue;\n@@ -1235,6 +1246,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla\n \t\tunlink_pack_path(pack_name, 0);\n \t\tfree(pack_name);\n \t}\n+\tstop_progress(&progress);\n \n \tfree(count);\n \n-- \ngitgitgadget\n\n"},{"id":"383336","messageId":"0117f55c9d98a641af11bfdd81b4c7c997a19dfb.1570125210.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 2/6] midx: add progress to write_midx_file","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-03T17:53:32Z","receivedAt":"2019-10-03T17:53:38Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nAdd progress to write_midx_file. Progress is displayed\nwhen the MIDX_PROGRESS flag is set.\n\nSigned-off-by: William Baker \n---\n midx.c | 25 +++++++++++++++++++++----\n 1 file changed, 21 insertions(+), 4 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex b2673f52e8..0808a40dd4 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -449,6 +449,8 @@ struct pack_list {\n \tuint32_t nr;\n \tuint32_t alloc;\n \tstruct multi_pack_index *m;\n+\tstruct progress *progress;\n+\tunsigned pack_paths_checked;\n };\n \n static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n@@ -457,6 +459,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n \tstruct pack_list *packs = (struct pack_list *)data;\n \n \tif (ends_with(file_name, \".idx\")) {\n+\t\tdisplay_progress(packs->progress, ++packs->pack_paths_checked);\n \t\tif (packs->m && midx_contains_pack(packs->m, file_name))\n \t\t\treturn;\n \n@@ -786,7 +789,7 @@ static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_off\n }\n \n static int write_midx_internal(const char *object_dir, struct multi_pack_index *m,\n-\t\t\t struct string_list *packs_to_drop)\n+\t\t\t struct string_list *packs_to_drop, unsigned flags)\n {\n \tunsigned char cur_chunk, num_chunks = 0;\n \tchar *midx_name;\n@@ -800,6 +803,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \tuint64_t chunk_offsets[MIDX_MAX_CHUNKS + 1];\n \tuint32_t nr_entries, num_large_offsets = 0;\n \tstruct pack_midx_entry *entries = NULL;\n+\tstruct progress *progress = NULL;\n \tint large_offsets_needed = 0;\n \tint pack_name_concat_len = 0;\n \tint dropped_packs = 0;\n@@ -833,8 +837,15 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \t\t\tpacks.nr++;\n \t\t}\n \t}\n+\t\n+\tpacks.pack_paths_checked = 0;\n+\tif (flags & MIDX_PROGRESS)\n+\t\tpacks.progress = start_progress(_(\"Adding packfiles to multi-pack-index\"), 0);\n+\telse\n+\t\tpacks.progress = NULL;\n \n \tfor_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs);\n+\tstop_progress(&packs.progress);\n \n \tif (packs.m && packs.nr == packs.m->num_packs && !packs_to_drop)\n \t\tgoto cleanup;\n@@ -959,6 +970,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \t\twritten += MIDX_CHUNKLOOKUP_WIDTH;\n \t}\n \n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Writing chunks to multi-pack-index\"),\n+\t\t\t\t\t num_chunks);\n \tfor (i = 0; i < num_chunks; i++) {\n \t\tif (written != chunk_offsets[i])\n \t\t\tBUG(\"incorrect chunk offset (%\"PRIu64\" != %\"PRIu64\") for chunk id %\"PRIx32,\n@@ -991,7 +1005,10 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \t\t\t\tBUG(\"trying to write unknown chunk id %\"PRIx32,\n \t\t\t\t chunk_ids[i]);\n \t\t}\n+\n+\t\tdisplay_progress(progress, i + 1);\n \t}\n+\tstop_progress(&progress);\n \n \tif (written != chunk_offsets[num_chunks])\n \t\tBUG(\"incorrect final offset %\"PRIu64\" != %\"PRIu64,\n@@ -1019,7 +1036,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \n int write_midx_file(const char *object_dir, unsigned flags)\n {\n-\treturn write_midx_internal(object_dir, NULL, NULL);\n+\treturn write_midx_internal(object_dir, NULL, NULL, flags);\n }\n \n void clear_midx_file(struct repository *r)\n@@ -1222,7 +1239,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla\n \tfree(count);\n \n \tif (packs_to_drop.nr)\n-\t\tresult = write_midx_internal(object_dir, m, &packs_to_drop);\n+\t\tresult = write_midx_internal(object_dir, m, &packs_to_drop, flags);\n \n \tstring_list_clear(&packs_to_drop, 0);\n \treturn result;\n@@ -1371,7 +1388,7 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,\n \t\tgoto cleanup;\n \t}\n \n-\tresult = write_midx_internal(object_dir, m, NULL);\n+\tresult = write_midx_internal(object_dir, m, NULL, flags);\n \tm = NULL;\n \n cleanup:\n-- \ngitgitgadget\n\n"},{"id":"383338","messageId":"7566090769eb74b2c9c99ab1455fb84c9215be16.1570125210.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 5/6] midx: honor the MIDX_PROGRESS flag in midx_repack","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-03T17:53:35Z","receivedAt":"2019-10-03T17:53:38Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nUpdate midx_repack to only display progress when\nthe MIDX_PROGRESS flag is set.\n\nSigned-off-by: William Baker \n---\n midx.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex ced5898bbf..1c5ddeb007 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1374,6 +1374,12 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,\n \tstrbuf_addstr(&base_name, object_dir);\n \tstrbuf_addstr(&base_name, \"/pack/pack\");\n \targv_array_push(&cmd.args, base_name.buf);\n+\n+\tif (flags & MIDX_PROGRESS)\n+\t\targv_array_push(&cmd.args, \"--progress\");\n+\telse\n+\t\targv_array_push(&cmd.args, \"-q\");\n+\n \tstrbuf_release(&base_name);\n \n \tcmd.git_cmd = 1;\n-- \ngitgitgadget\n\n"},{"id":"383339","messageId":"f208c09639257877479ed33b607a26f7cf435038.1570125210.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 4/6] midx: honor the MIDX_PROGRESS flag in verify_midx_file","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-03T17:53:34Z","receivedAt":"2019-10-03T17:53:39Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nUpdate verify_midx_file to only display progress when\nthe MIDX_PROGRESS flag is set.\n\nSigned-off-by: William Baker \n---\n midx.c | 20 ++++++++++++--------\n 1 file changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex f14bebb092..ced5898bbf 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1098,15 +1098,16 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n {\n \tstruct pair_pos_vs_id *pairs = NULL;\n \tuint32_t i;\n-\tstruct progress *progress;\n+\tstruct progress *progress = NULL;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n \tverify_midx_error = 0;\n \n \tif (!m)\n \t\treturn 0;\n \n-\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n-\t\t\t\t m->num_packs);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n+\t\t\t\t \t m->num_packs);\n \tfor (i = 0; i < m->num_packs; i++) {\n \t\tif (prepare_midx_pack(r, m, i))\n \t\t\tmidx_report(\"failed to load pack in position %d\", i);\n@@ -1124,8 +1125,9 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n \t\t\t\t i, oid_fanout1, oid_fanout2, i + 1);\n \t}\n \n-\tprogress = start_sparse_progress(_(\"Verifying OID order in MIDX\"),\n-\t\t\t\t\t m->num_objects - 1);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Verifying OID order in multi-pack-index\"),\n+\t\t\t\t\t\t m->num_objects - 1);\n \tfor (i = 0; i < m->num_objects - 1; i++) {\n \t\tstruct object_id oid1, oid2;\n \n@@ -1152,13 +1154,15 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n \t\tpairs[i].pack_int_id = nth_midxed_pack_int_id(m, i);\n \t}\n \n-\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n-\t\t\t\t\t m->num_objects);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n+\t\t\t\t\t\t m->num_objects);\n \tdisplay_progress(progress, 0); /* TODO: Measure QSORT() progress */\n \tQSORT(pairs, m->num_objects, compare_pair_pos_vs_id);\n \tstop_progress(&progress);\n \n-\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n \tfor (i = 0; i < m->num_objects; i++) {\n \t\tstruct object_id oid;\n \t\tstruct pack_entry e;\n-- \ngitgitgadget\n\n"},{"id":"383340","messageId":"a3c50948d9df62c18228c4c1a1a99da1250237c2.1570125210.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v3 6/6] multi-pack-index: add [--[no-]progress] option.","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-03T17:53:36Z","receivedAt":"2019-10-03T17:53:41Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nAdd the --[no-]progress option to git multi-pack-index.\nPass the MIDX_PROGRESS flag to the subcommand functions\nwhen progress should be displayed by multi-pack-index.\nThe progress feature was added to 'verify' in 144d703\n(\"multi-pack-index: report progress during 'verify'\", 2018-09-13)\nbut some subcommands were not updated to display progress, and\nthe ability to opt-out was overlooked.\n\nSigned-off-by: William Baker \n---\n Documentation/git-multi-pack-index.txt | 6 ++-\n builtin/multi-pack-index.c | 18 +++++--\n t/t5319-multi-pack-index.sh | 69 ++++++++++++++++++++++++++\n 3 files changed, 87 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex 233b2b7862..d7a02cc6fa 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -9,7 +9,7 @@ git-multi-pack-index - Write and verify multi-pack-indexes\n SYNOPSIS\n --------\n [verse]\n-'git multi-pack-index' [--object-dir=] \n+'git multi-pack-index' [--object-dir=] [--[no-]progress] \n \n DESCRIPTION\n -----------\n@@ -23,6 +23,10 @@ OPTIONS\n \t`/packs/multi-pack-index` for the current MIDX file, and\n \t`/packs` for the pack-files to index.\n \n+--[no-]progress::\n+\tTurn progress on/off explicitly. If neither is specified, progress is \n+\tshown if standard error is connected to a terminal.\n+\n The following subcommands are available:\n \n write::\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex e86b8cd17d..1730b21901 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -6,21 +6,25 @@\n #include \"trace2.h\"\n \n static char const * const builtin_multi_pack_index_usage[] = {\n-\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=)\"),\n+\tN_(\"git multi-pack-index [] (write|verify|expire|repack --batch-size=)\"),\n \tNULL\n };\n \n static struct opts_multi_pack_index {\n \tconst char *object_dir;\n \tunsigned long batch_size;\n+\tint progress;\n } opts;\n \n int cmd_multi_pack_index(int argc, const char **argv,\n \t\t\t const char *prefix)\n {\n+\tunsigned flags = 0;\n+\n \tstatic struct option builtin_multi_pack_index_options[] = {\n \t\tOPT_FILENAME(0, \"object-dir\", &opts.object_dir,\n \t\t N_(\"object directory containing set of packfile and pack-index pairs\")),\n+\t\tOPT_BOOL(0, \"progress\", &opts.progress, N_(\"force progress reporting\")),\n \t\tOPT_MAGNITUDE(0, \"batch-size\", &opts.batch_size,\n \t\t N_(\"during repack, collect pack-files of smaller size into a batch that is larger than this size\")),\n \t\tOPT_END(),\n@@ -28,12 +32,15 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \n \tgit_config(git_default_config, NULL);\n \n+\topts.progress = isatty(2);\n \targc = parse_options(argc, argv, prefix,\n \t\t\t builtin_multi_pack_index_options,\n \t\t\t builtin_multi_pack_index_usage, 0);\n \n \tif (!opts.object_dir)\n \t\topts.object_dir = get_object_directory();\n+\tif (opts.progress)\n+\t\tflags |= MIDX_PROGRESS;\n \n \tif (argc == 0)\n \t\tusage_with_options(builtin_multi_pack_index_usage,\n@@ -47,16 +54,17 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \ttrace2_cmd_mode(argv[0]);\n \n \tif (!strcmp(argv[0], \"repack\"))\n-\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0);\n+\t\treturn midx_repack(the_repository, opts.object_dir, \n+\t\t\t(size_t)opts.batch_size, flags);\n \tif (opts.batch_size)\n \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n \n \tif (!strcmp(argv[0], \"write\"))\n-\t\treturn write_midx_file(opts.object_dir, 0);\n+\t\treturn write_midx_file(opts.object_dir, flags);\n \tif (!strcmp(argv[0], \"verify\"))\n-\t\treturn verify_midx_file(the_repository, opts.object_dir, 0);\n+\t\treturn verify_midx_file(the_repository, opts.object_dir, flags);\n \tif (!strcmp(argv[0], \"expire\"))\n-\t\treturn expire_midx_packs(the_repository, opts.object_dir, 0);\n+\t\treturn expire_midx_packs(the_repository, opts.object_dir, flags);\n \n \tdie(_(\"unrecognized subcommand: %s\"), argv[0]);\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex c72ca04399..cd2f87be6a 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -147,6 +147,21 @@ test_expect_success 'write midx with two packs' '\n \n compare_results_with_midx \"two packs\"\n \n+test_expect_success 'write progress off for redirected stderr' '\n+\tgit multi-pack-index --object-dir=$objdir write 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'write force progress on for stderr' '\n+\tgit multi-pack-index --object-dir=$objdir --progress write 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'write with the --no-progress option' '\n+\tgit multi-pack-index --object-dir=$objdir --no-progress write 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n test_expect_success 'add more packs' '\n \tfor j in $(test_seq 11 20)\n \tdo\n@@ -169,6 +184,21 @@ test_expect_success 'verify multi-pack-index success' '\n \tgit multi-pack-index verify --object-dir=$objdir\n '\n \n+test_expect_success 'verify progress off for redirected stderr' '\n+\tgit multi-pack-index verify --object-dir=$objdir 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'verify force progress on for stderr' '\n+\tgit multi-pack-index verify --object-dir=$objdir --progress 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'verify with the --no-progress option' '\n+\tgit multi-pack-index verify --object-dir=$objdir --no-progress 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n # usage: corrupt_midx_and_verify \n corrupt_midx_and_verify() {\n \tPOS=$1 &&\n@@ -284,6 +314,21 @@ test_expect_success 'git-fsck incorrect offset' '\n \t\t\"git -c core.multipackindex=true fsck\"\n '\n \n+test_expect_success 'repack progress off for redirected stderr' '\n+\tgit multi-pack-index --object-dir=$objdir repack 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'repack force progress on for stderr' '\n+\tgit multi-pack-index --object-dir=$objdir --progress repack 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'repack with the --no-progress option' '\n+\tgit multi-pack-index --object-dir=$objdir --no-progress repack 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tGIT_TEST_MULTI_PACK_INDEX=0 git repack -adf &&\n@@ -413,6 +458,30 @@ test_expect_success 'expire does not remove any packs' '\n \t)\n '\n \n+test_expect_success 'expire progress off for redirected stderr' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit multi-pack-index expire 2>err &&\n+\t\ttest_line_count = 0 err\n+\t)\n+'\n+\n+test_expect_success 'expire force progress on for stderr' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit multi-pack-index --progress expire 2>err &&\n+\t\ttest_file_not_empty err\n+\t)\n+'\n+\n+test_expect_success 'expire with the --no-progress option' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit multi-pack-index --no-progress expire 2>err &&\n+\t\ttest_line_count = 0 err\n+\t)\n+'\n+\n test_expect_success 'expire removes unreferenced packs' '\n \t(\n \t\tcd dup &&\n-- \ngitgitgadget\n"},{"id":"383375","messageId":"xmqqtv8pa1lx.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"pull.337.v3.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/6] multi-pack-index: add --no-progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-03T22:46:50Z","receivedAt":"2019-10-03T22:46:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"William Baker via GitGitGadget\" writes:\n\n> This is the third iteration of changes for adding support for\n> --[no-]progress to multi-pack-index, and it includes the following updates\n> for the feedback I received on v2:\n>\n> * Fixed commit message formatting\n> * Updated 'pack_paths_checked' from u32 to unsigned\n>\n> Thanks, William\n\nThanks, will take a look.\n"},{"id":"383590","messageId":"20191007171249.GB11529@szeder.dev","threadId":"51830","inReplyTo":"xmqqsgobg0rv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-07T17:12:49Z","receivedAt":"2019-10-07T17:12:55Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Oct 02, 2019 at 02:43:48PM +0900, Junio C Hamano wrote:\n> William Baker writes:\n> \n> > Although my debugger might not be the smartest, I haven't noticed any\n> > downsides to switching this to an enum.\n> \n> Well, if you write\n> \n> \tenum { BIT_0 = 1, BIT_1 = 2, BIT_3 = 4 } var;\n> \n> it's pretty much a promise that the normal value for the var is one\n> of these listed values to your readers. But bit flags are meant to\n> be used combined (after all, they are cheaper alternative for 1-bit\n> wide bitfields in a structure), so it is misleading to use enum as\n> such.\n\nHaving the combination of enum constants with power-of-two values is\nnot misleading, but rather an idiom.\n\nBack when debugging the racy split index issues I only saw gibberish\nlike this:\n\n (gdb) p ce->ce_flags \n $2 = 469762048\n\nWith an enum I now have this instead:\n\n (gdb) p ce->ce_flags\n $2 = (CE_MATCHED | CE_UPDATE_IN_BASE | CE_STRIP_NAME)\n\nThe latter is about as many times more readable as the int value of\nthat 'ce_flags'.\n\nThe sooner everyone gets on board with this the better.\n\n"},{"id":"383593","messageId":"20191007172951.GC11529@szeder.dev","threadId":"51830","inReplyTo":"xmqqlfu9krzv.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-07T17:29:51Z","receivedAt":"2019-10-07T17:29:58Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Sep 28, 2019 at 12:40:52PM +0900, Junio C Hamano wrote:\n> SZEDER Gábor writes:\n> \n> > On Fri, Sep 20, 2019 at 09:53:48AM -0700, William Baker via GitGitGadget wrote:\n> >> diff --git a/midx.h b/midx.h\n> >> index f0ae656b5d..e6fa356b5c 100644\n> >> --- a/midx.h\n> >> +++ b/midx.h\n> >> @@ -37,6 +37,8 @@ struct multi_pack_index {\n> >> \tchar object_dir[FLEX_ARRAY];\n> >> };\n> >> \n> >> +#define MIDX_PROGRESS (1 << 0)\n> >\n> > Please consider using an enum.\n> \n> If they are used by assiging one of their values, definitely a good\n> idea to use an enum. Are debuggers clever enough that they can\n> tell, when they see something like this:\n> \n> \tenum gress {\n> \t\tPROGRESS = 1,\n> \t\tREGRESS = 2,\n> \t};\n> \n> \tvoid func(enum gress v);\n> \n> \t...\n> \n> void caller(void)\n> \t{\n> \t\tfunc(PROGRESS | REGRESS);\n> \t\tfunc(PROGRESS + REGRESS);\n> \t\tfunc(PROGRESS * 3);\n> \t}\n> \n> how caller came about to give 3?\n\nNo, they tend to show (PROGRESS | REGRESS), at least both gdb and lldb\ndo. If the enum has only constants with power-of-two values, then that\nis the right way to write it, and the other two are asking for trouble\n(e.g. after someone changes the value of REGRESS from 2 to 8,\n'PROGRESS * 3' will mean something different).\n\n"},{"id":"383652","messageId":"xmqqk19fn9jp.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"20191007172951.GC11529@szeder.dev","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-08T04:30:34Z","receivedAt":"2019-10-08T04:30:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor writes:\n\n>> \t\tfunc(PROGRESS | REGRESS);\n>> \t\tfunc(PROGRESS + REGRESS);\n>> \t\tfunc(PROGRESS * 3);\n>> \t}\n>> \n>> how caller came about to give 3?\n>\n> No, they tend to show (PROGRESS | REGRESS), at least both gdb and lldb\n> do.\n\nOK.\n\n> If the enum has only constants with power-of-two values, then that\n> is the right way to write it, and the other two are asking for trouble\n\nIf the programmer and the debugger knows the constants are used to\nrepresent bits that can be or'ed together, what you say is correct,\nbut that is entirely irrelevant.\n\nWhat I was worried about is that the constants that are used to\nrepresent something that are *NOT* set of bits (hence \"PROGRESS * 3\"\nmay be perfectly a reasonable thing for such an application) may be\nmistaken by an overly clever debugger and \"3\" may end up getting\nshown as \"PROGRESS | REGRESS\". When there are only two constants\n(PROGRESS=1 and REGRESS=2), we humans nor debuggers can tell if that\nis to represent two bits that can be or'ed together, or it is an\nenumeration.\n\nUntil we gain the third constant, that is, at which time the third\none may likely be 3 (if enumeration) or 4 (if bits).\n\n"},{"id":"383689","messageId":"202ed2c5-9729-120f-fa0e-cee9ecec3b77@gmail.com","threadId":"51830","inReplyTo":"xmqqk19fn9jp.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-10-08T16:24:56Z","receivedAt":"2019-10-08T16:25:00Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 10/7/19 9:30 PM, Junio C Hamano wrote:\n> SZEDER Gábor writes:\n> \n>>> \t\tfunc(PROGRESS | REGRESS);\n>>> \t\tfunc(PROGRESS + REGRESS);\n>>> \t\tfunc(PROGRESS * 3);\n>>> \t}\n>>>\n>>> how caller came about to give 3?\n>>\n>> No, they tend to show (PROGRESS | REGRESS), at least both gdb and lldb\n>> do.\n> \n> OK.\n> \n>> If the enum has only constants with power-of-two values, then that\n>> is the right way to write it, and the other two are asking for trouble\n> \n> If the programmer and the debugger knows the constants are used to\n> represent bits that can be or'ed together, what you say is correct,\n> but that is entirely irrelevant.\n> \n> What I was worried about is that the constants that are used to\n> represent something that are *NOT* set of bits (hence \"PROGRESS * 3\"\n> may be perfectly a reasonable thing for such an application) may be\n> mistaken by an overly clever debugger and \"3\" may end up getting\n> shown as \"PROGRESS | REGRESS\". When there are only two constants\n> (PROGRESS=1 and REGRESS=2), we humans nor debuggers can tell if that\n> is to represent two bits that can be or'ed together, or it is an\n> enumeration.\n> \n> Until we gain the third constant, that is, at which time the third\n> one may likely be 3 (if enumeration) or 4 (if bits).\n> \n\nI tried to see how lldb would handle the \"PROGRESS * 3\" scenario\nbut I was unable to get lldb to display the \"PROGRESS | REGRESS\" format\neven when ORing the flags:\n\n(lldb) l 399\n 399 \t\tenum test_flags {\n 400 \t\t\tTEST_FLAG_1 = 1 << 0,\n 401 \t\t\tTEST_FLAG_2 = 1 << 1,\n 402 \t\t};\n 403 \t\t\n 404 \t\tenum test_flags flags_1 = TEST_FLAG_1;\n 405 \t\tenum test_flags flags_2 = TEST_FLAG_2;\n 406 \t\tenum test_flags flags_both = TEST_FLAG_1 | TEST_FLAG_2;\n 407 \t\t\n 408 \t\tif (flags_1 || flags_2 || flags_both)\n(lldb) p flags_1\n(test_flags) $0 = TEST_FLAG_1\n(lldb) p flags_2\n(test_flags) $1 = TEST_FLAG_2\n(lldb) p flags_both\n(test_flags) $2 = 3\n(lldb) fr v flags_both\n(test_flags) flags_both = 3\n(lldb) fr v --format enum flags_both\n(test_flags) flags_both = 3\n(lldb) version\nlldb-902.0.79.7\n Swift-4.1\n\nIs there something that needs to be adjusted in the config or with\n--format to display \"TEST_FLAG_1 | TEST_FLAG_2\" in this example?\n\nThanks,\nWilliam \n"},{"id":"383715","messageId":"20191008232439.GA7212@szeder.dev","threadId":"51830","inReplyTo":"202ed2c5-9729-120f-fa0e-cee9ecec3b77@gmail.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-09T00:16:40Z","receivedAt":"2019-10-09T00:16:49Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Oct 08, 2019 at 09:24:56AM -0700, William Baker wrote:\n> On 10/7/19 9:30 PM, Junio C Hamano wrote:\n> > SZEDER Gábor writes:\n> > \n> >>> \t\tfunc(PROGRESS | REGRESS);\n> >>> \t\tfunc(PROGRESS + REGRESS);\n> >>> \t\tfunc(PROGRESS * 3);\n> >>> \t}\n> >>>\n> >>> how caller came about to give 3?\n> >>\n> >> No, they tend to show (PROGRESS | REGRESS), at least both gdb and lldb\n> >> do.\n\n> I tried to see how lldb would handle the \"PROGRESS * 3\" scenario\n> but I was unable to get lldb to display the \"PROGRESS | REGRESS\" format\n> even when ORing the flags:\n> \n> (lldb) l 399\n> 399 \t\tenum test_flags {\n> 400 \t\t\tTEST_FLAG_1 = 1 << 0,\n> 401 \t\t\tTEST_FLAG_2 = 1 << 1,\n> 402 \t\t};\n> 403 \t\t\n> 404 \t\tenum test_flags flags_1 = TEST_FLAG_1;\n> 405 \t\tenum test_flags flags_2 = TEST_FLAG_2;\n> 406 \t\tenum test_flags flags_both = TEST_FLAG_1 | TEST_FLAG_2;\n> 407 \t\t\n> 408 \t\tif (flags_1 || flags_2 || flags_both)\n> (lldb) p flags_1\n> (test_flags) $0 = TEST_FLAG_1\n> (lldb) p flags_2\n> (test_flags) $1 = TEST_FLAG_2\n> (lldb) p flags_both\n> (test_flags) $2 = 3\n> (lldb) fr v flags_both\n> (test_flags) flags_both = 3\n> (lldb) fr v --format enum flags_both\n> (test_flags) flags_both = 3\n> (lldb) version\n> lldb-902.0.79.7\n> Swift-4.1\n> \n> Is there something that needs to be adjusted in the config or with\n> --format to display \"TEST_FLAG_1 | TEST_FLAG_2\" in this example?\n\nI'm afraid you are right and lldb doesn't do this, at least not by\ndefault, what a pity. I tried different versions of gdb and lldb,\ndistro-shipped older ones and some more recent in containers a couple\nof days ago; now I tried it again and see the same behavior as you, so\nI must have misremembered. Sorry for the confusion.\n\ngdb still does the user-friendly thing, though.\n\n"},{"id":"383719","messageId":"20191009013231.GF29845@szeder.dev","threadId":"51830","inReplyTo":"xmqqk19fn9jp.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-10-09T01:32:31Z","receivedAt":"2019-10-09T01:32:38Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Tue, Oct 08, 2019 at 01:30:34PM +0900, Junio C Hamano wrote:\n> SZEDER Gábor writes:\n> \n> >> \t\tfunc(PROGRESS | REGRESS);\n> >> \t\tfunc(PROGRESS + REGRESS);\n> >> \t\tfunc(PROGRESS * 3);\n> >> \t}\n> >> \n> >> how caller came about to give 3?\n> >\n> > No, they tend to show (PROGRESS | REGRESS), at least both gdb and lldb\n> > do.\n\nI was wrong here, gdb does this, but lldb, unfortunately, doesn't; see\nmy other reply in this thread.\n\n> OK.\n> \n> > If the enum has only constants with power-of-two values, then that\n> > is the right way to write it, and the other two are asking for trouble\n> \n> If the programmer and the debugger knows the constants are used to\n> represent bits that can be or'ed together, what you say is correct,\n> but that is entirely irrelevant.\n> \n> What I was worried about is that the constants that are used to\n> represent something that are *NOT* set of bits (hence \"PROGRESS * 3\"\n> may be perfectly a reasonable thing for such an application)\n\nI don't really see how that could be reasonable, it's prone to break\nwhen changing the values of the enum constants.\n\n> may be\n> mistaken by an overly clever debugger and \"3\" may end up getting\n> shown as \"PROGRESS | REGRESS\". When there are only two constants\n> (PROGRESS=1 and REGRESS=2), we humans nor debuggers can tell if that\n> is to represent two bits that can be or'ed together, or it is an\n> enumeration.\n> \n> Until we gain the third constant, that is, at which time the third\n> one may likely be 3 (if enumeration) or 4 (if bits).\n\nHumans benefit from context: they understand the name of the enum type\n(e.g. does it end with \"_flags\"?), the name of the enum constants, and\nthe comment above the enum's definition (if any), and can then infer\nwhether those constants represent OR-able bits or not. If they can't\nfind this out, then that enum is poorly named and/or documented, which\nshould be fixed. As for the patch that I originally commented on, I\nwould expect the enum to be called e.g. 'midx_flags', and thus already\nwith that single constant in there it'll be clear that it is intended\nas a collection of related OR-able bits.\n\nAs for the debugger, if it sees a variable of an enum type whose value\ndoesn't match any of the enum constants, then there are basically\nthree possibilities:\n\n - All constants in that enum have power-of-two values. In this case\n it's reasonable from the debugger to assume that those constants\n are OR'ed together, and is extremely helpful to display the value\n that way.\n\n - The constants are just a set of values (1, 2, 3, 42, etc). In\n this case the variable shouldn't have a value that doesn't match\n one of the constants in the first place, and I would first suspect\n that the program might be buggy.\n\n - A \"mostly\" power-of-two enum might contain shorthand constants for\n combinations of a set of other constants, e.g.:\n\n enum flags {\n BIT0 = (1 << 0),\n BIT1 = (1 << 1),\n BIT2 = (1 << 2),\n\n FIRST_TWO = (BIT0 | BIT1),\n };\n enum flags f0 = BIT0;\n enum flags f1 = BIT0 | BIT1;\n enum flags f2 = BIT0 | BIT2;\n enum flags f3 = BIT0 | BIT1 | BIT2;\n\n In this case, sadly, gdb shows only matching constants:\n\n (gdb) p f0\n $1 = BIT0\n (gdb) p f1\n $2 = FIRST_TWO\n (gdb) p f2\n $3 = 5\n (gdb) p f3\n $4 = 7\n\n"},{"id":"384118","messageId":"04342d12-fffc-afb6-fa4e-c2e2bf88d1b6@gmail.com","threadId":"51830","inReplyTo":"20191009013231.GF29845@szeder.dev","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-10-15T20:00:45Z","receivedAt":"2019-10-15T20:00:50Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 10/8/19 6:32 PM, SZEDER Gábor wrote:\n>>> No, they tend to show (PROGRESS | REGRESS), at least both gdb and lldb\n>>> do.\n> \n> I was wrong here, gdb does this, but lldb, unfortunately, doesn't; see\n> my other reply in this thread.\n>\n>> What I was worried about is that the constants that are used to\n>> represent something that are *NOT* set of bits (hence \"PROGRESS * 3\"\n>> may be perfectly a reasonable thing for such an application)\n> \n> I don't really see how that could be reasonable, it's prone to break\n> when changing the values of the enum constants.\n> \n>> may be\n>> mistaken by an overly clever debugger and \"3\" may end up getting\n>> shown as \"PROGRESS | REGRESS\". When there are only two constants\n>> (PROGRESS=1 and REGRESS=2), we humans nor debuggers can tell if that\n>> is to represent two bits that can be or'ed together, or it is an\n>> enumeration.\n>>\n>> Until we gain the third constant, that is, at which time the third\n>> one may likely be 3 (if enumeration) or 4 (if bits).\n> \n> Humans benefit from context: they understand the name of the enum type\n> (e.g. does it end with \"_flags\"?), the name of the enum constants, and\n> the comment above the enum's definition (if any), and can then infer\n> whether those constants represent OR-able bits or not. If they can't\n> find this out, then that enum is poorly named and/or documented, which\n> should be fixed. As for the patch that I originally commented on, I\n> would expect the enum to be called e.g. 'midx_flags', and thus already\n> with that single constant in there it'll be clear that it is intended\n> as a collection of related OR-able bits.\n> \n> As for the debugger, if it sees a variable of an enum type whose value\n> doesn't match any of the enum constants, then there are basically\n> three possibilities:\n> \n> - All constants in that enum have power-of-two values. In this case\n> it's reasonable from the debugger to assume that those constants\n> are OR'ed together, and is extremely helpful to display the value\n> that way.\n> \n> - The constants are just a set of values (1, 2, 3, 42, etc). In\n> this case the variable shouldn't have a value that doesn't match\n> one of the constants in the first place, and I would first suspect\n> that the program might be buggy.\n> \n> - A \"mostly\" power-of-two enum might contain shorthand constants for\n> combinations of a set of other constants, e.g.:\n> \n> enum flags {\n> BIT0 = (1 << 0),\n> BIT1 = (1 << 1),\n> BIT2 = (1 << 2),\n> \n> FIRST_TWO = (BIT0 | BIT1),\n> };\n> enum flags f0 = BIT0;\n> enum flags f1 = BIT0 | BIT1;\n> enum flags f2 = BIT0 | BIT2;\n> enum flags f3 = BIT0 | BIT1 | BIT2;\n> \n> In this case, sadly, gdb shows only matching constants:\n> \n> (gdb) p f0\n> $1 = BIT0\n> (gdb) p f1\n> $2 = FIRST_TWO\n> (gdb) p f2\n> $3 = 5\n> (gdb) p f3\n> $4 = 7\n> \n\nI believe this is the last open thread/discussion on the \"midx: add MIDX_PROGRESS\nflag\" patch series. \n\nA quick summary of where the discussion stands:\n\n- The patch series currently uses #define for the new MIDX_PROGRESS flag.\n- The git codebase uses both #defines and enums for flags.\n- The debugger always shows the enum value's name if there is a match, if values\n are OR-ed together there a few possibilities (see discussion above and earlier\n in the thread).\n- There are concerns regarding misinterpreting constants that are not a set of\n bits (e.g. \"PROGRESS * 3\").\n\nAre there any other details I can provide that would help in reaching a\nconclusion as to how to proceed?\n\nAt this time there are no other MIDX_* flags and so there's always the option\nto revisit the best way to handle multiple MIDX_* flags if/when additional\nflags are added.\n\nThanks,\nWilliam\n\n"},{"id":"384154","messageId":"xmqqh849a1b4.fsf@gitster-ct.c.googlers.com","threadId":"51830","inReplyTo":"04342d12-fffc-afb6-fa4e-c2e2bf88d1b6@gmail.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-16T02:09:35Z","receivedAt":"2019-10-16T02:09:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"William Baker writes:\n\n> At this time there are no other MIDX_* flags and so there's always the option\n> to revisit the best way to handle multiple MIDX_* flags if/when additional\n> flags are added.\n\nI do not care too deeply either way, but if you wrote it in one way,\nhow about completing the series without changing it in the middle,\nand leave the clean-ups to a follow-up series (if needed)?\n"},{"id":"384210","messageId":"35cd8f8c-de66-819e-0a9f-c0905255721d@gmail.com","threadId":"51830","inReplyTo":"xmqqh849a1b4.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-10-16T19:48:59Z","receivedAt":"2019-10-16T19:49:03Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 10/15/19 7:09 PM, Junio C Hamano wrote:\n>> At this time there are no other MIDX_* flags and so there's always the option\n>> to revisit the best way to handle multiple MIDX_* flags if/when additional\n>> flags are added.\n> \n> I do not care too deeply either way, but if you wrote it in one way,\n> how about completing the series without changing it in the middle,\n> and leave the clean-ups to a follow-up series (if needed)?\n\n\nThat plan sounds good to me. The most recent series (v3) should be ready\nto go, I don't believe there is any outstanding feedback to address.\n\nThanks,\nWilliam\n"},{"id":"384416","messageId":"ca215a07-79b4-7b7f-7dbe-a0541120cde0@gmail.com","threadId":"51830","inReplyTo":"35cd8f8c-de66-819e-0a9f-c0905255721d@gmail.com","subject":"Re: [PATCH v2 1/6] midx: add MIDX_PROGRESS flag ","fromName":"William Baker","fromEmail":"williamtbakeremail@gmail.com","sentAt":"2019-10-18T21:35:45Z","receivedAt":"2019-10-18T21:35:48Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"On 10/16/19 12:48 PM, William Baker wrote:>> I do not care too deeply either way, but if you wrote it in one way,\n>> how about completing the series without changing it in the middle,\n>> and leave the clean-ups to a follow-up series (if needed)?\n> \n> \n> That plan sounds good to me. The most recent series (v3) should be ready\n> to go, I don't believe there is any outstanding feedback to address.\n\nFollow-up question on this patch series.\n\nI noticed there is a branch named 'wb/midx-progress' in\nhttps://github.com/gitster/git but it does not appear to have the commits\nfrom this patch series and I have not seen it mentioned in\n\"What's cooking in git.git\" emails.\n\nIs there anything else I should do to get these changes picked up in 'pu'?\n\nThanks again,\nWilliam\n"},{"id":"384539","messageId":"pull.337.v4.git.1571683203.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v3.git.gitgitgadget@gmail.com","subject":"[PATCH v4 0/6] multi-pack-index: add --no-progress","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-21T18:39:57Z","receivedAt":"2019-10-21T18:40:07Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"Hello Git contributors,\n\nThis is the fourth iteration of changes for adding support for\n--[no-]progress to multi-pack-index. \n\nI'm resubmitting the series after a discussion regarding 'MIDX_PROGRESS'\nthat concluded with a decision to stick with the original changes in the v3\npatch.\n\nThanks, William\n\nWilliam Baker (6):\n midx: add MIDX_PROGRESS flag\n midx: add progress to write_midx_file\n midx: add progress to expire_midx_packs\n midx: honor the MIDX_PROGRESS flag in verify_midx_file\n midx: honor the MIDX_PROGRESS flag in midx_repack\n multi-pack-index: add [--[no-]progress] option.\n\n Documentation/git-multi-pack-index.txt | 6 ++-\n builtin/multi-pack-index.c | 18 +++++--\n builtin/repack.c | 2 +-\n midx.c | 71 ++++++++++++++++++++------\n midx.h | 10 ++--\n t/t5319-multi-pack-index.sh | 69 +++++++++++++++++++++++++\n 6 files changed, 149 insertions(+), 27 deletions(-)\n\n\nbase-commit: d966095db01190a2196e31195ea6fa0c722aa732\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-337%2Fwilbaker%2Fmulti-pack-index-progress-toggle-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-337/wilbaker/multi-pack-index-progress-toggle-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/337\n\nRange-diff vs v3:\n\n 1: cd041107de = 1: e7b1cc633b midx: add MIDX_PROGRESS flag\n 2: 0117f55c9d = 2: 37768f5886 midx: add progress to write_midx_file\n 3: d741dc60bc = 3: 4ac9527d98 midx: add progress to expire_midx_packs\n 4: f208c09639 = 4: 3468acf39a midx: honor the MIDX_PROGRESS flag in verify_midx_file\n 5: 7566090769 = 5: d6ae4e1c54 midx: honor the MIDX_PROGRESS flag in midx_repack\n 6: a3c50948d9 = 6: ea4f869780 multi-pack-index: add [--[no-]progress] option.\n\n-- \ngitgitgadget\n"},{"id":"384540","messageId":"4ac9527d984bffea22a399506500a18823b4bd53.1571683203.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v4.git.1571683203.gitgitgadget@gmail.com","subject":"[PATCH v4 3/6] midx: add progress to expire_midx_packs","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-21T18:40:00Z","receivedAt":"2019-10-21T18:40:09Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nAdd progress to expire_midx_packs. Progress is\ndisplayed when the MIDX_PROGRESS flag is set.\n\nSigned-off-by: William Baker \n---\n midx.c | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex 006f36b570..7157259ca2 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1205,18 +1205,29 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla\n \tuint32_t i, *count, result = 0;\n \tstruct string_list packs_to_drop = STRING_LIST_INIT_DUP;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n+\tstruct progress *progress = NULL;\n \n \tif (!m)\n \t\treturn 0;\n \n \tcount = xcalloc(m->num_packs, sizeof(uint32_t));\n+\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Counting referenced objects\"), \n+\t\t\t\t\t m->num_objects);\n \tfor (i = 0; i < m->num_objects; i++) {\n \t\tint pack_int_id = nth_midxed_pack_int_id(m, i);\n \t\tcount[pack_int_id]++;\n+\t\tdisplay_progress(progress, i + 1);\n \t}\n+\tstop_progress(&progress);\n \n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Finding and deleting unreferenced packfiles\"),\n+\t\t\t\t\t m->num_packs);\n \tfor (i = 0; i < m->num_packs; i++) {\n \t\tchar *pack_name;\n+\t\tdisplay_progress(progress, i + 1);\n \n \t\tif (count[i])\n \t\t\tcontinue;\n@@ -1234,6 +1245,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla\n \t\tunlink_pack_path(pack_name, 0);\n \t\tfree(pack_name);\n \t}\n+\tstop_progress(&progress);\n \n \tfree(count);\n \n-- \ngitgitgadget\n\n"},{"id":"384541","messageId":"e7b1cc633bc8f5dbdb4f113456d20f1f11476662.1571683203.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v4.git.1571683203.gitgitgadget@gmail.com","subject":"[PATCH v4 1/6] midx: add MIDX_PROGRESS flag","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-21T18:39:58Z","receivedAt":"2019-10-21T18:40:10Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nAdd the MIDX_PROGRESS flag and update the\nwrite|verify|expire|repack functions in midx.h\nto accept a flags parameter. The MIDX_PROGRESS\nflag indicates whether the caller of the function\nwould like progress information to be displayed.\nThis patch only changes the method prototypes\nand does not change the functionality. The\nfunctionality change will be handled by a later patch.\n\nSigned-off-by: William Baker \n---\n builtin/multi-pack-index.c | 8 ++++----\n builtin/repack.c | 2 +-\n midx.c | 8 ++++----\n midx.h | 10 ++++++----\n 4 files changed, 15 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex b1ea1a6aa1..e86b8cd17d 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -47,16 +47,16 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \ttrace2_cmd_mode(argv[0]);\n \n \tif (!strcmp(argv[0], \"repack\"))\n-\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size);\n+\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0);\n \tif (opts.batch_size)\n \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n \n \tif (!strcmp(argv[0], \"write\"))\n-\t\treturn write_midx_file(opts.object_dir);\n+\t\treturn write_midx_file(opts.object_dir, 0);\n \tif (!strcmp(argv[0], \"verify\"))\n-\t\treturn verify_midx_file(the_repository, opts.object_dir);\n+\t\treturn verify_midx_file(the_repository, opts.object_dir, 0);\n \tif (!strcmp(argv[0], \"expire\"))\n-\t\treturn expire_midx_packs(the_repository, opts.object_dir);\n+\t\treturn expire_midx_packs(the_repository, opts.object_dir, 0);\n \n \tdie(_(\"unrecognized subcommand: %s\"), argv[0]);\n }\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 094c2f8ea4..397081d568 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -562,7 +562,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \tremove_temporary_files();\n \n \tif (git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0))\n-\t\twrite_midx_file(get_object_directory());\n+\t\twrite_midx_file(get_object_directory(), 0);\n \n \tstring_list_clear(&names, 0);\n \tstring_list_clear(&rollback, 0);\ndiff --git a/midx.c b/midx.c\nindex f29afc0d2d..f169a681dd 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1016,7 +1016,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \treturn result;\n }\n \n-int write_midx_file(const char *object_dir)\n+int write_midx_file(const char *object_dir, unsigned flags)\n {\n \treturn write_midx_internal(object_dir, NULL, NULL);\n }\n@@ -1076,7 +1076,7 @@ static int compare_pair_pos_vs_id(const void *_a, const void *_b)\n \t\t\tdisplay_progress(progress, _n); \\\n \t} while (0)\n \n-int verify_midx_file(struct repository *r, const char *object_dir)\n+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags)\n {\n \tstruct pair_pos_vs_id *pairs = NULL;\n \tuint32_t i;\n@@ -1183,7 +1183,7 @@ int verify_midx_file(struct repository *r, const char *object_dir)\n \treturn verify_midx_error;\n }\n \n-int expire_midx_packs(struct repository *r, const char *object_dir)\n+int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags)\n {\n \tuint32_t i, *count, result = 0;\n \tstruct string_list packs_to_drop = STRING_LIST_INIT_DUP;\n@@ -1315,7 +1315,7 @@ static int fill_included_packs_batch(struct repository *r,\n \treturn 0;\n }\n \n-int midx_repack(struct repository *r, const char *object_dir, size_t batch_size)\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags)\n {\n \tint result = 0;\n \tuint32_t i;\ndiff --git a/midx.h b/midx.h\nindex f0ae656b5d..e6fa356b5c 100644\n--- a/midx.h\n+++ b/midx.h\n@@ -37,6 +37,8 @@ struct multi_pack_index {\n \tchar object_dir[FLEX_ARRAY];\n };\n \n+#define MIDX_PROGRESS (1 << 0)\n+\n struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local);\n int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id);\n int bsearch_midx(const struct object_id *oid, struct multi_pack_index *m, uint32_t *result);\n@@ -47,11 +49,11 @@ int fill_midx_entry(struct repository *r, const struct object_id *oid, struct pa\n int midx_contains_pack(struct multi_pack_index *m, const char *idx_or_pack_name);\n int prepare_multi_pack_index_one(struct repository *r, const char *object_dir, int local);\n \n-int write_midx_file(const char *object_dir);\n+int write_midx_file(const char *object_dir, unsigned flags);\n void clear_midx_file(struct repository *r);\n-int verify_midx_file(struct repository *r, const char *object_dir);\n-int expire_midx_packs(struct repository *r, const char *object_dir);\n-int midx_repack(struct repository *r, const char *object_dir, size_t batch_size);\n+int verify_midx_file(struct repository *r, const char *object_dir, unsigned flags);\n+int expire_midx_packs(struct repository *r, const char *object_dir, unsigned flags);\n+int midx_repack(struct repository *r, const char *object_dir, size_t batch_size, unsigned flags);\n \n void close_midx(struct multi_pack_index *m);\n \n-- \ngitgitgadget\n\n"},{"id":"384542","messageId":"d6ae4e1c54b50d33571ce0529da43ba1d41a630e.1571683203.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v4.git.1571683203.gitgitgadget@gmail.com","subject":"[PATCH v4 5/6] midx: honor the MIDX_PROGRESS flag in midx_repack","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-21T18:40:02Z","receivedAt":"2019-10-21T18:40:11Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nUpdate midx_repack to only display progress when\nthe MIDX_PROGRESS flag is set.\n\nSigned-off-by: William Baker \n---\n midx.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git a/midx.c b/midx.c\nindex 35bdc5f72c..b5f7f1c54a 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1373,6 +1373,12 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,\n \tstrbuf_addstr(&base_name, object_dir);\n \tstrbuf_addstr(&base_name, \"/pack/pack\");\n \targv_array_push(&cmd.args, base_name.buf);\n+\n+\tif (flags & MIDX_PROGRESS)\n+\t\targv_array_push(&cmd.args, \"--progress\");\n+\telse\n+\t\targv_array_push(&cmd.args, \"-q\");\n+\n \tstrbuf_release(&base_name);\n \n \tcmd.git_cmd = 1;\n-- \ngitgitgadget\n\n"},{"id":"384543","messageId":"3468acf39ad402826878136f3c3ba7ab076ec0c3.1571683203.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v4.git.1571683203.gitgitgadget@gmail.com","subject":"[PATCH v4 4/6] midx: honor the MIDX_PROGRESS flag in verify_midx_file","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-21T18:40:01Z","receivedAt":"2019-10-21T18:40:12Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nUpdate verify_midx_file to only display progress when\nthe MIDX_PROGRESS flag is set.\n\nSigned-off-by: William Baker \n---\n midx.c | 20 ++++++++++++--------\n 1 file changed, 12 insertions(+), 8 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex 7157259ca2..35bdc5f72c 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -1097,15 +1097,16 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n {\n \tstruct pair_pos_vs_id *pairs = NULL;\n \tuint32_t i;\n-\tstruct progress *progress;\n+\tstruct progress *progress = NULL;\n \tstruct multi_pack_index *m = load_multi_pack_index(object_dir, 1);\n \tverify_midx_error = 0;\n \n \tif (!m)\n \t\treturn 0;\n \n-\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n-\t\t\t\t m->num_packs);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Looking for referenced packfiles\"),\n+\t\t\t\t \t m->num_packs);\n \tfor (i = 0; i < m->num_packs; i++) {\n \t\tif (prepare_midx_pack(r, m, i))\n \t\t\tmidx_report(\"failed to load pack in position %d\", i);\n@@ -1123,8 +1124,9 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n \t\t\t\t i, oid_fanout1, oid_fanout2, i + 1);\n \t}\n \n-\tprogress = start_sparse_progress(_(\"Verifying OID order in MIDX\"),\n-\t\t\t\t\t m->num_objects - 1);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Verifying OID order in multi-pack-index\"),\n+\t\t\t\t\t\t m->num_objects - 1);\n \tfor (i = 0; i < m->num_objects - 1; i++) {\n \t\tstruct object_id oid1, oid2;\n \n@@ -1151,13 +1153,15 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n \t\tpairs[i].pack_int_id = nth_midxed_pack_int_id(m, i);\n \t}\n \n-\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n-\t\t\t\t\t m->num_objects);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Sorting objects by packfile\"),\n+\t\t\t\t\t\t m->num_objects);\n \tdisplay_progress(progress, 0); /* TODO: Measure QSORT() progress */\n \tQSORT(pairs, m->num_objects, compare_pair_pos_vs_id);\n \tstop_progress(&progress);\n \n-\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_sparse_progress(_(\"Verifying object offsets\"), m->num_objects);\n \tfor (i = 0; i < m->num_objects; i++) {\n \t\tstruct object_id oid;\n \t\tstruct pack_entry e;\n-- \ngitgitgadget\n\n"},{"id":"384544","messageId":"ea4f869780fc95ccaf00fa9b95a0df3a53f88951.1571683203.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v4.git.1571683203.gitgitgadget@gmail.com","subject":"[PATCH v4 6/6] multi-pack-index: add [--[no-]progress] option.","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-21T18:40:03Z","receivedAt":"2019-10-21T18:40:12Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nAdd the --[no-]progress option to git multi-pack-index.\nPass the MIDX_PROGRESS flag to the subcommand functions\nwhen progress should be displayed by multi-pack-index.\nThe progress feature was added to 'verify' in 144d703\n(\"multi-pack-index: report progress during 'verify'\", 2018-09-13)\nbut some subcommands were not updated to display progress, and\nthe ability to opt-out was overlooked.\n\nSigned-off-by: William Baker \n---\n Documentation/git-multi-pack-index.txt | 6 ++-\n builtin/multi-pack-index.c | 18 +++++--\n t/t5319-multi-pack-index.sh | 69 ++++++++++++++++++++++++++\n 3 files changed, 87 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt\nindex 233b2b7862..d7a02cc6fa 100644\n--- a/Documentation/git-multi-pack-index.txt\n+++ b/Documentation/git-multi-pack-index.txt\n@@ -9,7 +9,7 @@ git-multi-pack-index - Write and verify multi-pack-indexes\n SYNOPSIS\n --------\n [verse]\n-'git multi-pack-index' [--object-dir=] \n+'git multi-pack-index' [--object-dir=] [--[no-]progress] \n \n DESCRIPTION\n -----------\n@@ -23,6 +23,10 @@ OPTIONS\n \t`/packs/multi-pack-index` for the current MIDX file, and\n \t`/packs` for the pack-files to index.\n \n+--[no-]progress::\n+\tTurn progress on/off explicitly. If neither is specified, progress is \n+\tshown if standard error is connected to a terminal.\n+\n The following subcommands are available:\n \n write::\ndiff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c\nindex e86b8cd17d..1730b21901 100644\n--- a/builtin/multi-pack-index.c\n+++ b/builtin/multi-pack-index.c\n@@ -6,21 +6,25 @@\n #include \"trace2.h\"\n \n static char const * const builtin_multi_pack_index_usage[] = {\n-\tN_(\"git multi-pack-index [--object-dir=] (write|verify|expire|repack --batch-size=)\"),\n+\tN_(\"git multi-pack-index [] (write|verify|expire|repack --batch-size=)\"),\n \tNULL\n };\n \n static struct opts_multi_pack_index {\n \tconst char *object_dir;\n \tunsigned long batch_size;\n+\tint progress;\n } opts;\n \n int cmd_multi_pack_index(int argc, const char **argv,\n \t\t\t const char *prefix)\n {\n+\tunsigned flags = 0;\n+\n \tstatic struct option builtin_multi_pack_index_options[] = {\n \t\tOPT_FILENAME(0, \"object-dir\", &opts.object_dir,\n \t\t N_(\"object directory containing set of packfile and pack-index pairs\")),\n+\t\tOPT_BOOL(0, \"progress\", &opts.progress, N_(\"force progress reporting\")),\n \t\tOPT_MAGNITUDE(0, \"batch-size\", &opts.batch_size,\n \t\t N_(\"during repack, collect pack-files of smaller size into a batch that is larger than this size\")),\n \t\tOPT_END(),\n@@ -28,12 +32,15 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \n \tgit_config(git_default_config, NULL);\n \n+\topts.progress = isatty(2);\n \targc = parse_options(argc, argv, prefix,\n \t\t\t builtin_multi_pack_index_options,\n \t\t\t builtin_multi_pack_index_usage, 0);\n \n \tif (!opts.object_dir)\n \t\topts.object_dir = get_object_directory();\n+\tif (opts.progress)\n+\t\tflags |= MIDX_PROGRESS;\n \n \tif (argc == 0)\n \t\tusage_with_options(builtin_multi_pack_index_usage,\n@@ -47,16 +54,17 @@ int cmd_multi_pack_index(int argc, const char **argv,\n \ttrace2_cmd_mode(argv[0]);\n \n \tif (!strcmp(argv[0], \"repack\"))\n-\t\treturn midx_repack(the_repository, opts.object_dir, (size_t)opts.batch_size, 0);\n+\t\treturn midx_repack(the_repository, opts.object_dir, \n+\t\t\t(size_t)opts.batch_size, flags);\n \tif (opts.batch_size)\n \t\tdie(_(\"--batch-size option is only for 'repack' subcommand\"));\n \n \tif (!strcmp(argv[0], \"write\"))\n-\t\treturn write_midx_file(opts.object_dir, 0);\n+\t\treturn write_midx_file(opts.object_dir, flags);\n \tif (!strcmp(argv[0], \"verify\"))\n-\t\treturn verify_midx_file(the_repository, opts.object_dir, 0);\n+\t\treturn verify_midx_file(the_repository, opts.object_dir, flags);\n \tif (!strcmp(argv[0], \"expire\"))\n-\t\treturn expire_midx_packs(the_repository, opts.object_dir, 0);\n+\t\treturn expire_midx_packs(the_repository, opts.object_dir, flags);\n \n \tdie(_(\"unrecognized subcommand: %s\"), argv[0]);\n }\ndiff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh\nindex c72ca04399..cd2f87be6a 100755\n--- a/t/t5319-multi-pack-index.sh\n+++ b/t/t5319-multi-pack-index.sh\n@@ -147,6 +147,21 @@ test_expect_success 'write midx with two packs' '\n \n compare_results_with_midx \"two packs\"\n \n+test_expect_success 'write progress off for redirected stderr' '\n+\tgit multi-pack-index --object-dir=$objdir write 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'write force progress on for stderr' '\n+\tgit multi-pack-index --object-dir=$objdir --progress write 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'write with the --no-progress option' '\n+\tgit multi-pack-index --object-dir=$objdir --no-progress write 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n test_expect_success 'add more packs' '\n \tfor j in $(test_seq 11 20)\n \tdo\n@@ -169,6 +184,21 @@ test_expect_success 'verify multi-pack-index success' '\n \tgit multi-pack-index verify --object-dir=$objdir\n '\n \n+test_expect_success 'verify progress off for redirected stderr' '\n+\tgit multi-pack-index verify --object-dir=$objdir 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'verify force progress on for stderr' '\n+\tgit multi-pack-index verify --object-dir=$objdir --progress 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'verify with the --no-progress option' '\n+\tgit multi-pack-index verify --object-dir=$objdir --no-progress 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n # usage: corrupt_midx_and_verify \n corrupt_midx_and_verify() {\n \tPOS=$1 &&\n@@ -284,6 +314,21 @@ test_expect_success 'git-fsck incorrect offset' '\n \t\t\"git -c core.multipackindex=true fsck\"\n '\n \n+test_expect_success 'repack progress off for redirected stderr' '\n+\tgit multi-pack-index --object-dir=$objdir repack 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n+test_expect_success 'repack force progress on for stderr' '\n+\tgit multi-pack-index --object-dir=$objdir --progress repack 2>err &&\n+\ttest_file_not_empty err\n+'\n+\n+test_expect_success 'repack with the --no-progress option' '\n+\tgit multi-pack-index --object-dir=$objdir --no-progress repack 2>err &&\n+\ttest_line_count = 0 err\n+'\n+\n test_expect_success 'repack removes multi-pack-index' '\n \ttest_path_is_file $objdir/pack/multi-pack-index &&\n \tGIT_TEST_MULTI_PACK_INDEX=0 git repack -adf &&\n@@ -413,6 +458,30 @@ test_expect_success 'expire does not remove any packs' '\n \t)\n '\n \n+test_expect_success 'expire progress off for redirected stderr' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit multi-pack-index expire 2>err &&\n+\t\ttest_line_count = 0 err\n+\t)\n+'\n+\n+test_expect_success 'expire force progress on for stderr' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit multi-pack-index --progress expire 2>err &&\n+\t\ttest_file_not_empty err\n+\t)\n+'\n+\n+test_expect_success 'expire with the --no-progress option' '\n+\t(\n+\t\tcd dup &&\n+\t\tgit multi-pack-index --no-progress expire 2>err &&\n+\t\ttest_line_count = 0 err\n+\t)\n+'\n+\n test_expect_success 'expire removes unreferenced packs' '\n \t(\n \t\tcd dup &&\n-- \ngitgitgadget\n"},{"id":"384545","messageId":"37768f58860d113e21730f2640ca773ebb54dff0.1571683203.git.gitgitgadget@gmail.com","threadId":"51830","inReplyTo":"pull.337.v4.git.1571683203.gitgitgadget@gmail.com","subject":"[PATCH v4 2/6] midx: add progress to write_midx_file","fromName":"William Baker via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-10-21T18:39:59Z","receivedAt":"2019-10-21T18:40:17Z","isPatch":true,"sender":{"key":"williamtbakeremail@gmail.com","avatar":null},"body":"From: William Baker \n\nAdd progress to write_midx_file. Progress is displayed\nwhen the MIDX_PROGRESS flag is set.\n\nSigned-off-by: William Baker \n---\n midx.c | 25 +++++++++++++++++++++----\n 1 file changed, 21 insertions(+), 4 deletions(-)\n\ndiff --git a/midx.c b/midx.c\nindex f169a681dd..006f36b570 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -448,6 +448,8 @@ struct pack_list {\n \tuint32_t nr;\n \tuint32_t alloc;\n \tstruct multi_pack_index *m;\n+\tstruct progress *progress;\n+\tunsigned pack_paths_checked;\n };\n \n static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n@@ -456,6 +458,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len,\n \tstruct pack_list *packs = (struct pack_list *)data;\n \n \tif (ends_with(file_name, \".idx\")) {\n+\t\tdisplay_progress(packs->progress, ++packs->pack_paths_checked);\n \t\tif (packs->m && midx_contains_pack(packs->m, file_name))\n \t\t\treturn;\n \n@@ -785,7 +788,7 @@ static size_t write_midx_large_offsets(struct hashfile *f, uint32_t nr_large_off\n }\n \n static int write_midx_internal(const char *object_dir, struct multi_pack_index *m,\n-\t\t\t struct string_list *packs_to_drop)\n+\t\t\t struct string_list *packs_to_drop, unsigned flags)\n {\n \tunsigned char cur_chunk, num_chunks = 0;\n \tchar *midx_name;\n@@ -799,6 +802,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \tuint64_t chunk_offsets[MIDX_MAX_CHUNKS + 1];\n \tuint32_t nr_entries, num_large_offsets = 0;\n \tstruct pack_midx_entry *entries = NULL;\n+\tstruct progress *progress = NULL;\n \tint large_offsets_needed = 0;\n \tint pack_name_concat_len = 0;\n \tint dropped_packs = 0;\n@@ -832,8 +836,15 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \t\t\tpacks.nr++;\n \t\t}\n \t}\n+\t\n+\tpacks.pack_paths_checked = 0;\n+\tif (flags & MIDX_PROGRESS)\n+\t\tpacks.progress = start_progress(_(\"Adding packfiles to multi-pack-index\"), 0);\n+\telse\n+\t\tpacks.progress = NULL;\n \n \tfor_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs);\n+\tstop_progress(&packs.progress);\n \n \tif (packs.m && packs.nr == packs.m->num_packs && !packs_to_drop)\n \t\tgoto cleanup;\n@@ -958,6 +969,9 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \t\twritten += MIDX_CHUNKLOOKUP_WIDTH;\n \t}\n \n+\tif (flags & MIDX_PROGRESS)\n+\t\tprogress = start_progress(_(\"Writing chunks to multi-pack-index\"),\n+\t\t\t\t\t num_chunks);\n \tfor (i = 0; i < num_chunks; i++) {\n \t\tif (written != chunk_offsets[i])\n \t\t\tBUG(\"incorrect chunk offset (%\"PRIu64\" != %\"PRIu64\") for chunk id %\"PRIx32,\n@@ -990,7 +1004,10 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \t\t\t\tBUG(\"trying to write unknown chunk id %\"PRIx32,\n \t\t\t\t chunk_ids[i]);\n \t\t}\n+\n+\t\tdisplay_progress(progress, i + 1);\n \t}\n+\tstop_progress(&progress);\n \n \tif (written != chunk_offsets[num_chunks])\n \t\tBUG(\"incorrect final offset %\"PRIu64\" != %\"PRIu64,\n@@ -1018,7 +1035,7 @@ static int write_midx_internal(const char *object_dir, struct multi_pack_index *\n \n int write_midx_file(const char *object_dir, unsigned flags)\n {\n-\treturn write_midx_internal(object_dir, NULL, NULL);\n+\treturn write_midx_internal(object_dir, NULL, NULL, flags);\n }\n \n void clear_midx_file(struct repository *r)\n@@ -1221,7 +1238,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla\n \tfree(count);\n \n \tif (packs_to_drop.nr)\n-\t\tresult = write_midx_internal(object_dir, m, &packs_to_drop);\n+\t\tresult = write_midx_internal(object_dir, m, &packs_to_drop, flags);\n \n \tstring_list_clear(&packs_to_drop, 0);\n \treturn result;\n@@ -1370,7 +1387,7 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,\n \t\tgoto cleanup;\n \t}\n \n-\tresult = write_midx_internal(object_dir, m, NULL);\n+\tresult = write_midx_internal(object_dir, m, NULL, flags);\n \tm = NULL;\n \n cleanup:\n-- \ngitgitgadget\n\n"}]}