{"thread":{"id":"65393","subject":"[GSoC PATCH] backfill: auto-detect sparse-checkout from config","startedAt":"2026-03-31T11:25:22Z","lastAt":"2026-04-02T16:05:11Z","messageCount":5,"participants":["Trieu Huynh","Junio C Hamano","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"540503","messageId":"20260331112516.772635-1-vikingtc4@gmail.com","threadId":"65393","inReplyTo":null,"subject":"[GSoC PATCH] backfill: auto-detect sparse-checkout from config","fromName":"Trieu Huynh","fromEmail":"vikingtc4@gmail.com","sentAt":"2026-03-31T11:25:16Z","receivedAt":"2026-03-31T11:25:22Z","isPatch":true,"body":"git backfill currently initializes the `sparse` field in\nbackfill_context to 0. This causes the command to always perform a\nfull backfill by default, even when the repository has sparse-checkout\nenabled in its configuration (core.sparseCheckout).\n\nBecause 'sparse' is explicitly set to 0 at initialization, any later\nlogic intended to auto-detect the setting from the repository\nconfiguration becomes dead code, as it only triggers if the value\nis negative (sentinel).\n\nChange the initial value of .sparse to -1. This allows the command\nto correctly fallback to the repository's sparse-checkout settings\nwhen the '--sparse' or '--no-sparse' options are not provided on the\ncommand line.\n\nAdd a test case in t5620-backfill.sh to verify that 'git backfill'\nautomatically respects the sparse-checkout configuration without\nexplicit flags.\n\nSigned-off-by: Trieu Huynh <vikingtc4@gmail.com>\n---\n builtin/backfill.c  |  2 +-\n t/t5620-backfill.sh | 15 +++++++++++++++\n 2 files changed, 16 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/backfill.c b/builtin/backfill.c\nindex 4b2db94173..0f31844ce7 100644\n--- a/builtin/backfill.c\n+++ b/builtin/backfill.c\n@@ -124,7 +124,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \t\t.repo = repo,\n \t\t.current_batch = OID_ARRAY_INIT,\n \t\t.min_batch_size = 50000,\n-\t\t.sparse = 0,\n+\t\t.sparse = -1,\n \t\t.show_progress = -1,\n \t};\n \tstruct option options[] = {\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 91b5115732..a1a8d736db 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -149,6 +149,21 @@ test_expect_success 'backfill --sparse' '\n \ttest_line_count = 0 missing\n '\n \n+test_expect_success 'backfill auto-detects sparse-checkout from config' '\n+\tgit clone --sparse --filter=blob:none \\\n+\t\t--single-branch --branch=main \\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-auto-sparse &&\n+\n+\tgit -C backfill-auto-sparse rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 44 missing &&\n+\n+\tGIT_TRACE2_EVENT=\"$(pwd)/auto-sparse-trace\" git \\\n+\t\t-C backfill-auto-sparse backfill &&\n+\n+\ttest_trace2_data promisor fetch_count 4 <auto-sparse-trace &&\n+\ttest_trace2_data path-walk paths 5 <auto-sparse-trace\n+'\n+\n test_expect_success 'backfill --sparse without cone mode (positive)' '\n \tgit clone --no-checkout --filter=blob:none\t\t\\\n \t\t--single-branch --branch=main \t\t\\\n-- \n2.43.0\n\n"},{"id":"540540","messageId":"xmqqo6k40wbl.fsf@gitster.g","threadId":"65393","inReplyTo":"20260331112516.772635-1-vikingtc4@gmail.com","subject":"Re: [GSoC PATCH] backfill: auto-detect sparse-checkout from config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-31T16:59:10Z","receivedAt":"2026-03-31T16:59:12Z","isPatch":true,"body":"Trieu Huynh <vikingtc4@gmail.com> writes:\n\n> git backfill currently initializes the `sparse` field in\n> backfill_context to 0. This causes the command to always perform a\n> full backfill by default, even when the repository has sparse-checkout\n> enabled in its configuration (core.sparseCheckout).\n>\n> Because 'sparse' is explicitly set to 0 at initialization, any later\n> logic intended to auto-detect the setting from the repository\n> configuration becomes dead code, as it only triggers if the value\n> is negative (sentinel).\n>\n> Change the initial value of .sparse to -1. This allows the command\n> to correctly fallback to the repository's sparse-checkout settings\n> when the '--sparse' or '--no-sparse' options are not provided on the\n> command line.\n\nThe author of bff45557 (backfill: add --sparse option, 2025-02-03),\nwhere this .sparse member originates, CC'ed for more intelligent\ninput than my review can offer ;-)\n\n\n> Add a test case in t5620-backfill.sh to verify that 'git backfill'\n> automatically respects the sparse-checkout configuration without\n> explicit flags.\n>\n> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>\n> ---\n>  builtin/backfill.c  |  2 +-\n>  t/t5620-backfill.sh | 15 +++++++++++++++\n>  2 files changed, 16 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/backfill.c b/builtin/backfill.c\n> index 4b2db94173..0f31844ce7 100644\n> --- a/builtin/backfill.c\n> +++ b/builtin/backfill.c\n> @@ -124,7 +124,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n>  \t\t.repo = repo,\n>  \t\t.current_batch = OID_ARRAY_INIT,\n>  \t\t.min_batch_size = 50000,\n> -\t\t.sparse = 0,\n> +\t\t.sparse = -1,\n>  \t\t.show_progress = -1,\n>  \t};\n>  \tstruct option options[] = {\n\nI am a bit confused by this change.  What's the difference between\nusing -1 (which you picked) and 1 as the initial value for this\nmember?  From the proposed log message, I would have expected a new\ncode that says \"ah, we notice, from this member being -1, that the\nuser did not specify --no-sparse or --sparse, so let's figure out if\nour working tree is sparsely checked out ourselves and set it either\nto 0 or to 1\", but there is nothing like that in the code.  It seems\nthat the updated code relies on the fact that this part of\ndo_backfill() only cares if .sparse is zero or not, and ...\n\n\tif (ctx->sparse) {\n\t\tCALLOC_ARRAY(info.pl, 1);\n\t\tif (get_sparse_checkout_patterns(info.pl)) {\n\t\t\tpath_walk_info_clear(&info);\n\t\t\treturn error(_(\"problem loading sparse-checkout\"));\n\t\t}\n\t}\n\n... relies on get_sparse_checkout_patterns() not to do any harm when\nthe working tree is not sparsely checked out.\n\nI am not sure if we want to call it \"auto-detction\".  It looks more\nlike \"default to --sparse, relying that --sparse is a no-op in a\nnon-sparse working tree\" at least to me.  Not that it is necessarily\nwrong, and when people do \"backfill\" knowing that the working tree\nis sparse, I am sympathetic if they prefer to keep the sparseness,\nso such a change of default may be beneficial.\n\nDerrick, what do you think?\n\n> diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\n> index 91b5115732..a1a8d736db 100755\n> --- a/t/t5620-backfill.sh\n> +++ b/t/t5620-backfill.sh\n> @@ -149,6 +149,21 @@ test_expect_success 'backfill --sparse' '\n>  \ttest_line_count = 0 missing\n>  '\n>  \n> +test_expect_success 'backfill auto-detects sparse-checkout from config' '\n> +\tgit clone --sparse --filter=blob:none \\\n> +\t\t--single-branch --branch=main \\\n> +\t\t\"file://$(pwd)/srv.bare\" backfill-auto-sparse &&\n> +\n> +\tgit -C backfill-auto-sparse rev-list --quiet --objects --missing=print HEAD >missing &&\n> +\ttest_line_count = 44 missing &&\n> +\n> +\tGIT_TRACE2_EVENT=\"$(pwd)/auto-sparse-trace\" git \\\n> +\t\t-C backfill-auto-sparse backfill &&\n> +\n> +\ttest_trace2_data promisor fetch_count 4 <auto-sparse-trace &&\n> +\ttest_trace2_data path-walk paths 5 <auto-sparse-trace\n> +'\n> +\n>  test_expect_success 'backfill --sparse without cone mode (positive)' '\n>  \tgit clone --no-checkout --filter=blob:none\t\t\\\n>  \t\t--single-branch --branch=main \t\t\\\n"},{"id":"540666","messageId":"buisigjsw3zrcy6bqaic2zefypq37kimju32eufquppsvkgkvx@cqd3cwj6an6t","threadId":"65393","inReplyTo":"xmqqo6k40wbl.fsf@gitster.g","subject":"Re: [GSoC PATCH] backfill: auto-detect sparse-checkout from config","fromName":"Trieu Huynh","fromEmail":"vikingtc4@gmail.com","sentAt":"2026-04-01T19:31:34Z","receivedAt":"2026-04-01T19:31:40Z","isPatch":true,"body":"On Tue, Mar 31, 2026 at 09:59:10AM -0700, Junio C Hamano wrote:\n> Trieu Huynh <vikingtc4@gmail.com> writes:\n> \n> > git backfill currently initializes the `sparse` field in\n> > backfill_context to 0. This causes the command to always perform a\n> > full backfill by default, even when the repository has sparse-checkout\n> > enabled in its configuration (core.sparseCheckout).\n> >\n> > Because 'sparse' is explicitly set to 0 at initialization, any later\n> > logic intended to auto-detect the setting from the repository\n> > configuration becomes dead code, as it only triggers if the value\n> > is negative (sentinel).\n> >\n> > Change the initial value of .sparse to -1. This allows the command\n> > to correctly fallback to the repository's sparse-checkout settings\n> > when the '--sparse' or '--no-sparse' options are not provided on the\n> > command line.\n> \n> The author of bff45557 (backfill: add --sparse option, 2025-02-03),\n> where this .sparse member originates, CC'ed for more intelligent\n> input than my review can offer ;-)\n> \n> \n> > Add a test case in t5620-backfill.sh to verify that 'git backfill'\n> > automatically respects the sparse-checkout configuration without\n> > explicit flags.\n> >\n> > Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>\n> > ---\n> >  builtin/backfill.c  |  2 +-\n> >  t/t5620-backfill.sh | 15 +++++++++++++++\n> >  2 files changed, 16 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/builtin/backfill.c b/builtin/backfill.c\n> > index 4b2db94173..0f31844ce7 100644\n> > --- a/builtin/backfill.c\n> > +++ b/builtin/backfill.c\n> > @@ -124,7 +124,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n> >  \t\t.repo = repo,\n> >  \t\t.current_batch = OID_ARRAY_INIT,\n> >  \t\t.min_batch_size = 50000,\n> > -\t\t.sparse = 0,\n> > +\t\t.sparse = -1,\n> >  \t\t.show_progress = -1,\n> >  \t};\n> >  \tstruct option options[] = {\n> \n> I am a bit confused by this change.  What's the difference between\n> using -1 (which you picked) and 1 as the initial value for this\n> member?  From the proposed log message, I would have expected a new\n> code that says \"ah, we notice, from this member being -1, that the\n> user did not specify --no-sparse or --sparse, so let's figure out if\n> our working tree is sparsely checked out ourselves and set it either\n> to 0 or to 1\", but there is nothing like that in the code.  It seems\n> that the updated code relies on the fact that this part of\n> do_backfill() only cares if .sparse is zero or not, and ...\n> \n> \tif (ctx->sparse) {\n> \t\tCALLOC_ARRAY(info.pl, 1);\n> \t\tif (get_sparse_checkout_patterns(info.pl)) {\n> \t\t\tpath_walk_info_clear(&info);\n> \t\t\treturn error(_(\"problem loading sparse-checkout\"));\n> \t\t}\n> \t}\n> \n> ... relies on get_sparse_checkout_patterns() not to do any harm when\n> the working tree is not sparsely checked out.\n> \n> I am not sure if we want to call it \"auto-detction\".  It looks more\n> like \"default to --sparse, relying that --sparse is a no-op in a\n> non-sparse working tree\" at least to me.  Not that it is necessarily\n> wrong, and when people do \"backfill\" knowing that the working tree\n> is sparse, I am sympathetic if they prefer to keep the sparseness,\n> so such a change of default may be beneficial.\n> \nactually, the logic IIUC here is:\n- first, ctx.sprase originally is set to 0.\n- then, it check user's options. Assume, there is no option passed,\nstill 0.\n- then, it check repo's config (core.sparseCheckout (default to 0 in\nenviroment.c) but it doesn't since the guard:\n\tif (ctx.sparse < 0)\n\t\tctx.sparse = cfg->apply_sparse_checkout;\n- evenly. ctx.sparse still 0 eventhough in case the\ncore.sparseCheckout = 1 (git config core.sparseCheckout true)\n\nIMHO, this change set the default value to -1, then it can fallback to\nrepo's config value if user has no-op passing (default to 0 (full\nbackfill if user doesnt intent to config previously either).\n> Derrick, what do you think?\n> \n> > diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\n> > index 91b5115732..a1a8d736db 100755\n> > --- a/t/t5620-backfill.sh\n> > +++ b/t/t5620-backfill.sh\n> > @@ -149,6 +149,21 @@ test_expect_success 'backfill --sparse' '\n> >  \ttest_line_count = 0 missing\n> >  '\n> >  \n> > +test_expect_success 'backfill auto-detects sparse-checkout from config' '\n> > +\tgit clone --sparse --filter=blob:none \\\n> > +\t\t--single-branch --branch=main \\\n> > +\t\t\"file://$(pwd)/srv.bare\" backfill-auto-sparse &&\n> > +\n> > +\tgit -C backfill-auto-sparse rev-list --quiet --objects --missing=print HEAD >missing &&\n> > +\ttest_line_count = 44 missing &&\n> > +\n> > +\tGIT_TRACE2_EVENT=\"$(pwd)/auto-sparse-trace\" git \\\n> > +\t\t-C backfill-auto-sparse backfill &&\n> > +\n> > +\ttest_trace2_data promisor fetch_count 4 <auto-sparse-trace &&\n> > +\ttest_trace2_data path-walk paths 5 <auto-sparse-trace\n> > +'\n> > +\n> >  test_expect_success 'backfill --sparse without cone mode (positive)' '\n> >  \tgit clone --no-checkout --filter=blob:none\t\t\\\n> >  \t\t--single-branch --branch=main \t\t\\\n"},{"id":"540742","messageId":"b7164e46-0521-4c0c-984e-35fc1891e4bd@gmail.com","threadId":"65393","inReplyTo":"buisigjsw3zrcy6bqaic2zefypq37kimju32eufquppsvkgkvx@cqd3cwj6an6t","subject":"Re: [GSoC PATCH] backfill: auto-detect sparse-checkout from config","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-04-02T14:24:45Z","receivedAt":"2026-04-02T14:24:47Z","isPatch":true,"body":"On 4/1/26 3:31 PM, Trieu Huynh wrote:\n> On Tue, Mar 31, 2026 at 09:59:10AM -0700, Junio C Hamano wrote:\n\n>>> diff --git a/builtin/backfill.c b/builtin/backfill.c\n>>> index 4b2db94173..0f31844ce7 100644\n>>> --- a/builtin/backfill.c\n>>> +++ b/builtin/backfill.c\n>>> @@ -124,7 +124,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n>>>   \t\t.repo = repo,\n>>>   \t\t.current_batch = OID_ARRAY_INIT,\n>>>   \t\t.min_batch_size = 50000,\n>>> -\t\t.sparse = 0,\n>>> +\t\t.sparse = -1,\n>>>   \t\t.show_progress = -1,\n>>>   \t};\n>>>   \tstruct option options[] = {\n>>\n>> I am a bit confused by this change.  What's the difference between\n>> using -1 (which you picked) and 1 as the initial value for this\n>> member?  From the proposed log message, I would have expected a new\n>> code that says \"ah, we notice, from this member being -1, that the\n>> user did not specify --no-sparse or --sparse, so let's figure out if\n>> our working tree is sparsely checked out ourselves and set it either\n>> to 0 or to 1\", but there is nothing like that in the code.  It seems\n>> that the updated code relies on the fact that this part of\n>> do_backfill() only cares if .sparse is zero or not, and ...\n>>\n>> \tif (ctx->sparse) {\n>> \t\tCALLOC_ARRAY(info.pl, 1);\n>> \t\tif (get_sparse_checkout_patterns(info.pl)) {\n>> \t\t\tpath_walk_info_clear(&info);\n>> \t\t\treturn error(_(\"problem loading sparse-checkout\"));\n>> \t\t}\n>> \t}\n>>\n>> ... relies on get_sparse_checkout_patterns() not to do any harm when\n>> the working tree is not sparsely checked out.\n>>\n>> I am not sure if we want to call it \"auto-detction\".  It looks more\n>> like \"default to --sparse, relying that --sparse is a no-op in a\n>> non-sparse working tree\" at least to me.  Not that it is necessarily\n>> wrong, and when people do \"backfill\" knowing that the working tree\n>> is sparse, I am sympathetic if they prefer to keep the sparseness,\n>> so such a change of default may be beneficial.\n>>\n> actually, the logic IIUC here is:\n> - first, ctx.sprase originally is set to 0.\n> - then, it check user's options. Assume, there is no option passed,\n> still 0.\n> - then, it check repo's config (core.sparseCheckout (default to 0 in\n> enviroment.c) but it doesn't since the guard:\n> \tif (ctx.sparse < 0)\n> \t\tctx.sparse = cfg->apply_sparse_checkout;\n> - evenly. ctx.sparse still 0 eventhough in case the\n> core.sparseCheckout = 1 (git config core.sparseCheckout true)\n> \n> IMHO, this change set the default value to -1, then it can fallback to\n> repo's config value if user has no-op passing (default to 0 (full\n> backfill if user doesnt intent to config previously either).\n>> Derrick, what do you think?\n\nIndeed, I thought this was how it already worked, as 85127bcdea\n(backfill: assume --sparse when sparse-checkout is enabled,\n2025-02-03) (introduced in [1]) should have covered.\n\n[1] \nhttps://lore.kernel.org/git/f22cf8b34851a3ba4cd6ab1d31f0835579143c40.1738602667.git.gitgitgadget@gmail.com/\n\nHowever, that change did not include this test:\n\n>>> +test_expect_success 'backfill auto-detects sparse-checkout from config' '\n>>> +\tgit clone --sparse --filter=blob:none \\\n>>> +\t\t--single-branch --branch=main \\\n>>> +\t\t\"file://$(pwd)/srv.bare\" backfill-auto-sparse &&\n>>> +\n>>> +\tgit -C backfill-auto-sparse rev-list --quiet --objects --missing=print HEAD >missing &&\n>>> +\ttest_line_count = 44 missing &&\n>>> +\n>>> +\tGIT_TRACE2_EVENT=\"$(pwd)/auto-sparse-trace\" git \\\n>>> +\t\t-C backfill-auto-sparse backfill &&\n>>> +\n>>> +\ttest_trace2_data promisor fetch_count 4 <auto-sparse-trace &&\n>>> +\ttest_trace2_data path-walk paths 5 <auto-sparse-trace\n>>> +'\n\nThis actually demonstrates the behavior that I was intending\nto introduce in that change, but made an error in (1) not testing\nit like I thought I had, and (2) not doing exactly what this change\ndoes by having the .sparse option initialized as -1 (unset).\n\nThe code and commit message do a good job of identifying this bug\nand difference in behavior. The only suggestion I have is to update\nthe commit message to point to my original commit that failed to\nimplement this behavior in the expected way.\n\nThanks,\n-Stolee\n\n\n\n"},{"id":"540772","messageId":"xmqqh5pts5zf.fsf@gitster.g","threadId":"65393","inReplyTo":"b7164e46-0521-4c0c-984e-35fc1891e4bd@gmail.com","subject":"Re: [GSoC PATCH] backfill: auto-detect sparse-checkout from config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-04-02T16:05:08Z","receivedAt":"2026-04-02T16:05:11Z","isPatch":true,"body":"Derrick Stolee <stolee@gmail.com> writes:\n\n> On 4/1/26 3:31 PM, Trieu Huynh wrote:\n>> On Tue, Mar 31, 2026 at 09:59:10AM -0700, Junio C Hamano wrote:\n> ...\n>>> I am a bit confused by this change.  What's the difference between\n>>> using -1 (which you picked) and 1 as the initial value for this\n>>> member?  From the proposed log message, I would have expected a new\n>>> code that says \"ah, we notice, from this member being -1, that the\n>>> user did not specify --no-sparse or --sparse, so let's figure out if\n>>> our working tree is sparsely checked out ourselves and set it either\n>>> to 0 or to 1\", but there is nothing like that in the code.\n>> ...\n>> IMHO, this change set the default value to -1, then it can fallback to\n>> repo's config value if user has no-op passing (default to 0 (full\n>> backfill if user doesnt intent to config previously either).\n>>> Derrick, what do you think?\n>\n> Indeed, I thought this was how it already worked, as 85127bcdea\n> (backfill: assume --sparse when sparse-checkout is enabled,\n> 2025-02-03) (introduced in [1]) should have covered.\n\nAh, OK, in other words, the code to use how the repository is\nconfigured when the user does not override from the command line was\nalready there; it was just the way the code checked if the user gave\nsomething from the command line was wrong (i.e., initialized to 0,\npretending that '--no-foo\" was given even when there isn't), and\nthat is why we do not see \"ok, there is nothing on the command line,\nso let's check the repository\" _added_ by the patch---because it has\nalways been there.  Makes sense.\n\n> The code and commit message do a good job of identifying this bug\n> and difference in behavior. The only suggestion I have is to update\n> the commit message to point to my original commit that failed to\n> implement this behavior in the expected way.\n\nSounds sensible and makes sense.\n\nThanks, both.  Let's see a (hopefully small and final) reroll.\n\n"}]}