Re: [PATCH 4/6] update-ref: add support for 'symref-create' command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Patrick Steinhardt <ps@xxxxxx> writes:

> On Tue, May 14, 2024 at 02:44:09PM +0200, Karthik Nayak wrote:
>> From: Karthik Nayak <karthik.188@xxxxxxxxx>
>> diff --git a/refs.c b/refs.c
>> index c2c9889466..6b724343fe 100644
>> --- a/refs.c
>> +++ b/refs.c
>> @@ -1302,15 +1302,16 @@ int ref_transaction_update(struct ref_transaction *transaction,
>>  int ref_transaction_create(struct ref_transaction *transaction,
>>  			   const char *refname,
>>  			   const struct object_id *new_oid,
>> +			   const char *new_target,
>>  			   unsigned int flags, const char *msg,
>>  			   struct strbuf *err)
>>  {
>> -	if (!new_oid || is_null_oid(new_oid)) {
>> -		strbuf_addf(err, "'%s' has a null OID", refname);
>> +	if ((!new_oid || is_null_oid(new_oid)) && !new_target) {
>> +		strbuf_addf(err, "'%s' has a null OID or no new target", refname);
>>  		return 1;
>>  	}
>
> Shouldn't this be "'%s' has neither an OID nor a target"?
>

Well it's actually all three. If new_oid is not set, or if new_oid is
NULL or there is no new_target.

I think "'%s' has neither a valid OID nor a target" would be best.

> Also, we again miss `new_oid && new_target`.
>

Will fix for all the commands!

>> diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh
>> index a390cffc80..cc7e20431e 100755
>> --- a/t/t0600-reffiles-backend.sh
>> +++ b/t/t0600-reffiles-backend.sh
>> @@ -468,4 +468,36 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' '
>>  	esac
>>  '
>>
>> +test_expect_success SYMLINKS 'symref transaction supports symlinks' '
>> +	test_when_finished "git symbolic-ref -d TESTSYMREFONE" &&
>> +	git update-ref refs/heads/new @ &&
>> +	test_config core.prefersymlinkrefs true &&
>> +	cat >stdin <<-EOF &&
>> +	start
>> +	symref-create TESTSYMREFONE refs/heads/new
>
> Let's future proof this and create the ref with a name that matches our
> root ref restrictions, like "TEST_SYMREF_HEAD". We do plan to enforce
> those soonish, so these tests would break.
>
> Patrick

Makes sense, will fix. Thanks.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux