Patrick Steinhardt <ps@xxxxxx> writes:
> On Thu, Feb 17, 2022 at 12:41:33PM -0800, Junio C Hamano wrote:
>> Patrick Steinhardt <ps@xxxxxx> writes:
> [snip]
>> > +test_expect_success 'atomic fetch with failing backfill' '
>> > + git init clone3 &&
>> > +
>> > + # We want to test whether a failure when backfilling tags correctly
>> > + # aborts the complete transaction when `--atomic` is passed: we should
>> > + # neither create the branch nor should we create the tag when either
>> > + # one of both fails to update correctly.
>> > + #
>> > + # To trigger failure we simply abort when backfilling a tag.
>>
>> What does this paragraph want the phrase `backfilling tags` to mean?
>> Just from end-user's perspective, there is only one (i.e. if an
>> object that is tagged gets fetched and that tag is not here, fetch
>> it too), but at the mechanism level, there are two distinct code
>> paths (i.e. if OPT_FOLLOWTAGS gets the job done, there is no need to
>> call backfill_tags()). Which failure does this talk about, or it
>> does not matter which code path gave us the tag?
>
> It refers to `backfill_tags()`. Should I update this comment to clarify?
After reading the patch, the issue is only about the case where we
perform the second fetch and the case where OPT_FOLLOWTAGS does
everything necessary is not relevant. So hinting that we are
interested to see what a failure in the follow-on fetch does to the
atomicity would be a good idea, and mentioning backfill_tags() would
have been a good way to do so. Perhaps like "whether a failure in
backfill_tags() correctly aborts..." or something?
Thanks.
