[PATCH 1/3] backfill: reject rev-list arguments that do not make sense

["Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>]

From: Elijah Newren <newren@gmail.com>

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.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 builtin/backfill.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/builtin/backfill.c b/builtin/backfill.c
index d794dd842f..a9edddcb7e 100644
--- a/builtin/backfill.c
+++ b/builtin/backfill.c
@@ -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