@matheusbsbs / matheus-bagliano / commits / 09c817383f

refs: fix format migration on Cygwin

It was reported that t1460-refs-migrate.sh fails when using Cygwin with errors like the following: error: could not link file '.git/ref_migration.sr9pEF/reftable' to '.git/reftable': Permission denied As some debugging surfaced, the root cause of this is that some files of the newly-initialized ref store are still open when the target format is the "reftable" format, and Cygwin refuses to rename open files. Fix this issue by closing the new ref store before renaming its files into place. This is a slight change in behaviour compared to before, where we kept the new ref store open and then updated the repository's ref store to point to it. While we could re-open the new ref store after we have moved files around, this is ultimately unnecessary. We know that the only user of `repo_migrate_ref_storage_format()` is the git-refs(1) command, and it won't access the ref store after it has been migrated anyway. So reinitializing the ref store would be a waste of time. Regardless of that it is still sensible to leave the repository in a consistent state. But instead of reinitializing the ref store, we can simply unset the repo's ref store altogether and let `get_main_ref_store()` lazily initialize the new ref store as required. Reported-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Jul 23, 2024 at 14:31 UTC 09c817383f56d4bbcfb8089e7c4ed5dd9db0f547

1 file changed +18 -4

refs.c

+18 -4

@@ -2827,6 +2827,14 @@ int repo_migrate_ref_storage_format(struct repository *repo,

2827 goto done;

2828 }

2829

2830 + /*

2831 + * Release the new ref store such that any potentially-open files will

2832 + * be closed. This is required for platforms like Cygwin, where

2833 + * renaming an open file results in EPERM.

2834 + */

2835 + ref_store_release(new_refs);

2836 + FREE_AND_NULL(new_refs);

2837 +

2838 /*

2839 * Until now we were in the non-destructive phase, where we only

2840 * populated the new ref store. From hereon though we are about

@@ -2858,10 +2866,14 @@ int repo_migrate_ref_storage_format(struct repository *repo,

2866 */

2867 initialize_repository_version(hash_algo_by_ptr(repo->hash_algo), format, 1);

2868

2861 - free(new_refs->gitdir);

2862 - new_refs->gitdir = xstrdup(old_refs->gitdir);

2863 - repo->refs_private = new_refs;

2869 + /*

2870 + * Unset the old ref store and release it. `get_main_ref_store()` will

2871 + * make sure to lazily re-initialize the repository's ref store with

2872 + * the new format.

2873 + */

2874 ref_store_release(old_refs);

2875 + FREE_AND_NULL(old_refs);

2876 + repo->refs_private = NULL;

2877

2878 ret = 0;

2879

@@ -2872,8 +2884,10 @@ done:

2884 new_gitdir.buf);

2885 }

2886

2875 - if (ret && new_refs)

2887 + if (new_refs) {

2888 ref_store_release(new_refs);

2889 + free(new_refs);

2890 + }

2891 ref_transaction_free(transaction);

2892 strbuf_release(&new_gitdir);

2893 return ret;