Re: [PATCH] refs: introduce API function to write invalid null ref

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 22, 2021, 03:09 UTC
Message-ID: <CAPig+cT9FDZ78mTmuvaOyzhc+JKV3U8DRBfFRezWr2bsW4gmGA@mail.gmail.com>
In-Reply-To: <CAPig+cT9ZUqkOZWZS3+gEd3soh-xyfxu2yvQ_gY-LMgVV-rAiw@mail.gmail.com>

On Sun, Feb 21, 2021 at 8:20 PM Eric Sunshine <sunshine@sunshineco.com> wrote:

Show 5 quoted lines

> The reason I ask is that the bit of code in
> builtin/worktree.c:add_worktree() which this patch targets is itself a
> hack to work around the shortcoming in which is_git_directory() won't
> consider the newly-created worktree as being legitimate if it doesn't
> have a well-formed HEAD ref. [...]

By the way, the only reason the hack of creating a temporary HEAD (containing arbitrary OID) is needed is that add_worktree() shells out to invoke one of git-update-ref or git-symbolic-ref. It is that shell-out to invoke a Git command which triggers the necessity of pacifying is_git_directory()...

Show 5 quoted lines

> On the other hand, I could see this as acceptable if "invalid" is
> removed from the function name and if it accepts an OID to write
> rather than unconditionally writing a zero-ID. In that case, it would
> become a generally useful function without the bad smells associated
> with the too-special-purpose write_invalid_head_ref().

... so, a much cleaner fix would be to stop shelling out to set up HEAD in the newly-created worktree, and instead call C API to perform those functions (update-ref and symbolic-ref) directly. If that C API does not yet exist, then it should be added.