Re: [RFC/PATCH 0/5] Fix fetch regression with transport helpers

[Felipe Contreras <felipe.contreras@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 845 bytes --]

On Tue, Jun 4, 2019 at 9:35 AM Jeff King <peff@peff.net> wrote:
>
> On Mon, Jun 03, 2019 at 09:13:25PM -0500, Felipe Contreras wrote:

> > I'm not exactly sure the solution is the one we want, but hopefull it gives an
> > idea as to what is needed.
>
> It looks good to me. Thanks for fixing this.
>
> The breakage is in v2.20, so I don't think this needs to be rushed into
> v2.22 (which is already at -rc3). But it should probably go to 'maint'
> sooner rather than later.

Indeed, it's probably not so urgent since people have not even been
complaining (that I know of). However, I think it's clear that there's
a regression and the fix is simple, so maybe just apply the obvious
fix, and leave the rest of the patches for later?

I'm attaching only the fix, just to temp the idea of applying that
tiny thing into v2.2.

-- 
Felipe Contreras

[-- Attachment #2: 0001-fetch-fix-regression-with-transport-helpers.patch --]
[-- Type: text/x-patch, Size: 2538 bytes --]

From a59903e9828a4f265647014ca87fa3be9016a225 Mon Sep 17 00:00:00 2001
From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Tue, 4 Jun 2019 14:10:43 -0500
Subject: [PATCH] fetch: fix regression with transport helpers

Commit e198b3a740 changed the behavior of fetch with regards to tags.
Before, null oids where not ignored, now they are, regardless of whether
the refs have been explicitly cleared or not.

  e198b3a740 (fetch: replace string-list used as a look-up table with a hashmap)

When using a transport helper the oids can certainly be null. So now
tags are ignored and fetching them is impossible.

This patch fixes that by having a specific flag that is set only when we
explicitly want to ignore the refs, restoring the original behavior.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 builtin/fetch.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 4ba63d5ac6..f4962b93d6 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -239,6 +239,7 @@ static int will_fetch(struct ref **head, const unsigned char *sha1)
 struct refname_hash_entry {
 	struct hashmap_entry ent; /* must be the first member */
 	struct object_id oid;
+	int ignore;
 	char refname[FLEX_ARRAY];
 };
 
@@ -319,7 +320,7 @@ static void find_non_local_tags(const struct ref *refs,
 			    !will_fetch(head, ref->old_oid.hash) &&
 			    !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) &&
 			    !will_fetch(head, item->oid.hash))
-				oidclr(&item->oid);
+				item->ignore = 1;
 			item = NULL;
 			continue;
 		}
@@ -333,7 +334,7 @@ static void find_non_local_tags(const struct ref *refs,
 		if (item &&
 		    !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) &&
 		    !will_fetch(head, item->oid.hash))
-			oidclr(&item->oid);
+			item->ignore = 1;
 
 		item = NULL;
 
@@ -354,7 +355,7 @@ static void find_non_local_tags(const struct ref *refs,
 	if (item &&
 	    !has_object_file_with_flags(&item->oid, OBJECT_INFO_QUICK) &&
 	    !will_fetch(head, item->oid.hash))
-		oidclr(&item->oid);
+		item->ignore = 1;
 
 	/*
 	 * For all the tags in the remote_refs_list,
@@ -368,7 +369,7 @@ static void find_non_local_tags(const struct ref *refs,
 			BUG("unseen remote ref?");
 
 		/* Unless we have already decided to ignore this item... */
-		if (!is_null_oid(&item->oid)) {
+		if (!item->ignore) {
 			struct ref *rm = alloc_ref(item->refname);
 			rm->peer_ref = alloc_ref(item->refname);
 			oidcpy(&rm->old_oid, &item->oid);
-- 
2.21.0