Re: [PATCH 3/3] Change update_refs to run a single commit loop once all work is finished.

Hopefully Michael would respond with more in-depth reviews as he has been touching this area heavily recently, but a few comments.

The project convention is to prefix with the "<area>", colon, SP, a sentence starting with lowercase, without the final full stop, e.g.

  Subject: [PATCH 3/3] refs.c: make update_refs() update and delete in a single loop

or something like that.

Ronnie Sahlberg <sahlberg@google.com> writes:

This comment, and the matching comment on the deletion loop below you removed, are curious, aren't they?

These blame down to 98aee92d (refs: add update_refs for multiple simultaneous updates, 2013-09-04) where it says:

    Though the refs themselves cannot be modified together in a
    single atomic transaction, this function does enable some useful
    semantics.  For example, a caller may create a new branch
    starting from the head of another branch and rewind the original
    branch at the same time.  This transfers ownership of commits
    between branches without risk of losing commits added to the
    original branch by a concurrent process, or risk of a concurrent
    process creating the new branch first.

My reading of the above is that the user can do an equivalent of "moving" an existing ref A to a new ref B (and want to see A disappear in the end), and do not want that to be turned into this sequence of events:

    $ git update-ref --stdin                $ git gc
    delete A
			                    starts without seeing neither A nor B
    create B
					    finishes, the object was not referenced
					    and excluded from the pack

And the approach 98aee92d took to avoid the above sequence was to create/update before delete.

It is possible that the particular approach to protect such an object at the tip of old A is not such a good one and is not necessary [*1*], but it needs to be justified in the log message. Also the stale in-code comments need to be updated (or removed).

Thanks.

[Footnote]

*1* For example, you can argue that the user can use two "git update-ref" back to back to first delete and then create, or the user is doing "git add" of many paths without yet creating commits or writing out the final index file, hence having many young loose objects that are not referenced from anything, and we designed these use cases to be safe enough by teaching gc not to prune young loose objects, and that same grace period in gc may make the above "delete then create" safe enough.