{"thread":{"id":"65272","subject":"[PATCH 0/5] backfill: accept revision arguments","startedAt":"2026-03-17T00:29:24Z","lastAt":"2026-03-27T07:07:21Z","messageCount":46,"participants":["Derrick Stolee via GitGitGadget","Junio C Hamano","Derrick Stolee","Kristoffer Haugsbakk","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"539183","messageId":"pull.2070.git.1773707361.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":null,"subject":"[PATCH 0/5] backfill: accept revision arguments","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-17T00:29:16Z","receivedAt":"2026-03-17T00:29:24Z","isPatch":true,"body":"The git backfill command assists in downloading missing blobs for blobless\npartial clones. However, its current version lacks some valuable\nfunctionality. It currently:\n\n 1. Only walks commits reachable from HEAD.\n 2. It walks all reachable commits to the full history.\n 3. It can focus on the current sparse-checkout definition, but otherwise it\n doesn't focus on a given pathspec.\n\nAll of these are being updated by this patch series, which allows rev-list\noptions to impact the path-walk. These include:\n\n 1. Specifying a given refspec, including --all.\n 2. Modifying the commit walk, including --first-parent, commit ranges, or\n recency using --since.\n 3. Modifying the set of paths to download using pathspecs.\n\nOne particularly valuable situation here is that now a user can run git\nbackfill -- to download all versions of a specific file or a specific\ndirectory, accelerating history queries within that path without downloading\nmore than necessary. This can accelerate git blame or git log -L for these\npaths, where normally those commands download missing blobs one-by-one\nduring its diff algorithms.\n\nThis patch series is organized in the following way:\n\n 1. A missing #include is added to prevent future compilation issues.\n 2. The test repo in t5620 is expanded to make later tests more interesting.\n 3. The backfill builtin parses the rev-list arguments. We test the top\n arguments that work as expected, though the pathspec arguments need\n extra work.\n 4. Update the path-walk logic to work efficiently with some pathspecs, such\n as fixed prefix pathspecs, accelerating the computation.\n 5. For more complicated pathspecs, do a post-filter in builtin/backfill.c\n instead of restricting the walk in the path-walk API.\n\nThe main goal of this series is to make such customizations possible, and to\nimprove performance where common use cases are expected. I'm open to\nfeedback as to whether we should consider more detailed performance analysis\nor whether we should wait for how users interact with these new options\nbefore overoptimizing unlikely use cases.\n\nThanks, -Stolee\n\nDerrick Stolee (5):\n revision: include object-name.h\n t5620: prepare branched repo for revision tests\n backfill: accept revision arguments\n backfill: work with prefix pathspecs\n path-walk: support wildcard pathspecs for blob filtering\n\n Documentation/git-backfill.adoc | 3 +\n builtin/backfill.c | 19 ++-\n path-walk.c | 61 ++++++++++\n revision.h | 1 +\n t/t5620-backfill.sh | 203 +++++++++++++++++++++++++++++++-\n 5 files changed, 279 insertions(+), 8 deletions(-)\n\n\nbase-commit: 67ad42147a7acc2af6074753ebd03d904476118f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2070%2Fderrickstolee%2Fbackfill-revs-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2070/derrickstolee/backfill-revs-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2070\n-- \ngitgitgadget\n"},{"id":"539184","messageId":"fda0239103f6e2b9e76403144b9ed2e9205e1c2a.1773707361.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.git.1773707361.gitgitgadget@gmail.com","subject":"[PATCH 1/5] revision: include object-name.h","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-17T00:29:17Z","receivedAt":"2026-03-17T00:29:25Z","isPatch":true,"body":"From: Derrick Stolee \n\nThe REV_INFO_INIT macro includes a use of the DEFAULT_ABBREV macro, which is\ndefined in object-name.h. Include it in revision.h so consumers of\nREV_INFO_INIT do not need to include this hidden dependency.\n\nSigned-off-by: Derrick Stolee \n---\n revision.h | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/revision.h b/revision.h\nindex b36acfc2d9..18c9bbd822 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -4,6 +4,7 @@\n #include \"commit.h\"\n #include \"grep.h\"\n #include \"notes.h\"\n+#include \"object-name.h\"\n #include \"oidset.h\"\n #include \"pretty.h\"\n #include \"diff.h\"\n-- \ngitgitgadget\n\n"},{"id":"539185","messageId":"55a45b2fc81895d2b712f126f716a8152d73fc93.1773707361.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.git.1773707361.gitgitgadget@gmail.com","subject":"[PATCH 2/5] t5620: prepare branched repo for revision tests","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-17T00:29:18Z","receivedAt":"2026-03-17T00:29:27Z","isPatch":true,"body":"From: Derrick Stolee \n\nPrepare the test infrastructure for upcoming changes that teach 'git\nbackfill' to accept revision arguments and pathspecs.\n\nAdd test_tick before each commit in the setup loop so that commit dates\nare deterministic. This enables reliable testing with '--since'.\n\nRename the 'd/e/' directory to 'd/f/' so that the prefix 'd/f' is\nambiguous with the files 'd/file.*.txt'. This exercises the subtlety\nin prefix pathspec matching that will be added in a later commit.\n\nCreate a branched version of the test repository (src-revs) with:\n - A 'side' branch merged into main, adding s/file.{1,2}.txt with\n two versions (4 new blobs, 52 total from main HEAD).\n - An unmerged 'other' branch adding o/file.{1,2}.txt (2 more blobs,\n 54 total reachable from --all).\n\nThis structure makes --all, --first-parent, and --since produce\nmeaningfully different results when used with 'git backfill'.\n\nSigned-off-by: Derrick Stolee \n---\n t/t5620-backfill.sh | 52 +++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 50 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 58c81556e7..1331949be4 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -15,7 +15,7 @@ test_expect_success 'setup repo for object creation' '\n \tgit init src &&\n \n \tmkdir -p src/a/b/c &&\n-\tmkdir -p src/d/e &&\n+\tmkdir -p src/d/f &&\n \n \tfor i in 1 2\n \tdo\n@@ -26,8 +26,9 @@ test_expect_success 'setup repo for object creation' '\n \t\t\techo \"Version $i of file a/b/$n\" > src/a/b/file.$n.txt &&\n \t\t\techo \"Version $i of file a/b/c/$n\" > src/a/b/c/file.$n.txt &&\n \t\t\techo \"Version $i of file d/$n\" > src/d/file.$n.txt &&\n-\t\t\techo \"Version $i of file d/e/$n\" > src/d/e/file.$n.txt &&\n+\t\t\techo \"Version $i of file d/f/$n\" > src/d/f/file.$n.txt &&\n \t\t\tgit -C src add . &&\n+\t\t\ttest_tick &&\n \t\t\tgit -C src commit -m \"Iteration $n\" || return 1\n \t\tdone\n \tdone\n@@ -41,6 +42,53 @@ test_expect_success 'setup bare clone for server' '\n \tgit -C srv.bare config --local uploadpack.allowanysha1inwant 1\n '\n \n+# Create a version of the repo with branches for testing revision\n+# arguments like --all, --first-parent, and --since.\n+#\n+# main: 8 commits (linear) + merge of side branch\n+# 48 original blobs + 4 side blobs = 52 blobs from main HEAD\n+# side: 2 commits adding s/file.{1,2}.txt (v1, v2), merged into main\n+# other: 1 commit adding o/file.{1,2}.txt (not merged)\n+# 54 total blobs reachable from --all\n+test_expect_success 'setup branched repo for revision tests' '\n+\tgit clone src src-revs &&\n+\n+\t# Side branch from tip of main with unique files\n+\tgit -C src-revs checkout -b side HEAD &&\n+\tmkdir -p src-revs/s &&\n+\techo \"Side version 1 of file 1\" >src-revs/s/file.1.txt &&\n+\techo \"Side version 1 of file 2\" >src-revs/s/file.2.txt &&\n+\ttest_tick &&\n+\tgit -C src-revs add . &&\n+\tgit -C src-revs commit -m \"Side commit 1\" &&\n+\n+\techo \"Side version 2 of file 1\" >src-revs/s/file.1.txt &&\n+\techo \"Side version 2 of file 2\" >src-revs/s/file.2.txt &&\n+\ttest_tick &&\n+\tgit -C src-revs add . &&\n+\tgit -C src-revs commit -m \"Side commit 2\" &&\n+\n+\t# Merge side into main\n+\tgit -C src-revs checkout main &&\n+\ttest_tick &&\n+\tgit -C src-revs merge side --no-ff -m \"Merge side branch\" &&\n+\n+\t# Other branch (not merged) for --all testing\n+\tgit -C src-revs checkout -b other main~1 &&\n+\tmkdir -p src-revs/o &&\n+\techo \"Other content 1\" >src-revs/o/file.1.txt &&\n+\techo \"Other content 2\" >src-revs/o/file.2.txt &&\n+\ttest_tick &&\n+\tgit -C src-revs add . &&\n+\tgit -C src-revs commit -m \"Other commit\" &&\n+\n+\tgit -C src-revs checkout main &&\n+\n+\tgit clone --bare \"file://$(pwd)/src-revs\" srv-revs.bare &&\n+\tgit -C srv-revs.bare config --local uploadpack.allowfilter 1 &&\n+\tgit -C srv-revs.bare config --local uploadpack.allowanysha1inwant 1\n+'\n+\n # do basic partial clone from \"srv.bare\"\n test_expect_success 'do partial clone 1, backfill gets all objects' '\n \tgit clone --no-checkout --filter=blob:none\t\\\n-- \ngitgitgadget\n\n"},{"id":"539186","messageId":"dc6652c84c8d37b124eb76c2a9cdfdc4db4a149d.1773707361.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.git.1773707361.gitgitgadget@gmail.com","subject":"[PATCH 3/5] backfill: accept revision arguments","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-17T00:29:19Z","receivedAt":"2026-03-17T00:29:28Z","isPatch":true,"body":"From: Derrick Stolee \n\nThe existing implementation of 'git backfill' only includes downloading\nmissing blobs reachable from HEAD. Advanced uses may desire more general\ncommit limiting options, such as '--all' for all references, specifying a\ncommit range via negative references, or specifying a recency of use such as\nwith '--since='.\n\nAll of these options are available if we use setup_revisions() to parse the\nunknown arguments with the revision machinery. This opens up a large number\nof possibilities, only a small set of which are tested here.\n\nFor documentation, we avoid duplicating the option documentation and instead\nlink to the documentation of 'git rev-list'.\n\nNote that these arguments currently allow specifying a pathspec, which\nmodifies the commit history checks but does not limit the paths used in the\nbackfill logic. This will be updated in a future change.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-backfill.adoc | 3 +\n builtin/backfill.c | 19 ++--\n t/t5620-backfill.sh | 156 ++++++++++++++++++++++++++++++++\n 3 files changed, 172 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-backfill.adoc b/Documentation/git-backfill.adoc\nindex b8394dcf22..fdfe22d623 100644\n--- a/Documentation/git-backfill.adoc\n+++ b/Documentation/git-backfill.adoc\n@@ -63,9 +63,12 @@ OPTIONS\n \tcurrent sparse-checkout. If the sparse-checkout feature is enabled,\n \tthen `--sparse` is assumed and can be disabled with `--no-sparse`.\n \n+You may also specify the commit limiting options from linkgit:git-rev-list[1].\n+\n SEE ALSO\n --------\n linkgit:git-clone[1].\n+linkgit:git-rev-list[1].\n \n GIT\n ---\ndiff --git a/builtin/backfill.c b/builtin/backfill.c\nindex e80fc1b694..1b5595b27c 100644\n--- a/builtin/backfill.c\n+++ b/builtin/backfill.c\n@@ -35,6 +35,7 @@ struct backfill_context {\n \tstruct oid_array current_batch;\n \tsize_t min_batch_size;\n \tint sparse;\n+\tstruct rev_info revs;\n };\n \n static void backfill_context_clear(struct backfill_context *ctx)\n@@ -80,7 +81,6 @@ static int fill_missing_blobs(const char *path UNUSED,\n \n static int do_backfill(struct backfill_context *ctx)\n {\n-\tstruct rev_info revs;\n \tstruct path_walk_info info = PATH_WALK_INFO_INIT;\n \tint ret;\n \n@@ -92,13 +92,14 @@ static int do_backfill(struct backfill_context *ctx)\n \t\t}\n \t}\n \n-\trepo_init_revisions(ctx->repo, &revs, \"\");\n-\thandle_revision_arg(\"HEAD\", &revs, 0, 0);\n+\t/* Walk from HEAD if otherwise unspecified. */\n+\tif (!ctx->revs.pending.nr)\n+\t\thandle_revision_arg(\"HEAD\", &ctx->revs, 0, 0);\n \n \tinfo.blobs = 1;\n \tinfo.tags = info.commits = info.trees = 0;\n \n-\tinfo.revs = &revs;\n+\tinfo.revs = &ctx->revs;\n \tinfo.path_fn = fill_missing_blobs;\n \tinfo.path_fn_data = ctx;\n \n@@ -109,7 +110,6 @@ static int do_backfill(struct backfill_context *ctx)\n \t\tdownload_batch(ctx);\n \n \tpath_walk_info_clear(&info);\n-\trelease_revisions(&revs);\n \treturn ret;\n }\n \n@@ -121,6 +121,7 @@ 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.revs = REV_INFO_INIT,\n \t};\n \tstruct option options[] = {\n \t\tOPT_UNSIGNED(0, \"min-batch-size\", &ctx.min_batch_size,\n@@ -134,7 +135,12 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \t\t\t\t\t builtin_backfill_usage, options);\n \n \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n-\t\t\t 0);\n+\t\t\t PARSE_OPT_KEEP_UNKNOWN_OPT |\n+\t\t\t PARSE_OPT_KEEP_ARGV0 |\n+\t\t\t PARSE_OPT_KEEP_DASHDASH);\n+\n+\trepo_init_revisions(repo, &ctx.revs, prefix);\n+\targc = setup_revisions(argc, argv, &ctx.revs, NULL);\n \n \trepo_config(repo, git_default_config, NULL);\n \n@@ -143,5 +149,6 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \n \tresult = do_backfill(&ctx);\n \tbackfill_context_clear(&ctx);\n+\trelease_revisions(&ctx.revs);\n \treturn result;\n }\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 1331949be4..db66d8b614 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -224,6 +224,162 @@ test_expect_success 'backfill --sparse without cone mode (negative)' '\n \ttest_line_count = 12 missing\n '\n \n+test_expect_success 'backfill with revision range' '\n+\ttest_when_finished rm -rf backfill-revs &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-revs &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-revs rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\tgit -C backfill-revs backfill HEAD~2..HEAD &&\n+\n+\t# 30 objects downloaded.\n+\tgit -C backfill-revs rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 18 missing\n+'\n+\n+test_expect_success 'backfill with revisions over stdin' '\n+\ttest_when_finished rm -rf backfill-revs &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-revs &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-revs rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\tcat >in <<-EOF &&\n+\tHEAD\n+\t^HEAD~2\n+\tEOF\n+\n+\tgit -C backfill-revs backfill --stdin missing &&\n+\ttest_line_count = 18 missing\n+'\n+\n+test_expect_success 'backfill with prefix pathspec' '\n+\ttest_when_finished rm -rf backfill-path &&\n+\tgit clone --bare --filter=blob:none\t\t \\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-path &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\t# TODO: The pathspec should limit the downloaded blobs to\n+\t# only those matching the prefix \"d/f\", but currently all\n+\t# blobs are downloaded.\n+\tgit -C backfill-path backfill HEAD -- d/f &&\n+\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with multiple pathspecs' '\n+\ttest_when_finished rm -rf backfill-path &&\n+\tgit clone --bare --filter=blob:none\t\t \\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-path &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\t# TODO: The pathspecs should limit the downloaded blobs to\n+\t# only those matching \"d/f\" or \"a\", but currently all blobs\n+\t# are downloaded.\n+\tgit -C backfill-path backfill HEAD -- d/f a &&\n+\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with wildcard pathspec' '\n+\ttest_when_finished rm -rf backfill-path &&\n+\tgit clone --bare --filter=blob:none\t\t \\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-path &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\t# TODO: The wildcard pathspec should limit downloaded blobs,\n+\t# but currently all blobs are downloaded.\n+\tgit -C backfill-path backfill HEAD -- \"d/file.*.txt\" &&\n+\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with --all' '\n+\ttest_when_finished rm -rf backfill-all &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t\"file://$(pwd)/srv-revs.bare\" backfill-all &&\n+\n+\t# All blobs from all refs are missing\n+\tgit -C backfill-all rev-list --quiet --objects --all --missing=print >missing &&\n+\ttest_line_count = 54 missing &&\n+\n+\t# Backfill from HEAD gets main blobs only\n+\tgit -C backfill-all backfill HEAD &&\n+\n+\t# Other branch blobs still missing\n+\tgit -C backfill-all rev-list --quiet --objects --all --missing=print >missing &&\n+\ttest_line_count = 2 missing &&\n+\n+\t# Backfill with --all gets everything\n+\tgit -C backfill-all backfill --all &&\n+\n+\tgit -C backfill-all rev-list --quiet --objects --all --missing=print >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with --first-parent' '\n+\ttest_when_finished rm -rf backfill-fp &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main\t\t\t\\\n+\t\t\"file://$(pwd)/srv-revs.bare\" backfill-fp &&\n+\n+\tgit -C backfill-fp rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 52 missing &&\n+\n+\t# --first-parent skips the side branch commits, so\n+\t# s/file.{1,2}.txt v1 blobs (only in side commit 1) are missed.\n+\tgit -C backfill-fp backfill --first-parent HEAD &&\n+\n+\tgit -C backfill-fp rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 2 missing\n+'\n+\n+test_expect_success 'backfill with --since' '\n+\ttest_when_finished rm -rf backfill-since &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main\t\t\t\\\n+\t\t\"file://$(pwd)/srv-revs.bare\" backfill-since &&\n+\n+\tgit -C backfill-since rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 52 missing &&\n+\n+\t# Use a cutoff between commits 4 and 5 (between v1 and v2\n+\t# iterations). Commits 5-8 still carry v1 of files 2-4 in\n+\t# their trees, but v1 of file.1.txt is only in commits 1-4.\n+\tSINCE=$(git -C backfill-since log --first-parent --reverse \\\n+\t\t--format=%ct HEAD~1 | sed -n 5p) &&\n+\tgit -C backfill-since backfill --since=\"@$((SINCE - 1))\" HEAD &&\n+\n+\t# 6 missing: v1 of file.1.txt in all 6 directories\n+\tgit -C backfill-since rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 6 missing\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n \n-- \ngitgitgadget\n\n"},{"id":"539187","messageId":"977f62faa550ecc4ef7774b4ecf53a9a520fc62a.1773707361.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.git.1773707361.gitgitgadget@gmail.com","subject":"[PATCH 4/5] backfill: work with prefix pathspecs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-17T00:29:20Z","receivedAt":"2026-03-17T00:29:29Z","isPatch":true,"body":"From: Derrick Stolee \n\nThe previous change allowed specifying revision arguments over the 'git\nbackfill' command-line. This created the opportunity for pathspecs that\nspecify a smaller set of starting commits, but otherwise did not restrict\nthe blob paths that were downloaded.\n\nUpdate the path-walk API to accept certain kinds of pathspecs and to\nsilently ignore anything too complex. The current behavior focuses on\npathspecs that match paths exactly. This includes exact filenames,\nincluding directory names as prefixes. Pathspecs containing wildcards\nor magic are cleared so the path walk downloads all blobs, as before.\n\nThe reason for this restriction is to allow for a faster execution by\npruning the path walk to only trees that could contribute towards one of\nthose paths as a parent directory.\n\nThe test directory 'd/f/' (next to 'd/file*.txt') was prepared in a\nprevious commit to exercise the subtlety in prefix matching.\n\nSigned-off-by: Derrick Stolee \n---\n path-walk.c | 55 +++++++++++++++++++++++++++++++++++++++++++++\n t/t5620-backfill.sh | 16 +++++--------\n 2 files changed, 61 insertions(+), 10 deletions(-)\n\ndiff --git a/path-walk.c b/path-walk.c\nindex 364e4cfa19..e1ad4b0208 100644\n--- a/path-walk.c\n+++ b/path-walk.c\n@@ -206,6 +206,49 @@ static int add_tree_entries(struct path_walk_context *ctx,\n \t\t\t\t match != MATCHED)\n \t\t\t\tcontinue;\n \t\t}\n+\t\tif (ctx->revs->prune_data.nr) {\n+\t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n+\t\t\tbool found = false;\n+\n+\t\t\tfor (int i = 0; i < pd->nr; i++) {\n+\t\t\t\tstruct pathspec_item *item = &pd->items[i];\n+\n+\t\t\t\t/*\n+\t\t\t\t * Is this path a parent directory of\n+\t\t\t\t * the pathspec item?\n+\t\t\t\t */\n+\t\t\t\tif (path.len < (size_t)item->len &&\n+\t\t\t\t !strncmp(path.buf, item->match, path.len) &&\n+\t\t\t\t item->match[path.len - 1] == '/') {\n+\t\t\t\t\tfound = true;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\n+\t\t\t\t/*\n+\t\t\t\t * Or, is the pathspec an exact match?\n+\t\t\t\t */\n+\t\t\t\tif (path.len == (size_t)item->len &&\n+\t\t\t\t !strcmp(path.buf, item->match)) {\n+\t\t\t\t\tfound = true;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\n+\t\t\t\t/*\n+\t\t\t\t * Or, is the pathspec a directory prefix\n+\t\t\t\t * match?\n+\t\t\t\t */\n+\t\t\t\tif (path.len > (size_t)item->len &&\n+\t\t\t\t !strncmp(path.buf, item->match, item->len) &&\n+\t\t\t\t path.buf[item->len] == '/') {\n+\t\t\t\t\tfound = true;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\t/* Skip paths that do not match the prefix. */\n+\t\t\tif (!found)\n+\t\t\t\tcontinue;\n+\t\t}\n \n \t\tadd_path_to_list(ctx, path.buf, type, &entry.oid,\n \t\t\t\t !(o->flags & UNINTERESTING));\n@@ -481,6 +524,18 @@ int walk_objects_by_path(struct path_walk_info *info)\n \tif (info->tags)\n \t\tinfo->revs->tag_objects = 1;\n \n+\tif (ctx.revs->prune_data.nr) {\n+\t\t/*\n+\t\t * Only exact prefix pathspecs are currently supported.\n+\t\t * Clear any wildcard or magic pathspecs to avoid\n+\t\t * incorrect prefix matching.\n+\t\t */\n+\t\tstruct pathspec *pd = &ctx.revs->prune_data;\n+\n+\t\tif (pd->has_wildcard || pd->magic)\n+\t\t\tpd->nr = 0;\n+\t}\n+\n \t/* Insert a single list for the root tree into the paths. */\n \tCALLOC_ARRAY(root_tree_list, 1);\n \troot_tree_list->type = OBJ_TREE;\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex db66d8b614..52f6484ca1 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -273,13 +273,11 @@ test_expect_success 'backfill with prefix pathspec' '\n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n \ttest_line_count = 48 missing &&\n \n-\t# TODO: The pathspec should limit the downloaded blobs to\n-\t# only those matching the prefix \"d/f\", but currently all\n-\t# blobs are downloaded.\n-\tgit -C backfill-path backfill HEAD -- d/f &&\n+\tgit -C backfill-path backfill HEAD -- d/f 2>err &&\n+\ttest_must_be_empty err &&\n \n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n-\ttest_line_count = 0 missing\n+\ttest_line_count = 40 missing\n '\n \n test_expect_success 'backfill with multiple pathspecs' '\n@@ -292,13 +290,11 @@ test_expect_success 'backfill with multiple pathspecs' '\n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n \ttest_line_count = 48 missing &&\n \n-\t# TODO: The pathspecs should limit the downloaded blobs to\n-\t# only those matching \"d/f\" or \"a\", but currently all blobs\n-\t# are downloaded.\n-\tgit -C backfill-path backfill HEAD -- d/f a &&\n+\tgit -C backfill-path backfill HEAD -- d/f a 2>err &&\n+\ttest_must_be_empty err &&\n \n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n-\ttest_line_count = 0 missing\n+\ttest_line_count = 16 missing\n '\n \n test_expect_success 'backfill with wildcard pathspec' '\n-- \ngitgitgadget\n\n"},{"id":"539188","messageId":"beb1c92554c76907315a4d1a7983226d2bf5a828.1773707361.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.git.1773707361.gitgitgadget@gmail.com","subject":"[PATCH 5/5] path-walk: support wildcard pathspecs for blob filtering","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-17T00:29:21Z","receivedAt":"2026-03-17T00:29:31Z","isPatch":true,"body":"From: Derrick Stolee \n\nPreviously, walk_objects_by_path() silently ignored pathspecs containing\nwildcards or magic by clearing them. This caused all blobs to be\ndownloaded regardless of the given pathspec. Wildcard pathspecs like\n\"d/file.*.txt\" are useful for narrowing which blobs to process (e.g.,\nduring 'git backfill').\n\nSupport wildcard pathspecs by making three changes:\n\n 1. Add an 'exact_pathspecs' flag to path_walk_context. When the\n pathspec has no wildcards or magic, set this flag and use the\n existing fast-path prefix matching in add_tree_entries(). When\n wildcards are present, skip that block since prefix matching\n cannot handle glob patterns.\n\n 2. Disable revision-level commit pruning (revs->prune = 0) for\n wildcard pathspecs. The revision walk uses the pathspec to filter\n commits via TREESAME detection. For exact prefix pathspecs this\n works well, but wildcard pathspecs may fail to match through\n TREESAME because fnmatch with WM_PATHNAME does not cross directory\n boundaries. Disabling pruning ensures all commits are visited and\n their trees are available for the path-walk to filter.\n\n 3. Add a match_pathspec() check in walk_path() to filter out blobs\n whose full path does not match the pathspec. This provides the\n actual blob-level filtering for wildcard pathspecs.\n\nSigned-off-by: Derrick Stolee \nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>\n---\n path-walk.c | 22 ++++++++++++++--------\n t/t5620-backfill.sh | 7 +++----\n 2 files changed, 17 insertions(+), 12 deletions(-)\n\ndiff --git a/path-walk.c b/path-walk.c\nindex e1ad4b0208..67fb0f7572 100644\n--- a/path-walk.c\n+++ b/path-walk.c\n@@ -62,6 +62,8 @@ struct path_walk_context {\n \t */\n \tstruct prio_queue path_stack;\n \tstruct strset path_stack_pushed;\n+\n+\tunsigned exact_pathspecs:1;\n };\n \n static int compare_by_type(const void *one, const void *two, void *cb_data)\n@@ -206,7 +208,7 @@ static int add_tree_entries(struct path_walk_context *ctx,\n \t\t\t\t match != MATCHED)\n \t\t\t\tcontinue;\n \t\t}\n-\t\tif (ctx->revs->prune_data.nr) {\n+\t\tif (ctx->revs->prune_data.nr && ctx->exact_pathspecs) {\n \t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n \t\t\tbool found = false;\n \n@@ -317,6 +319,13 @@ static int walk_path(struct path_walk_context *ctx,\n \t\t\treturn 0;\n \t}\n \n+\tif (list->type == OBJ_BLOB &&\n+\t ctx->revs->prune_data.nr &&\n+\t !match_pathspec(ctx->repo->index, &ctx->revs->prune_data,\n+\t\t\t path, strlen(path), 0,\n+\t\t\t NULL, 0))\n+\t\treturn 0;\n+\n \t/* Evaluate function pointer on this data, if requested. */\n \tif ((list->type == OBJ_TREE && ctx->info->trees) ||\n \t (list->type == OBJ_BLOB && ctx->info->blobs) ||\n@@ -525,15 +534,12 @@ int walk_objects_by_path(struct path_walk_info *info)\n \t\tinfo->revs->tag_objects = 1;\n \n \tif (ctx.revs->prune_data.nr) {\n-\t\t/*\n-\t\t * Only exact prefix pathspecs are currently supported.\n-\t\t * Clear any wildcard or magic pathspecs to avoid\n-\t\t * incorrect prefix matching.\n-\t\t */\n \t\tstruct pathspec *pd = &ctx.revs->prune_data;\n \n-\t\tif (pd->has_wildcard || pd->magic)\n-\t\t\tpd->nr = 0;\n+\t\tif (!pd->has_wildcard && !pd->magic)\n+\t\t\tctx.exact_pathspecs = 1;\n+\t\telse\n+\t\t\tctx.revs->prune = 0;\n \t}\n \n \t/* Insert a single list for the root tree into the paths. */\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 52f6484ca1..c6f54ee91c 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -307,12 +307,11 @@ test_expect_success 'backfill with wildcard pathspec' '\n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n \ttest_line_count = 48 missing &&\n \n-\t# TODO: The wildcard pathspec should limit downloaded blobs,\n-\t# but currently all blobs are downloaded.\n-\tgit -C backfill-path backfill HEAD -- \"d/file.*.txt\" &&\n+\tgit -C backfill-path backfill HEAD -- \"d/file.*.txt\" 2>err &&\n+\ttest_must_be_empty err &&\n \n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n-\ttest_line_count = 0 missing\n+\ttest_line_count = 40 missing\n '\n \n test_expect_success 'backfill with --all' '\n-- \ngitgitgadget\n"},{"id":"539261","messageId":"xmqq7brajfjd.fsf@gitster.g","threadId":"65272","inReplyTo":"pull.2070.git.1773707361.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/5] backfill: accept revision arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-17T21:45:58Z","receivedAt":"2026-03-17T21:46:01Z","isPatch":true,"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> The git backfill command assists in downloading missing blobs for blobless\n> partial clones. However, its current version lacks some valuable\n> functionality. It currently:\n>\n> 1. Only walks commits reachable from HEAD.\n> 2. It walks all reachable commits to the full history.\n> 3. It can focus on the current sparse-checkout definition, but otherwise it\n> doesn't focus on a given pathspec.\n>\n> All of these are being updated by this patch series, which allows rev-list\n> options to impact the path-walk. These include:\n>\n> 1. Specifying a given refspec, including --all.\n\nMakes sense. You can only be on a single branch at a time, but may\nwant to work on multiple topics in reasonably quick succession in a\nsingle repository. Being able to prepare enough material to go back\nto when working on whichever topic in a single backfill invocation\nwould be a welcome addition.\n\n> 2. Modifying the commit walk, including --first-parent, commit ranges, or\n> recency using --since.\n> 3. Modifying the set of paths to download using pathspecs.\n\nBoth are good mechanisms to express which subset of history you will\nbe working on.\n\n> One particularly valuable situation here is that now a user can run git\n> backfill -- to download all versions of a specific file or a specific\n> directory, accelerating history queries within that path without downloading\n> more than necessary. This can accelerate git blame or git log -L for these\n> paths, where normally those commands download missing blobs one-by-one\n> during its diff algorithms.\n\nYup. Even if your project is a huge monorepo that contains all, you\ndo not necessarily have to look at everything the organization has\nall the time. \"git blame -C -C -C\" would of course not work in such\nan environment (would it end up on-demand lazy fetch these blobs, or\nare there ways to say \"I know the object store of my repository is\nonly sparsely populated, and I do not want you to on-demand download\nthe missing blobs---do your best to work with only what is already\navailable?), but that's a tradeoff a monorepo makes.\n\n"},{"id":"539262","messageId":"xmqq341yjf8g.fsf@gitster.g","threadId":"65272","inReplyTo":"fda0239103f6e2b9e76403144b9ed2e9205e1c2a.1773707361.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/5] revision: include object-name.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-17T21:52:31Z","receivedAt":"2026-03-17T21:52:33Z","isPatch":true,"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> From: Derrick Stolee \n>\n> The REV_INFO_INIT macro includes a use of the DEFAULT_ABBREV macro, which is\n> defined in object-name.h. Include it in revision.h so consumers of\n> REV_INFO_INIT do not need to include this hidden dependency.\n>\n> Signed-off-by: Derrick Stolee \n> ---\n> revision.h | 1 +\n> 1 file changed, 1 insertion(+)\n>\n> diff --git a/revision.h b/revision.h\n> index b36acfc2d9..18c9bbd822 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -4,6 +4,7 @@\n> #include \"commit.h\"\n> #include \"grep.h\"\n> #include \"notes.h\"\n> +#include \"object-name.h\"\n> #include \"oidset.h\"\n> #include \"pretty.h\"\n> #include \"diff.h\"\n\nOK. Other symbols REV_INFO_INIT needs are REV_SORT_IN_GRAPH_ORDER\n(in ), CMIT_FMT_DEFAULT (in ), and STRVEC_INIT\n(in ), and all three are already included there.\n\nMakes sense.\n"},{"id":"539263","messageId":"xmqqy0jqi08s.fsf@gitster.g","threadId":"65272","inReplyTo":"dc6652c84c8d37b124eb76c2a9cdfdc4db4a149d.1773707361.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/5] backfill: accept revision arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-17T22:01:39Z","receivedAt":"2026-03-17T22:01:42Z","isPatch":true,"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> -\trepo_init_revisions(ctx->repo, &revs, \"\");\n> -\thandle_revision_arg(\"HEAD\", &revs, 0, 0);\n\nSo we used to \"cheat\" and did an initialization without even knowing\nin which directory we were started ...\n\n> +\t/* Walk from HEAD if otherwise unspecified. */\n> +\tif (!ctx->revs.pending.nr)\n> +\t\thandle_revision_arg(\"HEAD\", &ctx->revs, 0, 0);\n\n... but by initializing the revs correctly in the caller, we would\nbe correcting it. Looking good.\n\n> @@ -134,7 +135,12 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n> \t\t\t\t\t builtin_backfill_usage, options);\n> \n> \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n> -\t\t\t 0);\n> +\t\t\t PARSE_OPT_KEEP_UNKNOWN_OPT |\n> +\t\t\t PARSE_OPT_KEEP_ARGV0 |\n> +\t\t\t PARSE_OPT_KEEP_DASHDASH);\n> +\n> +\trepo_init_revisions(repo, &ctx.revs, prefix);\n> +\targc = setup_revisions(argc, argv, &ctx.revs, NULL);\n\nOK.\n"},{"id":"539266","messageId":"xmqqse9yhzu8.fsf@gitster.g","threadId":"65272","inReplyTo":"977f62faa550ecc4ef7774b4ecf53a9a520fc62a.1773707361.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/5] backfill: work with prefix pathspecs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-17T22:10:23Z","receivedAt":"2026-03-17T22:10:25Z","isPatch":true,"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> From: Derrick Stolee \n>\n> The previous change allowed specifying revision arguments over the 'git\n> backfill' command-line. This created the opportunity for pathspecs that\n> specify a smaller set of starting commits, but otherwise did not restrict\n> the blob paths that were downloaded.\n\n\"pathspecs that specify a smaller set of starting commits\" is\npuzzling, as starting commits would be coming from the revision\narguments. \"opportunity for pathspec to further filter commits\nto those that touch only the matching paths...\", or something?\n\n> Update the path-walk API to accept certain kinds of pathspecs and to\n> silently ignore anything too complex.\n\nHmph, \"silently ignore\", instead of \"no, you cannot use that! and\ndie\", or at least \"sorry, I cannot do that, so the result may not be\nwhat you wanted, you've been warned\"?\n\n> The current behavior focuses on\n> pathspecs that match paths exactly. This includes exact filenames,\n> including directory names as prefixes. Pathspecs containing wildcards\n> or magic are cleared so the path walk downloads all blobs, as before.\n\nAh, \"we punt and lift the limitation to grab everything, so at least\neverything you wanted to have will become available to you, even\nthough we may download more than what you asked\"? OK, users would\nsurvive that, and as we improve the pathspec support, the user\nexperience would only improve. OK.\n\n"},{"id":"539268","messageId":"xmqqms06hzfv.fsf@gitster.g","threadId":"65272","inReplyTo":"beb1c92554c76907315a4d1a7983226d2bf5a828.1773707361.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 5/5] path-walk: support wildcard pathspecs for blob filtering","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-17T22:19:00Z","receivedAt":"2026-03-17T22:19:03Z","isPatch":true,"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> From: Derrick Stolee \n>\n> Previously, walk_objects_by_path() silently ignored pathspecs containing\n> wildcards or magic by clearing them. This caused all blobs to be\n> downloaded regardless of the given pathspec. Wildcard pathspecs like\n> \"d/file.*.txt\" are useful for narrowing which blobs to process (e.g.,\n> during 'git backfill').\n>\n> Support wildcard pathspecs by making three changes:\n>\n> 1. Add an 'exact_pathspecs' flag to path_walk_context. When the\n> pathspec has no wildcards or magic, set this flag and use the\n> existing fast-path prefix matching in add_tree_entries(). When\n> wildcards are present, skip that block since prefix matching\n> cannot handle glob patterns.\n>\n> 2. Disable revision-level commit pruning (revs->prune = 0) for\n> wildcard pathspecs. The revision walk uses the pathspec to filter\n> commits via TREESAME detection. For exact prefix pathspecs this\n> works well, but wildcard pathspecs may fail to match through\n> TREESAME because fnmatch with WM_PATHNAME does not cross directory\n> boundaries. Disabling pruning ensures all commits are visited and\n> their trees are available for the path-walk to filter.\n\nHmph, I wonder how significant an impact does it have on the\nperformance that we have to disable pruning here. With the bog\nstandard tree traversal, wouldn't tree_entry_interesting() already\nbe capable of doing this, even with fnmatch / WM_PATHNAME ?\n\n> 3. Add a match_pathspec() check in walk_path() to filter out blobs\n> whose full path does not match the pathspec. This provides the\n> actual blob-level filtering for wildcard pathspecs.\n>\n> Signed-off-by: Derrick Stolee \n> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>\n\nThe latter person cannot sign DCO or vouch for the origin of what\nthey have written in this patch, can they?\n\n> ---\n> path-walk.c | 22 ++++++++++++++--------\n> t/t5620-backfill.sh | 7 +++----\n> 2 files changed, 17 insertions(+), 12 deletions(-)\n"},{"id":"539287","messageId":"5122920d-078e-46c6-a494-c6e430df7e3f@gmail.com","threadId":"65272","inReplyTo":"xmqqse9yhzu8.fsf@gitster.g","subject":"Re: [PATCH 4/5] backfill: work with prefix pathspecs","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-18T13:15:00Z","receivedAt":"2026-03-18T13:15:02Z","isPatch":true,"body":"On 3/17/2026 6:10 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> From: Derrick Stolee \n>>\n>> The previous change allowed specifying revision arguments over the 'git\n>> backfill' command-line. This created the opportunity for pathspecs that\n>> specify a smaller set of starting commits, but otherwise did not restrict\n>> the blob paths that were downloaded.\n> \n> \"pathspecs that specify a smaller set of starting commits\" is\n> puzzling, as starting commits would be coming from the revision\n> arguments. \"opportunity for pathspec to further filter commits\n> to those that touch only the matching paths...\", or something?\n\nYou're right. I'm using \"starting commits\" incorrectly. My view was too\nfocused on how the path-walk API starts from the commits output by the\nrevision walk to get a list of root trees and then walks by path from\nthat point.\n\nI'll reword to make this more clear.\n\n>> Update the path-walk API to accept certain kinds of pathspecs and to\n>> silently ignore anything too complex.\n> \n> Hmph, \"silently ignore\", instead of \"no, you cannot use that! and\n> die\", or at least \"sorry, I cannot do that, so the result may not be\n> what you wanted, you've been warned\"?\n\nThe behavior when silently ignoring is to over-download. The revision\nwalk still filters commits, but the path-walk then walks paths beyond\nthat pathspec. This will be fixed in the next commit, so adding an\nerror case didn't seem worth it. I'll do a better job foreshadowing.\n\n>> The current behavior focuses on\n>> pathspecs that match paths exactly. This includes exact filenames,\n>> including directory names as prefixes. Pathspecs containing wildcards\n>> or magic are cleared so the path walk downloads all blobs, as before.\n> \n> Ah, \"we punt and lift the limitation to grab everything, so at least\n> everything you wanted to have will become available to you, even\n> though we may download more than what you asked\"? OK, users would\n> survive that, and as we improve the pathspec support, the user\n> experience would only improve. OK.\n\nExactly. I can word things better.\n\nThanks,\n-Stolee\n\n\n"},{"id":"539288","messageId":"443e5b25-46a1-4990-96b3-122e66d81828@gmail.com","threadId":"65272","inReplyTo":"xmqqms06hzfv.fsf@gitster.g","subject":"Re: [PATCH 5/5] path-walk: support wildcard pathspecs for blob filtering","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-18T13:16:14Z","receivedAt":"2026-03-18T13:16:16Z","isPatch":true,"body":"On 3/17/2026 6:19 PM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> From: Derrick Stolee \n>>\n>> Previously, walk_objects_by_path() silently ignored pathspecs containing\n>> wildcards or magic by clearing them. This caused all blobs to be\n>> downloaded regardless of the given pathspec. Wildcard pathspecs like\n>> \"d/file.*.txt\" are useful for narrowing which blobs to process (e.g.,\n>> during 'git backfill').\n>>\n>> Support wildcard pathspecs by making three changes:\n>>\n>> 1. Add an 'exact_pathspecs' flag to path_walk_context. When the\n>> pathspec has no wildcards or magic, set this flag and use the\n>> existing fast-path prefix matching in add_tree_entries(). When\n>> wildcards are present, skip that block since prefix matching\n>> cannot handle glob patterns.\n>>\n>> 2. Disable revision-level commit pruning (revs->prune = 0) for\n>> wildcard pathspecs. The revision walk uses the pathspec to filter\n>> commits via TREESAME detection. For exact prefix pathspecs this\n>> works well, but wildcard pathspecs may fail to match through\n>> TREESAME because fnmatch with WM_PATHNAME does not cross directory\n>> boundaries. Disabling pruning ensures all commits are visited and\n>> their trees are available for the path-walk to filter.\n> \n> Hmph, I wonder how significant an impact does it have on the\n> performance that we have to disable pruning here. With the bog\n> standard tree traversal, wouldn't tree_entry_interesting() already\n> be capable of doing this, even with fnmatch / WM_PATHNAME ?\n\nI will explore what's possible here and see what I can do.\n\n>> 3. Add a match_pathspec() check in walk_path() to filter out blobs\n>> whose full path does not match the pathspec. This provides the\n>> actual blob-level filtering for wildcard pathspecs.\n>>\n>> Signed-off-by: Derrick Stolee \n>> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>\n> \n> The latter person cannot sign DCO or vouch for the origin of what\n> they have written in this patch, can they?\nNo they cannot. Sorry for this error.\n\nThanks,\n-Stolee\n\n"},{"id":"539291","messageId":"b0a73540-bf56-4c6d-bc67-b94a824ddbf3@app.fastmail.com","threadId":"65272","inReplyTo":"dc6652c84c8d37b124eb76c2a9cdfdc4db4a149d.1773707361.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/5] backfill: accept revision arguments","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2026-03-18T15:37:13Z","receivedAt":"2026-03-18T15:37:35Z","isPatch":true,"body":"On Tue, Mar 17, 2026, at 01:29, Derrick Stolee via GitGitGadget wrote:\n>[snip]\n> diff --git a/Documentation/git-backfill.adoc b/Documentation/git-backfill.adoc\n> index b8394dcf22..fdfe22d623 100644\n> --- a/Documentation/git-backfill.adoc\n> +++ b/Documentation/git-backfill.adoc\n> @@ -63,9 +63,12 @@ OPTIONS\n> \tcurrent sparse-checkout. If the sparse-checkout feature is enabled,\n> \tthen `--sparse` is assumed and can be disabled with `--no-sparse`.\n>\n> +You may also specify the commit limiting options from linkgit:git-rev-list[1].\n> +\n> SEE ALSO\n> --------\n> linkgit:git-clone[1].\n> +linkgit:git-rev-list[1].\n\nShould there be a comma between these two?\n\n>[snip]\n"},{"id":"539374","messageId":"abvH29wYAFzk6V0F@pks.im","threadId":"65272","inReplyTo":"pull.2070.git.1773707361.gitgitgadget@gmail.com","subject":"Re: [PATCH 0/5] backfill: accept revision arguments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-19T09:54:35Z","receivedAt":"2026-03-19T09:54:46Z","isPatch":true,"body":"On Tue, Mar 17, 2026 at 12:29:16AM +0000, Derrick Stolee via GitGitGadget wrote:\n> The git backfill command assists in downloading missing blobs for blobless\n> partial clones. However, its current version lacks some valuable\n> functionality. It currently:\n> \n> 1. Only walks commits reachable from HEAD.\n> 2. It walks all reachable commits to the full history.\n> 3. It can focus on the current sparse-checkout definition, but otherwise it\n> doesn't focus on a given pathspec.\n> \n> All of these are being updated by this patch series, which allows rev-list\n> options to impact the path-walk. These include:\n> \n> 1. Specifying a given refspec, including --all.\n> 2. Modifying the commit walk, including --first-parent, commit ranges, or\n> recency using --since.\n> 3. Modifying the set of paths to download using pathspecs.\n> \n> One particularly valuable situation here is that now a user can run git\n> backfill -- to download all versions of a specific file or a specific\n> directory, accelerating history queries within that path without downloading\n> more than necessary. This can accelerate git blame or git log -L for these\n> paths, where normally those commands download missing blobs one-by-one\n> during its diff algorithms.\n\nNice.\n\nI think especially blaming is a bit of a sore spot -- downloading blobs\none by one simply doesn't cut it there. I wonder whether we can easily\nuse the backfill mechanism to fetch blobs automatically in git-blame(1)\nso that the user doesn't need to know about git-backfill(1) at all?\n\nPatrick\n"},{"id":"539375","messageId":"abvH6LUu953rrQzS@pks.im","threadId":"65272","inReplyTo":"dc6652c84c8d37b124eb76c2a9cdfdc4db4a149d.1773707361.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/5] backfill: accept revision arguments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-19T09:54:48Z","receivedAt":"2026-03-19T09:54:53Z","isPatch":true,"body":"On Tue, Mar 17, 2026 at 12:29:19AM +0000, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n> \n> The existing implementation of 'git backfill' only includes downloading\n> missing blobs reachable from HEAD. Advanced uses may desire more general\n> commit limiting options, such as '--all' for all references, specifying a\n> commit range via negative references, or specifying a recency of use such as\n> with '--since='.\n> \n> All of these options are available if we use setup_revisions() to parse the\n> unknown arguments with the revision machinery. This opens up a large number\n> of possibilities, only a small set of which are tested here.\n> \n> For documentation, we avoid duplicating the option documentation and instead\n> link to the documentation of 'git rev-list'.\n> \n> Note that these arguments currently allow specifying a pathspec, which\n> modifies the commit history checks but does not limit the paths used in the\n> backfill logic. This will be updated in a future change.\n\nMakes me wonder whether reversing the order would have avoided this\nslight awkwardness. But let's just stick with the current order, the end\nresult would be the same anyway.\n\n> Signed-off-by: Derrick Stolee \n> ---\n> Documentation/git-backfill.adoc | 3 +\n> builtin/backfill.c | 19 ++--\n> t/t5620-backfill.sh | 156 ++++++++++++++++++++++++++++++++\n> 3 files changed, 172 insertions(+), 6 deletions(-)\n> \n> diff --git a/builtin/backfill.c b/builtin/backfill.c\n> index e80fc1b694..1b5595b27c 100644\n> --- a/builtin/backfill.c\n> +++ b/builtin/backfill.c\n> @@ -92,13 +92,14 @@ static int do_backfill(struct backfill_context *ctx)\n> \t\t}\n> \t}\n> \n> -\trepo_init_revisions(ctx->repo, &revs, \"\");\n> -\thandle_revision_arg(\"HEAD\", &revs, 0, 0);\n> +\t/* Walk from HEAD if otherwise unspecified. */\n> +\tif (!ctx->revs.pending.nr)\n> +\t\thandle_revision_arg(\"HEAD\", &ctx->revs, 0, 0);\n\nCan we use `add_head_to_pending(&ctx->revs)` instead?\n\nPatrick\n"},{"id":"539376","messageId":"abvH8ttRkgu1R57C@pks.im","threadId":"65272","inReplyTo":"5122920d-078e-46c6-a494-c6e430df7e3f@gmail.com","subject":"Re: [PATCH 4/5] backfill: work with prefix pathspecs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-19T09:54:58Z","receivedAt":"2026-03-19T09:55:04Z","isPatch":true,"body":"On Wed, Mar 18, 2026 at 09:15:00AM -0400, Derrick Stolee wrote:\n> On 3/17/2026 6:10 PM, Junio C Hamano wrote:\n> >> Update the path-walk API to accept certain kinds of pathspecs and to\n> >> silently ignore anything too complex.\n> > \n> > Hmph, \"silently ignore\", instead of \"no, you cannot use that! and\n> > die\", or at least \"sorry, I cannot do that, so the result may not be\n> > what you wanted, you've been warned\"?\n> \n> The behavior when silently ignoring is to over-download. The revision\n> walk still filters commits, but the path-walk then walks paths beyond\n> that pathspec. This will be fixed in the next commit, so adding an\n> error case didn't seem worth it. I'll do a better job foreshadowing.\n\nI guess this is a fine tradeoff when documented properly. But I think in\nthat case we should make very clear that this behaviour may change in\nthe future if find a way to efficiently limit the pathwalk, too.\n\nPatrick\n"},{"id":"539377","messageId":"abvIADZ8kt11t2kP@pks.im","threadId":"65272","inReplyTo":"977f62faa550ecc4ef7774b4ecf53a9a520fc62a.1773707361.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/5] backfill: work with prefix pathspecs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-19T09:55:12Z","receivedAt":"2026-03-19T09:55:17Z","isPatch":true,"body":"On Tue, Mar 17, 2026 at 12:29:20AM +0000, Derrick Stolee via GitGitGadget wrote:\n> diff --git a/path-walk.c b/path-walk.c\n> index 364e4cfa19..e1ad4b0208 100644\n> --- a/path-walk.c\n> +++ b/path-walk.c\n> @@ -481,6 +524,18 @@ int walk_objects_by_path(struct path_walk_info *info)\n> \tif (info->tags)\n> \t\tinfo->revs->tag_objects = 1;\n> \n> +\tif (ctx.revs->prune_data.nr) {\n> +\t\t/*\n> +\t\t * Only exact prefix pathspecs are currently supported.\n> +\t\t * Clear any wildcard or magic pathspecs to avoid\n> +\t\t * incorrect prefix matching.\n> +\t\t */\n> +\t\tstruct pathspec *pd = &ctx.revs->prune_data;\n> +\n> +\t\tif (pd->has_wildcard || pd->magic)\n> +\t\t\tpd->nr = 0;\n> +\t}\n\nHuh, curious. Won't this cause a leak? I guess we should rather use\n`clear_pathspec()` here.\n\nAlso shows that this path is missing test coverage.\n\nPatrick\n"},{"id":"539379","messageId":"abvMwBr_03_Z0nMX@pks.im","threadId":"65272","inReplyTo":"977f62faa550ecc4ef7774b4ecf53a9a520fc62a.1773707361.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/5] backfill: work with prefix pathspecs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-19T10:15:28Z","receivedAt":"2026-03-19T10:15:34Z","isPatch":true,"body":"On Tue, Mar 17, 2026 at 12:29:20AM +0000, Derrick Stolee via GitGitGadget wrote:\n> diff --git a/path-walk.c b/path-walk.c\n> index 364e4cfa19..e1ad4b0208 100644\n> --- a/path-walk.c\n> +++ b/path-walk.c\n> @@ -206,6 +206,49 @@ static int add_tree_entries(struct path_walk_context *ctx,\n> \t\t\t\t match != MATCHED)\n> \t\t\t\tcontinue;\n> \t\t}\n> +\t\tif (ctx->revs->prune_data.nr) {\n> +\t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n> +\t\t\tbool found = false;\n> +\n> +\t\t\tfor (int i = 0; i < pd->nr; i++) {\n> +\t\t\t\tstruct pathspec_item *item = &pd->items[i];\n> +\n> +\t\t\t\t/*\n> +\t\t\t\t * Is this path a parent directory of\n> +\t\t\t\t * the pathspec item?\n> +\t\t\t\t */\n> +\t\t\t\tif (path.len < (size_t)item->len &&\n> +\t\t\t\t !strncmp(path.buf, item->match, path.len) &&\n> +\t\t\t\t item->match[path.len - 1] == '/') {\n> +\t\t\t\t\tfound = true;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\n> +\t\t\t\t/*\n> +\t\t\t\t * Or, is the pathspec an exact match?\n> +\t\t\t\t */\n> +\t\t\t\tif (path.len == (size_t)item->len &&\n> +\t\t\t\t !strcmp(path.buf, item->match)) {\n> +\t\t\t\t\tfound = true;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\n> +\t\t\t\t/*\n> +\t\t\t\t * Or, is the pathspec a directory prefix\n> +\t\t\t\t * match?\n> +\t\t\t\t */\n> +\t\t\t\tif (path.len > (size_t)item->len &&\n> +\t\t\t\t !strncmp(path.buf, item->match, item->len) &&\n> +\t\t\t\t path.buf[item->len] == '/') {\n> +\t\t\t\t\tfound = true;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n\nAh, one more thing: we could expose `dir_prefix()` from \"path.c\" and\nreuse it here.\n\nPatrick\n"},{"id":"539381","messageId":"29ec9e5f-ad14-4a87-92f9-a3ffa4077f69@gmail.com","threadId":"65272","inReplyTo":"abvH29wYAFzk6V0F@pks.im","subject":"Re: [PATCH 0/5] backfill: accept revision arguments","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-19T12:59:01Z","receivedAt":"2026-03-19T12:59:03Z","isPatch":true,"body":"On 3/19/2026 5:54 AM, Patrick Steinhardt wrote:\n> I think especially blaming is a bit of a sore spot -- downloading blobs\n> one by one simply doesn't cut it there. I wonder whether we can easily\n> use the backfill mechanism to fetch blobs automatically in git-blame(1)\n> so that the user doesn't need to know about git-backfill(1) at all?\n\nI've thought about this a bit, and I'm not sure that we want to run\n'git backfill' directly. Instead, it would be nice if we did a \"staged\"\nalgorithm for 'git blame':\n\n1. Walk commits according to the pathspec to collect the commits that\n changed the path.\n\n2. Collect the list of blob OIDs that will be needed for computing diffs\n for the line-tracking algorithm.\n\n3. In batches, download groups of missing blobs and then process them\n for line-tracking diffs. (Stop if all lines are blamed; continue to\n next batch if more lines are needed.)\n\nThis would be a significant rewrite of the blame algorithm, though. I\nbriefly considered this approach about a year ago and decided it would\nbe easier to start with 'git backfill' and see whether that satisfies\nmost needs.\n\nThe biggest reason to maybe avoid 'git backfill HEAD -- ' before\n_every_ blame operation is that this will add overhead on repeated\ncalls that may be obnoxious in its own way. Maybe doing an opt-in\n'git blame --backfill ' would make it easier for users to opt-in\nwhen they want to.\n\nThanks,\n-Stolee\n\n"},{"id":"539496","messageId":"abz4xiIxx4crr6al@pks.im","threadId":"65272","inReplyTo":"29ec9e5f-ad14-4a87-92f9-a3ffa4077f69@gmail.com","subject":"Re: [PATCH 0/5] backfill: accept revision arguments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-20T07:35:34Z","receivedAt":"2026-03-20T07:35:41Z","isPatch":true,"body":"On Thu, Mar 19, 2026 at 08:59:01AM -0400, Derrick Stolee wrote:\n> On 3/19/2026 5:54 AM, Patrick Steinhardt wrote:\n> > I think especially blaming is a bit of a sore spot -- downloading blobs\n> > one by one simply doesn't cut it there. I wonder whether we can easily\n> > use the backfill mechanism to fetch blobs automatically in git-blame(1)\n> > so that the user doesn't need to know about git-backfill(1) at all?\n> \n> I've thought about this a bit, and I'm not sure that we want to run\n> 'git backfill' directly. Instead, it would be nice if we did a \"staged\"\n> algorithm for 'git blame':\n> \n> 1. Walk commits according to the pathspec to collect the commits that\n> changed the path.\n> \n> 2. Collect the list of blob OIDs that will be needed for computing diffs\n> for the line-tracking algorithm.\n> \n> 3. In batches, download groups of missing blobs and then process them\n> for line-tracking diffs. (Stop if all lines are blamed; continue to\n> next batch if more lines are needed.)\n> \n> This would be a significant rewrite of the blame algorithm, though. I\n> briefly considered this approach about a year ago and decided it would\n> be easier to start with 'git backfill' and see whether that satisfies\n> most needs.\n> \n> The biggest reason to maybe avoid 'git backfill HEAD -- ' before\n> _every_ blame operation is that this will add overhead on repeated\n> calls that may be obnoxious in its own way. Maybe doing an opt-in\n> 'git blame --backfill ' would make it easier for users to opt-in\n> when they want to.\n\nThat's fair. I fully agree that just blindly doing this would be likely\nbe inefficient. Ideally, the batching logic would only kick in whenever\nwe see a missing object.\n\nAnyway, this definitely doesn't have to be part of this series, I was\nmostly wondering how hard it is to do.\n\nThanks!\n\nPatrick\n"},{"id":"539679","messageId":"b038752f-c950-4620-8d45-58741a5c9213@gmail.com","threadId":"65272","inReplyTo":"b0a73540-bf56-4c6d-bc67-b94a824ddbf3@app.fastmail.com","subject":"Re: [PATCH 3/5] backfill: accept revision arguments","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-23T00:31:42Z","receivedAt":"2026-03-23T00:31:45Z","isPatch":true,"body":"On 3/18/26 11:37 AM, Kristoffer Haugsbakk wrote:\n> On Tue, Mar 17, 2026, at 01:29, Derrick Stolee via GitGitGadget wrote:\n>> [snip]\n>> diff --git a/Documentation/git-backfill.adoc b/Documentation/git-backfill.adoc\n>> index b8394dcf22..fdfe22d623 100644\n>> --- a/Documentation/git-backfill.adoc\n>> +++ b/Documentation/git-backfill.adoc\n>> @@ -63,9 +63,12 @@ OPTIONS\n>> \tcurrent sparse-checkout. If the sparse-checkout feature is enabled,\n>> \tthen `--sparse` is assumed and can be disabled with `--no-sparse`.\n>>\n>> +You may also specify the commit limiting options from linkgit:git-rev-list[1].\n>> +\n>> SEE ALSO\n>> --------\n>> linkgit:git-clone[1].\n>> +linkgit:git-rev-list[1].\n> \n> Should there be a comma between these two?\n\nGood catch. Also there shouldn't be a hard stop, either.\n\nThanks,\n-Stolee\n\n"},{"id":"539680","messageId":"89d1cc54-f2db-4177-ba5a-4d20a2cbd9c2@gmail.com","threadId":"65272","inReplyTo":"abvH6LUu953rrQzS@pks.im","subject":"Re: [PATCH 3/5] backfill: accept revision arguments","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-23T00:35:09Z","receivedAt":"2026-03-23T00:35:12Z","isPatch":true,"body":"On 3/19/26 5:54 AM, Patrick Steinhardt wrote:\n> On Tue, Mar 17, 2026 at 12:29:19AM +0000, Derrick Stolee via GitGitGadget wrote:\n>> From: Derrick Stolee \n>>\n>> The existing implementation of 'git backfill' only includes downloading\n>> missing blobs reachable from HEAD. Advanced uses may desire more general\n>> commit limiting options, such as '--all' for all references, specifying a\n>> commit range via negative references, or specifying a recency of use such as\n>> with '--since='.\n>>\n>> All of these options are available if we use setup_revisions() to parse the\n>> unknown arguments with the revision machinery. This opens up a large number\n>> of possibilities, only a small set of which are tested here.\n>>\n>> For documentation, we avoid duplicating the option documentation and instead\n>> link to the documentation of 'git rev-list'.\n>>\n>> Note that these arguments currently allow specifying a pathspec, which\n>> modifies the commit history checks but does not limit the paths used in the\n>> backfill logic. This will be updated in a future change.\n> \n> Makes me wonder whether reversing the order would have avoided this\n> slight awkwardness. But let's just stick with the current order, the end\n> result would be the same anyway.\nTrue, we could have added the pathspec logic first, but we wouldn't be able\nto test it right away because the parsing comes through the rev-list.\n\n>> Signed-off-by: Derrick Stolee \n>> ---\n>> Documentation/git-backfill.adoc | 3 +\n>> builtin/backfill.c | 19 ++--\n>> t/t5620-backfill.sh | 156 ++++++++++++++++++++++++++++++++\n>> 3 files changed, 172 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/builtin/backfill.c b/builtin/backfill.c\n>> index e80fc1b694..1b5595b27c 100644\n>> --- a/builtin/backfill.c\n>> +++ b/builtin/backfill.c\n>> @@ -92,13 +92,14 @@ static int do_backfill(struct backfill_context *ctx)\n>> \t\t}\n>> \t}\n>> \n>> -\trepo_init_revisions(ctx->repo, &revs, \"\");\n>> -\thandle_revision_arg(\"HEAD\", &revs, 0, 0);\n>> +\t/* Walk from HEAD if otherwise unspecified. */\n>> +\tif (!ctx->revs.pending.nr)\n>> +\t\thandle_revision_arg(\"HEAD\", &ctx->revs, 0, 0);\n> \n> Can we use `add_head_to_pending(&ctx->revs)` instead?\n\nNice. We absolutely can and should.\n\nThanks,\n-Stolee\n\n\n"},{"id":"539681","messageId":"90210abe-8e98-469a-aa15-fe379d2eb3e6@gmail.com","threadId":"65272","inReplyTo":"abvMwBr_03_Z0nMX@pks.im","subject":"Re: [PATCH 4/5] backfill: work with prefix pathspecs","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-23T00:47:42Z","receivedAt":"2026-03-23T00:47:44Z","isPatch":true,"body":"On 3/19/26 6:15 AM, Patrick Steinhardt wrote:\n> Ah, one more thing: we could expose `dir_prefix()` from \"path.c\" and\n> reuse it here.\n\nGood idea. This becomes\n\n\t\t\t/*\n\t\t\t * Continue if either is a directory prefix\n\t\t\t * of the other.\n\t\t\t */\n\t\t\tif (dir_prefix(path.buf, item->match) ||\n\t\t\t dir_prefix(item->match, path.buf)) {\n\t\t\t\tfound = true;\n\t\t\t\tbreak;\n\t\t\t}\n\nWith the idea that we need to walk the parents of each prefix in\naddition to walking all of their children.\n\nThanks,\n-Stolee\n\n\n"},{"id":"539683","messageId":"bf0cc84b-b4ea-4100-87b6-d29777155037@gmail.com","threadId":"65272","inReplyTo":"443e5b25-46a1-4990-96b3-122e66d81828@gmail.com","subject":"Re: [PATCH 5/5] path-walk: support wildcard pathspecs for blob filtering","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-23T01:33:20Z","receivedAt":"2026-03-23T01:33:22Z","isPatch":true,"body":"On 3/18/26 9:16 AM, Derrick Stolee wrote:\n> On 3/17/2026 6:19 PM, Junio C Hamano wrote:\n>> \"Derrick Stolee via GitGitGadget\" writes:\n>>\n>>> From: Derrick Stolee \n>>>\n>>> Previously, walk_objects_by_path() silently ignored pathspecs containing\n>>> wildcards or magic by clearing them. This caused all blobs to be\n>>> downloaded regardless of the given pathspec. Wildcard pathspecs like\n>>> \"d/file.*.txt\" are useful for narrowing which blobs to process (e.g.,\n>>> during 'git backfill').\n>>>\n>>> Support wildcard pathspecs by making three changes:\n>>>\n>>> 1. Add an 'exact_pathspecs' flag to path_walk_context. When the\n>>> pathspec has no wildcards or magic, set this flag and use the\n>>> existing fast-path prefix matching in add_tree_entries(). When\n>>> wildcards are present, skip that block since prefix matching\n>>> cannot handle glob patterns.\n>>>\n>>> 2. Disable revision-level commit pruning (revs->prune = 0) for\n>>> wildcard pathspecs. The revision walk uses the pathspec to filter\n>>> commits via TREESAME detection. For exact prefix pathspecs this\n>>> works well, but wildcard pathspecs may fail to match through\n>>> TREESAME because fnmatch with WM_PATHNAME does not cross directory\n>>> boundaries. Disabling pruning ensures all commits are visited and\n>>> their trees are available for the path-walk to filter.\n>>\n>> Hmph, I wonder how significant an impact does it have on the\n>> performance that we have to disable pruning here. With the bog\n>> standard tree traversal, wouldn't tree_entry_interesting() already\n>> be capable of doing this, even with fnmatch / WM_PATHNAME ?\n> \n> I will explore what's possible here and see what I can do.\n\nI must have needed the 'revs->prune = 0' at some point during development\nand left it even though it isn't actually necessary. Leaving it\nimplicitly at '1' should indeed be faster due to traversing fewer commits\nand parsing fewer trees while still reaching all necessary blobs.\n\nOnly changes 1 and 3 are necessary.\n\nThanks,\n-Stolee\n\n"},{"id":"539723","messageId":"pull.2070.v2.git.1774266019.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.git.1773707361.gitgitgadget@gmail.com","subject":"[PATCH v2 0/6] backfill: accept revision arguments","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-23T11:40:13Z","receivedAt":"2026-03-23T11:40:23Z","isPatch":true,"body":"The git backfill command assists in downloading missing blobs for blobless\npartial clones. However, its current version lacks some valuable\nfunctionality. It currently:\n\n 1. Only walks commits reachable from HEAD.\n 2. It walks all reachable commits to the full history.\n 3. It can focus on the current sparse-checkout definition, but otherwise it\n doesn't focus on a given pathspec.\n\nAll of these are being updated by this patch series, which allows rev-list\noptions to impact the path-walk. These include:\n\n 1. Specifying a given refspec, including --all.\n 2. Modifying the commit walk, including --first-parent, commit ranges, or\n recency using --since.\n 3. Modifying the set of paths to download using pathspecs.\n\nOne particularly valuable situation here is that now a user can run git\nbackfill -- to download all versions of a specific file or a specific\ndirectory, accelerating history queries within that path without downloading\nmore than necessary. This can accelerate git blame or git log -L for these\npaths, where normally those commands download missing blobs one-by-one\nduring its diff algorithms.\n\nThis patch series is organized in the following way:\n\n 1. A missing #include is added to prevent future compilation issues.\n 2. The test repo in t5620 is expanded to make later tests more interesting.\n 3. The backfill builtin parses the rev-list arguments. We test the top\n arguments that work as expected, though the pathspec arguments need\n extra work.\n 4. Update the path-walk logic to work efficiently with some pathspecs, such\n as fixed prefix pathspecs, accelerating the computation.\n 5. For more complicated pathspecs, do a post-filter in builtin/backfill.c\n instead of restricting the walk in the path-walk API.\n\nThe main goal of this series is to make such customizations possible, and to\nimprove performance where common use cases are expected. I'm open to\nfeedback as to whether we should consider more detailed performance analysis\nor whether we should wait for how users interact with these new options\nbefore overoptimizing unlikely use cases.\n\n\nUpdates in v2\n=============\n\n * Hard stops are replaced with a comma (and no punctuation) in the docs.\n * add_head_to_pending() simplifies some code.\n * My poor explanation of \"starting commits\" is updated.\n * Language around temporary prefix restriction is clarified.\n * Prefix match logic is simplified with dir_prefix().\n * Temporary memory leak (introduced in v1's patch 4 and removed in v1's\n patch 5) is removed in v2's patch 4.\n * Commit pruning is reenabled in v2's patch 5. There was no need for that\n with the way the logic works in the patch.\n * Add a new patch with a test demonstrating the new behavior that was being\n discussed in [1].\n\n[1]\nhttps://lore.kernel.org/git/20260321031643.5185-1-r.siddharth.shrimali@gmail.com/\n\nThanks, -Stolee\n\nDerrick Stolee (6):\n revision: include object-name.h\n t5620: prepare branched repo for revision tests\n backfill: accept revision arguments\n backfill: work with prefix pathspecs\n path-walk: support wildcard pathspecs for blob filtering\n t5620: test backfill's unknown argument handling\n\n Documentation/git-backfill.adoc | 5 +-\n builtin/backfill.c | 19 ++-\n path-walk.c | 44 +++++++\n path.c | 2 +-\n path.h | 6 +\n revision.h | 1 +\n t/t5620-backfill.sh | 211 +++++++++++++++++++++++++++++++-\n 7 files changed, 278 insertions(+), 10 deletions(-)\n\n\nbase-commit: 67ad42147a7acc2af6074753ebd03d904476118f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2070%2Fderrickstolee%2Fbackfill-revs-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2070/derrickstolee/backfill-revs-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/2070\n\nRange-diff vs v1:\n\n 1: fda0239103 = 1: fda0239103 revision: include object-name.h\n 2: 55a45b2fc8 = 2: 55a45b2fc8 t5620: prepare branched repo for revision tests\n 3: dc6652c84c ! 3: 610a162973 backfill: accept revision arguments\n @@ Documentation/git-backfill.adoc: OPTIONS\n +\n SEE ALSO\n --------\n - linkgit:git-clone[1].\n -+linkgit:git-rev-list[1].\n +-linkgit:git-clone[1].\n ++linkgit:git-clone[1],\n ++linkgit:git-rev-list[1]\n \n GIT\n ---\n @@ builtin/backfill.c: static int do_backfill(struct backfill_context *ctx)\n -\thandle_revision_arg(\"HEAD\", &revs, 0, 0);\n +\t/* Walk from HEAD if otherwise unspecified. */\n +\tif (!ctx->revs.pending.nr)\n -+\t\thandle_revision_arg(\"HEAD\", &ctx->revs, 0, 0);\n ++\t\tadd_head_to_pending(&ctx->revs);\n \n \tinfo.blobs = 1;\n \tinfo.tags = info.commits = info.trees = 0;\n 4: 977f62faa5 ! 4: f8f2c61326 backfill: work with prefix pathspecs\n @@ Commit message\n backfill: work with prefix pathspecs\n \n The previous change allowed specifying revision arguments over the 'git\n - backfill' command-line. This created the opportunity for pathspecs that\n - specify a smaller set of starting commits, but otherwise did not restrict\n - the blob paths that were downloaded.\n + backfill' command-line. This created the opportunity for restricting the\n + initial commit set by filtering the revision walk through a pathspec. Other\n + than filtering the commit set (and thereby the root trees), this did not\n + restrict the path-walk implementation of 'git backfill' and did not restrict\n + the blobs that were downloaded to only those matching the pathspec.\n \n Update the path-walk API to accept certain kinds of pathspecs and to\n - silently ignore anything too complex. The current behavior focuses on\n - pathspecs that match paths exactly. This includes exact filenames,\n - including directory names as prefixes. Pathspecs containing wildcards\n - or magic are cleared so the path walk downloads all blobs, as before.\n + silently ignore anything too complex, for now. We will update this in the\n + next change to properly restrict to even complex pathspecs.\n +\n + The current behavior focuses on pathspecs that match paths exactly. This\n + includes exact filenames, including directory names as prefixes. Pathspecs\n + containing wildcards or magic are cleared so the path walk downloads all\n + blobs, as before.\n \n The reason for this restriction is to allow for a faster execution by\n pruning the path walk to only trees that could contribute towards one of\n @@ Commit message\n Signed-off-by: Derrick Stolee \n \n ## path-walk.c ##\n +@@\n + #include \"list-objects.h\"\n + #include \"object.h\"\n + #include \"oid-array.h\"\n ++#include \"path.h\"\n + #include \"prio-queue.h\"\n + #include \"repository.h\"\n + #include \"revision.h\"\n @@ path-walk.c: static int add_tree_entries(struct path_walk_context *ctx,\n \t\t\t\t match != MATCHED)\n \t\t\t\tcontinue;\n @@ path-walk.c: static int add_tree_entries(struct path_walk_context *ctx,\n +\t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n +\t\t\tbool found = false;\n +\n ++\t\t\t/* remove '/' for these checks. */\n ++\t\t\tpath.buf[path.len - 1] = 0;\n ++\n +\t\t\tfor (int i = 0; i < pd->nr; i++) {\n +\t\t\t\tstruct pathspec_item *item = &pd->items[i];\n +\n +\t\t\t\t/*\n -+\t\t\t\t * Is this path a parent directory of\n -+\t\t\t\t * the pathspec item?\n -+\t\t\t\t */\n -+\t\t\t\tif (path.len < (size_t)item->len &&\n -+\t\t\t\t !strncmp(path.buf, item->match, path.len) &&\n -+\t\t\t\t item->match[path.len - 1] == '/') {\n -+\t\t\t\t\tfound = true;\n -+\t\t\t\t\tbreak;\n -+\t\t\t\t}\n -+\n -+\t\t\t\t/*\n -+\t\t\t\t * Or, is the pathspec an exact match?\n -+\t\t\t\t */\n -+\t\t\t\tif (path.len == (size_t)item->len &&\n -+\t\t\t\t !strcmp(path.buf, item->match)) {\n -+\t\t\t\t\tfound = true;\n -+\t\t\t\t\tbreak;\n -+\t\t\t\t}\n -+\n -+\t\t\t\t/*\n -+\t\t\t\t * Or, is the pathspec a directory prefix\n -+\t\t\t\t * match?\n ++\t\t\t\t * Continue if either is a directory prefix\n ++\t\t\t\t * of the other.\n +\t\t\t\t */\n -+\t\t\t\tif (path.len > (size_t)item->len &&\n -+\t\t\t\t !strncmp(path.buf, item->match, item->len) &&\n -+\t\t\t\t path.buf[item->len] == '/') {\n ++\t\t\t\tif (dir_prefix(path.buf, item->match) ||\n ++\t\t\t\t dir_prefix(item->match, path.buf)) {\n +\t\t\t\t\tfound = true;\n +\t\t\t\t\tbreak;\n +\t\t\t\t}\n +\t\t\t}\n +\n ++\t\t\t/* return '/' after these checks. */\n ++\t\t\tpath.buf[path.len - 1] = '/';\n ++\n +\t\t\t/* Skip paths that do not match the prefix. */\n +\t\t\tif (!found)\n +\t\t\t\tcontinue;\n @@ path-walk.c: int walk_objects_by_path(struct path_walk_info *info)\n +\t\t * Clear any wildcard or magic pathspecs to avoid\n +\t\t * incorrect prefix matching.\n +\t\t */\n -+\t\tstruct pathspec *pd = &ctx.revs->prune_data;\n -+\n -+\t\tif (pd->has_wildcard || pd->magic)\n -+\t\t\tpd->nr = 0;\n ++\t\tif (ctx.revs->prune_data.has_wildcard ||\n ++\t\t ctx.revs->prune_data.magic)\n ++\t\t\tclear_pathspec(&ctx.revs->prune_data);\n +\t}\n +\n \t/* Insert a single list for the root tree into the paths. */\n \tCALLOC_ARRAY(root_tree_list, 1);\n \troot_tree_list->type = OBJ_TREE;\n \n + ## path.c ##\n +@@ path.c: static void strbuf_cleanup_path(struct strbuf *sb)\n + \t\tstrbuf_remove(sb, 0, path - sb->buf);\n + }\n + \n +-static int dir_prefix(const char *buf, const char *dir)\n ++int dir_prefix(const char *buf, const char *dir)\n + {\n + \tint len = strlen(dir);\n + \treturn !strncmp(buf, dir, len) &&\n +\n + ## path.h ##\n +@@ path.h: const char *repo_submodule_path_replace(struct repository *repo,\n + \t\t\t\t\tconst char *fmt, ...)\n + \t__attribute__((format (printf, 4, 5)));\n + \n ++/*\n ++ * Given a directory name 'dir' (not ending with a trailing '/'),\n ++ * determine if 'buf' is equal to 'dir' or has prefix 'dir'+'/'.\n ++ */\n ++int dir_prefix(const char *buf, const char *dir);\n ++\n + void report_linked_checkout_garbage(struct repository *r);\n + \n + /*\n +\n ## t/t5620-backfill.sh ##\n @@ t/t5620-backfill.sh: test_expect_success 'backfill with prefix pathspec' '\n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n 5: beb1c92554 ! 5: 1168edfb96 path-walk: support wildcard pathspecs for blob filtering\n @@ Commit message\n \"d/file.*.txt\" are useful for narrowing which blobs to process (e.g.,\n during 'git backfill').\n \n - Support wildcard pathspecs by making three changes:\n + Support wildcard pathspecs by making two changes:\n \n 1. Add an 'exact_pathspecs' flag to path_walk_context. When the\n pathspec has no wildcards or magic, set this flag and use the\n @@ Commit message\n wildcards are present, skip that block since prefix matching\n cannot handle glob patterns.\n \n - 2. Disable revision-level commit pruning (revs->prune = 0) for\n - wildcard pathspecs. The revision walk uses the pathspec to filter\n - commits via TREESAME detection. For exact prefix pathspecs this\n - works well, but wildcard pathspecs may fail to match through\n - TREESAME because fnmatch with WM_PATHNAME does not cross directory\n - boundaries. Disabling pruning ensures all commits are visited and\n - their trees are available for the path-walk to filter.\n -\n - 3. Add a match_pathspec() check in walk_path() to filter out blobs\n + 2. Add a match_pathspec() check in walk_path() to filter out blobs\n whose full path does not match the pathspec. This provides the\n actual blob-level filtering for wildcard pathspecs.\n \n Signed-off-by: Derrick Stolee \n - Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>\n \n ## path-walk.c ##\n @@ path-walk.c: struct path_walk_context {\n @@ path-walk.c: int walk_objects_by_path(struct path_walk_info *info)\n -\t\t * Clear any wildcard or magic pathspecs to avoid\n -\t\t * incorrect prefix matching.\n -\t\t */\n - \t\tstruct pathspec *pd = &ctx.revs->prune_data;\n - \n --\t\tif (pd->has_wildcard || pd->magic)\n --\t\t\tpd->nr = 0;\n -+\t\tif (!pd->has_wildcard && !pd->magic)\n +-\t\tif (ctx.revs->prune_data.has_wildcard ||\n +-\t\t ctx.revs->prune_data.magic)\n +-\t\t\tclear_pathspec(&ctx.revs->prune_data);\n ++\t\tif (!ctx.revs->prune_data.has_wildcard &&\n ++\t\t !ctx.revs->prune_data.magic)\n +\t\t\tctx.exact_pathspecs = 1;\n -+\t\telse\n -+\t\t\tctx.revs->prune = 0;\n \t}\n \n \t/* Insert a single list for the root tree into the paths. */\n -: ---------- > 6: 9699650aa7 t5620: test backfill's unknown argument handling\n\n-- \ngitgitgadget\n"},{"id":"539724","messageId":"fda0239103f6e2b9e76403144b9ed2e9205e1c2a.1774266019.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v2.git.1774266019.gitgitgadget@gmail.com","subject":"[PATCH v2 1/6] revision: include object-name.h","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-23T11:40:14Z","receivedAt":"2026-03-23T11:40:24Z","isPatch":true,"body":"From: Derrick Stolee \n\nThe REV_INFO_INIT macro includes a use of the DEFAULT_ABBREV macro, which is\ndefined in object-name.h. Include it in revision.h so consumers of\nREV_INFO_INIT do not need to include this hidden dependency.\n\nSigned-off-by: Derrick Stolee \n---\n revision.h | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/revision.h b/revision.h\nindex b36acfc2d9..18c9bbd822 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -4,6 +4,7 @@\n #include \"commit.h\"\n #include \"grep.h\"\n #include \"notes.h\"\n+#include \"object-name.h\"\n #include \"oidset.h\"\n #include \"pretty.h\"\n #include \"diff.h\"\n-- \ngitgitgadget\n\n"},{"id":"539725","messageId":"55a45b2fc81895d2b712f126f716a8152d73fc93.1774266019.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v2.git.1774266019.gitgitgadget@gmail.com","subject":"[PATCH v2 2/6] t5620: prepare branched repo for revision tests","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-23T11:40:15Z","receivedAt":"2026-03-23T11:40:26Z","isPatch":true,"body":"From: Derrick Stolee \n\nPrepare the test infrastructure for upcoming changes that teach 'git\nbackfill' to accept revision arguments and pathspecs.\n\nAdd test_tick before each commit in the setup loop so that commit dates\nare deterministic. This enables reliable testing with '--since'.\n\nRename the 'd/e/' directory to 'd/f/' so that the prefix 'd/f' is\nambiguous with the files 'd/file.*.txt'. This exercises the subtlety\nin prefix pathspec matching that will be added in a later commit.\n\nCreate a branched version of the test repository (src-revs) with:\n - A 'side' branch merged into main, adding s/file.{1,2}.txt with\n two versions (4 new blobs, 52 total from main HEAD).\n - An unmerged 'other' branch adding o/file.{1,2}.txt (2 more blobs,\n 54 total reachable from --all).\n\nThis structure makes --all, --first-parent, and --since produce\nmeaningfully different results when used with 'git backfill'.\n\nSigned-off-by: Derrick Stolee \n---\n t/t5620-backfill.sh | 52 +++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 50 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 58c81556e7..1331949be4 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -15,7 +15,7 @@ test_expect_success 'setup repo for object creation' '\n \tgit init src &&\n \n \tmkdir -p src/a/b/c &&\n-\tmkdir -p src/d/e &&\n+\tmkdir -p src/d/f &&\n \n \tfor i in 1 2\n \tdo\n@@ -26,8 +26,9 @@ test_expect_success 'setup repo for object creation' '\n \t\t\techo \"Version $i of file a/b/$n\" > src/a/b/file.$n.txt &&\n \t\t\techo \"Version $i of file a/b/c/$n\" > src/a/b/c/file.$n.txt &&\n \t\t\techo \"Version $i of file d/$n\" > src/d/file.$n.txt &&\n-\t\t\techo \"Version $i of file d/e/$n\" > src/d/e/file.$n.txt &&\n+\t\t\techo \"Version $i of file d/f/$n\" > src/d/f/file.$n.txt &&\n \t\t\tgit -C src add . &&\n+\t\t\ttest_tick &&\n \t\t\tgit -C src commit -m \"Iteration $n\" || return 1\n \t\tdone\n \tdone\n@@ -41,6 +42,53 @@ test_expect_success 'setup bare clone for server' '\n \tgit -C srv.bare config --local uploadpack.allowanysha1inwant 1\n '\n \n+# Create a version of the repo with branches for testing revision\n+# arguments like --all, --first-parent, and --since.\n+#\n+# main: 8 commits (linear) + merge of side branch\n+# 48 original blobs + 4 side blobs = 52 blobs from main HEAD\n+# side: 2 commits adding s/file.{1,2}.txt (v1, v2), merged into main\n+# other: 1 commit adding o/file.{1,2}.txt (not merged)\n+# 54 total blobs reachable from --all\n+test_expect_success 'setup branched repo for revision tests' '\n+\tgit clone src src-revs &&\n+\n+\t# Side branch from tip of main with unique files\n+\tgit -C src-revs checkout -b side HEAD &&\n+\tmkdir -p src-revs/s &&\n+\techo \"Side version 1 of file 1\" >src-revs/s/file.1.txt &&\n+\techo \"Side version 1 of file 2\" >src-revs/s/file.2.txt &&\n+\ttest_tick &&\n+\tgit -C src-revs add . &&\n+\tgit -C src-revs commit -m \"Side commit 1\" &&\n+\n+\techo \"Side version 2 of file 1\" >src-revs/s/file.1.txt &&\n+\techo \"Side version 2 of file 2\" >src-revs/s/file.2.txt &&\n+\ttest_tick &&\n+\tgit -C src-revs add . &&\n+\tgit -C src-revs commit -m \"Side commit 2\" &&\n+\n+\t# Merge side into main\n+\tgit -C src-revs checkout main &&\n+\ttest_tick &&\n+\tgit -C src-revs merge side --no-ff -m \"Merge side branch\" &&\n+\n+\t# Other branch (not merged) for --all testing\n+\tgit -C src-revs checkout -b other main~1 &&\n+\tmkdir -p src-revs/o &&\n+\techo \"Other content 1\" >src-revs/o/file.1.txt &&\n+\techo \"Other content 2\" >src-revs/o/file.2.txt &&\n+\ttest_tick &&\n+\tgit -C src-revs add . &&\n+\tgit -C src-revs commit -m \"Other commit\" &&\n+\n+\tgit -C src-revs checkout main &&\n+\n+\tgit clone --bare \"file://$(pwd)/src-revs\" srv-revs.bare &&\n+\tgit -C srv-revs.bare config --local uploadpack.allowfilter 1 &&\n+\tgit -C srv-revs.bare config --local uploadpack.allowanysha1inwant 1\n+'\n+\n # do basic partial clone from \"srv.bare\"\n test_expect_success 'do partial clone 1, backfill gets all objects' '\n \tgit clone --no-checkout --filter=blob:none\t\\\n-- \ngitgitgadget\n\n"},{"id":"539726","messageId":"610a162973a7ad59eba4ef4d5a9288f1fea1d2e8.1774266019.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v2.git.1774266019.gitgitgadget@gmail.com","subject":"[PATCH v2 3/6] backfill: accept revision arguments","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-23T11:40:16Z","receivedAt":"2026-03-23T11:40:27Z","isPatch":true,"body":"From: Derrick Stolee \n\nThe existing implementation of 'git backfill' only includes downloading\nmissing blobs reachable from HEAD. Advanced uses may desire more general\ncommit limiting options, such as '--all' for all references, specifying a\ncommit range via negative references, or specifying a recency of use such as\nwith '--since='.\n\nAll of these options are available if we use setup_revisions() to parse the\nunknown arguments with the revision machinery. This opens up a large number\nof possibilities, only a small set of which are tested here.\n\nFor documentation, we avoid duplicating the option documentation and instead\nlink to the documentation of 'git rev-list'.\n\nNote that these arguments currently allow specifying a pathspec, which\nmodifies the commit history checks but does not limit the paths used in the\nbackfill logic. This will be updated in a future change.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-backfill.adoc | 5 +-\n builtin/backfill.c | 19 ++--\n t/t5620-backfill.sh | 156 ++++++++++++++++++++++++++++++++\n 3 files changed, 173 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-backfill.adoc b/Documentation/git-backfill.adoc\nindex b8394dcf22..246ab417c2 100644\n--- a/Documentation/git-backfill.adoc\n+++ b/Documentation/git-backfill.adoc\n@@ -63,9 +63,12 @@ OPTIONS\n \tcurrent sparse-checkout. If the sparse-checkout feature is enabled,\n \tthen `--sparse` is assumed and can be disabled with `--no-sparse`.\n \n+You may also specify the commit limiting options from linkgit:git-rev-list[1].\n+\n SEE ALSO\n --------\n-linkgit:git-clone[1].\n+linkgit:git-clone[1],\n+linkgit:git-rev-list[1]\n \n GIT\n ---\ndiff --git a/builtin/backfill.c b/builtin/backfill.c\nindex e80fc1b694..90c9d84793 100644\n--- a/builtin/backfill.c\n+++ b/builtin/backfill.c\n@@ -35,6 +35,7 @@ struct backfill_context {\n \tstruct oid_array current_batch;\n \tsize_t min_batch_size;\n \tint sparse;\n+\tstruct rev_info revs;\n };\n \n static void backfill_context_clear(struct backfill_context *ctx)\n@@ -80,7 +81,6 @@ static int fill_missing_blobs(const char *path UNUSED,\n \n static int do_backfill(struct backfill_context *ctx)\n {\n-\tstruct rev_info revs;\n \tstruct path_walk_info info = PATH_WALK_INFO_INIT;\n \tint ret;\n \n@@ -92,13 +92,14 @@ static int do_backfill(struct backfill_context *ctx)\n \t\t}\n \t}\n \n-\trepo_init_revisions(ctx->repo, &revs, \"\");\n-\thandle_revision_arg(\"HEAD\", &revs, 0, 0);\n+\t/* Walk from HEAD if otherwise unspecified. */\n+\tif (!ctx->revs.pending.nr)\n+\t\tadd_head_to_pending(&ctx->revs);\n \n \tinfo.blobs = 1;\n \tinfo.tags = info.commits = info.trees = 0;\n \n-\tinfo.revs = &revs;\n+\tinfo.revs = &ctx->revs;\n \tinfo.path_fn = fill_missing_blobs;\n \tinfo.path_fn_data = ctx;\n \n@@ -109,7 +110,6 @@ static int do_backfill(struct backfill_context *ctx)\n \t\tdownload_batch(ctx);\n \n \tpath_walk_info_clear(&info);\n-\trelease_revisions(&revs);\n \treturn ret;\n }\n \n@@ -121,6 +121,7 @@ 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.revs = REV_INFO_INIT,\n \t};\n \tstruct option options[] = {\n \t\tOPT_UNSIGNED(0, \"min-batch-size\", &ctx.min_batch_size,\n@@ -134,7 +135,12 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \t\t\t\t\t builtin_backfill_usage, options);\n \n \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n-\t\t\t 0);\n+\t\t\t PARSE_OPT_KEEP_UNKNOWN_OPT |\n+\t\t\t PARSE_OPT_KEEP_ARGV0 |\n+\t\t\t PARSE_OPT_KEEP_DASHDASH);\n+\n+\trepo_init_revisions(repo, &ctx.revs, prefix);\n+\targc = setup_revisions(argc, argv, &ctx.revs, NULL);\n \n \trepo_config(repo, git_default_config, NULL);\n \n@@ -143,5 +149,6 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \n \tresult = do_backfill(&ctx);\n \tbackfill_context_clear(&ctx);\n+\trelease_revisions(&ctx.revs);\n \treturn result;\n }\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 1331949be4..db66d8b614 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -224,6 +224,162 @@ test_expect_success 'backfill --sparse without cone mode (negative)' '\n \ttest_line_count = 12 missing\n '\n \n+test_expect_success 'backfill with revision range' '\n+\ttest_when_finished rm -rf backfill-revs &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-revs &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-revs rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\tgit -C backfill-revs backfill HEAD~2..HEAD &&\n+\n+\t# 30 objects downloaded.\n+\tgit -C backfill-revs rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 18 missing\n+'\n+\n+test_expect_success 'backfill with revisions over stdin' '\n+\ttest_when_finished rm -rf backfill-revs &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-revs &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-revs rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\tcat >in <<-EOF &&\n+\tHEAD\n+\t^HEAD~2\n+\tEOF\n+\n+\tgit -C backfill-revs backfill --stdin missing &&\n+\ttest_line_count = 18 missing\n+'\n+\n+test_expect_success 'backfill with prefix pathspec' '\n+\ttest_when_finished rm -rf backfill-path &&\n+\tgit clone --bare --filter=blob:none\t\t \\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-path &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\t# TODO: The pathspec should limit the downloaded blobs to\n+\t# only those matching the prefix \"d/f\", but currently all\n+\t# blobs are downloaded.\n+\tgit -C backfill-path backfill HEAD -- d/f &&\n+\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with multiple pathspecs' '\n+\ttest_when_finished rm -rf backfill-path &&\n+\tgit clone --bare --filter=blob:none\t\t \\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-path &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\t# TODO: The pathspecs should limit the downloaded blobs to\n+\t# only those matching \"d/f\" or \"a\", but currently all blobs\n+\t# are downloaded.\n+\tgit -C backfill-path backfill HEAD -- d/f a &&\n+\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with wildcard pathspec' '\n+\ttest_when_finished rm -rf backfill-path &&\n+\tgit clone --bare --filter=blob:none\t\t \\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-path &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\t# TODO: The wildcard pathspec should limit downloaded blobs,\n+\t# but currently all blobs are downloaded.\n+\tgit -C backfill-path backfill HEAD -- \"d/file.*.txt\" &&\n+\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with --all' '\n+\ttest_when_finished rm -rf backfill-all &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t\"file://$(pwd)/srv-revs.bare\" backfill-all &&\n+\n+\t# All blobs from all refs are missing\n+\tgit -C backfill-all rev-list --quiet --objects --all --missing=print >missing &&\n+\ttest_line_count = 54 missing &&\n+\n+\t# Backfill from HEAD gets main blobs only\n+\tgit -C backfill-all backfill HEAD &&\n+\n+\t# Other branch blobs still missing\n+\tgit -C backfill-all rev-list --quiet --objects --all --missing=print >missing &&\n+\ttest_line_count = 2 missing &&\n+\n+\t# Backfill with --all gets everything\n+\tgit -C backfill-all backfill --all &&\n+\n+\tgit -C backfill-all rev-list --quiet --objects --all --missing=print >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with --first-parent' '\n+\ttest_when_finished rm -rf backfill-fp &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main\t\t\t\\\n+\t\t\"file://$(pwd)/srv-revs.bare\" backfill-fp &&\n+\n+\tgit -C backfill-fp rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 52 missing &&\n+\n+\t# --first-parent skips the side branch commits, so\n+\t# s/file.{1,2}.txt v1 blobs (only in side commit 1) are missed.\n+\tgit -C backfill-fp backfill --first-parent HEAD &&\n+\n+\tgit -C backfill-fp rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 2 missing\n+'\n+\n+test_expect_success 'backfill with --since' '\n+\ttest_when_finished rm -rf backfill-since &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main\t\t\t\\\n+\t\t\"file://$(pwd)/srv-revs.bare\" backfill-since &&\n+\n+\tgit -C backfill-since rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 52 missing &&\n+\n+\t# Use a cutoff between commits 4 and 5 (between v1 and v2\n+\t# iterations). Commits 5-8 still carry v1 of files 2-4 in\n+\t# their trees, but v1 of file.1.txt is only in commits 1-4.\n+\tSINCE=$(git -C backfill-since log --first-parent --reverse \\\n+\t\t--format=%ct HEAD~1 | sed -n 5p) &&\n+\tgit -C backfill-since backfill --since=\"@$((SINCE - 1))\" HEAD &&\n+\n+\t# 6 missing: v1 of file.1.txt in all 6 directories\n+\tgit -C backfill-since rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 6 missing\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n \n-- \ngitgitgadget\n\n"},{"id":"539727","messageId":"f8f2c613260458e3c86109888ee1c51313095319.1774266019.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v2.git.1774266019.gitgitgadget@gmail.com","subject":"[PATCH v2 4/6] backfill: work with prefix pathspecs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-23T11:40:17Z","receivedAt":"2026-03-23T11:40:29Z","isPatch":true,"body":"From: Derrick Stolee \n\nThe previous change allowed specifying revision arguments over the 'git\nbackfill' command-line. This created the opportunity for restricting the\ninitial commit set by filtering the revision walk through a pathspec. Other\nthan filtering the commit set (and thereby the root trees), this did not\nrestrict the path-walk implementation of 'git backfill' and did not restrict\nthe blobs that were downloaded to only those matching the pathspec.\n\nUpdate the path-walk API to accept certain kinds of pathspecs and to\nsilently ignore anything too complex, for now. We will update this in the\nnext change to properly restrict to even complex pathspecs.\n\nThe current behavior focuses on pathspecs that match paths exactly. This\nincludes exact filenames, including directory names as prefixes. Pathspecs\ncontaining wildcards or magic are cleared so the path walk downloads all\nblobs, as before.\n\nThe reason for this restriction is to allow for a faster execution by\npruning the path walk to only trees that could contribute towards one of\nthose paths as a parent directory.\n\nThe test directory 'd/f/' (next to 'd/file*.txt') was prepared in a\nprevious commit to exercise the subtlety in prefix matching.\n\nSigned-off-by: Derrick Stolee \n---\n path-walk.c | 40 ++++++++++++++++++++++++++++++++++++++++\n path.c | 2 +-\n path.h | 6 ++++++\n t/t5620-backfill.sh | 16 ++++++----------\n 4 files changed, 53 insertions(+), 11 deletions(-)\n\ndiff --git a/path-walk.c b/path-walk.c\nindex 364e4cfa19..0d640e2f24 100644\n--- a/path-walk.c\n+++ b/path-walk.c\n@@ -11,6 +11,7 @@\n #include \"list-objects.h\"\n #include \"object.h\"\n #include \"oid-array.h\"\n+#include \"path.h\"\n #include \"prio-queue.h\"\n #include \"repository.h\"\n #include \"revision.h\"\n@@ -206,6 +207,34 @@ static int add_tree_entries(struct path_walk_context *ctx,\n \t\t\t\t match != MATCHED)\n \t\t\t\tcontinue;\n \t\t}\n+\t\tif (ctx->revs->prune_data.nr) {\n+\t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n+\t\t\tbool found = false;\n+\n+\t\t\t/* remove '/' for these checks. */\n+\t\t\tpath.buf[path.len - 1] = 0;\n+\n+\t\t\tfor (int i = 0; i < pd->nr; i++) {\n+\t\t\t\tstruct pathspec_item *item = &pd->items[i];\n+\n+\t\t\t\t/*\n+\t\t\t\t * Continue if either is a directory prefix\n+\t\t\t\t * of the other.\n+\t\t\t\t */\n+\t\t\t\tif (dir_prefix(path.buf, item->match) ||\n+\t\t\t\t dir_prefix(item->match, path.buf)) {\n+\t\t\t\t\tfound = true;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\t/* return '/' after these checks. */\n+\t\t\tpath.buf[path.len - 1] = '/';\n+\n+\t\t\t/* Skip paths that do not match the prefix. */\n+\t\t\tif (!found)\n+\t\t\t\tcontinue;\n+\t\t}\n \n \t\tadd_path_to_list(ctx, path.buf, type, &entry.oid,\n \t\t\t\t !(o->flags & UNINTERESTING));\n@@ -481,6 +510,17 @@ int walk_objects_by_path(struct path_walk_info *info)\n \tif (info->tags)\n \t\tinfo->revs->tag_objects = 1;\n \n+\tif (ctx.revs->prune_data.nr) {\n+\t\t/*\n+\t\t * Only exact prefix pathspecs are currently supported.\n+\t\t * Clear any wildcard or magic pathspecs to avoid\n+\t\t * incorrect prefix matching.\n+\t\t */\n+\t\tif (ctx.revs->prune_data.has_wildcard ||\n+\t\t ctx.revs->prune_data.magic)\n+\t\t\tclear_pathspec(&ctx.revs->prune_data);\n+\t}\n+\n \t/* Insert a single list for the root tree into the paths. */\n \tCALLOC_ARRAY(root_tree_list, 1);\n \troot_tree_list->type = OBJ_TREE;\ndiff --git a/path.c b/path.c\nindex d726537622..aebb10b2e9 100644\n--- a/path.c\n+++ b/path.c\n@@ -57,7 +57,7 @@ static void strbuf_cleanup_path(struct strbuf *sb)\n \t\tstrbuf_remove(sb, 0, path - sb->buf);\n }\n \n-static int dir_prefix(const char *buf, const char *dir)\n+int dir_prefix(const char *buf, const char *dir)\n {\n \tint len = strlen(dir);\n \treturn !strncmp(buf, dir, len) &&\ndiff --git a/path.h b/path.h\nindex 0ec95a0b07..829fafd7e9 100644\n--- a/path.h\n+++ b/path.h\n@@ -114,6 +114,12 @@ const char *repo_submodule_path_replace(struct repository *repo,\n \t\t\t\t\tconst char *fmt, ...)\n \t__attribute__((format (printf, 4, 5)));\n \n+/*\n+ * Given a directory name 'dir' (not ending with a trailing '/'),\n+ * determine if 'buf' is equal to 'dir' or has prefix 'dir'+'/'.\n+ */\n+int dir_prefix(const char *buf, const char *dir);\n+\n void report_linked_checkout_garbage(struct repository *r);\n \n /*\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex db66d8b614..52f6484ca1 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -273,13 +273,11 @@ test_expect_success 'backfill with prefix pathspec' '\n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n \ttest_line_count = 48 missing &&\n \n-\t# TODO: The pathspec should limit the downloaded blobs to\n-\t# only those matching the prefix \"d/f\", but currently all\n-\t# blobs are downloaded.\n-\tgit -C backfill-path backfill HEAD -- d/f &&\n+\tgit -C backfill-path backfill HEAD -- d/f 2>err &&\n+\ttest_must_be_empty err &&\n \n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n-\ttest_line_count = 0 missing\n+\ttest_line_count = 40 missing\n '\n \n test_expect_success 'backfill with multiple pathspecs' '\n@@ -292,13 +290,11 @@ test_expect_success 'backfill with multiple pathspecs' '\n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n \ttest_line_count = 48 missing &&\n \n-\t# TODO: The pathspecs should limit the downloaded blobs to\n-\t# only those matching \"d/f\" or \"a\", but currently all blobs\n-\t# are downloaded.\n-\tgit -C backfill-path backfill HEAD -- d/f a &&\n+\tgit -C backfill-path backfill HEAD -- d/f a 2>err &&\n+\ttest_must_be_empty err &&\n \n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n-\ttest_line_count = 0 missing\n+\ttest_line_count = 16 missing\n '\n \n test_expect_success 'backfill with wildcard pathspec' '\n-- \ngitgitgadget\n\n"},{"id":"539728","messageId":"1168edfb96c739581dd510f86bf1dbf5e28603e6.1774266019.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v2.git.1774266019.gitgitgadget@gmail.com","subject":"[PATCH v2 5/6] path-walk: support wildcard pathspecs for blob filtering","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-23T11:40:18Z","receivedAt":"2026-03-23T11:40:30Z","isPatch":true,"body":"From: Derrick Stolee \n\nPreviously, walk_objects_by_path() silently ignored pathspecs containing\nwildcards or magic by clearing them. This caused all blobs to be\ndownloaded regardless of the given pathspec. Wildcard pathspecs like\n\"d/file.*.txt\" are useful for narrowing which blobs to process (e.g.,\nduring 'git backfill').\n\nSupport wildcard pathspecs by making two changes:\n\n 1. Add an 'exact_pathspecs' flag to path_walk_context. When the\n pathspec has no wildcards or magic, set this flag and use the\n existing fast-path prefix matching in add_tree_entries(). When\n wildcards are present, skip that block since prefix matching\n cannot handle glob patterns.\n\n 2. Add a match_pathspec() check in walk_path() to filter out blobs\n whose full path does not match the pathspec. This provides the\n actual blob-level filtering for wildcard pathspecs.\n\nSigned-off-by: Derrick Stolee \n---\n path-walk.c | 22 +++++++++++++---------\n t/t5620-backfill.sh | 7 +++----\n 2 files changed, 16 insertions(+), 13 deletions(-)\n\ndiff --git a/path-walk.c b/path-walk.c\nindex 0d640e2f24..6b83e0e1d5 100644\n--- a/path-walk.c\n+++ b/path-walk.c\n@@ -63,6 +63,8 @@ struct path_walk_context {\n \t */\n \tstruct prio_queue path_stack;\n \tstruct strset path_stack_pushed;\n+\n+\tunsigned exact_pathspecs:1;\n };\n \n static int compare_by_type(const void *one, const void *two, void *cb_data)\n@@ -207,7 +209,7 @@ static int add_tree_entries(struct path_walk_context *ctx,\n \t\t\t\t match != MATCHED)\n \t\t\t\tcontinue;\n \t\t}\n-\t\tif (ctx->revs->prune_data.nr) {\n+\t\tif (ctx->revs->prune_data.nr && ctx->exact_pathspecs) {\n \t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n \t\t\tbool found = false;\n \n@@ -303,6 +305,13 @@ static int walk_path(struct path_walk_context *ctx,\n \t\t\treturn 0;\n \t}\n \n+\tif (list->type == OBJ_BLOB &&\n+\t ctx->revs->prune_data.nr &&\n+\t !match_pathspec(ctx->repo->index, &ctx->revs->prune_data,\n+\t\t\t path, strlen(path), 0,\n+\t\t\t NULL, 0))\n+\t\treturn 0;\n+\n \t/* Evaluate function pointer on this data, if requested. */\n \tif ((list->type == OBJ_TREE && ctx->info->trees) ||\n \t (list->type == OBJ_BLOB && ctx->info->blobs) ||\n@@ -511,14 +520,9 @@ int walk_objects_by_path(struct path_walk_info *info)\n \t\tinfo->revs->tag_objects = 1;\n \n \tif (ctx.revs->prune_data.nr) {\n-\t\t/*\n-\t\t * Only exact prefix pathspecs are currently supported.\n-\t\t * Clear any wildcard or magic pathspecs to avoid\n-\t\t * incorrect prefix matching.\n-\t\t */\n-\t\tif (ctx.revs->prune_data.has_wildcard ||\n-\t\t ctx.revs->prune_data.magic)\n-\t\t\tclear_pathspec(&ctx.revs->prune_data);\n+\t\tif (!ctx.revs->prune_data.has_wildcard &&\n+\t\t !ctx.revs->prune_data.magic)\n+\t\t\tctx.exact_pathspecs = 1;\n \t}\n \n \t/* Insert a single list for the root tree into the paths. */\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 52f6484ca1..c6f54ee91c 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -307,12 +307,11 @@ test_expect_success 'backfill with wildcard pathspec' '\n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n \ttest_line_count = 48 missing &&\n \n-\t# TODO: The wildcard pathspec should limit downloaded blobs,\n-\t# but currently all blobs are downloaded.\n-\tgit -C backfill-path backfill HEAD -- \"d/file.*.txt\" &&\n+\tgit -C backfill-path backfill HEAD -- \"d/file.*.txt\" 2>err &&\n+\ttest_must_be_empty err &&\n \n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n-\ttest_line_count = 0 missing\n+\ttest_line_count = 40 missing\n '\n \n test_expect_success 'backfill with --all' '\n-- \ngitgitgadget\n\n"},{"id":"539729","messageId":"9699650aa7dc04cf1cdc26803caa8304b29c1662.1774266019.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v2.git.1774266019.gitgitgadget@gmail.com","subject":"[PATCH v2 6/6] t5620: test backfill's unknown argument handling","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-23T11:40:19Z","receivedAt":"2026-03-23T11:40:32Z","isPatch":true,"body":"From: Derrick Stolee \n\nBefore the recent changes to parse rev-list arguments inside of 'git\nbackfill', the builtin would take arbitrary arguments without complaint (and\nignore them). This was noticed and a patch was sent [1] which motivates this\nchange to encode this behavior in test.\n\n[1] https://lore.kernel.org/git/20260321031643.5185-1-r.siddharth.shrimali@gmail.com/\n\nReported-by: Siddharth Shrimali \nSigned-off-by: Derrick Stolee \n---\n t/t5620-backfill.sh | 8 ++++++++\n 1 file changed, 8 insertions(+)\n\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex c6f54ee91c..85740f1f13 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -7,6 +7,14 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n \n+test_expect_success 'backfill rejects unexpected arguments' '\n+\ttest_must_fail git backfill unexpected-arg 2>err &&\n+\ttest_grep \"ambiguous argument .*unexpected-arg\" err &&\n+\n+\ttest_must_fail git backfill --all --firt-parent unexpected-arg 2>err &&\n+\ttest_grep \"ambiguous argument .*unexpected-arg\" err\n+'\n+\n # We create objects in the 'src' repo.\n test_expect_success 'setup repo for object creation' '\n \techo \"{print \\$1}\" >print_1.awk &&\n-- \ngitgitgadget\n"},{"id":"539748","messageId":"xmqqzf3y5zu2.fsf@gitster.g","threadId":"65272","inReplyTo":"9699650aa7dc04cf1cdc26803caa8304b29c1662.1774266019.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 6/6] t5620: test backfill's unknown argument handling","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-23T15:29:25Z","receivedAt":"2026-03-23T15:29:28Z","isPatch":true,"body":"\"Derrick Stolee via GitGitGadget\" writes:\n\n> From: Derrick Stolee \n>\n> Before the recent changes to parse rev-list arguments inside of 'git\n> backfill', the builtin would take arbitrary arguments without complaint (and\n> ignore them). This was noticed and a patch was sent [1] which motivates this\n> change to encode this behavior in test.\n>\n> [1] https://lore.kernel.org/git/20260321031643.5185-1-r.siddharth.shrimali@gmail.com/\n>\n> Reported-by: Siddharth Shrimali \n> Signed-off-by: Derrick Stolee \n> ---\n> t/t5620-backfill.sh | 8 ++++++++\n> 1 file changed, 8 insertions(+)\n>\n> diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\n> index c6f54ee91c..85740f1f13 100755\n> --- a/t/t5620-backfill.sh\n> +++ b/t/t5620-backfill.sh\n> @@ -7,6 +7,14 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n> \n> . ./test-lib.sh\n> \n> +test_expect_success 'backfill rejects unexpected arguments' '\n> +\ttest_must_fail git backfill unexpected-arg 2>err &&\n> +\ttest_grep \"ambiguous argument .*unexpected-arg\" err &&\n> +\n> +\ttest_must_fail git backfill --all --firt-parent unexpected-arg 2>err &&\n> +\ttest_grep \"ambiguous argument .*unexpected-arg\" err\n> +'\n\nHmph, I would have expected that an earlier --firt-parent on the\ncommand line would trigger \"unknown option\" instead.\n\nHaving said that, if the code lets the setup_revisions() parse the\ncommand line, the usual \"unless disambiguated with a double-dash\n'--', stop at the first non-revision and take everything as paths\nbut for safety all of them must refer to an existing path in the\nworking tree\" behaviour should trigger, and it is not specific to\n\"backfill\", and may already be tested centrally (if not, I do not\nobject to such a new set of tests).\n\nFor any cmd that take revisions and pathspec (e.g., log, rev-list,\ngrep) these should hold true:\n\n $ git $cmd []... Makefile HEAD\n\n Without disambiguation the command should say \"Ah, Makefile\n is not a revision, so we will see no more revisions, and\n everything, including the current one we are looking at, must be\n an existing path on the working tree\", and barfs on HEAD that\n does not exist as a file/directory.\n\n $ git $cmd []... Makefile -- HEAD\n\n With disambiguation, the command should verify everything before\n the double-dash to be a rev, and barf that Makefile is not a\n rev.\n\n $ git $cmd []... -- Makefile HEAD\n\n With disambiguation, the command should take everything after\n the double-dash to be a pathspec element without barfing. After\n all, it may be referring to a path that used to exist in some\n revision the command will look at.\n\nThanks.\n\n"},{"id":"539780","messageId":"7279f449-7bac-4364-bda0-5115980fa717@gmail.com","threadId":"65272","inReplyTo":"xmqqzf3y5zu2.fsf@gitster.g","subject":"Re: [PATCH v2 6/6] t5620: test backfill's unknown argument handling","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-23T20:39:16Z","receivedAt":"2026-03-23T20:39:18Z","isPatch":true,"body":"On 3/23/2026 11:29 AM, Junio C Hamano wrote:\n> \"Derrick Stolee via GitGitGadget\" writes:\n> \n>> From: Derrick Stolee \n>>\n>> Before the recent changes to parse rev-list arguments inside of 'git\n>> backfill', the builtin would take arbitrary arguments without complaint (and\n>> ignore them). This was noticed and a patch was sent [1] which motivates this\n>> change to encode this behavior in test.\n>>\n>> [1] https://lore.kernel.org/git/20260321031643.5185-1-r.siddharth.shrimali@gmail.com/\n>>\n>> Reported-by: Siddharth Shrimali \n>> Signed-off-by: Derrick Stolee \n>> ---\n>> t/t5620-backfill.sh | 8 ++++++++\n>> 1 file changed, 8 insertions(+)\n>>\n>> diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\n>> index c6f54ee91c..85740f1f13 100755\n>> --- a/t/t5620-backfill.sh\n>> +++ b/t/t5620-backfill.sh\n>> @@ -7,6 +7,14 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n>> \n>> . ./test-lib.sh\n>> \n>> +test_expect_success 'backfill rejects unexpected arguments' '\n>> +\ttest_must_fail git backfill unexpected-arg 2>err &&\n>> +\ttest_grep \"ambiguous argument .*unexpected-arg\" err &&\n>> +\n>> +\ttest_must_fail git backfill --all --firt-parent unexpected-arg 2>err &&\n>> +\ttest_grep \"ambiguous argument .*unexpected-arg\" err\n>> +'\n> \n> Hmph, I would have expected that an earlier --firt-parent on the\n> command line would trigger \"unknown option\" instead.\n\nInteresting that my mistype has demonstrated an interesting\nbehavior here. It turns out that random options starting with\n'--' are accepted here, including --unexpected-arg.\n\nThis means that we actually have room here for some improvement!\n\nI'll see what can be done to make even these arguments be seen\nas failures.\n\n> Having said that, if the code lets the setup_revisions() parse the\n> command line, the usual \"unless disambiguated with a double-dash\n> '--', stop at the first non-revision and take everything as paths\n> but for safety all of them must refer to an existing path in the\n> working tree\" behaviour should trigger, and it is not specific to\n> \"backfill\", and may already be tested centrally (if not, I do not\n> object to such a new set of tests).\n\nThanks,\n-Stolee\n\n"},{"id":"539812","messageId":"acJESdSvJUSCSWtK@pks.im","threadId":"65272","inReplyTo":"610a162973a7ad59eba4ef4d5a9288f1fea1d2e8.1774266019.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 3/6] backfill: accept revision arguments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-24T07:59:05Z","receivedAt":"2026-03-24T07:59:17Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 11:40:16AM +0000, Derrick Stolee via GitGitGadget wrote:\n> diff --git a/builtin/backfill.c b/builtin/backfill.c\n> index e80fc1b694..90c9d84793 100644\n> --- a/builtin/backfill.c\n> +++ b/builtin/backfill.c\n> @@ -134,7 +135,12 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n> \t\t\t\t\t builtin_backfill_usage, options);\n> \n> \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n> -\t\t\t 0);\n> +\t\t\t PARSE_OPT_KEEP_UNKNOWN_OPT |\n> +\t\t\t PARSE_OPT_KEEP_ARGV0 |\n> +\t\t\t PARSE_OPT_KEEP_DASHDASH);\n> +\n> +\trepo_init_revisions(repo, &ctx.revs, prefix);\n> +\targc = setup_revisions(argc, argv, &ctx.revs, NULL);\n\nWe should probably die here in case we still have unknown arguments.\n\nPatrick\n"},{"id":"539813","messageId":"acJEV18rQAh2i2a4@pks.im","threadId":"65272","inReplyTo":"f8f2c613260458e3c86109888ee1c51313095319.1774266019.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 4/6] backfill: work with prefix pathspecs","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-24T07:59:19Z","receivedAt":"2026-03-24T07:59:24Z","isPatch":true,"body":"On Mon, Mar 23, 2026 at 11:40:17AM +0000, Derrick Stolee via GitGitGadget wrote:\n> diff --git a/path-walk.c b/path-walk.c\n> index 364e4cfa19..0d640e2f24 100644\n> --- a/path-walk.c\n> +++ b/path-walk.c\n> @@ -206,6 +207,34 @@ static int add_tree_entries(struct path_walk_context *ctx,\n> \t\t\t\t match != MATCHED)\n> \t\t\t\tcontinue;\n> \t\t}\n> +\t\tif (ctx->revs->prune_data.nr) {\n> +\t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n> +\t\t\tbool found = false;\n> +\n> +\t\t\t/* remove '/' for these checks. */\n> +\t\t\tpath.buf[path.len - 1] = 0;\n\nHm. Is this _always_ safe to do? We add the directory separator a few\nlines further up, but only in the case where `type == OBJ_TREE`. So in\nreverse this may mean that there are cases where we don't have a\ntrailing '/'.\n\nMaybe we should instead:\n\n did_strip_suffix = strbuf_strip_suffix(path, \"/\");\n\n ...\n\n if (did_strip_suffix)\n strbuf_addch(path, \"/\");\n\nPatrick\n"},{"id":"540070","messageId":"a6bb702d-08c6-4c1d-9d3a-0a32bd73cfed@gmail.com","threadId":"65272","inReplyTo":"acJESdSvJUSCSWtK@pks.im","subject":"Re: [PATCH v2 3/6] backfill: accept revision arguments","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-26T12:55:12Z","receivedAt":"2026-03-26T12:55:15Z","isPatch":true,"body":"On 3/24/2026 3:59 AM, Patrick Steinhardt wrote:\n> On Mon, Mar 23, 2026 at 11:40:16AM +0000, Derrick Stolee via GitGitGadget wrote:\n>> diff --git a/builtin/backfill.c b/builtin/backfill.c\n>> index e80fc1b694..90c9d84793 100644\n>> --- a/builtin/backfill.c\n>> +++ b/builtin/backfill.c\n>> @@ -134,7 +135,12 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n>> \t\t\t\t\t builtin_backfill_usage, options);\n>> \n>> \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n>> -\t\t\t 0);\n>> +\t\t\t PARSE_OPT_KEEP_UNKNOWN_OPT |\n>> +\t\t\t PARSE_OPT_KEEP_ARGV0 |\n>> +\t\t\t PARSE_OPT_KEEP_DASHDASH);\n>> +\n>> +\trepo_init_revisions(repo, &ctx.revs, prefix);\n>> +\targc = setup_revisions(argc, argv, &ctx.revs, NULL);\n> \n> We should probably die here in case we still have unknown arguments.\n\nThat is indeed the fix for the bad test in patch 6. I'll make the\nnecessary update in v3's patch 6 along with the test for it.\n\nThanks,\n-Stolee\n\n"},{"id":"540071","messageId":"47cc997a-9a9e-4733-b5a3-ecf9f860a8c7@gmail.com","threadId":"65272","inReplyTo":"acJEV18rQAh2i2a4@pks.im","subject":"Re: [PATCH v2 4/6] backfill: work with prefix pathspecs","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-03-26T12:58:31Z","receivedAt":"2026-03-26T12:58:33Z","isPatch":true,"body":"On 3/24/2026 3:59 AM, Patrick Steinhardt wrote:\n> On Mon, Mar 23, 2026 at 11:40:17AM +0000, Derrick Stolee via GitGitGadget wrote:\n>> diff --git a/path-walk.c b/path-walk.c\n>> index 364e4cfa19..0d640e2f24 100644\n>> --- a/path-walk.c\n>> +++ b/path-walk.c\n>> @@ -206,6 +207,34 @@ static int add_tree_entries(struct path_walk_context *ctx,\n>> \t\t\t\t match != MATCHED)\n>> \t\t\t\tcontinue;\n>> \t\t}\n>> +\t\tif (ctx->revs->prune_data.nr) {\n>> +\t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n>> +\t\t\tbool found = false;\n>> +\n>> +\t\t\t/* remove '/' for these checks. */\n>> +\t\t\tpath.buf[path.len - 1] = 0;\n> \n> Hm. Is this _always_ safe to do? We add the directory separator a few\n> lines further up, but only in the case where `type == OBJ_TREE`. So in\n> reverse this may mean that there are cases where we don't have a\n> trailing '/'.\n> \n> Maybe we should instead:\n> \n> did_strip_suffix = strbuf_strip_suffix(path, \"/\");\n> \n> ...\n> \n> if (did_strip_suffix)\n> strbuf_addch(path, \"/\");\n\nThis is much cleaner, too! Thanks.\n\n-Stolee\n"},{"id":"540085","messageId":"pull.2070.v3.git.1774538094.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v2.git.1774266019.gitgitgadget@gmail.com","subject":"[PATCH v3 0/6] backfill: accept revision arguments","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-26T15:14:48Z","receivedAt":"2026-03-26T15:14:57Z","isPatch":true,"body":"The git backfill command assists in downloading missing blobs for blobless\npartial clones. However, its current version lacks some valuable\nfunctionality. It currently:\n\n 1. Only walks commits reachable from HEAD.\n 2. It walks all reachable commits to the full history.\n 3. It can focus on the current sparse-checkout definition, but otherwise it\n doesn't focus on a given pathspec.\n\nAll of these are being updated by this patch series, which allows rev-list\noptions to impact the path-walk. These include:\n\n 1. Specifying a given refspec, including --all.\n 2. Modifying the commit walk, including --first-parent, commit ranges, or\n recency using --since.\n 3. Modifying the set of paths to download using pathspecs.\n\nOne particularly valuable situation here is that now a user can run git\nbackfill -- to download all versions of a specific file or a specific\ndirectory, accelerating history queries within that path without downloading\nmore than necessary. This can accelerate git blame or git log -L for these\npaths, where normally those commands download missing blobs one-by-one\nduring its diff algorithms.\n\nThis patch series is organized in the following way:\n\n 1. A missing #include is added to prevent future compilation issues.\n 2. The test repo in t5620 is expanded to make later tests more interesting.\n 3. The backfill builtin parses the rev-list arguments. We test the top\n arguments that work as expected, though the pathspec arguments need\n extra work.\n 4. Update the path-walk logic to work efficiently with some pathspecs, such\n as fixed prefix pathspecs, accelerating the computation.\n 5. For more complicated pathspecs, do a post-filter in builtin/backfill.c\n instead of restricting the walk in the path-walk API.\n\nThe main goal of this series is to make such customizations possible, and to\nimprove performance where common use cases are expected. I'm open to\nfeedback as to whether we should consider more detailed performance analysis\nor whether we should wait for how users interact with these new options\nbefore overoptimizing unlikely use cases.\n\n\nUpdates in v2\n=============\n\n * Hard stops are replaced with a comma (and no punctuation) in the docs.\n * add_head_to_pending() simplifies some code.\n * My poor explanation of \"starting commits\" is updated.\n * Language around temporary prefix restriction is clarified.\n * Prefix match logic is simplified with dir_prefix().\n * Temporary memory leak (introduced in v1's patch 4 and removed in v1's\n patch 5) is removed in v2's patch 4.\n * Commit pruning is reenabled in v2's patch 5. There was no need for that\n with the way the logic works in the patch.\n * Add a new patch with a test demonstrating the new behavior that was being\n discussed in [1].\n\n[1]\nhttps://lore.kernel.org/git/20260321031643.5185-1-r.siddharth.shrimali@gmail.com/\n\n\nUpdates in v3\n=============\n\n * Fixed the argument checks to actually catch unknown arguments, because\n the revision machinery will skip unknown options starting with --.\n\nThanks, -Stolee\n\nDerrick Stolee (6):\n revision: include object-name.h\n t5620: prepare branched repo for revision tests\n backfill: accept revision arguments\n backfill: work with prefix pathspecs\n path-walk: support wildcard pathspecs for blob filtering\n t5620: test backfill's unknown argument handling\n\n Documentation/git-backfill.adoc | 5 +-\n builtin/backfill.c | 22 +++-\n path-walk.c | 43 +++++++\n path.c | 2 +-\n path.h | 6 +\n revision.h | 1 +\n t/t5620-backfill.sh | 211 +++++++++++++++++++++++++++++++-\n 7 files changed, 280 insertions(+), 10 deletions(-)\n\n\nbase-commit: 67ad42147a7acc2af6074753ebd03d904476118f\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2070%2Fderrickstolee%2Fbackfill-revs-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2070/derrickstolee/backfill-revs-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/2070\n\nRange-diff vs v2:\n\n 1: fda0239103 = 1: fda0239103 revision: include object-name.h\n 2: 55a45b2fc8 = 2: 55a45b2fc8 t5620: prepare branched repo for revision tests\n 3: 610a162973 = 3: 610a162973 backfill: accept revision arguments\n 4: f8f2c61326 ! 4: 7223124fb3 backfill: work with prefix pathspecs\n @@ path-walk.c: static int add_tree_entries(struct path_walk_context *ctx,\n +\t\tif (ctx->revs->prune_data.nr) {\n +\t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n +\t\t\tbool found = false;\n ++\t\t\tint did_strip_suffix = strbuf_strip_suffix(&path, \"/\");\n +\n -+\t\t\t/* remove '/' for these checks. */\n -+\t\t\tpath.buf[path.len - 1] = 0;\n +\n +\t\t\tfor (int i = 0; i < pd->nr; i++) {\n +\t\t\t\tstruct pathspec_item *item = &pd->items[i];\n @@ path-walk.c: static int add_tree_entries(struct path_walk_context *ctx,\n +\t\t\t\t}\n +\t\t\t}\n +\n -+\t\t\t/* return '/' after these checks. */\n -+\t\t\tpath.buf[path.len - 1] = '/';\n ++\t\t\tif (did_strip_suffix)\n ++\t\t\t\tstrbuf_addch(&path, '/');\n +\n +\t\t\t/* Skip paths that do not match the prefix. */\n +\t\t\tif (!found)\n 5: 1168edfb96 ! 5: 1ea278bd10 path-walk: support wildcard pathspecs for blob filtering\n @@ path-walk.c: static int add_tree_entries(struct path_walk_context *ctx,\n +\t\tif (ctx->revs->prune_data.nr && ctx->exact_pathspecs) {\n \t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n \t\t\tbool found = false;\n - \n + \t\t\tint did_strip_suffix = strbuf_strip_suffix(&path, \"/\");\n @@ path-walk.c: static int walk_path(struct path_walk_context *ctx,\n \t\t\treturn 0;\n \t}\n 6: 9699650aa7 ! 6: b6423f9595 t5620: test backfill's unknown argument handling\n @@ Commit message\n \n Before the recent changes to parse rev-list arguments inside of 'git\n backfill', the builtin would take arbitrary arguments without complaint (and\n - ignore them). This was noticed and a patch was sent [1] which motivates this\n - change to encode this behavior in test.\n + ignore them). This was noticed and a patch was sent [1] which motivates\n + this change.\n \n [1] https://lore.kernel.org/git/20260321031643.5185-1-r.siddharth.shrimali@gmail.com/\n \n + Note that the revision machinery can output an \"ambiguous argument\"\n + warning if a value not starting with '--' is found and doesn't make\n + sense as a reference or a pathspec. For unrecognized arguments starting\n + with '--' we need to add logic into builtin/backfill.c to catch leftover\n + arguments.\n +\n Reported-by: Siddharth Shrimali \n Signed-off-by: Derrick Stolee \n \n + ## builtin/backfill.c ##\n +@@ builtin/backfill.c: int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n + \trepo_init_revisions(repo, &ctx.revs, prefix);\n + \targc = setup_revisions(argc, argv, &ctx.revs, NULL);\n + \n ++\tif (argc > 1)\n ++\t\tdie(_(\"unrecognized argument: %s\"), argv[1]);\n ++\n + \trepo_config(repo, git_default_config, NULL);\n + \n + \tif (ctx.sparse < 0)\n +\n ## t/t5620-backfill.sh ##\n @@ t/t5620-backfill.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n @@ t/t5620-backfill.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n +\ttest_must_fail git backfill unexpected-arg 2>err &&\n +\ttest_grep \"ambiguous argument .*unexpected-arg\" err &&\n +\n -+\ttest_must_fail git backfill --all --firt-parent unexpected-arg 2>err &&\n -+\ttest_grep \"ambiguous argument .*unexpected-arg\" err\n ++\ttest_must_fail git backfill --all --unexpected-arg --first-parent 2>err &&\n ++\ttest_grep \"unrecognized argument: --unexpected-arg\" err\n +'\n +\n # We create objects in the 'src' repo.\n\n-- \ngitgitgadget\n"},{"id":"540086","messageId":"fda0239103f6e2b9e76403144b9ed2e9205e1c2a.1774538094.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v3.git.1774538094.gitgitgadget@gmail.com","subject":"[PATCH v3 1/6] revision: include object-name.h","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-26T15:14:49Z","receivedAt":"2026-03-26T15:14:58Z","isPatch":true,"body":"From: Derrick Stolee \n\nThe REV_INFO_INIT macro includes a use of the DEFAULT_ABBREV macro, which is\ndefined in object-name.h. Include it in revision.h so consumers of\nREV_INFO_INIT do not need to include this hidden dependency.\n\nSigned-off-by: Derrick Stolee \n---\n revision.h | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/revision.h b/revision.h\nindex b36acfc2d9..18c9bbd822 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -4,6 +4,7 @@\n #include \"commit.h\"\n #include \"grep.h\"\n #include \"notes.h\"\n+#include \"object-name.h\"\n #include \"oidset.h\"\n #include \"pretty.h\"\n #include \"diff.h\"\n-- \ngitgitgadget\n\n"},{"id":"540087","messageId":"55a45b2fc81895d2b712f126f716a8152d73fc93.1774538094.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v3.git.1774538094.gitgitgadget@gmail.com","subject":"[PATCH v3 2/6] t5620: prepare branched repo for revision tests","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-26T15:14:50Z","receivedAt":"2026-03-26T15:14:59Z","isPatch":true,"body":"From: Derrick Stolee \n\nPrepare the test infrastructure for upcoming changes that teach 'git\nbackfill' to accept revision arguments and pathspecs.\n\nAdd test_tick before each commit in the setup loop so that commit dates\nare deterministic. This enables reliable testing with '--since'.\n\nRename the 'd/e/' directory to 'd/f/' so that the prefix 'd/f' is\nambiguous with the files 'd/file.*.txt'. This exercises the subtlety\nin prefix pathspec matching that will be added in a later commit.\n\nCreate a branched version of the test repository (src-revs) with:\n - A 'side' branch merged into main, adding s/file.{1,2}.txt with\n two versions (4 new blobs, 52 total from main HEAD).\n - An unmerged 'other' branch adding o/file.{1,2}.txt (2 more blobs,\n 54 total reachable from --all).\n\nThis structure makes --all, --first-parent, and --since produce\nmeaningfully different results when used with 'git backfill'.\n\nSigned-off-by: Derrick Stolee \n---\n t/t5620-backfill.sh | 52 +++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 50 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 58c81556e7..1331949be4 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -15,7 +15,7 @@ test_expect_success 'setup repo for object creation' '\n \tgit init src &&\n \n \tmkdir -p src/a/b/c &&\n-\tmkdir -p src/d/e &&\n+\tmkdir -p src/d/f &&\n \n \tfor i in 1 2\n \tdo\n@@ -26,8 +26,9 @@ test_expect_success 'setup repo for object creation' '\n \t\t\techo \"Version $i of file a/b/$n\" > src/a/b/file.$n.txt &&\n \t\t\techo \"Version $i of file a/b/c/$n\" > src/a/b/c/file.$n.txt &&\n \t\t\techo \"Version $i of file d/$n\" > src/d/file.$n.txt &&\n-\t\t\techo \"Version $i of file d/e/$n\" > src/d/e/file.$n.txt &&\n+\t\t\techo \"Version $i of file d/f/$n\" > src/d/f/file.$n.txt &&\n \t\t\tgit -C src add . &&\n+\t\t\ttest_tick &&\n \t\t\tgit -C src commit -m \"Iteration $n\" || return 1\n \t\tdone\n \tdone\n@@ -41,6 +42,53 @@ test_expect_success 'setup bare clone for server' '\n \tgit -C srv.bare config --local uploadpack.allowanysha1inwant 1\n '\n \n+# Create a version of the repo with branches for testing revision\n+# arguments like --all, --first-parent, and --since.\n+#\n+# main: 8 commits (linear) + merge of side branch\n+# 48 original blobs + 4 side blobs = 52 blobs from main HEAD\n+# side: 2 commits adding s/file.{1,2}.txt (v1, v2), merged into main\n+# other: 1 commit adding o/file.{1,2}.txt (not merged)\n+# 54 total blobs reachable from --all\n+test_expect_success 'setup branched repo for revision tests' '\n+\tgit clone src src-revs &&\n+\n+\t# Side branch from tip of main with unique files\n+\tgit -C src-revs checkout -b side HEAD &&\n+\tmkdir -p src-revs/s &&\n+\techo \"Side version 1 of file 1\" >src-revs/s/file.1.txt &&\n+\techo \"Side version 1 of file 2\" >src-revs/s/file.2.txt &&\n+\ttest_tick &&\n+\tgit -C src-revs add . &&\n+\tgit -C src-revs commit -m \"Side commit 1\" &&\n+\n+\techo \"Side version 2 of file 1\" >src-revs/s/file.1.txt &&\n+\techo \"Side version 2 of file 2\" >src-revs/s/file.2.txt &&\n+\ttest_tick &&\n+\tgit -C src-revs add . &&\n+\tgit -C src-revs commit -m \"Side commit 2\" &&\n+\n+\t# Merge side into main\n+\tgit -C src-revs checkout main &&\n+\ttest_tick &&\n+\tgit -C src-revs merge side --no-ff -m \"Merge side branch\" &&\n+\n+\t# Other branch (not merged) for --all testing\n+\tgit -C src-revs checkout -b other main~1 &&\n+\tmkdir -p src-revs/o &&\n+\techo \"Other content 1\" >src-revs/o/file.1.txt &&\n+\techo \"Other content 2\" >src-revs/o/file.2.txt &&\n+\ttest_tick &&\n+\tgit -C src-revs add . &&\n+\tgit -C src-revs commit -m \"Other commit\" &&\n+\n+\tgit -C src-revs checkout main &&\n+\n+\tgit clone --bare \"file://$(pwd)/src-revs\" srv-revs.bare &&\n+\tgit -C srv-revs.bare config --local uploadpack.allowfilter 1 &&\n+\tgit -C srv-revs.bare config --local uploadpack.allowanysha1inwant 1\n+'\n+\n # do basic partial clone from \"srv.bare\"\n test_expect_success 'do partial clone 1, backfill gets all objects' '\n \tgit clone --no-checkout --filter=blob:none\t\\\n-- \ngitgitgadget\n\n"},{"id":"540088","messageId":"610a162973a7ad59eba4ef4d5a9288f1fea1d2e8.1774538094.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v3.git.1774538094.gitgitgadget@gmail.com","subject":"[PATCH v3 3/6] backfill: accept revision arguments","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-26T15:14:51Z","receivedAt":"2026-03-26T15:15:00Z","isPatch":true,"body":"From: Derrick Stolee \n\nThe existing implementation of 'git backfill' only includes downloading\nmissing blobs reachable from HEAD. Advanced uses may desire more general\ncommit limiting options, such as '--all' for all references, specifying a\ncommit range via negative references, or specifying a recency of use such as\nwith '--since='.\n\nAll of these options are available if we use setup_revisions() to parse the\nunknown arguments with the revision machinery. This opens up a large number\nof possibilities, only a small set of which are tested here.\n\nFor documentation, we avoid duplicating the option documentation and instead\nlink to the documentation of 'git rev-list'.\n\nNote that these arguments currently allow specifying a pathspec, which\nmodifies the commit history checks but does not limit the paths used in the\nbackfill logic. This will be updated in a future change.\n\nSigned-off-by: Derrick Stolee \n---\n Documentation/git-backfill.adoc | 5 +-\n builtin/backfill.c | 19 ++--\n t/t5620-backfill.sh | 156 ++++++++++++++++++++++++++++++++\n 3 files changed, 173 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-backfill.adoc b/Documentation/git-backfill.adoc\nindex b8394dcf22..246ab417c2 100644\n--- a/Documentation/git-backfill.adoc\n+++ b/Documentation/git-backfill.adoc\n@@ -63,9 +63,12 @@ OPTIONS\n \tcurrent sparse-checkout. If the sparse-checkout feature is enabled,\n \tthen `--sparse` is assumed and can be disabled with `--no-sparse`.\n \n+You may also specify the commit limiting options from linkgit:git-rev-list[1].\n+\n SEE ALSO\n --------\n-linkgit:git-clone[1].\n+linkgit:git-clone[1],\n+linkgit:git-rev-list[1]\n \n GIT\n ---\ndiff --git a/builtin/backfill.c b/builtin/backfill.c\nindex e80fc1b694..90c9d84793 100644\n--- a/builtin/backfill.c\n+++ b/builtin/backfill.c\n@@ -35,6 +35,7 @@ struct backfill_context {\n \tstruct oid_array current_batch;\n \tsize_t min_batch_size;\n \tint sparse;\n+\tstruct rev_info revs;\n };\n \n static void backfill_context_clear(struct backfill_context *ctx)\n@@ -80,7 +81,6 @@ static int fill_missing_blobs(const char *path UNUSED,\n \n static int do_backfill(struct backfill_context *ctx)\n {\n-\tstruct rev_info revs;\n \tstruct path_walk_info info = PATH_WALK_INFO_INIT;\n \tint ret;\n \n@@ -92,13 +92,14 @@ static int do_backfill(struct backfill_context *ctx)\n \t\t}\n \t}\n \n-\trepo_init_revisions(ctx->repo, &revs, \"\");\n-\thandle_revision_arg(\"HEAD\", &revs, 0, 0);\n+\t/* Walk from HEAD if otherwise unspecified. */\n+\tif (!ctx->revs.pending.nr)\n+\t\tadd_head_to_pending(&ctx->revs);\n \n \tinfo.blobs = 1;\n \tinfo.tags = info.commits = info.trees = 0;\n \n-\tinfo.revs = &revs;\n+\tinfo.revs = &ctx->revs;\n \tinfo.path_fn = fill_missing_blobs;\n \tinfo.path_fn_data = ctx;\n \n@@ -109,7 +110,6 @@ static int do_backfill(struct backfill_context *ctx)\n \t\tdownload_batch(ctx);\n \n \tpath_walk_info_clear(&info);\n-\trelease_revisions(&revs);\n \treturn ret;\n }\n \n@@ -121,6 +121,7 @@ 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.revs = REV_INFO_INIT,\n \t};\n \tstruct option options[] = {\n \t\tOPT_UNSIGNED(0, \"min-batch-size\", &ctx.min_batch_size,\n@@ -134,7 +135,12 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \t\t\t\t\t builtin_backfill_usage, options);\n \n \targc = parse_options(argc, argv, prefix, options, builtin_backfill_usage,\n-\t\t\t 0);\n+\t\t\t PARSE_OPT_KEEP_UNKNOWN_OPT |\n+\t\t\t PARSE_OPT_KEEP_ARGV0 |\n+\t\t\t PARSE_OPT_KEEP_DASHDASH);\n+\n+\trepo_init_revisions(repo, &ctx.revs, prefix);\n+\targc = setup_revisions(argc, argv, &ctx.revs, NULL);\n \n \trepo_config(repo, git_default_config, NULL);\n \n@@ -143,5 +149,6 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \n \tresult = do_backfill(&ctx);\n \tbackfill_context_clear(&ctx);\n+\trelease_revisions(&ctx.revs);\n \treturn result;\n }\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 1331949be4..db66d8b614 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -224,6 +224,162 @@ test_expect_success 'backfill --sparse without cone mode (negative)' '\n \ttest_line_count = 12 missing\n '\n \n+test_expect_success 'backfill with revision range' '\n+\ttest_when_finished rm -rf backfill-revs &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-revs &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-revs rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\tgit -C backfill-revs backfill HEAD~2..HEAD &&\n+\n+\t# 30 objects downloaded.\n+\tgit -C backfill-revs rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 18 missing\n+'\n+\n+test_expect_success 'backfill with revisions over stdin' '\n+\ttest_when_finished rm -rf backfill-revs &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-revs &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-revs rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\tcat >in <<-EOF &&\n+\tHEAD\n+\t^HEAD~2\n+\tEOF\n+\n+\tgit -C backfill-revs backfill --stdin missing &&\n+\ttest_line_count = 18 missing\n+'\n+\n+test_expect_success 'backfill with prefix pathspec' '\n+\ttest_when_finished rm -rf backfill-path &&\n+\tgit clone --bare --filter=blob:none\t\t \\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-path &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\t# TODO: The pathspec should limit the downloaded blobs to\n+\t# only those matching the prefix \"d/f\", but currently all\n+\t# blobs are downloaded.\n+\tgit -C backfill-path backfill HEAD -- d/f &&\n+\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with multiple pathspecs' '\n+\ttest_when_finished rm -rf backfill-path &&\n+\tgit clone --bare --filter=blob:none\t\t \\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-path &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\t# TODO: The pathspecs should limit the downloaded blobs to\n+\t# only those matching \"d/f\" or \"a\", but currently all blobs\n+\t# are downloaded.\n+\tgit -C backfill-path backfill HEAD -- d/f a &&\n+\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with wildcard pathspec' '\n+\ttest_when_finished rm -rf backfill-path &&\n+\tgit clone --bare --filter=blob:none\t\t \\\n+\t\t--single-branch --branch=main \t\t\\\n+\t\t\"file://$(pwd)/srv.bare\" backfill-path &&\n+\n+\t# No blobs yet\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 48 missing &&\n+\n+\t# TODO: The wildcard pathspec should limit downloaded blobs,\n+\t# but currently all blobs are downloaded.\n+\tgit -C backfill-path backfill HEAD -- \"d/file.*.txt\" &&\n+\n+\tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with --all' '\n+\ttest_when_finished rm -rf backfill-all &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t\"file://$(pwd)/srv-revs.bare\" backfill-all &&\n+\n+\t# All blobs from all refs are missing\n+\tgit -C backfill-all rev-list --quiet --objects --all --missing=print >missing &&\n+\ttest_line_count = 54 missing &&\n+\n+\t# Backfill from HEAD gets main blobs only\n+\tgit -C backfill-all backfill HEAD &&\n+\n+\t# Other branch blobs still missing\n+\tgit -C backfill-all rev-list --quiet --objects --all --missing=print >missing &&\n+\ttest_line_count = 2 missing &&\n+\n+\t# Backfill with --all gets everything\n+\tgit -C backfill-all backfill --all &&\n+\n+\tgit -C backfill-all rev-list --quiet --objects --all --missing=print >missing &&\n+\ttest_line_count = 0 missing\n+'\n+\n+test_expect_success 'backfill with --first-parent' '\n+\ttest_when_finished rm -rf backfill-fp &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main\t\t\t\\\n+\t\t\"file://$(pwd)/srv-revs.bare\" backfill-fp &&\n+\n+\tgit -C backfill-fp rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 52 missing &&\n+\n+\t# --first-parent skips the side branch commits, so\n+\t# s/file.{1,2}.txt v1 blobs (only in side commit 1) are missed.\n+\tgit -C backfill-fp backfill --first-parent HEAD &&\n+\n+\tgit -C backfill-fp rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 2 missing\n+'\n+\n+test_expect_success 'backfill with --since' '\n+\ttest_when_finished rm -rf backfill-since &&\n+\tgit clone --no-checkout --filter=blob:none\t\t\\\n+\t\t--single-branch --branch=main\t\t\t\\\n+\t\t\"file://$(pwd)/srv-revs.bare\" backfill-since &&\n+\n+\tgit -C backfill-since rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 52 missing &&\n+\n+\t# Use a cutoff between commits 4 and 5 (between v1 and v2\n+\t# iterations). Commits 5-8 still carry v1 of files 2-4 in\n+\t# their trees, but v1 of file.1.txt is only in commits 1-4.\n+\tSINCE=$(git -C backfill-since log --first-parent --reverse \\\n+\t\t--format=%ct HEAD~1 | sed -n 5p) &&\n+\tgit -C backfill-since backfill --since=\"@$((SINCE - 1))\" HEAD &&\n+\n+\t# 6 missing: v1 of file.1.txt in all 6 directories\n+\tgit -C backfill-since rev-list --quiet --objects --missing=print HEAD >missing &&\n+\ttest_line_count = 6 missing\n+'\n+\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n \n-- \ngitgitgadget\n\n"},{"id":"540089","messageId":"7223124fb3229fc3a06a3208a43181716cec2eac.1774538094.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v3.git.1774538094.gitgitgadget@gmail.com","subject":"[PATCH v3 4/6] backfill: work with prefix pathspecs","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-26T15:14:52Z","receivedAt":"2026-03-26T15:15:02Z","isPatch":true,"body":"From: Derrick Stolee \n\nThe previous change allowed specifying revision arguments over the 'git\nbackfill' command-line. This created the opportunity for restricting the\ninitial commit set by filtering the revision walk through a pathspec. Other\nthan filtering the commit set (and thereby the root trees), this did not\nrestrict the path-walk implementation of 'git backfill' and did not restrict\nthe blobs that were downloaded to only those matching the pathspec.\n\nUpdate the path-walk API to accept certain kinds of pathspecs and to\nsilently ignore anything too complex, for now. We will update this in the\nnext change to properly restrict to even complex pathspecs.\n\nThe current behavior focuses on pathspecs that match paths exactly. This\nincludes exact filenames, including directory names as prefixes. Pathspecs\ncontaining wildcards or magic are cleared so the path walk downloads all\nblobs, as before.\n\nThe reason for this restriction is to allow for a faster execution by\npruning the path walk to only trees that could contribute towards one of\nthose paths as a parent directory.\n\nThe test directory 'd/f/' (next to 'd/file*.txt') was prepared in a\nprevious commit to exercise the subtlety in prefix matching.\n\nSigned-off-by: Derrick Stolee \n---\n path-walk.c | 39 +++++++++++++++++++++++++++++++++++++++\n path.c | 2 +-\n path.h | 6 ++++++\n t/t5620-backfill.sh | 16 ++++++----------\n 4 files changed, 52 insertions(+), 11 deletions(-)\n\ndiff --git a/path-walk.c b/path-walk.c\nindex 364e4cfa19..3750552978 100644\n--- a/path-walk.c\n+++ b/path-walk.c\n@@ -11,6 +11,7 @@\n #include \"list-objects.h\"\n #include \"object.h\"\n #include \"oid-array.h\"\n+#include \"path.h\"\n #include \"prio-queue.h\"\n #include \"repository.h\"\n #include \"revision.h\"\n@@ -206,6 +207,33 @@ static int add_tree_entries(struct path_walk_context *ctx,\n \t\t\t\t match != MATCHED)\n \t\t\t\tcontinue;\n \t\t}\n+\t\tif (ctx->revs->prune_data.nr) {\n+\t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n+\t\t\tbool found = false;\n+\t\t\tint did_strip_suffix = strbuf_strip_suffix(&path, \"/\");\n+\n+\n+\t\t\tfor (int i = 0; i < pd->nr; i++) {\n+\t\t\t\tstruct pathspec_item *item = &pd->items[i];\n+\n+\t\t\t\t/*\n+\t\t\t\t * Continue if either is a directory prefix\n+\t\t\t\t * of the other.\n+\t\t\t\t */\n+\t\t\t\tif (dir_prefix(path.buf, item->match) ||\n+\t\t\t\t dir_prefix(item->match, path.buf)) {\n+\t\t\t\t\tfound = true;\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n+\n+\t\t\tif (did_strip_suffix)\n+\t\t\t\tstrbuf_addch(&path, '/');\n+\n+\t\t\t/* Skip paths that do not match the prefix. */\n+\t\t\tif (!found)\n+\t\t\t\tcontinue;\n+\t\t}\n \n \t\tadd_path_to_list(ctx, path.buf, type, &entry.oid,\n \t\t\t\t !(o->flags & UNINTERESTING));\n@@ -481,6 +509,17 @@ int walk_objects_by_path(struct path_walk_info *info)\n \tif (info->tags)\n \t\tinfo->revs->tag_objects = 1;\n \n+\tif (ctx.revs->prune_data.nr) {\n+\t\t/*\n+\t\t * Only exact prefix pathspecs are currently supported.\n+\t\t * Clear any wildcard or magic pathspecs to avoid\n+\t\t * incorrect prefix matching.\n+\t\t */\n+\t\tif (ctx.revs->prune_data.has_wildcard ||\n+\t\t ctx.revs->prune_data.magic)\n+\t\t\tclear_pathspec(&ctx.revs->prune_data);\n+\t}\n+\n \t/* Insert a single list for the root tree into the paths. */\n \tCALLOC_ARRAY(root_tree_list, 1);\n \troot_tree_list->type = OBJ_TREE;\ndiff --git a/path.c b/path.c\nindex d726537622..aebb10b2e9 100644\n--- a/path.c\n+++ b/path.c\n@@ -57,7 +57,7 @@ static void strbuf_cleanup_path(struct strbuf *sb)\n \t\tstrbuf_remove(sb, 0, path - sb->buf);\n }\n \n-static int dir_prefix(const char *buf, const char *dir)\n+int dir_prefix(const char *buf, const char *dir)\n {\n \tint len = strlen(dir);\n \treturn !strncmp(buf, dir, len) &&\ndiff --git a/path.h b/path.h\nindex 0ec95a0b07..829fafd7e9 100644\n--- a/path.h\n+++ b/path.h\n@@ -114,6 +114,12 @@ const char *repo_submodule_path_replace(struct repository *repo,\n \t\t\t\t\tconst char *fmt, ...)\n \t__attribute__((format (printf, 4, 5)));\n \n+/*\n+ * Given a directory name 'dir' (not ending with a trailing '/'),\n+ * determine if 'buf' is equal to 'dir' or has prefix 'dir'+'/'.\n+ */\n+int dir_prefix(const char *buf, const char *dir);\n+\n void report_linked_checkout_garbage(struct repository *r);\n \n /*\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex db66d8b614..52f6484ca1 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -273,13 +273,11 @@ test_expect_success 'backfill with prefix pathspec' '\n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n \ttest_line_count = 48 missing &&\n \n-\t# TODO: The pathspec should limit the downloaded blobs to\n-\t# only those matching the prefix \"d/f\", but currently all\n-\t# blobs are downloaded.\n-\tgit -C backfill-path backfill HEAD -- d/f &&\n+\tgit -C backfill-path backfill HEAD -- d/f 2>err &&\n+\ttest_must_be_empty err &&\n \n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n-\ttest_line_count = 0 missing\n+\ttest_line_count = 40 missing\n '\n \n test_expect_success 'backfill with multiple pathspecs' '\n@@ -292,13 +290,11 @@ test_expect_success 'backfill with multiple pathspecs' '\n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n \ttest_line_count = 48 missing &&\n \n-\t# TODO: The pathspecs should limit the downloaded blobs to\n-\t# only those matching \"d/f\" or \"a\", but currently all blobs\n-\t# are downloaded.\n-\tgit -C backfill-path backfill HEAD -- d/f a &&\n+\tgit -C backfill-path backfill HEAD -- d/f a 2>err &&\n+\ttest_must_be_empty err &&\n \n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n-\ttest_line_count = 0 missing\n+\ttest_line_count = 16 missing\n '\n \n test_expect_success 'backfill with wildcard pathspec' '\n-- \ngitgitgadget\n\n"},{"id":"540090","messageId":"1ea278bd10bdd0b7980750c9d0c450b044aec196.1774538094.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v3.git.1774538094.gitgitgadget@gmail.com","subject":"[PATCH v3 5/6] path-walk: support wildcard pathspecs for blob filtering","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-26T15:14:53Z","receivedAt":"2026-03-26T15:15:03Z","isPatch":true,"body":"From: Derrick Stolee \n\nPreviously, walk_objects_by_path() silently ignored pathspecs containing\nwildcards or magic by clearing them. This caused all blobs to be\ndownloaded regardless of the given pathspec. Wildcard pathspecs like\n\"d/file.*.txt\" are useful for narrowing which blobs to process (e.g.,\nduring 'git backfill').\n\nSupport wildcard pathspecs by making two changes:\n\n 1. Add an 'exact_pathspecs' flag to path_walk_context. When the\n pathspec has no wildcards or magic, set this flag and use the\n existing fast-path prefix matching in add_tree_entries(). When\n wildcards are present, skip that block since prefix matching\n cannot handle glob patterns.\n\n 2. Add a match_pathspec() check in walk_path() to filter out blobs\n whose full path does not match the pathspec. This provides the\n actual blob-level filtering for wildcard pathspecs.\n\nSigned-off-by: Derrick Stolee \n---\n path-walk.c | 22 +++++++++++++---------\n t/t5620-backfill.sh | 7 +++----\n 2 files changed, 16 insertions(+), 13 deletions(-)\n\ndiff --git a/path-walk.c b/path-walk.c\nindex 3750552978..2aa3e7d8a4 100644\n--- a/path-walk.c\n+++ b/path-walk.c\n@@ -63,6 +63,8 @@ struct path_walk_context {\n \t */\n \tstruct prio_queue path_stack;\n \tstruct strset path_stack_pushed;\n+\n+\tunsigned exact_pathspecs:1;\n };\n \n static int compare_by_type(const void *one, const void *two, void *cb_data)\n@@ -207,7 +209,7 @@ static int add_tree_entries(struct path_walk_context *ctx,\n \t\t\t\t match != MATCHED)\n \t\t\t\tcontinue;\n \t\t}\n-\t\tif (ctx->revs->prune_data.nr) {\n+\t\tif (ctx->revs->prune_data.nr && ctx->exact_pathspecs) {\n \t\t\tstruct pathspec *pd = &ctx->revs->prune_data;\n \t\t\tbool found = false;\n \t\t\tint did_strip_suffix = strbuf_strip_suffix(&path, \"/\");\n@@ -302,6 +304,13 @@ static int walk_path(struct path_walk_context *ctx,\n \t\t\treturn 0;\n \t}\n \n+\tif (list->type == OBJ_BLOB &&\n+\t ctx->revs->prune_data.nr &&\n+\t !match_pathspec(ctx->repo->index, &ctx->revs->prune_data,\n+\t\t\t path, strlen(path), 0,\n+\t\t\t NULL, 0))\n+\t\treturn 0;\n+\n \t/* Evaluate function pointer on this data, if requested. */\n \tif ((list->type == OBJ_TREE && ctx->info->trees) ||\n \t (list->type == OBJ_BLOB && ctx->info->blobs) ||\n@@ -510,14 +519,9 @@ int walk_objects_by_path(struct path_walk_info *info)\n \t\tinfo->revs->tag_objects = 1;\n \n \tif (ctx.revs->prune_data.nr) {\n-\t\t/*\n-\t\t * Only exact prefix pathspecs are currently supported.\n-\t\t * Clear any wildcard or magic pathspecs to avoid\n-\t\t * incorrect prefix matching.\n-\t\t */\n-\t\tif (ctx.revs->prune_data.has_wildcard ||\n-\t\t ctx.revs->prune_data.magic)\n-\t\t\tclear_pathspec(&ctx.revs->prune_data);\n+\t\tif (!ctx.revs->prune_data.has_wildcard &&\n+\t\t !ctx.revs->prune_data.magic)\n+\t\t\tctx.exact_pathspecs = 1;\n \t}\n \n \t/* Insert a single list for the root tree into the paths. */\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex 52f6484ca1..c6f54ee91c 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -307,12 +307,11 @@ test_expect_success 'backfill with wildcard pathspec' '\n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n \ttest_line_count = 48 missing &&\n \n-\t# TODO: The wildcard pathspec should limit downloaded blobs,\n-\t# but currently all blobs are downloaded.\n-\tgit -C backfill-path backfill HEAD -- \"d/file.*.txt\" &&\n+\tgit -C backfill-path backfill HEAD -- \"d/file.*.txt\" 2>err &&\n+\ttest_must_be_empty err &&\n \n \tgit -C backfill-path rev-list --quiet --objects --missing=print HEAD >missing &&\n-\ttest_line_count = 0 missing\n+\ttest_line_count = 40 missing\n '\n \n test_expect_success 'backfill with --all' '\n-- \ngitgitgadget\n\n"},{"id":"540091","messageId":"b6423f9595bf003fc87670658f172a98c6257155.1774538094.git.gitgitgadget@gmail.com","threadId":"65272","inReplyTo":"pull.2070.v3.git.1774538094.gitgitgadget@gmail.com","subject":"[PATCH v3 6/6] t5620: test backfill's unknown argument handling","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-03-26T15:14:54Z","receivedAt":"2026-03-26T15:15:04Z","isPatch":true,"body":"From: Derrick Stolee \n\nBefore the recent changes to parse rev-list arguments inside of 'git\nbackfill', the builtin would take arbitrary arguments without complaint (and\nignore them). This was noticed and a patch was sent [1] which motivates\nthis change.\n\n[1] https://lore.kernel.org/git/20260321031643.5185-1-r.siddharth.shrimali@gmail.com/\n\nNote that the revision machinery can output an \"ambiguous argument\"\nwarning if a value not starting with '--' is found and doesn't make\nsense as a reference or a pathspec. For unrecognized arguments starting\nwith '--' we need to add logic into builtin/backfill.c to catch leftover\narguments.\n\nReported-by: Siddharth Shrimali \nSigned-off-by: Derrick Stolee \n---\n builtin/backfill.c | 3 +++\n t/t5620-backfill.sh | 8 ++++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/builtin/backfill.c b/builtin/backfill.c\nindex 90c9d84793..edc19c01e5 100644\n--- a/builtin/backfill.c\n+++ b/builtin/backfill.c\n@@ -142,6 +142,9 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n \trepo_init_revisions(repo, &ctx.revs, prefix);\n \targc = setup_revisions(argc, argv, &ctx.revs, NULL);\n \n+\tif (argc > 1)\n+\t\tdie(_(\"unrecognized argument: %s\"), argv[1]);\n+\n \trepo_config(repo, git_default_config, NULL);\n \n \tif (ctx.sparse < 0)\ndiff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh\nindex c6f54ee91c..2c347a91fe 100755\n--- a/t/t5620-backfill.sh\n+++ b/t/t5620-backfill.sh\n@@ -7,6 +7,14 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n \n . ./test-lib.sh\n \n+test_expect_success 'backfill rejects unexpected arguments' '\n+\ttest_must_fail git backfill unexpected-arg 2>err &&\n+\ttest_grep \"ambiguous argument .*unexpected-arg\" err &&\n+\n+\ttest_must_fail git backfill --all --unexpected-arg --first-parent 2>err &&\n+\ttest_grep \"unrecognized argument: --unexpected-arg\" err\n+'\n+\n # We create objects in the 'src' repo.\n test_expect_success 'setup repo for object creation' '\n \techo \"{print \\$1}\" >print_1.awk &&\n-- \ngitgitgadget\n"},{"id":"540163","messageId":"acYsortL6kx7yneC@pks.im","threadId":"65272","inReplyTo":"pull.2070.v3.git.1774538094.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 0/6] backfill: accept revision arguments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-03-27T07:07:14Z","receivedAt":"2026-03-27T07:07:21Z","isPatch":true,"body":"On Thu, Mar 26, 2026 at 03:14:48PM +0000, Derrick Stolee via GitGitGadget wrote:\n> 6: 9699650aa7 ! 6: b6423f9595 t5620: test backfill's unknown argument handling\n> @@ Commit message\n> \n> Before the recent changes to parse rev-list arguments inside of 'git\n> backfill', the builtin would take arbitrary arguments without complaint (and\n> - ignore them). This was noticed and a patch was sent [1] which motivates this\n> - change to encode this behavior in test.\n> + ignore them). This was noticed and a patch was sent [1] which motivates\n> + this change.\n> \n> [1] https://lore.kernel.org/git/20260321031643.5185-1-r.siddharth.shrimali@gmail.com/\n> \n> + Note that the revision machinery can output an \"ambiguous argument\"\n> + warning if a value not starting with '--' is found and doesn't make\n> + sense as a reference or a pathspec. For unrecognized arguments starting\n> + with '--' we need to add logic into builtin/backfill.c to catch leftover\n> + arguments.\n> +\n> Reported-by: Siddharth Shrimali \n> Signed-off-by: Derrick Stolee \n> \n> + ## builtin/backfill.c ##\n> +@@ builtin/backfill.c: int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit\n> + \trepo_init_revisions(repo, &ctx.revs, prefix);\n> + \targc = setup_revisions(argc, argv, &ctx.revs, NULL);\n> + \n> ++\tif (argc > 1)\n> ++\t\tdie(_(\"unrecognized argument: %s\"), argv[1]);\n> ++\n> + \trepo_config(repo, git_default_config, NULL);\n> + \n> + \tif (ctx.sparse < 0)\n> +\n> ## t/t5620-backfill.sh ##\n\nI would've expected this chunk to already come in patch 3, but that\nalone isn't really worth a reroll. All the other changes look good to\nme, thanks!\n\nPatrick\n"}]}