Hopefully Michael would respond with more in-depth reviews as he has
been touching this area heavily recently, but a few comments.
> Subject: Re: [PATCH 3/3] Change update_refs to run a single commit loop once all work is fi
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@xxxxxxxxxx> writes:
>
> /* Perform updates first so live commits remain referenced */
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.
--
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