Subject: Re: [PATCH v3 00/12] refs: ref storage format migrations

From: Junio C Hamano <[email-masked]>
Date: Tue, 28 May 2024 11:16:17 -0700

Patrick Steinhardt <[email-masked]> writes:

>   - Swapped out calls to `remove_path()` to `unlink()`. We do not want
>     to walk up and remove empty parent directories, even though this is
>     harmless in practice.

Hmph.

It is customary to remove a directory when the last file in it gets
removed in the working tree, because Git tracks contents and does
not track directories, and it seems that the files backend does the
equivalent in the files_transaction_finish() method with
unlink_or_warn() followed by try_remove_empty_parents().  If we are
transitioning from the files backend to the reftable backend, don't
we want to end with no loose ref files under $GIT_DIR/refs/ and no
empty directories to house those loose ref files that would be
created in the future?

Let's find out why this is needed in [10/12].  It may just be a
simple matter of "let's not bother removing directories as we remove
loose ref files one by one---we know the whole hierarchy can be
removed after we are done", in which case I do think it is nicer.

>   - Release the reftable refdb before removing it. This closes the
>     cached "tables.list" file descriptor, which would otherwise break
>     removal of this file on Windows.
>
>   - Fix a bug with worktrees where we store the current worktree refdb
>     twice. This caused us to keep file descriptors open, which breaks
>     removal of the refdb on Windows.

Wow.  Windows' limitation sometimes helps us catch real bugs ;-).

Thanks, will replace to take a look.