From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 28 May 2014 22:17:20 GMT
Subject: Re: [PATCH v11 25/41] fast-import.c: use a ref transaction when dumping tags
Message-ID: <20140528221720.GB12314@google.com>
In-Reply-To: <CAL=YDWkUhdoJkdg_zaq+p=XRu7H9fqNXDz89uPhbr4equTyVLQ@mail.gmail.com>

Ronnie Sahlberg wrote:
> On Wed, May 28, 2014 at 12:47 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

>> --- i/fast-import.c
>> +++ w/fast-import.c
>> @@ -1735,21 +1735,28 @@ static void dump_tags(void)
>>  {
>>         static const char *msg = "fast-import";
>>         struct tag *t;
>> -       char ref_name[PATH_MAX];
>> +       struct strbuf ref_name = STRBUF_INIT;
>>         struct strbuf err = STRBUF_INIT;
>>         struct ref_transaction *transaction;
>>
>> +       strbuf_addstr(&ref_name, "refs/tags/");
>> +
>>         transaction = ref_transaction_begin(&err);
>>         for (t = first_tag; t; t = t->next_tag) {
>> -               snprintf(ref_name, PATH_MAX, "refs/tags/%s", t->name);
>> +               strbuf_setlen(&ref_name, strlen("refs/tags/"));
>> +               strbuf_addstr(&ref_name, t->name);
>>
>> -               if (ref_transaction_update(transaction, ref_name, t->sha1,
>> -                                          NULL, 0, 0, &err))
>> -                       break;
>> +               if (ref_transaction_update(transaction, ref_name.buf, t->sha1,
>> +                                          NULL, 0, 0, &err)) {
>> +                       failure |= error("%s", err.buf);
>> +                       goto done;
>> +               }
>>         }
>>         if (ref_transaction_commit(transaction, msg, &err))
>>                 failure |= error("%s", err.buf);
>> +done:
>>         ref_transaction_free(transaction);
>> +       strbuf_release(&ref_name);
>>         strbuf_release(&err);
>>  }
>
> Changed to strbuf.  Thanks.

Thanks.  The semantics when ref_transaction_update() fail seem weird ---
see above.  (refs.h tells me "A failure to update means that the
transaction as a whole has failed and will need to be rolled back", so I
assume that the function should be rolling back instead of calling
_commit at that point.)