Re: [PATCH 6/6] update-ref: add support for 'symref-update' command
- From
Karthik Nayak <karthik.188@gmail.com>
- Date
- May 22, 2024, 07:59 UTC
- Message-ID
- <CAOLa=ZRNNBOzsw+EzEJVnAU0ZsJ-bcTpciExdsQz2iDatYkqNQ@mail.gmail.com>
- In-Reply-To
- <CAOLa=ZQW-cCV5BP_fCvuZimfkjwAzjEiqXYRPft1Wf9kAX=_bw@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
Show 35 quoted lines
> Patrick Steinhardt <ps@pks.im> writes: > >> On Tue, May 14, 2024 at 02:44:11PM +0200, Karthik Nayak wrote: >>> From: Karthik Nayak <karthik.188@gmail.com> >>> >>> Add 'symref-update' command to the '--stdin' mode of 'git-update-ref' to >>> allow updates of symbolic refs. The 'symref-update' command takes in a >>> <new-target>, which the <ref> will be updated to. If the <ref> doesn't >>> exist it will be created. >>> >>> It also optionally takes either an `ref <old-target>` or `oid >>> <old-oid>`. If the <old-target> is provided, it checks to see if the >>> <ref> targets the <old-target> before the update. If <old-oid> is provided >>> it checks <ref> to ensure that it is a regular ref and <old-oid> is the >>> OID before the update. This by extension also means that this when a >>> zero <old-oid> is provided, it ensures that the ref didn't exist before. >> >> It's somewhat unfortunate that the syntax diverges from the "update" >> command. "update" also has essentially the same issue now, that we >> cannot verify that its old value is a symref, right? Can we fix that in >> a backwards compatible way? >> > > I think Peff mentioned [1] of a way. So we convert the existing > > update SP <ref> SP <newvalue> [SP <oldvalue>] LF > update SP <ref> NUL <newvalue> NUL [<oldvalue>] NUL // -z > > to > > update SP <ref> SP <newvalue> [SP (<oldvalue> | ref <old_target>)] LF > update SP <ref> NUL <newvalue> NUL [(<oldvalue> | ref NUL <old_target>)] NUL // -z > > this should work, I think. I will play around this and add it in. Please > let me know if you can think of a scenario where this breaks.
Thinking about this more, I'll actually do this in a follow up series. I want to cleanup the existing tests already to move them to the loop that I introduced in this series. That would make it easier to test and ensure that we can port the existing 'update' command to also support `old_target`.
[snip]