git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] refs: fix migration of reflogs respecting "core.logAllRefUpdates"
@ 2025-01-22  9:48 Patrick Steinhardt
  2025-01-22 18:02 ` Junio C Hamano
  0 siblings, 1 reply; 2+ messages in thread
From: Patrick Steinhardt @ 2025-01-22  9:48 UTC (permalink / raw)
  To: git; +Cc: brian m. carlson, Karthik Nayak

In 246cebe320 (refs: add support for migrating reflogs, 2024-12-16) we
have added support to git-refs(1) to migrate reflogs between reference
backends. It was reported [1] though that not we don't migrate reflogs
for a subset of references, most importantly "refs/stash".

This issue is caused by us still honoring "core.logAllRefUpdates" when
trying to migrate reflogs: we do queue the updates, but depending on the
value of that config we may decide to just skip writing the reflog entry
altogether. And given that:

  - The default for "core.logAllRefUpdates" is to only create reflogs
    for branches, remotes, note refs and "HEAD"

  - "refs/stash" is neither of these ref types.

We end up skipping the reflog creation for that particular reference.

Fix the bug by setting `REF_FORCE_CREATE_REFLOG`, which instructs the
ref backends to create the reflog entry regardless of the config or any
preexisting state.

[1]: <Z5BTQRlsOj1sygun@tapette.crustytoothpaste.net>

Reported-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
Hi,

this patch addresses the issue reported by brian where "refs/stash"
isn't getting migrated correctly. The patch is based on top of "maint"
so that it can be easily backported.

Thanks for the report!

Patrick
---
 refs.c                  |  2 +-
 t/t1460-refs-migrate.sh | 17 +++++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index 0f41b2fd4a..37b8cfb90c 100644
--- a/refs.c
+++ b/refs.c
@@ -1330,7 +1330,7 @@ int ref_transaction_update_reflog(struct ref_transaction *transaction,
 
 	assert(err);
 
-	flags |= REF_LOG_ONLY | REF_NO_DEREF;
+	flags |= REF_LOG_ONLY | REF_FORCE_CREATE_REFLOG | REF_NO_DEREF;
 
 	if (!transaction_refname_valid(refname, new_oid, flags, err))
 		return -1;
diff --git a/t/t1460-refs-migrate.sh b/t/t1460-refs-migrate.sh
index f59bc4860f..ceb0c4977d 100755
--- a/t/t1460-refs-migrate.sh
+++ b/t/t1460-refs-migrate.sh
@@ -224,6 +224,23 @@ do
 			test_commit --date "100003000 +0700" --no-tag -C repo second &&
 			test_migration repo "$to_format"
 		'
+
+		test_expect_success "$from_format -> $to_format: stash is retained" '
+			test_when_finished "rm -rf repo" &&
+			git init --ref-format=$from_format repo &&
+			(
+				cd repo &&
+				test_commit initial A &&
+				echo foo >A &&
+				git stash push &&
+				echo bar >A &&
+				git stash push &&
+				git stash list >expect.reflog &&
+				test_migration . "$to_format" &&
+				git stash list >actual.reflog &&
+				test_cmp expect.reflog actual.reflog
+			)
+		'
 	done
 done
 

---
base-commit: f93ff170b93a1782659637824b25923245ac9dd1
change-id: 20250122-b4-pks-reflog-migration-fix-stash-d1fe7380f84a


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] refs: fix migration of reflogs respecting "core.logAllRefUpdates"
  2025-01-22  9:48 [PATCH] refs: fix migration of reflogs respecting "core.logAllRefUpdates" Patrick Steinhardt
@ 2025-01-22 18:02 ` Junio C Hamano
  0 siblings, 0 replies; 2+ messages in thread
From: Junio C Hamano @ 2025-01-22 18:02 UTC (permalink / raw)
  To: Patrick Steinhardt; +Cc: git, brian m. carlson, Karthik Nayak

Patrick Steinhardt <ps@pks.im> writes:

> In 246cebe320 (refs: add support for migrating reflogs, 2024-12-16) we
> have added support to git-refs(1) to migrate reflogs between reference
> backends. It was reported [1] though that not we don't migrate reflogs
> for a subset of references, most importantly "refs/stash".
>
> This issue is caused by us still honoring "core.logAllRefUpdates" when
> trying to migrate reflogs: we do queue the updates, but depending on the
> value of that config we may decide to just skip writing the reflog entry
> altogether. And given that:
>
>   - The default for "core.logAllRefUpdates" is to only create reflogs
>     for branches, remotes, note refs and "HEAD"
>
>   - "refs/stash" is neither of these ref types.
>
> We end up skipping the reflog creation for that particular reference.
>
> Fix the bug by setting `REF_FORCE_CREATE_REFLOG`, which instructs the
> ref backends to create the reflog entry regardless of the config or any
> preexisting state.

Thanks for a clear problem analysis description.  The appraoch makes
perfect sense.

Will queue.

> +		test_expect_success "$from_format -> $to_format: stash is retained" '
> +			test_when_finished "rm -rf repo" &&
> +			git init --ref-format=$from_format repo &&
> +			(
> +				cd repo &&
> +				test_commit initial A &&
> +				echo foo >A &&
> +				git stash push &&
> +				echo bar >A &&
> +				git stash push &&
> +				git stash list >expect.reflog &&
> +				test_migration . "$to_format" &&
> +				git stash list >actual.reflog &&
> +				test_cmp expect.reflog actual.reflog
> +			)
> +		'
>  	done
>  done
>  
>
> ---
> base-commit: f93ff170b93a1782659637824b25923245ac9dd1
> change-id: 20250122-b4-pks-reflog-migration-fix-stash-d1fe7380f84a

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-01-22 18:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-22  9:48 [PATCH] refs: fix migration of reflogs respecting "core.logAllRefUpdates" Patrick Steinhardt
2025-01-22 18:02 ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).