{"thread":{"id":"65381","subject":"[GSoC PATCH] backfill: add --[no-]progress option","startedAt":"2026-03-29T15:24:50Z","lastAt":"2026-04-07T19:42:47Z","messageCount":6,"participants":["Trieu Huynh","Derrick Stolee","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"540329","messageId":"20260329152443.525493-1-vikingtc4@gmail.com","threadId":"65381","inReplyTo":null,"subject":"[GSoC PATCH] backfill: add --[no-]progress option","fromName":"Trieu Huynh","fromEmail":"vikingtc4@gmail.com","sentAt":"2026-03-29T15:24:43Z","receivedAt":"2026-03-29T15:24:50Z","isPatch":true,"body":"'git backfill' is silent when downloading missing objects, giving\nno feedback during potentially long-running operations on large\nrepositories. By contrast, 'git fetch', 'git gc', and\n'git index-pack' all support --[no-]progress.\n\nAdd a --[no-]progress option to 'git backfill' by:\n\n - Calling display_progress() in download_batch() to advance the\n   counter after each batch is fetched.\n - Defaulting to showing progress when stderr is a terminal\n   (isatty(2)), matching the behaviour of 'git fetch'.\n - Allowing the user to override the default via --[no-]progress.\n\nnit: progress.h was already included but unused, this commit puts\nit to use.\n\nAdd tests to verify that:\n - --progress shows \"Downloading batches\" on stderr.\n - --no-progress suppresses output even on a TTY.\n - No flag on a non-TTY is silent (isatty default).\n\nSigned-off-by: Trieu Huynh <vikingtc4@gmail.com>\n---\n builtin/backfill.c  | 16 ++++++++++++++++\n t/t5620-backfill.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 40 insertions(+)\n\ndiff --git a/builtin/backfill.c b/builtin/backfill.c\nindex e9a33e81be..27a301f9b2 100644\n--- a/builtin/backfill.c\n+++ b/builtin/backfill.c\n@@ -35,6 +35,9 @@ struct backfill_context {\n \tstruct oid_array current_batch;\n \tsize_t min_batch_size;\n \tint sparse;\n+\tint show_progress;\n+\tsize_t batches_requested;\n+\tstruct progress *progress;\n };\n \n static void backfill_context_clear(struct backfill_context *ctx)\n@@ -54,6 +57,7 @@ static void download_batch(struct backfill_context *ctx)\n \t * avoid possible duplicate downloads of the same objects.\n \t */\n \todb_reprepare(ctx->repo->objects);\n+\tdisplay_progress(ctx->progress, ++ctx->batches_requested);\n }\n \n static int fill_missing_blobs(const char *path UNUSED,\n@@ -120,12 +124,15 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \t\t.current_batch = OID_ARRAY_INIT,\n \t\t.min_batch_size = 50000,\n \t\t.sparse = 0,\n+\t\t.show_progress = -1,\n \t};\n \tstruct option options[] = {\n \t\tOPT_UNSIGNED(0, \"min-batch-size\", &ctx.min_batch_size,\n \t\t\t     N_(\"Minimum number of objects to request at a time\")),\n \t\tOPT_BOOL(0, \"sparse\", &ctx.sparse,\n \t\t\t N_(\"Restrict the missing objects to the current sparse-checkout\")),\n+\t\tOPT_BOOL(0, \"progress\", &ctx.show_progress,\n+\t\t\t N_(\"show progress while downloading missing objects\")),\n \t\tOPT_END(),\n \t};\n \tstruct repo_config_values *cfg = repo_config_values(the_repository);\n@@ -141,7 +148,16 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \tif (ctx.sparse < 0)\n \t\tctx.sparse = cfg->apply_sparse_checkout;\n \n+\tif (ctx.show_progress < 0)\n+\t\tctx.show_progress = isatty(2);\n+\n+\tif (ctx.show_progress)\n+\t\tctx.progress = start_progress(ctx.repo, _(\"Downloading batches\"), 0);\n+\n \tresult = do_backfill(&ctx);\n+\tif (ctx.show_progress)\n+\t\tstop_progress(&ctx.progress);\n+\n \tbackfill_context_clear(&ctx);\n \treturn result;\n }\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 58c81556e7..ff67e8ecea 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -77,6 +77,30 @@ test_expect_success 'do partial clone 2, backfill min batch size' '\n \ttest_line_count = 0 revs2\n '\n \n+test_expect_success 'backfill --progress shows progress' '\n+\tgit clone --no-checkout --filter=blob:none \\\n+\t\t--single-branch --branch=main \\\n+\t\t\"file://$(pwd)/srv.bare\" clone-progress &&\n+\tgit -C clone-progress backfill --progress 2>err &&\n+\ttest_grep \"Downloading batches\" err\n+'\n+\n+test_expect_success 'backfill --no-progress is silent' '\n+\tgit clone --no-checkout --filter=blob:none \\\n+\t\t--single-branch --branch=main \\\n+\t\t\"file://$(pwd)/srv.bare\" clone-no-progress &&\n+\tgit -C clone-no-progress backfill --no-progress 2>err &&\n+\ttest_grep ! \"Downloading batches\" err\n+'\n+\n+test_expect_success 'backfill no flag on non-TTY is silent' '\n+\tgit clone --no-checkout --filter=blob:none \\\n+\t\t--single-branch --branch=main \\\n+\t\t\"file://$(pwd)/srv.bare\" clone-notty &&\n+\tgit -C clone-notty backfill 2>err &&\n+\ttest_grep ! \"Downloading batches\" err\n+'\n+\n test_expect_success 'backfill --sparse without sparse-checkout fails' '\n \tgit init not-sparse &&\n \ttest_must_fail git -C not-sparse backfill --sparse 2>err &&\n-- \n2.43.0\n\n"},{"id":"540970","messageId":"8db10441-2fce-43ad-bcdc-331d26ec38ed@gmail.com","threadId":"65381","inReplyTo":"20260329152443.525493-1-vikingtc4@gmail.com","subject":"Re: [GSoC PATCH] backfill: add --[no-]progress option","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-04-06T13:16:30Z","receivedAt":"2026-04-06T13:16:33Z","isPatch":true,"body":"On 3/29/2026 11:24 AM, Trieu Huynh wrote:\n> 'git backfill' is silent when downloading missing objects, giving\n> no feedback during potentially long-running operations on large\n> repositories. By contrast, 'git fetch', 'git gc', and\n> 'git index-pack' all support --[no-]progress.\n\nI wouldn't use the word \"silent\" because the output is actually\nquite verbose by default. Each batch has progress output with the\nremote. For example, this is the output I get when running 'git\nbackfill' on a blobless partial clone of the Git repo:\n\n$ git backfill\nremote: Enumerating objects: 50083, done.\nremote: Counting objects: 100% (865/865), done.\nremote: Compressing objects: 100% (177/177), done.\nremote: Total 50083 (delta 760), reused 688 (delta 688), pack-reused 49218 (from 1)\nReceiving objects: 100% (50083/50083), 37.13 MiB | 27.75 MiB/s, done.\nResolving deltas: 100% (47710/47710), done.\nremote: Enumerating objects: 50393, done.\nremote: Counting objects: 100% (1559/1559), done.\nremote: Compressing objects: 100% (366/366), done.\nremote: Total 50393 (delta 1366), reused 1193 (delta 1193), pack-reused 48834 (from 2)\nReceiving objects: 100% (50393/50393), 44.56 MiB | 31.56 MiB/s, done.\nResolving deltas: 100% (47261/47261), done.\nremote: Enumerating objects: 50000, done.\nremote: Counting objects: 100% (2313/2313), done.\nremote: Compressing objects: 100% (592/592), done.\nremote: Total 50000 (delta 1982), reused 1721 (delta 1721), pack-reused 47687 (from 2)\nReceiving objects: 100% (50000/50000), 90.49 MiB | 17.85 MiB/s, done.\nResolving deltas: 100% (45321/45321), done.\nremote: Enumerating objects: 2155, done.\nremote: Counting objects: 100% (27/27), done.\nremote: Compressing objects: 100% (26/26), done.\nremote: Total 2155 (delta 6), reused 1 (delta 1), pack-reused 2128 (from 1)\nReceiving objects: 100% (2155/2155), 891.74 KiB | 3.75 MiB/s, done.\nResolving deltas: 100% (1717/1717), done.\n\nWith your patch, I think there would be some extra progress\nindicators between these batched fetch requests.\n\nBefore moving forward with review of this patch, I think that\nit would be valuable to demonstrate the full output with and\nwithout your change.\n\nIn addition, I think there would be value in a progress indicator\n_instead_ of these verbose outputs from the remote. That would\nrequire a change to how we initialize the fetches in a quiet mode.\n\n(I also understand that this output would probably not be the same\nif we have a filesystem protocol for fetching from a local repo,\nlike we frequently do in the test suite.)\n\n>  static void backfill_context_clear(struct backfill_context *ctx)\n> @@ -54,6 +57,7 @@ static void download_batch(struct backfill_context *ctx)\n>  \t * avoid possible duplicate downloads of the same objects.\n>  \t */\n>  \todb_reprepare(ctx->repo->objects);\n> +\tdisplay_progress(ctx->progress, ++ctx->batches_requested);\n\nThis looks correct. My preference is to not use prefix operators\nlike this on struct members (it reads like you are incrementing\n'ctx' and not 'batches_requested', even though it is correct).\n\nHowever, I'm not sure that we want the progress to indicate the\nnumber of _batches_ but instead should be the number of _objects_.\n  \n>  static int fill_missing_blobs(const char *path UNUSED,\n> @@ -120,12 +124,15 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n>  \t\t.current_batch = OID_ARRAY_INIT,\n>  \t\t.min_batch_size = 50000,\n>  \t\t.sparse = 0,\n> +\t\t.show_progress = -1,\n>  \t};\n>  \tstruct option options[] = {\n>  \t\tOPT_UNSIGNED(0, \"min-batch-size\", &ctx.min_batch_size,\n>  \t\t\t     N_(\"Minimum number of objects to request at a time\")),\n>  \t\tOPT_BOOL(0, \"sparse\", &ctx.sparse,\n>  \t\t\t N_(\"Restrict the missing objects to the current sparse-checkout\")),\n> +\t\tOPT_BOOL(0, \"progress\", &ctx.show_progress,\n> +\t\t\t N_(\"show progress while downloading missing objects\")),\n>  \t\tOPT_END(),\n>  \t};\n\nI hope that this does not cause any issues with the recent changes\nto include the rev-list options in git-backfill. Worth checking.\n\n> +test_expect_success 'backfill --progress shows progress' '\n> +\tgit clone --no-checkout --filter=blob:none \\\n> +\t\t--single-branch --branch=main \\\n> +\t\t\"file://$(pwd)/srv.bare\" clone-progress &&\n> +\tgit -C clone-progress backfill --progress 2>err &&\n> +\ttest_grep \"Downloading batches\" err\n> +'\n> +\n> +test_expect_success 'backfill --no-progress is silent' '\n> +\tgit clone --no-checkout --filter=blob:none \\\n> +\t\t--single-branch --branch=main \\\n> +\t\t\"file://$(pwd)/srv.bare\" clone-no-progress &&\n> +\tgit -C clone-no-progress backfill --no-progress 2>err &&\n> +\ttest_grep ! \"Downloading batches\" err\n> +'\n> +\n> +test_expect_success 'backfill no flag on non-TTY is silent' '\n> +\tgit clone --no-checkout --filter=blob:none \\\n> +\t\t--single-branch --branch=main \\\n> +\t\t\"file://$(pwd)/srv.bare\" clone-notty &&\n> +\tgit -C clone-notty backfill 2>err &&\n> +\ttest_grep ! \"Downloading batches\" err\n> +'\n\nWhat you are missing here is that the progress isn't silent when\na TTY is present. There are several tests in the test suite that\nuse the TTY prerequisite for this kind of behavior, such as this\none from t9211-scalar-clone.sh:\n\ntest_expect_success TTY 'progress with tty' '\n\tenlistment=progress1 &&\n\n\ttest_config -C to-clone uploadpack.allowfilter true &&\n\ttest_config -C to-clone uploadpack.allowanysha1inwant true &&\n\n\ttest_terminal env GIT_PROGRESS_DELAY=0 \\\n\t\tscalar clone \"file://$(pwd)/to-clone\" \"$enlistment\" 2>stderr &&\n\tgrep \"Enumerating objects\" stderr >actual &&\n\ttest_line_count = 2 actual &&\n\tcleanup_clone $enlistment\n'\n\nThanks,\n-Stolee\n\n"},{"id":"540988","messageId":"xmqqh5poat4x.fsf@gitster.g","threadId":"65381","inReplyTo":"8db10441-2fce-43ad-bcdc-331d26ec38ed@gmail.com","subject":"Re: [GSoC PATCH] backfill: add --[no-]progress option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-06T17:35:58Z","receivedAt":"2026-04-06T17:36:01Z","isPatch":true,"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> On 3/29/2026 11:24 AM, Trieu Huynh wrote:\n>> 'git backfill' is silent when downloading missing objects, giving\n>> no feedback during potentially long-running operations on large\n>> repositories. By contrast, 'git fetch', 'git gc', and\n>> 'git index-pack' all support --[no-]progress.\n>\n> I wouldn't use the word \"silent\" because the output is actually\n> quite verbose by default.\n\n;-)\n\n> With your patch, I think there would be some extra progress\n> indicators between these batched fetch requests.\n\n>>  static void backfill_context_clear(struct backfill_context *ctx)\n>> @@ -54,6 +57,7 @@ static void download_batch(struct backfill_context *ctx)\n>>  \t * avoid possible duplicate downloads of the same objects.\n>>  \t */\n>>  \todb_reprepare(ctx->repo->objects);\n>> +\tdisplay_progress(ctx->progress, ++ctx->batches_requested);\n>\n> This looks correct. My preference is to not use prefix operators\n> like this on struct members (it reads like you are incrementing\n> 'ctx' and not 'batches_requested', even though it is correct).\n\nThanks for paying extra attention to such details.  In general,\npost-increment and pre-decrement are the norm when evaluated in a\nvoid context, so the use of pre-increment above violates that norm\ntoo.\n\n> However, I'm not sure that we want the progress to indicate the\n> number of _batches_ but instead should be the number of _objects_.\n\nTrue, too.\n\nThanks.\n"},{"id":"541086","messageId":"j7abmtc6jlgciddkceduueglt2w2nfw76bbplisq63p7jio2xc@yht6tzxgetuk","threadId":"65381","inReplyTo":"8db10441-2fce-43ad-bcdc-331d26ec38ed@gmail.com","subject":"Re: [GSoC PATCH] backfill: add --[no-]progress option","fromName":"Trieu Huynh","fromEmail":"vikingtc4@gmail.com","sentAt":"2026-04-07T19:15:12Z","receivedAt":"2026-04-07T19:15:17Z","isPatch":true,"body":"On Mon, Apr 06, 2026 at 09:16:30AM -0400, Derrick Stolee wrote:\n> On 3/29/2026 11:24 AM, Trieu Huynh wrote:\n> > 'git backfill' is silent when downloading missing objects, giving\n> > no feedback during potentially long-running operations on large\n> > repositories. By contrast, 'git fetch', 'git gc', and\n> > 'git index-pack' all support --[no-]progress.\n> \n> I wouldn't use the word \"silent\" because the output is actually\n> quite verbose by default. Each batch has progress output with the\n> remote. For example, this is the output I get when running 'git\n> backfill' on a blobless partial clone of the Git repo:\n> \nMake sense to me, will reword it to be more precise in v2.\n> $ git backfill\n> remote: Enumerating objects: 50083, done.\n> remote: Counting objects: 100% (865/865), done.\n> remote: Compressing objects: 100% (177/177), done.\n> remote: Total 50083 (delta 760), reused 688 (delta 688), pack-reused 49218 (from 1)\n> Receiving objects: 100% (50083/50083), 37.13 MiB | 27.75 MiB/s, done.\n> Resolving deltas: 100% (47710/47710), done.\n> remote: Enumerating objects: 50393, done.\n> remote: Counting objects: 100% (1559/1559), done.\n> remote: Compressing objects: 100% (366/366), done.\n> remote: Total 50393 (delta 1366), reused 1193 (delta 1193), pack-reused 48834 (from 2)\n> Receiving objects: 100% (50393/50393), 44.56 MiB | 31.56 MiB/s, done.\n> Resolving deltas: 100% (47261/47261), done.\n> remote: Enumerating objects: 50000, done.\n> remote: Counting objects: 100% (2313/2313), done.\n> remote: Compressing objects: 100% (592/592), done.\n> remote: Total 50000 (delta 1982), reused 1721 (delta 1721), pack-reused 47687 (from 2)\n> Receiving objects: 100% (50000/50000), 90.49 MiB | 17.85 MiB/s, done.\n> Resolving deltas: 100% (45321/45321), done.\n> remote: Enumerating objects: 2155, done.\n> remote: Counting objects: 100% (27/27), done.\n> remote: Compressing objects: 100% (26/26), done.\n> remote: Total 2155 (delta 6), reused 1 (delta 1), pack-reused 2128 (from 1)\n> Receiving objects: 100% (2155/2155), 891.74 KiB | 3.75 MiB/s, done.\n> Resolving deltas: 100% (1717/1717), done.\n> \n> With your patch, I think there would be some extra progress\n> indicators between these batched fetch requests.\n> \n> Before moving forward with review of this patch, I think that\n> it would be valuable to demonstrate the full output with and\n> without your change.\n> \nAgree, will include a side-by-side comparison in the v2.\n> In addition, I think there would be value in a progress indicator\n> _instead_ of these verbose outputs from the remote. That would\n> require a change to how we initialize the fetches in a quiet mode.\n> \n> (I also understand that this output would probably not be the same\n> if we have a filesystem protocol for fetching from a local repo,\n> like we frequently do in the test suite.)\n> \nAgreed, initialize the fetch in quiet mode and replacing it with a\nsingle consolidated indicator would be a nicer UX. I'll look into\nhow git-fetch sets up quiet mode and try to wire that through\ndownload_batch() in v2.\n> >  static void backfill_context_clear(struct backfill_context *ctx)\n> > @@ -54,6 +57,7 @@ static void download_batch(struct backfill_context *ctx)\n> >  \t * avoid possible duplicate downloads of the same objects.\n> >  \t */\n> >  \todb_reprepare(ctx->repo->objects);\n> > +\tdisplay_progress(ctx->progress, ++ctx->batches_requested);\n> \n> This looks correct. My preference is to not use prefix operators\n> like this on struct members (it reads like you are incrementing\n> 'ctx' and not 'batches_requested', even though it is correct).\n> \n> However, I'm not sure that we want the progress to indicate the\n> number of _batches_ but instead should be the number of _objects_.\n>   \n> >  static int fill_missing_blobs(const char *path UNUSED,\n> > @@ -120,12 +124,15 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n> >  \t\t.current_batch = OID_ARRAY_INIT,\n> >  \t\t.min_batch_size = 50000,\n> >  \t\t.sparse = 0,\n> > +\t\t.show_progress = -1,\n> >  \t};\n> >  \tstruct option options[] = {\n> >  \t\tOPT_UNSIGNED(0, \"min-batch-size\", &ctx.min_batch_size,\n> >  \t\t\t     N_(\"Minimum number of objects to request at a time\")),\n> >  \t\tOPT_BOOL(0, \"sparse\", &ctx.sparse,\n> >  \t\t\t N_(\"Restrict the missing objects to the current sparse-checkout\")),\n> > +\t\tOPT_BOOL(0, \"progress\", &ctx.show_progress,\n> > +\t\t\t N_(\"show progress while downloading missing objects\")),\n> >  \t\tOPT_END(),\n> >  \t};\n> \n> I hope that this does not cause any issues with the recent changes\n> to include the rev-list options in git-backfill. Worth checking.\n> \nThanks for point, worth checking if any conflicts.\n> > +test_expect_success 'backfill --progress shows progress' '\n> > +\tgit clone --no-checkout --filter=blob:none \\\n> > +\t\t--single-branch --branch=main \\\n> > +\t\t\"file://$(pwd)/srv.bare\" clone-progress &&\n> > +\tgit -C clone-progress backfill --progress 2>err &&\n> > +\ttest_grep \"Downloading batches\" err\n> > +'\n> > +\n> > +test_expect_success 'backfill --no-progress is silent' '\n> > +\tgit clone --no-checkout --filter=blob:none \\\n> > +\t\t--single-branch --branch=main \\\n> > +\t\t\"file://$(pwd)/srv.bare\" clone-no-progress &&\n> > +\tgit -C clone-no-progress backfill --no-progress 2>err &&\n> > +\ttest_grep ! \"Downloading batches\" err\n> > +'\n> > +\n> > +test_expect_success 'backfill no flag on non-TTY is silent' '\n> > +\tgit clone --no-checkout --filter=blob:none \\\n> > +\t\t--single-branch --branch=main \\\n> > +\t\t\"file://$(pwd)/srv.bare\" clone-notty &&\n> > +\tgit -C clone-notty backfill 2>err &&\n> > +\ttest_grep ! \"Downloading batches\" err\n> > +'\n> \n> What you are missing here is that the progress isn't silent when\n> a TTY is present. There are several tests in the test suite that\n> use the TTY prerequisite for this kind of behavior, such as this\n> one from t9211-scalar-clone.sh:\n> \n> test_expect_success TTY 'progress with tty' '\n> \tenlistment=progress1 &&\n> \n> \ttest_config -C to-clone uploadpack.allowfilter true &&\n> \ttest_config -C to-clone uploadpack.allowanysha1inwant true &&\n> \n> \ttest_terminal env GIT_PROGRESS_DELAY=0 \\\n> \t\tscalar clone \"file://$(pwd)/to-clone\" \"$enlistment\" 2>stderr &&\n> \tgrep \"Enumerating objects\" stderr >actual &&\n> \ttest_line_count = 2 actual &&\n> \tcleanup_clone $enlistment\n> '\n> \nAgree, will add such that test as per above reference.\n> Thanks,\n> -Stolee\n> \nBRs,\nTrieu Huynh\n"},{"id":"541087","messageId":"ktjgf2gyf5wkktiquy4cfzdcifd2yhqk3mngckaih4bwca6fda@nkcju65ku7hf","threadId":"65381","inReplyTo":"xmqqh5poat4x.fsf@gitster.g","subject":"Re: [GSoC PATCH] backfill: add --[no-]progress option","fromName":"Trieu Huynh","fromEmail":"vikingtc4@gmail.com","sentAt":"2026-04-07T19:22:33Z","receivedAt":"2026-04-07T19:22:37Z","isPatch":true,"body":"On Mon, Apr 06, 2026 at 10:35:58AM -0700, Junio C Hamano wrote:\n> Derrick Stolee <stolee@gmail.com> writes:\n> \n> > On 3/29/2026 11:24 AM, Trieu Huynh wrote:\n> >> 'git backfill' is silent when downloading missing objects, giving\n> >> no feedback during potentially long-running operations on large\n> >> repositories. By contrast, 'git fetch', 'git gc', and\n> >> 'git index-pack' all support --[no-]progress.\n> >\n> > I wouldn't use the word \"silent\" because the output is actually\n> > quite verbose by default.\n> \n> ;-)\n> \n> > With your patch, I think there would be some extra progress\n> > indicators between these batched fetch requests.\n> \n> >>  static void backfill_context_clear(struct backfill_context *ctx)\n> >> @@ -54,6 +57,7 @@ static void download_batch(struct backfill_context *ctx)\n> >>  \t * avoid possible duplicate downloads of the same objects.\n> >>  \t */\n> >>  \todb_reprepare(ctx->repo->objects);\n> >> +\tdisplay_progress(ctx->progress, ++ctx->batches_requested);\n> >\n> > This looks correct. My preference is to not use prefix operators\n> > like this on struct members (it reads like you are incrementing\n> > 'ctx' and not 'batches_requested', even though it is correct).\n> \n> Thanks for paying extra attention to such details.  In general,\n> post-increment and pre-decrement are the norm when evaluated in a\n> void context, so the use of pre-increment above violates that norm\n> too.\n> \nThanks for pointing it out. Will update, eg:\n++counter;\nfoo(counter);\n> > However, I'm not sure that we want the progress to indicate the\n> > number of _batches_ but instead should be the number of _objects_.\n> \n> True, too.\n> \nMake sense to me, worth checking if we can feasibly track the total\nobject count instead of just batches to make the progress more\nmeaningful.\n> Thanks.\nThank you for all your kind review.\nI'll update v2 as per your comments. Hopefully, it can address\nthese concerns. \n\nBRs,\nTrieu Huynh\n"},{"id":"541089","messageId":"xmqq5x624kwb.fsf@gitster.g","threadId":"65381","inReplyTo":"ktjgf2gyf5wkktiquy4cfzdcifd2yhqk3mngckaih4bwca6fda@nkcju65ku7hf","subject":"Re: [GSoC PATCH] backfill: add --[no-]progress option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-07T19:42:44Z","receivedAt":"2026-04-07T19:42:47Z","isPatch":true,"body":"Trieu Huynh <vikingtc4@gmail.com> writes:\n\n>> >> +\tdisplay_progress(ctx->progress, ++ctx->batches_requested);\n>> >\n>> > This looks correct. My preference is to not use prefix operators\n>> > like this on struct members (it reads like you are incrementing\n>> > 'ctx' and not 'batches_requested', even though it is correct).\n>> \n>> Thanks for paying extra attention to such details.  In general,\n>> post-increment and pre-decrement are the norm when evaluated in a\n>> void context, so the use of pre-increment above violates that norm\n>> too.\n>> \n> Thanks for pointing it out. Will update, eg:\n> ++counter;\n> foo(counter);\n\nI think you meant \"counter++; foo(counter);\" instead.  Otherwise,\nthe first line is exactly a pre-increment in a void context.\n"}]}