On Tue, Jul 29, 2014 at 2:09 PM, Junio C Hamano <gits...@pobox.com> wrote:
> Ronnie Sahlberg <sahlb...@google.com> writes:
>> +     /*
>> +      * Always copy loose refs that are to be deleted to the packed refs.
>> +      * If we are updating multiple refs then copy all non symref refs
>> +      * to the packed refs too.
>> +      */
>>       for (i = 0; i < n; i++) {
>>               struct ref_update *update = updates[i];
>>               unsigned char sha1[20];
>> +             int flag;
>>               if (update->update_type != UPDATE_SHA1)
>>                       continue;
>> -             if (!is_null_sha1(update->new_sha1))
>> +             if (num_updates < 2 && !is_null_sha1(update->new_sha1))
>>                       continue;
>>               if (get_packed_ref(update->refname))
>>                       continue;
>>               if (!resolve_ref_unsafe(update->refname, sha1,
>> -                                     RESOLVE_REF_READING, NULL))
>> +                                     RESOLVE_REF_READING, &flag))
>> +                     continue;
>> +             if (flag & REF_ISSYMREF)
>>                       continue;
> This skipping of symref didn't use to be here.
> Is this an enhancement needed to implement the new "feature"
> (i.e. use packed refs to represent multi-update as an atomic
> operation)?  It looks to me more like an unrelated "oops, forgot
> that we cannot use packed refs to store symrefs" fix-up, unless no
> caller passed symref in updates[] in the original code, and now we
> have callers that do.
> Puzzled...

It was mainly as an enhancement.
Prior to this patch we were only using the packed-refs trick for the
delete+rename operation part of a rename_ref() call.
And for that case we already explicitly test for and disallow symbolic
refs already in rename_ref().

I added this just for extra safety for "now that we possibly delete
multiple refs in one transaction, should I special case/disallow the
packed refs trick for symrefs?"
Looking at it again I don't think we need this special casing and it
only makes the code more complex and confusing.
I will review the use of this a little and run some tests and if all
looks sane I will remove this ISSYMREF special casing.

ronnie sahlberg
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to