On Tue Oct 15, 2024 at 09:41, karthik nayak <karthik.188@xxxxxxxxx> wrote: > Bence Ferdinandy <bence@xxxxxxxxxxxxxx> writes: > > [snip] > >> diff --git a/refs.c b/refs.c >> index 5f729ed412..b964ac44d0 100644 >> --- a/refs.c >> +++ b/refs.c >> @@ -2114,7 +2114,8 @@ int peel_iterated_oid(struct repository *r, const struct object_id *base, struct >> } >> >> int refs_update_symref(struct ref_store *refs, const char *ref, >> - const char *target, const char *logmsg) >> + const char *target, const char *logmsg, >> + struct strbuf *referent) >> { >> struct ref_transaction *transaction; >> struct strbuf err = STRBUF_INIT; >> @@ -2122,17 +2123,23 @@ int refs_update_symref(struct ref_store *refs, const char *ref, >> >> transaction = ref_store_transaction_begin(refs, &err); >> if (!transaction || >> - ref_transaction_update(transaction, ref, NULL, NULL, >> + ref_transaction_update(transaction, ref, NULL, NULL, >> target, NULL, REF_NO_DEREF, >> logmsg, &err) || >> - ref_transaction_commit(transaction, &err)) { >> + ref_transaction_prepare(transaction, &err)) { >> ret = error("%s", err.buf); >> + goto cleanup; >> } >> + if (referent) >> + refs_read_symbolic_ref(refs, ref, referent); > > Shouldn't we also check the return value here? My reasoning was that if this fails referent will just look like as if it did not exist. Since this is an addition to set-head and fetch failing to set the HEAD in a case which would have previously worked I did not think it prudent to now fail on this for any reason. > >> + >> + if (ref_transaction_commit(transaction, &err)) >> + ret = error("%s", err.buf); >> >> +cleanup: >> strbuf_release(&err); >> if (transaction) >> ref_transaction_free(transaction); >> - >> > > Why remove this whiteline? Looks like a mistake made during rebase. -- bence.ferdinandy.com