On Mon, Jul 28, 2025 at 04:43:12PM +0200, Patrick Steinhardt wrote:
> > > @@ -2508,8 +2507,9 @@ static enum ref_transaction_error check_old_oid(struct ref_update *update,
> > > struct object_id *oid,
> > > struct strbuf *err)
> > > {
> > > - if (!(update->flags & REF_HAVE_OLD) ||
> > > - oideq(oid, &update->old_oid))
> > > + if (update->flags & REF_LOG_ONLY ||
> > > + !(update->flags & REF_HAVE_OLD) ||
> > > + oideq(oid, &update->old_oid))
> > > return 0;
> > >
> > > if (is_null_oid(&update->old_oid)) {
> >
> > Which make sense to me. But the weird thing I noticed is that when we do
> > something similar for split_head_update(), we don't strip REF_HAVE_OLD!
>
> And we shouldn't do that, as in the next commit we actually build on
> always having `REF_HAVE_OLD` set for reflog-only updates. So I'd argue
> that the problem is actually the other way round: when splitting off the
> HEAD update we must resolve the old object ID if `REF_HAVE_OLD` is not
> set.
Yeah, I agree that after your patches, split_head_update() should
definitely not be clearing that flag. What I more meant was: this patch
is introducing a behavior change for those split HEAD updates, which
used to do the extra old-oid check but now won't (whereas for other
symref log-only updates, you are preserving the behavior).
I _think_ that's a reasonable thing, but I wanted to make sure.
However...
> > (For those not familiar with that
...did you mean to write more? I know you've been running into weird
email truncation issues lately.
-Peff
