Re: [PATCH v3] refs: return conflict error when checking packed refs

From: Ivan Tse <ivan.tse1@gmail.com>
Date: May 7, 2024, 05:51 UTC
Message-ID: <CA+WCWMyxBA90QRsxn-i0H1TRi7WrhRcD=Tgj07f_Y_=UVNkhbw@mail.gmail.com>
In-Reply-To: <xmqqttjaydbw.fsf@gitster.g>

On Mon, May 6, 2024 at 3:01 PM Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

> >> diff --git a/refs/files-backend.c b/refs/files-backend.c
> >> index a098d14ea00..97473f377d1 100644
> >> --- a/refs/files-backend.c
> >> +++ b/refs/files-backend.c
> >> @@ -794,8 +794,10 @@ static int lock_raw_ref(struct files_ref_store *refs,
> >>               */
> >>              if (refs_verify_refname_available(
> >>                                  refs->packed_ref_store, refname,
> >> -                                extras, NULL, err))
> >> +                                extras, NULL, err)) {
> >> +                    ret = TRANSACTION_NAME_CONFLICT;
> >>                      goto error_return;
> >> +            }
> >>      }
> >>
> >>      ret = 0;
> >>
> >
> > Shouldn't we also do this change in `lock_ref_oid_basic` where we gather
> > the same lock again for creating the reflog entry?
>
> An interesting question.

Hi!

Apologies but I'm not sure I follow - could you elaborate? I am not too familiar with the Git source code (or C language) but from looking at `lock_ref_oid_basic`, it looks like that function returns a lock struct and not an integer success/error code. I'm not sure how we would apply this change in that function as well?