Backfill fixes and edges

This topic fixes a few minor issues in git backfill (from ds/backfill-revs this cycle), although some might see the third patch as more feature than fix, and the first two patches are pretty minor and probably do not merit consideration before the release this late in the cycle.

Overview:

* Patch 1: As a wise man once said, "Sending arbitrary command-line arguments to setup_revisions() creates an opportunity for behavior you are not expecting. For instance, can users...supply --first-parent? What happens if they add an --author filter?" ;-) I think these particular cases might work, but other rev-list options don't make sense, so let's error on ones that don't.
* Patch 2: Making documentation more consistent with other commands
* Patch 3: Tweak the ranges so we actually prevent on-demand blob downloading better with a new --[no-]include-edges flag.

Elijah Newren (3):
backfill: reject rev-list arguments that do not make sense
backfill: document acceptance of revision-range in more standard
manner
backfill: default to grabbing edge blobs too

 Documentation/git-backfill.adoc |  22 ++++++-
 builtin/backfill.c              |  31 ++++++++-
 t/t5620-backfill.sh             | 110 ++++++++++++++++++++++++++++++--
 3 files changed, 153 insertions(+), 10 deletions(-)

base-commit: 9f223ef1c026d91c7ac68cc0211bde255dda6199
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2088%2Fnewren%2Fbackfill-fixes-and-edges-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2088/newren/backfill-fixes-and-edges-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2088

gitgitgadget

From: Elijah Newren <[email protected]>

Some rev-list options accepted by setup_revisions() are silently ignored or actively counterproductive when used with 'git backfill', because the path-walk API has its own tree-walking logic that bypasses the mechanisms these options rely on:

* -S/-G (pickaxe) and --diff-filter work by computing per-commit diffs in get_revision_1() and filtering commits whose diffs don't match. Since backfill's goal is to download all blobs reachable from commits in the range, filtering out commits based on diff content would silently skip blobs -- the opposite of what users want.

* --follow disables path pruning (revs->prune) and only makes sense for tracking a single file through renames in log output. It has no useful interaction with backfill.

* -L (line-log) computes line-level diffs to track the evolution of a function or line range. Like pickaxe, it filters commits based on diff content, which would cause blobs to be silently skipped.

* --diff-merges controls how merge commit diffs are displayed. The path-walk API walks trees directly and never computes per-commit diffs, so this option would be silently ignored.

* --filter (object filtering, e.g. --filter=blob:none) is used by the list-objects traversal but is completely ignored by the path-walk API, so it would silently do nothing.

Rather than letting users think these options are being honored, reject them with a clear error message.

---
 builtin/backfill.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

index d794dd842f..a9edddcb7e 100644
@@ -78,6 +78,28 @@ static int fill_missing_blobs(const char *path UNUSED,
 	return 0;
 }
 
+static void reject_unsupported_rev_list_options(struct rev_info *revs)
+{
+	if (revs->diffopt.pickaxe)
+		die(_("'%s' cannot be used with 'git backfill'"),
+		    (revs->diffopt.pickaxe_opts & DIFF_PICKAXE_REGEX) ? "-G" : "-S");
+	if (revs->diffopt.filter || revs->diffopt.filter_not)
+		die(_("'%s' cannot be used with 'git backfill'"),
+		    "--diff-filter");
+	if (revs->diffopt.flags.follow_renames)
+		die(_("'%s' cannot be used with 'git backfill'"),
+		    "--follow");
+	if (revs->line_level_traverse)
+		die(_("'%s' cannot be used with 'git backfill'"),
+		    "-L");
+	if (revs->explicit_diff_merges)
+		die(_("'%s' cannot be used with 'git backfill'"),
+		    "--diff-merges");
+	if (revs->filter.choice)
+		die(_("'%s' cannot be used with 'git backfill'"),
+		    "--filter");
+}
+
 static int do_backfill(struct backfill_context *ctx)
 {
 	struct path_walk_info info = PATH_WALK_INFO_INIT;

@@ -144,6 +166,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
 
 	if (argc > 1)
 		die(_("unrecognized argument: %s"), argv[1]);
+	reject_unsupported_rev_list_options(&ctx.revs);
 
 	repo_config(repo, git_default_config, NULL);
 

gitgitgadget

From: Elijah Newren <[email protected]>

302aff09223f (backfill: accept revision arguments, 2026-03-26) added support for passing revision arguments to 'git backfill' but documented them only with a prose sentence:

You may also specify the commit limiting options from git-rev-list(1).

No other command that accepts revision arguments documents them this way. Commands like log, shortlog, and replay define a formal <revision-range> entry and include rev-list-options.adoc. Commands like bundle, fast-export, and filter-branch, which pass arguments through to the revision machinery without including the full options file, still define a formal <git-rev-list-args> entry explaining what is accepted.

Add a formal <revision-range> entry in the synopsis and OPTIONS section, following the convention used by other commands, and mention that commit-limiting options from git-rev-list(1) are also accepted.

---
 Documentation/git-backfill.adoc | 15 ++++++++++++---
 builtin/backfill.c              |  2 +-
 2 files changed, 13 insertions(+), 4 deletions(-)

index 246ab417c2..bf26d7694f 100644
@@ -9,7 +9,7 @@ git-backfill - Download missing objects in a partial clone
 SYNOPSIS
 --------
 [synopsis]
-git backfill [--min-batch-size=<n>] [--[no-]sparse]
+git backfill [--min-batch-size=<n>] [--[no-]sparse] [<revision-range>]
 
 DESCRIPTION
 -----------

@@ -43,7 +43,7 @@ smaller network calls than downloading the entire repository at clone
 time.
 
 By default, `git backfill` downloads all blobs reachable from the `HEAD`
-commit. This set can be restricted or expanded using various options.
+commit. This set can be restricted or expanded using various options below.
 
 THIS COMMAND IS EXPERIMENTAL. ITS BEHAVIOR MAY CHANGE IN THE FUTURE.
 

@@ -63,7 +63,16 @@ OPTIONS
 	current sparse-checkout. If the sparse-checkout feature is enabled,
 	then `--sparse` is assumed and can be disabled with `--no-sparse`.
 
-You may also specify the commit limiting options from linkgit:git-rev-list[1].
+`<revision-range>`::
+	Backfill only blobs reachable from commits in the specified
+	revision range.  When no _<revision-range>_ is specified, it
+	defaults to `HEAD` (i.e. the whole history leading to the
+	current commit).  For a complete list of ways to spell
+	_<revision-range>_, see the "Specifying Ranges" section of
+	linkgit:gitrevisions[7].
++
+You may also use commit-limiting options understood by
+linkgit:git-rev-list[1] such as `--first-parent`, `--since`, or pathspecs.
 
 SEE ALSO
 --------

index a9edddcb7e..e934d360fd 100644
@@ -26,7 +26,7 @@
 #include "path-walk.h"
 
 static const char * const builtin_backfill_usage[] = {
-	N_("git backfill [--min-batch-size=<n>] [--[no-]sparse]"),
+	N_("git backfill [--min-batch-size=<n>] [--[no-]sparse] [<revision-range>]"),
 	NULL
 };
 

gitgitgadget

From: Elijah Newren <[email protected]>

Commit 302aff09223f (backfill: accept revision arguments, 2026-03-26) added support for accepting revision arguments to backfill. This allows users to do things like

   git backfill --remotes ^v2.3.0

and then run many commands without triggering on-demand downloads of blobs. However, if they have topics based on v2.3.0, they will likely still trigger on-demand downloads. Consider, for example, the command

git log -p v2.3.0..topic

This would still trigger on-demand blob loadings after the backfill command above, because the commit(s) with A as a parent will need to diff against the blobs in A. In fact, multiple commands need blobs from the lower boundary of the revision range:

   * git log -p A..B                # After backfill A..B
   * git replay --onto TARGET A..B  # After backfill TARGET^! A..B
   * git checkout A && git merge B  # After backfill A...B

Add an extra --[no-]include-edges flag to allow grabbing blobs from edge commits. Since the point of backfill is to prevent on-demand blob loading and these are common commands, default to --include-edges.

---
 Documentation/git-backfill.adoc |   9 ++-
 builtin/backfill.c              |   8 ++-
 t/t5620-backfill.sh             | 110 ++++++++++++++++++++++++++++++--
 3 files changed, 119 insertions(+), 8 deletions(-)

index bf26d7694f..c0a3b80615 100644
@@ -9,7 +9,7 @@ git-backfill - Download missing objects in a partial clone
 SYNOPSIS
 --------
 [synopsis]
-git backfill [--min-batch-size=<n>] [--[no-]sparse] [<revision-range>]
+git backfill [--min-batch-size=<n>] [--[no-]sparse] [--[no-]include-edges] [<revision-range>]
 
 DESCRIPTION
 -----------

@@ -63,6 +63,13 @@ OPTIONS
 	current sparse-checkout. If the sparse-checkout feature is enabled,
 	then `--sparse` is assumed and can be disabled with `--no-sparse`.
 
+`--include-edges`::
+`--no-include-edges`::
+	Include blobs from boundary commits in the backfill.  Useful in
+	preparation for commands like `git log -p A..B` or `git replay
+	--onto TARGET A..B`, where A..B normally excludes A but you need
+	the blobs from A as well.  `--include-edges` is the default.
+
 `<revision-range>`::
 	Backfill only blobs reachable from commits in the specified
 	revision range.  When no _<revision-range>_ is specified, it

index e934d360fd..7ffab2ea74 100644
@@ -26,7 +26,7 @@
 #include "path-walk.h"
 
 static const char * const builtin_backfill_usage[] = {
-	N_("git backfill [--min-batch-size=<n>] [--[no-]sparse] [<revision-range>]"),
+	N_("git backfill [--min-batch-size=<n>] [--[no-]sparse] [--[no-]include-edges] [<revision-range>]"),
 	NULL
 };
 

@@ -35,6 +35,7 @@ struct backfill_context {
 	struct oid_array current_batch;
 	size_t min_batch_size;
 	int sparse;
+	int include_edges;
 	struct rev_info revs;
 };
 

@@ -116,6 +117,8 @@ static int do_backfill(struct backfill_context *ctx)
 	/* Walk from HEAD if otherwise unspecified. */
 	if (!ctx->revs.pending.nr)
 		add_head_to_pending(&ctx->revs);
+	if (ctx->include_edges)
+		ctx->revs.edge_hint = 1;
 
 	info.blobs = 1;
 	info.tags = info.commits = info.trees = 0;

@@ -143,12 +146,15 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit
 		.min_batch_size = 50000,
 		.sparse = -1,
 		.revs = REV_INFO_INIT,
+		.include_edges = 1,
 	};
 	struct option options[] = {
 		OPT_UNSIGNED(0, "min-batch-size", &ctx.min_batch_size,
 			     N_("Minimum number of objects to request at a time")),
 		OPT_BOOL(0, "sparse", &ctx.sparse,
 			 N_("Restrict the missing objects to the current sparse-checkout")),
+		OPT_BOOL(0, "include-edges", &ctx.include_edges,
+			 N_("Include blobs from boundary commits in the backfill")),
 		OPT_END(),
 	};
 	struct repo_config_values *cfg = repo_config_values(the_repository);

gitgitgadget

I think these make a lot of sense, especially because these computations require downloading missing blobs in order to find the diffs that justify some of the choices of commit filtering.

I think there are a few other "format" based options that were silently ignored on purpose, because there's no output. Perhaps we should change the use of options like this to a warning instead of a failure?

This is correct to remove because while it doesn't work with path-walk right now, it might in the future. We don't want the filter to mess with the functionality of 'git backfill' that sets its own scope for which blobs to download.

I agree that the majority of these should be hard failures. As mentioned, some could be soft warnings. That could be an adjustment to make in the future, so is not blocking for this patch.

Thanks for your attention to detail here. I like this version.

-Stolee

I like this option and your motivation for including it.

I know that --first-parent was one of the options I _did_ want to include as a potential option (it helps focus the set to a "core" of commits and we can get more on-demand off the core if needed). Yes, --author is a little silly, but it didn't seem necessary to block it.

I agree with the reasons you gave to block _most_ of the options you blocked. The output-formatting options don't need to be a hard failure, but that could be a later improvement. For now, I think your change is entirely positive so doesn't need change.

I gave notes on every patch, but no meaningful changes are required.

Thanks for helping to polish this feature!
-Stolee