{"thread":{"id":"47224","subject":"[PATCH v4 04/10] fsck: support referenced promisor objects","startedAt":"2017-11-16T18:13:28Z","lastAt":"2017-11-20T15:56:17Z","messageCount":17,"participants":["Jeff Hostetler","Ramsay Jones","Jonathan Tan","Stefan Beller"],"isPatch":true,"patchVersion":4,"patchTotal":10},"messages":[{"id":"332690","messageId":"20171116181257.61673-5-git@jeffhostetler.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"[PATCH v4 04/10] fsck: support referenced promisor objects","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:51Z","receivedAt":"2017-11-16T18:13:28Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jonathan Tan <jonathantanmy@google.com>\n\nTeach fsck to not treat missing promisor objects indirectly pointed to\nby refs as an error when extensions.partialclone is set.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n builtin/fsck.c           | 11 +++++++++++\n t/t0410-partial-clone.sh | 23 +++++++++++++++++++++++\n 2 files changed, 34 insertions(+)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex ee937bb..4c2a56d 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -149,6 +149,15 @@ static int mark_object(struct object *obj, int type, void *data, struct fsck_opt\n \tif (obj->flags & REACHABLE)\n \t\treturn 0;\n \tobj->flags |= REACHABLE;\n+\n+\tif (is_promisor_object(&obj->oid))\n+\t\t/*\n+\t\t * Further recursion does not need to be performed on this\n+\t\t * object since it is a promisor object (so it does not need to\n+\t\t * be added to \"pending\").\n+\t\t */\n+\t\treturn 0;\n+\n \tif (!(obj->flags & HAS_OBJ)) {\n \t\tif (parent && !has_object_file(&obj->oid)) {\n \t\t\tprintf(\"broken link from %7s %s\\n\",\n@@ -208,6 +217,8 @@ static void check_reachable_object(struct object *obj)\n \t * do a full fsck\n \t */\n \tif (!(obj->flags & HAS_OBJ)) {\n+\t\tif (is_promisor_object(&obj->oid))\n+\t\t\treturn;\n \t\tif (has_sha1_pack(obj->oid.hash))\n \t\t\treturn; /* it is in pack - forget about it */\n \t\tprintf(\"missing %s %s\\n\", printable_type(obj),\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex bf75162..4f9931f 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -102,4 +102,27 @@ test_expect_success 'missing ref object, but promised, passes fsck' '\n \tgit -C repo fsck\n '\n \n+test_expect_success 'missing object, but promised, passes fsck' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo 1 &&\n+\ttest_commit -C repo 2 &&\n+\ttest_commit -C repo 3 &&\n+\tgit -C repo tag -a annotated_tag -m \"annotated tag\" &&\n+\n+\tC=$(git -C repo rev-parse 1) &&\n+\tT=$(git -C repo rev-parse 2^{tree}) &&\n+\tB=$(git hash-object repo/3.t) &&\n+\tAT=$(git -C repo rev-parse annotated_tag) &&\n+\n+\tpromise_and_delete \"$C\" &&\n+\tpromise_and_delete \"$T\" &&\n+\tpromise_and_delete \"$B\" &&\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 fsck\n+'\n+\n test_done\n-- \n2.9.3\n\n"},{"id":"332691","messageId":"20171116181257.61673-6-git@jeffhostetler.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"[PATCH v4 05/10] fsck: support promisor objects as CLI argument","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:52Z","receivedAt":"2017-11-16T18:13:31Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jonathan Tan <jonathantanmy@google.com>\n\nTeach fsck to not treat missing promisor objects provided on the CLI as\nan error when extensions.partialclone is set.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n builtin/fsck.c           |  2 ++\n t/t0410-partial-clone.sh | 13 +++++++++++++\n 2 files changed, 15 insertions(+)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 4c2a56d..578a7c8 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -750,6 +750,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \t\t\tstruct object *obj = lookup_object(oid.hash);\n \n \t\t\tif (!obj || !(obj->flags & HAS_OBJ)) {\n+\t\t\t\tif (is_promisor_object(&oid))\n+\t\t\t\t\tcontinue;\n \t\t\t\terror(\"%s: object missing\", oid_to_hex(&oid));\n \t\t\t\terrors_found |= ERROR_OBJECT;\n \t\t\t\tcontinue;\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex 4f9931f..e96f436 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -125,4 +125,17 @@ test_expect_success 'missing object, but promised, passes fsck' '\n \tgit -C repo fsck\n '\n \n+test_expect_success 'missing CLI object, but promised, passes fsck' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo my_commit &&\n+\n+\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\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 fsck \"$A\"\n+'\n+\n test_done\n-- \n2.9.3\n\n"},{"id":"332692","messageId":"20171116181257.61673-4-git@jeffhostetler.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"[PATCH v4 03/10] fsck: support refs pointing to promisor objects","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:50Z","receivedAt":"2017-11-16T18:13:33Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jonathan Tan <jonathantanmy@google.com>\n\nTeach fsck to not treat refs referring to missing promisor objects as an\nerror when extensions.partialclone is set.\n\nFor the purposes of warning about no default refs, such refs are still\ntreated as legitimate refs.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n builtin/fsck.c           |  8 ++++++++\n t/t0410-partial-clone.sh | 24 ++++++++++++++++++++++++\n 2 files changed, 32 insertions(+)\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 2934299..ee937bb 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -434,6 +434,14 @@ static int fsck_handle_ref(const char *refname, const struct object_id *oid,\n \n \tobj = parse_object(oid);\n \tif (!obj) {\n+\t\tif (is_promisor_object(oid)) {\n+\t\t\t/*\n+\t\t\t * Increment default_refs anyway, because this is a\n+\t\t\t * valid ref.\n+\t\t\t */\n+\t\t\t default_refs++;\n+\t\t\t return 0;\n+\t\t}\n \t\terror(\"%s: invalid sha1 pointer %s\", refname, oid_to_hex(oid));\n \t\terrors_found |= ERROR_REACHABLE;\n \t\t/* We'll continue with the rest despite the error.. */\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex 3ddb3b9..bf75162 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -13,6 +13,14 @@ pack_as_from_promisor () {\n \t>repo/.git/objects/pack/pack-$HASH.promisor\n }\n \n+promise_and_delete () {\n+\tHASH=$(git -C repo rev-parse \"$1\") &&\n+\tgit -C repo tag -a -m message my_annotated_tag \"$HASH\" &&\n+\tgit -C repo rev-parse my_annotated_tag | pack_as_from_promisor &&\n+\tgit -C repo tag -d my_annotated_tag &&\n+\tdelete_object repo \"$HASH\"\n+}\n+\n test_expect_success 'missing reflog object, but promised by a commit, passes fsck' '\n \ttest_create_repo repo &&\n \ttest_commit -C repo my_commit &&\n@@ -78,4 +86,20 @@ test_expect_success 'missing reflog object alone fails fsck, even with extension\n \ttest_must_fail git -C repo fsck\n '\n \n+test_expect_success 'missing ref object, but promised, passes fsck' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo my_commit &&\n+\n+\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n+\n+\t# Reference $A only from ref\n+\tgit -C repo branch my_branch \"$A\" &&\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 fsck\n+'\n+\n test_done\n-- \n2.9.3\n\n"},{"id":"332693","messageId":"20171116181257.61673-7-git@jeffhostetler.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"[PATCH v4 06/10] index-pack: refactor writing of .keep files","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:53Z","receivedAt":"2017-11-16T18:13:36Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jonathan Tan <jonathantanmy@google.com>\n\nIn a subsequent commit, index-pack will be taught to write \".promisor\"\nfiles which are similar to the \".keep\" files it knows how to write.\nRefactor the writing of \".keep\" files, so that the implementation of\nwriting \".promisor\" files becomes easier.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n builtin/index-pack.c | 99 ++++++++++++++++++++++++++++------------------------\n 1 file changed, 53 insertions(+), 46 deletions(-)\n\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 8ec459f..4f305a7 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1389,15 +1389,58 @@ static void fix_unresolved_deltas(struct sha1file *f)\n \tfree(sorted_by_pos);\n }\n \n+static const char *derive_filename(const char *pack_name, const char *suffix,\n+\t\t\t\t   struct strbuf *buf)\n+{\n+\tsize_t len;\n+\tif (!strip_suffix(pack_name, \".pack\", &len))\n+\t\tdie(_(\"packfile name '%s' does not end with '.pack'\"),\n+\t\t    pack_name);\n+\tstrbuf_add(buf, pack_name, len);\n+\tstrbuf_addch(buf, '.');\n+\tstrbuf_addstr(buf, suffix);\n+\treturn buf->buf;\n+}\n+\n+static void write_special_file(const char *suffix, const char *msg,\n+\t\t\t       const char *pack_name, const unsigned char *sha1,\n+\t\t\t       const char **report)\n+{\n+\tstruct strbuf name_buf = STRBUF_INIT;\n+\tconst char *filename;\n+\tint fd;\n+\tint msg_len = strlen(msg);\n+\n+\tif (pack_name)\n+\t\tfilename = derive_filename(pack_name, suffix, &name_buf);\n+\telse\n+\t\tfilename = odb_pack_name(&name_buf, sha1, suffix);\n+\n+\tfd = odb_pack_keep(filename);\n+\tif (fd < 0) {\n+\t\tif (errno != EEXIST)\n+\t\t\tdie_errno(_(\"cannot write %s file '%s'\"),\n+\t\t\t\t  suffix, filename);\n+\t} else {\n+\t\tif (msg_len > 0) {\n+\t\t\twrite_or_die(fd, msg, msg_len);\n+\t\t\twrite_or_die(fd, \"\\n\", 1);\n+\t\t}\n+\t\tif (close(fd) != 0)\n+\t\t\tdie_errno(_(\"cannot close written %s file '%s'\"),\n+\t\t\t\t  suffix, filename);\n+\t\t*report = suffix;\n+\t}\n+\tstrbuf_release(&name_buf);\n+}\n+\n static void final(const char *final_pack_name, const char *curr_pack_name,\n \t\t  const char *final_index_name, const char *curr_index_name,\n-\t\t  const char *keep_name, const char *keep_msg,\n-\t\t  unsigned char *sha1)\n+\t\t  const char *keep_msg, unsigned char *sha1)\n {\n \tconst char *report = \"pack\";\n \tstruct strbuf pack_name = STRBUF_INIT;\n \tstruct strbuf index_name = STRBUF_INIT;\n-\tstruct strbuf keep_name_buf = STRBUF_INIT;\n \tint err;\n \n \tif (!from_stdin) {\n@@ -1409,28 +1452,9 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n \t\t\tdie_errno(_(\"error while closing pack file\"));\n \t}\n \n-\tif (keep_msg) {\n-\t\tint keep_fd, keep_msg_len = strlen(keep_msg);\n-\n-\t\tif (!keep_name)\n-\t\t\tkeep_name = odb_pack_name(&keep_name_buf, sha1, \"keep\");\n-\n-\t\tkeep_fd = odb_pack_keep(keep_name);\n-\t\tif (keep_fd < 0) {\n-\t\t\tif (errno != EEXIST)\n-\t\t\t\tdie_errno(_(\"cannot write keep file '%s'\"),\n-\t\t\t\t\t  keep_name);\n-\t\t} else {\n-\t\t\tif (keep_msg_len > 0) {\n-\t\t\t\twrite_or_die(keep_fd, keep_msg, keep_msg_len);\n-\t\t\t\twrite_or_die(keep_fd, \"\\n\", 1);\n-\t\t\t}\n-\t\t\tif (close(keep_fd) != 0)\n-\t\t\t\tdie_errno(_(\"cannot close written keep file '%s'\"),\n-\t\t\t\t\t  keep_name);\n-\t\t\treport = \"keep\";\n-\t\t}\n-\t}\n+\tif (keep_msg)\n+\t\twrite_special_file(\"keep\", keep_msg, final_pack_name, sha1,\n+\t\t\t\t   &report);\n \n \tif (final_pack_name != curr_pack_name) {\n \t\tif (!final_pack_name)\n@@ -1472,7 +1496,6 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n \n \tstrbuf_release(&index_name);\n \tstrbuf_release(&pack_name);\n-\tstrbuf_release(&keep_name_buf);\n }\n \n static int git_index_pack_config(const char *k, const char *v, void *cb)\n@@ -1615,26 +1638,13 @@ static void show_pack_info(int stat_only)\n \t}\n }\n \n-static const char *derive_filename(const char *pack_name, const char *suffix,\n-\t\t\t\t   struct strbuf *buf)\n-{\n-\tsize_t len;\n-\tif (!strip_suffix(pack_name, \".pack\", &len))\n-\t\tdie(_(\"packfile name '%s' does not end with '.pack'\"),\n-\t\t    pack_name);\n-\tstrbuf_add(buf, pack_name, len);\n-\tstrbuf_addstr(buf, suffix);\n-\treturn buf->buf;\n-}\n-\n int cmd_index_pack(int argc, const char **argv, const char *prefix)\n {\n \tint i, fix_thin_pack = 0, verify = 0, stat_only = 0;\n \tconst char *curr_index;\n \tconst char *index_name = NULL, *pack_name = NULL;\n-\tconst char *keep_name = NULL, *keep_msg = NULL;\n-\tstruct strbuf index_name_buf = STRBUF_INIT,\n-\t\t      keep_name_buf = STRBUF_INIT;\n+\tconst char *keep_msg = NULL;\n+\tstruct strbuf index_name_buf = STRBUF_INIT;\n \tstruct pack_idx_entry **idx_objects;\n \tstruct pack_idx_option opts;\n \tunsigned char pack_sha1[20];\n@@ -1745,9 +1755,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (from_stdin && !startup_info->have_repository)\n \t\tdie(_(\"--stdin requires a git repository\"));\n \tif (!index_name && pack_name)\n-\t\tindex_name = derive_filename(pack_name, \".idx\", &index_name_buf);\n-\tif (keep_msg && !keep_name && pack_name)\n-\t\tkeep_name = derive_filename(pack_name, \".keep\", &keep_name_buf);\n+\t\tindex_name = derive_filename(pack_name, \"idx\", &index_name_buf);\n \n \tif (verify) {\n \t\tif (!index_name)\n@@ -1795,13 +1803,12 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (!verify)\n \t\tfinal(pack_name, curr_pack,\n \t\t      index_name, curr_index,\n-\t\t      keep_name, keep_msg,\n+\t\t      keep_msg,\n \t\t      pack_sha1);\n \telse\n \t\tclose(input_fd);\n \tfree(objects);\n \tstrbuf_release(&index_name_buf);\n-\tstrbuf_release(&keep_name_buf);\n \tif (pack_name == NULL)\n \t\tfree((void *) curr_pack);\n \tif (index_name == NULL)\n-- \n2.9.3\n\n"},{"id":"332694","messageId":"20171116181257.61673-8-git@jeffhostetler.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"[PATCH v4 07/10] introduce fetch-object: fetch one promisor object","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:54Z","receivedAt":"2017-11-16T18:13:40Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jonathan Tan <jonathantanmy@google.com>\n\nIntroduce fetch-object, providing the ability to fetch one object from a\npromisor remote.\n\nThis uses fetch-pack. To do this, the transport mechanism has been\nupdated with 2 flags, \"from-promisor\" to indicate that the resulting\npack comes from a promisor remote (and thus should be annotated as such\nby index-pack), and \"no-haves\" to suppress the sending of \"have\" lines.\n\nThis will be tested in a subsequent commit.\n\nNEEDSWORK: update this when we have more information about protocol v2,\nwhich should allow a way to suppress the ref advertisement and\nofficially allow any object type to be \"want\"-ed.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n Documentation/gitremote-helpers.txt |  6 ++++++\n Makefile                            |  1 +\n builtin/fetch-pack.c                |  8 ++++++++\n builtin/index-pack.c                | 16 +++++++++++++---\n fetch-object.c                      | 23 +++++++++++++++++++++++\n fetch-object.h                      |  6 ++++++\n fetch-pack.c                        |  8 ++++++--\n fetch-pack.h                        |  2 ++\n remote-curl.c                       | 14 +++++++++++++-\n transport.c                         |  8 ++++++++\n transport.h                         |  8 ++++++++\n 11 files changed, 94 insertions(+), 6 deletions(-)\n create mode 100644 fetch-object.c\n create mode 100644 fetch-object.h\n\ndiff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt\nindex 4a584f3..1ceab89 100644\n--- a/Documentation/gitremote-helpers.txt\n+++ b/Documentation/gitremote-helpers.txt\n@@ -466,6 +466,12 @@ set by Git if the remote helper has the 'option' capability.\n \tTransmit <string> as a push option. As the push option\n \tmust not contain LF or NUL characters, the string is not encoded.\n \n+'option from-promisor' {'true'|'false'}::\n+\tIndicate that these objects are being fetch by a promisor.\n+\n+'option no-haves' {'true'|'false'}::\n+\tDo not send \"have\" lines.\n+\n SEE ALSO\n --------\n linkgit:git-remote[1]\ndiff --git a/Makefile b/Makefile\nindex ca378a4..795e0c7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -792,6 +792,7 @@ LIB_OBJS += ewah/ewah_bitmap.o\n LIB_OBJS += ewah/ewah_io.o\n LIB_OBJS += ewah/ewah_rlw.o\n LIB_OBJS += exec_cmd.o\n+LIB_OBJS += fetch-object.o\n LIB_OBJS += fetch-pack.o\n LIB_OBJS += fsck.o\n LIB_OBJS += gettext.o\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 366b9d1..9f303cf 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -143,6 +143,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\t\targs.update_shallow = 1;\n \t\t\tcontinue;\n \t\t}\n+\t\tif (!strcmp(\"--from-promisor\", arg)) {\n+\t\t\targs.from_promisor = 1;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (!strcmp(\"--no-haves\", arg)) {\n+\t\t\targs.no_haves = 1;\n+\t\t\tcontinue;\n+\t\t}\n \t\tusage(fetch_pack_usage);\n \t}\n \tif (deepen_not.nr)\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 4f305a7..24c2f05 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1429,14 +1429,16 @@ static void write_special_file(const char *suffix, const char *msg,\n \t\tif (close(fd) != 0)\n \t\t\tdie_errno(_(\"cannot close written %s file '%s'\"),\n \t\t\t\t  suffix, filename);\n-\t\t*report = suffix;\n+\t\tif (report)\n+\t\t\t*report = suffix;\n \t}\n \tstrbuf_release(&name_buf);\n }\n \n static void final(const char *final_pack_name, const char *curr_pack_name,\n \t\t  const char *final_index_name, const char *curr_index_name,\n-\t\t  const char *keep_msg, unsigned char *sha1)\n+\t\t  const char *keep_msg, const char *promisor_msg,\n+\t\t  unsigned char *sha1)\n {\n \tconst char *report = \"pack\";\n \tstruct strbuf pack_name = STRBUF_INIT;\n@@ -1455,6 +1457,9 @@ static void final(const char *final_pack_name, const char *curr_pack_name,\n \tif (keep_msg)\n \t\twrite_special_file(\"keep\", keep_msg, final_pack_name, sha1,\n \t\t\t\t   &report);\n+\tif (promisor_msg)\n+\t\twrite_special_file(\"promisor\", promisor_msg, final_pack_name,\n+\t\t\t\t   sha1, NULL);\n \n \tif (final_pack_name != curr_pack_name) {\n \t\tif (!final_pack_name)\n@@ -1644,6 +1649,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tconst char *curr_index;\n \tconst char *index_name = NULL, *pack_name = NULL;\n \tconst char *keep_msg = NULL;\n+\tconst char *promisor_msg = NULL;\n \tstruct strbuf index_name_buf = STRBUF_INIT;\n \tstruct pack_idx_entry **idx_objects;\n \tstruct pack_idx_option opts;\n@@ -1693,6 +1699,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tkeep_msg = \"\";\n \t\t\t} else if (starts_with(arg, \"--keep=\")) {\n \t\t\t\tkeep_msg = arg + 7;\n+\t\t\t} else if (!strcmp(arg, \"--promisor\")) {\n+\t\t\t\tpromisor_msg = \"\";\n+\t\t\t} else if (starts_with(arg, \"--promisor=\")) {\n+\t\t\t\tpromisor_msg = arg + strlen(\"--promisor=\");\n \t\t\t} else if (starts_with(arg, \"--threads=\")) {\n \t\t\t\tchar *end;\n \t\t\t\tnr_threads = strtoul(arg+10, &end, 0);\n@@ -1803,7 +1813,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tif (!verify)\n \t\tfinal(pack_name, curr_pack,\n \t\t      index_name, curr_index,\n-\t\t      keep_msg,\n+\t\t      keep_msg, promisor_msg,\n \t\t      pack_sha1);\n \telse\n \t\tclose(input_fd);\ndiff --git a/fetch-object.c b/fetch-object.c\nnew file mode 100644\nindex 0000000..f89dbba\n--- /dev/null\n+++ b/fetch-object.c\n@@ -0,0 +1,23 @@\n+#include \"cache.h\"\n+#include \"packfile.h\"\n+#include \"pkt-line.h\"\n+#include \"strbuf.h\"\n+#include \"transport.h\"\n+\n+void fetch_object(const char *remote_name, const unsigned char *sha1)\n+{\n+\tstruct remote *remote;\n+\tstruct transport *transport;\n+\tstruct ref *ref;\n+\n+\tremote = remote_get(remote_name);\n+\tif (!remote->url[0])\n+\t\tdie(_(\"Remote with no URL\"));\n+\ttransport = transport_get(remote, remote->url[0]);\n+\n+\tref = alloc_ref(sha1_to_hex(sha1));\n+\thashcpy(ref->old_oid.hash, sha1);\n+\ttransport_set_option(transport, TRANS_OPT_FROM_PROMISOR, \"1\");\n+\ttransport_set_option(transport, TRANS_OPT_NO_HAVES, \"1\");\n+\ttransport_fetch_refs(transport, ref);\n+}\ndiff --git a/fetch-object.h b/fetch-object.h\nnew file mode 100644\nindex 0000000..f371300\n--- /dev/null\n+++ b/fetch-object.h\n@@ -0,0 +1,6 @@\n+#ifndef FETCH_OBJECT_H\n+#define FETCH_OBJECT_H\n+\n+extern void fetch_object(const char *remote_name, const unsigned char *sha1);\n+\n+#endif\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 008b25d..4640b4e 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -450,6 +450,8 @@ static int find_common(struct fetch_pack_args *args,\n \n \tflushes = 0;\n \tretval = -1;\n+\tif (args->no_haves)\n+\t\tgoto done;\n \twhile ((oid = get_rev())) {\n \t\tpacket_buf_write(&req_buf, \"have %s\\n\", oid_to_hex(oid));\n \t\tprint_verbose(args, \"have %s\", oid_to_hex(oid));\n@@ -832,7 +834,7 @@ static int get_pack(struct fetch_pack_args *args,\n \t\targv_array_push(&cmd.args, alternate_shallow_file);\n \t}\n \n-\tif (do_keep) {\n+\tif (do_keep || args->from_promisor) {\n \t\tif (pack_lockfile)\n \t\t\tcmd.out = -1;\n \t\tcmd_name = \"index-pack\";\n@@ -842,7 +844,7 @@ static int get_pack(struct fetch_pack_args *args,\n \t\t\targv_array_push(&cmd.args, \"-v\");\n \t\tif (args->use_thin_pack)\n \t\t\targv_array_push(&cmd.args, \"--fix-thin\");\n-\t\tif (args->lock_pack || unpack_limit) {\n+\t\tif (do_keep && (args->lock_pack || unpack_limit)) {\n \t\t\tchar hostname[HOST_NAME_MAX + 1];\n \t\t\tif (xgethostname(hostname, sizeof(hostname)))\n \t\t\t\txsnprintf(hostname, sizeof(hostname), \"localhost\");\n@@ -852,6 +854,8 @@ static int get_pack(struct fetch_pack_args *args,\n \t\t}\n \t\tif (args->check_self_contained_and_connected)\n \t\t\targv_array_push(&cmd.args, \"--check-self-contained-and-connected\");\n+\t\tif (args->from_promisor)\n+\t\t\targv_array_push(&cmd.args, \"--promisor\");\n \t}\n \telse {\n \t\tcmd_name = \"unpack-objects\";\ndiff --git a/fetch-pack.h b/fetch-pack.h\nindex b6aeb43..84904c3 100644\n--- a/fetch-pack.h\n+++ b/fetch-pack.h\n@@ -29,6 +29,8 @@ struct fetch_pack_args {\n \tunsigned cloning:1;\n \tunsigned update_shallow:1;\n \tunsigned deepen:1;\n+\tunsigned from_promisor:1;\n+\tunsigned no_haves:1;\n };\n \n /*\ndiff --git a/remote-curl.c b/remote-curl.c\nindex 0053b09..34a81b8 100644\n--- a/remote-curl.c\n+++ b/remote-curl.c\n@@ -33,7 +33,9 @@ struct options {\n \t\tthin : 1,\n \t\t/* One of the SEND_PACK_PUSH_CERT_* constants. */\n \t\tpush_cert : 2,\n-\t\tdeepen_relative : 1;\n+\t\tdeepen_relative : 1,\n+\t\tfrom_promisor : 1,\n+\t\tno_haves : 1;\n };\n static struct options options;\n static struct string_list cas_options = STRING_LIST_INIT_DUP;\n@@ -157,6 +159,12 @@ static int set_option(const char *name, const char *value)\n \t\t\treturn -1;\n \t\treturn 0;\n #endif /* LIBCURL_VERSION_NUM >= 0x070a08 */\n+\t} else if (!strcmp(name, \"from-promisor\")) {\n+\t\toptions.from_promisor = 1;\n+\t\treturn 0;\n+\t} else if (!strcmp(name, \"no-haves\")) {\n+\t\toptions.no_haves = 1;\n+\t\treturn 0;\n \t} else {\n \t\treturn 1 /* unsupported */;\n \t}\n@@ -822,6 +830,10 @@ static int fetch_git(struct discovery *heads,\n \t\t\t\t options.deepen_not.items[i].string);\n \tif (options.deepen_relative && options.depth)\n \t\targv_array_push(&args, \"--deepen-relative\");\n+\tif (options.from_promisor)\n+\t\targv_array_push(&args, \"--from-promisor\");\n+\tif (options.no_haves)\n+\t\targv_array_push(&args, \"--no-haves\");\n \targv_array_push(&args, url.buf);\n \n \tfor (i = 0; i < nr_heads; i++) {\ndiff --git a/transport.c b/transport.c\nindex f1e2f61..8211f82 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -160,6 +160,12 @@ static int set_git_option(struct git_transport_options *opts,\n \t} else if (!strcmp(name, TRANS_OPT_DEEPEN_RELATIVE)) {\n \t\topts->deepen_relative = !!value;\n \t\treturn 0;\n+\t} else if (!strcmp(name, TRANS_OPT_FROM_PROMISOR)) {\n+\t\topts->from_promisor = !!value;\n+\t\treturn 0;\n+\t} else if (!strcmp(name, TRANS_OPT_NO_HAVES)) {\n+\t\topts->no_haves = !!value;\n+\t\treturn 0;\n \t}\n \treturn 1;\n }\n@@ -228,6 +234,8 @@ static int fetch_refs_via_pack(struct transport *transport,\n \t\tdata->options.check_self_contained_and_connected;\n \targs.cloning = transport->cloning;\n \targs.update_shallow = data->options.update_shallow;\n+\targs.from_promisor = data->options.from_promisor;\n+\targs.no_haves = data->options.no_haves;\n \n \tif (!data->got_remote_heads) {\n \t\tconnect_setup(transport, 0);\ndiff --git a/transport.h b/transport.h\nindex bc55715..67428f6 100644\n--- a/transport.h\n+++ b/transport.h\n@@ -15,6 +15,8 @@ struct git_transport_options {\n \tunsigned self_contained_and_connected : 1;\n \tunsigned update_shallow : 1;\n \tunsigned deepen_relative : 1;\n+\tunsigned from_promisor : 1;\n+\tunsigned no_haves : 1;\n \tint depth;\n \tconst char *deepen_since;\n \tconst struct string_list *deepen_not;\n@@ -210,6 +212,12 @@ void transport_check_allowed(const char *type);\n /* Send push certificates */\n #define TRANS_OPT_PUSH_CERT \"pushcert\"\n \n+/* Indicate that these objects are being fetched by a promisor */\n+#define TRANS_OPT_FROM_PROMISOR \"from-promisor\"\n+\n+/* Do not send \"have\" lines */\n+#define TRANS_OPT_NO_HAVES \"no-haves\"\n+\n /**\n  * Returns 0 if the option was used, non-zero otherwise. Prints a\n  * message to stderr if the option is not used.\n-- \n2.9.3\n\n"},{"id":"332695","messageId":"20171116181257.61673-3-git@jeffhostetler.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"[PATCH v4 02/10] fsck: introduce partialclone extension","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:49Z","receivedAt":"2017-11-16T18:13:43Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jonathan Tan <jonathantanmy@google.com>\n\nCurrently, Git does not support repos with very large numbers of objects\nor repos that wish to minimize manipulation of certain blobs (for\nexample, because they are very large) very well, even if the user\noperates mostly on part of the repo, because Git is designed on the\nassumption that every referenced object is available somewhere in the\nrepo storage. In such an arrangement, the full set of objects is usually\navailable in remote storage, ready to be lazily downloaded.\n\nTeach fsck about the new state of affairs. In this commit, teach fsck\nthat missing promisor objects referenced from the reflog are not an\nerror case; in future commits, fsck will be taught about other cases.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n builtin/fsck.c           |  2 +-\n cache.h                  |  3 +-\n packfile.c               | 77 +++++++++++++++++++++++++++++++++++++++++++--\n packfile.h               | 13 ++++++++\n t/t0410-partial-clone.sh | 81 ++++++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 171 insertions(+), 5 deletions(-)\n create mode 100755 t/t0410-partial-clone.sh\n\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 56afe40..2934299 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -398,7 +398,7 @@ static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,\n \t\t\t\t\txstrfmt(\"%s@{%\"PRItime\"}\", refname, timestamp));\n \t\t\tobj->flags |= USED;\n \t\t\tmark_object_reachable(obj);\n-\t\t} else {\n+\t\t} else if (!is_promisor_object(oid)) {\n \t\t\terror(\"%s: invalid reflog entry %s\", refname, oid_to_hex(oid));\n \t\t\terrors_found |= ERROR_REACHABLE;\n \t\t}\ndiff --git a/cache.h b/cache.h\nindex 35e3f5e..c76f2e9 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1587,7 +1587,8 @@ extern struct packed_git {\n \tunsigned pack_local:1,\n \t\t pack_keep:1,\n \t\t freshened:1,\n-\t\t do_not_close:1;\n+\t\t do_not_close:1,\n+\t\t pack_promisor:1;\n \tunsigned char sha1[20];\n \tstruct revindex_entry *revindex;\n \t/* something like \".git/objects/pack/xxxxx.pack\" */\ndiff --git a/packfile.c b/packfile.c\nindex 4a5fe7a..234797c 100644\n--- a/packfile.c\n+++ b/packfile.c\n@@ -8,6 +8,11 @@\n #include \"list.h\"\n #include \"streaming.h\"\n #include \"sha1-lookup.h\"\n+#include \"commit.h\"\n+#include \"object.h\"\n+#include \"tag.h\"\n+#include \"tree-walk.h\"\n+#include \"tree.h\"\n \n char *odb_pack_name(struct strbuf *buf,\n \t\t    const unsigned char *sha1,\n@@ -643,10 +648,10 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local)\n \t\treturn NULL;\n \n \t/*\n-\t * \".pack\" is long enough to hold any suffix we're adding (and\n+\t * \".promisor\" is long enough to hold any suffix we're adding (and\n \t * the use xsnprintf double-checks that)\n \t */\n-\talloc = st_add3(path_len, strlen(\".pack\"), 1);\n+\talloc = st_add3(path_len, strlen(\".promisor\"), 1);\n \tp = alloc_packed_git(alloc);\n \tmemcpy(p->pack_name, path, path_len);\n \n@@ -654,6 +659,10 @@ struct packed_git *add_packed_git(const char *path, size_t path_len, int local)\n \tif (!access(p->pack_name, F_OK))\n \t\tp->pack_keep = 1;\n \n+\txsnprintf(p->pack_name + path_len, alloc - path_len, \".promisor\");\n+\tif (!access(p->pack_name, F_OK))\n+\t\tp->pack_promisor = 1;\n+\n \txsnprintf(p->pack_name + path_len, alloc - path_len, \".pack\");\n \tif (stat(p->pack_name, &st) || !S_ISREG(st.st_mode)) {\n \t\tfree(p);\n@@ -781,7 +790,8 @@ static void prepare_packed_git_one(char *objdir, int local)\n \t\tif (ends_with(de->d_name, \".idx\") ||\n \t\t    ends_with(de->d_name, \".pack\") ||\n \t\t    ends_with(de->d_name, \".bitmap\") ||\n-\t\t    ends_with(de->d_name, \".keep\"))\n+\t\t    ends_with(de->d_name, \".keep\") ||\n+\t\t    ends_with(de->d_name, \".promisor\"))\n \t\t\tstring_list_append(&garbage, path.buf);\n \t\telse\n \t\t\treport_garbage(PACKDIR_FILE_GARBAGE, path.buf);\n@@ -1889,6 +1899,9 @@ int for_each_packed_object(each_packed_object_fn cb, void *data, unsigned flags)\n \tfor (p = packed_git; p; p = p->next) {\n \t\tif ((flags & FOR_EACH_OBJECT_LOCAL_ONLY) && !p->pack_local)\n \t\t\tcontinue;\n+\t\tif ((flags & FOR_EACH_OBJECT_PROMISOR_ONLY) &&\n+\t\t    !p->pack_promisor)\n+\t\t\tcontinue;\n \t\tif (open_pack_index(p)) {\n \t\t\tpack_errors = 1;\n \t\t\tcontinue;\n@@ -1899,3 +1912,61 @@ int for_each_packed_object(each_packed_object_fn cb, void *data, unsigned flags)\n \t}\n \treturn r ? r : pack_errors;\n }\n+\n+static int add_promisor_object(const struct object_id *oid,\n+\t\t\t       struct packed_git *pack,\n+\t\t\t       uint32_t pos,\n+\t\t\t       void *set_)\n+{\n+\tstruct oidset *set = set_;\n+\tstruct object *obj = parse_object(oid);\n+\tif (!obj)\n+\t\treturn 1;\n+\n+\toidset_insert(set, oid);\n+\n+\t/*\n+\t * If this is a tree, commit, or tag, the objects it refers\n+\t * to are also promisor objects. (Blobs refer to no objects.)\n+\t */\n+\tif (obj->type == OBJ_TREE) {\n+\t\tstruct tree *tree = (struct tree *)obj;\n+\t\tstruct tree_desc desc;\n+\t\tstruct name_entry entry;\n+\t\tif (init_tree_desc_gently(&desc, tree->buffer, tree->size))\n+\t\t\t/*\n+\t\t\t * Error messages are given when packs are\n+\t\t\t * verified, so do not print any here.\n+\t\t\t */\n+\t\t\treturn 0;\n+\t\twhile (tree_entry_gently(&desc, &entry))\n+\t\t\toidset_insert(set, entry.oid);\n+\t} else if (obj->type == OBJ_COMMIT) {\n+\t\tstruct commit *commit = (struct commit *) obj;\n+\t\tstruct commit_list *parents = commit->parents;\n+\n+\t\toidset_insert(set, &commit->tree->object.oid);\n+\t\tfor (; parents; parents = parents->next)\n+\t\t\toidset_insert(set, &parents->item->object.oid);\n+\t} else if (obj->type == OBJ_TAG) {\n+\t\tstruct tag *tag = (struct tag *) obj;\n+\t\toidset_insert(set, &tag->tagged->oid);\n+\t}\n+\treturn 0;\n+}\n+\n+int is_promisor_object(const struct object_id *oid)\n+{\n+\tstatic struct oidset promisor_objects;\n+\tstatic int promisor_objects_prepared;\n+\n+\tif (!promisor_objects_prepared) {\n+\t\tif (repository_format_partial_clone) {\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);\n+\t\t}\n+\t\tpromisor_objects_prepared = 1;\n+\t}\n+\treturn oidset_contains(&promisor_objects, oid);\n+}\ndiff --git a/packfile.h b/packfile.h\nindex 0cdeb54..a7fca59 100644\n--- a/packfile.h\n+++ b/packfile.h\n@@ -1,6 +1,8 @@\n #ifndef PACKFILE_H\n #define PACKFILE_H\n \n+#include \"oidset.h\"\n+\n /*\n  * Generate the filename to be used for a pack file with checksum \"sha1\" and\n  * extension \"ext\". The result is written into the strbuf \"buf\", overwriting\n@@ -125,6 +127,11 @@ extern int has_sha1_pack(const unsigned char *sha1);\n extern int has_pack_index(const unsigned char *sha1);\n \n /*\n+ * Only iterate over packs obtained from the promisor remote.\n+ */\n+#define FOR_EACH_OBJECT_PROMISOR_ONLY 2\n+\n+/*\n  * Iterate over packed objects in both the local\n  * repository and any alternates repositories (unless the\n  * FOR_EACH_OBJECT_LOCAL_ONLY flag, defined in cache.h, is set).\n@@ -135,4 +142,10 @@ typedef int each_packed_object_fn(const struct object_id *oid,\n \t\t\t\t  void *data);\n extern int for_each_packed_object(each_packed_object_fn, void *, unsigned flags);\n \n+/*\n+ * Return 1 if an object in a promisor packfile is or refers to the given\n+ * object, 0 otherwise.\n+ */\n+extern int is_promisor_object(const struct object_id *oid);\n+\n #endif\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nnew file mode 100755\nindex 0000000..3ddb3b9\n--- /dev/null\n+++ b/t/t0410-partial-clone.sh\n@@ -0,0 +1,81 @@\n+#!/bin/sh\n+\n+test_description='partial clone'\n+\n+. ./test-lib.sh\n+\n+delete_object () {\n+\trm $1/.git/objects/$(echo $2 | sed -e 's|^..|&/|')\n+}\n+\n+pack_as_from_promisor () {\n+\tHASH=$(git -C repo pack-objects .git/objects/pack/pack) &&\n+\t>repo/.git/objects/pack/pack-$HASH.promisor\n+}\n+\n+test_expect_success 'missing reflog object, but promised by a commit, passes fsck' '\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo my_commit &&\n+\n+\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n+\tC=$(git -C repo commit-tree -m c -p $A HEAD^{tree}) &&\n+\n+\t# Reference $A only from reflog, and delete it\n+\tgit -C repo branch my_branch \"$A\" &&\n+\tgit -C repo branch -f my_branch my_commit &&\n+\tdelete_object repo \"$A\" &&\n+\n+\t# State that we got $C, which refers to $A, from promisor\n+\tprintf \"$C\\n\" | pack_as_from_promisor &&\n+\n+\t# Normally, it fails\n+\ttest_must_fail git -C repo fsck &&\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 fsck\n+'\n+\n+test_expect_success 'missing reflog object, but promised by a tag, passes fsck' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo my_commit &&\n+\n+\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n+\tgit -C repo tag -a -m d my_tag_name $A &&\n+\tT=$(git -C repo rev-parse my_tag_name) &&\n+\tgit -C repo tag -d my_tag_name &&\n+\n+\t# Reference $A only from reflog, and delete it\n+\tgit -C repo branch my_branch \"$A\" &&\n+\tgit -C repo branch -f my_branch my_commit &&\n+\tdelete_object repo \"$A\" &&\n+\n+\t# State that we got $T, which refers to $A, from promisor\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 fsck\n+'\n+\n+test_expect_success 'missing reflog object alone fails fsck, even with extension set' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo my_commit &&\n+\n+\tA=$(git -C repo commit-tree -m a HEAD^{tree}) &&\n+\tB=$(git -C repo commit-tree -m b HEAD^{tree}) &&\n+\n+\t# Reference $A only from reflog, and delete it\n+\tgit -C repo branch my_branch \"$A\" &&\n+\tgit -C repo branch -f my_branch my_commit &&\n+\tdelete_object repo \"$A\" &&\n+\n+\tgit -C repo config core.repositoryformatversion 1 &&\n+\tgit -C repo config extensions.partialclone \"arbitrary string\" &&\n+\ttest_must_fail git -C repo fsck\n+'\n+\n+test_done\n-- \n2.9.3\n\n"},{"id":"332696","messageId":"20171116181257.61673-9-git@jeffhostetler.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"[PATCH v4 08/10] sha1_file: support lazily fetching missing objects","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:55Z","receivedAt":"2017-11-16T18:13:46Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jonathan Tan <jonathantanmy@google.com>\n\nTeach sha1_file to fetch objects from the remote configured in\nextensions.partialclone whenever an object is requested but missing.\n\nThe fetching of objects can be suppressed through a global variable.\nThis is used by fsck and index-pack.\n\nHowever, by default, such fetching is not suppressed. This is meant as a\ntemporary measure to ensure that all Git commands work in such a\nsituation. Future patches will update some commands to either tolerate\nmissing objects (without fetching them) or be more efficient in fetching\nthem.\n\nIn order to determine the code changes in sha1_file.c necessary, I\ninvestigated the following:\n (1) functions in sha1_file.c that take in a hash, without the user\n     regarding how the object is stored (loose or packed)\n (2) functions in packfile.c (because I need to check callers that know\n     about the loose/packed distinction and operate on both differently,\n     and ensure that they can handle the concept of objects that are\n     neither loose nor packed)\n\n(1) is handled by the modification to sha1_object_info_extended().\n\nFor (2), I looked at for_each_packed_object and others.  For\nfor_each_packed_object, the callers either already work or are fixed in\nthis patch:\n - reachable - only to find recent objects\n - builtin/fsck - already knows about missing objects\n - builtin/cat-file - warning message added in this commit\n\nCallers of the other functions do not need to be changed:\n - parse_pack_index\n   - http - indirectly from http_get_info_packs\n   - find_pack_entry_one\n     - this searches a single pack that is provided as an argument; the\n       caller already knows (through other means) that the sought object\n       is in a specific pack\n - find_sha1_pack\n   - fast-import - appears to be an optimization to not store a file if\n     it is already in a pack\n   - http-walker - to search through a struct alt_base\n   - http-push - to search through remote packs\n - has_sha1_pack\n   - builtin/fsck - already knows about promisor objects\n   - builtin/count-objects - informational purposes only (check if loose\n     object is also packed)\n   - builtin/prune-packed - check if object to be pruned is packed (if\n     not, don't prune it)\n   - revision - used to exclude packed objects if requested by user\n   - diff - just for optimization\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n builtin/cat-file.c       |  2 ++\n builtin/fetch-pack.c     |  2 ++\n builtin/fsck.c           |  3 +++\n builtin/index-pack.c     |  6 ++++++\n cache.h                  |  8 ++++++++\n fetch-object.c           |  3 +++\n sha1_file.c              | 38 ++++++++++++++++++++++++------------\n t/t0410-partial-clone.sh | 51 ++++++++++++++++++++++++++++++++++++++++++++++++\n 8 files changed, 100 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex f5fa4fd..cf9ea5c 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -475,6 +475,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 \n \t\tcb.opt = opt;\n \t\tcb.expand = &data;\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 9f303cf..9a7ebf6 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -53,6 +53,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \tstruct oid_array shallow = OID_ARRAY_INIT;\n \tstruct string_list deepen_not = STRING_LIST_INIT_DUP;\n \n+\tfetch_if_missing = 0;\n+\n \tpacket_trace_identity(\"fetch-pack\");\n \n \tmemset(&args, 0, sizeof(args));\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex 578a7c8..3b76c0e 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -678,6 +678,9 @@ int cmd_fsck(int argc, const char **argv, const char *prefix)\n \tint i;\n \tstruct alternate_object_database *alt;\n \n+\t/* fsck knows how to handle missing promisor objects */\n+\tfetch_if_missing = 0;\n+\n \terrors_found = 0;\n \tcheck_replace_refs = 0;\n \ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 24c2f05..a0a35e6 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1657,6 +1657,12 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \tunsigned foreign_nr = 1;\t/* zero is a \"good\" value, assume bad */\n \tint report_end_of_input = 0;\n \n+\t/*\n+\t * index-pack never needs to fetch missing objects, since it only\n+\t * accesses the repo to do hash collision checks\n+\t */\n+\tfetch_if_missing = 0;\n+\n \tif (argc == 2 && !strcmp(argv[1], \"-h\"))\n \t\tusage(index_pack_usage);\n \ndiff --git a/cache.h b/cache.h\nindex c76f2e9..6980072 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1727,6 +1727,14 @@ struct object_info {\n #define OBJECT_INFO_QUICK 8\n extern int sha1_object_info_extended(const unsigned char *, struct object_info *, unsigned flags);\n \n+/*\n+ * Set this to 0 to prevent sha1_object_info_extended() from fetching missing\n+ * blobs. This has a difference only if extensions.partialClone is set.\n+ *\n+ * Its default value is 1.\n+ */\n+extern int fetch_if_missing;\n+\n /* Dumb servers support */\n extern int update_server_info(int);\n \ndiff --git a/fetch-object.c b/fetch-object.c\nindex f89dbba..369b61c 100644\n--- a/fetch-object.c\n+++ b/fetch-object.c\n@@ -9,7 +9,9 @@ void fetch_object(const char *remote_name, const unsigned char *sha1)\n \tstruct remote *remote;\n \tstruct transport *transport;\n \tstruct ref *ref;\n+\tint original_fetch_if_missing = fetch_if_missing;\n \n+\tfetch_if_missing = 0;\n \tremote = remote_get(remote_name);\n \tif (!remote->url[0])\n \t\tdie(_(\"Remote with no URL\"));\n@@ -20,4 +22,5 @@ void fetch_object(const char *remote_name, const unsigned char *sha1)\n \ttransport_set_option(transport, TRANS_OPT_FROM_PROMISOR, \"1\");\n \ttransport_set_option(transport, TRANS_OPT_NO_HAVES, \"1\");\n \ttransport_fetch_refs(transport, ref);\n+\tfetch_if_missing = original_fetch_if_missing;\n }\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 10c3a00..fc7718a 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -29,6 +29,7 @@\n #include \"mergesort.h\"\n #include \"quote.h\"\n #include \"packfile.h\"\n+#include \"fetch-object.h\"\n \n const unsigned char null_sha1[GIT_MAX_RAWSZ];\n const struct object_id null_oid;\n@@ -1144,6 +1145,8 @@ static int sha1_loose_object_info(const unsigned char *sha1,\n \treturn (status < 0) ? status : 0;\n }\n \n+int fetch_if_missing = 1;\n+\n int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi, unsigned flags)\n {\n \tstatic struct object_info blank_oi = OBJECT_INFO_INIT;\n@@ -1152,6 +1155,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \tconst unsigned char *real = (flags & OBJECT_INFO_LOOKUP_REPLACE) ?\n \t\t\t\t    lookup_replace_object(sha1) :\n \t\t\t\t    sha1;\n+\tint already_retried = 0;\n \n \tif (!oi)\n \t\toi = &blank_oi;\n@@ -1176,28 +1180,36 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi,\n \t\t}\n \t}\n \n-\tif (!find_pack_entry(real, &e)) {\n-\t\t/* Most likely it's a loose object. */\n-\t\tif (!sha1_loose_object_info(real, oi, flags))\n-\t\t\treturn 0;\n+retry:\n+\tif (find_pack_entry(real, &e))\n+\t\tgoto found_packed;\n \n-\t\t/* Not a loose object; someone else may have just packed it. */\n-\t\tif (flags & OBJECT_INFO_QUICK) {\n-\t\t\treturn -1;\n-\t\t} else {\n-\t\t\treprepare_packed_git();\n-\t\t\tif (!find_pack_entry(real, &e))\n-\t\t\t\treturn -1;\n-\t\t}\n+\t/* Most likely it's a loose object. */\n+\tif (!sha1_loose_object_info(real, oi, flags))\n+\t\treturn 0;\n+\n+\t/* Not a loose object; someone else may have just packed it. */\n+\treprepare_packed_git();\n+\tif (find_pack_entry(real, &e))\n+\t\tgoto found_packed;\n+\n+\t/* Check if it is a missing object */\n+\tif (fetch_if_missing && repository_format_partial_clone &&\n+\t    !already_retried) {\n+\t\tfetch_object(repository_format_partial_clone, real);\n+\t\talready_retried = 1;\n+\t\tgoto retry;\n \t}\n \n+\treturn -1;\n+\n+found_packed:\n \tif (oi == &blank_oi)\n \t\t/*\n \t\t * We know that the caller doesn't actually need the\n \t\t * information below, so return early.\n \t\t */\n \t\treturn 0;\n-\n \trtype = packed_object_info(e.p, e.offset, oi);\n \tif (rtype < 0) {\n \t\tmark_bad_packed_object(e.p, real);\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex e96f436..8a90f6a 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -138,4 +138,55 @@ test_expect_success 'missing CLI object, but promised, passes fsck' '\n \tgit -C repo fsck \"$A\"\n '\n \n+test_expect_success 'fetching of missing objects' '\n+\trm -rf repo &&\n+\ttest_create_repo server &&\n+\ttest_commit -C server foo &&\n+\tgit -C server repack -a -d --write-bitmap-index &&\n+\n+\tgit clone \"file://$(pwd)/server\" repo &&\n+\tHASH=$(git -C repo rev-parse foo) &&\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 cat-file -p \"$HASH\" &&\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 \"$HASH\"\n+'\n+\n+LIB_HTTPD_PORT=12345  # default port, 410, cannot be used as non-root\n+. \"$TEST_DIRECTORY\"/lib-httpd.sh\n+start_httpd\n+\n+test_expect_success 'fetching of missing objects from an HTTP server' '\n+\trm -rf repo &&\n+\tSERVER=\"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\ttest_create_repo \"$SERVER\" &&\n+\ttest_commit -C \"$SERVER\" foo &&\n+\tgit -C \"$SERVER\" repack -a -d --write-bitmap-index &&\n+\n+\tgit clone $HTTPD_URL/smart/server repo &&\n+\tHASH=$(git -C repo rev-parse foo) &&\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 cat-file -p \"$HASH\" &&\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 \"$HASH\"\n+'\n+\n+stop_httpd\n+\n test_done\n-- \n2.9.3\n\n"},{"id":"332697","messageId":"20171116181257.61673-11-git@jeffhostetler.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"[PATCH v4 10/10] gc: do not repack promisor packfiles","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:57Z","receivedAt":"2017-11-16T18:13:51Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jonathan Tan <jonathantanmy@google.com>\n\nTeach gc to stop traversal at promisor objects, and to leave promisor\npackfiles alone. This has the effect of only repacking non-promisor\npackfiles, and preserves the distinction between promisor packfiles and\nnon-promisor packfiles.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\nSigned-off-by: Jeff Hostetler <jeffhost@microsoft.com>\n---\n Documentation/git-pack-objects.txt | 12 ++++++++-\n builtin/gc.c                       |  3 +++\n builtin/pack-objects.c             | 36 ++++++++++++++++++++++++++\n builtin/prune.c                    |  7 +++++\n builtin/repack.c                   |  8 ++++--\n t/t0410-partial-clone.sh           | 52 +++++++++++++++++++++++++++++++++++++-\n 6 files changed, 114 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\nindex 5fad696..33a824e 100644\n--- a/Documentation/git-pack-objects.txt\n+++ b/Documentation/git-pack-objects.txt\n@@ -242,9 +242,19 @@ So does `git bundle` (see linkgit:git-bundle[1]) when it creates a bundle.\n \tthe resulting packfile.  See linkgit:git-rev-list[1] for valid\n \t`<filter-spec>` forms.\n \n---missing=(error|allow-any):\n+--missing=(error|allow-any|allow-promisor):\n \tSpecifies how missing objects are handled.  This is useful, for\n \texample, when there are missing objects from a prior partial clone.\n+\tThis is stronger than `--missing=allow-promisor` because it limits\n+\tthe traversal, rather than just silencing errors about missing\n+\tobjects.\n+\n+--exclude-promisor-objects::\n+\tOmit objects that are known to be in the promisor remote\". (This\n+\toption has the purpose of operating only on locally created objects,\n+\tso that when we repack, we still maintain a distinction between\n+\tlocally created objects [without .promisor] and objects from the\n+\tpromisor remote [with .promisor].)  This is used with partial clone.\n \n SEE ALSO\n --------\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 3c5eae0..77fa720 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -458,6 +458,9 @@ 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\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))\n \t\t\t\treturn error(FAILED_RUN, prune.argv[0]);\n \t\t}\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex 45ad35d..4534209 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -75,6 +75,8 @@ static int use_bitmap_index = -1;\n static int write_bitmap_index;\n static uint16_t write_bitmap_options;\n \n+static int exclude_promisor_objects;\n+\n static unsigned long delta_cache_size = 0;\n static unsigned long max_delta_cache_size = 256 * 1024 * 1024;\n static unsigned long cache_max_small_delta_size = 1000;\n@@ -86,6 +88,7 @@ static struct list_objects_filter_options filter_options;\n enum missing_action {\n \tMA_ERROR = 0,    /* fail if any missing objects are encountered */\n \tMA_ALLOW_ANY,    /* silently allow ALL missing objects */\n+\tMA_ALLOW_PROMISOR, /* silently allow all missing PROMISOR objects */\n };\n static enum missing_action arg_missing_action;\n static show_object_fn fn_show_object;\n@@ -2577,6 +2580,20 @@ static void show_object__ma_allow_any(struct object *obj, const char *name, void\n \tshow_object(obj, name, data);\n }\n \n+static void show_object__ma_allow_promisor(struct object *obj, const char *name, void *data)\n+{\n+\tassert(arg_missing_action == MA_ALLOW_PROMISOR);\n+\n+\t/*\n+\t * Quietly ignore EXPECTED missing objects.  This avoids problems with\n+\t * staging them now and getting an odd error later.\n+\t */\n+\tif (!has_object_file(&obj->oid) && is_promisor_object(&obj->oid))\n+\t\treturn;\n+\n+\tshow_object(obj, name, data);\n+}\n+\n static int option_parse_missing_action(const struct option *opt,\n \t\t\t\t       const char *arg, int unset)\n {\n@@ -2591,10 +2608,18 @@ static int option_parse_missing_action(const struct option *opt,\n \n \tif (!strcmp(arg, \"allow-any\")) {\n \t\targ_missing_action = MA_ALLOW_ANY;\n+\t\tfetch_if_missing = 0;\n \t\tfn_show_object = show_object__ma_allow_any;\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(arg, \"allow-promisor\")) {\n+\t\targ_missing_action = MA_ALLOW_PROMISOR;\n+\t\tfetch_if_missing = 0;\n+\t\tfn_show_object = show_object__ma_allow_promisor;\n+\t\treturn 0;\n+\t}\n+\n \tdie(_(\"invalid value for --missing\"));\n \treturn 0;\n }\n@@ -3008,6 +3033,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\t{ OPTION_CALLBACK, 0, \"missing\", NULL, N_(\"action\"),\n \t\t  N_(\"handling for missing objects\"), PARSE_OPT_NONEG,\n \t\t  option_parse_missing_action },\n+\t\tOPT_BOOL(0, \"exclude-promisor-objects\", &exclude_promisor_objects,\n+\t\t\t N_(\"do not pack objects in promisor packfiles\")),\n \t\tOPT_END(),\n \t};\n \n@@ -3053,6 +3080,15 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n \t\targv_array_push(&rp, \"--unpacked\");\n \t}\n \n+\tif (exclude_promisor_objects) {\n+\t\tuse_internal_rev_list = 1;\n+\t\tfetch_if_missing = 0;\n+\t\targv_array_push(&rp, \"--exclude-promisor-objects\");\n+\t\t/* silently override any \"--missing=\" value */\n+\t\targ_missing_action = MA_ALLOW_PROMISOR;\n+\t\tfn_show_object = show_object__ma_allow_promisor;\n+\t}\n+\n \tif (!reuse_object)\n \t\treuse_delta = 0;\n \tif (pack_compression_level == -1)\ndiff --git a/builtin/prune.c b/builtin/prune.c\nindex cddabf2..be34645 100644\n--- a/builtin/prune.c\n+++ b/builtin/prune.c\n@@ -101,12 +101,15 @@ int cmd_prune(int argc, const char **argv, const char *prefix)\n {\n \tstruct rev_info revs;\n \tstruct progress *progress = NULL;\n+\tint exclude_promisor_objects = 0;\n \tconst struct option options[] = {\n \t\tOPT__DRY_RUN(&show_only, N_(\"do not remove, show only\")),\n \t\tOPT__VERBOSE(&verbose, N_(\"report pruned objects\")),\n \t\tOPT_BOOL(0, \"progress\", &show_progress, N_(\"show progress\")),\n \t\tOPT_EXPIRY_DATE(0, \"expire\", &expire,\n \t\t\t\tN_(\"expire objects older than <time>\")),\n+\t\tOPT_BOOL(0, \"exclude-promisor-objects\", &exclude_promisor_objects,\n+\t\t\t N_(\"limit traversal to objects outside promisor packfiles\")),\n \t\tOPT_END()\n \t};\n \tchar *s;\n@@ -139,6 +142,10 @@ int cmd_prune(int argc, const char **argv, const char *prefix)\n \t\tshow_progress = isatty(2);\n \tif (show_progress)\n \t\tprogress = start_delayed_progress(_(\"Checking connectivity\"), 0);\n+\tif (exclude_promisor_objects) {\n+\t\tfetch_if_missing = 0;\n+\t\trevs.exclude_promisor_objects = 1;\n+\t}\n \n \tmark_reachable_objects(&revs, 1, expire, progress);\n \tstop_progress(&progress);\ndiff --git a/builtin/repack.c b/builtin/repack.c\nindex f17a68a..7bdb401 100644\n--- a/builtin/repack.c\n+++ b/builtin/repack.c\n@@ -83,7 +83,8 @@ static void remove_pack_on_signal(int signo)\n \n /*\n  * Adds all packs hex strings to the fname list, which do not\n- * have a corresponding .keep file.\n+ * have a corresponding .keep or .promisor file. These packs are not to\n+ * be kept if we are going to pack everything into one file.\n  */\n static void get_non_kept_pack_filenames(struct string_list *fname_list)\n {\n@@ -101,7 +102,8 @@ static void get_non_kept_pack_filenames(struct string_list *fname_list)\n \n \t\tfname = xmemdupz(e->d_name, len);\n \n-\t\tif (!file_exists(mkpath(\"%s/%s.keep\", packdir, fname)))\n+\t\tif (!file_exists(mkpath(\"%s/%s.keep\", packdir, fname)) &&\n+\t\t    !file_exists(mkpath(\"%s/%s.promisor\", packdir, fname)))\n \t\t\tstring_list_append_nodup(fname_list, fname);\n \t\telse\n \t\t\tfree(fname);\n@@ -232,6 +234,8 @@ 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+\t\targv_array_push(&cmd.args, \"--exclude-promisor-objects\");\n \tif (window)\n \t\targv_array_pushf(&cmd.args, \"--window=%s\", window);\n \tif (window_memory)\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex 3ca6af5..071736c 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -11,13 +11,15 @@ delete_object () {\n pack_as_from_promisor () {\n \tHASH=$(git -C repo pack-objects .git/objects/pack/pack) &&\n \t>repo/.git/objects/pack/pack-$HASH.promisor\n+\techo $HASH\n }\n \n promise_and_delete () {\n \tHASH=$(git -C repo rev-parse \"$1\") &&\n \tgit -C repo tag -a -m message my_annotated_tag \"$HASH\" &&\n \tgit -C repo rev-parse my_annotated_tag | pack_as_from_promisor &&\n-\tgit -C repo tag -d my_annotated_tag &&\n+\t# tag -d prints a message to stdout, so redirect it\n+\tgit -C repo tag -d my_annotated_tag >/dev/null &&\n \tdelete_object repo \"$HASH\"\n }\n \n@@ -261,6 +263,54 @@ test_expect_success 'rev-list accepts missing and promised objects on command li\n \tgit -C repo rev-list --exclude-promisor-objects --objects \"$COMMIT\" \"$TREE\" \"$BLOB\"\n '\n \n+test_expect_success 'gc does not repack promisor objects' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo my_commit &&\n+\n+\tTREE_HASH=$(git -C repo rev-parse HEAD^{tree}) &&\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 gc &&\n+\n+\t# Ensure that the promisor packfile still exists, and remove it\n+\ttest -e repo/.git/objects/pack/pack-$HASH.pack &&\n+\trm repo/.git/objects/pack/pack-$HASH.* &&\n+\n+\t# Ensure that the single other pack contains the commit, but not the tree\n+\tls repo/.git/objects/pack/pack-*.pack >packlist &&\n+\ttest_line_count = 1 packlist &&\n+\tgit verify-pack repo/.git/objects/pack/pack-*.pack -v >out &&\n+\tgrep \"$(git -C repo rev-parse HEAD)\" out &&\n+\t! grep \"$TREE_HASH\" out\n+'\n+\n+test_expect_success 'gc stops traversal when a missing but promised object is reached' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo my_commit &&\n+\n+\tTREE_HASH=$(git -C repo rev-parse HEAD^{tree}) &&\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 gc &&\n+\n+\t# Ensure that the promisor packfile still exists, and remove it\n+\ttest -e repo/.git/objects/pack/pack-$HASH.pack &&\n+\trm repo/.git/objects/pack/pack-$HASH.* &&\n+\n+\t# Ensure that the single other pack contains the commit, but not the tree\n+\tls repo/.git/objects/pack/pack-*.pack >packlist &&\n+\ttest_line_count = 1 packlist &&\n+\tgit verify-pack repo/.git/objects/pack/pack-*.pack -v >out &&\n+\tgrep \"$(git -C repo rev-parse HEAD)\" out &&\n+\t! grep \"$TREE_HASH\" out\n+'\n+\n LIB_HTTPD_PORT=12345  # default port, 410, cannot be used as non-root\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n-- \n2.9.3\n\n"},{"id":"332698","messageId":"20171116181257.61673-10-git@jeffhostetler.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"[PATCH v4 09/10] rev-list: support termination at promisor objects","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:56Z","receivedAt":"2017-11-16T18:13:55Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jonathan Tan <jonathantanmy@google.com>\n\nTeach rev-list to support termination of an object traversal at any\nobject from a promisor remote (whether one that the local repo also has,\nor one that the local repo knows about because it has another promisor\nobject that references it).\n\nThis will be used subsequently in gc and in the connectivity check used\nby fetch.\n\nFor efficiency, if an object is referenced by a promisor object, and is\nin the local repo only as a non-promisor object, object traversal will\nnot stop there. This is to avoid building the list of promisor object\nreferences.\n\n(In list-objects.c, the case where obj is NULL in process_blob() and\nprocess_tree() do not need to be changed because those happen only when\nthere is a conflict between the expected type and the existing object.\nIf the object doesn't exist, an object will be synthesized, which is\nfine.)\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\nSigned-off-by: Jeff Hostetler <jeffhost@microsoft.com>\n---\n Documentation/rev-list-options.txt |  12 ++++-\n builtin/rev-list.c                 |  74 ++++++++++++++++++++++++---\n list-objects.c                     |  29 ++++++++++-\n object.c                           |   2 +-\n revision.c                         |  33 +++++++++++-\n revision.h                         |   5 +-\n t/t0410-partial-clone.sh           | 101 +++++++++++++++++++++++++++++++++++++\n 7 files changed, 243 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex c84e465..2beffe3 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -730,7 +730,7 @@ specification contained in <path>.\n \tOnly useful with `--filter=`; prints a list of the omitted objects.\n \tObject IDs are prefixed with a ``~'' character.\n \n---missing=(error|allow-any|print)::\n+--missing=(error|allow-any|allow-promisor|print)::\n \tSpecifies how missing objects are handled.  The repository may\n \thave missing objects after a partial clone, for example.\n +\n@@ -741,10 +741,20 @@ The value 'allow-any' will allow object traversal to continue if a\n missing object is encountered.  Missing objects will silently be omitted\n from the results.\n +\n+The value 'allow-promisor' is like 'allow-any' in that it will allow\n+object traversal to continue, but only for EXPECTED missing objects.\n++\n The value 'print' is like 'allow-any', but will also print a list of the\n missing objects.  Object IDs are prefixed with a ``?'' character.\n endif::git-rev-list[]\n \n+--exclude-promisor-objects::\n+\t(For internal use only.)  Prefilter object traversal at\n+\tpromisor boundary.  This is used with partial clone.  This is\n+\tstronger than `--missing=allow-promisor` because it limits the\n+\ttraversal, rather than just silencing errors about missing\n+\tobjects.\n+\n --no-walk[=(sorted|unsorted)]::\n \tOnly show the given commits, but do not traverse their ancestors.\n \tThis has no effect if a range is specified. If the argument\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex da4a39b..d144d66 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -15,6 +15,7 @@\n #include \"progress.h\"\n #include \"reflog-walk.h\"\n #include \"oidset.h\"\n+#include \"packfile.h\"\n \n static const char rev_list_usage[] =\n \"git rev-list [OPTION] <commit-id>... [ -- paths... ]\\n\"\n@@ -67,6 +68,7 @@ enum missing_action {\n \tMA_ERROR = 0,    /* fail if any missing objects are encountered */\n \tMA_ALLOW_ANY,    /* silently allow ALL missing objects */\n \tMA_PRINT,        /* print ALL missing objects in special section */\n+\tMA_ALLOW_PROMISOR, /* silently allow all missing PROMISOR objects */\n };\n static enum missing_action arg_missing_action;\n \n@@ -197,6 +199,12 @@ static void finish_commit(struct commit *commit, void *data)\n \n static inline void finish_object__ma(struct object *obj)\n {\n+\t/*\n+\t * Whether or not we try to dynamically fetch missing objects\n+\t * from the server, we currently DO NOT have the object.  We\n+\t * can either print, allow (ignore), or conditionally allow\n+\t * (ignore) them.\n+\t */\n \tswitch (arg_missing_action) {\n \tcase MA_ERROR:\n \t\tdie(\"missing blob object '%s'\", oid_to_hex(&obj->oid));\n@@ -209,25 +217,36 @@ static inline void finish_object__ma(struct object *obj)\n \t\toidset_insert(&missing_objects, &obj->oid);\n \t\treturn;\n \n+\tcase MA_ALLOW_PROMISOR:\n+\t\tif (is_promisor_object(&obj->oid))\n+\t\t\treturn;\n+\t\tdie(\"unexpected missing blob object '%s'\",\n+\t\t    oid_to_hex(&obj->oid));\n+\t\treturn;\n+\n \tdefault:\n \t\tBUG(\"unhandled missing_action\");\n \t\treturn;\n \t}\n }\n \n-static void finish_object(struct object *obj, const char *name, void *cb_data)\n+static int finish_object(struct object *obj, const char *name, void *cb_data)\n {\n \tstruct rev_list_info *info = cb_data;\n-\tif (obj->type == OBJ_BLOB && !has_object_file(&obj->oid))\n+\tif (obj->type == OBJ_BLOB && !has_object_file(&obj->oid)) {\n \t\tfinish_object__ma(obj);\n+\t\treturn 1;\n+\t}\n \tif (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n \t\tparse_object(&obj->oid);\n+\treturn 0;\n }\n \n static void show_object(struct object *obj, const char *name, void *cb_data)\n {\n \tstruct rev_list_info *info = cb_data;\n-\tfinish_object(obj, name, cb_data);\n+\tif (finish_object(obj, name, cb_data))\n+\t\treturn;\n \tdisplay_progress(progress, ++progress_counter);\n \tif (info->flags & REV_LIST_QUIET)\n \t\treturn;\n@@ -315,11 +334,19 @@ static inline int parse_missing_action_value(const char *value)\n \n \tif (!strcmp(value, \"allow-any\")) {\n \t\targ_missing_action = MA_ALLOW_ANY;\n+\t\tfetch_if_missing = 0;\n \t\treturn 1;\n \t}\n \n \tif (!strcmp(value, \"print\")) {\n \t\targ_missing_action = MA_PRINT;\n+\t\tfetch_if_missing = 0;\n+\t\treturn 1;\n+\t}\n+\n+\tif (!strcmp(value, \"allow-promisor\")) {\n+\t\targ_missing_action = MA_ALLOW_PROMISOR;\n+\t\tfetch_if_missing = 0;\n \t\treturn 1;\n \t}\n \n@@ -344,6 +371,38 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \tinit_revisions(&revs, prefix);\n \trevs.abbrev = DEFAULT_ABBREV;\n \trevs.commit_format = CMIT_FMT_UNSPECIFIED;\n+\n+\t/*\n+\t * Scan the argument list before invoking setup_revisions(), so that we\n+\t * know if fetch_if_missing needs to be set to 0.\n+\t *\n+\t * \"--exclude-promisor-objects\" acts as a pre-filter on missing objects\n+\t * by not crossing the boundary from realized objects to promisor\n+\t * objects.  This overrides any \"--missing\" value.  For consistency in\n+\t * our variables, we force MA_ALLOW_PROMISOR knowning that list-objects\n+\t * should not touch other missing objects unless there is an error.\n+\t *\n+\t * Otherwise, let \"--missing\" to conditionally set fetch_if_missing.\n+\t */\n+\tfor (i = 1; i < argc; i++) {\n+\t\tconst char *arg = argv[i];\n+\t\tif (!strcmp(arg, \"--exclude-promisor-objects\")) {\n+\t\t\tfetch_if_missing = 0;\n+\t\t\trevs.exclude_promisor_objects = 1;\n+\t\t\targ_missing_action = MA_ALLOW_PROMISOR;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\tfor (i = 1; i < argc; i++) {\n+\t\tconst char *arg = argv[i];\n+\t\tif (skip_prefix(arg, \"--missing=\", &arg)) {\n+\t\t\tif (revs.exclude_promisor_objects)\n+\t\t\t\tdie(_(\"cannot combine --exclude-promisor-objects and --missing\"));\n+\t\t\tif (parse_missing_action_value(arg))\n+\t\t\t\tbreak;\n+\t\t}\n+\t}\n+\n \targc = setup_revisions(argc, argv, &revs, NULL);\n \n \tmemset(&info, 0, sizeof(info));\n@@ -408,10 +467,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \n-\t\tif (skip_prefix(arg, \"--missing=\", &arg) &&\n-\t\t    parse_missing_action_value(arg))\n-\t\t\tcontinue;\n-\t\t\n+\t\tif (!strcmp(arg, \"--exclude-promisor-objects\"))\n+\t\t\tcontinue; /* already handled above */\n+\t\tif (skip_prefix(arg, \"--missing=\", &arg))\n+\t\t\tcontinue; /* already handled above */\n+\n \t\tusage(rev_list_usage);\n \n \t}\ndiff --git a/list-objects.c b/list-objects.c\nindex d9e83d0..58621fc 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -9,6 +9,7 @@\n #include \"list-objects.h\"\n #include \"list-objects-filter.h\"\n #include \"list-objects-filter-options.h\"\n+#include \"packfile.h\"\n \n static void process_blob(struct rev_info *revs,\n \t\t\t struct blob *blob,\n@@ -30,6 +31,20 @@ static void process_blob(struct rev_info *revs,\n \tif (obj->flags & (UNINTERESTING | SEEN))\n \t\treturn;\n \n+\t/*\n+\t * Pre-filter known-missing objects when explicitly requested.\n+\t * Otherwise, a missing object error message may be reported\n+\t * later (depending on other filtering criteria).\n+\t *\n+\t * Note that this \"--exclude-promisor-objects\" pre-filtering\n+\t * may cause the actual filter to report an incomplete list\n+\t * of missing objects.\n+\t */\n+\tif (revs->exclude_promisor_objects &&\n+\t    !has_object_file(&obj->oid) &&\n+\t    is_promisor_object(&obj->oid))\n+\t\treturn;\n+\n \tpathlen = path->len;\n \tstrbuf_addstr(path, name);\n \tif (filter_fn)\n@@ -91,6 +106,8 @@ static void process_tree(struct rev_info *revs,\n \t\tall_entries_interesting: entry_not_interesting;\n \tint baselen = base->len;\n \tenum list_objects_filter_result r = LOFR_MARK_SEEN | LOFR_DO_SHOW;\n+\tint gently = revs->ignore_missing_links ||\n+\t\t     revs->exclude_promisor_objects;\n \n \tif (!revs->tree_objects)\n \t\treturn;\n@@ -98,9 +115,19 @@ static void process_tree(struct rev_info *revs,\n \t\tdie(\"bad tree object\");\n \tif (obj->flags & (UNINTERESTING | SEEN))\n \t\treturn;\n-\tif (parse_tree_gently(tree, revs->ignore_missing_links) < 0) {\n+\tif (parse_tree_gently(tree, gently) < 0) {\n \t\tif (revs->ignore_missing_links)\n \t\t\treturn;\n+\n+\t\t/*\n+\t\t * Pre-filter known-missing tree objects when explicitly\n+\t\t * requested.  This may cause the actual filter to report\n+\t\t * an incomplete list of missing objects.\n+\t\t */\n+\t\tif (revs->exclude_promisor_objects &&\n+\t\t    is_promisor_object(&obj->oid))\n+\t\t\treturn;\n+\n \t\tdie(\"bad tree object %s\", oid_to_hex(&obj->oid));\n \t}\n \ndiff --git a/object.c b/object.c\nindex b9a4a0e..4c222d6 100644\n--- a/object.c\n+++ b/object.c\n@@ -252,7 +252,7 @@ struct object *parse_object(const struct object_id *oid)\n \tif (obj && obj->parsed)\n \t\treturn obj;\n \n-\tif ((obj && obj->type == OBJ_BLOB) ||\n+\tif ((obj && obj->type == OBJ_BLOB && has_object_file(oid)) ||\n \t    (!obj && has_object_file(oid) &&\n \t     sha1_object_info(oid->hash, NULL) == OBJ_BLOB)) {\n \t\tif (check_sha1_signature(repl, NULL, 0, NULL) < 0) {\ndiff --git a/revision.c b/revision.c\nindex d167223..05a7aac 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -198,6 +198,8 @@ static struct object *get_reference(struct rev_info *revs, const char *name,\n \tif (!object) {\n \t\tif (revs->ignore_missing)\n \t\t\treturn object;\n+\t\tif (revs->exclude_promisor_objects && is_promisor_object(oid))\n+\t\t\treturn NULL;\n \t\tdie(\"bad object %s\", name);\n \t}\n \tobject->flags |= flags;\n@@ -790,9 +792,17 @@ static int add_parents_to_list(struct rev_info *revs, struct commit *commit,\n \n \tfor (parent = commit->parents; parent; parent = parent->next) {\n \t\tstruct commit *p = parent->item;\n-\n-\t\tif (parse_commit_gently(p, revs->ignore_missing_links) < 0)\n+\t\tint gently = revs->ignore_missing_links ||\n+\t\t\t     revs->exclude_promisor_objects;\n+\t\tif (parse_commit_gently(p, gently) < 0) {\n+\t\t\tif (revs->exclude_promisor_objects &&\n+\t\t\t    is_promisor_object(&p->object.oid)) {\n+\t\t\t\tif (revs->first_parent_only)\n+\t\t\t\t\tbreak;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\treturn -1;\n+\t\t}\n \t\tif (revs->show_source && !p->util)\n \t\t\tp->util = commit->util;\n \t\tp->object.flags |= left_flag;\n@@ -2088,6 +2098,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->limited = 1;\n \t} else if (!strcmp(arg, \"--ignore-missing\")) {\n \t\trevs->ignore_missing = 1;\n+\t} else if (!strcmp(arg, \"--exclude-promisor-objects\")) {\n+\t\tif (fetch_if_missing)\n+\t\t\tdie(\"BUG: exclude_promisor_objects can only be used when fetch_if_missing is 0\");\n+\t\trevs->exclude_promisor_objects = 1;\n \t} else {\n \t\tint opts = diff_opt_parse(&revs->diffopt, argv, argc, revs->prefix);\n \t\tif (!opts)\n@@ -2830,6 +2844,16 @@ void reset_revision_walk(void)\n \tclear_object_flags(SEEN | ADDED | SHOWN);\n }\n \n+static int mark_uninteresting(const struct object_id *oid,\n+\t\t\t      struct packed_git *pack,\n+\t\t\t      uint32_t pos,\n+\t\t\t      void *unused)\n+{\n+\tstruct object *o = parse_object(oid);\n+\to->flags |= UNINTERESTING | SEEN;\n+\treturn 0;\n+}\n+\n int prepare_revision_walk(struct rev_info *revs)\n {\n \tint i;\n@@ -2858,6 +2882,11 @@ int prepare_revision_walk(struct rev_info *revs)\n \t    (revs->limited && limiting_can_increase_treesame(revs)))\n \t\trevs->treesame.name = \"treesame\";\n \n+\tif (revs->exclude_promisor_objects) {\n+\t\tfor_each_packed_object(mark_uninteresting, NULL,\n+\t\t\t\t       FOR_EACH_OBJECT_PROMISOR_ONLY);\n+\t}\n+\n \tif (revs->no_walk != REVISION_WALK_NO_WALK_UNSORTED)\n \t\tcommit_list_sort_by_date(&revs->commits);\n \tif (revs->no_walk)\ndiff --git a/revision.h b/revision.h\nindex 5476120..5f9a49c 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -121,7 +121,10 @@ struct rev_info {\n \t\t\tbisect:1,\n \t\t\tancestry_path:1,\n \t\t\tfirst_parent_only:1,\n-\t\t\tline_level_traverse:1;\n+\t\t\tline_level_traverse:1,\n+\n+\t\t\t/* for internal use only */\n+\t\t\texclude_promisor_objects:1;\n \n \t/* Diff flags */\n \tunsigned int\tdiff:1,\ndiff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh\nindex 8a90f6a..3ca6af5 100755\n--- a/t/t0410-partial-clone.sh\n+++ b/t/t0410-partial-clone.sh\n@@ -160,6 +160,107 @@ test_expect_success 'fetching of missing objects' '\n \tgit verify-pack --verbose \"$IDX\" | grep \"$HASH\"\n '\n \n+test_expect_success 'rev-list stops traversal at missing and promised commit' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo foo &&\n+\ttest_commit -C repo bar &&\n+\n+\tFOO=$(git -C repo rev-parse foo) &&\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 rev-list --exclude-promisor-objects --objects bar >out &&\n+\tgrep $(git -C repo rev-parse bar) out &&\n+\t! grep $FOO out\n+'\n+\n+test_expect_success 'rev-list stops traversal at missing and promised tree' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo foo &&\n+\tmkdir repo/a_dir &&\n+\techo something >repo/a_dir/something &&\n+\tgit -C repo add a_dir/something &&\n+\tgit -C repo commit -m bar &&\n+\n+\t# foo^{tree} (tree referenced from commit)\n+\tTREE=$(git -C repo rev-parse foo^{tree}) &&\n+\n+\t# a tree referenced by HEAD^{tree} (tree referenced from tree)\n+\tTREE2=$(git -C repo ls-tree HEAD^{tree} | grep \" tree \" | head -1 | cut -b13-52) &&\n+\n+\tpromise_and_delete \"$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 rev-list --exclude-promisor-objects --objects HEAD >out &&\n+\tgrep $(git -C repo rev-parse foo) out &&\n+\t! grep $TREE out &&\n+\tgrep $(git -C repo rev-parse HEAD) out &&\n+\t! grep $TREE2 out\n+'\n+\n+test_expect_success 'rev-list stops traversal at missing and promised blob' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\techo something >repo/something &&\n+\tgit -C repo add something &&\n+\tgit -C repo commit -m foo &&\n+\n+\tBLOB=$(git -C repo hash-object -w something) &&\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 rev-list --exclude-promisor-objects --objects HEAD >out &&\n+\tgrep $(git -C repo rev-parse HEAD) out &&\n+\t! grep $BLOB out\n+'\n+\n+test_expect_success 'rev-list stops traversal at promisor commit, tree, and blob' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo foo &&\n+\ttest_commit -C repo bar &&\n+\ttest_commit -C repo baz &&\n+\n+\tCOMMIT=$(git -C repo rev-parse foo) &&\n+\tTREE=$(git -C repo rev-parse bar^{tree}) &&\n+\tBLOB=$(git hash-object repo/baz.t) &&\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 rev-list --exclude-promisor-objects --objects HEAD >out &&\n+\t! grep $COMMIT out &&\n+\t! grep $TREE out &&\n+\t! grep $BLOB out &&\n+\tgrep $(git -C repo rev-parse bar) out  # sanity check that some walking was done\n+'\n+\n+test_expect_success 'rev-list accepts missing and promised objects on command line' '\n+\trm -rf repo &&\n+\ttest_create_repo repo &&\n+\ttest_commit -C repo foo &&\n+\ttest_commit -C repo bar &&\n+\ttest_commit -C repo baz &&\n+\n+\tCOMMIT=$(git -C repo rev-parse foo) &&\n+\tTREE=$(git -C repo rev-parse bar^{tree}) &&\n+\tBLOB=$(git hash-object repo/baz.t) &&\n+\n+\tpromise_and_delete $COMMIT &&\n+\tpromise_and_delete $TREE &&\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 rev-list --exclude-promisor-objects --objects \"$COMMIT\" \"$TREE\" \"$BLOB\"\n+'\n+\n LIB_HTTPD_PORT=12345  # default port, 410, cannot be used as non-root\n . \"$TEST_DIRECTORY\"/lib-httpd.sh\n start_httpd\n-- \n2.9.3\n\n"},{"id":"332699","messageId":"20171116181257.61673-2-git@jeffhostetler.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"[PATCH v4 01/10] extension.partialclone: introduce partial clone extension","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:48Z","receivedAt":"2017-11-16T18:13:58Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jonathan Tan <jonathantanmy@google.com>\n\nIntroduce new repository extension option:\n    `extensions.partialclone`\n\nSee the update to Documentation/technical/repository-version.txt\nin this patch for more information.\n\nSigned-off-by: Jonathan Tan <jonathantanmy@google.com>\n---\n Documentation/technical/repository-version.txt | 12 ++++++++++++\n cache.h                                        |  2 ++\n environment.c                                  |  1 +\n setup.c                                        |  7 ++++++-\n 4 files changed, 21 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/technical/repository-version.txt b/Documentation/technical/repository-version.txt\nindex 00ad379..e03eacc 100644\n--- a/Documentation/technical/repository-version.txt\n+++ b/Documentation/technical/repository-version.txt\n@@ -86,3 +86,15 @@ for testing format-1 compatibility.\n When the config key `extensions.preciousObjects` is set to `true`,\n objects in the repository MUST NOT be deleted (e.g., by `git-prune` or\n `git repack -d`).\n+\n+`partialclone`\n+~~~~~~~~~~~~~~\n+\n+When the config key `extensions.partialclone` is set, it indicates\n+that the repo was created with a partial clone (or later performed\n+a partial fetch) and that the remote may have omitted sending\n+certain unwanted objects.  Such a remote is called a \"promisor remote\"\n+and it promises that all such omitted objects can be fetched from it\n+in the future.\n+\n+The value of this key is the name of the promisor remote.\ndiff --git a/cache.h b/cache.h\nindex 6440e2b..35e3f5e 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -860,10 +860,12 @@ 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 \n struct repository_format {\n \tint version;\n \tint precious_objects;\n+\tchar *partial_clone; /* value of extensions.partialclone */\n \tint is_bare;\n \tchar *work_tree;\n \tstruct string_list unknown_extensions;\ndiff --git a/environment.c b/environment.c\nindex 8289c25..e52aab3 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -27,6 +27,7 @@ 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 *git_commit_encoding;\n const char *git_log_output_encoding;\n const char *apply_default_whitespace;\ndiff --git a/setup.c b/setup.c\nindex 03f51e0..58536bd 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -420,7 +420,11 @@ 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\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\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@@ -463,6 +467,7 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)\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) {\n-- \n2.9.3\n\n"},{"id":"332700","messageId":"20171116181257.61673-1-git@jeffhostetler.com","threadId":"47224","inReplyTo":null,"subject":"[PATCH v4 00/10] Partial clone part 2: fsck and promisors","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-16T18:12:47Z","receivedAt":"2017-11-16T18:14:11Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"From: Jeff Hostetler <jeffhost@microsoft.com>\n\nThis is part 2 of a 3 part sequence for partial clone.\nPart 2 assumes part 1 is in place.\n\nThis patch series is labeled V4 to keep it in sync with the\nV4 version of part 1.  (There was no V3 of this part.)\n\nPart 2 is concerned with fsck, gc, initial support for dynamic\nobject fetching, and tracking promisor objects.  Jonathan Tan\noriginally developed this code.  I have moved it on top of\npart 1 and updated it slightly.\n\nJonathan Tan (10):\n  extension.partialclone: introduce partial clone extension\n  fsck: introduce partialclone extension\n  fsck: support refs pointing to promisor objects\n  fsck: support referenced promisor objects\n  fsck: support promisor objects as CLI argument\n  index-pack: refactor writing of .keep files\n  introduce fetch-object: fetch one promisor object\n  sha1_file: support lazily fetching missing objects\n  rev-list: support termination at promisor objects\n  gc: do not repack promisor packfiles\n\n Documentation/git-pack-objects.txt             |  12 +-\n Documentation/gitremote-helpers.txt            |   6 +\n Documentation/rev-list-options.txt             |  12 +-\n Documentation/technical/repository-version.txt |  12 +\n Makefile                                       |   1 +\n builtin/cat-file.c                             |   2 +\n builtin/fetch-pack.c                           |  10 +\n builtin/fsck.c                                 |  26 +-\n builtin/gc.c                                   |   3 +\n builtin/index-pack.c                           | 113 ++++----\n builtin/pack-objects.c                         |  36 +++\n builtin/prune.c                                |   7 +\n builtin/repack.c                               |   8 +-\n builtin/rev-list.c                             |  74 +++++-\n cache.h                                        |  13 +-\n environment.c                                  |   1 +\n fetch-object.c                                 |  26 ++\n fetch-object.h                                 |   6 +\n fetch-pack.c                                   |   8 +-\n fetch-pack.h                                   |   2 +\n list-objects.c                                 |  29 ++-\n object.c                                       |   2 +-\n packfile.c                                     |  77 +++++-\n packfile.h                                     |  13 +\n remote-curl.c                                  |  14 +-\n revision.c                                     |  33 ++-\n revision.h                                     |   5 +-\n setup.c                                        |   7 +-\n sha1_file.c                                    |  38 ++-\n t/t0410-partial-clone.sh                       | 343 +++++++++++++++++++++++++\n transport.c                                    |   8 +\n transport.h                                    |   8 +\n 32 files changed, 872 insertions(+), 83 deletions(-)\n create mode 100644 fetch-object.c\n create mode 100644 fetch-object.h\n create mode 100755 t/t0410-partial-clone.sh\n\n-- \n2.9.3\n\n"},{"id":"332719","messageId":"e7b5e42f-b13c-50c8-501c-0100d8c48d00@ramsayjones.plus.com","threadId":"47224","inReplyTo":"20171116181257.61673-8-git@jeffhostetler.com","subject":"Re: [PATCH v4 07/10] introduce fetch-object: fetch one promisor object","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-11-16T19:57:19Z","receivedAt":"2017-11-16T19:57:28Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 16/11/17 18:12, Jeff Hostetler wrote:\n> From: Jonathan Tan <jonathantanmy@google.com>\n> \n> Introduce fetch-object, providing the ability to fetch one object from a\n> promisor remote.\n> \n> This uses fetch-pack. To do this, the transport mechanism has been\n> updated with 2 flags, \"from-promisor\" to indicate that the resulting\n> pack comes from a promisor remote (and thus should be annotated as such\n> by index-pack), and \"no-haves\" to suppress the sending of \"have\" lines.\n> \n> This will be tested in a subsequent commit.\n> \n> NEEDSWORK: update this when we have more information about protocol v2,\n> which should allow a way to suppress the ref advertisement and\n> officially allow any object type to be \"want\"-ed.\n> \n> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>\n> ---\n>  Documentation/gitremote-helpers.txt |  6 ++++++\n>  Makefile                            |  1 +\n>  builtin/fetch-pack.c                |  8 ++++++++\n>  builtin/index-pack.c                | 16 +++++++++++++---\n>  fetch-object.c                      | 23 +++++++++++++++++++++++\n>  fetch-object.h                      |  6 ++++++\n>  fetch-pack.c                        |  8 ++++++--\n>  fetch-pack.h                        |  2 ++\n>  remote-curl.c                       | 14 +++++++++++++-\n>  transport.c                         |  8 ++++++++\n>  transport.h                         |  8 ++++++++\n>  11 files changed, 94 insertions(+), 6 deletions(-)\n>  create mode 100644 fetch-object.c\n>  create mode 100644 fetch-object.h\n> \n[snip]\n> diff --git a/fetch-object.c b/fetch-object.c\n> new file mode 100644\n> index 0000000..f89dbba\n> --- /dev/null\n> +++ b/fetch-object.c\n> @@ -0,0 +1,23 @@\n> +#include \"cache.h\"\n> +#include \"packfile.h\"\n> +#include \"pkt-line.h\"\n> +#include \"strbuf.h\"\n> +#include \"transport.h\"\n\nI note that this still does not #include \"fetch_object.h\".\n[If you recall, this suppresses a sparse warning].\n\n> +\n> +void fetch_object(const char *remote_name, const unsigned char *sha1)\n> +{\n> +\tstruct remote *remote;\n> +\tstruct transport *transport;\n> +\tstruct ref *ref;\n> +\n> +\tremote = remote_get(remote_name);\n> +\tif (!remote->url[0])\n> +\t\tdie(_(\"Remote with no URL\"));\n> +\ttransport = transport_get(remote, remote->url[0]);\n> +\n> +\tref = alloc_ref(sha1_to_hex(sha1));\n> +\thashcpy(ref->old_oid.hash, sha1);\n> +\ttransport_set_option(transport, TRANS_OPT_FROM_PROMISOR, \"1\");\n> +\ttransport_set_option(transport, TRANS_OPT_NO_HAVES, \"1\");\n> +\ttransport_fetch_refs(transport, ref);\n> +}\n> diff --git a/fetch-object.h b/fetch-object.h\n> new file mode 100644\n> index 0000000..f371300\n> --- /dev/null\n> +++ b/fetch-object.h\n> @@ -0,0 +1,6 @@\n> +#ifndef FETCH_OBJECT_H\n> +#define FETCH_OBJECT_H\n> +\n> +extern void fetch_object(const char *remote_name, const unsigned char *sha1);\n> +\n> +#endif\n\nATB,\nRamsay Jones\n\n\n"},{"id":"332726","messageId":"20171116135715.7d4650f3365a678399397d01@google.com","threadId":"47224","inReplyTo":"20171116181257.61673-1-git@jeffhostetler.com","subject":"Re: [PATCH v4 00/10] Partial clone part 2: fsck and promisors","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-11-16T21:57:15Z","receivedAt":"2017-11-16T21:57:24Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"I patched both this series and the first 9 patches of mine [1] on part 1\nof the entire partial clone implementation [2], and then\ndiffed them. I'll review just the differences between the two.\n\nYou can see the entire diff below (minus means in my patch set but not\nin Jeff's, plus means the contrary) - I did not trim it.\n\n[1] https://public-inbox.org/git/cover.1506714999.git.jonathantanmy@google.com/\n[2] https://public-inbox.org/git/20171116180743.61353-1-git@jeffhostetler.com/T/#t\n\n> diff --git a/Documentation/git-pack-objects.txt b/Documentation/git-pack-objects.txt\n> index 5fad696c5..33a824eed 100644\n> --- a/Documentation/git-pack-objects.txt\n> +++ b/Documentation/git-pack-objects.txt\n> @@ -242,9 +242,19 @@ So does `git bundle` (see linkgit:git-bundle[1]) when it creates a bundle.\n>  \tthe resulting packfile.  See linkgit:git-rev-list[1] for valid\n>  \t`<filter-spec>` forms.\n>  \n> ---missing=(error|allow-any):\n> +--missing=(error|allow-any|allow-promisor):\n>  \tSpecifies how missing objects are handled.  This is useful, for\n>  \texample, when there are missing objects from a prior partial clone.\n> +\tThis is stronger than `--missing=allow-promisor` because it limits\n> +\tthe traversal, rather than just silencing errors about missing\n> +\tobjects.\n\nWhat is stronger than `--missing=allow-promisor`?\n\n> +\n> +--exclude-promisor-objects::\n> +\tOmit objects that are known to be in the promisor remote\". (This\n\nStray quote.\n\n> +\toption has the purpose of operating only on locally created objects,\n> +\tso that when we repack, we still maintain a distinction between\n> +\tlocally created objects [without .promisor] and objects from the\n> +\tpromisor remote [with .promisor].)  This is used with partial clone.\n>  \n>  SEE ALSO\n>  --------\n> diff --git a/Documentation/gitremote-helpers.txt b/Documentation/gitremote-helpers.txt\n> index 4a584f3c5..1ceab8944 100644\n> --- a/Documentation/gitremote-helpers.txt\n> +++ b/Documentation/gitremote-helpers.txt\n> @@ -466,6 +466,12 @@ set by Git if the remote helper has the 'option' capability.\n>  \tTransmit <string> as a push option. As the push option\n>  \tmust not contain LF or NUL characters, the string is not encoded.\n>  \n> +'option from-promisor' {'true'|'false'}::\n> +\tIndicate that these objects are being fetch by a promisor.\n\nShould be: ...are being fetched from a promisor.\n\n> +\n> +'option no-haves' {'true'|'false'}::\n> +\tDo not send \"have\" lines.\n> +\n>  SEE ALSO\n>  --------\n>  linkgit:git-remote[1]\n> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\n> index c84e46522..2beffe320 100644\n> --- a/Documentation/rev-list-options.txt\n> +++ b/Documentation/rev-list-options.txt\n> @@ -730,7 +730,7 @@ specification contained in <path>.\n>  \tOnly useful with `--filter=`; prints a list of the omitted objects.\n>  \tObject IDs are prefixed with a ``~'' character.\n>  \n> ---missing=(error|allow-any|print)::\n> +--missing=(error|allow-any|allow-promisor|print)::\n>  \tSpecifies how missing objects are handled.  The repository may\n>  \thave missing objects after a partial clone, for example.\n>  +\n> @@ -741,10 +741,20 @@ The value 'allow-any' will allow object traversal to continue if a\n>  missing object is encountered.  Missing objects will silently be omitted\n>  from the results.\n>  +\n> +The value 'allow-promisor' is like 'allow-any' in that it will allow\n> +object traversal to continue, but only for EXPECTED missing objects.\n> ++\n>  The value 'print' is like 'allow-any', but will also print a list of the\n>  missing objects.  Object IDs are prefixed with a ``?'' character.\n>  endif::git-rev-list[]\n>  \n> +--exclude-promisor-objects::\n> +\t(For internal use only.)  Prefilter object traversal at\n> +\tpromisor boundary.  This is used with partial clone.  This is\n> +\tstronger than `--missing=allow-promisor` because it limits the\n> +\ttraversal, rather than just silencing errors about missing\n> +\tobjects.\n> +\n>  --no-walk[=(sorted|unsorted)]::\n>  \tOnly show the given commits, but do not traverse their ancestors.\n>  \tThis has no effect if a range is specified. If the argument\n> diff --git a/Documentation/technical/repository-version.txt b/Documentation/technical/repository-version.txt\n> index 074ccb9e0..e03eacceb 100644\n> --- a/Documentation/technical/repository-version.txt\n> +++ b/Documentation/technical/repository-version.txt\n> @@ -87,14 +87,14 @@ When the config key `extensions.preciousObjects` is set to `true`,\n>  objects in the repository MUST NOT be deleted (e.g., by `git-prune` or\n>  `git repack -d`).\n>  \n> -`partialClone`\n> +`partialclone`\n>  ~~~~~~~~~~~~~~\n>  \n> -When the config key `extensions.partialClone` is set, a remote is\n> -designated as a \"promisor remote\". Objects referenced by packed objects\n> -obtained from that promisor remote do not need to be in the local repo.\n> -Instead, the promisor remote promises that all such objects can be\n> -fetched from it in the future, and Git will fetch those objects when\n> -needed.\n> +When the config key `extensions.partialclone` is set, it indicates\n> +that the repo was created with a partial clone (or later performed\n> +a partial fetch) and that the remote may have omitted sending\n> +certain unwanted objects.  Such a remote is called a \"promisor remote\"\n> +and it promises that all such omitted objects can be fetched from it\n> +in the future.\n>  \n> -The value of this key is the name of the aforementioned promisor remote.\n> +The value of this key is the name of the promisor remote.\n> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\n> index f31f92a8e..32e92dbeb 100644\n> --- a/builtin/pack-objects.c\n> +++ b/builtin/pack-objects.c\n> @@ -88,6 +88,7 @@ static struct list_objects_filter_options filter_options;\n>  enum missing_action {\n>  \tMA_ERROR = 0,    /* fail if any missing objects are encountered */\n>  \tMA_ALLOW_ANY,    /* silently allow ALL missing objects */\n> +\tMA_ALLOW_PROMISOR, /* silently allow all missing PROMISOR objects */\n>  };\n>  static enum missing_action arg_missing_action;\n>  static show_object_fn fn_show_object;\n> @@ -2580,6 +2581,20 @@ static void show_object__ma_allow_any(struct object *obj, const char *name, void\n>  \tshow_object(obj, name, data);\n>  }\n>  \n> +static void show_object__ma_allow_promisor(struct object *obj, const char *name, void *data)\n> +{\n> +\tassert(arg_missing_action == MA_ALLOW_PROMISOR);\n> +\n> +\t/*\n> +\t * Quietly ignore EXPECTED missing objects.  This avoids problems with\n> +\t * staging them now and getting an odd error later.\n> +\t */\n> +\tif (!has_object_file(&obj->oid) && is_promisor_object(&obj->oid))\n> +\t\treturn;\n> +\n> +\tshow_object(obj, name, data);\n> +}\n> +\n>  static int option_parse_missing_action(const struct option *opt,\n>  \t\t\t\t       const char *arg, int unset)\n>  {\n> @@ -2594,10 +2609,18 @@ static int option_parse_missing_action(const struct option *opt,\n>  \n>  \tif (!strcmp(arg, \"allow-any\")) {\n>  \t\targ_missing_action = MA_ALLOW_ANY;\n> +\t\tfetch_if_missing = 0;\n>  \t\tfn_show_object = show_object__ma_allow_any;\n>  \t\treturn 0;\n>  \t}\n>  \n> +\tif (!strcmp(arg, \"allow-promisor\")) {\n> +\t\targ_missing_action = MA_ALLOW_PROMISOR;\n> +\t\tfetch_if_missing = 0;\n> +\t\tfn_show_object = show_object__ma_allow_promisor;\n> +\t\treturn 0;\n> +\t}\n> +\n>  \tdie(_(\"invalid value for --missing\"));\n>  \treturn 0;\n>  }\n> @@ -3062,6 +3085,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)\n>  \t\tuse_internal_rev_list = 1;\n>  \t\tfetch_if_missing = 0;\n>  \t\targv_array_push(&rp, \"--exclude-promisor-objects\");\n> +\t\t/* silently override any \"--missing=\" value */\n> +\t\targ_missing_action = MA_ALLOW_PROMISOR;\n> +\t\tfn_show_object = show_object__ma_allow_promisor;\n\n--exclude-promisor-objects should be orthogonal to handling of missing\nobjects (and the tests pass when I remove these lines, showing that to\nbe true in practice).\n\nIf mixing the two is problematic from a UI standpoint, maybe it's better\nto warn or die about them. But probably the best thing to do is to\ndocument --exclude-promisor-objects as \"internal use only\".\n\n>  \t}\n>  \n>  \tif (!reuse_object)\n> diff --git a/builtin/repack.c b/builtin/repack.c\n> index f43317bb5..7bdb40142 100644\n> --- a/builtin/repack.c\n> +++ b/builtin/repack.c\n> @@ -234,7 +234,8 @@ 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> -\targv_array_push(&cmd.args, \"--exclude-promisor-objects\");\n> +\tif (repository_format_partial_clone)\n> +\t\targv_array_push(&cmd.args, \"--exclude-promisor-objects\");\n>  \tif (window)\n>  \t\targv_array_pushf(&cmd.args, \"--window=%s\", window);\n>  \tif (window_memory)\n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index e17ab2b0f..5c5acd3d6 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -68,6 +68,7 @@ enum missing_action {\n>  \tMA_ERROR = 0,    /* fail if any missing objects are encountered */\n>  \tMA_ALLOW_ANY,    /* silently allow ALL missing objects */\n>  \tMA_PRINT,        /* print ALL missing objects in special section */\n> +\tMA_ALLOW_PROMISOR, /* silently allow all missing PROMISOR objects */\n>  };\n>  static enum missing_action arg_missing_action;\n>  \n> @@ -198,6 +199,12 @@ static void finish_commit(struct commit *commit, void *data)\n>  \n>  static inline void finish_object__ma(struct object *obj)\n>  {\n> +\t/*\n> +\t * Whether or not we try to dynamically fetch missing objects\n> +\t * from the server, we currently DO NOT have the object.  We\n> +\t * can either print, allow (ignore), or conditionally allow\n> +\t * (ignore) them.\n> +\t */\n>  \tswitch (arg_missing_action) {\n>  \tcase MA_ERROR:\n>  \t\tdie(\"missing blob object '%s'\", oid_to_hex(&obj->oid));\n> @@ -210,25 +217,36 @@ static inline void finish_object__ma(struct object *obj)\n>  \t\toidset_insert(&missing_objects, &obj->oid);\n>  \t\treturn;\n>  \n> +\tcase MA_ALLOW_PROMISOR:\n> +\t\tif (is_promisor_object(&obj->oid))\n> +\t\t\treturn;\n> +\t\tdie(\"unexpected missing blob object '%s'\",\n> +\t\t    oid_to_hex(&obj->oid));\n> +\t\treturn;\n> +\n>  \tdefault:\n>  \t\tBUG(\"unhandled missing_action\");\n>  \t\treturn;\n>  \t}\n>  }\n>  \n> -static void finish_object(struct object *obj, const char *name, void *cb_data)\n> +static int finish_object(struct object *obj, const char *name, void *cb_data)\n>  {\n>  \tstruct rev_list_info *info = cb_data;\n> -\tif (obj->type == OBJ_BLOB && !has_object_file(&obj->oid))\n> +\tif (obj->type == OBJ_BLOB && !has_object_file(&obj->oid)) {\n>  \t\tfinish_object__ma(obj);\n> +\t\treturn 1;\n> +\t}\n>  \tif (info->revs->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n>  \t\tparse_object(&obj->oid);\n> +\treturn 0;\n>  }\n\nAs I said in [3], making finish_object() return int makes sense, because\nwe might want to suppress the object from being shown. (My version of\nthe code was not designed with the --missing parameter in mind.)\n\n[3] https://public-inbox.org/git/20171108135116.f96c58500caa302583bb2810@google.com/\n\n>  \n>  static void show_object(struct object *obj, const char *name, void *cb_data)\n>  {\n>  \tstruct rev_list_info *info = cb_data;\n> -\tfinish_object(obj, name, cb_data);\n> +\tif (finish_object(obj, name, cb_data))\n> +\t\treturn;\n>  \tdisplay_progress(progress, ++progress_counter);\n>  \tif (info->flags & REV_LIST_QUIET)\n>  \t\treturn;\n> @@ -316,11 +334,19 @@ static inline int parse_missing_action_value(const char *value)\n>  \n>  \tif (!strcmp(value, \"allow-any\")) {\n>  \t\targ_missing_action = MA_ALLOW_ANY;\n> +\t\tfetch_if_missing = 0;\n>  \t\treturn 1;\n>  \t}\n>  \n>  \tif (!strcmp(value, \"print\")) {\n>  \t\targ_missing_action = MA_PRINT;\n> +\t\tfetch_if_missing = 0;\n> +\t\treturn 1;\n> +\t}\n> +\n> +\tif (!strcmp(value, \"allow-promisor\")) {\n> +\t\targ_missing_action = MA_ALLOW_PROMISOR;\n> +\t\tfetch_if_missing = 0;\n>  \t\treturn 1;\n>  \t}\n>  \n> @@ -349,13 +375,33 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n>  \t/*\n>  \t * Scan the argument list before invoking setup_revisions(), so that we\n>  \t * know if fetch_if_missing needs to be set to 0.\n> +\t *\n> +\t * \"--exclude-promisor-objects\" acts as a pre-filter on missing objects\n> +\t * by not crossing the boundary from realized objects to promisor\n> +\t * objects.  This overrides any \"--missing\" value.  For consistency in\n> +\t * our variables, we force MA_ALLOW_PROMISOR knowning that list-objects\n> +\t * should not touch other missing objects unless there is an error.\n> +\t *\n> +\t * Otherwise, let \"--missing\" to conditionally set fetch_if_missing.\n>  \t */\n>  \tfor (i = 1; i < argc; i++) {\n> -\t\tif (!strcmp(argv[i], \"--exclude-promisor-objects\")) {\n> +\t\tconst char *arg = argv[i];\n> +\t\tif (!strcmp(arg, \"--exclude-promisor-objects\")) {\n>  \t\t\tfetch_if_missing = 0;\n> +\t\t\trevs.exclude_promisor_objects = 1;\n> +\t\t\targ_missing_action = MA_ALLOW_PROMISOR;\n>  \t\t\tbreak;\n>  \t\t}\n>  \t}\n> +\tfor (i = 1; i < argc; i++) {\n> +\t\tconst char *arg = argv[i];\n> +\t\tif (skip_prefix(arg, \"--missing=\", &arg)) {\n> +\t\t\tif (revs.exclude_promisor_objects)\n> +\t\t\t\tdie(_(\"cannot combine --exclude-promisor-objects and --missing\"));\n> +\t\t\tif (parse_missing_action_value(arg))\n> +\t\t\t\tbreak;\n> +\t\t}\n> +\t}\n\nSame comment as above about the orthogonality of\n--exclude-promisor-objects and --missing.\n\n>  \n>  \targc = setup_revisions(argc, argv, &revs, NULL);\n>  \n> @@ -421,10 +467,11 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)\n>  \t\t\tcontinue;\n>  \t\t}\n>  \n> -\t\tif (skip_prefix(arg, \"--missing=\", &arg) &&\n> -\t\t    parse_missing_action_value(arg))\n> -\t\t\tcontinue;\n> -\t\t\n> +\t\tif (!strcmp(arg, \"--exclude-promisor-objects\"))\n> +\t\t\tcontinue; /* already handled above */\n> +\t\tif (skip_prefix(arg, \"--missing=\", &arg))\n> +\t\t\tcontinue; /* already handled above */\n> +\n>  \t\tusage(rev_list_usage);\n>  \n>  \t}\n> diff --git a/cache.h b/cache.h\n> index 38ea41efc..05e979d4d 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -889,7 +889,7 @@ extern char *repository_format_partial_clone;\n>  struct repository_format {\n>  \tint version;\n>  \tint precious_objects;\n> -\tchar *partial_clone;\n> +\tchar *partial_clone; /* value of extensions.partialclone */\n>  \tint is_bare;\n>  \tchar *work_tree;\n>  \tstruct string_list unknown_extensions;\n> diff --git a/list-objects.c b/list-objects.c\n> index bdf12c500..58621fc6e 100644\n> --- a/list-objects.c\n> +++ b/list-objects.c\n> @@ -30,13 +30,20 @@ static void process_blob(struct rev_info *revs,\n>  \t\tdie(\"bad blob object\");\n>  \tif (obj->flags & (UNINTERESTING | SEEN))\n>  \t\treturn;\n> -\tif (!has_object_file(&obj->oid)) {\n> -\t\tif (revs->exclude_promisor_objects &&\n> -\t\t    is_promisor_object(&obj->oid)) {\n> -\t\t\treturn;\n> -\t\t}\n> -\t\t/* error message will be reported later */\n> -\t}\n> +\n> +\t/*\n> +\t * Pre-filter known-missing objects when explicitly requested.\n> +\t * Otherwise, a missing object error message may be reported\n> +\t * later (depending on other filtering criteria).\n> +\t *\n> +\t * Note that this \"--exclude-promisor-objects\" pre-filtering\n> +\t * may cause the actual filter to report an incomplete list\n> +\t * of missing objects.\n> +\t */\n> +\tif (revs->exclude_promisor_objects &&\n> +\t    !has_object_file(&obj->oid) &&\n> +\t    is_promisor_object(&obj->oid))\n> +\t\treturn;\n>  \n>  \tpathlen = path->len;\n>  \tstrbuf_addstr(path, name);\n> @@ -111,10 +118,16 @@ static void process_tree(struct rev_info *revs,\n>  \tif (parse_tree_gently(tree, gently) < 0) {\n>  \t\tif (revs->ignore_missing_links)\n>  \t\t\treturn;\n> +\n> +\t\t/*\n> +\t\t * Pre-filter known-missing tree objects when explicitly\n> +\t\t * requested.  This may cause the actual filter to report\n> +\t\t * an incomplete list of missing objects.\n> +\t\t */\n>  \t\tif (revs->exclude_promisor_objects &&\n> -\t\t    is_promisor_object(&obj->oid)) {\n> +\t\t    is_promisor_object(&obj->oid))\n>  \t\t\treturn;\n> -\t\t}\n> +\n\nThe {} here are unnecessary for a one-line block.\n\n>  \t\tdie(\"bad tree object %s\", oid_to_hex(&obj->oid));\n>  \t}\n"},{"id":"332768","messageId":"798096fe-dd01-5351-94d3-949039c84e69@jeffhostetler.com","threadId":"47224","inReplyTo":"e7b5e42f-b13c-50c8-501c-0100d8c48d00@ramsayjones.plus.com","subject":"Re: [PATCH v4 07/10] introduce fetch-object: fetch one promisor object","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-17T19:49:47Z","receivedAt":"2017-11-17T19:49:54Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 11/16/2017 2:57 PM, Ramsay Jones wrote:\n> \n> \n> On 16/11/17 18:12, Jeff Hostetler wrote:\n>> From: Jonathan Tan <jonathantanmy@google.com>\n>>\n>> Introduce fetch-object, providing the ability to fetch one object from a\n>> promisor remote.\n[snip]\n>> +#include \"transport.h\"\n> \n> I note that this still does not #include \"fetch_object.h\".\n> [If you recall, this suppresses a sparse warning].\n> \n\nSorry, I missed that.  I know I did a DEVELOPER=1 build and\nI didn't see a warning, but I'll check again.\n\nThanks,\nJeff\n\n"},{"id":"332770","messageId":"a414c170-3eba-e5b9-6235-c6785129097e@ramsayjones.plus.com","threadId":"47224","inReplyTo":"798096fe-dd01-5351-94d3-949039c84e69@jeffhostetler.com","subject":"Re: [PATCH v4 07/10] introduce fetch-object: fetch one promisor object","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-11-17T20:22:21Z","receivedAt":"2017-11-17T20:22:53Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 17/11/17 19:49, Jeff Hostetler wrote:\n> \n> \n> On 11/16/2017 2:57 PM, Ramsay Jones wrote:\n>>\n>>\n>> On 16/11/17 18:12, Jeff Hostetler wrote:\n>>> From: Jonathan Tan <jonathantanmy@google.com>\n>>>\n>>> Introduce fetch-object, providing the ability to fetch one object from a\n>>> promisor remote.\n> [snip]\n>>> +#include \"transport.h\"\n>>\n>> I note that this still does not #include \"fetch_object.h\".\n>> [If you recall, this suppresses a sparse warning].\n>>\n> \n> Sorry, I missed that.  I know I did a DEVELOPER=1 build and\n> I didn't see a warning, but I'll check again.\n\nUnless you have sparse installed and 'make sparse' as part of\nyour build, you won't see anything. ;-)\n\nHowever, ignore sparse for the moment, if you don't #include\nthe header (interface) file in fetch-object.c, you can't\nexpect the compiler to tell you when there is a mismatch\nbetween the interface and implementation.\n\nThere are some additional sparse warnings associated with\nthese series, ... (hopefully I can find some time tonight)\n\nATB,\nRamsay Jones\n\n\n"},{"id":"332772","messageId":"CAGZ79kZCdxN8h-mJRY9fQy5W3HtqcvFoL1W5jVcb3=A3vvk_Ww@mail.gmail.com","threadId":"47224","inReplyTo":"798096fe-dd01-5351-94d3-949039c84e69@jeffhostetler.com","subject":"Re: [PATCH v4 07/10] introduce fetch-object: fetch one promisor object","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-11-17T20:17:22Z","receivedAt":"2017-11-17T20:50:59Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Nov 17, 2017 at 11:49 AM, Jeff Hostetler <git@jeffhostetler.com> wrote:\n>\n>\n> On 11/16/2017 2:57 PM, Ramsay Jones wrote:\n>>\n>>\n>>\n>> On 16/11/17 18:12, Jeff Hostetler wrote:\n>>>\n>>> From: Jonathan Tan <jonathantanmy@google.com>\n>>>\n>>> Introduce fetch-object, providing the ability to fetch one object from a\n>>> promisor remote.\n>\n> [snip]\n>>>\n>>> +#include \"transport.h\"\n>>\n>>\n>> I note that this still does not #include \"fetch_object.h\".\n>> [If you recall, this suppresses a sparse warning].\n>>\n>\n> Sorry, I missed that.  I know I did a DEVELOPER=1 build and\n> I didn't see a warning, but I'll check again.\n\nsparse is an extra tool, which you have to run/install;\nit is not included in the developer build.\n\nhttps://en.wikipedia.org/wiki/Sparse\n"},{"id":"332913","messageId":"85b615eb-d152-ed7e-779c-8fc3e46d60e1@jeffhostetler.com","threadId":"47224","inReplyTo":"CAGZ79kZCdxN8h-mJRY9fQy5W3HtqcvFoL1W5jVcb3=A3vvk_Ww@mail.gmail.com","subject":"Re: [PATCH v4 07/10] introduce fetch-object: fetch one promisor object","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-11-20T15:56:10Z","receivedAt":"2017-11-20T15:56:17Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 11/17/2017 3:17 PM, Stefan Beller wrote:\n> On Fri, Nov 17, 2017 at 11:49 AM, Jeff Hostetler <git@jeffhostetler.com> wrote:\n>>\n>>\n>> On 11/16/2017 2:57 PM, Ramsay Jones wrote:\n>>>\n>>>\n>>>\n>>> On 16/11/17 18:12, Jeff Hostetler wrote:\n>>>>\n>>>> From: Jonathan Tan <jonathantanmy@google.com>\n>>>>\n>>>> Introduce fetch-object, providing the ability to fetch one object from a\n>>>> promisor remote.\n>>\n>> [snip]\n>>>>\n>>>> +#include \"transport.h\"\n>>>\n>>>\n>>> I note that this still does not #include \"fetch_object.h\".\n>>> [If you recall, this suppresses a sparse warning].\n>>>\n>>\n>> Sorry, I missed that.  I know I did a DEVELOPER=1 build and\n>> I didn't see a warning, but I'll check again.\n> \n> sparse is an extra tool, which you have to run/install;\n> it is not included in the developer build.\n> \n> https://en.wikipedia.org/wiki/Sparse\n> \n\nah, thanks!\nJeff\n"}]}