Subject: Re: [PATCH v5 2/9] builtin/refs: support multiple worktrees check for refs.

From: Patrick Steinhardt <[email-masked]>
Date: Mon, 7 Oct 2024 11:16:19 +0200

On Mon, Oct 07, 2024 at 04:42:21PM +0800, shejialuo wrote:
> On Mon, Oct 07, 2024 at 08:58:30AM +0200, Patrick Steinhardt wrote:
> > On Sun, Sep 29, 2024 at 03:15:26PM +0800, shejialuo wrote:
> > > We have already set up the infrastructure to check the consistency for
> > > refs, but we do not support multiple worktrees. As we decide to add more
> > > checks for ref content, we need to set up support for multiple
> > > worktrees. Use "get_worktrees" and "get_worktree_ref_store" to check
> > > refs under the worktrees.
> > 
> > Makes sense.
> > 
> > > Because we should only check once for "packed-refs", let's call the fsck
> > > function for packed-backend when in the main worktree. In order to know
> > > which directory we check, we should default print this information
> > > instead of specifying "--verbose".
> > 
> > This change should likely be evicted into its own commit with a bit more
> > explanation.
> > 
> > > It's not suitable to print these information to the stderr. So, change
> > > to stdout.
> > 
> > This one, too. Why exactly is in not suitable to print to stderr?
> > 
> 
> I am sorry for the confusion. We should not print which directory we
> check here into stderr. Because I think this will make test script
> contain many unrelated info when using "git refs verify 2>err".
> 
> The reason here is when checking the consistency of refs in multiple
> worktrees. The ref name could be repeat. For example, worktree A
> has its own ref called "test" under ".git/worktrees/A/refs/worktree/test"
> and worktree B has its own ref still called "test" under
> ".git/worktrees/B/refs/worktree/test".
> 
> However, the refname would be printed to "refs/worktree/test". It will
> make the user confused which "refs/worktree/test" is checked. So, we
> should print this information like:
> 
>     Checking references consistency in .git
>     ...
>     checking references consistency in .git/worktrees/A
>     ...
>     checking references consistency in .git/worktrees/B
> 
> However, when writing this, I feel a ".git" is a bad usage. It will make
> the user think it will check everything here. This should be improved in
> the next version.

But wouldn't it be the better solution if we printed the fully-qualified
reference name "worktrees/worktree/refs/worktree/test" instead? That
would remove the need to say which directory we're currently verifying
in the first place.

Patrick