From: Karthik Nayak <karthik.188@gmail.com>
Date: Fri, 07 Nov 2025 14:01:21 GMT
Subject: Re: [PATCH v2] fetch: fix non-conflicting tags not being committed
Message-ID: <CAOLa=ZR9oKD_Zz3R+1W=f3M9Rd_FgNZ+AvFqk+ia+BSaACTsfg@mail.gmail.com>
In-Reply-To: <cwayobvml63evuasdcamvkx5rpwectmwrwxr3cwxqrkxtketqa@lzm62c2xe75v>

Justin Tobler <jltobler@gmail.com> writes:

> 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

Good catch, will add this in. Thanks