From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Fri, 05 Jun 2015 14:48:20 GMT
Subject: Re: [PATCH v3 16/56] add_branch_for_removal(): don't set "util" field of string_list entries
Message-ID: <5571B6B4.10006@alum.mit.edu>
In-Reply-To: <1432579162-411464-17-git-send-email-sandals@crustytoothpaste.net>

On 05/25/2015 08:38 PM, brian m. carlson wrote:
> From: Michael Haggerty <mhagger@alum.mit.edu>
> 
> They were never used.
> [...]

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

-- 
Michael Haggerty
mhagger@alum.mit.edu