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

2015-06-05 Thread Michael Haggerty 
On 05/25/2015 08:38 PM, brian m. carlson wrote:
 From: Michael Haggerty mhag...@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
mhag...@alum.mit.edu

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2015-05-25 Thread brian m. carlson 
From: Michael Haggerty mhag...@alum.mit.edu

They were never used.

Signed-off-by: Michael Haggerty mhag...@alum.mit.edu
Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 builtin/remote.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/builtin/remote.c b/builtin/remote.c
index 1986e98..ab39fea 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -513,7 +513,6 @@ static int add_branch_for_removal(const char *refname,
 {
struct branches_for_remote *branches = cb_data;
struct refspec refspec;
-   struct string_list_item *item;
struct known_remote *kr;
 
memset(refspec, 0, sizeof(refspec));
@@ -543,9 +542,7 @@ static int add_branch_for_removal(const char *refname,
if (flags  REF_ISSYMREF)
return unlink(git_path(%s, refname));
 
-   item = string_list_append(branches-branches, refname);
-   item-util = xmalloc(20);
-   hashcpy(item-util, oid-hash);
+   string_list_append(branches-branches, refname);
 
return 0;
 }
@@ -828,7 +825,7 @@ static int rm(int argc, const char **argv)
 
if (!result)
result = remove_branches(branches);
-   string_list_clear(branches, 1);
+   string_list_clear(branches, 0);
 
if (skipped.nr) {
fprintf_ln(stderr,
-- 
2.4.0

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html