On Tue, Apr 09, 2024 at 09:15:59AM -0700, Karthik Nayak wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > On Tue, Apr 02, 2024 at 09:40:41AM -0700, Junio C Hamano wrote:
> >> Patrick Steinhardt <ps@pks.im> writes:
> >>
> >> > because they have been supported by Git all along. It simply makes our
> >> > lifes easier when we don't have to special-case creations and deletions
> >> > in any way.
> >> >
> >> > So I'd really not want those to go away or become deprecated.
> >>
> >> That is a good input.
> >>
> >> Do you have anything to add as a counter-proposal?  The "I do not
> >> care what was there before" update mode does make it necessary to
> >> have a "zero" value for symrefs that can be distinguishable from
> >> not having a value at all.
> >>
> >> Thanks.
> >
> > Sorry for taking this long to answer your question.
> >
> > I might have missed it while scanning through this thread, but why
> > exactly is the zero OID not a good enough placeholder here to say that
> > the ref must not exist? A symref cannot point to a ref named after the
> > zero OID anyway.
> >
> > In my opinion, "update-symref" with an old-value must be able to accept
> > both object IDs and symrefs as old value. Like this it would be possible
> > to update a proper ref to a symref in a race-free way. So you can say:
> >
> >     git update-ref SYMREF refs/heads/main 19981daefd7c147444462739375462b49412ce33
> >
> > To update "SYRMEF" to "refs/heads/main", but only in case it currently
> > is a proper ref that points to 19981daefd7c147444462739375462b49412ce33.
> > Similarly...
> >
> >     git update-ref SYMREF refs/heads/main refs/heads/master
> >
> > would update "SYMREF" to "refs/heads/main", but only if it currently
> > points to the symref "refs/heads/master". And by extension I think that
> > the zero OID should retain its established meaning of "This ref must not
> > exist":
> >
> >     git update-ref SYMREF refs/heads/main 0000000000000000000000000000000000000000
> >
> > This would only update "SYMREF" to "refs/heads/main" if it does not yet
> > exist.
> >
> 
> This is definitely nicer experience for the user. From looking at the
> other commands, {verify, create, delete} I can only see this applying to
> `symref-update`. Making the syntax for update something like:
> 
>     symref-update SP <ref> SP <new-ref> [SP (<old-oid> | <old-rev>)] LF
> 
> I think this makes sense to me, will incorporate this and send the next
> version in the next few days.
> 
> On a side-note: This would also mean that we should somehow support
> moving from symrefs to a regular ref via a transaction, so that means we
> should allow
> 
>     update SP <ref> SP <new-oid> [SP (<old-oid> | <old-rev>)] LF
> 
> too, but I'm not going to tackle that in my patches.

Yes, I think that would be a sensible idea, even though we have to be
careful with backwards compatibility here. In any case, I think it makes
sense to not extend the scope of your patch series and leave this for
the future.

Patrick