git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Jonathan Tan <jonathantanmy@google.com>
To: git@vger.kernel.org
Cc: Jonathan Tan <jonathantanmy@google.com>
Subject: [RFC 09/14] transport: put ref oid in out param
Date: Wed, 25 Jan 2017 14:03:02 -0800
Message-ID: <27ff4202986d2bb88705f37cab33f171df1427a2.1485381677.git.jonathantanmy@google.com> (raw)
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>

Return new OID information obtained through fetching in new structs
instead of reusing the existing ones. With this change, the input
structs are no longer used for output, and are thus marked const.

This is the 3rd of 3 patches to eliminate using input refs to
communicate information obtained by the fetch mechanism.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
 builtin/clone.c      | 14 ++++++++++++--
 builtin/fetch-pack.c |  4 ++--
 fetch-pack.c         | 26 +++++++++++++++-----------
 fetch-pack.h         |  2 +-
 transport-helper.c   | 34 +++++++++++++++++++++++-----------
 transport.c          |  6 +++---
 transport.h          | 13 +++++--------
 7 files changed, 61 insertions(+), 38 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 0135c5f1c..3191da391 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -858,6 +858,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	struct refspec *refspec;
 	const char *fetch_pattern;
 
+	struct ref *new_remote_refs = NULL;
+
 	packet_trace_identity("clone");
 	argc = parse_options(argc, argv, prefix, builtin_clone_options,
 			     builtin_clone_usage, 0);
@@ -1075,8 +1077,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 				break;
 			}
 
-		if (!is_local && !complete_refs_before_fetch)
-			transport_fetch_refs(transport, mapped_refs, NULL);
+		if (!is_local && !complete_refs_before_fetch) {
+			transport_fetch_refs(transport, mapped_refs,
+					     &new_remote_refs);
+			if (new_remote_refs) {
+				refs = new_remote_refs;
+				free_refs(mapped_refs);
+				mapped_refs = wanted_peer_refs(refs, refspec);
+			}
+		}
 
 		remote_head = find_ref_by_name(refs, "HEAD");
 		remote_head_points_at =
@@ -1148,5 +1157,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
 	junk_mode = JUNK_LEAVE_ALL;
 
 	free(refspec);
+	free_refs(new_remote_refs);
 	return err;
 }
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index f31f874a0..a18fd0c44 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -11,7 +11,7 @@ static const char fetch_pack_usage[] =
 "[--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] "
 "[--no-progress] [--diag-url] [-v] [<host>:]<directory> [<refs>...]";
 
-static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,
+static void add_sought_entry(const struct ref ***sought, int *nr, int *alloc,
 			     const char *name)
 {
 	struct ref *ref;
@@ -44,7 +44,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 	int i, ret;
 	struct ref *ref = NULL;
 	const char *dest = NULL;
-	struct ref **sought = NULL;
+	const struct ref **sought = NULL;
 	int nr_sought = 0, alloc_sought = 0;
 	int fd[2];
 	char *pack_lockfile = NULL;
diff --git a/fetch-pack.c b/fetch-pack.c
index d4642b05c..8cc85c19f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -52,6 +52,10 @@ static int non_common_revs, multi_ack, use_sideband;
 #define ALLOW_REACHABLE_SHA1	02
 static unsigned int allow_unadvertised_object_request;
 
+/* An arbitrary non-NULL non-const pointer to assign to the util field of
+ * string list items when we need one. */
+#define ARBITRARY (&transfer_unpack_limit)
+
 __attribute__((format (printf, 2, 3)))
 static inline void print_verbose(const struct fetch_pack_args *args,
 				 const char *fmt, ...)
@@ -556,7 +560,7 @@ static void mark_recent_complete_commits(struct fetch_pack_args *args,
 
 static void filter_refs(struct fetch_pack_args *args,
 			struct ref **refs,
-			struct ref **sought, int nr_sought)
+			const struct ref **sought, int nr_sought)
 {
 	struct ref *newlist = NULL;
 	struct ref **newtail = &newlist;
@@ -604,16 +608,16 @@ static void filter_refs(struct fetch_pack_args *args,
 		for (i = 0; i < nr_sought; i++) {
 			unsigned char sha1[20];
 
-			ref = sought[i];
+			const struct ref *sref = sought[i];
 			if (matched[i])
 				continue;
-			if (get_sha1_hex(ref->name, sha1) ||
-			    ref->name[40] != '\0' ||
-			    hashcmp(sha1, ref->old_oid.hash))
+			if (get_sha1_hex(sref->name, sha1) ||
+			    sref->name[40] != '\0' ||
+			    hashcmp(sha1, sref->old_oid.hash))
 				continue;
 
 			matched[i] = 1;
-			*newtail = copy_ref(ref);
+			*newtail = copy_ref(sref);
 			newtail = &(*newtail)->next;
 		}
 	}
@@ -629,7 +633,7 @@ static void mark_alternate_complete(const struct ref *ref, void *unused)
 
 static int everything_local(struct fetch_pack_args *args,
 			    struct ref **refs,
-			    struct ref **sought, int nr_sought)
+			    const struct ref **sought, int nr_sought)
 {
 	struct ref *ref;
 	int retval;
@@ -831,7 +835,7 @@ static int cmp_ref_by_name(const void *a_, const void *b_)
 static struct ref *do_fetch_pack(struct fetch_pack_args *args,
 				 int fd[2],
 				 const struct ref *orig_ref,
-				 struct ref **sought, int nr_sought,
+				 const struct ref **sought, int nr_sought,
 				 struct shallow_info *si,
 				 char **pack_lockfile)
 {
@@ -955,7 +959,7 @@ static void fetch_pack_setup(void)
 	did_setup = 1;
 }
 
-static int remove_duplicates_in_refs(struct ref **ref, int nr)
+static int remove_duplicates_in_refs(const struct ref **ref, int nr)
 {
 	struct string_list names = STRING_LIST_INIT_NODUP;
 	int src, dst;
@@ -965,7 +969,7 @@ static int remove_duplicates_in_refs(struct ref **ref, int nr)
 		item = string_list_insert(&names, ref[src]->name);
 		if (item->util)
 			continue; /* already have it */
-		item->util = ref[src];
+		item->util = ARBITRARY;
 		if (src != dst)
 			ref[dst] = ref[src];
 		dst++;
@@ -1078,7 +1082,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
 		       int fd[], struct child_process *conn,
 		       const struct ref *ref,
 		       const char *dest,
-		       struct ref **sought, int nr_sought,
+		       const struct ref **sought, int nr_sought,
 		       struct sha1_array *shallow,
 		       char **pack_lockfile)
 {
diff --git a/fetch-pack.h b/fetch-pack.h
index 76f7c719c..6e4fdbb68 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -38,7 +38,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
 		       int fd[], struct child_process *conn,
 		       const struct ref *ref,
 		       const char *dest,
-		       struct ref **sought,
+		       const struct ref **sought,
 		       int nr_sought,
 		       struct sha1_array *shallow,
 		       char **pack_lockfile);
diff --git a/transport-helper.c b/transport-helper.c
index f3d78bb9e..be0aa6d39 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -384,7 +384,7 @@ static int release_helper(struct transport *transport)
 }
 
 static int fetch_with_fetch(struct transport *transport,
-			    int nr_heads, struct ref **to_fetch)
+			    int nr_heads, const struct ref **to_fetch)
 {
 	struct helper_data *data = transport->data;
 	int i;
@@ -477,13 +477,14 @@ static int get_exporter(struct transport *transport,
 }
 
 static int fetch_with_import(struct transport *transport,
-			     int nr_heads, struct ref **to_fetch)
+			     int nr_heads, const struct ref **to_fetch, struct ref **fetched_refs)
 {
 	struct child_process fastimport;
 	struct helper_data *data = transport->data;
 	int i;
-	struct ref *posn;
+	const struct ref *posn;
 	struct strbuf buf = STRBUF_INIT;
+	struct ref *fr_head = NULL, **fr_tail = &fr_head;
 
 	get_helper(transport);
 
@@ -522,28 +523,38 @@ static int fetch_with_import(struct transport *transport,
 	 * (If no "refspec" capability was specified, for historical
 	 * reasons we default to the equivalent of *:*.)
 	 *
-	 * Store the result in to_fetch[i].old_sha1.  Callers such
+	 * Store the result in old_oid in fetched_refs.  Callers such
 	 * as "git fetch" can use the value to write feedback to the
 	 * terminal, populate FETCH_HEAD, and determine what new value
 	 * should be written to peer_ref if the update is a
 	 * fast-forward or this is a forced update.
 	 */
+	if (!fetched_refs)
+		goto cleanup;
 	for (i = 0; i < nr_heads; i++) {
-		char *private, *name;
-		posn = to_fetch[i];
-		if (posn->status & REF_STATUS_UPTODATE)
+		struct ref *ref;
+		char *private;
+		const char *name;
+
+		ref = copy_ref_peerless(to_fetch[i]);
+		*fr_tail = ref;
+		fr_tail = &ref->next;
+		if (ref->status & REF_STATUS_UPTODATE)
 			continue;
-		name = posn->symref ? posn->symref : posn->name;
+		name = ref->symref ? ref->symref : ref->name;
 		if (data->refspecs)
 			private = apply_refspecs(data->refspecs, data->refspec_nr, name);
 		else
 			private = xstrdup(name);
 		if (private) {
-			if (read_ref(private, posn->old_oid.hash) < 0)
+			if (read_ref(private, ref->old_oid.hash) < 0)
 				die("Could not read ref %s", private);
 			free(private);
 		}
 	}
+	*fetched_refs = fr_head;
+
+cleanup:
 	strbuf_release(&buf);
 	return 0;
 }
@@ -646,7 +657,8 @@ 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, const struct ref **to_fetch,
+		 struct ref **fetched_refs)
 {
 	struct helper_data *data = transport->data;
 	int i, count;
@@ -679,7 +691,7 @@ static int fetch(struct transport *transport,
 		return fetch_with_fetch(transport, nr_heads, to_fetch);
 
 	if (data->import)
-		return fetch_with_import(transport, nr_heads, to_fetch);
+		return fetch_with_import(transport, nr_heads, to_fetch, fetched_refs);
 
 	return -1;
 }
diff --git a/transport.c b/transport.c
index 80e007f2f..5ed3fc68e 100644
--- a/transport.c
+++ b/transport.c
@@ -95,7 +95,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus
 }
 
 static int fetch_refs_from_bundle(struct transport *transport,
-			       int nr_heads, struct ref **to_fetch,
+			       int nr_heads, const struct ref **to_fetch,
 			       struct ref **fetched_refs)
 {
 	struct bundle_transport_data *data = transport->data;
@@ -203,7 +203,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,
+			       int nr_heads, const struct ref **to_fetch,
 			       struct ref **fetched_refs)
 {
 	struct git_transport_data *data = transport->data;
@@ -1101,7 +1101,7 @@ 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;
+	const struct ref **heads = NULL;
 	struct ref *nop_head = NULL, **nop_tail = &nop_head;
 	struct ref *rm;
 
diff --git a/transport.h b/transport.h
index b9e7e4656..326ff9bd6 100644
--- a/transport.h
+++ b/transport.h
@@ -84,15 +84,12 @@ struct transport {
 	 *
 	 * 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.
+	 * that the caller does not know (for example, shallow status or ref
+	 * hashes), it should provide that list of refs and include that
+	 * information in the list.
 	 **/
-	int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs,
+	int (*fetch)(struct transport *transport,
+		     int refs_nr, const struct ref **refs,
 		     struct ref **fetched_refs);
 
 	/**
-- 
2.11.0.483.g087da7b7c-goog


  parent reply index

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-25 22:02 [RFC 00/14] Allow fetch-pack to send ref names (globs allowed) Jonathan Tan
2017-01-25 22:02 ` [RFC 01/14] upload-pack: move parsing of "want" line Jonathan Tan
2017-01-25 22:02 ` [RFC 02/14] upload-pack: allow ref name and glob requests Jonathan Tan
2017-01-26 22:23   ` Junio C Hamano
2017-01-27  0:35     ` Jonathan Tan
2017-01-27  1:54       ` Junio C Hamano
2017-01-25 22:02 ` [RFC 03/14] upload-pack: test negotiation with changing repo Jonathan Tan
2017-01-26 22:33   ` Junio C Hamano
2017-01-27  0:44     ` Jonathan Tan
2017-02-22 23:36       ` Junio C Hamano
2017-02-23 18:43         ` [PATCH] upload-pack: report "not our ref" to client Jonathan Tan
2017-02-23 20:14           ` Junio C Hamano
2017-01-25 22:02 ` [RFC 04/14] fetch: refactor the population of hashes Jonathan Tan
2017-01-25 22:02 ` [RFC 05/14] fetch: refactor fetch_refs into two functions Jonathan Tan
2017-01-25 22:02 ` [RFC 06/14] fetch: refactor to make function args narrower Jonathan Tan
2017-01-25 22:03 ` [RFC 07/14] fetch-pack: put shallow info in out param Jonathan Tan
2017-01-25 22:03 ` [RFC 08/14] fetch-pack: check returned refs for matches Jonathan Tan
2017-01-25 22:03 ` Jonathan Tan [this message]
2017-01-25 22:03 ` [RFC 10/14] fetch-pack: support partial names and globs Jonathan Tan
2017-01-25 22:03 ` [RFC 11/14] fetch-pack: support want-ref Jonathan Tan
2017-01-25 22:03 ` [RFC 12/14] fetch-pack: do not printf after closing stdout Jonathan Tan
2017-01-26  0:50   ` Stefan Beller
2017-01-26 18:18     ` Jonathan Tan
2017-01-25 22:03 ` [RFC 13/14] fetch: send want-ref and receive fetched refs Jonathan Tan
2017-01-25 22:03 ` [RFC 14/14] DONT USE advertise_ref_in_want=1 Jonathan Tan
2017-01-26 22:15 ` [RFC 00/14] Allow fetch-pack to send ref names (globs allowed) Stefan Beller
2017-01-26 23:00 ` Jeff King
2017-01-27  0:26   ` Jonathan Tan
2017-02-07 23:53 ` Jonathan Tan
2017-02-09  0:26   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=27ff4202986d2bb88705f37cab33f171df1427a2.1485381677.git.jonathantanmy@google.com \
    --to=jonathantanmy@google.com \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git