Merge branch 'jt/connectivity-check-after-unshallow'

"git fetch" sometimes failed to update the remote-tracking refs,
which has been corrected.

* jt/connectivity-check-after-unshallow:
  fetch-pack: unify ref in and out param

Author: gitster

builtin/clone.c

@@ -1166,7 +1166,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 			}
 
 		if (!is_local && !complete_refs_before_fetch)
-			transport_fetch_refs(transport, mapped_refs, NULL);
+			transport_fetch_refs(transport, mapped_refs);
 
 		remote_head = find_ref_by_name(refs, "HEAD");
 		remote_head_points_at =
@@ -1208,7 +1208,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	if (is_local)
 		clone_local(path, git_dir);
 	else if (refs && complete_refs_before_fetch)
-		transport_fetch_refs(transport, mapped_refs, NULL);
+		transport_fetch_refs(transport, mapped_refs);
 
 	update_remote_refs(refs, mapped_refs, remote_head_points_at,
 			   branch_top.buf, reflog_msg.buf, transport,

builtin/fetch.c

@@ -942,13 +942,11 @@ static int quickfetch(struct ref *ref_map)
 	return check_connected(iterate_ref_map, &rm, &opt);
 }
 
-static int fetch_refs(struct transport *transport, struct ref *ref_map,
-		      struct ref **updated_remote_refs)
+static int fetch_refs(struct transport *transport, struct ref *ref_map)
 {
 	int ret = quickfetch(ref_map);
 	if (ret)
-		ret = transport_fetch_refs(transport, ref_map,
-					   updated_remote_refs);
+		ret = transport_fetch_refs(transport, ref_map);
 	if (!ret)
 		/*
 		 * Keep the new pack's ".keep" file around to allow the caller
@@ -1153,7 +1151,7 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)
 	transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
 	transport_set_option(transport, TRANS_OPT_DEPTH, "0");
 	transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
-	if (!fetch_refs(transport, ref_map, NULL))
+	if (!fetch_refs(transport, ref_map))
 		consume_refs(transport, ref_map);
 
 	if (gsecondary) {
@@ -1169,7 +1167,6 @@ static int do_fetch(struct transport *transport,
 	int autotags = (transport->remote->fetch_tags == 1);
 	int retcode = 0;
 	const struct ref *remote_refs;
-	struct ref *updated_remote_refs = NULL;
 	struct argv_array ref_prefixes = ARGV_ARRAY_INIT;
 
 	if (tags == TAGS_DEFAULT) {
@@ -1220,24 +1217,7 @@ static int do_fetch(struct transport *transport,
 				   transport->url);
 		}
 	}
-
-	if (fetch_refs(transport, ref_map, &updated_remote_refs)) {
-		free_refs(ref_map);
-		retcode = 1;
-		goto cleanup;
-	}
-	if (updated_remote_refs) {
-		/*
-		 * Regenerate ref_map using the updated remote refs.  This is
-		 * to account for additional information which may be provided
-		 * by the transport (e.g. shallow info).
-		 */
-		free_refs(ref_map);
-		ref_map = get_ref_map(transport->remote, updated_remote_refs, rs,
-				      tags, &autotags);
-		free_refs(updated_remote_refs);
-	}
-	if (consume_refs(transport, ref_map)) {
+	if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) {
 		free_refs(ref_map);
 		retcode = 1;
 		goto cleanup;

fetch-object.c

@@ -19,7 +19,7 @@ static void fetch_refs(const char *remote_name, struct ref *ref)
 
 	transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
 	transport_set_option(transport, TRANS_OPT_NO_DEPENDENTS, "1");
-	transport_fetch_refs(transport, ref, NULL);
+	transport_fetch_refs(transport, ref);
 	fetch_if_missing = original_fetch_if_missing;
 }
 

fetch-pack.c

@@ -1278,25 +1278,26 @@ static void receive_shallow_info(struct fetch_pack_args *args,
 	args->deepen = 1;
 }
 
-static void receive_wanted_refs(struct packet_reader *reader, struct ref *refs)
+static void receive_wanted_refs(struct packet_reader *reader,
+				struct ref **sought, int nr_sought)
 {
 	process_section_header(reader, "wanted-refs", 0);
 	while (packet_reader_read(reader) == PACKET_READ_NORMAL) {
 		struct object_id oid;
 		const char *end;
-		struct ref *r = NULL;
+		int i;
 
 		if (parse_oid_hex(reader->line, &oid, &end) || *end++ != ' ')
 			die(_("expected wanted-ref, got '%s'"), reader->line);
 
-		for (r = refs; r; r = r->next) {
-			if (!strcmp(end, r->name)) {
-				oidcpy(&r->old_oid, &oid);
+		for (i = 0; i < nr_sought; i++) {
+			if (!strcmp(end, sought[i]->name)) {
+				oidcpy(&sought[i]->old_oid, &oid);
 				break;
 			}
 		}
 
-		if (!r)
+		if (i == nr_sought)
 			die(_("unexpected wanted-ref: '%s'"), reader->line);
 	}
 
@@ -1381,7 +1382,7 @@ static struct ref *do_fetch_pack_v2(struct fetch_pack_args *args,
 				receive_shallow_info(args, &reader);
 
 			if (process_section_header(&reader, "wanted-refs", 1))
-				receive_wanted_refs(&reader, ref);
+				receive_wanted_refs(&reader, sought, nr_sought);
 
 			/* get the pack */
 			process_section_header(&reader, "packfile", 0);
@@ -1448,13 +1449,12 @@ static int remove_duplicates_in_refs(struct ref **ref, int nr)
 }
 
 static void update_shallow(struct fetch_pack_args *args,
-			   struct ref *refs,
+			   struct ref **sought, int nr_sought,
 			   struct shallow_info *si)
 {
 	struct oid_array ref = OID_ARRAY_INIT;
 	int *status;
 	int i;
-	struct ref *r;
 
 	if (args->deepen && alternate_shallow_file) {
 		if (*alternate_shallow_file == '\0') { /* --unshallow */
@@ -1496,8 +1496,8 @@ static void update_shallow(struct fetch_pack_args *args,
 	remove_nonexistent_theirs_shallow(si);
 	if (!si->nr_ours && !si->nr_theirs)
 		return;
-	for (r = refs; r; r = r->next)
-		oid_array_append(&ref, &r->old_oid);
+	for (i = 0; i < nr_sought; i++)
+		oid_array_append(&ref, &sought[i]->old_oid);
 	si->ref = &ref;
 
 	if (args->update_shallow) {
@@ -1531,12 +1531,12 @@ static void update_shallow(struct fetch_pack_args *args,
 	 * remote is also shallow, check what ref is safe to update
 	 * without updating .git/shallow
 	 */
-	status = xcalloc(ref.nr, sizeof(*status));
+	status = xcalloc(nr_sought, sizeof(*status));
 	assign_shallow_commits_to_refs(si, NULL, status);
 	if (si->nr_ours || si->nr_theirs) {
-		for (r = refs, i = 0; r; r = r->next, i++)
+		for (i = 0; i < nr_sought; i++)
 			if (status[i])
-				r->status = REF_STATUS_REJECT_SHALLOW;
+				sought[i]->status = REF_STATUS_REJECT_SHALLOW;
 	}
 	free(status);
 	oid_array_clear(&ref);
@@ -1599,7 +1599,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
 		args->connectivity_checked = 1;
 	}
 
-	update_shallow(args, ref_cpy, &si);
+	update_shallow(args, sought, nr_sought, &si);
 cleanup:
 	clear_shallow_info(&si);
 	return ref_cpy;

t/t5551-http-fetch-smart.sh

@@ -363,6 +363,24 @@ test_expect_success 'custom http headers' '
 		submodule update sub
 '
 
+test_expect_success 'using fetch command in remote-curl updates refs' '
+	SERVER="$HTTPD_DOCUMENT_ROOT_PATH/twobranch" &&
+	rm -rf "$SERVER" client &&
+
+	git init "$SERVER" &&
+	test_commit -C "$SERVER" foo &&
+	git -C "$SERVER" update-ref refs/heads/anotherbranch foo &&
+
+	git clone $HTTPD_URL/smart/twobranch client &&
+
+	test_commit -C "$SERVER" bar &&
+	git -C client -c protocol.version=0 fetch &&
+
+	git -C "$SERVER" rev-parse master >expect &&
+	git -C client rev-parse origin/master >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'GIT_REDACT_COOKIES redacts cookies' '
 	rm -rf clone &&
 	echo "Set-Cookie: Foo=1" >cookies &&

transport-helper.c

@@ -651,16 +651,14 @@ static int connect_helper(struct transport *transport, const char *name,
 }
 
 static int fetch(struct transport *transport,
-		 int nr_heads, struct ref **to_fetch,
-		 struct ref **fetched_refs)
+		 int nr_heads, struct ref **to_fetch)
 {
 	struct helper_data *data = transport->data;
 	int i, count;
 
 	if (process_connect(transport, 0)) {
 		do_take_over(transport);
-		return transport->vtable->fetch(transport, nr_heads, to_fetch,
-						fetched_refs);
+		return transport->vtable->fetch(transport, nr_heads, to_fetch);
 	}
 
 	count = 0;

transport-internal.h

@@ -36,18 +36,11 @@ struct transport_vtable {
 	 * Fetch the objects for the given refs. Note that this gets
 	 * an array, and should ignore the list structure.
 	 *
-	 * The transport *may* provide, in fetched_refs, the list of refs that
-	 * it fetched.  If the transport knows anything about the fetched refs
-	 * that the caller does not know (for example, shallow status), it
-	 * should provide that list of refs and include that information in the
-	 * list.
-	 *
 	 * If the transport did not get hashes for refs in
 	 * get_refs_list(), it should set the old_sha1 fields in the
 	 * provided refs now.
 	 **/
-	int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs,
-		     struct ref **fetched_refs);
+	int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs);
 
 	/**
 	 * Push the objects and refs. Send the necessary objects, and

transport.c

@@ -151,8 +151,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport,
 }
 
 static int fetch_refs_from_bundle(struct transport *transport,
-			       int nr_heads, struct ref **to_fetch,
-			       struct ref **fetched_refs)
+			       int nr_heads, struct ref **to_fetch)
 {
 	struct bundle_transport_data *data = transport->data;
 	return unbundle(&data->header, data->fd,
@@ -288,8 +287,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
 }
 
 static int fetch_refs_via_pack(struct transport *transport,
-			       int nr_heads, struct ref **to_fetch,
-			       struct ref **fetched_refs)
+			       int nr_heads, struct ref **to_fetch)
 {
 	int ret = 0;
 	struct git_transport_data *data = transport->data;
@@ -358,12 +356,8 @@ static int fetch_refs_via_pack(struct transport *transport,
 	if (report_unmatched_refs(to_fetch, nr_heads))
 		ret = -1;
 
-	if (fetched_refs)
-		*fetched_refs = refs;
-	else
-		free_refs(refs);
-
 	free_refs(refs_tmp);
+	free_refs(refs);
 	free(dest);
 	return ret;
 }
@@ -1223,31 +1217,19 @@ const struct ref *transport_get_remote_refs(struct transport *transport,
 	return transport->remote_refs;
 }
 
-int transport_fetch_refs(struct transport *transport, struct ref *refs,
-			 struct ref **fetched_refs)
+int transport_fetch_refs(struct transport *transport, struct ref *refs)
 {
 	int rc;
 	int nr_heads = 0, nr_alloc = 0, nr_refs = 0;
 	struct ref **heads = NULL;
-	struct ref *nop_head = NULL, **nop_tail = &nop_head;
 	struct ref *rm;
 
 	for (rm = refs; rm; rm = rm->next) {
 		nr_refs++;
 		if (rm->peer_ref &&
 		    !is_null_oid(&rm->old_oid) &&
-		    !oidcmp(&rm->peer_ref->old_oid, &rm->old_oid)) {
-			/*
-			 * These need to be reported as fetched, but we don't
-			 * actually need to fetch them.
-			 */
-			if (fetched_refs) {
-				struct ref *nop_ref = copy_ref(rm);
-				*nop_tail = nop_ref;
-				nop_tail = &nop_ref->next;
-			}
+		    !oidcmp(&rm->peer_ref->old_oid, &rm->old_oid))
 			continue;
-		}
 		ALLOC_GROW(heads, nr_heads + 1, nr_alloc);
 		heads[nr_heads++] = rm;
 	}
@@ -1265,11 +1247,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs,
 			heads[nr_heads++] = rm;
 	}
 
-	rc = transport->vtable->fetch(transport, nr_heads, heads, fetched_refs);
-	if (fetched_refs && nop_head) {
-		*nop_tail = *fetched_refs;
-		*fetched_refs = nop_head;
-	}
+	rc = transport->vtable->fetch(transport, nr_heads, heads);
 
 	free(heads);
 	return rc;

transport.h

@@ -239,8 +239,7 @@ int transport_push(struct transport *connection,
 const struct ref *transport_get_remote_refs(struct transport *transport,
 					    const struct argv_array *ref_prefixes);
 
-int transport_fetch_refs(struct transport *transport, struct ref *refs,
-			 struct ref **fetched_refs);
+int transport_fetch_refs(struct transport *transport, struct ref *refs);
 void transport_unlock_pack(struct transport *transport);
 int transport_disconnect(struct transport *transport);
 char *transport_anonymize_url(const char *url);