On Tue, Jul 29, 2014 at 2:09 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Ronnie Sahlberg <sahlberg@xxxxxxxxxx> 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.
Thank!
ronnie sahlberg
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html