{"thread":{"id":"48874","subject":"[PATCH 0/2] Annotated tags pointing to missing but promised blobs","startedAt":"2018-07-13T00:03:12Z","lastAt":"2018-07-13T00:03:18Z","messageCount":3,"participants":["Jonathan Tan"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"352443","messageId":"cover.1531438861.git.jonathantanmy@google.com","threadId":"48874","inReplyTo":null,"subject":"[PATCH 0/2] Annotated tags pointing to missing but promised blobs","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-07-13T00:03:05Z","receivedAt":"2018-07-13T00:03:12Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"These are based on jt/partial-clone-fsck-connectivity.\n\nThe patches in jt/partial-clone-fsck-connectivity were motivated by bugs\nI discovered in partial clones when refs pointed to blobs directly.\nWhile continuing to work on this, I noticed issues with annotated tags -\nthat is, refs pointing to tags pointing to blobs. Here are some fixes.\n\nJonathan Tan (2):\n revision: tolerate promised targets of tags\n tag: don't warn if target is missing but promised\n\n revision.c | 3 +++\n t/t5616-partial-clone.sh | 44 ++++++++++++++++++++++++++++++++++++++++\n tag.c | 13 +++++++++---\n 3 files changed, 57 insertions(+), 3 deletions(-)\n\n-- \n2.18.0.203.gfac676dfb9-goog\n\n"},{"id":"352444","messageId":"f010da153f228b59c47deb4b1005df3e0e2255a1.1531438861.git.jonathantanmy@google.com","threadId":"48874","inReplyTo":"cover.1531438861.git.jonathantanmy@google.com","subject":"[PATCH 1/2] revision: tolerate promised targets of tags","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-07-13T00:03:06Z","receivedAt":"2018-07-13T00:03:17Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"In handle_commit(), it is fatal for an annotated tag to point to a\nnon-existent object. --exclude-promisor-objects should relax this rule\nand allow non-existent objects that are promisor objects, but this is\nnot the case. Update handle_commit() to tolerate this situation.\n\nThis was observed when cloning from a repository with an annotated tag\npointing to a blob. The test included in this patch demonstrates this\ncase.\n\nSigned-off-by: Jonathan Tan \n---\n---\n revision.c | 3 +++\n t/t5616-partial-clone.sh | 39 +++++++++++++++++++++++++++++++++++++++\n 2 files changed, 42 insertions(+)\n\ndiff --git a/revision.c b/revision.c\nindex 1b37da988..95546e6d4 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -248,6 +248,9 @@ static struct commit *handle_commit(struct rev_info *revs,\n \t\tif (!object) {\n \t\t\tif (revs->ignore_missing_links || (flags & UNINTERESTING))\n \t\t\t\treturn NULL;\n+\t\t\tif (revs->exclude_promisor_objects &&\n+\t\t\t is_promisor_object(&tag->tagged->oid))\n+\t\t\t\treturn NULL;\n \t\t\tdie(\"bad object %s\", oid_to_hex(&tag->tagged->oid));\n \t\t}\n \t\tobject->flags |= flags;\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex 44d8e8017..e8dfeafe7 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -216,6 +216,45 @@ test_expect_success 'upon cloning, check that all refs point to objects' '\n \t! test -e \"$HTTPD_ROOT_PATH/one-time-sed\"\n '\n \n+test_expect_success 'when partial cloning, tolerate server not sending target of tag' '\n+\tSERVER=\"$HTTPD_DOCUMENT_ROOT_PATH/server\" &&\n+\trm -rf \"$SERVER\" repo &&\n+\ttest_create_repo \"$SERVER\" &&\n+\ttest_commit -C \"$SERVER\" foo &&\n+\ttest_config -C \"$SERVER\" uploadpack.allowfilter 1 &&\n+\ttest_config -C \"$SERVER\" uploadpack.allowanysha1inwant 1 &&\n+\n+\t# Create an annotated tag pointing to a blob.\n+\tBLOB=$(echo blob-contents | git -C \"$SERVER\" hash-object --stdin -w) &&\n+\tgit -C \"$SERVER\" tag -m message -a myblob \"$BLOB\" &&\n+\n+\t# Craft a packfile including the tag, but not the blob it points to.\n+\tprintf \"%s\\n%s\\n--not\\n%s\\n\" \\\n+\t\t$(git -C \"$SERVER\" rev-parse HEAD) \\\n+\t\t$(git -C \"$SERVER\" rev-parse myblob) \\\n+\t\t$(git -C \"$SERVER\" rev-parse myblob^{blob}) |\n+\t\tgit -C \"$SERVER\" pack-objects --thin --stdout >incomplete.pack &&\n+\n+\t# Replace the existing packfile with the crafted one. The protocol\n+\t# requires that the packfile be sent in sideband 1, hence the extra\n+\t# \\x01 byte at the beginning.\n+\tprintf \"1,/packfile/!c %04x\\\\\\\\x01%s0000\" \\\n+\t\t\"$(($(wc -c \"$HTTPD_ROOT_PATH/one-time-sed\" &&\n+\n+\t# Use protocol v2 because the sed command looks for the \"packfile\"\n+\t# section header.\n+\ttest_config -C \"$SERVER\" protocol.version 2 &&\n+\n+\t# Exercise to make sure it works.\n+\tgit -c protocol.version=2 clone \\\n+\t\t--filter=blob:none $HTTPD_URL/one_time_sed/server repo &&\n+\n+\t# Ensure that the one-time-sed script was used.\n+\t! test -e \"$HTTPD_ROOT_PATH/one-time-sed\"\n+'\n+\n stop_httpd\n \n test_done\n-- \n2.18.0.203.gfac676dfb9-goog\n\n"},{"id":"352445","messageId":"5a7731b13f8607740f7519e781b22d4098b1e979.1531438861.git.jonathantanmy@google.com","threadId":"48874","inReplyTo":"cover.1531438861.git.jonathantanmy@google.com","subject":"[PATCH 2/2] tag: don't warn if target is missing but promised","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-07-13T00:03:07Z","receivedAt":"2018-07-13T00:03:18Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"deref_tag() prints a warning if the object that a tag refers to does not\nexist. However, when a partial clone has an annotated tag from its\npromisor remote, but not the object that it refers to, printing a\nwarning on such a tag is incorrect.\n\nThis occurs, for example, when the checkout that happens after a partial\nclone causes some objects to be fetched - and as part of the fetch, all\nlocal refs are read. The test included in this patch demonstrates this\nsituation.\n\nTherefore, do not print a warning in this case.\n\nSigned-off-by: Jonathan Tan \n---\n t/t5616-partial-clone.sh | 9 +++++++--\n tag.c | 13 ++++++++++---\n 2 files changed, 17 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t5616-partial-clone.sh b/t/t5616-partial-clone.sh\nindex e8dfeafe7..bbbe7537d 100755\n--- a/t/t5616-partial-clone.sh\n+++ b/t/t5616-partial-clone.sh\n@@ -229,9 +229,13 @@ test_expect_success 'when partial cloning, tolerate server not sending target of\n \tgit -C \"$SERVER\" tag -m message -a myblob \"$BLOB\" &&\n \n \t# Craft a packfile including the tag, but not the blob it points to.\n-\tprintf \"%s\\n%s\\n--not\\n%s\\n\" \\\n+\t# Also, omit objects referenced from HEAD in order to force a second\n+\t# fetch (to fetch missing objects) upon the automatic checkout that\n+\t# happens after a clone.\n+\tprintf \"%s\\n%s\\n--not\\n%s\\n%s\\n\" \\\n \t\t$(git -C \"$SERVER\" rev-parse HEAD) \\\n \t\t$(git -C \"$SERVER\" rev-parse myblob) \\\n+\t\t$(git -C \"$SERVER\" rev-parse HEAD^{tree}) \\\n \t\t$(git -C \"$SERVER\" rev-parse myblob^{blob}) |\n \t\tgit -C \"$SERVER\" pack-objects --thin --stdout >incomplete.pack &&\n \n@@ -249,7 +253,8 @@ test_expect_success 'when partial cloning, tolerate server not sending target of\n \n \t# Exercise to make sure it works.\n \tgit -c protocol.version=2 clone \\\n-\t\t--filter=blob:none $HTTPD_URL/one_time_sed/server repo &&\n+\t\t--filter=blob:none $HTTPD_URL/one_time_sed/server repo 2> err &&\n+\t! grep \"missing object referenced by\" err &&\n \n \t# Ensure that the one-time-sed script was used.\n \t! test -e \"$HTTPD_ROOT_PATH/one-time-sed\"\ndiff --git a/tag.c b/tag.c\nindex 3d37c1bd2..1110e3643 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -4,6 +4,7 @@\n #include \"tree.h\"\n #include \"blob.h\"\n #include \"gpg-interface.h\"\n+#include \"packfile.h\"\n \n const char *tag_type = \"tag\";\n \n@@ -64,12 +65,18 @@ int gpg_verify_tag(const struct object_id *oid, const char *name_to_report,\n \n struct object *deref_tag(struct object *o, const char *warn, int warnlen)\n {\n+\tstruct object_id *last_oid = NULL;\n \twhile (o && o->type == OBJ_TAG)\n-\t\tif (((struct tag *)o)->tagged)\n-\t\t\to = parse_object(&((struct tag *)o)->tagged->oid);\n-\t\telse\n+\t\tif (((struct tag *)o)->tagged) {\n+\t\t\tlast_oid = &((struct tag *)o)->tagged->oid;\n+\t\t\to = parse_object(last_oid);\n+\t\t} else {\n+\t\t\tlast_oid = NULL;\n \t\t\to = NULL;\n+\t\t}\n \tif (!o && warn) {\n+\t\tif (last_oid && is_promisor_object(last_oid))\n+\t\t\treturn NULL;\n \t\tif (!warnlen)\n \t\t\twarnlen = strlen(warn);\n \t\terror(\"missing object referenced by '%.*s'\", warnlen, warn);\n-- \n2.18.0.203.gfac676dfb9-goog\n\n"}]}