[PATCH] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[PATCH] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

The 'git-refs(1)' migrate subcommand, which transfers repositories
between reference backends, currently migrates reflogs by default as of
In 246cebe320 (refs: add support for migrating reflogs, 2024-12-16).
While this behavior is desirable for most client-side repositories,
server-side repositories typically don't use reflogs and the migration
of these entries is unnecessary overhead.

Add a '--skip-reflog' flag to the migrate subcommand to make reflog
migration optional. This is particularly useful for server-side
migrations where reflogs are not needed, improving migration performance
in these scenarios.

Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
---
 builtin/refs.c          |  3 +++
 refs.c                  |  8 +++++---
 refs.h                  |  5 ++++-
 t/t1460-refs-migrate.sh | 17 +++++++++++++++--
 4 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/builtin/refs.c b/builtin/refs.c
index a29f19583474518ee0942ea53c39cbdf9661c5e2..30be0254c14dd3d07693d70c25dddc9990756e9c 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -30,6 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
 		OPT_BIT(0, "dry-run", &flags,
 			N_("perform a non-destructive dry-run"),
 			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
+		OPT_BIT(0, "skip-reflog", &flags,
+			N_("skip migrating reflogs"),
+			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
 		OPT_END(),
 	};
 	struct strbuf errbuf = STRBUF_INIT;
diff --git a/refs.c b/refs.c
index f4094a326a9f88f979654b668cc9c3d27d83cb5d..5e8f5c06fa68d16c93ee11edd9742995eea994b6 100644
--- a/refs.c
+++ b/refs.c
@@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
 	if (ret < 0)
 		goto done;
 
-	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
-	if (ret < 0)
-		goto done;
+	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
+		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
+		if (ret < 0)
+			goto done;
+	}
 
 	ret = ref_transaction_commit(transaction, errbuf);
 	if (ret < 0)
diff --git a/refs.h b/refs.h
index a0cdd99250e8286b55808b697b0a94afac5d8319..ccee8fc6705e6e93a1c6234e7395180f8dfd815b 100644
--- a/refs.h
+++ b/refs.h
@@ -1157,8 +1157,11 @@ int is_pseudo_ref(const char *refname);
  *   - REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN: perform a dry-run migration
  *     without touching the main repository. The result will be written into a
  *     temporary ref storage directory.
+ *
+ *   - REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG: skip migration of reflogs.
  */
-#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN (1 << 0)
+#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN      (1 << 0)
+#define REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG (1 << 1)
 
 /*
  * Migrate the ref storage format used by the repository to the
diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
index a6d9b35a46eb59350aa0d59d982a2fbfaecf1448..9059d4c4121842a9d2e77dc4e54c537eeff8afab 100755
--- a/t/t1460-refs-migrate.sh
+++ b/t/t1460-refs-migrate.sh
@@ -9,14 +9,16 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 # Migrate the provided repository from one format to the other and
 # verify that the references and logs are migrated over correctly.
-# Usage: test_migration <repo> <format> <skip_reflog_verify>
+# Usage: test_migration <repo> <format> <skip_reflog_verify> <...options>
 #   <repo> is the relative path to the repo to be migrated.
 #   <format> is the ref format to be migrated to.
 #   <skip_reflog_verify> (true or false) whether to skip reflog verification.
+#   <...options> are other options be passed directly to 'git refs migrate'.
 test_migration () {
 	repo=$1 &&
 	format=$2 &&
 	skip_reflog_verify=${3:-false} &&
+	shift $(( $# >= 3 ? 3 : 2 )) &&
 	git -C "$repo" for-each-ref --include-root-refs \
 		--format='%(refname) %(objectname) %(symref)' >expect &&
 	if ! $skip_reflog_verify
@@ -25,7 +27,7 @@ test_migration () {
 	   git -C "$repo" reflog list >expect_log_list
 	fi &&
 
-	git -C "$repo" refs migrate --ref-format="$2" &&
+	git -C "$repo" refs migrate --ref-format="$format" $@ &&
 
 	git -C "$repo" for-each-ref --include-root-refs \
 		--format='%(refname) %(objectname) %(symref)' >actual &&
@@ -241,6 +243,17 @@ do
 				test_cmp expect.reflog actual.reflog
 			)
 		'
+
+		test_expect_success "$from_format -> $to_format: skip reflog with --skip-reflog" '
+			test_when_finished "rm -rf repo" &&
+			git init --ref-format=$from_format repo &&
+			test_commit -C repo initial &&
+			# we see that the repository contains reflogs.
+			test 2 = $(git -C repo reflog --all | wc -l) &&
+			test_migration repo "$to_format" true --skip-reflog &&
+			# there should be no reflogs post migration.
+			test 0 = $(git -C repo reflog --all | wc -l)
+		'
 	done
 done
 

---

base-commit: bc204b742735ae06f65bb20291c95985c9633b7f
change-id: 20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-8066f6df50ac

Thanks
- Karthik

Re: [PATCH] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Patrick Steinhardt]

On Fri, Feb 07, 2025 at 12:57:31PM +0100, Karthik Nayak wrote:
> The 'git-refs(1)' migrate subcommand, which transfers repositories
> between reference backends, currently migrates reflogs by default as of
> In 246cebe320 (refs: add support for migrating reflogs, 2024-12-16).

s/In//

> While this behavior is desirable for most client-side repositories,
> server-side repositories typically don't use reflogs and the migration
> of these entries is unnecessary overhead.

Nit: if the server-side repository doesn't _have_ reflogs, then there
cannot be any overhead caused by their migration either, right? I still
think that the flag makes sense (well, I proposed it). But to me the
argument is rather that we don't _expect_ there to be any reflogs, but
due to historic reasons there actually _might_ be some. This could for
example be caused by a bugs, misconfiguration, or an admin who has
enabled reflogs on the server-side to debug something.

So even if there are some reflogs, we don't want to migrate them. Which
coincidentally helps us to improve performance, but the real value-add
here is that it makes the result match our expectations.

> Add a '--skip-reflog' flag to the migrate subcommand to make reflog
> migration optional. This is particularly useful for server-side
> migrations where reflogs are not needed, improving migration performance
> in these scenarios.

The second sentence of this paragraph feels duplicated with what you
have already been saying in the preceding paragraph.

> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> ---

Another thing to teach b4: skip the comment in a single-patch patch
series in case you don't supply a cover letter :)

> diff --git a/refs.c b/refs.c
> index f4094a326a9f88f979654b668cc9c3d27d83cb5d..5e8f5c06fa68d16c93ee11edd9742995eea994b6 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
>  	if (ret < 0)
>  		goto done;
>  
> -	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
> -	if (ret < 0)
> -		goto done;
> +	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
> +		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
> +		if (ret < 0)
> +			goto done;
> +	}
>  
>  	ret = ref_transaction_commit(transaction, errbuf);
>  	if (ret < 0)

Nice and simple, as expected.

> diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
> index a6d9b35a46eb59350aa0d59d982a2fbfaecf1448..9059d4c4121842a9d2e77dc4e54c537eeff8afab 100755
> --- a/t/t1460-refs-migrate.sh
> +++ b/t/t1460-refs-migrate.sh
> @@ -9,14 +9,16 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  
>  # Migrate the provided repository from one format to the other and
>  # verify that the references and logs are migrated over correctly.
> -# Usage: test_migration <repo> <format> <skip_reflog_verify>
> +# Usage: test_migration <repo> <format> <skip_reflog_verify> <...options>
>  #   <repo> is the relative path to the repo to be migrated.
>  #   <format> is the ref format to be migrated to.
>  #   <skip_reflog_verify> (true or false) whether to skip reflog verification.
> +#   <...options> are other options be passed directly to 'git refs migrate'.
>  test_migration () {
>  	repo=$1 &&
>  	format=$2 &&
>  	skip_reflog_verify=${3:-false} &&
> +	shift $(( $# >= 3 ? 3 : 2 )) &&

I honestly have no idea whether this works with all supported shells. If
it does it's a bit funky, but should work alright for our purpose. I was
thinking a bit about how to improve this, but ultimately came to the
conclusion that there isn't really a need to overengineer this simple
test function.

> @@ -241,6 +243,17 @@ do
>  				test_cmp expect.reflog actual.reflog
>  			)
>  		'
> +
> +		test_expect_success "$from_format -> $to_format: skip reflog with --skip-reflog" '
> +			test_when_finished "rm -rf repo" &&
> +			git init --ref-format=$from_format repo &&
> +			test_commit -C repo initial &&
> +			# we see that the repository contains reflogs.
> +			test 2 = $(git -C repo reflog --all | wc -l) &&

Nit: we don't want to have Git on the left-hand side of a pipe, as it
might make use lose its exit code. This could instead be:

    git -C repo reflog --all >reflogs &&
    test_line_count = 2 reflogs

> +			test_migration repo "$to_format" true --skip-reflog &&
> +			# there should be no reflogs post migration.
> +			test 0 = $(git -C repo reflog --all | wc -l)

And here we could use `test_must_be_empty`.

Thanks!

Patrick

Re: [PATCH] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 5012 bytes --]

Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Feb 07, 2025 at 12:57:31PM +0100, Karthik Nayak wrote:
>> The 'git-refs(1)' migrate subcommand, which transfers repositories
>> between reference backends, currently migrates reflogs by default as of
>> In 246cebe320 (refs: add support for migrating reflogs, 2024-12-16).
>
> s/In//
>
>> While this behavior is desirable for most client-side repositories,
>> server-side repositories typically don't use reflogs and the migration
>> of these entries is unnecessary overhead.
>
> Nit: if the server-side repository doesn't _have_ reflogs, then there
> cannot be any overhead caused by their migration either, right? I still
> think that the flag makes sense (well, I proposed it). But to me the
> argument is rather that we don't _expect_ there to be any reflogs, but
> due to historic reasons there actually _might_ be some. This could for
> example be caused by a bugs, misconfiguration, or an admin who has
> enabled reflogs on the server-side to debug something.
>
> So even if there are some reflogs, we don't want to migrate them. Which
> coincidentally helps us to improve performance, but the real value-add
> here is that it makes the result match our expectations.
>

Fair enough, I agree that, finally, we mostly care about not having
reflogs in the end. I'll modify accordingly.

>> Add a '--skip-reflog' flag to the migrate subcommand to make reflog
>> migration optional. This is particularly useful for server-side
>> migrations where reflogs are not needed, improving migration performance
>> in these scenarios.
>
> The second sentence of this paragraph feels duplicated with what you
> have already been saying in the preceding paragraph.
>

Will cleanup.

>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> ---
>
> Another thing to teach b4: skip the comment in a single-patch patch
> series in case you don't supply a cover letter :)
>

True. I think this is because of lack of conditionals in the templating.

>> diff --git a/refs.c b/refs.c
>> index f4094a326a9f88f979654b668cc9c3d27d83cb5d..5e8f5c06fa68d16c93ee11edd9742995eea994b6 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
>>  	if (ret < 0)
>>  		goto done;
>>
>> -	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
>> -	if (ret < 0)
>> -		goto done;
>> +	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
>> +		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
>> +		if (ret < 0)
>> +			goto done;
>> +	}
>>
>>  	ret = ref_transaction_commit(transaction, errbuf);
>>  	if (ret < 0)
>
> Nice and simple, as expected.
>
>> diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
>> index a6d9b35a46eb59350aa0d59d982a2fbfaecf1448..9059d4c4121842a9d2e77dc4e54c537eeff8afab 100755
>> --- a/t/t1460-refs-migrate.sh
>> +++ b/t/t1460-refs-migrate.sh
>> @@ -9,14 +9,16 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>>
>>  # Migrate the provided repository from one format to the other and
>>  # verify that the references and logs are migrated over correctly.
>> -# Usage: test_migration <repo> <format> <skip_reflog_verify>
>> +# Usage: test_migration <repo> <format> <skip_reflog_verify> <...options>
>>  #   <repo> is the relative path to the repo to be migrated.
>>  #   <format> is the ref format to be migrated to.
>>  #   <skip_reflog_verify> (true or false) whether to skip reflog verification.
>> +#   <...options> are other options be passed directly to 'git refs migrate'.
>>  test_migration () {
>>  	repo=$1 &&
>>  	format=$2 &&
>>  	skip_reflog_verify=${3:-false} &&
>> +	shift $(( $# >= 3 ? 3 : 2 )) &&
>
> I honestly have no idea whether this works with all supported shells. If
> it does it's a bit funky, but should work alright for our purpose. I was
> thinking a bit about how to improve this, but ultimately came to the
> conclusion that there isn't really a need to overengineer this simple
> test function.
>

I was skeptical too, while not a complete test, the CI seemed to not
complain.

>> @@ -241,6 +243,17 @@ do
>>  				test_cmp expect.reflog actual.reflog
>>  			)
>>  		'
>> +
>> +		test_expect_success "$from_format -> $to_format: skip reflog with --skip-reflog" '
>> +			test_when_finished "rm -rf repo" &&
>> +			git init --ref-format=$from_format repo &&
>> +			test_commit -C repo initial &&
>> +			# we see that the repository contains reflogs.
>> +			test 2 = $(git -C repo reflog --all | wc -l) &&
>
> Nit: we don't want to have Git on the left-hand side of a pipe, as it
> might make use lose its exit code. This could instead be:
>
>     git -C repo reflog --all >reflogs &&
>     test_line_count = 2 reflogs
>

This looks cleaner.

>> +			test_migration repo "$to_format" true --skip-reflog &&
>> +			# there should be no reflogs post migration.
>> +			test 0 = $(git -C repo reflog --all | wc -l)
>
> And here we could use `test_must_be_empty`.
>

Will amend, Thanks for the review.

> Thanks!
>
> Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Justin Tobler]

On 25/02/07 12:57PM, Karthik Nayak wrote:
> The 'git-refs(1)' migrate subcommand, which transfers repositories
> between reference backends, currently migrates reflogs by default as of
> In 246cebe320 (refs: add support for migrating reflogs, 2024-12-16).

s/In 246cebe320/246cebe320/

> While this behavior is desirable for most client-side repositories,
> server-side repositories typically don't use reflogs and the migration
> of these entries is unnecessary overhead.
> 
> Add a '--skip-reflog' flag to the migrate subcommand to make reflog
> migration optional. This is particularly useful for server-side
> migrations where reflogs are not needed, improving migration performance
> in these scenarios.

Just to clarify, does a repository already without reflogs see improved
migration performance with this `--skip-reflog` flag? Or is the improved
performance soley due to repositories with reflogs skipping that part of
the migration?

> 
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> ---
>  builtin/refs.c          |  3 +++
>  refs.c                  |  8 +++++---
>  refs.h                  |  5 ++++-
>  t/t1460-refs-migrate.sh | 17 +++++++++++++++--
>  4 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/builtin/refs.c b/builtin/refs.c
> index a29f19583474518ee0942ea53c39cbdf9661c5e2..30be0254c14dd3d07693d70c25dddc9990756e9c 100644
> --- a/builtin/refs.c
> +++ b/builtin/refs.c
> @@ -30,6 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
>  		OPT_BIT(0, "dry-run", &flags,
>  			N_("perform a non-destructive dry-run"),
>  			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
> +		OPT_BIT(0, "skip-reflog", &flags,
> +			N_("skip migrating reflogs"),
> +			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
>  		OPT_END(),
>  	};
>  	struct strbuf errbuf = STRBUF_INIT;
> diff --git a/refs.c b/refs.c
> index f4094a326a9f88f979654b668cc9c3d27d83cb5d..5e8f5c06fa68d16c93ee11edd9742995eea994b6 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
>  	if (ret < 0)
>  		goto done;
>  
> -	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
> -	if (ret < 0)
> -		goto done;
> +	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
> +		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
> +		if (ret < 0)
> +			goto done;
> +	}

When the `REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG` flag is set, we
now skip over all the logs to perform the reflog migration. Makes sense.

-Justin

>  
>  	ret = ref_transaction_commit(transaction, errbuf);
>  	if (ret < 0)
> diff --git a/refs.h b/refs.h
> index a0cdd99250e8286b55808b697b0a94afac5d8319..ccee8fc6705e6e93a1c6234e7395180f8dfd815b 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -1157,8 +1157,11 @@ int is_pseudo_ref(const char *refname);
>   *   - REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN: perform a dry-run migration
>   *     without touching the main repository. The result will be written into a
>   *     temporary ref storage directory.
> + *
> + *   - REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG: skip migration of reflogs.
>   */
> -#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN (1 << 0)
> +#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN      (1 << 0)
> +#define REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG (1 << 1)
>  
>  /*
>   * Migrate the ref storage format used by the repository to the
> diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
> index a6d9b35a46eb59350aa0d59d982a2fbfaecf1448..9059d4c4121842a9d2e77dc4e54c537eeff8afab 100755
> --- a/t/t1460-refs-migrate.sh
> +++ b/t/t1460-refs-migrate.sh
> @@ -9,14 +9,16 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  
>  # Migrate the provided repository from one format to the other and
>  # verify that the references and logs are migrated over correctly.
> -# Usage: test_migration <repo> <format> <skip_reflog_verify>
> +# Usage: test_migration <repo> <format> <skip_reflog_verify> <...options>
>  #   <repo> is the relative path to the repo to be migrated.
>  #   <format> is the ref format to be migrated to.
>  #   <skip_reflog_verify> (true or false) whether to skip reflog verification.
> +#   <...options> are other options be passed directly to 'git refs migrate'.
>  test_migration () {
>  	repo=$1 &&
>  	format=$2 &&
>  	skip_reflog_verify=${3:-false} &&
> +	shift $(( $# >= 3 ? 3 : 2 )) &&
>  	git -C "$repo" for-each-ref --include-root-refs \
>  		--format='%(refname) %(objectname) %(symref)' >expect &&
>  	if ! $skip_reflog_verify
> @@ -25,7 +27,7 @@ test_migration () {
>  	   git -C "$repo" reflog list >expect_log_list
>  	fi &&
>  
> -	git -C "$repo" refs migrate --ref-format="$2" &&
> +	git -C "$repo" refs migrate --ref-format="$format" $@ &&
>  
>  	git -C "$repo" for-each-ref --include-root-refs \
>  		--format='%(refname) %(objectname) %(symref)' >actual &&
> @@ -241,6 +243,17 @@ do
>  				test_cmp expect.reflog actual.reflog
>  			)
>  		'
> +
> +		test_expect_success "$from_format -> $to_format: skip reflog with --skip-reflog" '
> +			test_when_finished "rm -rf repo" &&
> +			git init --ref-format=$from_format repo &&
> +			test_commit -C repo initial &&
> +			# we see that the repository contains reflogs.
> +			test 2 = $(git -C repo reflog --all | wc -l) &&
> +			test_migration repo "$to_format" true --skip-reflog &&
> +			# there should be no reflogs post migration.
> +			test 0 = $(git -C repo reflog --all | wc -l)
> +		'
>  	done
>  done
>  
> 
> ---
> 
> base-commit: bc204b742735ae06f65bb20291c95985c9633b7f
> change-id: 20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-8066f6df50ac
> 
> Thanks
> - Karthik
> 
> 

Re: [PATCH] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2861 bytes --]

Justin Tobler <jltobler@gmail.com> writes:

> On 25/02/07 12:57PM, Karthik Nayak wrote:
>> The 'git-refs(1)' migrate subcommand, which transfers repositories
>> between reference backends, currently migrates reflogs by default as of
>> In 246cebe320 (refs: add support for migrating reflogs, 2024-12-16).
>
> s/In 246cebe320/246cebe320/
>

Thanks.

>> While this behavior is desirable for most client-side repositories,
>> server-side repositories typically don't use reflogs and the migration
>> of these entries is unnecessary overhead.
>>
>> Add a '--skip-reflog' flag to the migrate subcommand to make reflog
>> migration optional. This is particularly useful for server-side
>> migrations where reflogs are not needed, improving migration performance
>> in these scenarios.
>
> Just to clarify, does a repository already without reflogs see improved
> migration performance with this `--skip-reflog` flag? Or is the improved
> performance soley due to repositories with reflogs skipping that part of
> the migration?
>

Since we iterate over all reflogs and add them, the perf gain would only
be for repositories which already have reflogs. Will modify accordingly.

>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> ---
>>  builtin/refs.c          |  3 +++
>>  refs.c                  |  8 +++++---
>>  refs.h                  |  5 ++++-
>>  t/t1460-refs-migrate.sh | 17 +++++++++++++++--
>>  4 files changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/builtin/refs.c b/builtin/refs.c
>> index a29f19583474518ee0942ea53c39cbdf9661c5e2..30be0254c14dd3d07693d70c25dddc9990756e9c 100644
>> --- a/builtin/refs.c
>> +++ b/builtin/refs.c
>> @@ -30,6 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
>>  		OPT_BIT(0, "dry-run", &flags,
>>  			N_("perform a non-destructive dry-run"),
>>  			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
>> +		OPT_BIT(0, "skip-reflog", &flags,
>> +			N_("skip migrating reflogs"),
>> +			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
>>  		OPT_END(),
>>  	};
>>  	struct strbuf errbuf = STRBUF_INIT;
>> diff --git a/refs.c b/refs.c
>> index f4094a326a9f88f979654b668cc9c3d27d83cb5d..5e8f5c06fa68d16c93ee11edd9742995eea994b6 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
>>  	if (ret < 0)
>>  		goto done;
>>
>> -	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
>> -	if (ret < 0)
>> -		goto done;
>> +	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
>> +		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
>> +		if (ret < 0)
>> +			goto done;
>> +	}
>
> When the `REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG` flag is set, we
> now skip over all the logs to perform the reflog migration. Makes sense.
>

Yup, thanks for the review.

> -Justin
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v2] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

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).

While this behavior is desirable for most client-side repositories,
server-side repositories are not expected to contain reflogs. However,
due to historical reasons, some may still have them. This could be
caused, for example, by bugs, misconfiguration, or an administrator
enabling reflogs on the server for debugging purposes.

To address this, introduce the --skip-reflog flag, allowing users to
bypass reflog migration. This ensures that the repository ends up in the
expected state after migration.

Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Changes in v2:
- Fix typo in commit mesasge and clarify the intent.
- Modify the test to use `test_line_count` and `test_must_be_empty`.
- Link to v1: https://lore.kernel.org/r/20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-v1-1-7d40f3b4e30b@gmail.com
---
Range-diff versus v1:

1:  ce14d3d07e ! 1:  6b83089348 builtin/refs: add '--skip-reflog' flag to bypass reflog migration
    @@ Commit message
     
         The 'git-refs(1)' migrate subcommand, which transfers repositories
         between reference backends, currently migrates reflogs by default as of
    -    In 246cebe320 (refs: add support for migrating reflogs, 2024-12-16).
    +    246cebe320 (refs: add support for migrating reflogs, 2024-12-16).
    +
         While this behavior is desirable for most client-side repositories,
    -    server-side repositories typically don't use reflogs and the migration
    -    of these entries is unnecessary overhead.
    +    server-side repositories are not expected to contain reflogs. However,
    +    due to historical reasons, some may still have them. This could be
    +    caused, for example, by bugs, misconfiguration, or an administrator
    +    enabling reflogs on the server for debugging purposes.
     
    -    Add a '--skip-reflog' flag to the migrate subcommand to make reflog
    -    migration optional. This is particularly useful for server-side
    -    migrations where reflogs are not needed, improving migration performance
    -    in these scenarios.
    +    To address this, introduce the --skip-reflog flag, allowing users to
    +    bypass reflog migration. This ensures that the repository ends up in the
    +    expected state after migration.
     
    +    Helped-by: Patrick Steinhardt <ps@pks.im>
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
      ## builtin/refs.c ##
    @@ t/t1460-refs-migrate.sh: do
     +			git init --ref-format=$from_format repo &&
     +			test_commit -C repo initial &&
     +			# we see that the repository contains reflogs.
    -+			test 2 = $(git -C repo reflog --all | wc -l) &&
    ++			git -C repo reflog --all >reflogs &&
    ++			test_line_count = 2 reflogs &&
     +			test_migration repo "$to_format" true --skip-reflog &&
     +			# there should be no reflogs post migration.
    -+			test 0 = $(git -C repo reflog --all | wc -l)
    ++			git -C repo reflog --all >reflogs &&
    ++			test_must_be_empty reflogs
     +		'
      	done
      done
---
 builtin/refs.c          |  3 +++
 refs.c                  |  8 +++++---
 refs.h                  |  5 ++++-
 t/t1460-refs-migrate.sh | 19 +++++++++++++++++--
 4 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/builtin/refs.c b/builtin/refs.c
index a29f19583474518ee0942ea53c39cbdf9661c5e2..30be0254c14dd3d07693d70c25dddc9990756e9c 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -30,6 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
 		OPT_BIT(0, "dry-run", &flags,
 			N_("perform a non-destructive dry-run"),
 			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
+		OPT_BIT(0, "skip-reflog", &flags,
+			N_("skip migrating reflogs"),
+			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
 		OPT_END(),
 	};
 	struct strbuf errbuf = STRBUF_INIT;
diff --git a/refs.c b/refs.c
index f4094a326a9f88f979654b668cc9c3d27d83cb5d..5e8f5c06fa68d16c93ee11edd9742995eea994b6 100644
--- a/refs.c
+++ b/refs.c
@@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
 	if (ret < 0)
 		goto done;
 
-	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
-	if (ret < 0)
-		goto done;
+	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
+		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
+		if (ret < 0)
+			goto done;
+	}
 
 	ret = ref_transaction_commit(transaction, errbuf);
 	if (ret < 0)
diff --git a/refs.h b/refs.h
index a0cdd99250e8286b55808b697b0a94afac5d8319..ccee8fc6705e6e93a1c6234e7395180f8dfd815b 100644
--- a/refs.h
+++ b/refs.h
@@ -1157,8 +1157,11 @@ int is_pseudo_ref(const char *refname);
  *   - REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN: perform a dry-run migration
  *     without touching the main repository. The result will be written into a
  *     temporary ref storage directory.
+ *
+ *   - REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG: skip migration of reflogs.
  */
-#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN (1 << 0)
+#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN      (1 << 0)
+#define REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG (1 << 1)
 
 /*
  * Migrate the ref storage format used by the repository to the
diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
index a6d9b35a46eb59350aa0d59d982a2fbfaecf1448..28c0024a4c8cb8282bf586840265edba442f5056 100755
--- a/t/t1460-refs-migrate.sh
+++ b/t/t1460-refs-migrate.sh
@@ -9,14 +9,16 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 # Migrate the provided repository from one format to the other and
 # verify that the references and logs are migrated over correctly.
-# Usage: test_migration <repo> <format> <skip_reflog_verify>
+# Usage: test_migration <repo> <format> <skip_reflog_verify> <...options>
 #   <repo> is the relative path to the repo to be migrated.
 #   <format> is the ref format to be migrated to.
 #   <skip_reflog_verify> (true or false) whether to skip reflog verification.
+#   <...options> are other options be passed directly to 'git refs migrate'.
 test_migration () {
 	repo=$1 &&
 	format=$2 &&
 	skip_reflog_verify=${3:-false} &&
+	shift $(( $# >= 3 ? 3 : 2 )) &&
 	git -C "$repo" for-each-ref --include-root-refs \
 		--format='%(refname) %(objectname) %(symref)' >expect &&
 	if ! $skip_reflog_verify
@@ -25,7 +27,7 @@ test_migration () {
 	   git -C "$repo" reflog list >expect_log_list
 	fi &&
 
-	git -C "$repo" refs migrate --ref-format="$2" &&
+	git -C "$repo" refs migrate --ref-format="$format" $@ &&
 
 	git -C "$repo" for-each-ref --include-root-refs \
 		--format='%(refname) %(objectname) %(symref)' >actual &&
@@ -241,6 +243,19 @@ do
 				test_cmp expect.reflog actual.reflog
 			)
 		'
+
+		test_expect_success "$from_format -> $to_format: skip reflog with --skip-reflog" '
+			test_when_finished "rm -rf repo" &&
+			git init --ref-format=$from_format repo &&
+			test_commit -C repo initial &&
+			# we see that the repository contains reflogs.
+			git -C repo reflog --all >reflogs &&
+			test_line_count = 2 reflogs &&
+			test_migration repo "$to_format" true --skip-reflog &&
+			# there should be no reflogs post migration.
+			git -C repo reflog --all >reflogs &&
+			test_must_be_empty reflogs
+		'
 	done
 done
 

---

base-commit: bc204b742735ae06f65bb20291c95985c9633b7f
change-id: 20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-8066f6df50ac

Thanks
- Karthik

Re: [PATCH v2] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Patrick Steinhardt]

On Tue, Feb 11, 2025 at 12:42:18PM +0100, Karthik Nayak wrote:
> 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).
> 
> While this behavior is desirable for most client-side repositories,
> server-side repositories are not expected to contain reflogs. However,
> due to historical reasons, some may still have them. This could be
> caused, for example, by bugs, misconfiguration, or an administrator
> enabling reflogs on the server for debugging purposes.
> 
> To address this, introduce the --skip-reflog flag, allowing users to
> bypass reflog migration. This ensures that the repository ends up in the
> expected state after migration.

Thanks, this version looks good to me.

Patrick

Re: [PATCH v2] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Junio C Hamano]

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).
>
> While this behavior is desirable for most client-side repositories,
> server-side repositories are not expected to contain reflogs. However,
> due to historical reasons, some may still have them. This could be
> caused, for example, by bugs, misconfiguration, or an administrator
> enabling reflogs on the server for debugging purposes.
>
> To address this, introduce the --skip-reflog flag, allowing users to
> bypass reflog migration. This ensures that the repository ends up in the
> expected state after migration.
>
> Helped-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> Changes in v2:
> - Fix typo in commit mesasge and clarify the intent.
> - Modify the test to use `test_line_count` and `test_must_be_empty`.
> - Link to v1: https://lore.kernel.org/r/20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-v1-1-7d40f3b4e30b@gmail.com
> ---
> Range-diff versus v1:
>
> 1:  ce14d3d07e ! 1:  6b83089348 builtin/refs: add '--skip-reflog' flag to bypass reflog migration
>     @@ Commit message
>      
>          The 'git-refs(1)' migrate subcommand, which transfers repositories
>          between reference backends, currently migrates reflogs by default as of
> ...
>      +		'
>       	done
>       done
> ---
>  builtin/refs.c          |  3 +++
>  refs.c                  |  8 +++++---
>  refs.h                  |  5 ++++-
>  t/t1460-refs-migrate.sh | 19 +++++++++++++++++--
>  4 files changed, 29 insertions(+), 6 deletions(-)

This is tangent that is totally unrelated to the theme of this
patch, but I find that the placement of range-diff makes it very
hard to follow.

After skimming the proposed log message, the next thing I would want
to see is the list of paths that are modified, before deciding I
want to review the patch now.  Once I decide to read it _now_, the
changes from the previous iteration and range-diff becomes relevant.

Is it just me who decides in what order to review the patches and
then reviews them in that order?

Anyway.

> diff --git a/builtin/refs.c b/builtin/refs.c
> index a29f19583474518ee0942ea53c39cbdf9661c5e2..30be0254c14dd3d07693d70c25dddc9990756e9c 100644
> --- a/builtin/refs.c
> +++ b/builtin/refs.c
> @@ -30,6 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
>  		OPT_BIT(0, "dry-run", &flags,
>  			N_("perform a non-destructive dry-run"),
>  			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
> +		OPT_BIT(0, "skip-reflog", &flags,
> +			N_("skip migrating reflogs"),
> +			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
>  		OPT_END(),
>  	};
>  	struct strbuf errbuf = STRBUF_INIT;

OK.

> diff --git a/refs.c b/refs.c
> index f4094a326a9f88f979654b668cc9c3d27d83cb5d..5e8f5c06fa68d16c93ee11edd9742995eea994b6 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
>  	if (ret < 0)
>  		goto done;
>  
> -	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
> -	if (ret < 0)
> -		goto done;
> +	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
> +		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
> +		if (ret < 0)
> +			goto done;
> +	}
>  
>  	ret = ref_transaction_commit(transaction, errbuf);
>  	if (ret < 0)
> diff --git a/refs.h b/refs.h
> index a0cdd99250e8286b55808b697b0a94afac5d8319..ccee8fc6705e6e93a1c6234e7395180f8dfd815b 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -1157,8 +1157,11 @@ int is_pseudo_ref(const char *refname);
>   *   - REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN: perform a dry-run migration
>   *     without touching the main repository. The result will be written into a
>   *     temporary ref storage directory.
> + *
> + *   - REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG: skip migration of reflogs.
>   */
> -#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN (1 << 0)
> +#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN      (1 << 0)
> +#define REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG (1 << 1)

These are quite a mouthful, but they are only used between the refs
subsystem and "git refs" command, so being overly verbose and
specific is a good thing.

>  /*
>   * Migrate the ref storage format used by the repository to the
> diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
> index a6d9b35a46eb59350aa0d59d982a2fbfaecf1448..28c0024a4c8cb8282bf586840265edba442f5056 100755
> --- a/t/t1460-refs-migrate.sh
> +++ b/t/t1460-refs-migrate.sh
> @@ -9,14 +9,16 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>  
>  # Migrate the provided repository from one format to the other and
>  # verify that the references and logs are migrated over correctly.
> -# Usage: test_migration <repo> <format> <skip_reflog_verify>
> +# Usage: test_migration <repo> <format> <skip_reflog_verify> <...options>
>  #   <repo> is the relative path to the repo to be migrated.
>  #   <format> is the ref format to be migrated to.
>  #   <skip_reflog_verify> (true or false) whether to skip reflog verification.
> +#   <...options> are other options be passed directly to 'git refs migrate'.

Yuck.  I've never seen preceding three-dots.  Makes readers wonder
how this thing behaves differently from the usual <options...>.

Doubly yuck is ...


>  test_migration () {
>  	repo=$1 &&
>  	format=$2 &&
>  	skip_reflog_verify=${3:-false} &&
> +	shift $(( $# >= 3 ? 3 : 2 )) &&

... this thing.  The above usage declares that <skip_reflog_verify>,
just like <repo> and <format>, are required so I do not see why we
need to be conditional here.  If you want to make it optional,
writing it this way instead ...

	repo=$1 &&
        format=$2 &&
	shift 2 &&

        skip_reflog_verify=false &&
	if test $# -gt 1
	then
		skip_reflog_verify=$1
		shift
	fi

... would make it easier to extend when you ever want to add the
next optional positional parameter.

>  	git -C "$repo" for-each-ref --include-root-refs \
>  		--format='%(refname) %(objectname) %(symref)' >expect &&
>  	if ! $skip_reflog_verify
> @@ -25,7 +27,7 @@ test_migration () {
>  	   git -C "$repo" reflog list >expect_log_list
>  	fi &&
>  
> -	git -C "$repo" refs migrate --ref-format="$2" &&
> +	git -C "$repo" refs migrate --ref-format="$format" $@ &&

It is a good change to use the named variable we captured upfront,
but did you mean to leave $@ unquoted?

Unless you want to use "$@" to invoke the magic "pass each parameter
separately, even if it has $IFS whitespace in it" semantics,
choosing between $* (when you want to split a parameter into many
when it has $IFS whitespaces) and "$*" (when you want everything in
one string) is preferred.

>  	git -C "$repo" for-each-ref --include-root-refs \
>  		--format='%(refname) %(objectname) %(symref)' >actual &&
> @@ -241,6 +243,19 @@ do
>  				test_cmp expect.reflog actual.reflog
>  			)
>  		'
> +
> +		test_expect_success "$from_format -> $to_format: skip reflog with --skip-reflog" '
> +			test_when_finished "rm -rf repo" &&
> +			git init --ref-format=$from_format repo &&
> +			test_commit -C repo initial &&
> +			# we see that the repository contains reflogs.
> +			git -C repo reflog --all >reflogs &&
> +			test_line_count = 2 reflogs &&
> +			test_migration repo "$to_format" true --skip-reflog &&
> +			# there should be no reflogs post migration.
> +			git -C repo reflog --all >reflogs &&
> +			test_must_be_empty reflogs
> +		'
>  	done
>  done
>  
>
> ---
>
> base-commit: bc204b742735ae06f65bb20291c95985c9633b7f
> change-id: 20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-8066f6df50ac
>
> Thanks
> - Karthik

Re: [PATCH v2] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 8553 bytes --]

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

> 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).
>>
>> While this behavior is desirable for most client-side repositories,
>> server-side repositories are not expected to contain reflogs. However,
>> due to historical reasons, some may still have them. This could be
>> caused, for example, by bugs, misconfiguration, or an administrator
>> enabling reflogs on the server for debugging purposes.
>>
>> To address this, introduce the --skip-reflog flag, allowing users to
>> bypass reflog migration. This ensures that the repository ends up in the
>> expected state after migration.
>>
>> Helped-by: Patrick Steinhardt <ps@pks.im>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> Changes in v2:
>> - Fix typo in commit mesasge and clarify the intent.
>> - Modify the test to use `test_line_count` and `test_must_be_empty`.
>> - Link to v1: https://lore.kernel.org/r/20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-v1-1-7d40f3b4e30b@gmail.com
>> ---
>> Range-diff versus v1:
>>
>> 1:  ce14d3d07e ! 1:  6b83089348 builtin/refs: add '--skip-reflog' flag to bypass reflog migration
>>     @@ Commit message
>>
>>          The 'git-refs(1)' migrate subcommand, which transfers repositories
>>          between reference backends, currently migrates reflogs by default as of
>> ...
>>      +		'
>>       	done
>>       done
>> ---
>>  builtin/refs.c          |  3 +++
>>  refs.c                  |  8 +++++---
>>  refs.h                  |  5 ++++-
>>  t/t1460-refs-migrate.sh | 19 +++++++++++++++++--
>>  4 files changed, 29 insertions(+), 6 deletions(-)
>
> This is tangent that is totally unrelated to the theme of this
> patch, but I find that the placement of range-diff makes it very
> hard to follow.
>
> After skimming the proposed log message, the next thing I would want
> to see is the list of paths that are modified, before deciding I
> want to review the patch now.  Once I decide to read it _now_, the
> changes from the previous iteration and range-diff becomes relevant.
>
> Is it just me who decides in what order to review the patches and
> then reviews them in that order?
>
> Anyway.
>
>> diff --git a/builtin/refs.c b/builtin/refs.c
>> index a29f19583474518ee0942ea53c39cbdf9661c5e2..30be0254c14dd3d07693d70c25dddc9990756e9c 100644
>> --- a/builtin/refs.c
>> +++ b/builtin/refs.c
>> @@ -30,6 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
>>  		OPT_BIT(0, "dry-run", &flags,
>>  			N_("perform a non-destructive dry-run"),
>>  			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
>> +		OPT_BIT(0, "skip-reflog", &flags,
>> +			N_("skip migrating reflogs"),
>> +			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
>>  		OPT_END(),
>>  	};
>>  	struct strbuf errbuf = STRBUF_INIT;
>
> OK.
>
>> diff --git a/refs.c b/refs.c
>> index f4094a326a9f88f979654b668cc9c3d27d83cb5d..5e8f5c06fa68d16c93ee11edd9742995eea994b6 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
>>  	if (ret < 0)
>>  		goto done;
>>
>> -	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
>> -	if (ret < 0)
>> -		goto done;
>> +	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
>> +		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
>> +		if (ret < 0)
>> +			goto done;
>> +	}
>>
>>  	ret = ref_transaction_commit(transaction, errbuf);
>>  	if (ret < 0)
>> diff --git a/refs.h b/refs.h
>> index a0cdd99250e8286b55808b697b0a94afac5d8319..ccee8fc6705e6e93a1c6234e7395180f8dfd815b 100644
>> --- a/refs.h
>> +++ b/refs.h
>> @@ -1157,8 +1157,11 @@ int is_pseudo_ref(const char *refname);
>>   *   - REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN: perform a dry-run migration
>>   *     without touching the main repository. The result will be written into a
>>   *     temporary ref storage directory.
>> + *
>> + *   - REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG: skip migration of reflogs.
>>   */
>> -#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN (1 << 0)
>> +#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN      (1 << 0)
>> +#define REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG (1 << 1)
>
> These are quite a mouthful, but they are only used between the refs
> subsystem and "git refs" command, so being overly verbose and
> specific is a good thing.
>
>>  /*
>>   * Migrate the ref storage format used by the repository to the
>> diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
>> index a6d9b35a46eb59350aa0d59d982a2fbfaecf1448..28c0024a4c8cb8282bf586840265edba442f5056 100755
>> --- a/t/t1460-refs-migrate.sh
>> +++ b/t/t1460-refs-migrate.sh
>> @@ -9,14 +9,16 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>>
>>  # Migrate the provided repository from one format to the other and
>>  # verify that the references and logs are migrated over correctly.
>> -# Usage: test_migration <repo> <format> <skip_reflog_verify>
>> +# Usage: test_migration <repo> <format> <skip_reflog_verify> <...options>
>>  #   <repo> is the relative path to the repo to be migrated.
>>  #   <format> is the ref format to be migrated to.
>>  #   <skip_reflog_verify> (true or false) whether to skip reflog verification.
>> +#   <...options> are other options be passed directly to 'git refs migrate'.
>
> Yuck.  I've never seen preceding three-dots.  Makes readers wonder
> how this thing behaves differently from the usual <options...>.
>
> Doubly yuck is ...
>

I will change it to <options...>. Thanks for pointing out.

>>  test_migration () {
>>  	repo=$1 &&
>>  	format=$2 &&
>>  	skip_reflog_verify=${3:-false} &&
>> +	shift $(( $# >= 3 ? 3 : 2 )) &&
>
> ... this thing.  The above usage declares that <skip_reflog_verify>,
> just like <repo> and <format>, are required so I do not see why we
> need to be conditional here.  If you want to make it optional,
> writing it this way instead ...
>

Right, so <skip_reflog_verify> and <options...> should be optional. So
let me change the description to be:

  Usage: test_migration <repo> <format> [<skip_reflog_verify> [<options...>]]

Which is more accurate.

> 	repo=$1 &&
>         format=$2 &&
> 	shift 2 &&
>
>         skip_reflog_verify=false &&
> 	if test $# -gt 1
> 	then
> 		skip_reflog_verify=$1
> 		shift
> 	fi
>
> ... would make it easier to extend when you ever want to add the
> next optional positional parameter.
>

Thanks this looks good too, let me use it :)

>>  	git -C "$repo" for-each-ref --include-root-refs \
>>  		--format='%(refname) %(objectname) %(symref)' >expect &&
>>  	if ! $skip_reflog_verify
>> @@ -25,7 +27,7 @@ test_migration () {
>>  	   git -C "$repo" reflog list >expect_log_list
>>  	fi &&
>>
>> -	git -C "$repo" refs migrate --ref-format="$2" &&
>> +	git -C "$repo" refs migrate --ref-format="$format" $@ &&
>
> It is a good change to use the named variable we captured upfront,
> but did you mean to leave $@ unquoted?
>

Good catch. I didn't mean to.

> Unless you want to use "$@" to invoke the magic "pass each parameter
> separately, even if it has $IFS whitespace in it" semantics,
> choosing between $* (when you want to split a parameter into many
> when it has $IFS whitespaces) and "$*" (when you want everything in
> one string) is preferred.
>

The intention is to keep the arguments as provided, so "$@" would be the
best choice here. Will change.

>>  	git -C "$repo" for-each-ref --include-root-refs \
>>  		--format='%(refname) %(objectname) %(symref)' >actual &&
>> @@ -241,6 +243,19 @@ do
>>  				test_cmp expect.reflog actual.reflog
>>  			)
>>  		'
>> +
>> +		test_expect_success "$from_format -> $to_format: skip reflog with --skip-reflog" '
>> +			test_when_finished "rm -rf repo" &&
>> +			git init --ref-format=$from_format repo &&
>> +			test_commit -C repo initial &&
>> +			# we see that the repository contains reflogs.
>> +			git -C repo reflog --all >reflogs &&
>> +			test_line_count = 2 reflogs &&
>> +			test_migration repo "$to_format" true --skip-reflog &&
>> +			# there should be no reflogs post migration.
>> +			git -C repo reflog --all >reflogs &&
>> +			test_must_be_empty reflogs
>> +		'
>>  	done
>>  done
>>
>>
>> ---
>>
>> base-commit: bc204b742735ae06f65bb20291c95985c9633b7f
>> change-id: 20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-8066f6df50ac
>>
>> Thanks
>> - Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v2] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2699 bytes --]

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

> 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).
>>
>> While this behavior is desirable for most client-side repositories,
>> server-side repositories are not expected to contain reflogs. However,
>> due to historical reasons, some may still have them. This could be
>> caused, for example, by bugs, misconfiguration, or an administrator
>> enabling reflogs on the server for debugging purposes.
>>
>> To address this, introduce the --skip-reflog flag, allowing users to
>> bypass reflog migration. This ensures that the repository ends up in the
>> expected state after migration.
>>
>> Helped-by: Patrick Steinhardt <ps@pks.im>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>> Changes in v2:
>> - Fix typo in commit mesasge and clarify the intent.
>> - Modify the test to use `test_line_count` and `test_must_be_empty`.
>> - Link to v1: https://lore.kernel.org/r/20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-v1-1-7d40f3b4e30b@gmail.com
>> ---
>> Range-diff versus v1:
>>
>> 1:  ce14d3d07e ! 1:  6b83089348 builtin/refs: add '--skip-reflog' flag to bypass reflog migration
>>     @@ Commit message
>>
>>          The 'git-refs(1)' migrate subcommand, which transfers repositories
>>          between reference backends, currently migrates reflogs by default as of
>> ...
>>      +		'
>>       	done
>>       done
>> ---
>>  builtin/refs.c          |  3 +++
>>  refs.c                  |  8 +++++---
>>  refs.h                  |  5 ++++-
>>  t/t1460-refs-migrate.sh | 19 +++++++++++++++++--
>>  4 files changed, 29 insertions(+), 6 deletions(-)
>
> This is tangent that is totally unrelated to the theme of this
> patch, but I find that the placement of range-diff makes it very
> hard to follow.
>
> After skimming the proposed log message, the next thing I would want
> to see is the list of paths that are modified, before deciding I
> want to review the patch now.  Once I decide to read it _now_, the
> changes from the previous iteration and range-diff becomes relevant.
>
> Is it just me who decides in what order to review the patches and
> then reviews them in that order?
>
> Anyway.
>

I missed responding to this in my previous email, but I've been using b4
for sending patches and this was primarily due to how it sends single
patch series.

I've fixed the template to show the diff-stat first, and this should be
the case for upcoming patches.

Thanks for pointing it out!

[snip]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

[PATCH v3] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

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).

While this behavior is desirable for most client-side repositories,
server-side repositories are not expected to contain reflogs. However,
due to historical reasons, some may still have them. This could be
caused, for example, by bugs, misconfiguration, or an administrator
enabling reflogs on the server for debugging purposes.

To address this, introduce the --skip-reflog flag, allowing users to
bypass reflog migration. This ensures that the repository ends up in the
expected state after migration.

Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/refs.c          |  3 +++
 refs.c                  |  8 +++++---
 refs.h                  |  5 ++++-
 t/t1460-refs-migrate.sh | 28 ++++++++++++++++++++++++----
 4 files changed, 36 insertions(+), 8 deletions(-)

Karthik Nayak (1):

builtin/refs: add '--skip-reflog' flag to bypass reflog migration
---
Changes in v3:
- Make changes to the test:
  - Use "$@" instead of $@
  - Mark optional arguments correctly
  - Use <options...> instead of <...options> as the former is more widely
    used.
- Link to v2: https://lore.kernel.org/r/20250211-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-v2-1-991a2ec9a796@gmail.com

Changes in v2:
- Fix typo in commit mesasge and clarify the intent.
- Modify the test to use `test_line_count` and `test_must_be_empty`.
- Link to v1: https://lore.kernel.org/r/20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-v1-1-7d40f3b4e30b@gmail.com

Range-diff versus v2:

1:  fcc313ad3b ! 1:  bbc3d154cf builtin/refs: add '--skip-reflog' flag to bypass reflog migration
    @@ t/t1460-refs-migrate.sh: export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
      # Migrate the provided repository from one format to the other and
      # verify that the references and logs are migrated over correctly.
     -# Usage: test_migration <repo> <format> <skip_reflog_verify>
    -+# Usage: test_migration <repo> <format> <skip_reflog_verify> <...options>
    ++# Usage: test_migration <repo> <format> [<skip_reflog_verify> [<options...>]]
      #   <repo> is the relative path to the repo to be migrated.
      #   <format> is the ref format to be migrated to.
    - #   <skip_reflog_verify> (true or false) whether to skip reflog verification.
    -+#   <...options> are other options be passed directly to 'git refs migrate'.
    +-#   <skip_reflog_verify> (true or false) whether to skip reflog verification.
    ++#   <skip_reflog_verify> (default: false) whether to skip reflog verification.
    ++#   <options...> are other options be passed directly to 'git refs migrate'.
      test_migration () {
      	repo=$1 &&
      	format=$2 &&
    - 	skip_reflog_verify=${3:-false} &&
    -+	shift $(( $# >= 3 ? 3 : 2 )) &&
    +-	skip_reflog_verify=${3:-false} &&
    ++	shift 2 &&
    ++	skip_reflog_verify=false &&
    ++	if test $# -ge 1
    ++	then
    ++		skip_reflog_verify=$1
    ++		shift
    ++	fi &&
      	git -C "$repo" for-each-ref --include-root-refs \
      		--format='%(refname) %(objectname) %(symref)' >expect &&
      	if ! $skip_reflog_verify
    @@ t/t1460-refs-migrate.sh: test_migration () {
      	fi &&
      
     -	git -C "$repo" refs migrate --ref-format="$2" &&
    -+	git -C "$repo" refs migrate --ref-format="$format" $@ &&
    ++	git -C "$repo" refs migrate --ref-format="$format" "$@" &&
      
      	git -C "$repo" for-each-ref --include-root-refs \
      		--format='%(refname) %(objectname) %(symref)' >actual &&


base-commit: bc204b742735ae06f65bb20291c95985c9633b7f

Thanks
- Karthik
---
 builtin/refs.c          |  3 +++
 refs.c                  |  8 +++++---
 refs.h                  |  5 ++++-
 t/t1460-refs-migrate.sh | 28 ++++++++++++++++++++++++----
 4 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/builtin/refs.c b/builtin/refs.c
index a29f19583474518ee0942ea53c39cbdf9661c5e2..30be0254c14dd3d07693d70c25dddc9990756e9c 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -30,6 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
 		OPT_BIT(0, "dry-run", &flags,
 			N_("perform a non-destructive dry-run"),
 			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
+		OPT_BIT(0, "skip-reflog", &flags,
+			N_("skip migrating reflogs"),
+			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
 		OPT_END(),
 	};
 	struct strbuf errbuf = STRBUF_INIT;
diff --git a/refs.c b/refs.c
index f4094a326a9f88f979654b668cc9c3d27d83cb5d..5e8f5c06fa68d16c93ee11edd9742995eea994b6 100644
--- a/refs.c
+++ b/refs.c
@@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
 	if (ret < 0)
 		goto done;
 
-	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
-	if (ret < 0)
-		goto done;
+	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
+		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
+		if (ret < 0)
+			goto done;
+	}
 
 	ret = ref_transaction_commit(transaction, errbuf);
 	if (ret < 0)
diff --git a/refs.h b/refs.h
index a0cdd99250e8286b55808b697b0a94afac5d8319..ccee8fc6705e6e93a1c6234e7395180f8dfd815b 100644
--- a/refs.h
+++ b/refs.h
@@ -1157,8 +1157,11 @@ int is_pseudo_ref(const char *refname);
  *   - REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN: perform a dry-run migration
  *     without touching the main repository. The result will be written into a
  *     temporary ref storage directory.
+ *
+ *   - REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG: skip migration of reflogs.
  */
-#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN (1 << 0)
+#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN      (1 << 0)
+#define REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG (1 << 1)
 
 /*
  * Migrate the ref storage format used by the repository to the
diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
index a6d9b35a46eb59350aa0d59d982a2fbfaecf1448..83466bde1865071ee633c738fd5c2ad9ae65bca3 100755
--- a/t/t1460-refs-migrate.sh
+++ b/t/t1460-refs-migrate.sh
@@ -9,14 +9,21 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 # Migrate the provided repository from one format to the other and
 # verify that the references and logs are migrated over correctly.
-# Usage: test_migration <repo> <format> <skip_reflog_verify>
+# Usage: test_migration <repo> <format> [<skip_reflog_verify> [<options...>]]
 #   <repo> is the relative path to the repo to be migrated.
 #   <format> is the ref format to be migrated to.
-#   <skip_reflog_verify> (true or false) whether to skip reflog verification.
+#   <skip_reflog_verify> (default: false) whether to skip reflog verification.
+#   <options...> are other options be passed directly to 'git refs migrate'.
 test_migration () {
 	repo=$1 &&
 	format=$2 &&
-	skip_reflog_verify=${3:-false} &&
+	shift 2 &&
+	skip_reflog_verify=false &&
+	if test $# -ge 1
+	then
+		skip_reflog_verify=$1
+		shift
+	fi &&
 	git -C "$repo" for-each-ref --include-root-refs \
 		--format='%(refname) %(objectname) %(symref)' >expect &&
 	if ! $skip_reflog_verify
@@ -25,7 +32,7 @@ test_migration () {
 	   git -C "$repo" reflog list >expect_log_list
 	fi &&
 
-	git -C "$repo" refs migrate --ref-format="$2" &&
+	git -C "$repo" refs migrate --ref-format="$format" "$@" &&
 
 	git -C "$repo" for-each-ref --include-root-refs \
 		--format='%(refname) %(objectname) %(symref)' >actual &&
@@ -241,6 +248,19 @@ do
 				test_cmp expect.reflog actual.reflog
 			)
 		'
+
+		test_expect_success "$from_format -> $to_format: skip reflog with --skip-reflog" '
+			test_when_finished "rm -rf repo" &&
+			git init --ref-format=$from_format repo &&
+			test_commit -C repo initial &&
+			# we see that the repository contains reflogs.
+			git -C repo reflog --all >reflogs &&
+			test_line_count = 2 reflogs &&
+			test_migration repo "$to_format" true --skip-reflog &&
+			# there should be no reflogs post migration.
+			git -C repo reflog --all >reflogs &&
+			test_must_be_empty reflogs
+		'
 	done
 done
 

Re: [PATCH v3] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Junio C Hamano]

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).
>
> While this behavior is desirable for most client-side repositories,
> server-side repositories are not expected to contain reflogs. However,
> due to historical reasons, some may still have them. This could be
> caused, for example, by bugs, misconfiguration, or an administrator
> enabling reflogs on the server for debugging purposes.
>
> To address this, introduce the --skip-reflog flag, allowing users to
> bypass reflog migration. This ensures that the repository ends up in the
> expected state after migration.

I do not quite understand the motivation behind this change.

If a repository has reflog that you do not need by mistake or
misconfiguration, I agree that there should be a way for you to
remove the reflog.  Removing it while converting the ref backend may
be a convenient way if and only if the reason why you noticed such a
repository with unwanted reflog is because you were about to migrate
it, but regardless of when you notice such refs with unwanted log,
you would want to be able to drop their logs.  You may not even be
planning to migrate your backend when you noticed that the refs have
unwanted log, you may have already migrated long time ago when you
noticed that the refs have unwanted log, or you may not even be
planning to migrate in the first place.  Even after you migrated
your backend, an administrator may have to enable reflog for
debugging, and then after the administrator is done, then what?
Should the backend migrated back from reftable to files and then
back again, only to pass this --skip-reflog option?

Wouldn't it be a lot more flexible if you add a new subcommand
"drop" to the "git reflog" command?

Re: [PATCH v3] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 5387 bytes --]

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

> 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).
>>
>> While this behavior is desirable for most client-side repositories,
>> server-side repositories are not expected to contain reflogs. However,
>> due to historical reasons, some may still have them. This could be
>> caused, for example, by bugs, misconfiguration, or an administrator
>> enabling reflogs on the server for debugging purposes.
>>
>> To address this, introduce the --skip-reflog flag, allowing users to
>> bypass reflog migration. This ensures that the repository ends up in the
>> expected state after migration.
>
> I do not quite understand the motivation behind this change.
>
> If a repository has reflog that you do not need by mistake or
> misconfiguration, I agree that there should be a way for you to
> remove the reflog.  Removing it while converting the ref backend may
> be a convenient way if and only if the reason why you noticed such a
> repository with unwanted reflog is because you were about to migrate
> it, but regardless of when you notice such refs with unwanted log,
> you would want to be able to drop their logs.  You may not even be
> planning to migrate your backend when you noticed that the refs have
> unwanted log, you may have already migrated long time ago when you
> noticed that the refs have unwanted log, or you may not even be
> planning to migrate in the first place.  Even after you migrated
> your backend, an administrator may have to enable reflog for
> debugging, and then after the administrator is done, then what?
> Should the backend migrated back from reftable to files and then
> back again, only to pass this --skip-reflog option?
>
> Wouldn't it be a lot more flexible if you add a new subcommand
> "drop" to the "git reflog" command?

> 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).
>>
>> While this behavior is desirable for most client-side repositories,
>> server-side repositories are not expected to contain reflogs. However,
>> due to historical reasons, some may still have them. This could be
>> caused, for example, by bugs, misconfiguration, or an administrator
>> enabling reflogs on the server for debugging purposes.
>>
>> To address this, introduce the --skip-reflog flag, allowing users to
>> bypass reflog migration. This ensures that the repository ends up in the
>> expected state after migration.
>
> I do not quite understand the motivation behind this change.
>
> If a repository has reflog that you do not need by mistake or
> misconfiguration, I agree that there should be a way for you to
> remove the reflog.  Removing it while converting the ref backend may
> be a convenient way if and only if the reason why you noticed such a
> repository with unwanted reflog is because you were about to migrate
> it, but regardless of when you notice such refs with unwanted log,
> you would want to be able to drop their logs.  You may not even be
> planning to migrate your backend when you noticed that the refs have
> unwanted log, you may have already migrated long time ago when you
> noticed that the refs have unwanted log, or you may not even be
> planning to migrate in the first place.  Even after you migrated
> your backend, an administrator may have to enable reflog for
> debugging, and then after the administrator is done, then what?
> Should the backend migrated back from reftable to files and then
> back again, only to pass this --skip-reflog option?
>
> Wouldn't it be a lot more flexible if you add a new subcommand
> "drop" to the "git reflog" command?

To just get rid of reflogs from a repository, I think 'git reflog drop'
or something similar would indeed be a better way to go about it. As you
stated, with this patch, we could still face the issue wherein the
administartor could re-enable reflog and we're back to square one.

Why I think this patch is important, is because while there could be
existing reflogs in a repository, if one doesn't care about _reflogs_
there could be significant performance gains while migrating repos from
one backend to the other, while also leaving the reflogs behind. The
intent of the patch is not to be a detterrent for reflogs. The `git refs
migrate` command is not the pathway for that. Rather, the goal is to
ensure we skip them during migraiton because we do not want to bother
migrating them.

Saying this, another option is to have `git reflog drop` and then
perform the migration, but I think it does makes sense to provide users
with a fine-tuning during migration to allow them to choose what they
want to migrate.

I know that in GitLab we have repositories with millions of references
and while we generally have reflogs disabled. There could be
repositories with reflogs enabled during debugging or just old
repositories where reflogs always existed. Since migrating backends
blocks all operations on the repository, it would be vital to also
optimize it as much as possible.

Thanks,
Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> To just get rid of reflogs from a repository, I think 'git reflog drop'
> or something similar would indeed be a better way to go about it. As you
> stated, with this patch, we could still face the issue wherein the
> administartor could re-enable reflog and we're back to square one.

Exactly.

> Why I think this patch is important, is because while there could be
> existing reflogs in a repository, if one doesn't care about _reflogs_
> there could be significant performance gains while migrating repos from
> one backend to the other, while also leaving the reflogs behind.

Sure.  It could be done with a combination of "git reflog drop &&
git refs migrate" (or "git refs migrate && git reflog drop", if the
migrated-to backend performs better when it drops reflogs).

With "git refs migrate --skip-reflog" alone, we are very limited.
We can lose reflogs _only_ when we are migrating.

Doing it _during_ migration may very well be more efficient than
dropping first and then migrate (or the other way around), so I do
not have much against the "migrate --skip-reflog" existing.  But I
find it backwards to add it first _before_ we have a tool that is
more generally applicable to wider situations, i.e., "reflog drop".

IOW, it feels as if we are worried about icing on the cake long
before we actually bake the cake.

Re: [PATCH v3] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]

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

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> To just get rid of reflogs from a repository, I think 'git reflog drop'
>> or something similar would indeed be a better way to go about it. As you
>> stated, with this patch, we could still face the issue wherein the
>> administartor could re-enable reflog and we're back to square one.
>
> Exactly.
>
>> Why I think this patch is important, is because while there could be
>> existing reflogs in a repository, if one doesn't care about _reflogs_
>> there could be significant performance gains while migrating repos from
>> one backend to the other, while also leaving the reflogs behind.
>
> Sure.  It could be done with a combination of "git reflog drop &&
> git refs migrate" (or "git refs migrate && git reflog drop", if the
> migrated-to backend performs better when it drops reflogs).
>
> With "git refs migrate --skip-reflog" alone, we are very limited.
> We can lose reflogs _only_ when we are migrating.
>
> Doing it _during_ migration may very well be more efficient than
> dropping first and then migrate (or the other way around), so I do
> not have much against the "migrate --skip-reflog" existing.  But I
> find it backwards to add it first _before_ we have a tool that is
> more generally applicable to wider situations, i.e., "reflog drop".
>
> IOW, it feels as if we are worried about icing on the cake long
> before we actually bake the cake.

I understand and agree. I do see a lot of benefits having 'git reflog
drop' too, so I'll pick it up and send a patch series towards the same
next (as soon as I send in the next version of the partial transactions
series :), which is taking me a bit of time with some other ongoing work
at $DAYJOB).

Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Toon Claes]

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).
>
> While this behavior is desirable for most client-side repositories,
> server-side repositories are not expected to contain reflogs. However,
> due to historical reasons, some may still have them. This could be
> caused, for example, by bugs, misconfiguration, or an administrator
> enabling reflogs on the server for debugging purposes.
>
> To address this, introduce the --skip-reflog flag, allowing users to
> bypass reflog migration. This ensures that the repository ends up in the
> expected state after migration.

It wasn't really obvious to me this change removes the reflog, instead
of "skipping". So I was surprised the reflog was removed after
migrating, instead of staying in tact using the files backend.

Only after reading through the test and the discussion in this thread I
realized that's the intented behavior.

So can I suggest to name the option `--no-reflog`? To me that makes it
more obvious the reflog won't exist no more after migrating, and is more
in line with the common UX of Git. Also emphasizing this more clearly in
the commit message and help message also would be advised.

-- 
Toon

Re: [PATCH v3] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Junio C Hamano]

Toon Claes <toon@iotcl.com> writes:

> So can I suggest to name the option `--no-reflog`? To me that makes it
> more obvious the reflog won't exist no more after migrating, and is more
> in line with the common UX of Git. Also emphasizing this more clearly in
> the commit message and help message also would be advised.

I have always thought, until I saw the message I am responding to,
that everybody would expect that "migrate --skip=X --skip=Y" that
usually migrates X and Y and Z would lose X and Y with the
transition.  But I realized that it was most likely because I happen
to know that the choice between reftable and files backends is
"which one do you take, you cannot have both at the same time", and
it was clear that "skip and keep using the old form" is not on the
table.  For all others, your interpretation of the option name is
entirely plausible.  So I agree `--no-reflog` is really an excellent
suggestion, even though `--reflog` option would be a no-op, and
`--no-refs` would be a "Huh?" option that only logically makes sense
to have for completeness but nobody would want to use ;-)

Re: [PATCH v3] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]

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

> Toon Claes <toon@iotcl.com> writes:
>
>> So can I suggest to name the option `--no-reflog`? To me that makes it
>> more obvious the reflog won't exist no more after migrating, and is more
>> in line with the common UX of Git. Also emphasizing this more clearly in
>> the commit message and help message also would be advised.
>
> I have always thought, until I saw the message I am responding to,
> that everybody would expect that "migrate --skip=X --skip=Y" that
> usually migrates X and Y and Z would lose X and Y with the
> transition.  But I realized that it was most likely because I happen
> to know that the choice between reftable and files backends is
> "which one do you take, you cannot have both at the same time", and
> it was clear that "skip and keep using the old form" is not on the
> table.  For all others, your interpretation of the option name is
> entirely plausible.  So I agree `--no-reflog` is really an excellent
> suggestion, even though `--reflog` option would be a no-op, and
> `--no-refs` would be a "Huh?" option that only logically makes sense
> to have for completeness but nobody would want to use ;-)

I share the same reaction. I didn't consider that flow of thought at
all. So I too agree with name change. Let me push in a new version.
Although I'm not sure if, Junio, you want to wait for the `git reflog
drop` command that we were discussing before accepting this topic [1].
I'll leave that to your discretion.

[1]: https://lore.kernel.org/all/xmqq4j0xpvmu.fsf@gitster.g/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

Re: [PATCH v3] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> I share the same reaction. I didn't consider that flow of thought at
> all. So I too agree with name change. Let me push in a new version.
> Although I'm not sure if, Junio, you want to wait for the `git reflog
> drop` command that we were discussing before accepting this topic [1].
> I'll leave that to your discretion.

Well, from my point of view, "reflog drop", if can be done for both
files and reftable without too much hassle, would be a greater
addition to the toolset than the value "you can drop while
migrating, but you need to remember to pass that option" gives us
;-)

So it really depends on how involved the work to add "drop" thing
would be.

Thanks.

Re: [PATCH v3] builtin/refs: add '--skip-reflog' flag to bypass reflog migration

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 921 bytes --]

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

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> I share the same reaction. I didn't consider that flow of thought at
>> all. So I too agree with name change. Let me push in a new version.
>> Although I'm not sure if, Junio, you want to wait for the `git reflog
>> drop` command that we were discussing before accepting this topic [1].
>> I'll leave that to your discretion.
>
> Well, from my point of view, "reflog drop", if can be done for both
> files and reftable without too much hassle, would be a greater
> addition to the toolset than the value "you can drop while
> migrating, but you need to remember to pass that option" gives us
> ;-)
>

I'm still dedicated to pick it up as the next topic that I'd be working
on. So overall I would like both of these topics to land.

> So it really depends on how involved the work to add "drop" thing
> would be.
>
> Thanks.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

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

[Karthik Nayak]

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).

While this behavior is desirable for most client-side repositories,
server-side repositories are not expected to contain reflogs. However,
due to historical reasons, some may still have them. This could be
caused, for example, by bugs, misconfiguration, or an administrator
enabling reflogs on the server for debugging purposes.

To handle this, introduce the '--no-reflog' flag, which skips reflog
migration. When this flag is used, reflogs from the original reference
backend are not transferred, and since only the new reference backend
remains in the repository, all previous reflogs are permanently removed.

Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 builtin/refs.c          |  3 +++
 refs.c                  |  8 +++++---
 refs.h                  |  5 ++++-
 t/t1460-refs-migrate.sh | 28 ++++++++++++++++++++++++----
 4 files changed, 36 insertions(+), 8 deletions(-)

Changes in v4:
- Modify the flag to `--no-reflog` to better indicate that reflogs will
  be dropped during the migration. This is also reflected in the help text
  and the commit message.

Changes in v3:
- Make changes to the test:
  - Use "$@" instead of $@
  - Mark optional arguments correctly
  - Use <options...> instead of <...options> as the former is more widely
    used.
- Link to v2: https://lore.kernel.org/r/20250211-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-v2-1-991a2ec9a796@gmail.com

Changes in v2:
- Fix typo in commit mesasge and clarify the intent.
- Modify the test to use `test_line_count` and `test_must_be_empty`.
- Link to v1: https://lore.kernel.org/r/20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-v1-1-7d40f3b4e30b@gmail.com

Range-diff:

1:  42c40d9617 ! 1:  3a02e8e526 builtin/refs: add '--skip-reflog' flag to bypass reflog migration
    @@ Metadata
     Author: Karthik Nayak <karthik.188@gmail.com>
     
      ## Commit message ##
    -    builtin/refs: add '--skip-reflog' flag to bypass reflog migration
    +    builtin/refs: add '--no-reflog' flag to drop reflogs
     
         The 'git-refs(1)' migrate subcommand, which transfers repositories
         between reference backends, currently migrates reflogs by default as of
    @@ Commit message
         caused, for example, by bugs, misconfiguration, or an administrator
         enabling reflogs on the server for debugging purposes.
     
    -    To address this, introduce the --skip-reflog flag, allowing users to
    -    bypass reflog migration. This ensures that the repository ends up in the
    -    expected state after migration.
    +    To handle this, introduce the '--no-reflog' flag, which skips reflog
    +    migration. When this flag is used, reflogs from the original reference
    +    backend are not transferred, and since only the new reference backend
    +    remains in the repository, all previous reflogs are permanently removed.
     
         Helped-by: Patrick Steinhardt <ps@pks.im>
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
    @@ builtin/refs.c: static int cmd_refs_migrate(int argc, const char **argv, const c
      		OPT_BIT(0, "dry-run", &flags,
      			N_("perform a non-destructive dry-run"),
      			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
    -+		OPT_BIT(0, "skip-reflog", &flags,
    -+			N_("skip migrating reflogs"),
    ++		OPT_BIT(0, "no-reflog", &flags,
    ++			N_("drop reflogs entirely during the migration"),
     +			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
      		OPT_END(),
      	};
    @@ t/t1460-refs-migrate.sh: do
     +			# we see that the repository contains reflogs.
     +			git -C repo reflog --all >reflogs &&
     +			test_line_count = 2 reflogs &&
    -+			test_migration repo "$to_format" true --skip-reflog &&
    ++			test_migration repo "$to_format" true --no-reflog &&
     +			# there should be no reflogs post migration.
     +			git -C repo reflog --all >reflogs &&
     +			test_must_be_empty reflogs

---

diff --git a/builtin/refs.c b/builtin/refs.c
index a29f195834..c459507d51 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -30,6 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
 		OPT_BIT(0, "dry-run", &flags,
 			N_("perform a non-destructive dry-run"),
 			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
+		OPT_BIT(0, "no-reflog", &flags,
+			N_("drop reflogs entirely during the migration"),
+			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
 		OPT_END(),
 	};
 	struct strbuf errbuf = STRBUF_INIT;
diff --git a/refs.c b/refs.c
index f4094a326a..5e8f5c06fa 100644
--- a/refs.c
+++ b/refs.c
@@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
 	if (ret < 0)
 		goto done;
 
-	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
-	if (ret < 0)
-		goto done;
+	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
+		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
+		if (ret < 0)
+			goto done;
+	}
 
 	ret = ref_transaction_commit(transaction, errbuf);
 	if (ret < 0)
diff --git a/refs.h b/refs.h
index a0cdd99250..ccee8fc670 100644
--- a/refs.h
+++ b/refs.h
@@ -1157,8 +1157,11 @@ int is_pseudo_ref(const char *refname);
  *   - REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN: perform a dry-run migration
  *     without touching the main repository. The result will be written into a
  *     temporary ref storage directory.
+ *
+ *   - REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG: skip migration of reflogs.
  */
-#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN (1 << 0)
+#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN      (1 << 0)
+#define REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG (1 << 1)
 
 /*
  * Migrate the ref storage format used by the repository to the
diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
index a6d9b35a46..2ab97e1b7d 100755
--- a/t/t1460-refs-migrate.sh
+++ b/t/t1460-refs-migrate.sh
@@ -9,14 +9,21 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 # Migrate the provided repository from one format to the other and
 # verify that the references and logs are migrated over correctly.
-# Usage: test_migration <repo> <format> <skip_reflog_verify>
+# Usage: test_migration <repo> <format> [<skip_reflog_verify> [<options...>]]
 #   <repo> is the relative path to the repo to be migrated.
 #   <format> is the ref format to be migrated to.
-#   <skip_reflog_verify> (true or false) whether to skip reflog verification.
+#   <skip_reflog_verify> (default: false) whether to skip reflog verification.
+#   <options...> are other options be passed directly to 'git refs migrate'.
 test_migration () {
 	repo=$1 &&
 	format=$2 &&
-	skip_reflog_verify=${3:-false} &&
+	shift 2 &&
+	skip_reflog_verify=false &&
+	if test $# -ge 1
+	then
+		skip_reflog_verify=$1
+		shift
+	fi &&
 	git -C "$repo" for-each-ref --include-root-refs \
 		--format='%(refname) %(objectname) %(symref)' >expect &&
 	if ! $skip_reflog_verify
@@ -25,7 +32,7 @@ test_migration () {
 	   git -C "$repo" reflog list >expect_log_list
 	fi &&
 
-	git -C "$repo" refs migrate --ref-format="$2" &&
+	git -C "$repo" refs migrate --ref-format="$format" "$@" &&
 
 	git -C "$repo" for-each-ref --include-root-refs \
 		--format='%(refname) %(objectname) %(symref)' >actual &&
@@ -241,6 +248,19 @@ do
 				test_cmp expect.reflog actual.reflog
 			)
 		'
+
+		test_expect_success "$from_format -> $to_format: skip reflog with --skip-reflog" '
+			test_when_finished "rm -rf repo" &&
+			git init --ref-format=$from_format repo &&
+			test_commit -C repo initial &&
+			# we see that the repository contains reflogs.
+			git -C repo reflog --all >reflogs &&
+			test_line_count = 2 reflogs &&
+			test_migration repo "$to_format" true --no-reflog &&
+			# there should be no reflogs post migration.
+			git -C repo reflog --all >reflogs &&
+			test_must_be_empty reflogs
+		'
 	done
 done
 
-- 
2.47.2

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

[Junio C Hamano]

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?

>  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.

> 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?

 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]
+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::

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

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 2873 bytes --]

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

> 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 :)

>>  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.

>
>> 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.

>
>  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]'.

> +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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]

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

[Karthik Nayak]

The "git refs migrate" subcommand converts the backend used for ref
storage. It always migrates reflog data as well as refs. Introduce an
option to exclude reflogs from migration, allowing them to be discarded
when they are unnecessary.

This is particularly useful in server-side repositories, where reflogs
are typically not expected. However, some repositories may still have
them due to historical reasons, such as bugs, misconfigurations, or
administrative decisions to enable reflogs for debugging. In such
repositories, it would be optimal to drop reflogs during the migration.

To address this, introduce the '--no-reflog' flag, which prevents reflog
migration. When this flag is used, reflogs from the original reference
backend are migrated. Since only the new reference backend remains in
the repository, all previous reflogs are permanently discarded.

Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
 Documentation/git-refs.txt | 11 ++++++++---
 builtin/refs.c             |  3 +++
 refs.c                     |  8 +++++---
 refs.h                     |  5 ++++-
 t/t1460-refs-migrate.sh    | 28 ++++++++++++++++++++++++----
 5 files changed, 44 insertions(+), 11 deletions(-)

Changes in v5:
- Add missing documentation and cleanup the commit message.
- I also see that 'git-refs.txt' in master has been renamed to 'git-refs.adoc',
  but I'm going to avoid rebasing on latest master, since the resolution is
  quite simple here. Happy to do it if needed. 

Changes in v4:
- Modify the flag to `--no-reflog` to better indicate that reflogs will
  be dropped during the migration. This is also reflected in the help text
  and the commit message.

Changes in v3:
- Make changes to the test:
  - Use "$@" instead of $@
  - Mark optional arguments correctly
  - Use <options...> instead of <...options> as the former is more widely
    used.
- Link to v2: https://lore.kernel.org/r/20250211-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-v2-1-991a2ec9a796@gmail.com

Changes in v2:
- Fix typo in commit mesasge and clarify the intent.
- Modify the test to use `test_line_count` and `test_must_be_empty`.
- Link to v1: https://lore.kernel.org/r/20250207-477-refs-migrate-add-a-flag-to-ignore-reflogs-during-migration-v1-1-7d40f3b4e30b@gmail.com

Range-diff:

1:  3a02e8e526 ! 1:  f0c11e6a4d builtin/refs: add '--no-reflog' flag to drop reflogs
    @@ Metadata
      ## Commit message ##
         builtin/refs: add '--no-reflog' flag to drop reflogs
     
    -    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).
    +    The "git refs migrate" subcommand converts the backend used for ref
    +    storage. It always migrates reflog data as well as refs. Introduce an
    +    option to exclude reflogs from migration, allowing them to be discarded
    +    when they are unnecessary.
     
    -    While this behavior is desirable for most client-side repositories,
    -    server-side repositories are not expected to contain reflogs. However,
    -    due to historical reasons, some may still have them. This could be
    -    caused, for example, by bugs, misconfiguration, or an administrator
    -    enabling reflogs on the server for debugging purposes.
    +    This is particularly useful in server-side repositories, where reflogs
    +    are typically not expected. However, some repositories may still have
    +    them due to historical reasons, such as bugs, misconfigurations, or
    +    administrative decisions to enable reflogs for debugging. In such
    +    repositories, it would be optimal to drop reflogs during the migration.
     
    -    To handle this, introduce the '--no-reflog' flag, which skips reflog
    +    To address this, introduce the '--no-reflog' flag, which prevents reflog
         migration. When this flag is used, reflogs from the original reference
    -    backend are not transferred, and since only the new reference backend
    -    remains in the repository, all previous reflogs are permanently removed.
    +    backend are migrated. Since only the new reference backend remains in
    +    the repository, all previous reflogs are permanently discarded.
     
    +    Helped-by: Junio C Hamano <gitster@pobox.com>
         Helped-by: Patrick Steinhardt <ps@pks.im>
         Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
     
    + ## Documentation/git-refs.txt ##
    +@@ Documentation/git-refs.txt: git-refs - Low-level access to refs
    + 
    + SYNOPSIS
    + --------
    +-[verse]
    +-'git refs migrate' --ref-format=<format> [--dry-run]
    +-'git refs verify' [--strict] [--verbose]
    ++[synopsis]
    ++git refs migrate --ref-format=<format> [--no-reflog] [--dry-run]
    ++git refs verify [--strict] [--verbose]
    + 
    + DESCRIPTION
    + -----------
    +@@ Documentation/git-refs.txt: 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::
    +
      ## builtin/refs.c ##
     @@ builtin/refs.c: static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
      		OPT_BIT(0, "dry-run", &flags,

---

diff --git a/Documentation/git-refs.txt b/Documentation/git-refs.txt
index 9829984b0a..733ada7d51 100644
--- a/Documentation/git-refs.txt
+++ b/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]
+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::
diff --git a/builtin/refs.c b/builtin/refs.c
index a29f195834..c459507d51 100644
--- a/builtin/refs.c
+++ b/builtin/refs.c
@@ -30,6 +30,9 @@ static int cmd_refs_migrate(int argc, const char **argv, const char *prefix,
 		OPT_BIT(0, "dry-run", &flags,
 			N_("perform a non-destructive dry-run"),
 			REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN),
+		OPT_BIT(0, "no-reflog", &flags,
+			N_("drop reflogs entirely during the migration"),
+			REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG),
 		OPT_END(),
 	};
 	struct strbuf errbuf = STRBUF_INIT;
diff --git a/refs.c b/refs.c
index f4094a326a..5e8f5c06fa 100644
--- a/refs.c
+++ b/refs.c
@@ -3035,9 +3035,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
 	if (ret < 0)
 		goto done;
 
-	ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
-	if (ret < 0)
-		goto done;
+	if (!(flags & REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG)) {
+		ret = refs_for_each_reflog(old_refs, migrate_one_reflog, &data);
+		if (ret < 0)
+			goto done;
+	}
 
 	ret = ref_transaction_commit(transaction, errbuf);
 	if (ret < 0)
diff --git a/refs.h b/refs.h
index a0cdd99250..ccee8fc670 100644
--- a/refs.h
+++ b/refs.h
@@ -1157,8 +1157,11 @@ int is_pseudo_ref(const char *refname);
  *   - REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN: perform a dry-run migration
  *     without touching the main repository. The result will be written into a
  *     temporary ref storage directory.
+ *
+ *   - REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG: skip migration of reflogs.
  */
-#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN (1 << 0)
+#define REPO_MIGRATE_REF_STORAGE_FORMAT_DRYRUN      (1 << 0)
+#define REPO_MIGRATE_REF_STORAGE_FORMAT_SKIP_REFLOG (1 << 1)
 
 /*
  * Migrate the ref storage format used by the repository to the
diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
index a6d9b35a46..2ab97e1b7d 100755
--- a/t/t1460-refs-migrate.sh
+++ b/t/t1460-refs-migrate.sh
@@ -9,14 +9,21 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
 # Migrate the provided repository from one format to the other and
 # verify that the references and logs are migrated over correctly.
-# Usage: test_migration <repo> <format> <skip_reflog_verify>
+# Usage: test_migration <repo> <format> [<skip_reflog_verify> [<options...>]]
 #   <repo> is the relative path to the repo to be migrated.
 #   <format> is the ref format to be migrated to.
-#   <skip_reflog_verify> (true or false) whether to skip reflog verification.
+#   <skip_reflog_verify> (default: false) whether to skip reflog verification.
+#   <options...> are other options be passed directly to 'git refs migrate'.
 test_migration () {
 	repo=$1 &&
 	format=$2 &&
-	skip_reflog_verify=${3:-false} &&
+	shift 2 &&
+	skip_reflog_verify=false &&
+	if test $# -ge 1
+	then
+		skip_reflog_verify=$1
+		shift
+	fi &&
 	git -C "$repo" for-each-ref --include-root-refs \
 		--format='%(refname) %(objectname) %(symref)' >expect &&
 	if ! $skip_reflog_verify
@@ -25,7 +32,7 @@ test_migration () {
 	   git -C "$repo" reflog list >expect_log_list
 	fi &&
 
-	git -C "$repo" refs migrate --ref-format="$2" &&
+	git -C "$repo" refs migrate --ref-format="$format" "$@" &&
 
 	git -C "$repo" for-each-ref --include-root-refs \
 		--format='%(refname) %(objectname) %(symref)' >actual &&
@@ -241,6 +248,19 @@ do
 				test_cmp expect.reflog actual.reflog
 			)
 		'
+
+		test_expect_success "$from_format -> $to_format: skip reflog with --skip-reflog" '
+			test_when_finished "rm -rf repo" &&
+			git init --ref-format=$from_format repo &&
+			test_commit -C repo initial &&
+			# we see that the repository contains reflogs.
+			git -C repo reflog --all >reflogs &&
+			test_line_count = 2 reflogs &&
+			test_migration repo "$to_format" true --no-reflog &&
+			# there should be no reflogs post migration.
+			git -C repo reflog --all >reflogs &&
+			test_must_be_empty reflogs
+		'
 	done
 done
 
-- 
2.47.2

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

[Junio C Hamano]

Karthik Nayak <karthik.188@gmail.com> writes:

> The "git refs migrate" subcommand converts the backend used for ref
> storage. It always migrates reflog data as well as refs. Introduce an
> option to exclude reflogs from migration, allowing them to be discarded
> when they are unnecessary.
>
> This is particularly useful in server-side repositories, where reflogs
> are typically not expected. However, some repositories may still have
> them due to historical reasons, such as bugs, misconfigurations, or
> administrative decisions to enable reflogs for debugging. In such
> repositories, it would be optimal to drop reflogs during the migration.
>
> To address this, introduce the '--no-reflog' flag, which prevents reflog
> migration. When this flag is used, reflogs from the original reference
> backend are migrated. Since only the new reference backend remains in
> the repository, all previous reflogs are permanently discarded.
>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Helped-by: Patrick Steinhardt <ps@pks.im>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
>  Documentation/git-refs.txt | 11 ++++++++---
>  builtin/refs.c             |  3 +++
>  refs.c                     |  8 +++++---
>  refs.h                     |  5 ++++-
>  t/t1460-refs-migrate.sh    | 28 ++++++++++++++++++++++++----
>  5 files changed, 44 insertions(+), 11 deletions(-)
>
> Changes in v5:
> - Add missing documentation and cleanup the commit message.
> - I also see that 'git-refs.txt' in master has been renamed to 'git-refs.adoc',
>   but I'm going to avoid rebasing on latest master, since the resolution is
>   quite simple here. Happy to do it if needed. 

It is a good rule of thumb to refrain from rebasing when in doubt.

Shall we declare victory and mark the topic for 'next' by now?

Thanks.

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

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1931 bytes --]

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

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> The "git refs migrate" subcommand converts the backend used for ref
>> storage. It always migrates reflog data as well as refs. Introduce an
>> option to exclude reflogs from migration, allowing them to be discarded
>> when they are unnecessary.
>>
>> This is particularly useful in server-side repositories, where reflogs
>> are typically not expected. However, some repositories may still have
>> them due to historical reasons, such as bugs, misconfigurations, or
>> administrative decisions to enable reflogs for debugging. In such
>> repositories, it would be optimal to drop reflogs during the migration.
>>
>> To address this, introduce the '--no-reflog' flag, which prevents reflog
>> migration. When this flag is used, reflogs from the original reference
>> backend are migrated. Since only the new reference backend remains in
>> the repository, all previous reflogs are permanently discarded.
>>
>> Helped-by: Junio C Hamano <gitster@pobox.com>
>> Helped-by: Patrick Steinhardt <ps@pks.im>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>  Documentation/git-refs.txt | 11 ++++++++---
>>  builtin/refs.c             |  3 +++
>>  refs.c                     |  8 +++++---
>>  refs.h                     |  5 ++++-
>>  t/t1460-refs-migrate.sh    | 28 ++++++++++++++++++++++++----
>>  5 files changed, 44 insertions(+), 11 deletions(-)
>>
>> Changes in v5:
>> - Add missing documentation and cleanup the commit message.
>> - I also see that 'git-refs.txt' in master has been renamed to 'git-refs.adoc',
>>   but I'm going to avoid rebasing on latest master, since the resolution is
>>   quite simple here. Happy to do it if needed.
>
> It is a good rule of thumb to refrain from rebasing when in doubt.
>
> Shall we declare victory and mark the topic for 'next' by now?
>
> Thanks.

That would be nice!

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]