Re: [PATCH v3 07/10] refs: root refs can be symbolic refs

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2018 bytes --]

On Wed, May 15, 2024 at 02:22:20AM -0400, Jeff King wrote:
> On Wed, May 15, 2024 at 06:39:52AM +0200, Patrick Steinhardt wrote:
> 
> > On Wed, May 15, 2024 at 06:16:18AM +0200, Patrick Steinhardt wrote:
> > > On Fri, May 03, 2024 at 02:13:39PM -0400, Jeff King wrote:
> > > > On Thu, May 02, 2024 at 10:17:42AM +0200, Patrick Steinhardt wrote:
> > [snip]
> > > > And arguably is_pseudoref_syntax() should be taking into account the
> > > > "_HEAD" restriction and special names anyway. It is a bit weird that
> > > > even if we tighten up the refname checking to use is_pseudoref_syntax(),
> > > > you'd still be able to "git update-ref FOO" but then not see it as a
> > > > root ref!
> > > 
> > > True, as well. I'm less comfortable with doing that change in this
> > > series though as it does impose a major restriction that did not exist
> > > previously. We probably want some escape hatches so that it would still
> > > be possible to modify those refs when really required, for example to
> > > delete such broken refs.
> > > 
> > > I would thus like to defer this to a follow up patch series, if you
> > > don't mind.
> > 
> > Arguably, we don't need `is_pseudoref_syntax()` (which is being renamed
> > to `is_root_ref_syntax()`) at all anymore after this series lands
> > because it can be neatly rolled into `is_root_ref()`. The only caller,
> > `is_current_worktree_ref()`, should really call `is_roof_ref()` and not
> > `is_root_ref_syntax()`.
> 
> Yeah, and I'd expect that the more-strict check_refname_format() that I
> proposed elsewhere would be in the same boat. The only reason I used the
> "_syntax()" variant is that it was obviously wrong to do existence
> checks there. Once those are gone, then naturally it should be able to
> rely on is_root_ref() itself.

This series hasn't been queued/merged yet, right? Do you plan to reroll
it? I think that the changes in there are a good complementary addition
to the clarifications in my patch series.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]