From: Karthik Nayak Date: Wed, 30 Sep 2026 11:06:10 GMT Subject: Re: [PATCH RFC 4/5] backfill: add --dry-run option Message-ID: In-Reply-To: <20260930-backfill-dryrun-v1-4-1128f247ee01@gmail.com> Pablo Sabater writes: > Users have no way to know how many blobs git backfill is going to > download before running it. > > Add a new --dry-run option to the backfill command. The objects are > walked as usual, but instead of fetching each batch of missing blobs > they are only counted, and the total is printed at the end. > > A subsequent commit will also print their size when the server supports > the object-info capability. > > Signed-off-by: Pablo Sabater > --- > Documentation/git-backfill.adoc | 6 +++++- > builtin/backfill.c | 37 +++++++++++++++++++++++++++++++++---- > t/t5620-backfill.sh | 26 ++++++++++++++++++++++++++ > 3 files changed, 64 insertions(+), 5 deletions(-) > > diff --git a/Documentation/git-backfill.adoc b/Documentation/git-backfill.adoc > index 82d6a1969d..08f19fea17 100644 > --- a/Documentation/git-backfill.adoc > +++ b/Documentation/git-backfill.adoc > @@ -9,7 +9,7 @@ git-backfill - Download missing objects in a partial clone > SYNOPSIS > -------- > [synopsis] > -git backfill [--min-batch-size=] [--[no-]sparse] [--[no-]include-edges] [] > +git backfill [--min-batch-size=] [--[no-]sparse] [--[no-]include-edges] [--dry-run] [] > > DESCRIPTION > ----------- > @@ -70,6 +70,10 @@ OPTIONS > --onto TARGET A..B`, where A..B normally excludes A but you need > the blobs from A as well. `--include-edges` is the default. > > +`--dry-run`:: > + Do not download any objects. Instead, print the number of > + missing blobs that would be downloaded. > + > ``:: > Backfill only blobs reachable from commits in the specified > revision range. When no __ is specified, it > diff --git a/builtin/backfill.c b/builtin/backfill.c > index e71e0f4742..6019112966 100644 > --- a/builtin/backfill.c > +++ b/builtin/backfill.c > @@ -26,7 +26,7 @@ > #include "path-walk.h" > > static const char * const builtin_backfill_usage[] = { > - N_("git backfill [--min-batch-size=] [--[no-]sparse] [--[no-]include-edges] []"), > + N_("git backfill [--min-batch-size=] [--[no-]sparse] [--[no-]include-edges] [--dry-run] []"), > NULL > }; > > @@ -36,6 +36,8 @@ struct backfill_context { > size_t min_batch_size; > int sparse; > int include_edges; > + int dry_run; Nit: This could be a bool, since `OPT__DRY_RUN` uses `OPT_BOOL` internally. > + size_t total_batch_nr; > struct rev_info revs; > }; > > @@ -58,6 +60,15 @@ static void download_batch(struct backfill_context *ctx) > odb_reprepare(ctx->repo->objects); > } > > +static void dry_run_batch(struct backfill_context *ctx) > +{ While it is used during dry_run, probably makes more sense to rename it to `count_batch()` since that's what it does. > + if (!ctx->current_batch.nr) > + return; > + > + ctx->total_batch_nr += ctx->current_batch.nr; > + oid_array_clear(&ctx->current_batch); > +} > + > static int fill_missing_blobs(const char *path UNUSED, > struct oid_array *list, > enum object_type type, > @@ -73,8 +84,12 @@ static int fill_missing_blobs(const char *path UNUSED, > oid_array_append(&ctx->current_batch, &list->oid[i]); > } > > - if (ctx->current_batch.nr >= ctx->min_batch_size) > - download_batch(ctx); > + if (ctx->current_batch.nr >= ctx->min_batch_size) { > + if (ctx->dry_run) > + dry_run_batch(ctx); > + else > + download_batch(ctx); > + } > > return 0; > } > @@ -131,10 +146,23 @@ static int do_backfill(struct backfill_context *ctx) > > ret = walk_objects_by_path(&info); > > + if (ret) > + goto end; > + > /* Download the objects that did not fill a batch. */ > - if (!ret) > + if (!ctx->dry_run) { > download_batch(ctx); > + goto end; > + } > + > + dry_run_batch(ctx); > + > + printf(Q_("After backfill, %" PRIuMAX " blob would be fetched.\n", > + "After backfill, %" PRIuMAX " blobs would be fetched.\n", > + (unsigned long)ctx->total_batch_nr), > + (uintmax_t)ctx->total_batch_nr); > Nit: Okay so we have a goto inside the first if(...), which skips this section. I would have found it easier to read if it was if (dry_run) count() else download() > +end: > path_walk_info_clear(&info); > return ret; > } > @@ -157,6 +185,7 @@ int cmd_backfill(int argc, const char **argv, const char *prefix, struct reposit > 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__DRY_RUN(&ctx.dry_run, N_("Preview the number of blobs to be fetched")), > OPT_END(), > }; > struct repo_config_values *cfg = repo_config_values(the_repository); > diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh > index 7462280470..e76fa6081b 100755 > --- a/t/t5620-backfill.sh > +++ b/t/t5620-backfill.sh > @@ -141,6 +141,32 @@ test_expect_success 'do partial clone 2, backfill min batch size' ' > test_line_count = 0 revs2 > ' > > +test_expect_success '--dry-run reports missing blobs without fetching them' ' > + test_when_finished "rm -rf backfill-dry-run dry-trace" && > + git clone --no-checkout --filter=blob:none \ > + --single-branch --branch=main \ > + "file://$(pwd)/srv.bare" backfill-dry-run && > + > + GIT_TRACE2_EVENT="$(pwd)/dry-trace" git \ > + -C backfill-dry-run backfill --dry-run >out && > + > + test_grep "48 blobs would be fetched" out && > + test_grep ! fetch_count dry-trace && > + git -C backfill-dry-run rev-list --quiet --objects --missing=print HEAD >missing && > + test_line_count = 48 missing > +' > + > +test_expect_success '--dry-run with no missing blobs' ' > + test_when_finished rm -rf backfill-dry-run && > + git clone --no-checkout --filter=blob:none \ > + --single-branch --branch=main \ > + "file://$(pwd)/srv.bare" backfill-dry-run && > + git -C backfill-dry-run backfill && > + > + git -C backfill-dry-run backfill --dry-run >out && > + test_grep "0 blobs would be fetched" out > +' > + > test_expect_success 'backfill --sparse without sparse-checkout fails' ' > git init not-sparse && > test_must_fail git -C not-sparse backfill --sparse 2>err && > > -- > 2.54.0