Re: [PATCH v4] builtin/refs: add '--no-reflog' flag to drop reflogs

From: Karthik Nayak <karthik.188@gmail.com>
Date: Feb 21, 2025, 08:44 UTC
Message-ID: <CAOLa=ZSL-X_8s6vcDiWBp8G3+M8GGGA1wBRtNqqThyq94YS16w@mail.gmail.com>
In-Reply-To: <xmqqikp4eji8.fsf@gitster.g>

FJunio C Hamano <gitster@pobox.com> writes:

Show 17 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> The 'git-refs(1)' migrate subcommand, which transfers repositories
>> between reference backends, currently migrates reflogs by default as of
>> 246cebe320 (refs: add support for migrating reflogs, 2024-12-16).
>
> "transfer" is a curious verb to use here, as it almost exclusively
> is used in the context of fetch-and-push object transfer over the
> wire.
>
> 	The "git refs migrate" subcommand converts the backend used
> 	for ref storage.  It always migrates reflog data as well as
> 	refs.  Allow it to optionally discard reflog data.  This is
> 	useful because ...
>
> or something?
>

Sure, I'll modify it to something along these lines :)

Show 8 quoted lines

>>  builtin/refs.c          |  3 +++
>>  refs.c                  |  8 +++++---
>>  refs.h                  |  5 ++++-
>>  t/t1460-refs-migrate.sh | 28 ++++++++++++++++++++++++----
>>  4 files changed, 36 insertions(+), 8 deletions(-)
>
> I notice there is something missing.
>

For a minute I thought I broke something here, but I'm assuming you mean the lack of documentation.

Show 15 quoted lines

>
>> diff --git a/builtin/refs.c b/builtin/refs.c
>> index a29f195834..c459507d51 100644
>> --- a/builtin/refs.c
>> +++ b/builtin/refs.c
>> ...
>> +		OPT_BIT(0, "no-reflog", &flags,
>> +			N_("drop reflogs entirely during the migration"),
>> +			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
>
> This is somewhat ugly, but parseopt API is nice enough to hide the
> "--no-no-reflog" nonsense from the end users, so this is OK.
>
> I think we are almost there but lack documentation updates?
>

Yeah, this was a total miss. Thanks for pointing out. Will add it in.

Show 16 quoted lines

>
>  Documentation/git-refs.txt | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git c/Documentation/git-refs.txt w/Documentation/git-refs.txt
> index 9829984b0a..bb50d6f888 100644
> --- c/Documentation/git-refs.txt
> +++ w/Documentation/git-refs.txt
> @@ -8,9 +8,9 @@ git-refs - Low-level access to refs
>
>  SYNOPSIS
>  --------
> -[verse]
> -'git refs migrate' --ref-format=<format> [--dry-run]
> -'git refs verify' [--strict] [--verbose]
> +[synopsis]

I see '[synopsis]' being called out in 'Documentation/CodingGuidelines', but nothing about '[verse]'.

Show 17 quoted lines

> +git refs migrate --ref-format=<format> [--no-reflog] [--dry-run]
> +git refs verify [--strict] [--verbose]
>
>  DESCRIPTION
>  -----------
> @@ -43,6 +43,11 @@ include::ref-storage-format.txt[]
>  	can be used to double check that the migration works as expected before
>  	performing the actual migration.
>
> +--reflog::
> +--no-reflog::
> +	Choose between migrating the reflog data to the new backend,
> +	and discarding them.  The default is "--reflog" to migrate.
> +
>  The following options are specific to 'git refs verify':
>
>  --strict::

Will add this in! Thanks for the review.