{"thread":{"id":"49003","subject":"[PATCH v4 0/9] Introducing remote ODBs","startedAt":"2018-08-02T06:15:30Z","lastAt":"2018-11-01T21:16:19Z","messageCount":23,"participants":["Christian Couder","Stefan Beller","Junio C Hamano","Jeff King","Taylor Blau","Jonathan Nieder","Jonathan Tan"],"isPatch":true,"patchVersion":4,"patchTotal":9},"messages":[{"id":"354213","messageId":"20180802061505.2983-1-chriscool@tuxfamily.org","threadId":"49003","inReplyTo":null,"subject":"[PATCH v4 0/9] Introducing remote ODBs","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-08-02T06:14:56Z","receivedAt":"2018-08-02T06:15:30Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"This path series is a follow up from the patch series called \"odb\nremote\" that I sent earlier this year, which was itself a follow up\nfrom previous series. See the links section for more information.\n\nAs with the previous \"odb remote\" series, this series is only about\nintegrating with the promisor/narrow clone work and showing that it\nmakes it possible to use more than one promisor remote. Everything\nthat is not necessary for that integration has been removed for now\n(though you can still find it in one of my branches on GitHub if you\nwant).\n\nThere is one test in patch 8/9 that shows that more than one promisor\nremote can now be used, but I still feel that it could be interesting\nto add other such tests, so I am open to ideas in this area.\n\nChanges compared to V3 of this patch series\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\n - the remote_odb_reinit() function is not \"inline\" anymore in patch\n 5/9 as suggested by Beat Bolli in:\n\n https://public-inbox.org/git/20180724215223.27516-3-dev+git@drbeat.li/\n\nHigh level overview of this patch series\n~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n\nAll the patches except 5/9 are unchanged compared to V3.\n\n - Patch 1/9:\n\nThis makes functions in fetch-object.c return an error code, which is\nnecessary to later tell that they failed and try another remote odb\nwhen there is more than one. This could also just be seen as a fix to\nthese functions.\n\n - Patch 2/9:\n\nThis introduces the minimum infrastructure for remote odbs.\n\n - Patches 3/9 and 4/9:\n\nThese patches implement remote_odb_get_direct() and\nremote_odb_get_many_direct() using the functions from fetch-object.c.\nThese new functions will be used in following patches to replace the\nfunctions from fetch-object.c.\n\n - Patch 5/9:\n\nThis implement remote_odb_reinit() which will be needed to reparse the\nremote odb configuration.\n\n - Patches 6/9 and 7/9:\n\nThese patches integrate the remote odb mechanism into the\npromisor/narrow clone code. The \"extensions.partialClone\" config\noption is replaced by \"odb..promisorRemote\" and\n\"core.partialCloneFilter\" is replaced by\n\"odb..partialCloneFilter\".\n\n - Patch 8/9:\n\nThis adds a test case that shows that now more than one promisor\nremote can be used.\n\n - Patch 9/9:\n\nThis starts documenting the remote odb mechanism.\n\nDiscussion\n~~~~~~~~~~\n\nI am not sure that it is ok to completely replace the\n\"extensions.partialclone\" config option. Even if it is fully replaced,\nno \"extensions.remoteodb\" is implemented in these patches, as maybe\nthe \"extensions.partialclone\" name could be kept even if the\nunderlying mechanism is the remote odb mechanism.\n\nAnyway I think that the remote odb mechanism is much more extensible,\nso I think using \"extensions.partialclone\" to specify a promisor\nremote should be at least deprecated.\n\nLinks\n~~~~~\n\nThis patch series on GitHub:\n\nV4: https://github.com/chriscool/git/commits/remote-odb\nV3: https://github.com/chriscool/git/commits/remote-odb3\nV2: https://github.com/chriscool/git/commits/remote-odb2\nV1: https://github.com/chriscool/git/commits/remote-odb1\n\nDiscussions related to previous versions:\n\nV3: https://public-inbox.org/git/20180713174959.16748-1-chriscool@tuxfamily.org/\nV2: https://public-inbox.org/git/20180630083542.20347-1-chriscool@tuxfamily.org/\nV1: https://public-inbox.org/git/20180623121846.19750-1-chriscool@tuxfamily.org/\n\nPrevious \"odb remote\" series:\n\nhttps://public-inbox.org/git/20180513103232.17514-1-chriscool@tuxfamily.org/\nhttps://github.com/chriscool/git/commits/odb-remote\n\nVersion 1 and 2 of the \"Promisor remotes and external ODB support\" series:\n\nhttps://public-inbox.org/git/20180103163403.11303-1-chriscool@tuxfamily.org/\nhttps://public-inbox.org/git/20180319133147.15413-1-chriscool@tuxfamily.org/\n\nVersion 1 and 2 of the \"Promisor remotes and external ODB support\" series on GitHub:\n\nhttps://github.com/chriscool/git/commits/gl-small-promisor-external-odb12\nhttps://github.com/chriscool/git/commits/gl-small-promisor-external-odb71\n\nChristian Couder (9):\n fetch-object: make functions return an error code\n Add initial remote odb support\n remote-odb: implement remote_odb_get_direct()\n remote-odb: implement remote_odb_get_many_direct()\n remote-odb: add remote_odb_reinit()\n Use remote_odb_get_direct() and has_remote_odb()\n Use odb.origin.partialclonefilter instead of core.partialclonefilter\n t0410: test fetching from many promisor remotes\n Documentation/config: add odb..promisorRemote\n\n Documentation/config.txt | 5 ++\n Makefile | 2 +\n builtin/cat-file.c | 5 +-\n builtin/fetch.c | 13 ++--\n builtin/gc.c | 3 +-\n builtin/repack.c | 3 +-\n cache.h | 2 -\n connected.c | 3 +-\n environment.c | 1 -\n fetch-object.c | 15 ++--\n fetch-object.h | 6 +-\n list-objects-filter-options.c | 51 +++++++------\n list-objects-filter-options.h | 3 +-\n odb-helper.c | 45 +++++++++++\n odb-helper.h | 24 ++++++\n packfile.c | 3 +-\n remote-odb.c | 137 ++++++++++++++++++++++++++++++++++\n remote-odb.h | 10 +++\n setup.c | 7 +-\n sha1-file.c | 14 ++--\n t/t0410-partial-clone.sh | 58 +++++++++-----\n t/t5500-fetch-pack.sh | 4 +-\n t/t5601-clone.sh | 2 +-\n t/t5616-partial-clone.sh | 4 +-\n t/t5702-protocol-v2.sh | 2 +-\n unpack-trees.c | 6 +-\n 26 files changed, 342 insertions(+), 86 deletions(-)\n create mode 100644 odb-helper.c\n create mode 100644 odb-helper.h\n create mode 100644 remote-odb.c\n create mode 100644 remote-odb.h\n\n-- \n2.18.0.330.g17eb9fed90\n\n"},{"id":"354214","messageId":"20180802061505.2983-2-chriscool@tuxfamily.org","threadId":"49003","inReplyTo":"20180802061505.2983-1-chriscool@tuxfamily.org","subject":"[PATCH v4 1/9] fetch-object: make functions return an error code","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-08-02T06:14:57Z","receivedAt":"2018-08-02T06:15:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder \n\nThe callers of the fetch_object() and fetch_objects() might\nbe interested in knowing if these functions succeeded or not.\n\nSigned-off-by: Christian Couder \nSigned-off-by: Junio C Hamano \n---\n fetch-object.c | 15 +++++++++------\n fetch-object.h | 6 +++---\n sha1-file.c | 4 ++--\n 3 files changed, 14 insertions(+), 11 deletions(-)\n\ndiff --git a/fetch-object.c b/fetch-object.c\nindex 48fe63dd6c..3c52009266 100644\n--- a/fetch-object.c\n+++ b/fetch-object.c\n@@ -5,11 +5,12 @@\n #include \"transport.h\"\n #include \"fetch-object.h\"\n \n-static void fetch_refs(const char *remote_name, struct ref *ref)\n+static int fetch_refs(const char *remote_name, struct ref *ref)\n {\n \tstruct remote *remote;\n \tstruct transport *transport;\n \tint original_fetch_if_missing = fetch_if_missing;\n+\tint res;\n \n \tfetch_if_missing = 0;\n \tremote = remote_get(remote_name);\n@@ -19,18 +20,20 @@ static void fetch_refs(const char *remote_name, struct ref *ref)\n \n \ttransport_set_option(transport, TRANS_OPT_FROM_PROMISOR, \"1\");\n \ttransport_set_option(transport, TRANS_OPT_NO_DEPENDENTS, \"1\");\n-\ttransport_fetch_refs(transport, ref, NULL);\n+\tres = transport_fetch_refs(transport, ref, NULL);\n \tfetch_if_missing = original_fetch_if_missing;\n+\n+\treturn res;\n }\n \n-void fetch_object(const char *remote_name, const unsigned char *sha1)\n+int fetch_object(const char *remote_name, const unsigned char *sha1)\n {\n \tstruct ref *ref = alloc_ref(sha1_to_hex(sha1));\n \thashcpy(ref->old_oid.hash, sha1);\n-\tfetch_refs(remote_name, ref);\n+\treturn fetch_refs(remote_name, ref);\n }\n \n-void fetch_objects(const char *remote_name, const struct oid_array *to_fetch)\n+int fetch_objects(const char *remote_name, const struct oid_array *to_fetch)\n {\n \tstruct ref *ref = NULL;\n \tint i;\n@@ -41,5 +44,5 @@ void fetch_objects(const char *remote_name, const struct oid_array *to_fetch)\n \t\tnew_ref->next = ref;\n \t\tref = new_ref;\n \t}\n-\tfetch_refs(remote_name, ref);\n+\treturn fetch_refs(remote_name, ref);\n }\ndiff --git a/fetch-object.h b/fetch-object.h\nindex 4b269d07ed..12e1f9ee70 100644\n--- a/fetch-object.h\n+++ b/fetch-object.h\n@@ -3,9 +3,9 @@\n \n #include \"sha1-array.h\"\n \n-extern void fetch_object(const char *remote_name, const unsigned char *sha1);\n+extern int fetch_object(const char *remote_name, const unsigned char *sha1);\n \n-extern void fetch_objects(const char *remote_name,\n-\t\t\t const struct oid_array *to_fetch);\n+extern int fetch_objects(const char *remote_name,\n+\t\t\t const struct oid_array *to_fetch);\n \n #endif\ndiff --git a/sha1-file.c b/sha1-file.c\nindex de4839e634..c099f5584d 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -1312,8 +1312,8 @@ int oid_object_info_extended(struct repository *r, const struct object_id *oid,\n \t\tif (fetch_if_missing && repository_format_partial_clone &&\n \t\t !already_retried && r == the_repository) {\n \t\t\t/*\n-\t\t\t * TODO Investigate having fetch_object() return\n-\t\t\t * TODO error/success and stopping the music here.\n+\t\t\t * TODO Investigate checking fetch_object() return\n+\t\t\t * TODO value and stopping on error here.\n \t\t\t * TODO Pass a repository struct through fetch_object,\n \t\t\t * such that arbitrary repositories work.\n \t\t\t */\n-- \n2.18.0.330.g17eb9fed90\n\n"},{"id":"354215","messageId":"20180802061505.2983-3-chriscool@tuxfamily.org","threadId":"49003","inReplyTo":"20180802061505.2983-1-chriscool@tuxfamily.org","subject":"[PATCH v4 2/9] Add initial remote odb support","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-08-02T06:14:58Z","receivedAt":"2018-08-02T06:15:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder \n\nThe remote-odb.{c,h} files will contain the functions\nthat are called by the rest of Git mostly from\n\"sha1-file.c\" to access the objects managed by the\nremote odbs.\n\nThe odb-helper.{c,h} files will contain the functions to\nactually implement communication with either the internal\nfunctions or the external scripts or processes that will\nmanage and provide remote git objects.\n\nFor now only infrastructure to create helpers from the\nconfig and to check if there are remote odbs and odb\nhelpers is implemented.\n\nWe expect that there will not be a lot of helpers, so it\nis ok to use a simple linked list to manage them.\n\nHelped-by: Jeff King \nSigned-off-by: Christian Couder \nSigned-off-by: Junio C Hamano \n---\n Makefile | 2 ++\n odb-helper.c | 16 +++++++++\n odb-helper.h | 19 +++++++++++\n remote-odb.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++\n remote-odb.h | 7 ++++\n 5 files changed, 135 insertions(+)\n create mode 100644 odb-helper.c\n create mode 100644 odb-helper.h\n create mode 100644 remote-odb.c\n create mode 100644 remote-odb.h\n\ndiff --git a/Makefile b/Makefile\nindex 08e5c54549..2a9bd02208 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -896,6 +896,8 @@ LIB_OBJS += notes-cache.o\n LIB_OBJS += notes-merge.o\n LIB_OBJS += notes-utils.o\n LIB_OBJS += object.o\n+LIB_OBJS += odb-helper.o\n+LIB_OBJS += remote-odb.o\n LIB_OBJS += oidmap.o\n LIB_OBJS += oidset.o\n LIB_OBJS += packfile.o\ndiff --git a/odb-helper.c b/odb-helper.c\nnew file mode 100644\nindex 0000000000..b4d403ffa9\n--- /dev/null\n+++ b/odb-helper.c\n@@ -0,0 +1,16 @@\n+#include \"cache.h\"\n+#include \"object.h\"\n+#include \"argv-array.h\"\n+#include \"odb-helper.h\"\n+#include \"run-command.h\"\n+#include \"sha1-lookup.h\"\n+\n+struct odb_helper *odb_helper_new(const char *name, int namelen)\n+{\n+\tstruct odb_helper *o;\n+\n+\to = xcalloc(1, sizeof(*o));\n+\to->name = xmemdupz(name, namelen);\n+\n+\treturn o;\n+}\ndiff --git a/odb-helper.h b/odb-helper.h\nnew file mode 100644\nindex 0000000000..4b792a3896\n--- /dev/null\n+++ b/odb-helper.h\n@@ -0,0 +1,19 @@\n+#ifndef ODB_HELPER_H\n+#define ODB_HELPER_H\n+\n+/*\n+ * An odb helper is a way to access a remote odb.\n+ *\n+ * Information in its fields comes from the config file [odb \"NAME\"]\n+ * entries.\n+ */\n+struct odb_helper {\n+\tconst char *name; /* odb..* */\n+\tconst char *remote; /* odb..promisorRemote */\n+\n+\tstruct odb_helper *next;\n+};\n+\n+extern struct odb_helper *odb_helper_new(const char *name, int namelen);\n+\n+#endif /* ODB_HELPER_H */\ndiff --git a/remote-odb.c b/remote-odb.c\nnew file mode 100644\nindex 0000000000..03be54ba2e\n--- /dev/null\n+++ b/remote-odb.c\n@@ -0,0 +1,91 @@\n+#include \"cache.h\"\n+#include \"remote-odb.h\"\n+#include \"odb-helper.h\"\n+#include \"config.h\"\n+\n+static struct odb_helper *helpers;\n+static struct odb_helper **helpers_tail = &helpers;\n+\n+static struct odb_helper *find_or_create_helper(const char *name, int len)\n+{\n+\tstruct odb_helper *o;\n+\n+\tfor (o = helpers; o; o = o->next)\n+\t\tif (!strncmp(o->name, name, len) && !o->name[len])\n+\t\t\treturn o;\n+\n+\to = odb_helper_new(name, len);\n+\t*helpers_tail = o;\n+\thelpers_tail = &o->next;\n+\n+\treturn o;\n+}\n+\n+static struct odb_helper *do_find_odb_helper(const char *remote)\n+{\n+\tstruct odb_helper *o;\n+\n+\tfor (o = helpers; o; o = o->next)\n+\t\tif (o->remote && !strcmp(o->remote, remote))\n+\t\t\treturn o;\n+\n+\treturn NULL;\n+}\n+\n+static int remote_odb_config(const char *var, const char *value, void *data)\n+{\n+\tstruct odb_helper *o;\n+\tconst char *name;\n+\tint namelen;\n+\tconst char *subkey;\n+\n+\tif (parse_config_key(var, \"odb\", &name, &namelen, &subkey) < 0)\n+\t\treturn 0;\n+\n+\to = find_or_create_helper(name, namelen);\n+\n+\tif (!strcmp(subkey, \"promisorremote\")) {\n+\t\tconst char *remote;\n+\t\tint res = git_config_string(&remote, var, value);\n+\n+\t\tif (res)\n+\t\t\treturn res;\n+\n+\t\tif (do_find_odb_helper(remote))\n+\t\t\treturn error(_(\"when parsing config key '%s' \"\n+\t\t\t\t \"helper for remote '%s' already exists\"),\n+\t\t\t\t var, remote);\n+\n+\t\to->remote = remote;\n+\n+\t\treturn 0;\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static void remote_odb_init(void)\n+{\n+\tstatic int initialized;\n+\n+\tif (initialized)\n+\t\treturn;\n+\tinitialized = 1;\n+\n+\tgit_config(remote_odb_config, NULL);\n+}\n+\n+struct odb_helper *find_odb_helper(const char *remote)\n+{\n+\tremote_odb_init();\n+\n+\tif (!remote)\n+\t\treturn helpers;\n+\n+\treturn do_find_odb_helper(remote);\n+}\n+\n+int has_remote_odb(void)\n+{\n+\treturn !!find_odb_helper(NULL);\n+}\ndiff --git a/remote-odb.h b/remote-odb.h\nnew file mode 100644\nindex 0000000000..4c7b13695f\n--- /dev/null\n+++ b/remote-odb.h\n@@ -0,0 +1,7 @@\n+#ifndef REMOTE_ODB_H\n+#define REMOTE_ODB_H\n+\n+extern struct odb_helper *find_odb_helper(const char *remote);\n+extern int has_remote_odb(void);\n+\n+#endif /* REMOTE_ODB_H */\n-- \n2.18.0.330.g17eb9fed90\n\n"},{"id":"354216","messageId":"20180802061505.2983-4-chriscool@tuxfamily.org","threadId":"49003","inReplyTo":"20180802061505.2983-1-chriscool@tuxfamily.org","subject":"[PATCH v4 3/9] remote-odb: implement remote_odb_get_direct()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-08-02T06:14:59Z","receivedAt":"2018-08-02T06:15:44Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder \n\nThis is implemented only in the promisor remote mode\nfor now by calling fetch_object().\n\nSigned-off-by: Christian Couder \nSigned-off-by: Junio C Hamano \n---\n odb-helper.c | 14 ++++++++++++++\n odb-helper.h | 3 ++-\n remote-odb.c | 17 +++++++++++++++++\n remote-odb.h | 1 +\n 4 files changed, 34 insertions(+), 1 deletion(-)\n\ndiff --git a/odb-helper.c b/odb-helper.c\nindex b4d403ffa9..99b5a345e8 100644\n--- a/odb-helper.c\n+++ b/odb-helper.c\n@@ -4,6 +4,7 @@\n #include \"odb-helper.h\"\n #include \"run-command.h\"\n #include \"sha1-lookup.h\"\n+#include \"fetch-object.h\"\n \n struct odb_helper *odb_helper_new(const char *name, int namelen)\n {\n@@ -14,3 +15,16 @@ struct odb_helper *odb_helper_new(const char *name, int namelen)\n \n \treturn o;\n }\n+\n+int odb_helper_get_direct(struct odb_helper *o,\n+\t\t\t const unsigned char *sha1)\n+{\n+\tint res;\n+\tuint64_t start = getnanotime();\n+\n+\tres = fetch_object(o->remote, sha1);\n+\n+\ttrace_performance_since(start, \"odb_helper_get_direct\");\n+\n+\treturn res;\n+}\ndiff --git a/odb-helper.h b/odb-helper.h\nindex 4b792a3896..4c52e81ce8 100644\n--- a/odb-helper.h\n+++ b/odb-helper.h\n@@ -15,5 +15,6 @@ struct odb_helper {\n };\n \n extern struct odb_helper *odb_helper_new(const char *name, int namelen);\n-\n+extern int odb_helper_get_direct(struct odb_helper *o,\n+\t\t\t\t const unsigned char *sha1);\n #endif /* ODB_HELPER_H */\ndiff --git a/remote-odb.c b/remote-odb.c\nindex 03be54ba2e..7f815c7ebb 100644\n--- a/remote-odb.c\n+++ b/remote-odb.c\n@@ -89,3 +89,20 @@ int has_remote_odb(void)\n {\n \treturn !!find_odb_helper(NULL);\n }\n+\n+int remote_odb_get_direct(const unsigned char *sha1)\n+{\n+\tstruct odb_helper *o;\n+\n+\ttrace_printf(\"trace: remote_odb_get_direct: %s\", sha1_to_hex(sha1));\n+\n+\tremote_odb_init();\n+\n+\tfor (o = helpers; o; o = o->next) {\n+\t\tif (odb_helper_get_direct(o, sha1) < 0)\n+\t\t\tcontinue;\n+\t\treturn 0;\n+\t}\n+\n+\treturn -1;\n+}\ndiff --git a/remote-odb.h b/remote-odb.h\nindex 4c7b13695f..c5384c922d 100644\n--- a/remote-odb.h\n+++ b/remote-odb.h\n@@ -3,5 +3,6 @@\n \n extern struct odb_helper *find_odb_helper(const char *remote);\n extern int has_remote_odb(void);\n+extern int remote_odb_get_direct(const unsigned char *sha1);\n \n #endif /* REMOTE_ODB_H */\n-- \n2.18.0.330.g17eb9fed90\n\n"},{"id":"354217","messageId":"20180802061505.2983-5-chriscool@tuxfamily.org","threadId":"49003","inReplyTo":"20180802061505.2983-1-chriscool@tuxfamily.org","subject":"[PATCH v4 4/9] remote-odb: implement remote_odb_get_many_direct()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-08-02T06:15:00Z","receivedAt":"2018-08-02T06:15:48Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder \n\nThis function will be used to get many objects from a promisor\nremote.\n\nSigned-off-by: Christian Couder \nSigned-off-by: Junio C Hamano \n---\n odb-helper.c | 15 +++++++++++++++\n odb-helper.h | 3 +++\n remote-odb.c | 17 +++++++++++++++++\n remote-odb.h | 1 +\n 4 files changed, 36 insertions(+)\n\ndiff --git a/odb-helper.c b/odb-helper.c\nindex 99b5a345e8..246ebf8f7a 100644\n--- a/odb-helper.c\n+++ b/odb-helper.c\n@@ -28,3 +28,18 @@ int odb_helper_get_direct(struct odb_helper *o,\n \n \treturn res;\n }\n+\n+int odb_helper_get_many_direct(struct odb_helper *o,\n+\t\t\t const struct oid_array *to_get)\n+{\n+\tint res;\n+\tuint64_t start;\n+\n+\tstart = getnanotime();\n+\n+\tres = fetch_objects(o->remote, to_get);\n+\n+\ttrace_performance_since(start, \"odb_helper_get_many_direct\");\n+\n+\treturn res;\n+}\ndiff --git a/odb-helper.h b/odb-helper.h\nindex 4c52e81ce8..154ce4c7e4 100644\n--- a/odb-helper.h\n+++ b/odb-helper.h\n@@ -17,4 +17,7 @@ struct odb_helper {\n extern struct odb_helper *odb_helper_new(const char *name, int namelen);\n extern int odb_helper_get_direct(struct odb_helper *o,\n \t\t\t\t const unsigned char *sha1);\n+extern int odb_helper_get_many_direct(struct odb_helper *o,\n+\t\t\t\t const struct oid_array *to_get);\n+\n #endif /* ODB_HELPER_H */\ndiff --git a/remote-odb.c b/remote-odb.c\nindex 7f815c7ebb..09dfc2e16f 100644\n--- a/remote-odb.c\n+++ b/remote-odb.c\n@@ -106,3 +106,20 @@ int remote_odb_get_direct(const unsigned char *sha1)\n \n \treturn -1;\n }\n+\n+int remote_odb_get_many_direct(const struct oid_array *to_get)\n+{\n+\tstruct odb_helper *o;\n+\n+\ttrace_printf(\"trace: remote_odb_get_many_direct: nr: %d\", to_get->nr);\n+\n+\tremote_odb_init();\n+\n+\tfor (o = helpers; o; o = o->next) {\n+\t\tif (odb_helper_get_many_direct(o, to_get) < 0)\n+\t\t\tcontinue;\n+\t\treturn 0;\n+\t}\n+\n+\treturn -1;\n+}\ndiff --git a/remote-odb.h b/remote-odb.h\nindex c5384c922d..e10a8bf855 100644\n--- a/remote-odb.h\n+++ b/remote-odb.h\n@@ -4,5 +4,6 @@\n extern struct odb_helper *find_odb_helper(const char *remote);\n extern int has_remote_odb(void);\n extern int remote_odb_get_direct(const unsigned char *sha1);\n+extern int remote_odb_get_many_direct(const struct oid_array *to_get);\n \n #endif /* REMOTE_ODB_H */\n-- \n2.18.0.330.g17eb9fed90\n\n"},{"id":"354218","messageId":"20180802061505.2983-6-chriscool@tuxfamily.org","threadId":"49003","inReplyTo":"20180802061505.2983-1-chriscool@tuxfamily.org","subject":"[PATCH v4 5/9] remote-odb: add remote_odb_reinit()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-08-02T06:15:01Z","receivedAt":"2018-08-02T06:15:50Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder \n\nWe will need to reinitialize the remote odb configuration\nas we will make some changes to it in a later commit when\nwe will detect that a remote is also a remote odb.\n\nSigned-off-by: Christian Couder \nSigned-off-by: Junio C Hamano \n---\n remote-odb.c | 14 ++++++++++++--\n remote-odb.h | 1 +\n 2 files changed, 13 insertions(+), 2 deletions(-)\n\ndiff --git a/remote-odb.c b/remote-odb.c\nindex 09dfc2e16f..f063ba0fda 100644\n--- a/remote-odb.c\n+++ b/remote-odb.c\n@@ -64,17 +64,27 @@ static int remote_odb_config(const char *var, const char *value, void *data)\n \treturn 0;\n }\n \n-static void remote_odb_init(void)\n+static void remote_odb_do_init(int force)\n {\n \tstatic int initialized;\n \n-\tif (initialized)\n+\tif (!force && initialized)\n \t\treturn;\n \tinitialized = 1;\n \n \tgit_config(remote_odb_config, NULL);\n }\n \n+static inline void remote_odb_init(void)\n+{\n+\tremote_odb_do_init(0);\n+}\n+\n+void remote_odb_reinit(void)\n+{\n+\tremote_odb_do_init(1);\n+}\n+\n struct odb_helper *find_odb_helper(const char *remote)\n {\n \tremote_odb_init();\ndiff --git a/remote-odb.h b/remote-odb.h\nindex e10a8bf855..79803782af 100644\n--- a/remote-odb.h\n+++ b/remote-odb.h\n@@ -1,6 +1,7 @@\n #ifndef REMOTE_ODB_H\n #define REMOTE_ODB_H\n \n+extern void remote_odb_reinit(void);\n extern struct odb_helper *find_odb_helper(const char *remote);\n extern int has_remote_odb(void);\n extern int remote_odb_get_direct(const unsigned char *sha1);\n-- \n2.18.0.330.g17eb9fed90\n\n"},{"id":"354219","messageId":"20180802061505.2983-7-chriscool@tuxfamily.org","threadId":"49003","inReplyTo":"20180802061505.2983-1-chriscool@tuxfamily.org","subject":"[PATCH v4 6/9] Use remote_odb_get_direct() and has_remote_odb()","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-08-02T06:15:02Z","receivedAt":"2018-08-02T06:15:59Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder \n\nInstead of using the repository_format_partial_clone global\nand fetch_object() directly, let's use has_remote_odb() and\nremote_odb_get_direct().\n\nSigned-off-by: Christian Couder \nSigned-off-by: Junio C Hamano \n---\n builtin/cat-file.c | 5 +++--\n builtin/fetch.c | 11 ++++++-----\n builtin/gc.c | 3 ++-\n builtin/repack.c | 3 ++-\n cache.h | 2 --\n connected.c | 3 ++-\n environment.c | 1 -\n list-objects-filter-options.c | 27 +++++++++++++++------------\n packfile.c | 3 ++-\n setup.c | 7 +------\n sha1-file.c | 14 ++++++++------\n t/t0410-partial-clone.sh | 34 +++++++++++++++++-----------------\n t/t5500-fetch-pack.sh | 4 ++--\n t/t5601-clone.sh | 2 +-\n t/t5616-partial-clone.sh | 2 +-\n t/t5702-protocol-v2.sh | 2 +-\n unpack-trees.c | 6 +++---\n 17 files changed, 66 insertions(+), 63 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex 4a44b2404f..4d19e9277e 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -14,6 +14,7 @@\n #include \"sha1-array.h\"\n #include \"packfile.h\"\n #include \"object-store.h\"\n+#include \"remote-odb.h\"\n \n struct batch_options {\n \tint enabled;\n@@ -478,8 +479,8 @@ static int batch_objects(struct batch_options *opt)\n \n \t\tfor_each_loose_object(batch_loose_object, &sa, 0);\n \t\tfor_each_packed_object(batch_packed_object, &sa, 0);\n-\t\tif (repository_format_partial_clone)\n-\t\t\twarning(\"This repository has extensions.partialClone set. Some objects may not be loaded.\");\n+\t\tif (has_remote_odb())\n+\t\t\twarning(\"This repository uses an odb. Some objects may not be loaded.\");\n \n \t\tcb.opt = opt;\n \t\tcb.expand = &data;\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex ac06f6a576..9c7df8356c 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -22,6 +22,7 @@\n #include \"utf8.h\"\n #include \"packfile.h\"\n #include \"list-objects-filter-options.h\"\n+#include \"remote-odb.h\"\n \n static const char * const builtin_fetch_usage[] = {\n \tN_(\"git fetch [] [ [...]]\"),\n@@ -1339,7 +1340,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)\n \t * If no prior partial clone/fetch and the current fetch DID NOT\n \t * request a partial-fetch, do a normal fetch.\n \t */\n-\tif (!repository_format_partial_clone && !filter_options.choice)\n+\tif (!has_remote_odb() && !filter_options.choice)\n \t\treturn;\n \n \t/*\n@@ -1347,7 +1348,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)\n \t * on this repo and remember the given filter-spec as the default\n \t * for subsequent fetches to this remote.\n \t */\n-\tif (!repository_format_partial_clone && filter_options.choice) {\n+\tif (!has_remote_odb() && filter_options.choice) {\n \t\tpartial_clone_register(remote->name, &filter_options);\n \t\treturn;\n \t}\n@@ -1356,7 +1357,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)\n \t * We are currently limited to only ONE promisor remote and only\n \t * allow partial-fetches from the promisor remote.\n \t */\n-\tif (strcmp(remote->name, repository_format_partial_clone)) {\n+\tif (!find_odb_helper(remote->name)) {\n \t\tif (filter_options.choice)\n \t\t\tdie(_(\"--filter can only be used with the remote configured in core.partialClone\"));\n \t\treturn;\n@@ -1487,7 +1488,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \tif (depth || deepen_since || deepen_not.nr)\n \t\tdeepen = 1;\n \n-\tif (filter_options.choice && !repository_format_partial_clone)\n+\tif (filter_options.choice && !has_remote_odb())\n \t\tdie(\"--filter can only be used when extensions.partialClone is set\");\n \n \tif (all) {\n@@ -1521,7 +1522,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (remote) {\n-\t\tif (filter_options.choice || repository_format_partial_clone)\n+\t\tif (filter_options.choice || has_remote_odb())\n \t\t\tfetch_one_setup_partial(remote);\n \t\tresult = fetch_one(remote, argc, argv, prune_tags_ok);\n \t} else {\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex ccfb1ceaeb..02c783b514 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -26,6 +26,7 @@\n #include \"pack-objects.h\"\n #include \"blob.h\"\n #include \"tree.h\"\n+#include \"remote-odb.h\"\n \n #define FAILED_RUN \"failed to run %s\"\n \n@@ -619,7 +620,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\t\targv_array_push(&prune, prune_expire);\n \t\t\tif (quiet)\n \t\t\t\targv_array_push(&prune, \"--no-progress\");\n-\t\t\tif (repository_format_partial_clone)\n+\t\t\tif (has_remote_odb())\n \t\t\t\targv_array_push(&prune,\n \t\t\t\t\t\t\"--exclude-promisor-objects\");\n \t\t\tif (run_command_v_opt(prune.argv, RUN_GIT_CMD))\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex 6c636e159e..72b1e2d94f 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -8,6 +8,7 @@\n #include \"strbuf.h\"\n #include \"string-list.h\"\n #include \"argv-array.h\"\n+#include \"remote-odb.h\"\n \n static int delta_base_offset = 1;\n static int pack_kept_objects = -1;\n@@ -249,7 +250,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)\n \targv_array_push(&cmd.args, \"--all\");\n \targv_array_push(&cmd.args, \"--reflog\");\n \targv_array_push(&cmd.args, \"--indexed-objects\");\n-\tif (repository_format_partial_clone)\n+\tif (has_remote_odb())\n \t\targv_array_push(&cmd.args, \"--exclude-promisor-objects\");\n \tif (window)\n \t\targv_array_pushf(&cmd.args, \"--window=%s\", window);\ndiff --git a/cache.h b/cache.h\nindex 8b447652a7..a0772bd099 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -904,13 +904,11 @@ extern int grafts_replace_parents;\n #define GIT_REPO_VERSION 0\n #define GIT_REPO_VERSION_READ 1\n extern int repository_format_precious_objects;\n-extern char *repository_format_partial_clone;\n extern const char *core_partial_clone_filter_default;\n \n struct repository_format {\n \tint version;\n \tint precious_objects;\n-\tchar *partial_clone; /* value of extensions.partialclone */\n \tint is_bare;\n \tint hash_algo;\n \tchar *work_tree;\ndiff --git a/connected.c b/connected.c\nindex 1bba888eff..7743b78290 100644\n--- a/connected.c\n+++ b/connected.c\n@@ -4,6 +4,7 @@\n #include \"connected.h\"\n #include \"transport.h\"\n #include \"packfile.h\"\n+#include \"remote-odb.h\"\n \n /*\n * If we feed all the commits we want to verify to this command\n@@ -56,7 +57,7 @@ int check_connected(oid_iterate_fn fn, void *cb_data,\n \targv_array_push(&rev_list.args,\"rev-list\");\n \targv_array_push(&rev_list.args, \"--objects\");\n \targv_array_push(&rev_list.args, \"--stdin\");\n-\tif (repository_format_partial_clone)\n+\tif (has_remote_odb())\n \t\targv_array_push(&rev_list.args, \"--exclude-promisor-objects\");\n \tif (!opt->is_deepening_fetch) {\n \t\targv_array_push(&rev_list.args, \"--not\");\ndiff --git a/environment.c b/environment.c\nindex 013e845235..8234940046 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -30,7 +30,6 @@ int warn_ambiguous_refs = 1;\n int warn_on_object_refname_ambiguity = 1;\n int ref_paranoia = -1;\n int repository_format_precious_objects;\n-char *repository_format_partial_clone;\n const char *core_partial_clone_filter_default;\n const char *git_commit_encoding;\n const char *git_log_output_encoding;\ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nindex c0e2bd6a06..60452c8f36 100644\n--- a/list-objects-filter-options.c\n+++ b/list-objects-filter-options.c\n@@ -6,6 +6,7 @@\n #include \"list-objects.h\"\n #include \"list-objects-filter.h\"\n #include \"list-objects-filter-options.h\"\n+#include \"remote-odb.h\"\n \n /*\n * Parse value of the argument to the \"filter\" keyword.\n@@ -114,30 +115,32 @@ void partial_clone_register(\n \tconst char *remote,\n \tconst struct list_objects_filter_options *filter_options)\n {\n-\t/*\n-\t * Record the name of the partial clone remote in the\n-\t * config and in the global variable -- the latter is\n-\t * used throughout to indicate that partial clone is\n-\t * enabled and to expect missing objects.\n-\t */\n-\tif (repository_format_partial_clone &&\n-\t *repository_format_partial_clone &&\n-\t strcmp(remote, repository_format_partial_clone))\n-\t\tdie(_(\"cannot change partial clone promisor remote\"));\n+\tchar *cfg_name;\n+\n+\t/* Check if it is already registered */\n+\tif (find_odb_helper(remote))\n+\t\treturn;\n \n \tgit_config_set(\"core.repositoryformatversion\", \"1\");\n-\tgit_config_set(\"extensions.partialclone\", remote);\n \n-\trepository_format_partial_clone = xstrdup(remote);\n+\t/* Add odb config for the remote */\n+\tcfg_name = xstrfmt(\"odb.%s.promisorRemote\", remote);\n+\tgit_config_set(cfg_name, remote);\n+\tfree(cfg_name);\n \n \t/*\n \t * Record the initial filter-spec in the config as\n \t * the default for subsequent fetches from this remote.\n+\t *\n+\t * TODO: move core.partialclonefilter into odb.\n \t */\n \tcore_partial_clone_filter_default =\n \t\txstrdup(filter_options->filter_spec);\n \tgit_config_set(\"core.partialclonefilter\",\n \t\t core_partial_clone_filter_default);\n+\n+\t/* Make sure the config info are reset */\n+\tremote_odb_reinit();\n }\n \n void partial_clone_get_default_filter_spec(\ndiff --git a/packfile.c b/packfile.c\nindex 7cd45aa4b2..9cbc974612 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -15,6 +15,7 @@\n #include \"tree-walk.h\"\n #include \"tree.h\"\n #include \"object-store.h\"\n+#include \"remote-odb.h\"\n \n char *odb_pack_name(struct strbuf *buf,\n \t\t const unsigned char *sha1,\n@@ -1976,7 +1977,7 @@ int is_promisor_object(const struct object_id *oid)\n \tstatic int promisor_objects_prepared;\n \n \tif (!promisor_objects_prepared) {\n-\t\tif (repository_format_partial_clone) {\n+\t\tif (has_remote_odb()) {\n \t\t\tfor_each_packed_object(add_promisor_object,\n \t\t\t\t\t &promisor_objects,\n \t\t\t\t\t FOR_EACH_OBJECT_PROMISOR_ONLY);\ndiff --git a/setup.c b/setup.c\nindex b24c811c1c..4eeb7fe073 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -419,11 +419,7 @@ static int check_repo_format(const char *var, const char *value, void *vdata)\n \t\t\t;\n \t\telse if (!strcmp(ext, \"preciousobjects\"))\n \t\t\tdata->precious_objects = git_config_bool(var, value);\n-\t\telse if (!strcmp(ext, \"partialclone\")) {\n-\t\t\tif (!value)\n-\t\t\t\treturn config_error_nonbool(var);\n-\t\t\tdata->partial_clone = xstrdup(value);\n-\t\t} else\n+\t\telse\n \t\t\tstring_list_append(&data->unknown_extensions, ext);\n \t} else if (strcmp(var, \"core.bare\") == 0) {\n \t\tdata->is_bare = git_config_bool(var, value);\n@@ -465,7 +461,6 @@ static int check_repository_format_gently(const char *gitdir, struct repository_\n \t}\n \n \trepository_format_precious_objects = candidate->precious_objects;\n-\trepository_format_partial_clone = candidate->partial_clone;\n \tstring_list_clear(&candidate->unknown_extensions, 0);\n \tif (!has_common) {\n \t\tif (candidate->is_bare != -1) {\ndiff --git a/sha1-file.c b/sha1-file.c\nindex c099f5584d..5e183d2cd4 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -32,6 +32,7 @@\n #include \"packfile.h\"\n #include \"fetch-object.h\"\n #include \"object-store.h\"\n+#include \"remote-odb.h\"\n \n /* The maximum size for an object header. */\n #define MAX_HEADER_LEN 32\n@@ -1309,15 +1310,16 @@ int oid_object_info_extended(struct repository *r, const struct object_id *oid,\n \t\t}\n \n \t\t/* Check if it is a missing object */\n-\t\tif (fetch_if_missing && repository_format_partial_clone &&\n+\t\tif (fetch_if_missing && has_remote_odb() &&\n \t\t !already_retried && r == the_repository) {\n \t\t\t/*\n-\t\t\t * TODO Investigate checking fetch_object() return\n-\t\t\t * TODO value and stopping on error here.\n-\t\t\t * TODO Pass a repository struct through fetch_object,\n-\t\t\t * such that arbitrary repositories work.\n+\t\t\t * TODO Investigate checking remote_odb_get_direct()\n+\t\t\t * TODO return value and stopping on error here.\n+\t\t\t * TODO Pass a repository struct through\n+\t\t\t * remote_odb_get_direct(), such that arbitrary\n+\t\t\t * repositories work.\n \t\t\t */\n-\t\t\tfetch_object(repository_format_partial_clone, real->hash);\n+\t\t\tremote_odb_get_direct(real->hash);\n \t\t\talready_retried = 1;\n \t\t\tcontinue;\n \t\t}\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex 4984ca583d..71a9b9a3e8 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -23,10 +23,10 @@ promise_and_delete () {\n \tdelete_object repo \"$HASH\"\n }\n \n-test_expect_success 'extensions.partialclone without filter' '\n+test_expect_success 'promisor remote without filter' '\n \ttest_create_repo server &&\n \tgit clone --filter=\"blob:none\" \"file://$(pwd)/server\" client &&\n-\tgit -C client config --unset core.partialclonefilter &&\n+\tgit -C client config --unset odb.origin.partialclonefilter &&\n \tgit -C client fetch origin\n '\n \n@@ -51,7 +51,7 @@ test_expect_success 'missing reflog object, but promised by a commit, passes fsc\n \n \t# But with the extension, it succeeds\n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo fsck\n '\n \n@@ -74,7 +74,7 @@ test_expect_success 'missing reflog object, but promised by a tag, passes fsck'\n \tprintf \"$T\\n\" | pack_as_from_promisor &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo fsck\n '\n \n@@ -92,7 +92,7 @@ test_expect_success 'missing reflog object alone fails fsck, even with extension\n \tdelete_object repo \"$A\" &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \ttest_must_fail git -C repo fsck\n '\n \n@@ -108,7 +108,7 @@ test_expect_success 'missing ref object, but promised, passes fsck' '\n \tpromise_and_delete \"$A\" &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo fsck\n '\n \n@@ -131,7 +131,7 @@ test_expect_success 'missing object, but promised, passes fsck' '\n \tpromise_and_delete \"$AT\" &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo fsck\n '\n \n@@ -144,7 +144,7 @@ test_expect_success 'missing CLI object, but promised, passes fsck' '\n \tpromise_and_delete \"$A\" &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo fsck \"$A\"\n '\n \n@@ -159,7 +159,7 @@ test_expect_success 'fetching of missing objects' '\n \trm -rf repo/.git/objects/* &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"origin\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"origin\" &&\n \tgit -C repo cat-file -p \"$HASH\" &&\n \n \t# Ensure that the .promisor file is written, and check that its\n@@ -180,7 +180,7 @@ test_expect_success 'rev-list stops traversal at missing and promised commit' '\n \tpromise_and_delete \"$FOO\" &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo rev-list --exclude-promisor-objects --objects bar >out &&\n \tgrep $(git -C repo rev-parse bar) out &&\n \t! grep $FOO out\n@@ -205,7 +205,7 @@ test_expect_success 'rev-list stops traversal at missing and promised tree' '\n \tpromise_and_delete \"$TREE2\" &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo rev-list --exclude-promisor-objects --objects HEAD >out &&\n \tgrep $(git -C repo rev-parse foo) out &&\n \t! grep $TREE out &&\n@@ -224,7 +224,7 @@ test_expect_success 'rev-list stops traversal at missing and promised blob' '\n \tpromise_and_delete \"$BLOB\" &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo rev-list --exclude-promisor-objects --objects HEAD >out &&\n \tgrep $(git -C repo rev-parse HEAD) out &&\n \t! grep $BLOB out\n@@ -243,7 +243,7 @@ test_expect_success 'rev-list stops traversal at promisor commit, tree, and blob\n \tprintf \"%s\\n%s\\n%s\\n\" $COMMIT $TREE $BLOB | pack_as_from_promisor &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo rev-list --exclude-promisor-objects --objects HEAD >out &&\n \t! grep $COMMIT out &&\n \t! grep $TREE out &&\n@@ -267,7 +267,7 @@ test_expect_success 'rev-list accepts missing and promised objects on command li\n \tpromise_and_delete $BLOB &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo rev-list --exclude-promisor-objects --objects \"$COMMIT\" \"$TREE\" \"$BLOB\"\n '\n \n@@ -280,7 +280,7 @@ test_expect_success 'gc does not repack promisor objects' '\n \tHASH=$(printf \"$TREE_HASH\\n\" | pack_as_from_promisor) &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo gc &&\n \n \t# Ensure that the promisor packfile still exists, and remove it\n@@ -304,7 +304,7 @@ test_expect_success 'gc stops traversal when a missing but promised object is re\n \tHASH=$(promise_and_delete $TREE_HASH) &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C repo gc &&\n \n \t# Ensure that the promisor packfile still exists, and remove it\n@@ -335,7 +335,7 @@ test_expect_success 'fetching of missing objects from an HTTP server' '\n \trm -rf repo/.git/objects/* &&\n \n \tgit -C repo config core.repositoryformatversion 1 &&\n-\tgit -C repo config extensions.partialclone \"origin\" &&\n+\tgit -C repo config odb.magic.promisorRemote \"origin\" &&\n \tgit -C repo cat-file -p \"$HASH\" &&\n \n \t# Ensure that the .promisor file is written, and check that its\ndiff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh\nindex 3d33ab3875..8605b9b3b5 100755\n--- a/t/t5500-fetch-pack.sh\n+++ b/t/t5500-fetch-pack.sh\n@@ -851,14 +851,14 @@ fetch_filter_blob_limit_zero () {\n \ttest_config -C \"$SERVER\" uploadpack.allowfilter 1 &&\n \n \tgit clone \"$URL\" client &&\n-\ttest_config -C client extensions.partialclone origin &&\n+\ttest_config -C client odb.magic.promisorRemote origin &&\n \n \ttest_commit -C \"$SERVER\" two &&\n \n \tgit -C client fetch --filter=blob:limit=0 origin HEAD:somewhere &&\n \n \t# Ensure that commit is fetched, but blob is not\n-\ttest_config -C client extensions.partialclone \"arbitrary string\" &&\n+\ttest_config -C client odb.magic.promisorRemote \"arbitrary string\" &&\n \tgit -C client cat-file -e $(git -C \"$SERVER\" rev-parse two) &&\n \ttest_must_fail git -C client cat-file -e $(git hash-object \"$SERVER/two.t\")\n }\ndiff --git a/t/t5601-clone.sh b/t/t5601-clone.sh\nindex 0b62037744..022c65a7ae 100755\n--- a/t/t5601-clone.sh\n+++ b/t/t5601-clone.sh\n@@ -647,7 +647,7 @@ partial_clone () {\n \tgit -C client fsck &&\n \n \t# Ensure that unneeded blobs are not inadvertently fetched.\n-\ttest_config -C client extensions.partialclone \"not a remote\" &&\n+\ttest_config -C client odb.origin.promisorRemote \"not a remote\" &&\n \ttest_must_fail git -C client cat-file -e \"$HASH1\" &&\n \n \t# But this blob was fetched, because clone performs an initial checkout\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex 44d8e80171..f7ed3c40e2 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -40,7 +40,7 @@ test_expect_success 'do partial clone 1' '\n \t\t| sort >observed.oids &&\n \ttest_cmp expect_1.oids observed.oids &&\n \ttest \"$(git -C pc1 config --local core.repositoryformatversion)\" = \"1\" &&\n-\ttest \"$(git -C pc1 config --local extensions.partialclone)\" = \"origin\" &&\n+\ttest \"$(git -C pc1 config --local odb.origin.promisorRemote)\" = \"origin\" &&\n \ttest \"$(git -C pc1 config --local core.partialclonefilter)\" = \"blob:none\"\n '\n \ndiff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh\nindex a4fe6508bd..0c47599568 100755\n--- a/t/t5702-protocol-v2.sh\n+++ b/t/t5702-protocol-v2.sh\n@@ -285,7 +285,7 @@ test_expect_success 'partial fetch' '\n \trm -rf client \"$(pwd)/trace\" &&\n \tgit init client &&\n \tSERVER=\"file://$(pwd)/server\" &&\n-\ttest_config -C client extensions.partialClone \"$SERVER\" &&\n+\ttest_config -C client odb.magic.promisorRemote \"$SERVER\" &&\n \n \tGIT_TRACE_PACKET=\"$(pwd)/trace\" git -C client -c protocol.version=2 \\\n \t\tfetch --filter=blob:none \"$SERVER\" master:refs/heads/other &&\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex cd0680f11e..13da008198 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -18,6 +18,7 @@\n #include \"fsmonitor.h\"\n #include \"object-store.h\"\n #include \"fetch-object.h\"\n+#include \"remote-odb.h\"\n \n /*\n * Error messages expected by scripts out of plumbing commands such as\n@@ -383,7 +384,7 @@ static int check_updates(struct unpack_trees_options *o)\n \t\tload_gitmodules_file(index, &state);\n \n \tenable_delayed_checkout(&state);\n-\tif (repository_format_partial_clone && o->update && !o->dry_run) {\n+\tif (has_remote_odb() && o->update && !o->dry_run) {\n \t\t/*\n \t\t * Prefetch the objects that are to be checked out in the loop\n \t\t * below.\n@@ -400,8 +401,7 @@ static int check_updates(struct unpack_trees_options *o)\n \t\t\t}\n \t\t}\n \t\tif (to_fetch.nr)\n-\t\t\tfetch_objects(repository_format_partial_clone,\n-\t\t\t\t &to_fetch);\n+\t\t\tremote_odb_get_many_direct(&to_fetch);\n \t\tfetch_if_missing = fetch_if_missing_store;\n \t\toid_array_clear(&to_fetch);\n \t}\n-- \n2.18.0.330.g17eb9fed90\n\n"},{"id":"354220","messageId":"20180802061505.2983-8-chriscool@tuxfamily.org","threadId":"49003","inReplyTo":"20180802061505.2983-1-chriscool@tuxfamily.org","subject":"[PATCH v4 7/9] Use odb.origin.partialclonefilter instead of core.partialclonefilter","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-08-02T06:15:03Z","receivedAt":"2018-08-02T06:16:02Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder \n\nLet's make the partial clone filter specific to one odb\ninstead of general to all the odbs.\n\nThis makes it possible to have different partial clone\nfilters for different odbs.\n\nSigned-off-by: Christian Couder \nSigned-off-by: Junio C Hamano \n---\n builtin/fetch.c | 2 +-\n list-objects-filter-options.c | 28 ++++++++++++++++------------\n list-objects-filter-options.h | 3 ++-\n odb-helper.h | 1 +\n remote-odb.c | 2 ++\n t/t5616-partial-clone.sh | 2 +-\n 6 files changed, 23 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 9c7df8356c..f3dee73179 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1369,7 +1369,7 @@ static inline void fetch_one_setup_partial(struct remote *remote)\n \t * the config.\n \t */\n \tif (!filter_options.choice)\n-\t\tpartial_clone_get_default_filter_spec(&filter_options);\n+\t\tpartial_clone_get_default_filter_spec(&filter_options, remote->name);\n \treturn;\n }\n \ndiff --git a/list-objects-filter-options.c b/list-objects-filter-options.c\nindex 60452c8f36..755a793664 100644\n--- a/list-objects-filter-options.c\n+++ b/list-objects-filter-options.c\n@@ -7,6 +7,7 @@\n #include \"list-objects-filter.h\"\n #include \"list-objects-filter-options.h\"\n #include \"remote-odb.h\"\n+#include \"odb-helper.h\"\n \n /*\n * Parse value of the argument to the \"filter\" keyword.\n@@ -29,6 +30,9 @@ static int gently_parse_list_objects_filter(\n {\n \tconst char *v0;\n \n+\tif (!arg)\n+\t\treturn 0;\n+\n \tif (filter_options->choice) {\n \t\tif (errbuf) {\n \t\t\tstrbuf_init(errbuf, 0);\n@@ -116,6 +120,7 @@ void partial_clone_register(\n \tconst struct list_objects_filter_options *filter_options)\n {\n \tchar *cfg_name;\n+\tchar *filter_name;\n \n \t/* Check if it is already registered */\n \tif (find_odb_helper(remote))\n@@ -131,27 +136,26 @@ void partial_clone_register(\n \t/*\n \t * Record the initial filter-spec in the config as\n \t * the default for subsequent fetches from this remote.\n-\t *\n-\t * TODO: move core.partialclonefilter into odb.\n \t */\n-\tcore_partial_clone_filter_default =\n-\t\txstrdup(filter_options->filter_spec);\n-\tgit_config_set(\"core.partialclonefilter\",\n-\t\t core_partial_clone_filter_default);\n+\tfilter_name = xstrfmt(\"odb.%s.partialclonefilter\", remote);\n+\tgit_config_set(filter_name, filter_options->filter_spec);\n+\tfree(filter_name);\n \n \t/* Make sure the config info are reset */\n \tremote_odb_reinit();\n }\n \n void partial_clone_get_default_filter_spec(\n-\tstruct list_objects_filter_options *filter_options)\n+\tstruct list_objects_filter_options *filter_options,\n+\tconst char *remote)\n {\n+\tstruct odb_helper *helper = find_odb_helper(remote);\n+\n \t/*\n \t * Parse default value, but silently ignore it if it is invalid.\n \t */\n-\tif (!core_partial_clone_filter_default)\n-\t\treturn;\n-\tgently_parse_list_objects_filter(filter_options,\n-\t\t\t\t\t core_partial_clone_filter_default,\n-\t\t\t\t\t NULL);\n+\tif (helper)\n+\t\tgently_parse_list_objects_filter(filter_options,\n+\t\t\t\t\t\t helper->partial_clone_filter,\n+\t\t\t\t\t\t NULL);\n }\ndiff --git a/list-objects-filter-options.h b/list-objects-filter-options.h\nindex 0000a61f82..12ceef3230 100644\n--- a/list-objects-filter-options.h\n+++ b/list-objects-filter-options.h\n@@ -74,6 +74,7 @@ void partial_clone_register(\n \tconst char *remote,\n \tconst struct list_objects_filter_options *filter_options);\n void partial_clone_get_default_filter_spec(\n-\tstruct list_objects_filter_options *filter_options);\n+\tstruct list_objects_filter_options *filter_options,\n+\tconst char *remote);\n \n #endif /* LIST_OBJECTS_FILTER_OPTIONS_H */\ndiff --git a/odb-helper.h b/odb-helper.h\nindex 154ce4c7e4..4af75ef55c 100644\n--- a/odb-helper.h\n+++ b/odb-helper.h\n@@ -10,6 +10,7 @@\n struct odb_helper {\n \tconst char *name; /* odb..* */\n \tconst char *remote; /* odb..promisorRemote */\n+\tconst char *partial_clone_filter; /* odb..partialCloneFilter */\n \n \tstruct odb_helper *next;\n };\ndiff --git a/remote-odb.c b/remote-odb.c\nindex f063ba0fda..49cf8e30aa 100644\n--- a/remote-odb.c\n+++ b/remote-odb.c\n@@ -60,6 +60,8 @@ static int remote_odb_config(const char *var, const char *value, void *data)\n \n \t\treturn 0;\n \t}\n+\tif (!strcmp(subkey, \"partialclonefilter\"))\n+\t\treturn git_config_string(&o->partial_clone_filter, var, value);\n \n \treturn 0;\n }\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex f7ed3c40e2..e2aeee1d7d 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -41,7 +41,7 @@ test_expect_success 'do partial clone 1' '\n \ttest_cmp expect_1.oids observed.oids &&\n \ttest \"$(git -C pc1 config --local core.repositoryformatversion)\" = \"1\" &&\n \ttest \"$(git -C pc1 config --local odb.origin.promisorRemote)\" = \"origin\" &&\n-\ttest \"$(git -C pc1 config --local core.partialclonefilter)\" = \"blob:none\"\n+\ttest \"$(git -C pc1 config --local odb.origin.partialclonefilter)\" = \"blob:none\"\n '\n \n # checkout master to force dynamic object fetch of blobs at HEAD.\n-- \n2.18.0.330.g17eb9fed90\n\n"},{"id":"354221","messageId":"20180802061505.2983-9-chriscool@tuxfamily.org","threadId":"49003","inReplyTo":"20180802061505.2983-1-chriscool@tuxfamily.org","subject":"[PATCH v4 8/9] t0410: test fetching from many promisor remotes","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-08-02T06:15:04Z","receivedAt":"2018-08-02T06:16:04Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder \n\nSigned-off-by: Christian Couder \nSigned-off-by: Junio C Hamano \n---\n t/t0410-partial-clone.sh | 24 +++++++++++++++++++++++-\n 1 file changed, 23 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex 71a9b9a3e8..9d513ebf57 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -170,8 +170,30 @@ test_expect_success 'fetching of missing objects' '\n \tgit verify-pack --verbose \"$IDX\" | grep \"$HASH\"\n '\n \n+test_expect_success 'fetching of missing objects from another odb remote' '\n+\tgit clone \"file://$(pwd)/server\" server2 &&\n+\ttest_commit -C server2 bar &&\n+\tgit -C server2 repack -a -d --write-bitmap-index &&\n+\tHASH2=$(git -C server2 rev-parse bar) &&\n+\n+\tgit -C repo remote add server2 \"file://$(pwd)/server2\" &&\n+\tgit -C repo config odb.magic2.promisorRemote server2 &&\n+\tgit -C repo cat-file -p \"$HASH2\" &&\n+\n+\tgit -C repo fetch server2 &&\n+\trm -rf repo/.git/objects/* &&\n+\tgit -C repo cat-file -p \"$HASH2\" &&\n+\n+\t# Ensure that the .promisor file is written, and check that its\n+\t# associated packfile contains the object\n+\tls repo/.git/objects/pack/pack-*.promisor >promisorlist &&\n+\ttest_line_count = 1 promisorlist &&\n+\tIDX=$(cat promisorlist | sed \"s/promisor$/idx/\") &&\n+\tgit verify-pack --verbose \"$IDX\" | grep \"$HASH2\"\n+'\n+\n test_expect_success 'rev-list stops traversal at missing and promised commit' '\n-\trm -rf repo &&\n+\trm -rf repo server server2 &&\n \ttest_create_repo repo &&\n \ttest_commit -C repo foo &&\n \ttest_commit -C repo bar &&\n-- \n2.18.0.330.g17eb9fed90\n\n"},{"id":"354222","messageId":"20180802061505.2983-10-chriscool@tuxfamily.org","threadId":"49003","inReplyTo":"20180802061505.2983-1-chriscool@tuxfamily.org","subject":"[PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-08-02T06:15:05Z","receivedAt":"2018-08-02T06:16:08Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"From: Christian Couder \n\nSigned-off-by: Junio C Hamano \n---\n Documentation/config.txt | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 43b2de7b5f..2d048d47f2 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -2513,6 +2513,11 @@ This setting can be overridden with the `GIT_NOTES_REWRITE_REF`\n environment variable, which must be a colon separated list of refs or\n globs.\n \n+odb..promisorRemote::\n+\tThe name of a promisor remote. For now promisor remotes are\n+\tthe only kind of remote object database (odb) that is\n+\tsupported.\n+\n pack.window::\n \tThe size of the window used by linkgit:git-pack-objects[1] when no\n \twindow size is given on the command line. Defaults to 10.\n-- \n2.18.0.330.g17eb9fed90\n\n"},{"id":"354337","messageId":"CAGZ79kYPik87zvLUPPKFzcATFUdBFcXrAkTYxpxvAQy6w1CcUw@mail.gmail.com","threadId":"49003","inReplyTo":"20180802061505.2983-10-chriscool@tuxfamily.org","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-02T22:55:56Z","receivedAt":"2018-08-02T22:56:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 1, 2018 at 11:16 PM Christian Couder\n wrote:\n>\n> From: Christian Couder \n>\n> Signed-off-by: Junio C Hamano \n> ---\n> Documentation/config.txt | 5 +++++\n> 1 file changed, 5 insertions(+)\n>\n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index 43b2de7b5f..2d048d47f2 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -2513,6 +2513,11 @@ This setting can be overridden with the `GIT_NOTES_REWRITE_REF`\n> environment variable, which must be a colon separated list of refs or\n> globs.\n>\n> +odb..promisorRemote::\n> + The name of a promisor remote. For now promisor remotes are\n> + the only kind of remote object database (odb) that is\n> + supported.\n> +\n\nCan you explain the end goal for this? (I did not find it in the cover letter,\nnor do I make sense of this documentation)\n\nSo from what I understand, this series relates to partialClone, which\nhas the remote name of the \"promisor\" in extensions.partialclone.\nThat is the remote to contact for any needs w.r.t. partial clone and\nfetching on demand.\n\nThis key \"odb..promisorRemote = \" introduces\n2 new names, where do each of these two names hook in?\nname2 is a remote, such as \"origin\" from what I can tell, but\nwhich naming scheme does name1 follow here?\n\nWhat makes the odb key different, in that the partial clone\nfeature only handles objects as well?\n\n> pack.window::\n> The size of the window used by linkgit:git-pack-objects[1] when no\n> window size is given on the command line. Defaults to 10.\n> --\n> 2.18.0.330.g17eb9fed90\n>\n"},{"id":"358822","messageId":"CAP8UFD1ia1xWk9pjfTUQ3zD7=dP=8UjKzf=G0ptsz=qRH8_X+Q@mail.gmail.com","threadId":"49003","inReplyTo":"CAGZ79kYPik87zvLUPPKFzcATFUdBFcXrAkTYxpxvAQy6w1CcUw@mail.gmail.com","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-09-25T08:07:18Z","receivedAt":"2018-09-25T08:07:21Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Aug 3, 2018 at 12:55 AM, Stefan Beller wrote:\n> On Wed, Aug 1, 2018 at 11:16 PM Christian Couder\n> wrote:\n>>\n>> From: Christian Couder \n>>\n>> Signed-off-by: Junio C Hamano \n>> ---\n>> Documentation/config.txt | 5 +++++\n>> 1 file changed, 5 insertions(+)\n>>\n>> diff --git a/Documentation/config.txt b/Documentation/config.txt\n>> index 43b2de7b5f..2d048d47f2 100644\n>> --- a/Documentation/config.txt\n>> +++ b/Documentation/config.txt\n>> @@ -2513,6 +2513,11 @@ This setting can be overridden with the `GIT_NOTES_REWRITE_REF`\n>> environment variable, which must be a colon separated list of refs or\n>> globs.\n>>\n>> +odb..promisorRemote::\n>> + The name of a promisor remote. For now promisor remotes are\n>> + the only kind of remote object database (odb) that is\n>> + supported.\n>> +\n>\n> Can you explain the end goal for this? (I did not find it in the cover letter,\n> nor do I make sense of this documentation)\n\nFirst thank you for (re)opening this discussion, as I think it may\nhelp resolve the issues related to my work.\n\nIn the cover letter there is a \"Discussion\" section which is about\nthis, but I agree that it might not be very clear.\n\nThe main issue that this patch series tries to solve is that\nextensions.partialclone config option limits the partial clone and\npromisor features to only one remote. One related issue is that it\nalso prevents to have other kind of promisor/partial clone/odb\nremotes. By other kind I mean remotes that would not necessarily be\ngit repos, but that could store objects (that's where ODB, for Object\nDataBase, comes from) and could provide those objects to Git through a\nhelper (or driver) script or program.\n\nFor reference I tried to raise these issues (especially the first one)\nat least twice before extensions.partialclone was merged:\n\nhttps://public-inbox.org/git/CAP8UFD3Jt+0Lq9Yx_7x3sJD+jG+A25bAgDg7zp+dZV43+1-vow@mail.gmail.com/\nhttps://public-inbox.org/git/CAP8UFD0P7kVo2NP4Wq7OaSV4H1+sqHapuzW5AQef+enNS0S5hw@mail.gmail.com/\n\nbut it was still merged as is.\n\n(So of course now it's not surprising that my work on this patch\nseries keeps conflicting with work that is still going on promisors\nand partial clone, and unfortunately the result is that my work keeps\nbeing ejected from pu when it can reach it.)\n\n> So from what I understand, this series relates to partialClone, which\n> has the remote name of the \"promisor\" in extensions.partialclone.\n> That is the remote to contact for any needs w.r.t. partial clone and\n> fetching on demand.\n\nYes.\n\n> This key \"odb..promisorRemote = \" introduces\n> 2 new names, where do each of these two names hook in?\n> name2 is a remote, such as \"origin\" from what I can tell, but\n> which naming scheme does name1 follow here?\n\nThere is just one new name. Instead of:\n\n extensions.partialclone = \n\nthere is:\n\n odb..promisorRemote = \n\nSo it is now like:\n\n remote..url = \n\nwhich we use for remote repositories.\n\nAnd it enables us to:\n\n - have more than one promisor remote\n - specify different parameters for each promisor remote\n - make it possible later to have other kind of promisor/odb remotes\n\nIt also restores the distributed nature of Git which was kind of\nbroken for promisor remotes.\n\n> What makes the odb key different, in that the partial clone\n> feature only handles objects as well?\n\nI am not sure I understand this question. Anyway if we want more than\none promisor remote, we need to be able to specify different\nparameters for each promisor remote. For example now\ncore.partialclonefilter is used to specify some filters for the\npromisor remote, but how can we nicely specify different partial clone\nfilters if we have more than one promisor remote?\n\nWith the changes in this patch series core.partialclonefilter is\nreplaced with odb..partialclonefilter, so that\nparameters for a remote odb are properly grouped together in the\nsection where the remote odb is defined.\n\nOne alternative scheme could be for example to have:\n\n remote..promisor = (true|false)\n\nor maybe:\n\n remote..partialclone = (true|false)\n\ninstead of:\n\n extensions.partialclone = \n\nAnd then we could also have:\n\n remote..partialclonefilter = \n\nThe issue with this scheme is that it kind of overloads the\n\"remote..*\" namespace for something that can be seen as\ndifferent especially if, as I want to do it later, we are going to\nhave other kind of promisor/odb remotes.\n\nI plan to send a V5 of this patch series really soon now, where I will\ntry to explain better the end goal.\n"},{"id":"358883","messageId":"xmqqd0t1nr93.fsf@gitster-ct.c.googlers.com","threadId":"49003","inReplyTo":"CAP8UFD1ia1xWk9pjfTUQ3zD7=dP=8UjKzf=G0ptsz=qRH8_X+Q@mail.gmail.com","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-25T22:31:36Z","receivedAt":"2018-09-25T22:31:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> The main issue that this patch series tries to solve is that\n> extensions.partialclone config option limits the partial clone and\n> promisor features to only one remote. One related issue is that it\n> also prevents to have other kind of promisor/partial clone/odb\n> remotes. By other kind I mean remotes that would not necessarily be\n> git repos, but that could store objects (that's where ODB, for Object\n> DataBase, comes from) and could provide those objects to Git through a\n> helper (or driver) script or program.\n\nI do not think \"sources that are not git repositories\" is all that\ninteresting, unless they can also serve as the source for ext::\nremote helper. And if they can serve \"git fetch ext::...\", I think\nthey can be treated just like a normal Git repository by the\nbackfill code that needs to lazily populate the partial clone.\n\nAnd it would be nice to be able to say \"I took these commits from\nthat remote and that remote should be able to backfill the trees and\nthe blobs necessary to complete these commits\" for more than one\nremote would obviously be a good thing. The way we mark the\npromisor packs currently is by a mere presence of a file, but\nnothing prevents us from extending it to write the nickname of the\nconfigured remote the pack was taken from to help us answer \"who can\nfeed us the remaining objects?\", for example, so I do not think it\nis an insurmountable problem\n\nI guess JTan is the primary person who is interested/working on the\npartial clone with backfill? Have you two been collaborating well?\n\nDo you two need help from us to make that happen, and if so what do\nyou need? Stop the world and declare this and that source files are\noff limits for two weeks, or something like that?\n\n\n"},{"id":"358925","messageId":"20180926041222.GA743@sigill.intra.peff.net","threadId":"49003","inReplyTo":"xmqqd0t1nr93.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-09-26T04:12:22Z","receivedAt":"2018-09-26T04:12:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 25, 2018 at 03:31:36PM -0700, Junio C Hamano wrote:\n\n> Christian Couder writes:\n> \n> > The main issue that this patch series tries to solve is that\n> > extensions.partialclone config option limits the partial clone and\n> > promisor features to only one remote. One related issue is that it\n> > also prevents to have other kind of promisor/partial clone/odb\n> > remotes. By other kind I mean remotes that would not necessarily be\n> > git repos, but that could store objects (that's where ODB, for Object\n> > DataBase, comes from) and could provide those objects to Git through a\n> > helper (or driver) script or program.\n> \n> I do not think \"sources that are not git repositories\" is all that\n> interesting, unless they can also serve as the source for ext::\n> remote helper. And if they can serve \"git fetch ext::...\", I think\n> they can be treated just like a normal Git repository by the\n> backfill code that needs to lazily populate the partial clone.\n\nI don't know about that. Imagine I had a regular Git repo with a bunch\nof large blobs, and then I also stored those large blobs in something\nlike S3 that provides caching, geographic locality, and resumable\ntransfers.\n\nIt would be nice to be able to say:\n\n 1. Clone from the real repo, but do not transfer any blobs larger than\n 10MB.\n\n 2. When you need a blob, check the external odb that points to S3. Git\n cannot know about this automatically, but presumably you would set\n a few config variables to point to an external-odb helper script.\n\n 3. If for some reason S3 doesn't work, you can always request it from\n the original repo. That part _doesn't_ need extra config, since we\n can assume that the source of the promisor pack can feed us the\n extra objects[1].\n\nBut you don't need to ever be able to \"git fetch\" from the S3 repo.\n\nNow if you are arguing that the interface to the external-odb helper\nscript should be that it _looks_ like upload-pack, but simply advertises\nno refs and will let you fetch any object, that makes more sense to me.\nIt's not something you could \"git clone\", but you can \"git fetch\" from\nit.\n\nHowever, that may be an overly constricting interface for the helper.\nE.g., we might want to be able to issue several requests and have them\ntransfer in parallel. But I suppose we could teach that trick to\nupload-pack in the long run, as it may be applicable even to fetching\nfrom \"real\" git repos.\n\nHmm. Actually, I kind of like that direction the more I think about it.\n\n-Peff\n"},{"id":"358943","messageId":"20180926134424.GB25697@syl","threadId":"49003","inReplyTo":"20180926041222.GA743@sigill.intra.peff.net","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-09-26T13:44:24Z","receivedAt":"2018-09-26T13:44:32Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Sep 26, 2018 at 12:12:22AM -0400, Jeff King wrote:\n> On Tue, Sep 25, 2018 at 03:31:36PM -0700, Junio C Hamano wrote:\n>\n> > Christian Couder writes:\n> >\n> > > The main issue that this patch series tries to solve is that\n> > > extensions.partialclone config option limits the partial clone and\n> > > promisor features to only one remote. One related issue is that it\n> > > also prevents to have other kind of promisor/partial clone/odb\n> > > remotes. By other kind I mean remotes that would not necessarily be\n> > > git repos, but that could store objects (that's where ODB, for Object\n> > > DataBase, comes from) and could provide those objects to Git through a\n> > > helper (or driver) script or program.\n> >\n> > I do not think \"sources that are not git repositories\" is all that\n> > interesting, unless they can also serve as the source for ext::\n> > remote helper. And if they can serve \"git fetch ext::...\", I think\n> > they can be treated just like a normal Git repository by the\n> > backfill code that needs to lazily populate the partial clone.\n>\n> I don't know about that. Imagine I had a regular Git repo with a bunch\n> of large blobs, and then I also stored those large blobs in something\n> like S3 that provides caching, geographic locality, and resumable\n> transfers.\n>\n> [ ... ]\n>\n> Now if you are arguing that the interface to the external-odb helper\n> script should be that it _looks_ like upload-pack, but simply advertises\n> no refs and will let you fetch any object, that makes more sense to me.\n> It's not something you could \"git clone\", but you can \"git fetch\" from\n> it.\n>\n> However, that may be an overly constricting interface for the helper.\n> E.g., we might want to be able to issue several requests and have them\n> transfer in parallel. But I suppose we could teach that trick to\n> upload-pack in the long run, as it may be applicable even to fetching\n> from \"real\" git repos.\n>\n> Hmm. Actually, I kind of like that direction the more I think about it.\n\nYes, this is an important design decision for Git LFS, which I believe\nis important to this series. Git LFS allows the caller to issue `n`\nparallel object transfers (uploads or downloads) at a time, which is\nuseful when, say, checking out a repository that has many large objects.\n\nWe do this trick with 'filter.lfs.process', where we accumulate many Git\nLFS objects that we wish to tell Git about so that it can check them out\ninto the working copy, and then promise that we will provide the\ncontents later (e.g., by sending status=delayed).\n\nWe then \"batch\" up all of those requests, issue them all at once (after\nwhich the LFS API will tell us the URLs of where to upload/download each\nitem), and then we open \"N\" threads to do that work.\n\nAfter all of that, we respond back with all of the objects that we had\nto download, and close the process filter.\n\nThanks,\nTaylor\n"},{"id":"358965","messageId":"xmqqpnx0ku1q.fsf@gitster-ct.c.googlers.com","threadId":"49003","inReplyTo":"20180926041222.GA743@sigill.intra.peff.net","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-26T18:11:45Z","receivedAt":"2018-09-26T18:11:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> I do not think \"sources that are not git repositories\" is all that\n>> interesting, unless they can also serve as the source for ext::\n>> remote helper. And if they can serve \"git fetch ext::...\", I think\n>> they can be treated just like a normal Git repository by the\n>> backfill code that needs to lazily populate the partial clone.\n>\n> I don't know about that. Imagine I had a regular Git repo with a bunch\n> of large blobs, and then I also stored those large blobs in something\n> like S3 that provides caching, geographic locality, and resumable\n> transfers.\n>\n> It would be nice to be able to say:\n>\n> 1. Clone from the real repo, but do not transfer any blobs larger than\n> 10MB.\n>\n> 2. When you need a blob, check the external odb that points to S3. Git\n> cannot know about this automatically, but presumably you would set\n> a few config variables to point to an external-odb helper script.\n>\n> 3. If for some reason S3 doesn't work, you can always request it from\n> the original repo. That part _doesn't_ need extra config, since we\n> can assume that the source of the promisor pack can feed us the\n> extra objects[1].\n>\n> But you don't need to ever be able to \"git fetch\" from the S3 repo.\n>\n> Now if you are arguing that the interface to the external-odb helper\n> script should be that it _looks_ like upload-pack, but simply advertises\n> no refs and will let you fetch any object, that makes more sense to me.\n> It's not something you could \"git clone\", but you can \"git fetch\" from\n> it.\n\nYup. The lazy backfill JTan has, if I understand correctly, only\nwants \"Please give me this and that object\" and use of \"upload-pack\"\nis an implementation detail. Over the existing Git protocols, you\nmay implement it as sending these object names as \"want\" and perhaps\nrestrict the traversal (if there is a \"want\" object that is commit)\nby giving some commits as \"have\", i.e. \"upload-pack\" may not be the\nbest model for the other side, but that is what we have readily\navailable. I was hoping that the way we take to move forward is to\nenhance that interface so that we can use different \"object store\"\nbackends as needed, to satisfy needs from both parties.\n"},{"id":"360644","messageId":"20181016174304.GA221682@aiede.svl.corp.google.com","threadId":"49003","inReplyTo":"CAP8UFD1ia1xWk9pjfTUQ3zD7=dP=8UjKzf=G0ptsz=qRH8_X+Q@mail.gmail.com","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-10-16T17:43:04Z","receivedAt":"2018-10-16T17:43:10Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Christian,\n\nOn Tue, Sep 25, 2018, Christian Couder wrote:\n\n> In the cover letter there is a \"Discussion\" section which is about\n> this, but I agree that it might not be very clear.\n>\n> The main issue that this patch series tries to solve is that\n> extensions.partialclone config option limits the partial clone and\n> promisor features to only one remote. One related issue is that it\n> also prevents to have other kind of promisor/partial clone/odb\n> remotes. By other kind I mean remotes that would not necessarily be\n> git repos, but that could store objects (that's where ODB, for Object\n> DataBase, comes from) and could provide those objects to Git through a\n> helper (or driver) script or program.\n\nThanks for this explanation. I took the opportunity to learn more\nwhile you were in the bay area for the google summer of code mentor\nsummit and learned a little more, which was very helpful to me.\n\nThe broader picture is that this is meant to make Git natively handle\nlarge blobs in a nicer way. The design in this series has a few\ncomponents:\n\n 1. Teaching partial clone to attempt to fetch missing objects from\n multiple remotes instead of only one. This is useful because you\n can have a server that is nearby and cheaper to serve from (some\n kind of local cache server) that you make requests to first before\n falling back to the canonical source of objects.\n\n 2. Simplifying the protocol for fetching missing objects so that it\n can be satisfied by a lighter weight object storage system than\n a full Git server. The ODB helpers introduced in this series are\n meant to speak such a simpler protocol since they are only used\n for one-off requests of a collection of missing objects instead of\n needing to understand refs, Git's negotiation, etc.\n\n 3. (possibly, though not in this series) Making the criteria for what\n objects can be missing more aggressive, so that I can \"git add\"\n a large file and work with it using Git without even having a\n second copy of that object in my local object store.\n\nFor (2), I would like to see us improve the remote helper\ninfrastructure instead of introducing a new ODB helper. Remote\nhelpers are already permitted to fetch some objects without listing\nrefs --- perhaps we will want to\n\n i. split listing refs to a separate capability, so that a remote\n helper can advertise that it doesn't support that. (Alternatively\n the remote could advertise that it has no refs.)\n\n ii. Use the \"long-running process\" mechanism to improve how Git\n communicates with a remote helper.\n\nFor (1), things get more tricky. In an object store from a partial\nclone today, we relax the ordinary \"closure under reachability\"\ninvariant but in a minor way. We'll need to work out how this works\nwith multiple promisor remotes.\n\nThe idea today is that there are two kinds of packs: promisor packs\n(from the promisor remote) and non-promisor packs. Promisor packs are\nallowed to have reachability edges (for example a tree->blob edge)\nthat point to a missing object, since the promisor remote has promised\nthat we will be able to access that object on demand. Non-promisor\npacks are also allowed to have reachability edges that point to a\nmissing object, as long as there is a reachability edge from an object\nin a promisor pack to the same object (because of the same promise).\nSee \"Handling Missing Objects\" in Documentation/technical/partial-clone.txt\nfor more details.\n\nTo prevent older versions of Git from being confused by partial clone\nrepositories, they use the repositoryFormatVersion mechanism:\n\n\t[core]\n\t\trepositoryFormatVersion = 1\n\t[extensions]\n\t\tpartialClone = ...\n\nIf we change the invariant, we will need to use a new extensions.* key\nto ensure that versions of Git that are not aware of the new invariant\ndo not operate on the repository.\n\nA promisor pack is indicated by there being a .promisor file next to\nthe usual .pack file. Currently the .promisor file is empty. The\nprevious idea was that once we want more metadata (e.g. for the sake of\nmultiple promisor remotes), we could write it in that file. For\nexample, remotes could be associated to a and the\n.promisor file could indicate which has promised to serve\nrequests for objects reachable from objects in this pack.\n\nThat will complicate the object access code as well, since currently\nwe only find who has promised an object during \"git fsck\" and similar\noperations. During everyday access we do not care which promisor\npack caused the object to be promised, since there is only one promisor\nremote to fetch from anyway.\n\nSo much for the current setup. For (1), I believe you are proposing to\nstill have only one effective , so it doesn't necessarily\nrequire modifying the extensions.* configuration. Instead, the idea is\nthat when trying to access an object, we would follow one of a list of\nsteps:\n\n 1. First, check the local object store. If it's there, we're done.\n 2. Second, try alternates --- maybe the object is in one of those!\n 3. Now, try promisor remotes, one at a time, in user-configured order.\n\nIn other words, I think that for (1) all we would need is a new\nconfiguration\n\n\t[object]\n\t\tmissingObjectRemote = local-cache-remote\n\t\tmissingObjectRemote = origin\n\nThe semantics would be that when trying to access a promised object,\nwe attempt to fetch from these remotes one at a time, in the order\nspecified. We could require that the remote named in\nextensions.partialClone be one of the listed remotes, without having\nto care where it shows up in the list.\n\nThat way, we get the benefit (1) without having to change the\nsemantics of extensions.partialClone and without having to care about\nthe order of sections in the config. What do you think?\n\nThanks,\nJonathan\n"},{"id":"360677","messageId":"20181016222243.58620-1-jonathantanmy@google.com","threadId":"49003","inReplyTo":"20181016174304.GA221682@aiede.svl.corp.google.com","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-10-16T22:22:43Z","receivedAt":"2018-10-16T22:22:49Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"> 1. Teaching partial clone to attempt to fetch missing objects from\n> multiple remotes instead of only one. This is useful because you\n> can have a server that is nearby and cheaper to serve from (some\n> kind of local cache server) that you make requests to first before\n> falling back to the canonical source of objects.\n\nQuoting the above definition of (1) for reference. I think Jonathan\nNieder has covered the relevant points well - I'll just expand on (1).\n\n> So much for the current setup. For (1), I believe you are proposing to\n> still have only one effective , so it doesn't necessarily\n> require modifying the extensions.* configuration. Instead, the idea is\n> that when trying to access an object, we would follow one of a list of\n> steps:\n> \n> 1. First, check the local object store. If it's there, we're done.\n> 2. Second, try alternates --- maybe the object is in one of those!\n> 3. Now, try promisor remotes, one at a time, in user-configured order.\n> \n> In other words, I think that for (1) all we would need is a new\n> configuration\n> \n> \t[object]\n> \t\tmissingObjectRemote = local-cache-remote\n> \t\tmissingObjectRemote = origin\n> \n> The semantics would be that when trying to access a promised object,\n> we attempt to fetch from these remotes one at a time, in the order\n> specified. We could require that the remote named in\n> extensions.partialClone be one of the listed remotes, without having\n> to care where it shows up in the list.\n\nOr allow extensions.partialClone= wherein is not in the\nmissingObjectRemote, in which case is tried first, so that we don't\nhave to reject some configurations.\n\n> That way, we get the benefit (1) without having to change the\n> semantics of extensions.partialClone and without having to care about\n> the order of sections in the config. What do you think?\n\nLet's define the promisor remotes of a repository as those in\nmissingObjectRemote or extensions.partialClone (currently, we talk about\n\"the promisor remote\" (singular), defined in extensions.partialClone).\n\nOverall, this seems like a reasonable idea to me, if we keep the\nrestriction that we can only fetch with filter from a promisor remote.\nThis allows us to extend the definition of a promisor object in a\nmanner consistent to the current definition - to say \"a promisor object\nis one promised by at least one promisor remote\" (currently, \"a promisor\nobject is promised by the promisor remote\").\n\nIn the presence of missingObjectRemote, old versions of Git, when lazily\nfetching, would only know to try the extensions.partialClone remote. But\nthis is safe because existing data wouldn't be clobbered (since we're\nnot using ideas like adding meaning to the contents of the .promisor\nfile). Also, other things like fsck and gc still work.\n"},{"id":"360884","messageId":"xmqqwoqe4xev.fsf@gitster-ct.c.googlers.com","threadId":"49003","inReplyTo":"20181016222243.58620-1-jonathantanmy@google.com","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-19T00:01:28Z","receivedAt":"2018-10-19T00:01:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n>> \t[object]\n>> \t\tmissingObjectRemote = local-cache-remote\n>> \t\tmissingObjectRemote = origin\n>> \n> In the presence of missingObjectRemote, old versions of Git, when lazily\n> fetching, would only know to try the extensions.partialClone remote. But\n> this is safe because existing data wouldn't be clobbered (since we're\n> not using ideas like adding meaning to the contents of the .promisor\n> file). Also, other things like fsck and gc still work.\n\nIt is a good idea to implicitly include the promisor-remote to the\nset of secondary places to consult to help existing versions of Git,\nbut once the repository starts fetching incomplete subgraphs and\nadding new object.missingobjectremote [*1*], these versions of Git\nwill stop working correctly, so I am not sure if it is all that\nuseful approach for compatibility in practice.\n\n\n[Footnote]\n\n*1* That name with two \"object\" in it sounds horrible. I think the\nsame keyname in 'core' section may sit better (this feature sounds\nmore 'core' than other cruft that crept into 'core' section over\ntime). \n\nOr \"odb.remoteAlternate\" (as opposed to object/info/alternates that\nare local alternates), perhaps.\n"},{"id":"360886","messageId":"20181019003325.GA30421@aiede.svl.corp.google.com","threadId":"49003","inReplyTo":"xmqqwoqe4xev.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2018-10-19T00:33:25Z","receivedAt":"2018-10-19T00:33:38Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Tan writes:\n>> Jonathan Nieder wrote:\n\n>>> \t[object]\n>>> \t\tmissingObjectRemote = local-cache-remote\n>>> \t\tmissingObjectRemote = origin\n>>\n>> In the presence of missingObjectRemote, old versions of Git, when lazily\n>> fetching, would only know to try the extensions.partialClone remote. But\n>> this is safe because existing data wouldn't be clobbered (since we're\n>> not using ideas like adding meaning to the contents of the .promisor\n>> file). Also, other things like fsck and gc still work.\n>\n> It is a good idea to implicitly include the promisor-remote to the\n> set of secondary places to consult to help existing versions of Git,\n> but once the repository starts fetching incomplete subgraphs and\n> adding new object.missingobjectremote [*1*], these versions of Git\n> will stop working correctly, so I am not sure if it is all that\n> useful approach for compatibility in practice.\n\nCan you spell this out for me more? Do you mean that a remote from\nthis list might make a promise that the original partialClone remote\ncan't keep?\n\nIf we're careful to only page in objects that were promised by the\noriginal partialClone remote, then a promise \"I promise to supply you\non demand with any object directly or indirectly reachable from any\nobject you have fetched from me\" from the partialClone remote should\nbe enough.\n\n> [Footnote]\n>\n> *1* That name with two \"object\" in it sounds horrible.\n\nSorry about that. Another name used while discussing this was\nobjectAccess.promisorRemote. As long as the idea is clear (that this\nmeans \"remotes to use when attempting to obtain an object that is not\nalready available locally\"), I am not attached to any particular name.\n\nThanks,\nJonathan\n"},{"id":"360895","messageId":"xmqqftx23asn.fsf@gitster-ct.c.googlers.com","threadId":"49003","inReplyTo":"20181019003325.GA30421@aiede.svl.corp.google.com","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-10-19T02:55:20Z","receivedAt":"2018-10-19T02:55:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Junio C Hamano wrote:\n> ...\n>> It is a good idea to implicitly include the promisor-remote to the\n>> set of secondary places to consult to help existing versions of Git,\n>> but once the repository starts fetching incomplete subgraphs and\n>> adding new object.missingobjectremote [*1*], these versions of Git\n>> will stop working correctly, so I am not sure if it is all that\n>> useful approach for compatibility in practice.\n>\n> Can you spell this out for me more? Do you mean that a remote from\n> this list might make a promise that the original partialClone remote\n> can't keep?\n\nIt was my failed attempt to demonstrate that I understood what was\nbeing discussed by rephrasing JTan's\n\n Or allow extensions.partialClone= wherein is not in the\n missingObjectRemote, in which case is tried first, so that\n we don't have to reject some configurations.\n\n"},{"id":"362078","messageId":"CAP8UFD3nrhjANwNDqTwx5ZtnZNcnbAFqUN=u=LrvzuH4+3wQQA@mail.gmail.com","threadId":"49003","inReplyTo":"20181016174304.GA221682@aiede.svl.corp.google.com","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-10-31T06:28:09Z","receivedAt":"2018-10-31T06:28:24Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi Jonathan,\n\nOn Tue, Oct 16, 2018 at 7:43 PM Jonathan Nieder wrote:\n>\n> Hi Christian,\n>\n> On Tue, Sep 25, 2018, Christian Couder wrote:\n>\n> > In the cover letter there is a \"Discussion\" section which is about\n> > this, but I agree that it might not be very clear.\n> >\n> > The main issue that this patch series tries to solve is that\n> > extensions.partialclone config option limits the partial clone and\n> > promisor features to only one remote. One related issue is that it\n> > also prevents to have other kind of promisor/partial clone/odb\n> > remotes. By other kind I mean remotes that would not necessarily be\n> > git repos, but that could store objects (that's where ODB, for Object\n> > DataBase, comes from) and could provide those objects to Git through a\n> > helper (or driver) script or program.\n>\n> Thanks for this explanation. I took the opportunity to learn more\n> while you were in the bay area for the google summer of code mentor\n> summit and learned a little more, which was very helpful to me.\n\nThanks for inviting me at the Google offices in Sunnyvale and San\nFrancisco to discuss about this and other issues.\n\n> The broader picture is that this is meant to make Git natively handle\n> large blobs in a nicer way. The design in this series has a few\n> components:\n>\n> 1. Teaching partial clone to attempt to fetch missing objects from\n> multiple remotes instead of only one. This is useful because you\n> can have a server that is nearby and cheaper to serve from (some\n> kind of local cache server) that you make requests to first before\n> falling back to the canonical source of objects.\n>\n> 2. Simplifying the protocol for fetching missing objects so that it\n> can be satisfied by a lighter weight object storage system than\n> a full Git server. The ODB helpers introduced in this series are\n> meant to speak such a simpler protocol since they are only used\n> for one-off requests of a collection of missing objects instead of\n> needing to understand refs, Git's negotiation, etc.\n>\n> 3. (possibly, though not in this series) Making the criteria for what\n> objects can be missing more aggressive, so that I can \"git add\"\n> a large file and work with it using Git without even having a\n> second copy of that object in my local object store.\n\nYeah, I think this is a good summary of the issues I have been trying\nto address.\n\n> For (2), I would like to see us improve the remote helper\n> infrastructure instead of introducing a new ODB helper. Remote\n> helpers are already permitted to fetch some objects without listing\n> refs --- perhaps we will want to\n>\n> i. split listing refs to a separate capability, so that a remote\n> helper can advertise that it doesn't support that. (Alternatively\n> the remote could advertise that it has no refs.)\n>\n> ii. Use the \"long-running process\" mechanism to improve how Git\n> communicates with a remote helper.\n\nYeah, I agree that improving the remote helper infrastructure is\nprobably better than what I have been trying to add. And I agree with\nthe above 2 steps you propose.\n\n> For (1), things get more tricky. In an object store from a partial\n> clone today, we relax the ordinary \"closure under reachability\"\n> invariant but in a minor way. We'll need to work out how this works\n> with multiple promisor remotes.\n>\n> The idea today is that there are two kinds of packs: promisor packs\n> (from the promisor remote) and non-promisor packs. Promisor packs are\n> allowed to have reachability edges (for example a tree->blob edge)\n> that point to a missing object, since the promisor remote has promised\n> that we will be able to access that object on demand. Non-promisor\n> packs are also allowed to have reachability edges that point to a\n> missing object, as long as there is a reachability edge from an object\n> in a promisor pack to the same object (because of the same promise).\n> See \"Handling Missing Objects\" in Documentation/technical/partial-clone.txt\n> for more details.\n>\n> To prevent older versions of Git from being confused by partial clone\n> repositories, they use the repositoryFormatVersion mechanism:\n>\n> [core]\n> repositoryFormatVersion = 1\n> [extensions]\n> partialClone = ...\n>\n> If we change the invariant, we will need to use a new extensions.* key\n> to ensure that versions of Git that are not aware of the new invariant\n> do not operate on the repository.\n\nMaybe the versions of Git that are not aware of the new invariant\ncould still work using only the specified remote while the new\nversions would know that they can use other remotes by looking at\nother config variables.\n\n> A promisor pack is indicated by there being a .promisor file next to\n> the usual .pack file. Currently the .promisor file is empty. The\n> previous idea was that once we want more metadata (e.g. for the sake of\n> multiple promisor remotes), we could write it in that file. For\n> example, remotes could be associated to a and the\n> .promisor file could indicate which has promised to serve\n> requests for objects reachable from objects in this pack.\n\nI think it would be nicer if we could avoid any modification of the\nfile format. Also I don't think it is important for the user to be\nable to blame any specific remote for not being able to serve a given\nobject, because sometimes it could be better for a remote that would\nbe too slow to provide a large blob to just reject any request for\nthis blob and to instead expect users to get it from another faster\nremote.\n\nI think that when Git cannot get a promised object it should just tell\nthat it could not get it from the configured promisor remotes and list\nthe promisor remotes (along maybe with the associated URLs).\n\n> That will complicate the object access code as well, since currently\n> we only find who has promised an object during \"git fsck\" and similar\n> operations. During everyday access we do not care which promisor\n> pack caused the object to be promised, since there is only one promisor\n> remote to fetch from anyway.\n\nI agree.\n\n> So much for the current setup. For (1), I believe you are proposing to\n> still have only one effective , so it doesn't necessarily\n> require modifying the extensions.* configuration.\n\nYes, it would be better if we don't need to modify the \"extensions.*\"\nconfiguration. So the remote given by the \"extensions.partialClone\"\nconfig option would only be a remote that can provide all the objects\nto previous versions of Git (those that don't work with more than one\npromisor remote).\n\n> Instead, the idea is\n> that when trying to access an object, we would follow one of a list of\n> steps:\n>\n> 1. First, check the local object store. If it's there, we're done.\n> 2. Second, try alternates --- maybe the object is in one of those!\n> 3. Now, try promisor remotes, one at a time, in user-configured order.\n\nYeah, 1. and 2. are already working, so we just need to implement 3..\n\n> In other words, I think that for (1) all we would need is a new\n> configuration\n>\n> [object]\n> missingObjectRemote = local-cache-remote\n> missingObjectRemote = origin\n\nYeah, we could add some configuration to define an order. I don't\nthink it's really important though to be able to configure an order\nright now. I think it's more important to first just have a mechanism\nto iterate over the promisor remotes. By default if no order is\nconfigured, the order we use to iterate can be the order of the\npromisor remotes in the config file(s).\n\n> The semantics would be that when trying to access a promised object,\n> we attempt to fetch from these remotes one at a time, in the order\n> specified. We could require that the remote named in\n> extensions.partialClone be one of the listed remotes, without having\n> to care where it shows up in the list.\n\nYeah, when we implement this in the config, it would make sense to\nhave such kind of semantics. If some promisor remotes are not\nconfigured in the order, we could also decide that these remotes\nshould be tried after those that are configured, or maybe we should\ndecide that they should not be tried except if the remote is specified\nby \"extensions.partialClone\".\n\n> That way, we get the benefit (1) without having to change the\n> semantics of extensions.partialClone and without having to care about\n> the order of sections in the config. What do you think?\n\nI mostly agree with you, though I would like to work on things in the\nfollowing order:\n\n 1) Teaching partial clone to attempt to fetch missing objects from\nmultiple remotes instead of only one using the order in the config.\n\n 2) Simplifying the protocol for fetching missing objects so that it\ncan be satisfied by a lighter weight object storage system than a full\nGit server.\n\n 3) Making it possible to explicitly define an order in which the\nremotes are accessed.\n\n 4) Making the criteria for what objects can be missing more\naggressive, so that I can \"git add\" a large file and work with it\nusing Git without even having a second copy of that object in my local\nobject store.\n\nOr maybe even switching the above 3) and 4).\n\nI think we should also decide how we specify different parameters for\neach promisor remote. There is already \"core.partialclonefilter\", but\nif we have more than one promisor remote, we might want to have\ndifferent partial clone filters for different remotes.\n\nI am ok with using the \"remote..*\" namespace, for example\n\"remote..partialclonefilter\". \"core.partialclonefilter\"\nwould then be the default partial clone filter for the remote\nspecified by \"extensions.partialClone\".\n\nAfter saying all the above though, I still wonder if keeping\n\"extensions.partialClone\" and \"core.partialclonefilter\" is not already\na burden that will make us implement unnecessarily complex semantics\nand will become even more a burden over time.\n\nFor example what if we later want to say something like:\n\n\"When fetching from this remote, I want to use these promisor remotes\nin this order, though, when pushing to that (maybe different) remote,\nI want to\nuse those (maybe a bit different) promisor remotes in that order.\"\n\nwhich I think should become quite reasonable to ask for.\n\nSo I think it might be interesting to look at how we would ideally\nwant to specify such kind of configuration, as it might lead us to\nfollow a different, though perhaps saner over time, direction.\n\nThanks,\nChristian.\n"},{"id":"362212","messageId":"20181101211616.GA3280@sigill.intra.peff.net","threadId":"49003","inReplyTo":"CAP8UFD3nrhjANwNDqTwx5ZtnZNcnbAFqUN=u=LrvzuH4+3wQQA@mail.gmail.com","subject":"Re: [PATCH v4 9/9] Documentation/config: add odb..promisorRemote","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-01T21:16:16Z","receivedAt":"2018-11-01T21:16:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 31, 2018 at 07:28:09AM +0100, Christian Couder wrote:\n\n> > For (2), I would like to see us improve the remote helper\n> > infrastructure instead of introducing a new ODB helper. Remote\n> > helpers are already permitted to fetch some objects without listing\n> > refs --- perhaps we will want to\n> >\n> > i. split listing refs to a separate capability, so that a remote\n> > helper can advertise that it doesn't support that. (Alternatively\n> > the remote could advertise that it has no refs.)\n> >\n> > ii. Use the \"long-running process\" mechanism to improve how Git\n> > communicates with a remote helper.\n> \n> Yeah, I agree that improving the remote helper infrastructure is\n> probably better than what I have been trying to add. And I agree with\n> the above 2 steps you propose.\n\nOne thing you might want to port over is the ability to ask the remote\nhelper \"tell me the type and size of these objects\". The reason I built\nthat into the original external-odb interface proposal was so that diff\ncould easily skip large objects without faulting them in (because\nthey're considered binary, and we'd just say \"binary files differ\"\nanyway). That makes things like \"git log -p\" work a lot better (try it\nwith a blob-less partial clone now; it's pretty painful).\n\nI know that's kind of the _opposite_ of how partial clones work now,\nwhere we try really hard not to have to even tell the client the full\nlist of objects. That's good if the reason you want a partial clone is\nbecause there are gigantic numbers of objects (e.g., the Windows repo).\nBut I think many people are interested in having a more moderate number\nof large objects (e.g., things like game development that are using\ngit-lfs now). It would be great if we could support both use cases\neasily.\n\n-Peff\n"}]}