Re: [PATCH 2/5] refs: make `is_pseudoref_syntax()` stricter
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Jan 23, 2024, 16:30 UTC
- Message-ID
- <9c800cd5-8d20-4df7-8834-f74ab000695e@gmail.com>
- In-Reply-To
- <Za-gF_Hp_lXViGWw@tanuki>
Hi Patrick
On 23/01/2024 11:16, Patrick Steinhardt wrote:
Show 20 quoted lines
> On Mon, Jan 22, 2024 at 12:22:49PM -0800, Junio C Hamano wrote: >> Phillip Wood <phillip.wood123@gmail.com> writes: >>> The list of hard coded exceptions also looks quite short, I >>> can see MERGE_AUTOSTASH and BISECT_START are missing and there are >>> probably others I've not thought of. >> >> I agree that it is something we need to fix. > > I've taken a deeper look at BISECT_START because I previously missed it > in my conversion to make special refs become normal pseudo refs. But as > it turns out, BISECT_START is not a ref at all. > > Depending on how you execute git-bisect(1), it will either contain the > object ID of the detached HEAD or the branch you're starting the bisect > from. This information is used to switch back to that state when you > abort the bisect. So far this sounds like a proper ref indeed. But in > case you're starting from a branch it will not be a symref that points > to this branch, but it will just contain the branch name. This is not a > valid format that could be read as a loose ref, and thus this file is > not a proper ref at all (except that sometimes it behaves like one when > starting from a detached HEAD).
Thank you, I'd missed that
Show 26 quoted lines
> My first hunch was to convert it so that it indeed always is a proper > ref. But thinking about it a bit more I'm less convinced that this is > sensible as it is deeply tied to the behaviour of git-bisect(1) and only > represents its internal state. I thus came to the conclusion that it is > more similar to the sequencer state that we have in ".git/rebase-merge" > and ".git/rebase-apply" than anything else. > > So if we wanted to rectify this, I think the most sensible way to > address this would be to introduce a new ".git/bisect-state" directory > that contains all of git-bisect(1)'s state: > > - BISECT_TERMS -> bisect-state/terms > - BISECT_LOG -> bisect-state/log > - BISECT_START -> bisect-state/start > - BISECT_RUN -> bisect-state/run > - BISECT_FIRST_PARENT -> bisect-state/first-parent > - BISECT_ANCESTORS_OK -> bisect-state/ancestors-ok > > I think this would make for a much cleaner solution overall as things > are neatly contained. Cleaning up after a bisect would thus only require > a delete of ".git/bisect-state/" and we're done. > > Of course, this would be a backwards-incompatible change. We could > transition to that newer schema by having newer Git versions recognize > both ways to store the state, but only ever write the new schema. But > I'm not sure whether it would ultimately be worth it.
I think that is a really good suggestion, it would bring bisect into line with am, rebase, cherry-pick etc. which keep their state in a subdirectory rather than cluttering up .git.
Best Wishes
Phillip
> Patrick