Re: [PATCH 4/7] sequencer: introduce functions to handle autostashes via refs

[Patrick Steinhardt <ps@pks.im>]

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

On Fri, Jan 19, 2024 at 12:09:25PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > We're about to convert the MERGE_AUTOSTASH ref to become non-special,
> > using the refs API instead of direct filesystem access to both read and
> > write the ref. The current interfaces to write autostashes is entirely
> > path-based though, so we need to extend them to also support writes via
> > the refs API instead.
> >
> > Ideally, we would be able to fully replace the old set of path-based
> > interfaces. But the sequencer will continue to write state into
> > "rebase-merge/autostash". This path is not considered to be a ref at all
> > and will thus stay is-is for now, which requires us to keep both path-
> 
> "is-is"???

Oops, "as-is" :)

> > and refs-based interfaces to handle autostashes.
> >
> > Signed-off-by: Patrick Steinhardt <ps@pks.im>
> > ---
> >  sequencer.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++----
> >  sequencer.h |  3 +++
> >  2 files changed, 64 insertions(+), 5 deletions(-)
> 
> The conversion (rather, the introduction to allow refs API to be
> used to access them) look correct, but offhand I do not know what
> the implication of leaving the file based interface would be.

We have said in past discussions that the sequencer state should remain
self contained, and that requires us to keep the files-based interface.
Refactoring it would likely be a larger undertaking, as we have also
said that refs must either have pseudo-ref syntax or start with "refs/".

The sequencer with its "rebase-merge/autostash" files doesn't conform to
either of those requirements, so we would also have to rename those
reflike files. I doubt that this is really worth it, so I would keep
those around as internal implementation details of how the sequencer
works. These refs are not supposed to be accessed by the user in the
first place, and we do not document their existence to the best of my
knowledge. Also, `git rev-parse rebase-merge/autostash` would fail.

So overall I think it's fine to leave this internal sequencer state as
self-contained as it is.

Patrick

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