Subject: Re: [PATCH v5 4/7] refs: add support for transactional symref updates

From: Junio C Hamano <[email-masked]>
Date: Thu, 02 May 2024 10:53:44 -0700

Karthik Nayak <[email-masked]> writes:

> @@ -2863,12 +2928,26 @@ static int files_transaction_finish(struct ref_store *ref_store,
>  
>  		if (update->flags & REF_NEEDS_COMMIT ||
>  		    update->flags & REF_LOG_ONLY) {
> -			if (files_log_ref_write(refs,
> -						lock->ref_name,
> -						&lock->old_oid,
> -						&update->new_oid,
> -						update->msg, update->flags,
> -						err)) {
> +			int create_reflog = 1;
> +
> +			if (update->new_target) {
> +				/*
> +				 * We want to get the resolved OID for the target, to ensure
> +				 * that the correct value is added to the reflog.
> +				 */
> +				if (!refs_resolve_ref_unsafe(&refs->base, update->new_target,
> +							     RESOLVE_REF_READING, &update->new_oid, NULL)) {
> +					/* for dangling symrefs we skip creating a reflog entry. */
> +					create_reflog = 0;
> +				}
> +			}
> +
> +			if (create_reflog && files_log_ref_write(refs,
> +								 lock->ref_name,
> +								 &lock->old_oid,
> +								 &update->new_oid,
> +								 update->msg, update->flags,
> +								 err)) {
>  				char *old_msg = strbuf_detach(err, NULL);
>  
>  				strbuf_addf(err, "cannot update the ref '%s': %s",

This hunk is overly wide.

You could of course fix it mechanically (e.g., by rewrapping overly
wide comment block, wrapping an expression after &&-), but a code
path that is too deeply indented may be a sign that the function may
want to be split into calls to a smaller helper function for
readability.