git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org,  Eric Sunshine <sunshine@sunshineco.com>,
	 Ramsay Jones <ramsay@ramsayjones.plus.com>,
	 Justin Tobler <jltobler@gmail.com>
Subject: Re: [PATCH v3 00/12] refs: ref storage format migrations
Date: Tue, 28 May 2024 11:16:17 -0700	[thread overview]
Message-ID: <xmqqo78p4xam.fsf@gitster.g> (raw)
In-Reply-To: <cover.1716877224.git.ps@pks.im> (Patrick Steinhardt's message of "Tue, 28 May 2024 08:31:10 +0200")

Patrick Steinhardt <ps@pks.im> 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.

  parent reply	other threads:[~2024-05-28 18:16 UTC|newest]

Thread overview: 103+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-23  8:25 [PATCH 0/9] refs: ref storage format migrations Patrick Steinhardt
2024-05-23  8:25 ` [PATCH 1/9] setup: unset ref storage when reinitializing repository version Patrick Steinhardt
2024-05-23  8:25 ` [PATCH 2/9] refs: convert ref storage format to an enum Patrick Steinhardt
2024-05-23  8:25 ` [PATCH 3/9] refs: pass storage format to `ref_store_init()` explicitly Patrick Steinhardt
2024-05-23  8:25 ` [PATCH 4/9] refs: allow to skip creation of reflog entries Patrick Steinhardt
2024-05-23  8:25 ` [PATCH 5/9] refs/files: refactor `add_pseudoref_and_head_entries()` Patrick Steinhardt
2024-05-23  8:25 ` [PATCH 6/9] refs/files: extract function to iterate through root refs Patrick Steinhardt
2024-05-23  8:25 ` [PATCH 7/9] refs: implement removal of ref storages Patrick Steinhardt
2024-05-23  8:25 ` [PATCH 8/9] refs: implement logic to migrate between ref storage formats Patrick Steinhardt
2024-05-23 17:31   ` Eric Sunshine
2024-05-24  7:35     ` Patrick Steinhardt
2024-05-24  9:01       ` Eric Sunshine
2024-05-23  8:25 ` [PATCH 9/9] builtin/refs: new command to migrate " Patrick Steinhardt
2024-05-23 17:40   ` Eric Sunshine
2024-05-24  7:35     ` Patrick Steinhardt
2024-05-23 16:09 ` [PATCH 0/9] refs: ref storage format migrations Junio C Hamano
2024-05-24  7:33   ` Patrick Steinhardt
2024-05-24 16:28     ` Junio C Hamano
2024-05-28  5:13       ` Patrick Steinhardt
2024-05-28 16:16         ` Junio C Hamano
2024-05-24 10:14 ` [PATCH v2 " Patrick Steinhardt
2024-05-24 10:14   ` [PATCH v2 1/9] setup: unset ref storage when reinitializing repository version Patrick Steinhardt
2024-05-24 21:33     ` Justin Tobler
2024-05-28  5:13       ` Patrick Steinhardt
2024-05-24 10:14   ` [PATCH v2 2/9] refs: convert ref storage format to an enum Patrick Steinhardt
2024-05-24 10:14   ` [PATCH v2 3/9] refs: pass storage format to `ref_store_init()` explicitly Patrick Steinhardt
2024-05-24 10:14   ` [PATCH v2 4/9] refs: allow to skip creation of reflog entries Patrick Steinhardt
2024-05-24 10:14   ` [PATCH v2 5/9] refs/files: refactor `add_pseudoref_and_head_entries()` Patrick Steinhardt
2024-05-24 10:14   ` [PATCH v2 6/9] refs/files: extract function to iterate through root refs Patrick Steinhardt
2024-05-24 10:15   ` [PATCH v2 7/9] refs: implement removal of ref storages Patrick Steinhardt
2024-05-24 10:15   ` [PATCH v2 8/9] refs: implement logic to migrate between ref storage formats Patrick Steinhardt
2024-05-24 22:32     ` Justin Tobler
2024-05-28  5:14       ` Patrick Steinhardt
2024-05-24 10:15   ` [PATCH v2 9/9] builtin/refs: new command to migrate " Patrick Steinhardt
2024-05-24 18:24     ` Ramsay Jones
2024-05-24 19:29       ` Eric Sunshine
2024-05-28  5:14         ` Patrick Steinhardt
2024-05-28  6:31 ` [PATCH v3 00/12] refs: ref storage format migrations Patrick Steinhardt
2024-05-28  6:31   ` [PATCH v3 01/12] setup: unset ref storage when reinitializing repository version Patrick Steinhardt
2024-05-28  6:31   ` [PATCH v3 02/12] refs: convert ref storage format to an enum Patrick Steinhardt
2024-05-28  6:31   ` [PATCH v3 03/12] refs: pass storage format to `ref_store_init()` explicitly Patrick Steinhardt
2024-05-28  6:31   ` [PATCH v3 04/12] refs: allow to skip creation of reflog entries Patrick Steinhardt
2024-05-28  6:31   ` [PATCH v3 05/12] refs/files: refactor `add_pseudoref_and_head_entries()` Patrick Steinhardt
2024-05-28  6:31   ` [PATCH v3 06/12] refs/files: extract function to iterate through root refs Patrick Steinhardt
2024-05-28  6:31   ` [PATCH v3 07/12] refs/files: fix NULL pointer deref when releasing ref store Patrick Steinhardt
2024-05-28  6:31   ` [PATCH v3 08/12] reftable: inline `merged_table_release()` Patrick Steinhardt
2024-05-28  6:31   ` [PATCH v3 09/12] worktree: don't store main worktree twice Patrick Steinhardt
2024-05-28  6:31   ` [PATCH v3 10/12] refs: implement removal of ref storages Patrick Steinhardt
2024-05-28  6:32   ` [PATCH v3 11/12] refs: implement logic to migrate between ref storage formats Patrick Steinhardt
2024-05-28  6:32   ` [PATCH v3 12/12] builtin/refs: new command to migrate " Patrick Steinhardt
2024-05-31 23:46     ` Junio C Hamano
2024-06-02  1:03       ` Junio C Hamano
2024-06-03  7:37         ` Patrick Steinhardt
2024-05-28 18:16   ` Junio C Hamano [this message]
2024-05-28 18:26     ` [PATCH v3 00/12] refs: ref storage format migrations Junio C Hamano
2024-06-03  9:30 ` [PATCH v4 00/12] refs: ref storage migrations Patrick Steinhardt
2024-06-03  9:30   ` [PATCH v4 01/12] setup: unset ref storage when reinitializing repository version Patrick Steinhardt
2024-06-03  9:30   ` [PATCH v4 02/12] refs: convert ref storage format to an enum Patrick Steinhardt
2024-06-03  9:30   ` [PATCH v4 03/12] refs: pass storage format to `ref_store_init()` explicitly Patrick Steinhardt
2024-06-04  8:23     ` Karthik Nayak
2024-06-03  9:30   ` [PATCH v4 04/12] refs: allow to skip creation of reflog entries Patrick Steinhardt
2024-06-04 11:04     ` Karthik Nayak
2024-06-03  9:30   ` [PATCH v4 05/12] refs/files: refactor `add_pseudoref_and_head_entries()` Patrick Steinhardt
2024-06-03  9:30   ` [PATCH v4 06/12] refs/files: extract function to iterate through root refs Patrick Steinhardt
2024-06-05 10:07     ` Jeff King
2024-06-06  4:50       ` Patrick Steinhardt
2024-06-06  5:15         ` Patrick Steinhardt
2024-06-06  6:32           ` Patrick Steinhardt
2024-06-03  9:30   ` [PATCH v4 07/12] refs/files: fix NULL pointer deref when releasing ref store Patrick Steinhardt
2024-06-03  9:30   ` [PATCH v4 08/12] reftable: inline `merged_table_release()` Patrick Steinhardt
2024-06-03  9:30   ` [PATCH v4 09/12] worktree: don't store main worktree twice Patrick Steinhardt
2024-06-03  9:30   ` [PATCH v4 10/12] refs: implement removal of ref storages Patrick Steinhardt
2024-06-04 11:17     ` Karthik Nayak
2024-06-05 10:12     ` Jeff King
2024-06-05 16:54       ` Junio C Hamano
2024-06-06  4:51       ` Patrick Steinhardt
2024-06-03  9:31   ` [PATCH v4 11/12] refs: implement logic to migrate between ref storage formats Patrick Steinhardt
2024-06-04 15:28     ` Karthik Nayak
2024-06-05  5:52       ` Patrick Steinhardt
2024-06-05 10:03     ` Jeff King
2024-06-05 16:59       ` Junio C Hamano
2024-06-06  4:51         ` Patrick Steinhardt
2024-06-06  7:01           ` Jeff King
2024-06-06 15:41             ` Junio C Hamano
2024-06-08 11:36               ` Jeff King
2024-06-08 19:06                 ` Junio C Hamano
2024-06-06  4:51       ` Patrick Steinhardt
2024-06-03  9:31   ` [PATCH v4 12/12] builtin/refs: new command to migrate " Patrick Steinhardt
2024-06-06  5:28 ` [PATCH v5 00/12] refs: ref storage migrations Patrick Steinhardt
2024-06-06  5:28   ` [PATCH v5 01/12] setup: unset ref storage when reinitializing repository version Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 02/12] refs: convert ref storage format to an enum Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 03/12] refs: pass storage format to `ref_store_init()` explicitly Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 04/12] refs: allow to skip creation of reflog entries Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 05/12] refs/files: refactor `add_pseudoref_and_head_entries()` Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 06/12] refs/files: extract function to iterate through root refs Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 07/12] refs/files: fix NULL pointer deref when releasing ref store Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 08/12] reftable: inline `merged_table_release()` Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 09/12] worktree: don't store main worktree twice Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 10/12] refs: implement removal of ref storages Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 11/12] refs: implement logic to migrate between ref storage formats Patrick Steinhardt
2024-06-06  5:29   ` [PATCH v5 12/12] builtin/refs: new command to migrate " Patrick Steinhardt
2024-06-06  7:06   ` [PATCH v5 00/12] refs: ref storage migrations Jeff King
2024-06-06 16:18   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqo78p4xam.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jltobler@gmail.com \
    --cc=ps@pks.im \
    --cc=ramsay@ramsayjones.plus.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).