From: Justin Tobler <jltobler@gmail.com>
Date: Thu, 06 Nov 2025 22:10:58 GMT
Subject: Re: [PATCH v2] fetch: fix non-conflicting tags not being committed
Message-ID: <cwayobvml63evuasdcamvkx5rpwectmwrwxr3cwxqrkxtketqa@lzm62c2xe75v>
In-Reply-To: <20251106-fix-tags-not-fetching-v2-1-610cb4b0e7c8@gmail.com>

On 25/11/06 09:39AM, Karthik Nayak wrote:
> @@ -1858,33 +1902,8 @@ static int do_fetch(struct transport *transport,
>  	if (retcode)
>  		goto cleanup;
>  
> -	retcode = ref_transaction_commit(transaction, &err);
> -	if (retcode) {
> -		/*
> -		 * Explicitly handle transaction cleanup to avoid
> -		 * aborting an already closed transaction.
> -		 */
> -		ref_transaction_free(transaction);
> -		transaction = NULL;
> -		goto cleanup;
> -	}
> -
> -	if (!atomic_fetch) {
> -		struct ref_rejection_data data = {
> -			.retcode = &retcode,
> -			.conflict_msg_shown = 0,
> -			.remote_name = transport->remote->name,
> -		};
> -
> -		ref_transaction_for_each_rejected_update(transaction,
> -							 ref_transaction_rejection_handler,
> -							 &data);
> -		if (retcode) {
> -			ref_transaction_free(transaction);
> -			transaction = NULL;
> -			goto cleanup;
> -		}
> -	}
> +	retcode = commit_ref_transaction(&transaction, atomic_fetch,
> +					 transport->remote->name, &err);

It looks like previously, whenever `ref_transaction_commit()` or
`ref_transaction_rejection_handler()` returned a non-zero value, we
would "goto cleanup". Now that these operations are handled via
`commit_ref_transaction()` though, it looks like we no longer handle the
"retcode" return value and just continue. I think we still need to check
the return value here.

-Justin