git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v2 00/13] Many promisor remotes
@ 2019-01-22 14:41 Christian Couder
  2019-01-22 14:42 ` [PATCH v2 01/13] fetch: fix extensions.partialclone name in error message Christian Couder
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:41 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli

This path series is a follow up from the "remote odb" patch series
that I sent last year, which were a follow up from previous
series. See the links section for more information.

The goal of this patch series is to make it possible to have and to
fetch missing objects from multiple remotes instead of only one.

For now the fetch order is the order of the remotes in the config.

I selected the name "Promisor remote" over "Partial clone remote"
because it is shorter and because it is not only about cloning.

The existing extensions.partialclone is respected, but it is not
written in the config when a partial clone or fetch is made. Instead
remote.<name>.promisor is set to "true". This may create a
compatibility issue, but it makes it possible to start using many
promisor remotes by just cloning and fetching from different remotes
with partial clone filters. The compatibility issue could be resolved
in a future iteration by just setting extensions.partialclone instead
of remote.<name>.promisor the first time a promisor remote is used.

In general I have tried to change as few things as possible.

High level changes since the V1
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  - added documentation patches
  
  - fixed issues when fetching from a new promisor remote
  
  - added a test to check that remote.<name>.promisor is set to "true"
    when fetching from a new promisor remote

  - renamed functions in promisor-remote.{c,h} to make the names more
    coherent and easier to understand

  - added 2 small typo fix patches that I sent separately

High level overview of this patch series
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  - Patch 1/13 and 2/13:

These are 2 new small patches that I sent separately. I am adding them
here as the other patches were developed on top of them and it creates
a small conflict is these 2 patches are removed.

  - Patch 3/13:

This patch, which hasn't changed since V1, makes functions in
fetch-object.c return an error code, which is necessary to later tell
that they failed and try another promisor remote when there is more
than one. This could also just be seen as a fix to these functions.

  - Patch 4/13:

This introduces the minimum infrastructure for promisor remotes. There
has been a number of renames and improvements since V1.

  - Patch 5/13, 6/13 and 7/13:

This add a few missing bits in the promisor remote infrastructure that
will be needed in the following patches. There has been a number of
renames and improvements since V1.

  - Patch 8/13:

This replaces the previous interface to use only one promisor remote
defined in extensions.partialclone by the new interface created by the
previous patches. There has been a few renames since V1.

  - Patch 9/13:

This replaces the way a partial clone filter was handled by a new way
based on the previous patches that support more than one partial clone
filter. There has been a few renames since V1.

  - Patch 10/13:

This new patch removes the limitation in builtin/fetch.c to have only
one promisor remote.

  - Patch 11/13:

This adds test cases that shows that now more than one promisor remote
can be used and that remote.<name>.promisor is set to "true" when
fetching from a new promisor remote.

  - Patch 12/13 and 13/13:

These are new documentation patches, to explain how things can work
with more than one promisor remote.

Links
~~~~~

This patch series on GitHub:

V2: https://github.com/chriscool/git/commits/many-promisor-remotes
V1: https://github.com/chriscool/git/commits/many-promisor-remotes17

On the mailing list:

V1: https://public-inbox.org/git/20181211052746.16218-1-chriscool@tuxfamily.org/

This patch series is a follow up from the discussions related to
the remote odb V4 patch series:

https://public-inbox.org/git/20180802061505.2983-1-chriscool@tuxfamily.org/

Especially in:

https://public-inbox.org/git/CAP8UFD3nrhjANwNDqTwx5ZtnZNcnbAFqUN=u=LrvzuH4+3wQQA@mail.gmail.com/

I said that I would like to work on things in the following order:

  1) Teaching partial clone to attempt to fetch missing objects from
multiple remotes instead of only one using the order in the config.

  2) Simplifying the protocol for fetching missing objects so that it
can be satisfied by a lighter weight object storage system than a full
Git server.

  3) Making it possible to explicitly define an order in which the
remotes are accessed.

  4) Making the criteria for what objects can be missing more
aggressive, so that I can "git add" a large file and work with it
using Git without even having a second copy of that object in my local
object store.

And this patch series is about the 1).

The previous remote odb patch series on GitHub:

V5: https://github.com/chriscool/git/commits/remote-odb
V4: https://github.com/chriscool/git/commits/remote-odb5
V3: https://github.com/chriscool/git/commits/remote-odb3
V2: https://github.com/chriscool/git/commits/remote-odb2
V1: https://github.com/chriscool/git/commits/remote-odb1

Discussions related to previous versions of the odb patch series:

V4: https://public-inbox.org/git/20180802061505.2983-1-chriscool@tuxfamily.org/
V3: https://public-inbox.org/git/20180713174959.16748-1-chriscool@tuxfamily.org/
V2: https://public-inbox.org/git/20180630083542.20347-1-chriscool@tuxfamily.org/
V1: https://public-inbox.org/git/20180623121846.19750-1-chriscool@tuxfamily.org/


Christian Couder (13):
  fetch: fix extensions.partialclone name in error message
  partial-clone: add missing 'is' in doc
  fetch-object: make functions return an error code
  Add initial support for many promisor remotes
  promisor-remote: implement promisors_get_direct()
  promisor-remote: add promisor_remote_reinit()
  promisor-remote: use repository_format_partial_clone
  Use promisor_remote_get_direct() and has_promisor_remote()
  promisor-remote: parse remote.*.partialclonefilter
  builtin/fetch: remove unique promisor remote limitation
  t0410: test fetching from many promisor remotes
  partial-clone: add multiple remotes in the doc
  remote: add promisor and partial clone config to the doc

 Documentation/config/remote.txt           |   8 ++
 Documentation/technical/partial-clone.txt |  85 +++++++++----
 Makefile                                  |   1 +
 builtin/cat-file.c                        |   5 +-
 builtin/fetch.c                           |  31 ++---
 builtin/gc.c                              |   3 +-
 builtin/repack.c                          |   3 +-
 cache-tree.c                              |   3 +-
 connected.c                               |   3 +-
 fetch-object.c                            |  13 +-
 fetch-object.h                            |   4 +-
 list-objects-filter-options.c             |  51 ++++----
 list-objects-filter-options.h             |   3 +-
 packfile.c                                |   3 +-
 promisor-remote.c                         | 148 ++++++++++++++++++++++
 promisor-remote.h                         |  22 ++++
 sha1-file.c                               |  14 +-
 t/t0410-partial-clone.sh                  |  49 ++++++-
 t/t5601-clone.sh                          |   3 +-
 t/t5616-partial-clone.sh                  |   4 +-
 unpack-trees.c                            |   6 +-
 21 files changed, 365 insertions(+), 97 deletions(-)
 create mode 100644 promisor-remote.c
 create mode 100644 promisor-remote.h

-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 01/13] fetch: fix extensions.partialclone name in error message
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 02/13] partial-clone: add missing 'is' in doc Christian Couder
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli

There is "extensions.partialclone" and "core.partialCloneFilter", but
not "core.partialclone". Only "extensions.partialclone" is meant to
contain a remote name.

While at it, let's wrap the relevant code lines to keep them at a
reasonable length.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/fetch.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index e0140327aa..c0ade48f5d 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1478,7 +1478,8 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 	 */
 	if (strcmp(remote->name, repository_format_partial_clone)) {
 		if (filter_options.choice)
-			die(_("--filter can only be used with the remote configured in core.partialClone"));
+			die(_("--filter can only be used with the remote "
+			      "configured in extensions.partialclone"));
 		return;
 	}
 
@@ -1646,7 +1647,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		result = fetch_one(remote, argc, argv, prune_tags_ok);
 	} else {
 		if (filter_options.choice)
-			die(_("--filter can only be used with the remote configured in core.partialClone"));
+			die(_("--filter can only be used with the remote "
+			      "configured in extensions.partialclone"));
 		/* TODO should this also die if we have a previous partial-clone? */
 		result = fetch_multiple(&list);
 	}
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 02/13] partial-clone: add missing 'is' in doc
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
  2019-01-22 14:42 ` [PATCH v2 01/13] fetch: fix extensions.partialclone name in error message Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 03/13] fetch-object: make functions return an error code Christian Couder
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/technical/partial-clone.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/technical/partial-clone.txt b/Documentation/technical/partial-clone.txt
index 1ef66bd788..896c7b3878 100644
--- a/Documentation/technical/partial-clone.txt
+++ b/Documentation/technical/partial-clone.txt
@@ -114,7 +114,7 @@ their "<name>.pack" and "<name>.idx" files.
   that it has, either because the local repository has that object in one of
   its promisor packfiles, or because another promisor object refers to it.
 +
-When Git encounters a missing object, Git can see if it a promisor object
+When Git encounters a missing object, Git can see if it is a promisor object
 and handle it appropriately.  If not, Git can report a corruption.
 +
 This means that there is no need for the client to explicitly maintain an
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 03/13] fetch-object: make functions return an error code
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
  2019-01-22 14:42 ` [PATCH v2 01/13] fetch: fix extensions.partialclone name in error message Christian Couder
  2019-01-22 14:42 ` [PATCH v2 02/13] partial-clone: add missing 'is' in doc Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 04/13] Add initial support for many promisor remotes Christian Couder
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli, Christian Couder

From: Christian Couder <christian.couder@gmail.com>

The callers of the fetch_object() and fetch_objects() might
be interested in knowing if these functions succeeded or not.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 fetch-object.c | 13 ++++++++-----
 fetch-object.h |  4 ++--
 sha1-file.c    |  4 ++--
 3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/fetch-object.c b/fetch-object.c
index 4266548800..eac4d448ef 100644
--- a/fetch-object.c
+++ b/fetch-object.c
@@ -5,11 +5,12 @@
 #include "transport.h"
 #include "fetch-object.h"
 
-static void fetch_refs(const char *remote_name, struct ref *ref)
+static int fetch_refs(const char *remote_name, struct ref *ref)
 {
 	struct remote *remote;
 	struct transport *transport;
 	int original_fetch_if_missing = fetch_if_missing;
+	int res;
 
 	fetch_if_missing = 0;
 	remote = remote_get(remote_name);
@@ -19,12 +20,14 @@ 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);
+	res = transport_fetch_refs(transport, ref);
 	fetch_if_missing = original_fetch_if_missing;
+
+	return res;
 }
 
-void fetch_objects(const char *remote_name, const struct object_id *oids,
-		   int oid_nr)
+int fetch_objects(const char *remote_name, const struct object_id *oids,
+		  int oid_nr)
 {
 	struct ref *ref = NULL;
 	int i;
@@ -36,5 +39,5 @@ void fetch_objects(const char *remote_name, const struct object_id *oids,
 		new_ref->next = ref;
 		ref = new_ref;
 	}
-	fetch_refs(remote_name, ref);
+	return fetch_refs(remote_name, ref);
 }
diff --git a/fetch-object.h b/fetch-object.h
index d6444caa5a..7bcc7cadb0 100644
--- a/fetch-object.h
+++ b/fetch-object.h
@@ -3,7 +3,7 @@
 
 struct object_id;
 
-void fetch_objects(const char *remote_name, const struct object_id *oids,
-		   int oid_nr);
+int fetch_objects(const char *remote_name, const struct object_id *oids,
+		  int oid_nr);
 
 #endif
diff --git a/sha1-file.c b/sha1-file.c
index 386b96e1d7..972f26e931 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -1304,8 +1304,8 @@ int oid_object_info_extended(struct repository *r, const struct object_id *oid,
 		if (fetch_if_missing && repository_format_partial_clone &&
 		    !already_retried && r == the_repository) {
 			/*
-			 * TODO Investigate having fetch_object() return
-			 * TODO error/success and stopping the music here.
+			 * TODO Investigate checking fetch_object() return
+			 * TODO value and stopping on error here.
 			 * TODO Pass a repository struct through fetch_object,
 			 * such that arbitrary repositories work.
 			 */
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 04/13] Add initial support for many promisor remotes
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
                   ` (2 preceding siblings ...)
  2019-01-22 14:42 ` [PATCH v2 03/13] fetch-object: make functions return an error code Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 05/13] promisor-remote: implement promisors_get_direct() Christian Couder
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli, Christian Couder

From: Christian Couder <christian.couder@gmail.com>

The promisor-remote.{c,h} files will contain functions to
manage many promisor remotes.

We expect that there will not be a lot of promisor remotes,
so it is ok to use a simple linked list to manage them.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Makefile          |   1 +
 promisor-remote.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++
 promisor-remote.h |  17 ++++++++
 3 files changed, 118 insertions(+)
 create mode 100644 promisor-remote.c
 create mode 100644 promisor-remote.h

diff --git a/Makefile b/Makefile
index 1a44c811aa..565a5fbb4e 100644
--- a/Makefile
+++ b/Makefile
@@ -949,6 +949,7 @@ LIB_OBJS += preload-index.o
 LIB_OBJS += pretty.o
 LIB_OBJS += prio-queue.o
 LIB_OBJS += progress.o
+LIB_OBJS += promisor-remote.o
 LIB_OBJS += prompt.o
 LIB_OBJS += protocol.o
 LIB_OBJS += quote.o
diff --git a/promisor-remote.c b/promisor-remote.c
new file mode 100644
index 0000000000..d2f574651e
--- /dev/null
+++ b/promisor-remote.c
@@ -0,0 +1,100 @@
+#include "cache.h"
+#include "promisor-remote.h"
+#include "config.h"
+
+static struct promisor_remote *promisors;
+static struct promisor_remote **promisors_tail = &promisors;
+
+struct promisor_remote *promisor_remote_new(const char *remote_name)
+{
+	struct promisor_remote *o;
+
+	o = xcalloc(1, sizeof(*o));
+	o->remote_name = xstrdup(remote_name);
+
+	*promisors_tail = o;
+	promisors_tail = &o->next;
+
+	return o;
+}
+
+static struct promisor_remote *promisor_remote_look_up(const char *remote_name,
+						       struct promisor_remote **previous)
+{
+	struct promisor_remote *o, *p;
+
+	for (p = NULL, o = promisors; o; p = o, o = o->next)
+		if (o->remote_name && !strcmp(o->remote_name, remote_name)) {
+			if (previous)
+				*previous = p;
+			return o;
+		}
+
+	return NULL;
+}
+
+static void promisor_remote_move_to_tail(struct promisor_remote *o,
+					 struct promisor_remote *previous)
+{
+	if (previous)
+		previous->next = o->next;
+	else
+		promisors = o->next ? o->next : o;
+	o->next = NULL;
+	*promisors_tail = o;
+	promisors_tail = &o->next;
+}
+
+static int promisor_remote_config(const char *var, const char *value, void *data)
+{
+	struct promisor_remote *o;
+	const char *name;
+	int namelen;
+	const char *subkey;
+
+	if (parse_config_key(var, "remote", &name, &namelen, &subkey) < 0)
+		return 0;
+
+	if (!strcmp(subkey, "promisor")) {
+		char *remote_name;
+
+		if (!git_config_bool(var, value))
+			return 0;
+
+		remote_name = xmemdupz(name, namelen);
+
+		if (!promisor_remote_look_up(remote_name, NULL))
+			promisor_remote_new(remote_name);
+
+		free(remote_name);
+		return 0;
+	}
+
+	return 0;
+}
+
+static void promisor_remote_init(void)
+{
+	static int initialized;
+
+	if (initialized)
+		return;
+	initialized = 1;
+
+	git_config(promisor_remote_config, NULL);
+}
+
+struct promisor_remote *promisor_remote_find(const char *remote_name)
+{
+	promisor_remote_init();
+
+	if (!remote_name)
+		return promisors;
+
+	return promisor_remote_look_up(remote_name, NULL);
+}
+
+int has_promisor_remote(void)
+{
+	return !!promisor_remote_find(NULL);
+}
diff --git a/promisor-remote.h b/promisor-remote.h
new file mode 100644
index 0000000000..bfbf7c0f21
--- /dev/null
+++ b/promisor-remote.h
@@ -0,0 +1,17 @@
+#ifndef PROMISOR_REMOTE_H
+#define PROMISOR_REMOTE_H
+
+/*
+ * Promisor remote linked list
+ * Its information come from remote.XXX config entries.
+ */
+struct promisor_remote {
+	const char *remote_name;
+	struct promisor_remote *next;
+};
+
+extern struct promisor_remote *promisor_remote_new(const char *remote_name);
+extern struct promisor_remote *promisor_remote_find(const char *remote_name);
+extern int has_promisor_remote(void);
+
+#endif /* PROMISOR_REMOTE_H */
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 05/13] promisor-remote: implement promisors_get_direct()
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
                   ` (3 preceding siblings ...)
  2019-01-22 14:42 ` [PATCH v2 04/13] Add initial support for many promisor remotes Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 06/13] promisor-remote: add promisor_remote_reinit() Christian Couder
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli, Christian Couder

From: Christian Couder <christian.couder@gmail.com>

This is implemented for now by calling fetch_objects(). It fetches
from all the promisor remotes.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 promisor-remote.c | 17 +++++++++++++++++
 promisor-remote.h |  1 +
 2 files changed, 18 insertions(+)

diff --git a/promisor-remote.c b/promisor-remote.c
index d2f574651e..f86f9d0b84 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -1,6 +1,7 @@
 #include "cache.h"
 #include "promisor-remote.h"
 #include "config.h"
+#include "fetch-object.h"
 
 static struct promisor_remote *promisors;
 static struct promisor_remote **promisors_tail = &promisors;
@@ -98,3 +99,19 @@ int has_promisor_remote(void)
 {
 	return !!promisor_remote_find(NULL);
 }
+
+int promisor_remote_get_direct(const struct object_id *oids, int oid_nr)
+{
+	struct promisor_remote *o;
+
+	promisor_remote_init();
+
+	for (o = promisors; o; o = o->next) {
+		if (fetch_objects(o->remote_name, oids, oid_nr) < 0)
+			continue;
+		return 0;
+	}
+
+	return -1;
+}
+
diff --git a/promisor-remote.h b/promisor-remote.h
index bfbf7c0f21..f9f5825417 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -13,5 +13,6 @@ struct promisor_remote {
 extern struct promisor_remote *promisor_remote_new(const char *remote_name);
 extern struct promisor_remote *promisor_remote_find(const char *remote_name);
 extern int has_promisor_remote(void);
+extern int promisor_remote_get_direct(const struct object_id *oids, int oid_nr);
 
 #endif /* PROMISOR_REMOTE_H */
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 06/13] promisor-remote: add promisor_remote_reinit()
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
                   ` (4 preceding siblings ...)
  2019-01-22 14:42 ` [PATCH v2 05/13] promisor-remote: implement promisors_get_direct() Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 07/13] promisor-remote: use repository_format_partial_clone Christian Couder
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli, Christian Couder

From: Christian Couder <christian.couder@gmail.com>

We will need to reinitialize the promisor remote configuration
as we will make some changes to it in a later commit.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 promisor-remote.c | 14 ++++++++++++--
 promisor-remote.h |  1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/promisor-remote.c b/promisor-remote.c
index f86f9d0b84..ea74f6d8a8 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -74,17 +74,27 @@ static int promisor_remote_config(const char *var, const char *value, void *data
 	return 0;
 }
 
-static void promisor_remote_init(void)
+static void promisor_remote_do_init(int force)
 {
 	static int initialized;
 
-	if (initialized)
+	if (!force && initialized)
 		return;
 	initialized = 1;
 
 	git_config(promisor_remote_config, NULL);
 }
 
+static inline void promisor_remote_init(void)
+{
+	promisor_remote_do_init(0);
+}
+
+void promisor_remote_reinit(void)
+{
+	promisor_remote_do_init(1);
+}
+
 struct promisor_remote *promisor_remote_find(const char *remote_name)
 {
 	promisor_remote_init();
diff --git a/promisor-remote.h b/promisor-remote.h
index f9f5825417..f96722bc66 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -10,6 +10,7 @@ struct promisor_remote {
 	struct promisor_remote *next;
 };
 
+extern void promisor_remote_reinit(void);
 extern struct promisor_remote *promisor_remote_new(const char *remote_name);
 extern struct promisor_remote *promisor_remote_find(const char *remote_name);
 extern int has_promisor_remote(void);
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 07/13] promisor-remote: use repository_format_partial_clone
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
                   ` (5 preceding siblings ...)
  2019-01-22 14:42 ` [PATCH v2 06/13] promisor-remote: add promisor_remote_reinit() Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 08/13] Use promisor_remote_get_direct() and has_promisor_remote() Christian Couder
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli

A remote specified using the extensions.partialClone config
option should be considered a promisor remote too.

This remote should be at the end of the promisor remote list,
so that it is used only if objects have not been found in other
remotes.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 promisor-remote.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/promisor-remote.c b/promisor-remote.c
index ea74f6d8a8..dcf6ef6521 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -83,6 +83,17 @@ static void promisor_remote_do_init(int force)
 	initialized = 1;
 
 	git_config(promisor_remote_config, NULL);
+
+	if (repository_format_partial_clone) {
+		struct promisor_remote *o, *previous;
+
+		o = promisor_remote_look_up(repository_format_partial_clone,
+					    &previous);
+		if (o)
+			promisor_remote_move_to_tail(o, previous);
+		else
+			promisor_remote_new(repository_format_partial_clone);
+	}
 }
 
 static inline void promisor_remote_init(void)
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 08/13] Use promisor_remote_get_direct() and has_promisor_remote()
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
                   ` (6 preceding siblings ...)
  2019-01-22 14:42 ` [PATCH v2 07/13] promisor-remote: use repository_format_partial_clone Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 09/13] promisor-remote: parse remote.*.partialclonefilter Christian Couder
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli

Instead of using the repository_format_partial_clone global
and fetch_objects() directly, let's use has_promisor_remote()
and promisor_remote_get_direct().

This way all the configured promisor remotes will be taken
into account, not only the one specified by
extensions.partialClone.

Also when cloning or fetching using a partial clone filter,
remote.origin.promisor will be set to "true" instead of
setting extensions.partialClone to "origin". This makes it
possible to use many promisor remote just by fetching from
them.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/cat-file.c            |  5 +++--
 builtin/fetch.c               | 11 ++++++-----
 builtin/gc.c                  |  3 ++-
 builtin/repack.c              |  3 ++-
 cache-tree.c                  |  3 ++-
 connected.c                   |  3 ++-
 list-objects-filter-options.c | 28 +++++++++++++++-------------
 packfile.c                    |  3 ++-
 sha1-file.c                   | 14 ++++++++------
 t/t5601-clone.sh              |  2 +-
 t/t5616-partial-clone.sh      |  2 +-
 unpack-trees.c                |  6 +++---
 12 files changed, 47 insertions(+), 36 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 2ca56fd086..bdb0331ba4 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -14,6 +14,7 @@
 #include "sha1-array.h"
 #include "packfile.h"
 #include "object-store.h"
+#include "promisor-remote.h"
 
 struct batch_options {
 	int enabled;
@@ -517,8 +518,8 @@ static int batch_objects(struct batch_options *opt)
 	if (opt->all_objects) {
 		struct object_cb_data cb;
 
-		if (repository_format_partial_clone)
-			warning("This repository has extensions.partialClone set. Some objects may not be loaded.");
+		if (has_promisor_remote())
+			warning("This repository uses promisor remotes. Some objects may not be loaded.");
 
 		cb.opt = opt;
 		cb.expand = &data;
diff --git a/builtin/fetch.c b/builtin/fetch.c
index c0ade48f5d..d327cd9ef5 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -23,6 +23,7 @@
 #include "packfile.h"
 #include "list-objects-filter-options.h"
 #include "commit-reach.h"
+#include "promisor-remote.h"
 
 static const char * const builtin_fetch_usage[] = {
 	N_("git fetch [<options>] [<repository> [<refspec>...]]"),
@@ -1459,7 +1460,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 	 * If no prior partial clone/fetch and the current fetch DID NOT
 	 * request a partial-fetch, do a normal fetch.
 	 */
-	if (!repository_format_partial_clone && !filter_options.choice)
+	if (!has_promisor_remote() && !filter_options.choice)
 		return;
 
 	/*
@@ -1467,7 +1468,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 	 * on this repo and remember the given filter-spec as the default
 	 * for subsequent fetches to this remote.
 	 */
-	if (!repository_format_partial_clone && filter_options.choice) {
+	if (!has_promisor_remote() && filter_options.choice) {
 		partial_clone_register(remote->name, &filter_options);
 		return;
 	}
@@ -1476,7 +1477,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 	 * We are currently limited to only ONE promisor remote and only
 	 * allow partial-fetches from the promisor remote.
 	 */
-	if (strcmp(remote->name, repository_format_partial_clone)) {
+	if (!promisor_remote_find(remote->name)) {
 		if (filter_options.choice)
 			die(_("--filter can only be used with the remote "
 			      "configured in extensions.partialclone"));
@@ -1608,7 +1609,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	if (depth || deepen_since || deepen_not.nr)
 		deepen = 1;
 
-	if (filter_options.choice && !repository_format_partial_clone)
+	if (filter_options.choice && !has_promisor_remote())
 		die("--filter can only be used when extensions.partialClone is set");
 
 	if (all) {
@@ -1642,7 +1643,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	}
 
 	if (remote) {
-		if (filter_options.choice || repository_format_partial_clone)
+		if (filter_options.choice || has_promisor_remote())
 			fetch_one_setup_partial(remote);
 		result = fetch_one(remote, argc, argv, prune_tags_ok);
 	} else {
diff --git a/builtin/gc.c b/builtin/gc.c
index 020f725acc..0bec41b25f 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -27,6 +27,7 @@
 #include "pack-objects.h"
 #include "blob.h"
 #include "tree.h"
+#include "promisor-remote.h"
 
 #define FAILED_RUN "failed to run %s"
 
@@ -640,7 +641,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)
 			argv_array_push(&prune, prune_expire);
 			if (quiet)
 				argv_array_push(&prune, "--no-progress");
-			if (repository_format_partial_clone)
+			if (has_promisor_remote())
 				argv_array_push(&prune,
 						"--exclude-promisor-objects");
 			if (run_command_v_opt(prune.argv, RUN_GIT_CMD))
diff --git a/builtin/repack.c b/builtin/repack.c
index 67f8978043..3b935690c8 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -11,6 +11,7 @@
 #include "midx.h"
 #include "packfile.h"
 #include "object-store.h"
+#include "promisor-remote.h"
 
 static int delta_base_offset = 1;
 static int pack_kept_objects = -1;
@@ -366,7 +367,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 	argv_array_push(&cmd.args, "--all");
 	argv_array_push(&cmd.args, "--reflog");
 	argv_array_push(&cmd.args, "--indexed-objects");
-	if (repository_format_partial_clone)
+	if (has_promisor_remote())
 		argv_array_push(&cmd.args, "--exclude-promisor-objects");
 	if (write_bitmaps)
 		argv_array_push(&cmd.args, "--write-bitmap-index");
diff --git a/cache-tree.c b/cache-tree.c
index eabb8fb654..3e79e22b3d 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -5,6 +5,7 @@
 #include "cache-tree.h"
 #include "object-store.h"
 #include "replace-object.h"
+#include "promisor-remote.h"
 
 #ifndef DEBUG
 #define DEBUG 0
@@ -357,7 +358,7 @@ static int update_one(struct cache_tree *it,
 		}
 
 		ce_missing_ok = mode == S_IFGITLINK || missing_ok ||
-			(repository_format_partial_clone &&
+			(has_promisor_remote() &&
 			 ce_skip_worktree(ce));
 		if (is_null_oid(oid) ||
 		    (!ce_missing_ok && !has_object_file(oid))) {
diff --git a/connected.c b/connected.c
index 1bba888eff..0eaaedee6a 100644
--- a/connected.c
+++ b/connected.c
@@ -4,6 +4,7 @@
 #include "connected.h"
 #include "transport.h"
 #include "packfile.h"
+#include "promisor-remote.h"
 
 /*
  * If we feed all the commits we want to verify to this command
@@ -56,7 +57,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,
 	argv_array_push(&rev_list.args,"rev-list");
 	argv_array_push(&rev_list.args, "--objects");
 	argv_array_push(&rev_list.args, "--stdin");
-	if (repository_format_partial_clone)
+	if (has_promisor_remote())
 		argv_array_push(&rev_list.args, "--exclude-promisor-objects");
 	if (!opt->is_deepening_fetch) {
 		argv_array_push(&rev_list.args, "--not");
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index e8da2e8581..4e6b47b177 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -6,6 +6,7 @@
 #include "list-objects.h"
 #include "list-objects-filter.h"
 #include "list-objects-filter-options.h"
+#include "promisor-remote.h"
 
 /*
  * Parse value of the argument to the "filter" keyword.
@@ -125,30 +126,31 @@ void partial_clone_register(
 	const char *remote,
 	const struct list_objects_filter_options *filter_options)
 {
-	/*
-	 * Record the name of the partial clone remote in the
-	 * config and in the global variable -- the latter is
-	 * used throughout to indicate that partial clone is
-	 * enabled and to expect missing objects.
-	 */
-	if (repository_format_partial_clone &&
-	    *repository_format_partial_clone &&
-	    strcmp(remote, repository_format_partial_clone))
-		die(_("cannot change partial clone promisor remote"));
+	char *cfg_name;
 
-	git_config_set("core.repositoryformatversion", "1");
-	git_config_set("extensions.partialclone", remote);
+	/* Check if it is already registered */
+	if (!promisor_remote_find(remote)) {
+		git_config_set("core.repositoryformatversion", "1");
 
-	repository_format_partial_clone = xstrdup(remote);
+		/* Add promisor config for the remote */
+		cfg_name = xstrfmt("remote.%s.promisor", remote);
+		git_config_set(cfg_name, "true");
+		free(cfg_name);
+	}
 
 	/*
 	 * Record the initial filter-spec in the config as
 	 * the default for subsequent fetches from this remote.
+	 *
+	 * TODO: record it into remote.<name>.partialclonefilter
 	 */
 	core_partial_clone_filter_default =
 		xstrdup(filter_options->filter_spec);
 	git_config_set("core.partialclonefilter",
 		       core_partial_clone_filter_default);
+
+	/* Make sure the config info are reset */
+	promisor_remote_reinit();
 }
 
 void partial_clone_get_default_filter_spec(
diff --git a/packfile.c b/packfile.c
index 0fe9c21bf1..128d31d94b 100644
--- a/packfile.c
+++ b/packfile.c
@@ -16,6 +16,7 @@
 #include "tree.h"
 #include "object-store.h"
 #include "midx.h"
+#include "promisor-remote.h"
 
 char *odb_pack_name(struct strbuf *buf,
 		    const unsigned char *sha1,
@@ -2118,7 +2119,7 @@ int is_promisor_object(const struct object_id *oid)
 	static int promisor_objects_prepared;
 
 	if (!promisor_objects_prepared) {
-		if (repository_format_partial_clone) {
+		if (has_promisor_remote()) {
 			for_each_packed_object(add_promisor_object,
 					       &promisor_objects,
 					       FOR_EACH_OBJECT_PROMISOR_ONLY);
diff --git a/sha1-file.c b/sha1-file.c
index 972f26e931..2a786f1067 100644
--- a/sha1-file.c
+++ b/sha1-file.c
@@ -32,6 +32,7 @@
 #include "packfile.h"
 #include "fetch-object.h"
 #include "object-store.h"
+#include "promisor-remote.h"
 
 /* The maximum size for an object header. */
 #define MAX_HEADER_LEN 32
@@ -1301,15 +1302,16 @@ int oid_object_info_extended(struct repository *r, const struct object_id *oid,
 		}
 
 		/* Check if it is a missing object */
-		if (fetch_if_missing && repository_format_partial_clone &&
+		if (fetch_if_missing && has_promisor_remote() &&
 		    !already_retried && r == the_repository) {
 			/*
-			 * TODO Investigate checking fetch_object() return
-			 * TODO value and stopping on error here.
-			 * TODO Pass a repository struct through fetch_object,
-			 * such that arbitrary repositories work.
+			 * TODO Investigate checking promisor_remote_get_direct()
+			 * TODO return value and stopping on error here.
+			 * TODO Pass a repository struct through
+			 * promisor_remote_get_direct(), such that arbitrary
+			 * repositories work.
 			 */
-			fetch_objects(repository_format_partial_clone, real, 1);
+			promisor_remote_get_direct(real, 1);
 			already_retried = 1;
 			continue;
 		}
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index d6948cbdab..dd658f8b32 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -653,7 +653,7 @@ partial_clone () {
 	git -C client fsck &&
 
 	# Ensure that unneeded blobs are not inadvertently fetched.
-	test_config -C client extensions.partialclone "not a remote" &&
+	test_config -C client remote.origin.promisor "false" &&
 	test_must_fail git -C client cat-file -e "$HASH1" &&
 
 	# But this blob was fetched, because clone performs an initial checkout
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index 9643acb161..e8ca825ab7 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -42,7 +42,7 @@ test_expect_success 'do partial clone 1' '
 
 	test_cmp expect_1.oids observed.oids &&
 	test "$(git -C pc1 config --local core.repositoryformatversion)" = "1" &&
-	test "$(git -C pc1 config --local extensions.partialclone)" = "origin" &&
+	test "$(git -C pc1 config --local remote.origin.promisor)" = "true" &&
 	test "$(git -C pc1 config --local core.partialclonefilter)" = "blob:none"
 '
 
diff --git a/unpack-trees.c b/unpack-trees.c
index 94265a7df0..7e89cc28f9 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -18,6 +18,7 @@
 #include "fsmonitor.h"
 #include "object-store.h"
 #include "fetch-object.h"
+#include "promisor-remote.h"
 
 /*
  * Error messages expected by scripts out of plumbing commands such as
@@ -418,7 +419,7 @@ static int check_updates(struct unpack_trees_options *o)
 		load_gitmodules_file(index, &state);
 
 	enable_delayed_checkout(&state);
-	if (repository_format_partial_clone && o->update && !o->dry_run) {
+	if (has_promisor_remote() && o->update && !o->dry_run) {
 		/*
 		 * Prefetch the objects that are to be checked out in the loop
 		 * below.
@@ -435,8 +436,7 @@ static int check_updates(struct unpack_trees_options *o)
 			}
 		}
 		if (to_fetch.nr)
-			fetch_objects(repository_format_partial_clone,
-				      to_fetch.oid, to_fetch.nr);
+			promisor_remote_get_direct(to_fetch.oid, to_fetch.nr);
 		fetch_if_missing = fetch_if_missing_store;
 		oid_array_clear(&to_fetch);
 	}
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 09/13] promisor-remote: parse remote.*.partialclonefilter
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
                   ` (7 preceding siblings ...)
  2019-01-22 14:42 ` [PATCH v2 08/13] Use promisor_remote_get_direct() and has_promisor_remote() Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 10/13] builtin/fetch: remove unique promisor remote limitation Christian Couder
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli

This makes it possible to specify a different partial clone
filter for each promisor remote.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/fetch.c               |  2 +-
 list-objects-filter-options.c | 27 +++++++++++++++------------
 list-objects-filter-options.h |  3 ++-
 promisor-remote.c             | 10 ++++++++++
 promisor-remote.h             |  5 ++++-
 t/t0410-partial-clone.sh      |  2 +-
 t/t5601-clone.sh              |  1 +
 t/t5616-partial-clone.sh      |  2 +-
 8 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index d327cd9ef5..75746c50e0 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1490,7 +1490,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 	 * the config.
 	 */
 	if (!filter_options.choice)
-		partial_clone_get_default_filter_spec(&filter_options);
+		partial_clone_get_default_filter_spec(&filter_options, remote->name);
 	return;
 }
 
diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
index 4e6b47b177..a70a16bf5c 100644
--- a/list-objects-filter-options.c
+++ b/list-objects-filter-options.c
@@ -29,6 +29,9 @@ static int gently_parse_list_objects_filter(
 {
 	const char *v0;
 
+	if (!arg)
+		return 0;
+
 	if (filter_options->choice) {
 		if (errbuf) {
 			strbuf_addstr(
@@ -127,6 +130,7 @@ void partial_clone_register(
 	const struct list_objects_filter_options *filter_options)
 {
 	char *cfg_name;
+	char *filter_name;
 
 	/* Check if it is already registered */
 	if (!promisor_remote_find(remote)) {
@@ -141,27 +145,26 @@ void partial_clone_register(
 	/*
 	 * Record the initial filter-spec in the config as
 	 * the default for subsequent fetches from this remote.
-	 *
-	 * TODO: record it into remote.<name>.partialclonefilter
 	 */
-	core_partial_clone_filter_default =
-		xstrdup(filter_options->filter_spec);
-	git_config_set("core.partialclonefilter",
-		       core_partial_clone_filter_default);
+	filter_name = xstrfmt("remote.%s.partialclonefilter", remote);
+	git_config_set(filter_name, filter_options->filter_spec);
+	free(filter_name);
 
 	/* Make sure the config info are reset */
 	promisor_remote_reinit();
 }
 
 void partial_clone_get_default_filter_spec(
-	struct list_objects_filter_options *filter_options)
+	struct list_objects_filter_options *filter_options,
+	const char *remote)
 {
+	struct promisor_remote *promisor = promisor_remote_find(remote);
+
 	/*
 	 * Parse default value, but silently ignore it if it is invalid.
 	 */
-	if (!core_partial_clone_filter_default)
-		return;
-	gently_parse_list_objects_filter(filter_options,
-					 core_partial_clone_filter_default,
-					 NULL);
+	if (promisor)
+		gently_parse_list_objects_filter(filter_options,
+						 promisor->partial_clone_filter,
+						 NULL);
 }
diff --git a/list-objects-filter-options.h b/list-objects-filter-options.h
index af64e5c66f..8c10ec2d3a 100644
--- a/list-objects-filter-options.h
+++ b/list-objects-filter-options.h
@@ -75,6 +75,7 @@ void partial_clone_register(
 	const char *remote,
 	const struct list_objects_filter_options *filter_options);
 void partial_clone_get_default_filter_spec(
-	struct list_objects_filter_options *filter_options);
+	struct list_objects_filter_options *filter_options,
+	const char *remote);
 
 #endif /* LIST_OBJECTS_FILTER_OPTIONS_H */
diff --git a/promisor-remote.c b/promisor-remote.c
index dcf6ef6521..ed33dfa86c 100644
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -70,6 +70,16 @@ static int promisor_remote_config(const char *var, const char *value, void *data
 		free(remote_name);
 		return 0;
 	}
+	if (!strcmp(subkey, "partialclonefilter")) {
+		char *remote_name = xmemdupz(name, namelen);
+
+		o = promisor_remote_look_up(remote_name, NULL);
+		if (!o)
+			o = promisor_remote_new(remote_name);
+
+		free(remote_name);
+		return git_config_string(&o->partial_clone_filter, var, value);
+	}
 
 	return 0;
 }
diff --git a/promisor-remote.h b/promisor-remote.h
index f96722bc66..f0d609a3f5 100644
--- a/promisor-remote.h
+++ b/promisor-remote.h
@@ -3,10 +3,13 @@
 
 /*
  * Promisor remote linked list
- * Its information come from remote.XXX config entries.
+ *
+ * Information in its fields come from remote.XXX config entries or
+ * from extensions.partialclone or core.partialclonefilter.
  */
 struct promisor_remote {
 	const char *remote_name;
+	const char *partial_clone_filter;
 	struct promisor_remote *next;
 };
 
diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index bce02788e6..9266037714 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -26,7 +26,7 @@ promise_and_delete () {
 test_expect_success 'extensions.partialclone without filter' '
 	test_create_repo server &&
 	git clone --filter="blob:none" "file://$(pwd)/server" client &&
-	git -C client config --unset core.partialclonefilter &&
+	git -C client config --unset remote.origin.partialclonefilter &&
 	git -C client fetch origin
 '
 
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index dd658f8b32..2e82c70f33 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -654,6 +654,7 @@ partial_clone () {
 
 	# Ensure that unneeded blobs are not inadvertently fetched.
 	test_config -C client remote.origin.promisor "false" &&
+	git -C client config --unset remote.origin.partialclonefilter &&
 	test_must_fail git -C client cat-file -e "$HASH1" &&
 
 	# But this blob was fetched, because clone performs an initial checkout
diff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh
index e8ca825ab7..ba0489b41a 100755
--- a/t/t5616-partial-clone.sh
+++ b/t/t5616-partial-clone.sh
@@ -43,7 +43,7 @@ test_expect_success 'do partial clone 1' '
 	test_cmp expect_1.oids observed.oids &&
 	test "$(git -C pc1 config --local core.repositoryformatversion)" = "1" &&
 	test "$(git -C pc1 config --local remote.origin.promisor)" = "true" &&
-	test "$(git -C pc1 config --local core.partialclonefilter)" = "blob:none"
+	test "$(git -C pc1 config --local remote.origin.partialclonefilter)" = "blob:none"
 '
 
 # checkout master to force dynamic object fetch of blobs at HEAD.
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 10/13] builtin/fetch: remove unique promisor remote limitation
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
                   ` (8 preceding siblings ...)
  2019-01-22 14:42 ` [PATCH v2 09/13] promisor-remote: parse remote.*.partialclonefilter Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 11/13] t0410: test fetching from many promisor remotes Christian Couder
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli

---
 builtin/fetch.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 75746c50e0..ed883399c7 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1464,26 +1464,16 @@ static inline void fetch_one_setup_partial(struct remote *remote)
 		return;
 
 	/*
-	 * If this is the FIRST partial-fetch request, we enable partial
-	 * on this repo and remember the given filter-spec as the default
-	 * for subsequent fetches to this remote.
+	 * If this is a partial-fetch request, we enable partial on
+	 * this repo if not already enabled and remember the given
+	 * filter-spec as the default for subsequent fetches to this
+	 * remote.
 	 */
-	if (!has_promisor_remote() && filter_options.choice) {
+	if (filter_options.choice) {
 		partial_clone_register(remote->name, &filter_options);
 		return;
 	}
 
-	/*
-	 * We are currently limited to only ONE promisor remote and only
-	 * allow partial-fetches from the promisor remote.
-	 */
-	if (!promisor_remote_find(remote->name)) {
-		if (filter_options.choice)
-			die(_("--filter can only be used with the remote "
-			      "configured in extensions.partialclone"));
-		return;
-	}
-
 	/*
 	 * Do a partial-fetch from the promisor remote using either the
 	 * explicitly given filter-spec or inherit the filter-spec from
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 11/13] t0410: test fetching from many promisor remotes
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
                   ` (9 preceding siblings ...)
  2019-01-22 14:42 ` [PATCH v2 10/13] builtin/fetch: remove unique promisor remote limitation Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 12/13] partial-clone: add multiple remotes in the doc Christian Couder
  2019-01-22 14:42 ` [PATCH v2 13/13] remote: add promisor and partial clone config to " Christian Couder
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli, Christian Couder

From: Christian Couder <christian.couder@gmail.com>

This shows that it is now possible to fetch objects from more
than one promisor remote, and that fetching from a new
promisor remote can configure it as one.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/t0410-partial-clone.sh | 47 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index 9266037714..146b0a1e03 100755
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -182,8 +182,53 @@ test_expect_success 'fetching of missing objects works with ref-in-want enabled'
 	grep "git< fetch=.*ref-in-want" trace
 '
 
+test_expect_success 'fetching of missing objects from another promisor remote' '
+	git clone "file://$(pwd)/server" server2 &&
+	test_commit -C server2 bar &&
+	git -C server2 repack -a -d --write-bitmap-index &&
+	HASH2=$(git -C server2 rev-parse bar) &&
+
+	git -C repo remote add server2 "file://$(pwd)/server2" &&
+	git -C repo config remote.server2.promisor true &&
+	git -C repo cat-file -p "$HASH2" &&
+
+	git -C repo fetch server2 &&
+	rm -rf repo/.git/objects/* &&
+	git -C repo cat-file -p "$HASH2" &&
+
+	# Ensure that the .promisor file is written, and check that its
+	# associated packfile contains the object
+	ls repo/.git/objects/pack/pack-*.promisor >promisorlist &&
+	test_line_count = 1 promisorlist &&
+	IDX=$(cat promisorlist | sed "s/promisor$/idx/") &&
+	git verify-pack --verbose "$IDX" | grep "$HASH2"
+'
+
+test_expect_success 'fetching of missing objects configures a promisor remote' '
+	git clone "file://$(pwd)/server" server3 &&
+	test_commit -C server3 baz &&
+	git -C server3 repack -a -d --write-bitmap-index &&
+	HASH3=$(git -C server3 rev-parse baz) &&
+	git -C server3 config uploadpack.allowfilter 1 &&
+
+	rm repo/.git/objects/pack/pack-*.promisor &&
+
+	git -C repo remote add server3 "file://$(pwd)/server3" &&
+	git -C repo fetch --filter="blob:none" server3 $HASH3 &&
+
+	test "$(git -C repo config remote.server3.promisor)" = "true" &&
+
+	# Ensure that the .promisor file is written, and check that its
+	# associated packfile contains the object
+	ls repo/.git/objects/pack/pack-*.promisor >promisorlist &&
+	test_line_count = 1 promisorlist &&
+	IDX=$(cat promisorlist | sed "s/promisor$/idx/") &&
+	git verify-pack --verbose "$IDX" | grep "$HASH3"
+'
+
 test_expect_success 'fetching of missing blobs works' '
-	rm -rf server repo &&
+	rm -rf server server2 repo &&
+	rm -rf server server3 repo &&
 	test_create_repo server &&
 	test_commit -C server foo &&
 	git -C server repack -a -d --write-bitmap-index &&
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 12/13] partial-clone: add multiple remotes in the doc
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
                   ` (10 preceding siblings ...)
  2019-01-22 14:42 ` [PATCH v2 11/13] t0410: test fetching from many promisor remotes Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  2019-01-22 14:42 ` [PATCH v2 13/13] remote: add promisor and partial clone config to " Christian Couder
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli

While at it, let's remove a reference to ODB effort as the ODB
effort has been replaced by directly enhancing partial clone
and promisor remote features.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/technical/partial-clone.txt | 83 ++++++++++++++++-------
 1 file changed, 58 insertions(+), 25 deletions(-)

diff --git a/Documentation/technical/partial-clone.txt b/Documentation/technical/partial-clone.txt
index 896c7b3878..58adcc5ce1 100644
--- a/Documentation/technical/partial-clone.txt
+++ b/Documentation/technical/partial-clone.txt
@@ -100,18 +100,18 @@ or commits that reference missing trees.
 Handling Missing Objects
 ------------------------
 
-- An object may be missing due to a partial clone or fetch, or missing due
-  to repository corruption.  To differentiate these cases, the local
-  repository specially indicates such filtered packfiles obtained from the
-  promisor remote as "promisor packfiles".
+- An object may be missing due to a partial clone or fetch, or missing
+  due to repository corruption.  To differentiate these cases, the
+  local repository specially indicates such filtered packfiles
+  obtained from promisor remotes as "promisor packfiles".
 +
 These promisor packfiles consist of a "<name>.promisor" file with
 arbitrary contents (like the "<name>.keep" files), in addition to
 their "<name>.pack" and "<name>.idx" files.
 
 - The local repository considers a "promisor object" to be an object that
-  it knows (to the best of its ability) that the promisor remote has promised
-  that it has, either because the local repository has that object in one of
+  it knows (to the best of its ability) that promisor remotes have promised
+  that they have, either because the local repository has that object in one of
   its promisor packfiles, or because another promisor object refers to it.
 +
 When Git encounters a missing object, Git can see if it is a promisor object
@@ -123,7 +123,7 @@ expensive-to-modify list of missing objects.[a]
 - Since almost all Git code currently expects any referenced object to be
   present locally and because we do not want to force every command to do
   a dry-run first, a fallback mechanism is added to allow Git to attempt
-  to dynamically fetch missing objects from the promisor remote.
+  to dynamically fetch missing objects from promisor remotes.
 +
 When the normal object lookup fails to find an object, Git invokes
 fetch-object to try to get the object from the server and then retry
@@ -157,8 +157,7 @@ and prefetch those objects in bulk.
 +
 We are not happy with this global variable and would like to remove it,
 but that requires significant refactoring of the object code to pass an
-additional flag.  We hope that concurrent efforts to add an ODB API can
-encompass this.
+additional flag.
 
 
 Fetching Missing Objects
@@ -182,21 +181,53 @@ has been updated to not use any object flags when the corresponding argument
   though they are not necessary.
 
 
+Using many promisor remotes
+---------------------------
+
+Many promisor remotes can be configured and used.
+
+This allows for example a user to have multiple geographically-close
+cache servers for fetching missing blobs while continuing to do
+filtered `git-fetch` commands from the central server.
+
+When fetching objects, promisor remotes are tried one after the other
+until all the objects have been fetched.
+
+Remotes that are considered "promisor" remotes are those specified by
+the following configuration variables:
+
+- `extensions.partialClone = <name>`
+
+- `remote.<name>.promisor = true`
+
+- `remote.<name>.partialCloneFilter = ...`
+
+Only one promisor remote can be configured using the
+`extensions.partialClone` config variable. This promisor remote will
+be the last one tried when fetching objects.
+
+The other promisor remotes will be tried in the order they appear in
+the config file.
+
 Current Limitations
 -------------------
 
-- The remote used for a partial clone (or the first partial fetch
-  following a regular clone) is marked as the "promisor remote".
+- It is not possible to specify the order in which the promisor
+  remotes are tried in other ways than the order in which they appear
+  in the config file.
 +
-We are currently limited to a single promisor remote and only that
-remote may be used for subsequent partial fetches.
+It is also not possible to specify an order to be used when fetching
+from one remote and a different order when fetching from another
+remote.
+
+- It is not possible to push only specific objects to a promisor
+  remote.
 +
-We accept this limitation because we believe initial users of this
-feature will be using it on repositories with a strong single central
-server.
+It is not possible to push at the same time to multiple promisor
+remote in a specific order.
 
-- Dynamic object fetching will only ask the promisor remote for missing
-  objects.  We assume that the promisor remote has a complete view of the
+- Dynamic object fetching will only ask promisor remotes for missing
+  objects.  We assume that promisor remotes have a complete view of the
   repository and can satisfy all such requests.
 
 - Repack essentially treats promisor and non-promisor packfiles as 2
@@ -218,15 +249,17 @@ server.
 Future Work
 -----------
 
-- Allow more than one promisor remote and define a strategy for fetching
-  missing objects from specific promisor remotes or of iterating over the
-  set of promisor remotes until a missing object is found.
+- Improve the way to specify the order in which promisor remotes are
+  tried.
 +
-A user might want to have multiple geographically-close cache servers
-for fetching missing blobs while continuing to do filtered `git-fetch`
-commands from the central server, for example.
+For example this could allow to specify explicitly something like:
+"When fetching from this remote, I want to use these promisor remotes
+in this order, though, when pushing or fetching to that remote, I want
+to use those promisor remotes in that order."
+
+- Allow pushing to promisor remotes.
 +
-Or the user might want to work in a triangular work flow with multiple
+The user might want to work in a triangular work flow with multiple
 promisor remotes that each have an incomplete view of the repository.
 
 - Allow repack to work on promisor packfiles (while keeping them distinct
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 13/13] remote: add promisor and partial clone config to the doc
  2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
                   ` (11 preceding siblings ...)
  2019-01-22 14:42 ` [PATCH v2 12/13] partial-clone: add multiple remotes in the doc Christian Couder
@ 2019-01-22 14:42 ` Christian Couder
  12 siblings, 0 replies; 14+ messages in thread
From: Christian Couder @ 2019-01-22 14:42 UTC (permalink / raw)
  To: git
  Cc: Junio C Hamano, Jeff King, Ben Peart, Jonathan Tan,
	Jonathan Nieder, Stefan Beller, Nguyen Thai Ngoc Duy, Mike Hommey,
	Lars Schneider, Eric Wong, Christian Couder, Jeff Hostetler,
	Eric Sunshine, Beat Bolli

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/config/remote.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/config/remote.txt b/Documentation/config/remote.txt
index 6c4cad83a2..a8e6437a90 100644
--- a/Documentation/config/remote.txt
+++ b/Documentation/config/remote.txt
@@ -76,3 +76,11 @@ remote.<name>.pruneTags::
 +
 See also `remote.<name>.prune` and the PRUNING section of
 linkgit:git-fetch[1].
+
+remote.<name>.promisor::
+	When set to true, this remote will be used to fetch promisor
+	objects.
+
+remote.<name>.partialclonefilter::
+	The filter that will be applied when fetching from this
+	promisor remote.
-- 
2.20.1.322.gd6b9ae60d4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2019-01-22 14:42 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-22 14:41 [PATCH v2 00/13] Many promisor remotes Christian Couder
2019-01-22 14:42 ` [PATCH v2 01/13] fetch: fix extensions.partialclone name in error message Christian Couder
2019-01-22 14:42 ` [PATCH v2 02/13] partial-clone: add missing 'is' in doc Christian Couder
2019-01-22 14:42 ` [PATCH v2 03/13] fetch-object: make functions return an error code Christian Couder
2019-01-22 14:42 ` [PATCH v2 04/13] Add initial support for many promisor remotes Christian Couder
2019-01-22 14:42 ` [PATCH v2 05/13] promisor-remote: implement promisors_get_direct() Christian Couder
2019-01-22 14:42 ` [PATCH v2 06/13] promisor-remote: add promisor_remote_reinit() Christian Couder
2019-01-22 14:42 ` [PATCH v2 07/13] promisor-remote: use repository_format_partial_clone Christian Couder
2019-01-22 14:42 ` [PATCH v2 08/13] Use promisor_remote_get_direct() and has_promisor_remote() Christian Couder
2019-01-22 14:42 ` [PATCH v2 09/13] promisor-remote: parse remote.*.partialclonefilter Christian Couder
2019-01-22 14:42 ` [PATCH v2 10/13] builtin/fetch: remove unique promisor remote limitation Christian Couder
2019-01-22 14:42 ` [PATCH v2 11/13] t0410: test fetching from many promisor remotes Christian Couder
2019-01-22 14:42 ` [PATCH v2 12/13] partial-clone: add multiple remotes in the doc Christian Couder
2019-01-22 14:42 ` [PATCH v2 13/13] remote: add promisor and partial clone config to " Christian Couder

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).