Re: [PATCH v3 16/56] add_branch_for_removal(): don't set "util" field of string_list entries

On 05/25/2015 08:38 PM, brian m. carlson wrote:

I happened to dig into the background of this change a little more, and this is what I found out.

When deleting remote-tracking references, we used to record the old_sha1 of each reference we found, and when deleting it verify that the recorded old_sha1 still agrees with the current value.

Later,

    c9e768b remote: repack packed-refs once when deleting multiple refs
(2014-05-23)

optimized this code to delete all of the packed refs at once, then delete the loose references one by one. That patch changed how delete_ref() was called, no longer passing it the old_sha1 value (and thus removing the verification step).

That commit doesn't give a justification for the change, but I can imagine what it would have been:

* Now that the packed refs and loose refs disappear at different times,
it would be awkward to check the old_sha1s
* We want to delete the references anyway, and they are remote-tracking
references (which shouldn't have any interesting local modifications),
so the protection is not really useful anyway.

I think that the change in c9e768b was OK, but I wanted to get this backstory out there in case anybody thinks that instead of removing the code to set "util", we should be re-adding the code that made use of "util" to verify the references' old values before deleting them.

Michael