Re: [GSoC PATCH] backfill: error out when HEAD cannot be parsed
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- Mar 30, 2026, 09:48 UTC
- Message-ID
- <CAOLa=ZSRAQtZKNWUw4YhX4bx6MwFccN5oR0tSKQFtex0sKZGmQ@mail.gmail.com>
- In-Reply-To
- <20260329183603.538241-1-vikingtc4@gmail.com>
Trieu Huynh <vikingtc4@gmail.com> writes:
> handle_revision_arg() returns non-zero on failure, but do_backfill() > ignored the return value. On an empty repo with no commits, HEAD is
Nit: s/ignored/ignores
We use present tense in the commit message. See the 'present-tense' section in 'Documentation/SubmittingPatches' for more info.
> unborn and handle_revision_arg() fails, but backfill silently > continues with an empty revision walk and exits zero, looks like > success but did nothing.
Perhaps: we want to say 'and silently exists with a zero return code.' to be more explicit.
> Check the return value and propagate the error, consistent with > how builtin/pack-objects.c handles handle_revision_arg() failures. >
Nice.
Show 21 quoted lines
> Add a test to verify that backfill on an empty repository fails
> with a clear error message.
>
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> ---
> builtin/backfill.c | 3 ++-
> t/t5620-backfill.sh | 6 ++++++
> 2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/backfill.c b/builtin/backfill.c
> index 27a301f9b2..4b2db94173 100644
> --- a/builtin/backfill.c
> +++ b/builtin/backfill.c
> @@ -96,7 +96,8 @@ static int do_backfill(struct backfill_context *ctx)
> }
>
> repo_init_revisions(ctx->repo, &revs, "");
> - handle_revision_arg("HEAD", &revs, 0, 0);
> + if (handle_revision_arg("HEAD", &revs, 0, 0))
> + return error(_("unable to parse HEAD revision"));
>Makes sense.
Show 16 quoted lines
> info.blobs = 1; > info.tags = info.commits = info.trees = 0; > diff --git a/t/t5620-backfill.sh b/t/t5620-backfill.sh > index ff67e8ecea..91b5115732 100755 > --- a/t/t5620-backfill.sh > +++ b/t/t5620-backfill.sh > @@ -101,6 +101,12 @@ test_expect_success 'backfill no flag on non-TTY is silent' ' > test_grep ! "Downloading batches" err > ' > > +test_expect_success 'backfill on empty repo fails gracefully' ' > + git init empty-repo && > + test_must_fail git -C empty-repo backfill 2>err && > + grep "unable to parse HEAD" err > +' > +
Looks good. Thanks
Show 5 quoted lines
> 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.43.0