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

From: phillip.wood123@gmail.com <phillip.wood123@gmail.com>
Date: Jan 23, 2024, 16:40 UTC
Message-ID: <ffb41e0f-f787-439a-9afe-8dcda5da21a3@gmail.com>
In-Reply-To: <CAOLa=ZQOcqGBJOSehok4BYGUE8RKtnE9eiJYogeT8E6NWZ25xw@mail.gmail.com>

Hi Karthik

On 23/01/2024 12:49, Karthik Nayak wrote:

Show 8 quoted lines

> Hello Phillip,
> 
> Phillip Wood <phillip.wood123@gmail.com> writes:
> [...]
> So there is no way to make `is_pseudoref_syntax()` stricter without
> breaking backward compatibility. While we do want to reach that goal, we
> have to go about in the other way around, that i.e.
> 1. Fix all pseudorefs to have the '_HEAD' suffix.

I'm wary of renaming user facing pseudorefs like AUTO_MERGE. In the future we should try and avoid adding any without a "_HEAD" suffix

Show 8 quoted lines

> 2. Move bisect files to '$GIT_DIR/bisect-state' (see [1] for more
> details).
> After this, we can safely make `is_pseudoref_syntax()` stricter.
> 
> Given this, I think for the next version, I'll do the following changes:
> 1. keep `is_pseudoref_syntax()` as is.
> 2. introduce `is_pseudoref()` which calls `is_pseudoref_syntax()` and
> also checks the content of the file.

We could perhaps make is_pseudoref() stricter from the start as we're adding a new function, it could have a list of exceptions to the "a pseudoref must end with '_HEAD'" rule like this patch. Being strict about the pseudorefs we list with "git for-each-ref" might encourage users to update any scripts they have that create "pseudorefs" that do not match the new rules without us making any changes that actually break those scripts.

> 3. replace use of `is_pseudoref_syntax()` with `is_pseudoref()` in this
> patch series.

That sounds like a good way forward to me, lets see if Junio agrees.

Best Wishes

Phillip

> [1]: https://public-inbox.org/git/20240119142705.139374-1-karthik.188@gmail.com/T/#m6e3790e30613fd68349708faaf5f4d9c704ba677