[PATCH 0/9] Multiple remotes without conflicts

STALE3736d

14 messages, 2 authors, 2016-06-15 · open the first message on its own page

[PATCH 0/9] Multiple remotes without conflicts

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:26

Hello,

I'm still trying to find a simple and painless way of sharing the object
store among multiple repositories: the idea is to have a "parent"
repository which contains the actual object store, and a number of
"child" repositories which link to that object store.  The obvious
problem is garbage collection: we can only garbage collect the parent
once it has all refs of all its children.

One way of ensuring that is to make each child a "remote" of the parent,
and to fetch all remotes first.  This works for branches, but not for
tags or for the reflog.

The problem with tags is that they all share the same namespace (they
are fetched with +refs/tags/*:refs/tags/*).  The obvious fix seems to be
to use a different refspec for tags, but with the built-in default
refspec, tags are then fetched twice (for example, with
+refs/*:refs/remotes/<remote>/*, refs/tags/foo is fetched as
refs/remotes/<remote>/tags/foo as well as refs/tags/foo).  Also, when
tags are included in refspecs, automatic tag fetching and the --tags and
--no-tags options break.

This patch series fixes this, and makes refspecs which match tags work
in a reasonable way.  (There may be problems with pruning still; I did
not look into that, yet.)

The other issue is that the "parent" won't know about things
referenced in the child's reflog.  This is not an issue for the setup I
have in mind because the children won't have reflogs (they will only be
accessed remotely), so I have not addressed this so far.

Here is an example for setting up a shared object store:

	TOP=$PWD
	mkdir ab a b

	cd $TOP/ab
	git init --bare
	for x in a b; do
		git remote add $x ../$x
		git config --unset remote.$x.fetch
		git config --add remote.$x.fetch "+refs/*:refs/remotes/$x/*"
	done

	for x in a b; do
		cd $TOP/$x
		git init
		git config gc.auto 0
		rm -r .git/objects
		ln -s ../../ab/objects .git/objects
	done

Comments welcome!

Thanks,
Andreas

---

Andreas Gruenbacher (9):
  fetch: Check for a "^{}" suffix with suffixcmp()
  fetch: Properly initialize refspec on stack
  fetch: Fix minor memory leak
  fetch: Move deepening fetch check into builtin/fetch.c
  fetch: Move loop checking which refs we have already
  fetch: Check if all objects exist after fetching
  fetch: Use the same ref map for all branches and tags
  fetch: Don't fetch tags twice
  fetch: Make automatic tag following work with arbitrary refspecs

 builtin/fetch.c |  430 ++++++++++++++++++++++++++++++++-----------------------
 cache.h         |    4 +-
 transport.c     |   38 ++----
 3 files changed, 264 insertions(+), 208 deletions(-)

[PATCH 8/9] fetch: Don't fetch tags twice

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:26

When refspecs are fetched which match tags (e.g.,
+refs/tags/*:refs/tags/<remote>/* or +refs/*:refs/remotes/<remote>/*)
and without --no-tags, we end up fetching the same tags twice because of
the +refs/tags/*:refs/tags/* refspec built into fetch.  Instead, when
processing the built-in refspec, ignore tags which are already mapped.

Signed-off-by: Andreas Gruenbacher <redacted>
---
 builtin/fetch.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8728153..8c01876 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -149,6 +149,7 @@ static void add_default_tags(const struct ref *remote_refs,
 			     struct ref *ref_map,
 			     struct ref ***tail)
 {
+	struct string_list mapped_refs = { NULL, 0, 0, 0 };
 	struct ref *tags = NULL, **tags_tail = &tags;
 	struct ref *ref, *peer_ref, **tag;

@@ -168,15 +169,19 @@ static void add_default_tags(const struct ref *remote_refs,
 	}
 	sort_string_list(peeled_map);
 
-	for (ref = ref_map; ref; ref = ref->next)
+	for (ref = ref_map; ref; ref = ref->next) {
+		string_list_append(ref->name, &mapped_refs);
 		if (ref->peer_ref)
 			string_list_insert(ref->peer_ref->name, existing_refs);
+	}
+	sort_string_list(&mapped_refs);
 
 	get_fetch_map(remote_refs, tag_refspec, &tags_tail, 0);
 	for (tag = &tags; *tag; ) {
 		ref = *tag;
 		peer_ref = ref->peer_ref;
-		if (!string_list_has_string(existing_refs, peer_ref->name)) {
+		if (!(string_list_has_string(existing_refs, peer_ref->name) ||
+		      string_list_has_string(&mapped_refs, ref->name))) {
 			struct string_list_item *item;
 
 			item = string_list_lookup(ref->name, peeled_map);

@@ -187,6 +192,7 @@ static void add_default_tags(const struct ref *remote_refs,
 		}
 		tag = &(*tag)->next;
 	}
+	string_list_clear(&mapped_refs, 0);
 
 	free_refs(tags);
 }

-- 
1.7.0.2.273.gc2413

[PATCH 4/9] fetch: Move deepening fetch check into builtin/fetch.c

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:26

Check if we are doing a deepening fetch in fetch instead of guessing in
transport_fetch_refs().

Signed-off-by: Andreas Gruenbacher <redacted>
---
 builtin/fetch.c |   34 ++++++++++++++++++++++++++++++++--
 cache.h         |    3 ++-
 transport.c     |   38 +++++++++++++-------------------------
 3 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 957be9f..fc6efc7 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -489,11 +489,41 @@ static int quickfetch(struct ref *ref_map)
 	return finish_command(&revlist) || err;
 }
 
+static void check_existing_refs(struct ref *ref_map) {
+	struct ref *ref;
+	int done = 1;
+
+	if (!depth) {
+		for (ref = ref_map; ref; ref = ref->next) {
+			if (ref->peer_ref &&
+			    !is_null_sha1(ref->old_sha1) &&
+			    !hashcmp(ref->peer_ref->old_sha1, ref->old_sha1))
+				ref->dont_fetch = 1;
+			else
+				done = 0;
+		}
+	}
+	if (done) {
+		/*
+		 * When deepening of a shallow repository is requested, local
+		 * and remote refs are likely to still be equal.  In addition,
+		 * we get here if all refs are up to date but quickfetch() did
+		 * not succeed.  In both cases, fetch all refs.
+		 */
+		for (ref = ref_map; ref; ref = ref->next)
+			ref->dont_fetch = 0;
+	}
+}
+
 static int fetch_refs(struct transport *transport, struct ref *ref_map)
 {
-	int ret = quickfetch(ref_map);
-	if (ret)
+	int ret;
+
+	ret = quickfetch(ref_map);
+	if (ret) {
+		check_existing_refs(ref_map);
 		ret = transport_fetch_refs(transport, ref_map);
+	}
 	if (!ret)
 		ret |= store_updated_refs(transport->url,
 				transport->remote->name,

diff --git a/cache.h b/cache.h
index 89f6a40..abfc682 100644
--- a/cache.h
+++ b/cache.h

@@ -869,7 +869,8 @@ struct ref {
 	unsigned int force:1,
 		merge:1,
 		nonfastforward:1,
-		deletion:1;
+		deletion:1,
+		dont_fetch:1;
 	enum {
 		REF_STATUS_NONE = 0,
 		REF_STATUS_OK,

diff --git a/transport.c b/transport.c
index 8ce3936..d1288af 100644
--- a/transport.c
+++ b/transport.c

@@ -1127,36 +1127,24 @@ const struct ref *transport_get_remote_refs(struct transport *transport)
 
 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;
+	int rc = 0;
+	int nr_heads;
+	struct ref **heads;
 	struct ref *rm;
 
-	for (rm = refs; rm; rm = rm->next) {
-		nr_refs++;
-		if (rm->peer_ref &&
-		    !is_null_sha1(rm->old_sha1) &&
-		    !hashcmp(rm->peer_ref->old_sha1, rm->old_sha1))
+	for (nr_heads = 0, rm = refs; rm; rm = rm->next) {
+		if (rm->dont_fetch)
 			continue;
-		ALLOC_GROW(heads, nr_heads + 1, nr_alloc);
-		heads[nr_heads++] = rm;
+		nr_heads++;
 	}
-
-	if (!nr_heads) {
-		/*
-		 * When deepening of a shallow repository is requested,
-		 * then local and remote refs are likely to still be equal.
-		 * Just feed them all to the fetch method in that case.
-		 * This condition shouldn't be met in a non-deepening fetch
-		 * (see builtin-fetch.c:quickfetch()).
-		 */
-		heads = xmalloc(nr_refs * sizeof(*heads));
-		for (rm = refs; rm; rm = rm->next)
-			heads[nr_heads++] = rm;
+	heads = xmalloc(nr_heads * sizeof(*heads));
+	for (nr_heads = 0, rm = refs; rm; rm = rm->next) {
+		if (rm->dont_fetch)
+			continue;
+		heads[nr_heads++] = rm;
 	}
-
-	rc = transport->fetch(transport, nr_heads, heads);
-
+	if (nr_heads)
+		rc = transport->fetch(transport, nr_heads, heads);
 	free(heads);
 	return rc;
 }

-- 
1.7.0.2.273.gc2413

[PATCH 1/9] fetch: Check for a "^{}" suffix with suffixcmp()

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:26

Otherwise, we will check random bytes for ref names < 3 characters.

Signed-off-by: Andreas Gruenbacher <redacted>
---
 builtin/fetch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 2bb75c1..77031e2 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -590,7 +590,7 @@ static void find_non_local_tags(struct transport *transport,
 		 * to fetch then we can mark the ref entry in the list
 		 * as one to ignore by setting util to NULL.
 		 */
-		if (!strcmp(ref->name + strlen(ref->name) - 3, "^{}")) {
+		if (!suffixcmp(ref->name, "^{}")) {
 			if (item && !has_sha1_file(ref->old_sha1) &&
 			    !will_fetch(head, ref->old_sha1) &&
 			    !has_sha1_file(item->util) &&

-- 
1.7.0.2.273.gc2413

[PATCH 6/9] fetch: Check if all objects exist after fetching

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:26

Check if all objects reachable from the fetched refs exist after
fetching instead of before: this allows us to distinguish between a
repository which is not up to date and a corrupted repository, and to
ensure that the repository is up to date and complete after the fetch.

In select_missing_refs() we must check which objects we have, instead of
checking which refs are up to date: otherwise, we might fetch objects
into the repository which exist in an alternate.

Signed-off-by: Andreas Gruenbacher <redacted>
---
 builtin/fetch.c |   82 ++++++++++++++++++++++++++-----------------------------
 1 files changed, 39 insertions(+), 43 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 6dc75d4..0f292b8 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -440,20 +440,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 }
 
 /*
- * We would want to bypass the object transfer altogether if
- * everything we are going to fetch already exists and is connected
- * locally.
- *
- * The refs we are going to fetch are in ref_map.  If running
+ * If running
  *
  *  $ git rev-list --objects --stdin --not --all
  *
  * (feeding all the refs in ref_map on its standard input)
  * does not error out, that means everything reachable from the
- * refs we are going to fetch exists and is connected to some of
- * our existing refs.
+ * refs we have fetched exists.
  */
-static int quickfetch(struct ref *ref_map)
+static int check_connectivity(struct ref *ref_map)
 {
 	struct child_process revlist;
 	struct ref *ref;

@@ -461,16 +456,6 @@ static int quickfetch(struct ref *ref_map)
 	const char *argv[] = {"rev-list",
 		"--quiet", "--objects", "--stdin", "--not", "--all", NULL};
 
-	/*
-	 * If we are deepening a shallow clone we already have these
-	 * objects reachable.  Running rev-list here will return with
-	 * a good (0) exit status and we'll bypass the fetch that we
-	 * really need to perform.  Claiming failure now will ensure
-	 * we perform the network exchange to deepen our history.
-	 */
-	if (depth)
-		return -1;
-
 	if (!ref_map)
 		return 0;

@@ -513,46 +498,57 @@ static int quickfetch(struct ref *ref_map)
 	return finish_command(&revlist) || err;
 }
 
-static void check_existing_refs(struct ref *ref_map) {
+static void select_missing_refs(struct ref *ref_map) {
 	struct ref *ref;
-	int done = 1;
 
-	if (!depth) {
-		for (ref = ref_map; ref; ref = ref->next) {
-			if (ref->peer_ref &&
-			    !is_null_sha1(ref->old_sha1) &&
-			    !hashcmp(ref->peer_ref->old_sha1, ref->old_sha1))
-				ref->dont_fetch = 1;
-			else
-				done = 0;
-		}
+	for (ref = ref_map; ref; ref = ref->next) {
+		if (has_sha1_file(ref->old_sha1))
+			ref->dont_fetch = 1;
 	}
-	if (done) {
-		/*
-		 * When deepening of a shallow repository is requested, local
-		 * and remote refs are likely to still be equal.  In addition,
-		 * we get here if all refs are up to date but quickfetch() did
-		 * not succeed.  In both cases, fetch all refs.
-		 */
-		for (ref = ref_map; ref; ref = ref->next)
+}
+
+static int select_all_refs(struct ref *ref_map) {
+	struct ref *ref;
+	int done = 1;
+
+	for (ref = ref_map; ref; ref = ref->next) {
+		if (ref->dont_fetch) {
 			ref->dont_fetch = 0;
+			done = 0;
+		}
 	}
+
+	return done;
 }
 
 static int fetch_refs(struct transport *transport, struct ref *ref_map)
 {
 	int ret;
 
-	ret = quickfetch(ref_map);
+	if (depth) {
+		/* When deepening a shallow clone, re-fetch all the refs:
+		 * it doesn't help to have the topmost commits of each ref.
+		 */
+		select_all_refs(ref_map);
+	} else
+		select_missing_refs(ref_map);
+	ret = transport_fetch_refs(transport, ref_map);
+	if (ret)
+		return ret;
+	ret = check_connectivity(ref_map);
 	if (ret) {
-		check_existing_refs(ref_map);
+		if (select_all_refs(ref_map))
+			return ret;
+		transport_unlock_pack(transport);
 		ret = transport_fetch_refs(transport, ref_map);
+		if (ret)
+			return ret;
 	}
-	if (!ret)
-		ret |= store_updated_refs(transport->url,
-				transport->remote->name,
-				ref_map);
+	ret = store_updated_refs(transport->url,
+				 transport->remote->name,
+				 ref_map);
 	transport_unlock_pack(transport);
+
 	return ret;
 }

-- 
1.7.0.2.273.gc2413

[PATCH 9/9] fetch: Make automatic tag following work with arbitrary refspecs

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:27

Make automatic tag following and the --tags and --no-tags options work
together with arbitrary refspecs: by default, tags will always be
fetched automatically, the --tags option will fetch all tags (and only
tags), and the --no-tags option will not fetch any tags.  This allows to
define how things are mapped and which things to fetch independently.

Signed-off-by: Andres Gruenbacher <redacted>
---
 builtin/fetch.c |   37 ++++++++++++++++++++++++++-----------
 1 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8c01876..9265d76 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -147,6 +147,7 @@ static void add_default_tags(const struct ref *remote_refs,
 			     struct string_list *existing_refs,
 			     struct string_list *peeled_map,
 			     struct ref *ref_map,
+			     int overwrite,
 			     struct ref ***tail)
 {
 	struct string_list mapped_refs = { NULL, 0, 0, 0 };

@@ -180,12 +181,8 @@ static void add_default_tags(const struct ref *remote_refs,
 	for (tag = &tags; *tag; ) {
 		ref = *tag;
 		peer_ref = ref->peer_ref;
-		if (!(string_list_has_string(existing_refs, peer_ref->name) ||
-		      string_list_has_string(&mapped_refs, ref->name))) {
-			struct string_list_item *item;
-
-			item = string_list_lookup(ref->name, peeled_map);
-			ref->automatic = !(item && will_fetch(ref_map, item->util));
+		if ((overwrite || !string_list_has_string(existing_refs, peer_ref->name)) &&
+		    !string_list_has_string(&mapped_refs, ref->name)) {
 			string_list_insert(peer_ref->name, existing_refs);
 			move_tag_to_tail(tag, tail);
 			continue;

@@ -218,6 +215,10 @@ int ref_is_automatic(const struct ref *ref) {
 	return ref->automatic;
 }
 
+int ref_is_tag(const struct ref *ref) {
+	return !prefixcmp(ref->name, "refs/tags/");
+}
+
 static struct ref *get_ref_map(struct transport *transport,
 			       struct refspec *refs, int ref_count, int tags,
 			       struct string_list *peeled_map,

@@ -231,7 +232,7 @@ static struct ref *get_ref_map(struct transport *transport,
 
 	const struct ref *remote_refs = transport_get_remote_refs(transport);
 
-	if (ref_count || tags == TAGS_SET) {
+	if (ref_count) {
 		for (i = 0; i < ref_count; i++) {
 			get_fetch_map(remote_refs, &refs[i], &tail, 0);
 			if (refs[i].dst && refs[i].dst[0])

@@ -240,8 +241,6 @@ static struct ref *get_ref_map(struct transport *transport,
 		/* Merge everything on the command line, but not --tags */
 		for (rm = ref_map; rm; rm = rm->next)
 			rm->merge = 1;
-		if (tags == TAGS_SET)
-			get_fetch_map(remote_refs, tag_refspec, &tail, 0);
 	} else {
 		/* Use the defaults */
 		struct remote *remote = transport->remote;

@@ -287,8 +286,24 @@ static struct ref *get_ref_map(struct transport *transport,
 		}
 	}
 
-	if (tags == TAGS_DEFAULT && *autotags)
-		add_default_tags(remote_refs, &existing_refs, peeled_map, ref_map, &tail);
+	if (tags == TAGS_SET) {
+		add_default_tags(remote_refs, &existing_refs, peeled_map, ref_map, 1, &tail);
+		discard_refs(&ref_map, 1, ref_is_tag);
+		free_refs(rm);
+	} else if (tags == TAGS_DEFAULT && *autotags) {
+		add_default_tags(remote_refs, &existing_refs, peeled_map, ref_map, 0, &tail);
+		for (rm = ref_map; rm; rm = rm->next) {
+			struct string_list_item *item;
+
+			if (prefixcmp(rm->name, "refs/tags/"))
+				continue;
+			item = string_list_lookup(rm->name, peeled_map);
+			rm->automatic = !(item && will_fetch(ref_map, item->util));
+		}
+	} else {
+		discard_refs(&ref_map, 0, ref_is_tag);
+		free_refs(rm);
+	}
 	string_list_clear(&existing_refs, 0);
 
 	ref_remove_duplicates(ref_map);

-- 
1.7.0.2.273.gc2413

[PATCH 5/9] fetch: Move loop checking which refs we have already

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:27

This loop fits better into get_ref_map(); move it there.

Signed-off-by: Andreas Gruenbacher <redacted>
---
 builtin/fetch.c |   48 ++++++++++++++++++++++++------------------------
 1 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index fc6efc7..6dc75d4 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -115,6 +115,15 @@ static void add_merge_config(struct ref **head,
 	}
 }
 
+static int add_existing(const char *refname, const unsigned char *sha1,
+			int flag, void *cbdata)
+{
+	struct string_list *list = (struct string_list *)cbdata;
+	struct string_list_item *item = string_list_insert(refname, list);
+	item->util = (void *)sha1;
+	return 0;
+}
+
 static void find_non_local_tags(struct transport *transport,
 			struct ref **head,
 			struct ref ***tail);

@@ -123,6 +132,7 @@ static struct ref *get_ref_map(struct transport *transport,
 			       struct refspec *refs, int ref_count, int tags,
 			       int *autotags)
 {
+	struct string_list existing_refs = { NULL, 0, 0, 0 };
 	int i;
 	struct ref *rm;
 	struct ref *ref_map = NULL;

@@ -176,6 +186,20 @@ static struct ref *get_ref_map(struct transport *transport,
 		find_non_local_tags(transport, &ref_map, &tail);
 	ref_remove_duplicates(ref_map);
 
+	for_each_ref(add_existing, &existing_refs);
+	for (rm = ref_map; rm; rm = rm->next) {
+		if (rm->peer_ref) {
+			struct string_list_item *peer_item;
+
+			peer_item = string_list_lookup(rm->peer_ref->name,
+						       &existing_refs);
+			if (peer_item)
+				hashcpy(rm->peer_ref->old_sha1,
+					peer_item->util);
+		}
+	}
+	string_list_clear(&existing_refs, 0);
+
 	return ref_map;
 }

@@ -554,15 +578,6 @@ static int prune_refs(struct transport *transport, struct ref *ref_map)
 	return result;
 }
 
-static int add_existing(const char *refname, const unsigned char *sha1,
-			int flag, void *cbdata)
-{
-	struct string_list *list = (struct string_list *)cbdata;
-	struct string_list_item *item = string_list_insert(refname, list);
-	item->util = (void *)sha1;
-	return 0;
-}
-
 static int will_fetch(struct ref **head, const unsigned char *sha1)
 {
 	struct ref *rm = *head;

@@ -696,14 +711,9 @@ static int truncate_fetch_head(void)
 static int do_fetch(struct transport *transport,
 		    struct refspec *refs, int ref_count)
 {
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
-	struct string_list_item *peer_item = NULL;
 	struct ref *ref_map;
-	struct ref *rm;
 	int autotags = (transport->remote->fetch_tags == 1);
 
-	for_each_ref(add_existing, &existing_refs);
-
 	if (transport->remote->fetch_tags == 2 && tags != TAGS_UNSET)
 		tags = TAGS_SET;
 	if (transport->remote->fetch_tags == -1)

@@ -723,16 +733,6 @@ static int do_fetch(struct transport *transport,
 	if (!update_head_ok)
 		check_not_current_branch(ref_map);
 
-	for (rm = ref_map; rm; rm = rm->next) {
-		if (rm->peer_ref) {
-			peer_item = string_list_lookup(rm->peer_ref->name,
-						       &existing_refs);
-			if (peer_item)
-				hashcpy(rm->peer_ref->old_sha1,
-					peer_item->util);
-		}
-	}
-
 	if (tags == TAGS_DEFAULT && autotags)
 		transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, "1");
 	if (fetch_refs(transport, ref_map)) {

-- 
1.7.0.2.273.gc2413

[PATCH 3/9] fetch: Fix minor memory leak

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:27

A temporary struct ref is allocated in store_updated_refs() but not
freed.

Signed-off-by: Andreas Gruenbacher <redacted>
---
 builtin/fetch.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 660af84..957be9f 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -389,9 +389,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 				fputc(url[i], fp);
 		fputc('\n', fp);
 
-		if (ref)
+		if (ref) {
 			rc |= update_local_ref(ref, what, note);
-		else
+			free(ref);
+		} else
 			sprintf(note, "* %-*s %-*s -> FETCH_HEAD",
 				TRANSPORT_SUMMARY_WIDTH, *kind ? kind : "branch",
 				 REFCOL_WIDTH, *what ? what : "HEAD");

-- 
1.7.0.2.273.gc2413

[PATCH 2/9] fetch: Properly initialize refspec on stack

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:27

Signed-off-by: Andreas Gruenbacher <redacted>
---
 builtin/fetch.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 77031e2..660af84 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -107,10 +107,8 @@ static void add_merge_config(struct ref **head,
 		 * there is no entry in the resulting FETCH_HEAD marked
 		 * for merging.
 		 */
+		memset(&refspec, 0, sizeof(refspec));
 		refspec.src = branch->merge[i]->src;
-		refspec.dst = NULL;
-		refspec.pattern = 0;
-		refspec.force = 0;
 		get_fetch_map(remote_refs, &refspec, tail, 1);
 		for (rm = *old_tail; rm; rm = rm->next)
 			rm->merge = 1;

-- 
1.7.0.2.273.gc2413

[PATCH 7/9] fetch: Use the same ref map for all branches and tags

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:27

With automatic tag following and the standard refspec for remotes
(+refs/heads/*:refs/remotes/<remote>/*), we fetch all branches and tags
that we know we need, and then we check which additional tags have
become reachable in a separate second pass.  The refspec used for tags
is always hardcoded (+refs/tags/*:refs/tags/*).

Change this to compute the mapping between remote and peer refs only
once, and flag tags which may become reachable as "automatic".  After
the first pass, go through all "automatic" tags in the mapping to see
which have actually become reachable, and process these.

This change preserves the existing behavior, but allows to subsequently
fix the handling of refspecs which match tags, like
+refs/tags/*:refs/tags/<remote>/*.

Signed-off-by: Andreas Gruenbacher <redacted>
---
 builtin/fetch.c |  286 +++++++++++++++++++++++++++++--------------------------
 cache.h         |    3 +-
 2 files changed, 155 insertions(+), 134 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 0f292b8..8728153 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c

@@ -124,12 +124,97 @@ static int add_existing(const char *refname, const unsigned char *sha1,
 	return 0;
 }
 
-static void find_non_local_tags(struct transport *transport,
-			struct ref **head,
-			struct ref ***tail);
+static void move_tag_to_tail(struct ref **tag, struct ref ***tail) {
+	struct ref *ref = *tag;
+
+	*tag = ref->next;
+	**tail = ref;
+	*tail = &ref->next;
+	ref->next = NULL;
+}
+
+static int will_fetch(struct ref *ref_map, const unsigned char *sha1)
+{
+	struct ref *ref;
+
+	for (ref = ref_map; ref; ref = ref->next)
+		if (!hashcmp(ref->old_sha1, sha1))
+			return 1;
+	return 0;
+}
+
+static void add_default_tags(const struct ref *remote_refs,
+			     struct string_list *existing_refs,
+			     struct string_list *peeled_map,
+			     struct ref *ref_map,
+			     struct ref ***tail)
+{
+	struct ref *tags = NULL, **tags_tail = &tags;
+	struct ref *ref, *peer_ref, **tag;
+
+	for (ref = (struct ref *)remote_refs; ref; ref = ref->next) {
+		int len;
+
+		if (!ref->next)
+			break;
+		len = strlen(ref->name);
+		if (!strncmp(ref->name, ref->next->name, len) &&
+		    !strcmp(ref->next->name + len, "^{}")) {
+			struct string_list_item *item;
+
+			item = string_list_append(ref->name, peeled_map);
+			item->util = ref->next->old_sha1;
+		}
+	}
+	sort_string_list(peeled_map);
+
+	for (ref = ref_map; ref; ref = ref->next)
+		if (ref->peer_ref)
+			string_list_insert(ref->peer_ref->name, existing_refs);
+
+	get_fetch_map(remote_refs, tag_refspec, &tags_tail, 0);
+	for (tag = &tags; *tag; ) {
+		ref = *tag;
+		peer_ref = ref->peer_ref;
+		if (!string_list_has_string(existing_refs, peer_ref->name)) {
+			struct string_list_item *item;
+
+			item = string_list_lookup(ref->name, peeled_map);
+			ref->automatic = !(item && will_fetch(ref_map, item->util));
+			string_list_insert(peer_ref->name, existing_refs);
+			move_tag_to_tail(tag, tail);
+			continue;
+		}
+		tag = &(*tag)->next;
+	}
+
+	free_refs(tags);
+}
+
+static void discard_refs(struct ref **ref_map, int not, int (*fn)(const struct ref *)) {
+	struct ref *discard = NULL;
+
+	while (*ref_map) {
+		struct ref *ref = *ref_map;
+
+		if (!not ^ !fn(ref)) {
+			*ref_map = ref->next;
+			ref->next = discard;
+			discard = ref;
+			continue;
+		}
+		ref_map = &ref->next;
+	}
+	free_refs(discard);
+}
+
+int ref_is_automatic(const struct ref *ref) {
+	return ref->automatic;
+}
 
 static struct ref *get_ref_map(struct transport *transport,
 			       struct refspec *refs, int ref_count, int tags,
+			       struct string_list *peeled_map,
 			       int *autotags)
 {
 	struct string_list existing_refs = { NULL, 0, 0, 0 };

@@ -182,9 +267,6 @@ static struct ref *get_ref_map(struct transport *transport,
 			tail = &ref_map->next;
 		}
 	}
-	if (tags == TAGS_DEFAULT && *autotags)
-		find_non_local_tags(transport, &ref_map, &tail);
-	ref_remove_duplicates(ref_map);
 
 	for_each_ref(add_existing, &existing_refs);
 	for (rm = ref_map; rm; rm = rm->next) {

@@ -198,8 +280,13 @@ static struct ref *get_ref_map(struct transport *transport,
 					peer_item->util);
 		}
 	}
+
+	if (tags == TAGS_DEFAULT && *autotags)
+		add_default_tags(remote_refs, &existing_refs, peeled_map, ref_map, &tail);
 	string_list_clear(&existing_refs, 0);
 
+	ref_remove_duplicates(ref_map);
+
 	return ref_map;
 }

@@ -353,12 +440,22 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	for (rm = ref_map; rm; rm = rm->next) {
 		struct ref *ref = NULL;
 
+		if (rm->automatic) {
+			if (has_sha1_file(rm->old_sha1))
+				rm->automatic = 0;
+			else
+				continue;
+		}
+
 		if (rm->peer_ref) {
 			ref = xcalloc(1, sizeof(*ref) + strlen(rm->peer_ref->name) + 1);
 			strcpy(ref->name, rm->peer_ref->name);
 			hashcpy(ref->old_sha1, rm->peer_ref->old_sha1);
 			hashcpy(ref->new_sha1, rm->old_sha1);
 			ref->force = rm->peer_ref->force;
+
+			/* Remember we have updated this ref */
+			hashcpy(rm->peer_ref->old_sha1, rm->old_sha1);
 		}
 
 		commit = lookup_commit_reference_gently(rm->old_sha1, 1);

@@ -439,6 +536,15 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
 	return rc;
 }
 
+static int contains_only_automatic_refs(struct ref *ref_map) {
+	struct ref *ref;
+
+	for (ref = ref_map; ref; ref = ref->next)
+		if (!ref->automatic)
+			return 0;
+	return 1;
+}
+
 /*
  * If running
  *

@@ -456,7 +562,7 @@ static int check_connectivity(struct ref *ref_map)
 	const char *argv[] = {"rev-list",
 		"--quiet", "--objects", "--stdin", "--not", "--all", NULL};
 
-	if (!ref_map)
+	if (contains_only_automatic_refs(ref_map))
 		return 0;
 
 	memset(&revlist, 0, sizeof(revlist));

@@ -479,6 +585,9 @@ static int check_connectivity(struct ref *ref_map)
 	sigchain_push(SIGPIPE, SIG_IGN);
 
 	for (ref = ref_map; ref; ref = ref->next) {
+		if (ref->automatic)
+			continue;
+
 		if (write_in_full(revlist.in, sha1_to_hex(ref->old_sha1), 40) < 0 ||
 		    write_str_in_full(revlist.in, "\n") < 0) {
 			if (errno != EPIPE && errno != EINVAL)

@@ -501,10 +610,9 @@ static int check_connectivity(struct ref *ref_map)
 static void select_missing_refs(struct ref *ref_map) {
 	struct ref *ref;
 
-	for (ref = ref_map; ref; ref = ref->next) {
-		if (has_sha1_file(ref->old_sha1))
-			ref->dont_fetch = 1;
-	}
+	for (ref = ref_map; ref; ref = ref->next)
+		ref->dont_fetch = ref->automatic ||
+				  has_sha1_file(ref->old_sha1);
 }
 
 static int select_all_refs(struct ref *ref_map) {

@@ -512,7 +620,7 @@ static int select_all_refs(struct ref *ref_map) {
 	int done = 1;
 
 	for (ref = ref_map; ref; ref = ref->next) {
-		if (ref->dont_fetch) {
+		if (ref->dont_fetch && !ref->automatic) {
 			ref->dont_fetch = 0;
 			done = 0;
 		}

@@ -574,111 +682,6 @@ static int prune_refs(struct transport *transport, struct ref *ref_map)
 	return result;
 }
 
-static int will_fetch(struct ref **head, const unsigned char *sha1)
-{
-	struct ref *rm = *head;
-	while (rm) {
-		if (!hashcmp(rm->old_sha1, sha1))
-			return 1;
-		rm = rm->next;
-	}
-	return 0;
-}
-
-struct tag_data {
-	struct ref **head;
-	struct ref ***tail;
-};
-
-static int add_to_tail(struct string_list_item *item, void *cb_data)
-{
-	struct tag_data *data = (struct tag_data *)cb_data;
-	struct ref *rm = NULL;
-
-	/* We have already decided to ignore this item */
-	if (!item->util)
-		return 0;
-
-	rm = alloc_ref(item->string);
-	rm->peer_ref = alloc_ref(item->string);
-	hashcpy(rm->old_sha1, item->util);
-
-	**data->tail = rm;
-	*data->tail = &rm->next;
-
-	return 0;
-}
-
-static void find_non_local_tags(struct transport *transport,
-			struct ref **head,
-			struct ref ***tail)
-{
-	struct string_list existing_refs = { NULL, 0, 0, 0 };
-	struct string_list remote_refs = { NULL, 0, 0, 0 };
-	struct tag_data data = {head, tail};
-	const struct ref *ref;
-	struct string_list_item *item = NULL;
-
-	for_each_ref(add_existing, &existing_refs);
-	for (ref = transport_get_remote_refs(transport); ref; ref = ref->next) {
-		if (prefixcmp(ref->name, "refs/tags"))
-			continue;
-
-		/*
-		 * The peeled ref always follows the matching base
-		 * ref, so if we see a peeled ref that we don't want
-		 * to fetch then we can mark the ref entry in the list
-		 * as one to ignore by setting util to NULL.
-		 */
-		if (!suffixcmp(ref->name, "^{}")) {
-			if (item && !has_sha1_file(ref->old_sha1) &&
-			    !will_fetch(head, ref->old_sha1) &&
-			    !has_sha1_file(item->util) &&
-			    !will_fetch(head, item->util))
-				item->util = NULL;
-			item = NULL;
-			continue;
-		}
-
-		/*
-		 * If item is non-NULL here, then we previously saw a
-		 * ref not followed by a peeled reference, so we need
-		 * to check if it is a lightweight tag that we want to
-		 * fetch.
-		 */
-		if (item && !has_sha1_file(item->util) &&
-		    !will_fetch(head, item->util))
-			item->util = NULL;
-
-		item = NULL;
-
-		/* skip duplicates and refs that we already have */
-		if (string_list_has_string(&remote_refs, ref->name) ||
-		    string_list_has_string(&existing_refs, ref->name))
-			continue;
-
-		item = string_list_insert(ref->name, &remote_refs);
-		item->util = (void *)ref->old_sha1;
-	}
-	string_list_clear(&existing_refs, 0);
-
-	/*
-	 * We may have a final lightweight tag that needs to be
-	 * checked to see if it needs fetching.
-	 */
-	if (item && !has_sha1_file(item->util) &&
-	    !will_fetch(head, item->util))
-		item->util = NULL;
-
-	/*
-	 * For all the tags in the remote_refs string list, call
-	 * add_to_tail to add them to the list of refs to be fetched
-	 */
-	for_each_string_list(add_to_tail, &remote_refs, &data);
-
-	string_list_clear(&remote_refs, 0);
-}
-
 static void check_not_current_branch(struct ref *ref_map)
 {
 	struct branch *current_branch = branch_get(NULL);

@@ -704,9 +707,29 @@ static int truncate_fetch_head(void)
 	return 0;
 }
 
+static int need_additional_refs(struct ref *ref_map, struct string_list *peeled_map) {
+	int additional_refs = 0;
+	struct ref *ref;
+
+	for (ref = ref_map; ref; ref = ref->next) {
+		struct string_list_item *item;
+
+		if (!ref->automatic)
+			continue;
+		item = string_list_lookup(ref->name, peeled_map);
+		if (item && has_sha1_file(item->util)) {
+			ref->automatic = 0;
+			additional_refs = 1;
+		}
+	}
+
+	return additional_refs;
+}
+
 static int do_fetch(struct transport *transport,
 		    struct refspec *refs, int ref_count)
 {
+	struct string_list peeled_map = { NULL, 0, 0, 0 };
 	struct ref *ref_map;
 	int autotags = (transport->remote->fetch_tags == 1);

@@ -725,7 +748,7 @@ static int do_fetch(struct transport *transport,
 			return errcode;
 	}
 
-	ref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);
+	ref_map = get_ref_map(transport, refs, ref_count, tags, &peeled_map, &autotags);
 	if (!update_head_ok)
 		check_not_current_branch(ref_map);

@@ -735,23 +758,20 @@ static int do_fetch(struct transport *transport,
 		free_refs(ref_map);
 		return 1;
 	}
-	if (prune)
-		prune_refs(transport, ref_map);
-	free_refs(ref_map);
-
-	/* if neither --no-tags nor --tags was specified, do automated tag
-	 * following ... */
-	if (tags == TAGS_DEFAULT && autotags) {
-		struct ref **tail = &ref_map;
-		ref_map = NULL;
-		find_non_local_tags(transport, &ref_map, &tail);
-		if (ref_map) {
-			transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
-			transport_set_option(transport, TRANS_OPT_DEPTH, "0");
-			fetch_refs(transport, ref_map);
+	if (need_additional_refs(ref_map, &peeled_map)) {
+		transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
+		transport_set_option(transport, TRANS_OPT_DEPTH, "0");
+		if (fetch_refs(transport, ref_map)) {
+			free_refs(ref_map);
+			return 1;
 		}
-		free_refs(ref_map);
 	}
+	if (prune) {
+		discard_refs(&ref_map, 0, ref_is_automatic);
+		prune_refs(transport, ref_map);
+	}
+	string_list_clear(&peeled_map, 0);
+	free_refs(ref_map);
 
 	return 0;
 }

diff --git a/cache.h b/cache.h
index abfc682..1aa3000 100644
--- a/cache.h
+++ b/cache.h

@@ -870,7 +870,8 @@ struct ref {
 		merge:1,
 		nonfastforward:1,
 		deletion:1,
-		dont_fetch:1;
+		dont_fetch:1,
+		automatic:1;
 	enum {
 		REF_STATUS_NONE = 0,
 		REF_STATUS_OK,

-- 
1.7.0.2.273.gc2413

Re: [PATCH 0/9] Multiple remotes without conflicts

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:48:27

Andreas Gruenbacher [off-list ref] wrote:

I'm still trying to find a simple and painless way of sharing the object
store among multiple repositories: the idea is to have a "parent"
repository which contains the actual object store, and a number of
"child" repositories which link to that object store.  The obvious
problem is garbage collection: we can only garbage collect the parent
once it has all refs of all its children.

One way of ensuring that is to make each child a "remote" of the parent,
and to fetch all remotes first.  This works for branches, but not for
tags or for the reflog.

This just feels like the wrong solution.

Why can't we have a "$GIT_DIR/children" subdirectory with a symlink
or file-containing-path to each child repository.  Modify the fsck
and gc paths to include these additional reference and reflog spaces,
and that's that.

Child registration is then just a matter of installing the symlink
in the parent, or removing it, and gc/fsck never needs to worry
about a fetch up front in order for it to be accurate.
 
-- 
Shawn.

Re: [PATCH 6/9] fetch: Check if all objects exist after fetching

From: Shawn O. Pearce <hidden>
Date: 2016-06-15 22:48:27

Andreas Gruenbacher [off-list ref] wrote:

Check if all objects reachable from the fetched refs exist after
fetching instead of before: this allows us to distinguish between a
repository which is not up to date and a corrupted repository, and to
ensure that the repository is up to date and complete after the fetch.

I'm against this particular change because it looks like it breaks
the idea of "quickfetch", which we introduced to support faster
fetches from the parent repository into a shared clone on the
same disk.
 
-- 
Shawn.

Re: [PATCH 6/9] fetch: Check if all objects exist after fetching

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:27

On Thursday 18 March 2010 20:08:16 Shawn O. Pearce wrote:

Andreas Gruenbacher [off-list ref] wrote:

quoted

Check if all objects reachable from the fetched refs exist after
fetching instead of before: this allows us to distinguish between a
repository which is not up to date and a corrupted repository, and to
ensure that the repository is up to date and complete after the fetch.

I'm against this particular change because it looks like it breaks
the idea of "quickfetch", which we introduced to support faster
fetches from the parent repository into a shared clone on the
same disk.

I think you misunderstand the patch.  Before the patch, we were doing a rev-
list to determine if all objects needed are present.  If rev-list fails, this 
can have two reasons: (a) some of the branches or tags needed do not exist, 
(b) all the branches and tags needed do exist, but other objects further up 
the tree are missing (i.e., a corrupted repository).

The patch changes that to first check which needed objects are missing (with 
has_sha1_file()), which is very efficient, by then fetching the objects which 
surely need to be fetched, and by then checking the repository consistency 
with rev-list.  If rev-list then fails, which should only happen in the rarest 
cases, we know that we need to fetch all branches and tags so that we are sure 
to catch missing objects further up the tree.

So we never fetch more than we did before, and in some cases, we fetch less. 
We are also guaranteed to end up with a consistent repository in the end. (The 
old logic does not always guarantee that AFAICT: there seems to be one corner 
case where a fetch succeeds without retrieving missing objects further up the 
tree.)

Thanks,
Andreas

Re: [PATCH 0/9] Multiple remotes without conflicts

From: Andreas Gruenbacher <hidden>
Date: 2016-06-15 22:48:27

On Thursday 18 March 2010 20:07:12 Shawn O. Pearce wrote:

Andreas Gruenbacher [off-list ref] wrote:

quoted

I'm still trying to find a simple and painless way of sharing the object
store among multiple repositories: the idea is to have a "parent"
repository which contains the actual object store, and a number of
"child" repositories which link to that object store.  The obvious
problem is garbage collection: we can only garbage collect the parent
once it has all refs of all its children.

One way of ensuring that is to make each child a "remote" of the parent,
and to fetch all remotes first.  This works for branches, but not for
tags or for the reflog.

This just feels like the wrong solution.

Why can't we have a "$GIT_DIR/children" subdirectory with a symlink
or file-containing-path to each child repository.  Modify the fsck
and gc paths to include these additional reference and reflog spaces,
and that's that.

Child registration is then just a matter of installing the symlink
in the parent, or removing it, and gc/fsck never needs to worry
about a fetch up front in order for it to be accurate.

Another way would be to add some kind of "child" flag to remotes, and to teach 
gc to fetch such children before doing its work (or check that they are up-to-
date).  I think it's much nicer to construct the "parent" repository in such a 
way that it includes all relevant refs locally: it will just look and behave 
like a "normal" repository.

Thanks,
Andreas

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help