Re: [PATCH 2/5] refs: make `is_pseudoref_syntax()` stricter

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Jan 23, 2024, 11:03 UTC
Message-ID: <ba721840-7b67-4822-8046-c0da4d3b9bde@gmail.com>
In-Reply-To: <xmqqplxtrucm.fsf@gitster.g>

Hi Junio

On 22/01/2024 20:22, Junio C Hamano wrote:

Show 12 quoted lines

> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> I'm concerned that this change is a regression. is_pseudoref_syntax()
>> is used by is_current_worktree_ref() and so scripts that create
>> pseudorefs that do not conform to your new rules will break as git
>> will no-longer consider the pseudorefs they create to be worktree
>> specific.
> 
> Ideally, when the exception list in the function becomes more
> complete, those "pseudorefs" created by those scripts shouldn't
> probably be created either as common or worktree specific thing
> if they are not "pseudoref".

I not sure I quite understand what you mean here. Are you saying that scripts should stop using "git update-ref" and "git rev-parse" for anything that does not match the new pseudoref syntax?

Show 9 quoted lines

>> Another approach would be to
>> read all the files whose name matches the pseudoref syntax and see if
>> its contents looks like a valid ref skipping names like COMMIT_EDITMSG
>> that we know are not pseudorefs.
> 
> In the longer term, I'd prefer to see a simpler rule, like "all-caps
> or underscore string, ending with _HEAD and nothing else are the
> pseudorefs but we have these small number of exceptions that are
> grandfathered".

Hopefully such a rule would stop us adding pseudorefs that are really private state like MERGE_AUTOSTASH. I think that is good in the long term but isn't it is happening now with this patch without any warning to users? This patch changes the behavior of parse_worktree_ref() which the files backend uses to figure out the path it should use when reading and writing a ref.

Best Wishes

Phillip