{"thread":{"id":"66102","subject":"[PATCH] read-cache: avoid sparse-index expansion for unborn HEAD","startedAt":"2026-08-02T21:28:40Z","lastAt":"2026-09-11T21:31:43Z","messageCount":6,"participants":["Sahitya Chandra","Elijah Newren"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"549448","messageId":"20260802212826.1090943-1-sahityajb@gmail.com","threadId":"66102","inReplyTo":null,"subject":"[PATCH] read-cache: avoid sparse-index expansion for unborn HEAD","fromName":"Sahitya Chandra","fromEmail":"sahityajb@gmail.com","sentAt":"2026-08-02T21:28:26Z","receivedAt":"2026-08-02T21:28:40Z","isPatch":true,"body":"repo_index_has_changes() normally checks whether the index differs from\na tree by passing that tree to the diff machinery. When no tree is\npassed, it tries to use HEAD for that comparison.\n\nIf HEAD does not resolve, as on an unborn branch, the function falls\nback to walking the index directly. With a sparse index, however, sparse\ndirectory entries may stand in for many paths, so the fallback first\nexpands the index before reporting the changed paths.\n\nThat expansion is unnecessary. An unborn HEAD is equivalent for this\ncheck to comparing the index against the empty tree: every index entry\nis new relative to that tree.\n\nUse the empty tree when HEAD cannot be resolved. This keeps the\nunborn-branch case on the same diff code path as the normal\ntree-comparison case, avoiding the sparse-index expansion while still\nletting callers see paths inside sparse directories.\n\nTeach test-tool read-cache to exercise repo_index_has_changes(), and\nadd a t1092 check that the unborn-branch case reports paths inside a\nsparse directory without expanding the index.\n\nSigned-off-by: Sahitya Chandra <sahityajb@gmail.com>\n---\n read-cache.c                             | 46 ++++++++++--------------\n t/helper/test-read-cache.c               | 19 ++++++++++\n t/t1092-sparse-checkout-compatibility.sh | 16 +++++++++\n 3 files changed, 53 insertions(+), 28 deletions(-)\n\ndiff --git a/read-cache.c b/read-cache.c\nindex 6c449f393d..88ee9ba935 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -2505,39 +2505,29 @@ int repo_index_has_changes(struct repository *repo,\n \t\t\t   struct tree *tree,\n \t\t\t   struct strbuf *sb)\n {\n-\tstruct index_state *istate = repo->index;\n+\tstruct diff_options opt;\n \tstruct object_id cmp;\n \tint i;\n \n \tif (tree)\n \t\tcmp = tree->object.oid;\n-\tif (tree || !repo_get_oid_tree(repo, \"HEAD\", &cmp)) {\n-\t\tstruct diff_options opt;\n-\n-\t\trepo_diff_setup(repo, &opt);\n-\t\topt.flags.exit_with_status = 1;\n-\t\tif (!sb)\n-\t\t\topt.flags.quick = 1;\n-\t\tdiff_setup_done(&opt);\n-\t\tdo_diff_cache(&cmp, &opt);\n-\t\tdiffcore_std(&opt);\n-\t\tfor (i = 0; sb && i < diff_queued_diff.nr; i++) {\n-\t\t\tif (i)\n-\t\t\t\tstrbuf_addch(sb, ' ');\n-\t\t\tstrbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);\n-\t\t}\n-\t\tdiff_flush(&opt);\n-\t\treturn opt.flags.has_changes != 0;\n-\t} else {\n-\t\t/* TODO: audit for interaction with sparse-index. */\n-\t\tensure_full_index(istate);\n-\t\tfor (i = 0; sb && i < istate->cache_nr; i++) {\n-\t\t\tif (i)\n-\t\t\t\tstrbuf_addch(sb, ' ');\n-\t\t\tstrbuf_addstr(sb, istate->cache[i]->name);\n-\t\t}\n-\t\treturn !!istate->cache_nr;\n-\t}\n+\telse if (repo_get_oid_tree(repo, \"HEAD\", &cmp))\n+\t\toidcpy(&cmp, repo->hash_algo->empty_tree);\n+\n+\trepo_diff_setup(repo, &opt);\n+\topt.flags.exit_with_status = 1;\n+\tif (!sb)\n+\t\topt.flags.quick = 1;\n+\tdiff_setup_done(&opt);\n+\tdo_diff_cache(&cmp, &opt);\n+\tdiffcore_std(&opt);\n+\tfor (i = 0; sb && i < diff_queued_diff.nr; i++) {\n+\t\tif (i)\n+\t\t\tstrbuf_addch(sb, ' ');\n+\t\tstrbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);\n+\t}\n+\tdiff_flush(&opt);\n+\treturn opt.flags.has_changes != 0;\n }\n \n static int write_index_ext_header(struct hashfile *f,\ndiff --git a/t/helper/test-read-cache.c b/t/helper/test-read-cache.c\nindex 6b08ba8f07..ee629fbc69 100644\n--- a/t/helper/test-read-cache.c\n+++ b/t/helper/test-read-cache.c\n@@ -4,6 +4,7 @@\n #include \"config.h\"\n #include \"environment.h\"\n #include \"read-cache-ll.h\"\n+#include \"repo-settings.h\"\n #include \"repository.h\"\n #include \"setup.h\"\n \n@@ -12,6 +13,24 @@ int cmd__read_cache(int argc, const char **argv)\n \tint i, cnt = 1;\n \tconst char *name = NULL;\n \n+\tif (argc == 2 && !strcmp(argv[1], \"--index-has-changes\")) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\t\tint ret;\n+\n+\t\tsetup_git_directory(the_repository);\n+\t\trepo_config(the_repository, git_default_config, NULL);\n+\t\tprepare_repo_settings(the_repository);\n+\t\tthe_repository->settings.command_requires_full_index = 0;\n+\n+\t\trepo_read_index(the_repository);\n+\t\tret = repo_index_has_changes(the_repository, NULL, &sb);\n+\t\tprintf(\"has_changes=%d\\n\", ret);\n+\t\tif (sb.len)\n+\t\t\tprintf(\"dirty=%s\\n\", sb.buf);\n+\t\tstrbuf_release(&sb);\n+\t\treturn 0;\n+\t}\n+\n \tif (argc > 1 && skip_prefix(argv[1], \"--print-and-refresh=\", &name)) {\n \t\targc--;\n \t\targv++;\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 4140c4d8ef..90239a862d 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -1558,6 +1558,22 @@ test_expect_success 'sparse-index is not expanded' '\n \t)\n '\n \n+test_expect_success 'sparse-index is not expanded: index has changes on unborn branch' '\n+\tinit_repos &&\n+\tgit -C sparse-index checkout --orphan unborn &&\n+\tgit -C sparse-index ls-files --sparse --stage >cache &&\n+\ttest_grep \"^040000 .*\tfolder1/$\" cache &&\n+\n+\trm -f trace2.txt &&\n+\tGIT_TRACE2_EVENT=\"$(pwd)/trace2.txt\" GIT_TRACE2_EVENT_NESTING=10 \\\n+\t\ttest-tool -C sparse-index read-cache --index-has-changes \\\n+\t\t>sparse-index-out 2>sparse-index-error &&\n+\ttest_region ! index ensure_full_index trace2.txt &&\n+\ttest_must_be_empty sparse-index-error &&\n+\ttest_grep \"has_changes=1\" sparse-index-out &&\n+\ttest_grep \"folder1/a\" sparse-index-out\n+'\n+\n test_expect_success 'sparse-index is not expanded: merge conflict in cone' '\n \tinit_repos &&\n \n\nbase-commit: a97fcc37c2bc6340a8d7ce78dedf227aac4e9aa7\n-- \n2.43.0\n\n"},{"id":"549858","messageId":"CAP=WS+vys5ob20mkxpzPqUjeCqG6hm7-EeDdec0Y0NaBc+tT1A@mail.gmail.com","threadId":"66102","inReplyTo":"20260802212826.1090943-1-sahityajb@gmail.com","subject":"Re: [PATCH] read-cache: avoid sparse-index expansion for unborn HEAD","fromName":"Sahitya Chandra","fromEmail":"sahityajb@gmail.com","sentAt":"2026-08-06T15:08:55Z","receivedAt":"2026-08-06T15:09:09Z","isPatch":true,"body":"Just a gentle ping on this patch.\n\nThanks,\nSahitya\n"},{"id":"549937","messageId":"CABPp-BGYuQA_ngR3xS-_Mndzf_ubkn7rSc25CJG=UbLCVGdnyg@mail.gmail.com","threadId":"66102","inReplyTo":"20260802212826.1090943-1-sahityajb@gmail.com","subject":"Re: [PATCH] read-cache: avoid sparse-index expansion for unborn HEAD","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-08-07T06:47:32Z","receivedAt":"2026-08-07T06:47:44Z","isPatch":true,"body":"On Sun, Aug 2, 2026 at 2:28 PM Sahitya Chandra <sahityajb@gmail.com> wrote:\n>\n> repo_index_has_changes() normally checks whether the index differs from\n> a tree by passing that tree to the diff machinery. When no tree is\n> passed, it tries to use HEAD for that comparison.\n>\n> If HEAD does not resolve, as on an unborn branch, the function falls\n> back to walking the index directly. With a sparse index, however, sparse\n> directory entries may stand in for many paths, so the fallback first\n> expands the index before reporting the changed paths.\n>\n> That expansion is unnecessary. An unborn HEAD is equivalent for this\n> check to comparing the index against the empty tree: every index entry\n> is new relative to that tree.\n>\n> Use the empty tree when HEAD cannot be resolved. This keeps the\n> unborn-branch case on the same diff code path as the normal\n> tree-comparison case, avoiding the sparse-index expansion while still\n> letting callers see paths inside sparse directories.\n>\n> Teach test-tool read-cache to exercise repo_index_has_changes(), and\n> add a t1092 check that the unborn-branch case reports paths inside a\n> sparse directory without expanding the index.\n\nThis explains what, but not why.  It feels like a pedagogical exercise\nwith no actual utility.  Why would someone with an unborn HEAD be\nusing a sparse index?  They have millions of files, with none of them\ncommitted, except they don't have millions of files because they only\nhave paths under certain directories?  How did they even get the\nrelevant tree entries into the sparse index in order to have one?\n\nPerhaps you have a great usecase and I've just missed it.  Could you\nexplain the motivation for enabling this?  Or was it more a case of\ntrying to take care of TODOs in the code?\n\n[...]\n> @@ -12,6 +13,24 @@ int cmd__read_cache(int argc, const char **argv)\n>         int i, cnt = 1;\n>         const char *name = NULL;\n>\n> +       if (argc == 2 && !strcmp(argv[1], \"--index-has-changes\")) {\n> +               struct strbuf sb = STRBUF_INIT;\n> +               int ret;\n> +\n> +               setup_git_directory(the_repository);\n> +               repo_config(the_repository, git_default_config, NULL);\n> +               prepare_repo_settings(the_repository);\n> +               the_repository->settings.command_requires_full_index = 0;\n> +\n> +               repo_read_index(the_repository);\n> +               ret = repo_index_has_changes(the_repository, NULL, &sb);\n> +               printf(\"has_changes=%d\\n\", ret);\n> +               if (sb.len)\n> +                       printf(\"dirty=%s\\n\", sb.buf);\n\nThis seems to presume a single dirty file, otherwise wouldn't the\nprinting look pretty odd?\n"},{"id":"549958","messageId":"CAP=WS+sp74WQ=xndQ+2a6W-qP3Zz8=bVnEymgVpS+gwMv1Dh7g@mail.gmail.com","threadId":"66102","inReplyTo":"CABPp-BGYuQA_ngR3xS-_Mndzf_ubkn7rSc25CJG=UbLCVGdnyg@mail.gmail.com","subject":"Re: [PATCH] read-cache: avoid sparse-index expansion for unborn HEAD","fromName":"Sahitya Chandra","fromEmail":"sahityajb@gmail.com","sentAt":"2026-08-07T08:05:08Z","receivedAt":"2026-08-07T08:05:22Z","isPatch":true,"body":"On Fri, Aug 7, 2026 at 12:17 PM Elijah Newren <newren@gmail.com> wrote:\n>\n> This explains what, but not why. It feels like a pedagogical exercise\n> with no actual utility.\n\nYou are right, I found this through the TODO comment and do not have a\nconcrete user bug report or use case driving it.\n\n> Why would someone with an unborn HEAD be using a sparse index? [...]\n\nI do not have a good answer to that. My thinking was simply that\nremoving the special-case fallback still has some value: it deletes a\nlong-standing TODO, unifies the unborn-branch path with the normal diff\npath, and removes an ensure_full_index() call that future readers would\nneed to reason about.\n\n> This seems to presume a single dirty file, otherwise wouldn't the\n> printing look pretty odd?\n\nI agree that \"dirty=%s\" looks wrong when multiple paths are\npresent. I can fix that in v2.\n\nThanks for the review.\n"},{"id":"550018","messageId":"CABPp-BHLaW6_CxMdPQURN7zMK1p7dEkihFMAkyWvcd2+j7gJqw@mail.gmail.com","threadId":"66102","inReplyTo":"CAP=WS+sp74WQ=xndQ+2a6W-qP3Zz8=bVnEymgVpS+gwMv1Dh7g@mail.gmail.com","subject":"Re: [PATCH] read-cache: avoid sparse-index expansion for unborn HEAD","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-08-07T15:33:23Z","receivedAt":"2026-08-07T15:33:35Z","isPatch":true,"body":"On Fri, Aug 7, 2026 at 1:05 AM Sahitya Chandra <sahityajb@gmail.com> wrote:\n>\n> On Fri, Aug 7, 2026 at 12:17 PM Elijah Newren <newren@gmail.com> wrote:\n> >\n> > This explains what, but not why. It feels like a pedagogical exercise\n> > with no actual utility.\n>\n> You are right, I found this through the TODO comment and do not have a\n> concrete user bug report or use case driving it.\n>\n> > Why would someone with an unborn HEAD be using a sparse index? [...]\n>\n> I do not have a good answer to that. My thinking was simply that\n> removing the special-case fallback still has some value: it deletes a\n> long-standing TODO, unifies the unborn-branch path with the normal diff\n> path, and removes an ensure_full_index() call that future readers would\n> need to reason about.\n\nAh, thanks for looking through the code for TODOs and trying to clean\nthem up.  That's noble.\n\nIf you submit a v2, it's probably worth just being upfront about this\nin the commit message -- that we don't expect this to be used in\npractice, but it makes sense both (a) to remove one more TODO, and (b)\nbecause it provides a net reduction in lines of code in read-cache.c.\n\n> > This seems to presume a single dirty file, otherwise wouldn't the\n> > printing look pretty odd?\n>\n> I agree that \"dirty=%s\" looks wrong when multiple paths are\n> present. I can fix that in v2.\n\n:-)\n\nI'm curious if the unittesting harness could help here and avoid the\nneed for the test helper changes.  Is that possible?  (I don't\nactually know much about the unittesting harness abilities, so I'm\ngenuinely curious).\n\n> Thanks for the review.\n\nThanks for contributing!\n"},{"id":"552603","messageId":"CAP=WS+vySE94LZ_4CtEU6Hd99DttpF-49F3OaChTU53hdbzf1g@mail.gmail.com","threadId":"66102","inReplyTo":"CABPp-BHLaW6_CxMdPQURN7zMK1p7dEkihFMAkyWvcd2+j7gJqw@mail.gmail.com","subject":"Re: [PATCH] read-cache: avoid sparse-index expansion for unborn HEAD","fromName":"Sahitya Chandra","fromEmail":"sahityajb@gmail.com","sentAt":"2026-09-11T21:31:29Z","receivedAt":"2026-09-11T21:31:43Z","isPatch":true,"body":"Hi Elijah,\nSorry for the long gap, and thanks for the review.\n\nOn Fri, Aug 7, 2026 at 9:03 PM Elijah Newren <newren@gmail.com> wrote:\n>\n> If you submit a v2, it's probably worth just being upfront about this\n> in the commit message\n\nThat makes sense. I'll frame it as a cleanup of the TODO and a reduction\nin read-cache.c, with no known practical use case.\n\n> I'm curious if the unittesting harness could help here and avoid the\n> need for the test helper changes. Is that possible?\n\nI looked into the Clar harness and the existing unit tests. It supports\nfixtures, but I didn't find repository/index setup helpers to reuse for\nthis case. A unit test may be possible with additional setup, though\nkeeping this in t1092 would let us reuse its sparse repository setup\nand Trace2 checks to verify that the index stays unexpanded.\n\nWould keeping the shell test and making the helper output clearer for\nmultiple paths be reasonable here?\n\nThanks,\nSahitya\n"}]}