From: Phillip Wood <phillip.wood123@gmail.com>
Date: Mon, 02 Jun 2025 09:53:50 GMT
Subject: Re: [PATCH] fsck: ignore missing "refs" directory for linked worktrees
Message-ID: <b92b5d93-7f7f-4370-ac79-7d9767bb0db5@gmail.com>
In-Reply-To: <aDp55upE6AhYunz7@ArchLinux>

Hi Shejialuo

On 31/05/2025 04:39, shejialuo wrote:
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 4d1f65a57a..bf6f89b1d1 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -3762,6 +3762,9 @@ static int files_fsck_refs_dir(struct ref_store *ref_store,
>   
>   	iter = dir_iterator_begin(sb.buf, 0);
>   	if (!iter) {
> +		if (errno == ENOENT && !is_main_worktree(wt))
> +			goto out;
> +
>   		ret = error_errno(_("cannot open directory %s"), sb.buf);
>   		goto out;
>   	}

I think it would be clearer to write this as

	if (is_main_worktree(wt) || errno != ENOENT)
		ret = error_errno(_("cannot open directory %s"), sb.buf);
	goto out;

so that the condition that triggers the error message is explicit rather 
than having to mentally invert the condition to figure out when we 
return an error

Best Wishes

Phillip

> diff --git a/t/t0602-reffiles-fsck.sh b/t/t0602-reffiles-fsck.sh
> index f671ac4d3a..615b7c0683 100755
> --- a/t/t0602-reffiles-fsck.sh
> +++ b/t/t0602-reffiles-fsck.sh
> @@ -110,6 +110,21 @@ test_expect_success 'ref name check should be adapted into fsck messages' '
>   	)
>   '
>   
> +test_expect_success 'no refs directory of worktree should not cause problems' '
> +	test_when_finished "rm -rf repo" &&
> +	git init repo &&
> +	(
> +		cd repo &&
> +		test_commit initial &&
> +
> +		git worktree add --detach ./worktree &&
> +		# Simulate old directory layout
> +		rm -rf ./git/worktrees/worktree/refs &&
> +		git refs verify 2>err &&
> +		test_must_be_empty err
> +	)
> +'
> +
>   test_expect_success 'ref name check should work for multiple worktrees' '
>   	test_when_finished "rm -rf repo" &&
>   	git init repo &&