{"thread":{"id":"28279","subject":"Funnies with \"git fetch\"","startedAt":"2011-09-01T17:53:12Z","lastAt":"2011-09-05T02:21:37Z","messageCount":18,"participants":["Junio C Hamano","Jeff King","Nguyen Thai Ngoc Duy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"174693","messageId":"7vpqjkw3nb.fsf@alter.siamese.dyndns.org","threadId":"28279","inReplyTo":null,"subject":"Funnies with \"git fetch\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-01T17:53:12Z","receivedAt":"2011-09-01T17:53:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I just did this in an empty directory.\n\n    $ git init src\n    $ cd src\n    $ echo hello >greetings ; git add . ; git commit -m greetings\n    $ S=$(git rev-parse :greetings | sed -e 's|^..|&/|')\n    $ X=$(echo bye | git hash-object -w --stdin | sed -e 's|^..|&/|')\n    $ mv -f .git/objects/$X .git/objects/$S\n\nThe tip commit _thinks_ it has \"greetings\" that contains \"hello\", but\nsomebody replaced it with a corrupt \"bye\" that does not match self\nintegrity.\n\n    $ git fsck\n    error: sha1 mismatch ce013625030ba8dba906f756967f9e9ca394464a\n\n    error: ce013625030ba8dba906f756967f9e9ca394464a: object corrupt or missing\n    missing blob ce013625030ba8dba906f756967f9e9ca394464a\n\nThe \"hello\" blob is ce0136, and the tree contained in HEAD expects \"hello\"\nin that loose object file, but notices the contents do not match the\nfilename.\n\nSo far, so good. Let's see what others see when they interact with this\nrepository.\n\ncd ../\ngit init dst\ncd dst\ngit config receive.fsckobjects true\ngit remote add origin ../src\ngit config branch.master.remote origin\ngit config branch.master.merge refs/heads/master\ngit fetch\n    remote: Counting objects: 3, done.\n    remote: Total 3 (delta 0), reused 0 (delta 0)\n    Unpacking objects: 100% (3/3), done.\n    From ../src\n     * [new branch]      master     -> origin/master\n\nOops? If we run \"fsck\" at this point, we would notice the breakage:\n\n    $ git fsck\n    notice: HEAD points to an unborn branch (master)\n    broken link from    tree 1c93b84c9756b083e5751db1f9ffa7f80ac667e2\n                  to    blob ce013625030ba8dba906f756967f9e9ca394464a\n    missing blob ce013625030ba8dba906f756967f9e9ca394464a\n    dangling blob b023018cabc396e7692c70bbf5784a93d3f738ab\n\nHere, b02301 is the true identity of the \"bye\" blob the src repository\ncrafted and tried to fool us into believing it is \"hello\".  We can see\nthat the object transfer gave three objects, and because we only propagate\nthe contents and have the receiving end compute the object names from the\ndata, we received b02301 but not ce0136.\n\n    $ ls .git/objects/??/?*\n    .git/objects/1c/93b84c9756b083e5751db1f9ffa7f80ac667e2\n    .git/objects/61/5d8c76daef6744635c87fb312a76a5ec7462ea\n    .git/objects/b0/23018cabc396e7692c70bbf5784a93d3f738ab\n\nAs a side note, if we did \"git pull\" instead of \"git fetch\", we would have\nalso noticed the breakage, like so:\n\n    $ git pull\n    remote: Counting objects: 3, done.\n    remote: Total 3 (delta 0), reused 0 (delta 0)\n    Unpacking objects: 100% (3/3), done.\n    From ../src\n     * [new branch]      master     -> origin/master\n    error: unable to find ce013625030ba8dba906f756967f9e9ca394464a\n    error: unable to read sha1 file of greetings (ce013625030ba...)\n\nBut the straight \"fetch\" did not notice anything fishy going on. Shouldn't\nwe have?  Even though we may be reasonably safe, unpack-objects should be\nable to do better, especially under receive.fsckobjects option.\n\nAlso as a side note, if we set \n\n    $ git config fetch.unpacklimit 1\n\nbefore we run this \"git fetch\", we end up storing a single pack, whose\ncontents are the same three objects above (as expected), and we do not get\nany indication of an error from the command.\n\nI think the breakages are:\n\n - The sending side does not give any indication that it _wanted_ to send\n   ce0136 but couldn't, and ended up sending another object;\n\n - The pack data sent over the wire was self consistent (no breakage here)\n   and sent three well-formed objects, but it was inconsistent with\n   respect to what history was being transferred (breakage is here);\n\n - The receiving end did not notice the inconsistency.\n\nThe first one is of the lower priority, as the client side should be able\nto notice an upstream with corruption in any case. Perhaps after asking\nfor objects between \"have\" and \"want\", \"git fetch\" should verify that it\ncan fully walk the subhistory that was supposed to be transferred down to\nthe blob level?\n"},{"id":"174713","messageId":"7vpqjjnau1.fsf@alter.siamese.dyndns.org","threadId":"28279","inReplyTo":"7vpqjkw3nb.fsf@alter.siamese.dyndns.org","subject":"Re: Funnies with \"git fetch\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-01T22:42:46Z","receivedAt":"2011-09-01T22:42:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I think the breakages are:\n>\n>  - The sending side does not give any indication that it _wanted_ to send\n>    ce0136 but couldn't, and ended up sending another object;\n>\n>  - The pack data sent over the wire was self consistent (no breakage here)\n>    and sent three well-formed objects, but it was inconsistent with\n>    respect to what history was being transferred (breakage is here);\n>\n>  - The receiving end did not notice the inconsistency.\n>\n> The first one is of the lower priority, as the client side should be able\n> to notice an upstream with corruption in any case. Perhaps after asking\n> for objects between \"have\" and \"want\", \"git fetch\" should verify that it\n> can fully walk the subhistory that was supposed to be transferred down to\n> the blob level?\n\nSo I have a series to fix the latter \"more important\" half I'll be sending\nout in this thread.\n"},{"id":"174714","messageId":"1314917015-3587-1-git-send-email-gitster@pobox.com","threadId":"28279","inReplyTo":"7vpqjkw3nb.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/3] Verify the objects fetch obtained before updating ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-01T22:43:32Z","receivedAt":"2011-09-01T22:43:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"So here is a series to make the client side a bit more careful.\n\nThese patches apply on top of jc/traverse-commit-list beba25a (revision.c:\nupdate show_object_with_name() without using malloc(), 2011-08-17) in\n'next'.\n\nJunio C Hamano (3):\n  list-objects: pass callback data to show_objects()\n  rev-list --verify-object\n  fetch: ensure we transferred everything we need before updating our\n    ref\n\n builtin/fetch.c        |  119 +++++++++++++++++++++++++----------------------\n builtin/pack-objects.c |    4 +-\n builtin/rev-list.c     |   14 ++++-\n list-objects.c         |   28 +++++++----\n list-objects.h         |    5 +-\n revision.c             |    5 ++\n revision.h             |    1 +\n upload-pack.c          |    4 +-\n 8 files changed, 105 insertions(+), 75 deletions(-)\n\n-- \n1.7.7.rc0.72.g4b5ea\n"},{"id":"174715","messageId":"1314917015-3587-2-git-send-email-gitster@pobox.com","threadId":"28279","inReplyTo":"1314917015-3587-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/3] list-objects: pass callback data to show_objects()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-01T22:43:33Z","receivedAt":"2011-09-01T22:43:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The traverse_commit_list() API takes two callback functions, one to show\ncommit objects, and the other to show other kinds of objects. Even though\nthe former has a callback data parameter, so that the callback does not\nhave to rely on global state, the latter does not.\n\nGive the show_objects() callback the same callback data parameter.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/pack-objects.c |    4 +++-\n builtin/rev-list.c     |   10 +++++++---\n list-objects.c         |   28 +++++++++++++++++-----------\n list-objects.h         |    5 ++---\n upload-pack.c          |    4 +++-\n 5 files changed, 32 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin/pack-objects.c b/builtin/pack-objects.c\nindex f402a84..fca6cb5 100644\n--- a/builtin/pack-objects.c\n+++ b/builtin/pack-objects.c\n@@ -1936,7 +1936,9 @@ static void show_commit(struct commit *commit, void *data)\n \tcommit->object.flags |= OBJECT_ADDED;\n }\n \n-static void show_object(struct object *obj, const struct name_path *path, const char *last)\n+static void show_object(struct object *obj,\n+\t\t\tconst struct name_path *path, const char *last,\n+\t\t\tvoid *data)\n {\n \tchar *name = path_name(path, last);\n \ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex f5ce487..920b91c 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -168,15 +168,19 @@ static void finish_commit(struct commit *commit, void *data)\n \tcommit->buffer = NULL;\n }\n \n-static void finish_object(struct object *obj, const struct name_path *path, const char *name)\n+static void finish_object(struct object *obj,\n+\t\t\t  const struct name_path *path, const char *name,\n+\t\t\t  void *cb_data)\n {\n \tif (obj->type == OBJ_BLOB && !has_sha1_file(obj->sha1))\n \t\tdie(\"missing blob object '%s'\", sha1_to_hex(obj->sha1));\n }\n \n-static void show_object(struct object *obj, const struct name_path *path, const char *component)\n+static void show_object(struct object *obj,\n+\t\t\tconst struct name_path *path, const char *component,\n+\t\t\tvoid *cb_data)\n {\n-\tfinish_object(obj, path, component);\n+\tfinish_object(obj, path, component, cb_data);\n \tshow_object_with_name(stdout, obj, path, component);\n }\n \ndiff --git a/list-objects.c b/list-objects.c\nindex 0fb44e7..39d80c0 100644\n--- a/list-objects.c\n+++ b/list-objects.c\n@@ -12,7 +12,8 @@ static void process_blob(struct rev_info *revs,\n \t\t\t struct blob *blob,\n \t\t\t show_object_fn show,\n \t\t\t struct name_path *path,\n-\t\t\t const char *name)\n+\t\t\t const char *name,\n+\t\t\t void *cb_data)\n {\n \tstruct object *obj = &blob->object;\n \n@@ -23,7 +24,7 @@ static void process_blob(struct rev_info *revs,\n \tif (obj->flags & (UNINTERESTING | SEEN))\n \t\treturn;\n \tobj->flags |= SEEN;\n-\tshow(obj, path, name);\n+\tshow(obj, path, name, cb_data);\n }\n \n /*\n@@ -52,7 +53,8 @@ static void process_gitlink(struct rev_info *revs,\n \t\t\t    const unsigned char *sha1,\n \t\t\t    show_object_fn show,\n \t\t\t    struct name_path *path,\n-\t\t\t    const char *name)\n+\t\t\t    const char *name,\n+\t\t\t    void *cb_data)\n {\n \t/* Nothing to do */\n }\n@@ -62,7 +64,8 @@ static void process_tree(struct rev_info *revs,\n \t\t\t show_object_fn show,\n \t\t\t struct name_path *path,\n \t\t\t struct strbuf *base,\n-\t\t\t const char *name)\n+\t\t\t const char *name,\n+\t\t\t void *cb_data)\n {\n \tstruct object *obj = &tree->object;\n \tstruct tree_desc desc;\n@@ -80,7 +83,7 @@ static void process_tree(struct rev_info *revs,\n \tif (parse_tree(tree) < 0)\n \t\tdie(\"bad tree object %s\", sha1_to_hex(obj->sha1));\n \tobj->flags |= SEEN;\n-\tshow(obj, path, name);\n+\tshow(obj, path, name, cb_data);\n \tme.up = path;\n \tme.elem = name;\n \tme.elem_len = strlen(name);\n@@ -106,14 +109,17 @@ static void process_tree(struct rev_info *revs,\n \t\tif (S_ISDIR(entry.mode))\n \t\t\tprocess_tree(revs,\n \t\t\t\t     lookup_tree(entry.sha1),\n-\t\t\t\t     show, &me, base, entry.path);\n+\t\t\t\t     show, &me, base, entry.path,\n+\t\t\t\t     cb_data);\n \t\telse if (S_ISGITLINK(entry.mode))\n \t\t\tprocess_gitlink(revs, entry.sha1,\n-\t\t\t\t\tshow, &me, entry.path);\n+\t\t\t\t\tshow, &me, entry.path,\n+\t\t\t\t\tcb_data);\n \t\telse\n \t\t\tprocess_blob(revs,\n \t\t\t\t     lookup_blob(entry.sha1),\n-\t\t\t\t     show, &me, entry.path);\n+\t\t\t\t     show, &me, entry.path,\n+\t\t\t\t     cb_data);\n \t}\n \tstrbuf_setlen(base, baselen);\n \tfree(tree->buffer);\n@@ -185,17 +191,17 @@ void traverse_commit_list(struct rev_info *revs,\n \t\t\tcontinue;\n \t\tif (obj->type == OBJ_TAG) {\n \t\t\tobj->flags |= SEEN;\n-\t\t\tshow_object(obj, NULL, name);\n+\t\t\tshow_object(obj, NULL, name, data);\n \t\t\tcontinue;\n \t\t}\n \t\tif (obj->type == OBJ_TREE) {\n \t\t\tprocess_tree(revs, (struct tree *)obj, show_object,\n-\t\t\t\t     NULL, &base, name);\n+\t\t\t\t     NULL, &base, name, data);\n \t\t\tcontinue;\n \t\t}\n \t\tif (obj->type == OBJ_BLOB) {\n \t\t\tprocess_blob(revs, (struct blob *)obj, show_object,\n-\t\t\t\t     NULL, name);\n+\t\t\t\t     NULL, name, data);\n \t\t\tcontinue;\n \t\t}\n \t\tdie(\"unknown pending object %s (%s)\",\ndiff --git a/list-objects.h b/list-objects.h\nindex d65dbf0..3db7bb6 100644\n--- a/list-objects.h\n+++ b/list-objects.h\n@@ -2,11 +2,10 @@\n #define LIST_OBJECTS_H\n \n typedef void (*show_commit_fn)(struct commit *, void *);\n-typedef void (*show_object_fn)(struct object *, const struct name_path *, const char *);\n-typedef void (*show_edge_fn)(struct commit *);\n-\n+typedef void (*show_object_fn)(struct object *, const struct name_path *, const char *, void *);\n void traverse_commit_list(struct rev_info *, show_commit_fn, show_object_fn, void *);\n \n+typedef void (*show_edge_fn)(struct commit *);\n void mark_edges_uninteresting(struct commit_list *, struct rev_info *, show_edge_fn);\n \n #endif\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 970a1eb..6be6259 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -83,7 +83,9 @@ static void show_commit(struct commit *commit, void *data)\n \tcommit->buffer = NULL;\n }\n \n-static void show_object(struct object *obj, const struct name_path *path, const char *component)\n+static void show_object(struct object *obj,\n+\t\t\tconst struct name_path *path, const char *component,\n+\t\t\tvoid *cb_data)\n {\n \tshow_object_with_name(pack_pipe, obj, path, component);\n }\n-- \n1.7.7.rc0.72.g4b5ea\n"},{"id":"174716","messageId":"1314917015-3587-3-git-send-email-gitster@pobox.com","threadId":"28279","inReplyTo":"1314917015-3587-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/3] rev-list --verify-object","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-01T22:43:34Z","receivedAt":"2011-09-01T22:43:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Often we want to verify everything reachable from a given set of commits\nare present in our repository and connected without a gap to the tips of\nour refs. We used to do this for this purpose:\n\n    $ rev-list --objects $commits_to_be_tested --not --all\n\nEven though this is good enough for catching missing commits and trees,\nwe show the object name but do not verify their existence, let alone their\nwell-formedness, for the blob objects at the leaf level.\n\nAdd a new \"--verify-object\" option so that we can catch missing and broken\nblobs as well.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/rev-list.c |    4 ++++\n revision.c         |    5 +++++\n revision.h         |    1 +\n 3 files changed, 10 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 920b91c..ab3be7c 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -180,7 +180,11 @@ static void show_object(struct object *obj,\n \t\t\tconst struct name_path *path, const char *component,\n \t\t\tvoid *cb_data)\n {\n+\tstruct rev_info *info = cb_data;\n+\n \tfinish_object(obj, path, component, cb_data);\n+\tif (info->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n+\t\tparse_object(obj->sha1);\n \tshow_object_with_name(stdout, obj, path, component);\n }\n \ndiff --git a/revision.c b/revision.c\nindex 072ddac..5ef498b 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1383,6 +1383,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->tree_objects = 1;\n \t\trevs->blob_objects = 1;\n \t\trevs->edge_hint = 1;\n+\t} else if (!strcmp(arg, \"--verify-objects\")) {\n+\t\trevs->tag_objects = 1;\n+\t\trevs->tree_objects = 1;\n+\t\trevs->blob_objects = 1;\n+\t\trevs->verify_objects = 1;\n \t} else if (!strcmp(arg, \"--unpacked\")) {\n \t\trevs->unpacked = 1;\n \t} else if (!prefixcmp(arg, \"--unpacked=\")) {\ndiff --git a/revision.h b/revision.h\nindex da00a58..648876b 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -53,6 +53,7 @@ struct rev_info {\n \t\t\ttag_objects:1,\n \t\t\ttree_objects:1,\n \t\t\tblob_objects:1,\n+\t\t\tverify_objects:1,\n \t\t\tedge_hint:1,\n \t\t\tlimited:1,\n \t\t\tunpacked:1,\n-- \n1.7.7.rc0.72.g4b5ea\n"},{"id":"174717","messageId":"1314917015-3587-4-git-send-email-gitster@pobox.com","threadId":"28279","inReplyTo":"1314917015-3587-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] fetch: verify we have everything we need before updating our ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-01T22:43:35Z","receivedAt":"2011-09-01T22:43:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The \"git fetch\" command works in two phases. The remote side tells us what\nobjects are at the tip of the refs we are fetching from, and transfers the\nobjects missing from our side. After storing the objects in our repository,\nwe update our remote tracking branches to point at the updated tips of the\nrefs.\n\nA broken or malicious remote side could send a perfectly well-formed pack\ndata during the object transfer phase, but there is no guarantee that the\ngiven data actually fill the gap between the objects we originally had and\nthe refs we are updating to.\n\nAlthough this kind of breakage can be caught by running fsck after a\nfetch, it is much cheaper to verify that everything that is reachable from\nthe tips of the refs we fetched are indeed fully connected to the tips of\nour current set of refs before we update them.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/fetch.c |  119 +++++++++++++++++++++++++++++--------------------------\n 1 files changed, 63 insertions(+), 56 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex f9c41da..bdb03ff 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -345,6 +345,64 @@ static int update_local_ref(struct ref *ref,\n \t}\n }\n \n+/*\n+ * The ref_map records the tips of the refs we are fetching. If\n+ *\n+ *  $ git rev-list --verify-objects --stdin --not --all\n+ *\n+ * (feeding all the refs in ref_map on its standard input) does not\n+ * error out, that means everything reachable from these updated refs\n+ * locally exists and is connected to some of our existing refs.\n+ *\n+ * Returns 0 if everything is connected, non-zero otherwise.\n+ */\n+static int check_everything_connected(struct ref *ref_map, int quiet)\n+{\n+\tstruct child_process rev_list;\n+\tconst char *argv[] = {\"rev-list\", \"--verify-objects\",\n+\t\t\t      \"--stdin\", \"--not\", \"--all\", NULL, NULL};\n+\tchar commit[41];\n+\tstruct ref *ref;\n+\tint err = 0;\n+\n+\tif (!ref_map)\n+\t\treturn 0;\n+\n+\tif (quiet)\n+\t\targv[5] = \"--quiet\";\n+\n+\tmemset(&rev_list, 0, sizeof(rev_list));\n+\trev_list.argv = argv;\n+\trev_list.git_cmd = 1;\n+\trev_list.in = -1;\n+\trev_list.no_stdout = 1;\n+\trev_list.no_stderr = quiet;\n+\tif (start_command(&rev_list))\n+\t\treturn error(_(\"Could not run 'git rev-list'\"));\n+\n+\tsigchain_push(SIGPIPE, SIG_IGN);\n+\n+\tmemcpy(commit + 40, \"\\n\", 2);\n+\tfor (ref = ref_map; ref; ref = ref->next) {\n+\t\tmemcpy(commit, sha1_to_hex(ref->old_sha1), 40);\n+\t\tif (write_in_full(rev_list.in, commit, 41) < 0) {\n+\t\t\tif (errno != EPIPE && errno != EINVAL)\n+\t\t\t\terror(_(\"failed write to rev-list: %s\"),\n+\t\t\t\t      strerror(errno));\n+\t\t\terr = -1;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n+\tif (close(rev_list.in)) {\n+\t\terror(_(\"failed to close rev-list's stdin: %s\"), strerror(errno));\n+\t\terr = -1;\n+\t}\n+\n+\tsigchain_pop(SIGPIPE);\n+\n+\treturn finish_command(&rev_list) || err;\n+}\n+\n static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\tstruct ref *ref_map)\n {\n@@ -364,6 +422,10 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\turl = transport_anonymize_url(raw_url);\n \telse\n \t\turl = xstrdup(\"foreign\");\n+\n+\tif (check_everything_connected(ref_map, 0))\n+\t\treturn error(_(\"%s did not send all necessary objects\\n\"), url);\n+\n \tfor (rm = ref_map; rm; rm = rm->next) {\n \t\tstruct ref *ref = NULL;\n \n@@ -457,24 +519,9 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n  * We would want to bypass the object transfer altogether if\n  * everything we are going to fetch already exists and is connected\n  * locally.\n- *\n- * The refs we are going to fetch are in ref_map.  If running\n- *\n- *  $ git rev-list --objects --stdin --not --all\n- *\n- * (feeding all the refs in ref_map on its standard input)\n- * does not error out, that means everything reachable from the\n- * refs we are going to fetch exists and is connected to some of\n- * our existing refs.\n  */\n static int quickfetch(struct ref *ref_map)\n {\n-\tstruct child_process revlist;\n-\tstruct ref *ref;\n-\tint err;\n-\tconst char *argv[] = {\"rev-list\",\n-\t\t\"--quiet\", \"--objects\", \"--stdin\", \"--not\", \"--all\", NULL};\n-\n \t/*\n \t * If we are deepening a shallow clone we already have these\n \t * objects reachable.  Running rev-list here will return with\n@@ -484,47 +531,7 @@ static int quickfetch(struct ref *ref_map)\n \t */\n \tif (depth)\n \t\treturn -1;\n-\n-\tif (!ref_map)\n-\t\treturn 0;\n-\n-\tmemset(&revlist, 0, sizeof(revlist));\n-\trevlist.argv = argv;\n-\trevlist.git_cmd = 1;\n-\trevlist.no_stdout = 1;\n-\trevlist.no_stderr = 1;\n-\trevlist.in = -1;\n-\n-\terr = start_command(&revlist);\n-\tif (err) {\n-\t\terror(_(\"could not run rev-list\"));\n-\t\treturn err;\n-\t}\n-\n-\t/*\n-\t * If rev-list --stdin encounters an unknown commit, it terminates,\n-\t * which will cause SIGPIPE in the write loop below.\n-\t */\n-\tsigchain_push(SIGPIPE, SIG_IGN);\n-\n-\tfor (ref = ref_map; ref; ref = ref->next) {\n-\t\tif (write_in_full(revlist.in, sha1_to_hex(ref->old_sha1), 40) < 0 ||\n-\t\t    write_str_in_full(revlist.in, \"\\n\") < 0) {\n-\t\t\tif (errno != EPIPE && errno != EINVAL)\n-\t\t\t\terror(_(\"failed write to rev-list: %s\"), strerror(errno));\n-\t\t\terr = -1;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n-\tif (close(revlist.in)) {\n-\t\terror(_(\"failed to close rev-list's stdin: %s\"), strerror(errno));\n-\t\terr = -1;\n-\t}\n-\n-\tsigchain_pop(SIGPIPE);\n-\n-\treturn finish_command(&revlist) || err;\n+\treturn check_everything_connected(ref_map, 1);\n }\n \n static int fetch_refs(struct transport *transport, struct ref *ref_map)\n-- \n1.7.7.rc0.72.g4b5ea\n"},{"id":"174719","messageId":"20110901233108.GA9339@sigill.intra.peff.net","threadId":"28279","inReplyTo":"7vpqjjnau1.fsf@alter.siamese.dyndns.org","subject":"Re: Funnies with \"git fetch\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-01T23:31:08Z","receivedAt":"2011-09-01T23:31:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 01, 2011 at 03:42:46PM -0700, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > I think the breakages are:\n> >\n> >  - The sending side does not give any indication that it _wanted_ to send\n> >    ce0136 but couldn't, and ended up sending another object;\n\nThis isn't fixable without the server re-hashing every outgoing object,\nthough, is it? It thinks it has ce0136, but the data is corrupted. The\nonly way to detect that is to rehash each object that we send. That's\npretty expensive when you consider decompression and delta\nreconstruction.\n\nBesides which, it doesn't help many malicious cases at all. It will\nhandle disk corruption or a malicious object-db tweak. But it does\nnothing against an attacker who can trojan git, or who can spoof the\nnetwork session (which is probably hard to do if its straight ssh, but\nkeep in mind that traffic may be going through cleartext proxies).\n\nSo it seems that the receiving side is the only sensible place to put\nsuch checks. It catches problems at more levels, and it can generally\nafford the extra processing load (under the assumption that pushes to\nservers are smaller and less frequent than fetches).\n\n> >  - The pack data sent over the wire was self consistent (no breakage here)\n> >    and sent three well-formed objects, but it was inconsistent with\n> >    respect to what history was being transferred (breakage is here);\n\nI don't think this matters. We have thin packs, after all. What goes on\nthe wire doesn't have to be sensible unto itself; it's about whether\nthe receiving end can do something useful with it.\n\n> >  - The receiving end did not notice the inconsistency.\n> >\n> > The first one is of the lower priority, as the client side should be able\n> > to notice an upstream with corruption in any case. Perhaps after asking\n> > for objects between \"have\" and \"want\", \"git fetch\" should verify that it\n> > can fully walk the subhistory that was supposed to be transferred down to\n> > the blob level?\n\nThat seems like a sensible improvement to me.\n\n> So I have a series to fix the latter \"more important\" half I'll be sending\n> out in this thread.\n\nIf I understand correctly, your series is just about checking that we\nhave newly-referenced blobs. We were already checking commits and trees,\nand we should already be hashing individual objects when we index the\npack. Right?\n\nIn that case, the performance change should be negligible.\n\n-Peff\n"},{"id":"174727","messageId":"7vippbmygy.fsf@alter.siamese.dyndns.org","threadId":"28279","inReplyTo":"20110901233108.GA9339@sigill.intra.peff.net","subject":"Re: Funnies with \"git fetch\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-02T03:09:49Z","receivedAt":"2011-09-02T03:09:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> If I understand correctly, your series is just about checking that we\n> have newly-referenced blobs. We were already checking commits and trees,\n> and we should already be hashing individual objects when we index the\n> pack. Right?\n\nYou may be slightly misunderstanding the series.\n\nWe let unpack-objects or index-pack consume the pack stream, either by\nexploding them into loose objects, or computing the object name for each\nobject to create the mapping from object name to the offset. During this\nprocess, we deflate to read the contents and resolve the delta to come up\nwith the object name for individual objects, so we would notice corruption\nat the individual object level. As pack stream does not say what name each\nobject is (the recipient is expected to compute it), there is no \"stream\nsays it is object X but the data is actually for object Y\" problem. The\nrecipient does not even see \"X\"---all it sees is Y.\n\nThe current code does not try to make sure we really have the objects\nnecessary to connect the updated tips to our original refs at all.  Not\njust blobs but neither commits nor trees are traversed. The new check in\nstore_updated_refs() is about that. So in that sense, the series is not\nabout \"just blobs\".\n\nThe \"rev-list --verify-objects\" patch is about \"blob vs everything else\".\nIt is used in the existing quickfetch() check, and also the additional\ncheck in store_updated_refs(). The existing check we run with \"--objects\"\nis capable of detecting corruptions of commits and trees (as we had to be\nable to read them to discover objects they refer to), but that is not a\nsufficient check if we worry about missing blobs.\n"},{"id":"174728","messageId":"7vd3fjmxl9.fsf@alter.siamese.dyndns.org","threadId":"28279","inReplyTo":"7vippbmygy.fsf@alter.siamese.dyndns.org","subject":"Re: Funnies with \"git fetch\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-02T03:28:50Z","receivedAt":"2011-09-02T03:28:50Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> If I understand correctly, your series is just about checking that we\n>> have newly-referenced blobs. We were already checking commits and trees,\n>> and we should already be hashing individual objects when we index the\n>> pack. Right?\n>\n> You may be slightly misunderstanding the series.\n>\n> We let unpack-objects or index-pack consume the pack stream, either by\n> exploding them into loose objects, or computing the object name for each\n> object to create the mapping from object name to the offset. During this\n> process, we deflate to read the contents and resolve the delta to come up\n> with the object name for individual objects, so we would notice corruption\n> at the individual object level. As pack stream does not say what name each\n> object is (the recipient is expected to compute it), there is no \"stream\n> says it is object X but the data is actually for object Y\" problem. The\n> recipient does not even see \"X\"---all it sees is Y.\n\nOne thing I forgot to mention.\n\nIf you have receive.fsckobjects set, unpack-objects and index-pack are\ncalled with the --strict option and causes fsck_objects() to run, so we\nwill also catch malformed commits and trees that way. But even then,\ncalling fsck_object() on a blob object always succeeds, so it is not a\nreal check.\n\nWe probably would want to flip the default for receive_fsck_objects to\ntrue one of these days.\n\n> The current code does not try to make sure we really have the objects\n> necessary to connect the updated tips to our original refs at all.  Not\n> just blobs but neither commits nor trees are traversed. The new check in\n> store_updated_refs() is about that. So in that sense, the series is not\n> about \"just blobs\".\n>\n> The \"rev-list --verify-objects\" patch is about \"blob vs everything else\".\n> It is used in the existing quickfetch() check, and also the additional\n> check in store_updated_refs(). The existing check we run with \"--objects\"\n> is capable of detecting corruptions of commits and trees (as we had to be\n> able to read them to discover objects they refer to), but that is not a\n> sufficient check if we worry about missing blobs.\n\nI am debating myself if we want to also add calls to fsck_object() to the\nnew codepath. An additional patch on top of [2/3] would look like this\n(totally untested).\n\nIt would make \"rev-list --verify-objects\" useful independently from its\nuse in the context of \"git fetch\", and more importantly, what it checks in\nthe context of \"git fetch\", while it is related, is more or less\northogonal to what receive.fsckobjects checks. The check done during the\ntransfer is to check the set of objects the other side threw at us. The\ncheck done in the check_everything_connected() by calling \"rev-list\n--verify-objects\" is to check the set of objects we are actually going to\nuse. The former _should_ be superset of the latter, but [3/3] is about\nmaking sure that the former indeed is the superset of the latter.\n\n builtin/rev-list.c |   13 +++++++++++--\n 1 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex ab3be7c..bd49615 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -7,6 +7,7 @@\n #include \"log-tree.h\"\n #include \"graph.h\"\n #include \"bisect.h\"\n+#include \"fsck.h\"\n \n static const char rev_list_usage[] =\n \"git rev-list [OPTION] <commit-id>... [ -- paths... ]\\n\"\n@@ -53,6 +54,11 @@ static void show_commit(struct commit *commit, void *data)\n \tstruct rev_info *revs = info->revs;\n \n \tgraph_show_commit(revs->graph);\n+\tif (revs->verify_objects) {\n+\t\tif (!commit->object.parsed)\n+\t\t\tparse_object(commit->object.sha1);\n+\t\tfsck_object(&commit->object, 1, fsck_error_function);\n+\t}\n \n \tif (revs->count) {\n \t\tif (commit->object.flags & PATCHSAME)\n@@ -183,8 +189,11 @@ static void show_object(struct object *obj,\n \tstruct rev_info *info = cb_data;\n \n \tfinish_object(obj, path, component, cb_data);\n-\tif (info->verify_objects && !obj->parsed && obj->type != OBJ_COMMIT)\n-\t\tparse_object(obj->sha1);\n+\tif (info->verify_objects && obj->type != OBJ_COMMIT) {\n+\t\tif (!obj->parsed)\n+\t\t\tparse_object(obj->sha1);\n+\t\tfsck_object(obj, 1, fsck_error_function);\n+\t}\n \tshow_object_with_name(stdout, obj, path, component);\n }\n \n"},{"id":"174729","messageId":"CACsJy8C4qX=p6d1v+i7TJanACOqL9R6-pv9AOaU_CM6DNjJfyQ@mail.gmail.com","threadId":"28279","inReplyTo":"1314917015-3587-4-git-send-email-gitster@pobox.com","subject":"Re: [PATCH 3/3] fetch: verify we have everything we need before updating our ref","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2011-09-02T03:55:33Z","receivedAt":"2011-09-02T03:55:33Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Sep 2, 2011 at 5:43 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> The \"git fetch\" command works in two phases. The remote side tells us what\n> objects are at the tip of the refs we are fetching from, and transfers the\n> objects missing from our side. After storing the objects in our repository,\n> we update our remote tracking branches to point at the updated tips of the\n> refs.\n>\n> A broken or malicious remote side could send a perfectly well-formed pack\n> data during the object transfer phase, but there is no guarantee that the\n> given data actually fill the gap between the objects we originally had and\n> the refs we are updating to.\n\nWhat about receive-pack? Does it have the same breakage?\n-- \nDuy\n"},{"id":"174730","messageId":"7v8vq7muzi.fsf@alter.siamese.dyndns.org","threadId":"28279","inReplyTo":"CACsJy8C4qX=p6d1v+i7TJanACOqL9R6-pv9AOaU_CM6DNjJfyQ@mail.gmail.com","subject":"Re: [PATCH 3/3] fetch: verify we have everything we need before updating our ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-02T04:25:05Z","receivedAt":"2011-09-02T04:25:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n\n> What about receive-pack? Does it have the same breakage?\n\nI didn't look---you tell me ;-)\n"},{"id":"174731","messageId":"20110902050359.GB11678@sigill.intra.peff.net","threadId":"28279","inReplyTo":"7vippbmygy.fsf@alter.siamese.dyndns.org","subject":"Re: Funnies with \"git fetch\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-09-02T05:03:59Z","receivedAt":"2011-09-02T05:03:59Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 01, 2011 at 08:09:49PM -0700, Junio C Hamano wrote:\n\n> You may be slightly misunderstanding the series.\n> [...]\n> The current code does not try to make sure we really have the objects\n> necessary to connect the updated tips to our original refs at all.  Not\n> just blobs but neither commits nor trees are traversed. The new check in\n> store_updated_refs() is about that. So in that sense, the series is not\n> about \"just blobs\".\n\nAh, OK, I see. I was too focused on pulling the bits out of quickfetch\ninto check_everything_connected, and missed the important new call in\nstore_updated_refs.\n\nSo what you are doing makes sense to me. I am curious, though, what the\nperformance impact is like. In particular, it seems that we will pull\neach blob into memory via parse_object. Until now, we were mostly\nstreaming the blobs straight into packs. That makes me a little nervous\ngiven the discussions recently about large blobs, and not accessing them\nunnecessarily. But maybe that is a silly concern, as we will have just\nreconstructed and hashed such an object anyway to get its name.\n\n-Peff\n"},{"id":"174776","messageId":"7v39ge1qqx.fsf@alter.siamese.dyndns.org","threadId":"28279","inReplyTo":"7v8vq7muzi.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 3/3] fetch: verify we have everything we need before updating our ref","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-02T23:14:30Z","receivedAt":"2011-09-02T23:14:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:\n>\n>> What about receive-pack? Does it have the same breakage?\n>\n> I didn't look---you tell me ;-)\n\nI looked. It needs a similar check, and I'd need to refactor the\ncheck_everything_connected() function a bit to take an iterator.\n\nWill send a follow-up series this weekend.\n"},{"id":"174825","messageId":"7vliu4yv8w.fsf@alter.siamese.dyndns.org","threadId":"28279","inReplyTo":"7vpqjkw3nb.fsf@alter.siamese.dyndns.org","subject":"Re: Funnies with \"git fetch\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-04T19:15:27Z","receivedAt":"2011-09-04T19:15:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I just did this in an empty directory.\n>\n>     $ git init src\n>     $ cd src\n>     $ echo hello >greetings ; git add . ; git commit -m greetings\n>     $ S=$(git rev-parse :greetings | sed -e 's|^..|&/|')\n>     $ X=$(echo bye | git hash-object -w --stdin | sed -e 's|^..|&/|')\n>     $ mv -f .git/objects/$X .git/objects/$S\n>\n> The tip commit _thinks_ it has \"greetings\" that contains \"hello\", but\n> somebody replaced it with a corrupt \"bye\" that does not match self\n> integrity.\n>\n>     $ git fsck\n>     error: sha1 mismatch ce013625030ba8dba906f756967f9e9ca394464a\n>\n>     error: ce013625030ba8dba906f756967f9e9ca394464a: object corrupt or missing\n>     missing blob ce013625030ba8dba906f756967f9e9ca394464a\n>\n> The \"hello\" blob is ce0136, and the tree contained in HEAD expects \"hello\"\n> in that loose object file, but notices the contents do not match the\n> filename.\n>\n> So far, so good. Let's see what others see when they interact with this\n> repository.\n>\n> cd ../\n> git init dst\n> cd dst\n> git config receive.fsckobjects true\n> git remote add origin ../src\n> git config branch.master.remote origin\n> git config branch.master.merge refs/heads/master\n> git fetch\n>     remote: Counting objects: 3, done.\n>     remote: Total 3 (delta 0), reused 0 (delta 0)\n>     Unpacking objects: 100% (3/3), done.\n>     From ../src\n>      * [new branch]      master     -> origin/master\n>\n> Oops? If we run \"fsck\" at this point, we would notice the breakage:\n> ...\n> But the straight \"fetch\" did not notice anything fishy going on. Shouldn't\n> we have?  Even though we may be reasonably safe, unpack-objects should be\n> able to do better, especially under receive.fsckobjects option.\n\nWe do not have fetch.fsckobjects, and here is a quick patch to add it. I\nwould also add transfer.fsckobjects to cover both with a single variable\nin a follow-up patch, but with this, it fails as expected:\n\n    $ git config fetch.fsckobjects true\n    $ git fetch\n    remote: Counting objects: 3, done.\n    remote: Total 3 (delta 0), reused 0 (delta 0)\n    Unpacking objects: 100% (3/3), done.\n    error: unable to find ce013625030ba8dba906f756967f9e9ca394464a\n    fatal: object of unexpected type\n    fatal: unpack-objects failed\n\nduring the transfer phase.  The \"rev-list --verify-objects\" addition is a\nfix to a related but independent issue, so both are probably good to have,\nbut this makes it unnecessary to run fsck_object() during the update-ref\nphase.\n\n builtin/fetch-pack.c |    8 ++++++++\n 1 files changed, 8 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 412bd32..bfed536 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -16,6 +16,7 @@ static int fetch_unpack_limit = -1;\n static int unpack_limit = 100;\n static int prefer_ofs_delta = 1;\n static int no_done = 0;\n+static int fetch_fsck_objects;\n static struct fetch_pack_args args = {\n \t/* .uploadpack = */ \"git-upload-pack\",\n };\n@@ -734,6 +735,8 @@ static int get_pack(int xd[2], char **pack_lockfile)\n \t}\n \tif (*hdr_arg)\n \t\t*av++ = hdr_arg;\n+\tif (fetch_fsck_objects)\n+\t\t*av++ = \"--strict\";\n \t*av++ = NULL;\n \n \tcmd.in = demux.out;\n@@ -853,6 +856,11 @@ static int fetch_pack_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"fetch.fsckobjects\")) {\n+\t\tfetch_fsck_objects = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(var, value, cb);\n }\n \n"},{"id":"174837","messageId":"1315189297-19497-1-git-send-email-gitster@pobox.com","threadId":"28279","inReplyTo":"7vliu4yv8w.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/3] Add fetch.fsckobjects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-05T02:21:34Z","receivedAt":"2011-09-05T02:21:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Interestingly enough, when we added receive.fsckobjects long time ago,\nfor some reason we forgot to add the corresponding fetch.fsckobjects,\nwhich would have been an obvious thing to do.\n\nSo here is a series to add it.\n\nJunio C Hamano (3):\n  fetch.fsckobjects: verify downloaded objects\n  transfer.fsckobjects: unify fetch/receive.fsckobjects\n  test: fetch/receive with fsckobjects\n\n Documentation/config.txt        |   15 +++++-\n builtin/fetch-pack.c            |   18 +++++++\n builtin/receive-pack.c          |   17 +++++-\n t/t5504-fetch-receive-strict.sh |  104 +++++++++++++++++++++++++++++++++++++++\n 4 files changed, 150 insertions(+), 4 deletions(-)\n create mode 100755 t/t5504-fetch-receive-strict.sh\n\n-- \n1.7.7.rc0.175.gb3212\n"},{"id":"174836","messageId":"1315189297-19497-2-git-send-email-gitster@pobox.com","threadId":"28279","inReplyTo":"1315189297-19497-1-git-send-email-gitster@pobox.com","subject":"[PATCH 1/3] fetch.fsckobjects: verify downloaded objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-05T02:21:35Z","receivedAt":"2011-09-05T02:21:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This corresponds to receive.fsckobjects configuration variable added (a\nlot) earlier in 20dc001 (receive-pack: allow using --strict mode for\nunpacking objects, 2008-02-25).\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt |    6 ++++++\n builtin/fetch-pack.c     |    8 ++++++++\n 2 files changed, 14 insertions(+), 0 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 664de6b..4cbc4b9 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -820,6 +820,12 @@ diff.wordRegex::\n \tsequences that match the regular expression are \"words\", all other\n \tcharacters are *ignorable* whitespace.\n \n+fetch.fsckObjects::\n+\tIf it is set to true, git-fetch-pack will check all fetched\n+\tobjects. It will abort in the case of a malformed object or a\n+\tbroken link. The result of an abort are only dangling objects.\n+\tDefaults to false.\n+\n fetch.unpackLimit::\n \tIf the number of objects fetched over the git native\n \ttransfer is below this\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex dbd8b7b..df6a8dc 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -14,6 +14,7 @@ static int transfer_unpack_limit = -1;\n static int fetch_unpack_limit = -1;\n static int unpack_limit = 100;\n static int prefer_ofs_delta = 1;\n+static int fetch_fsck_objects;\n static struct fetch_pack_args args = {\n \t/* .uploadpack = */ \"git-upload-pack\",\n };\n@@ -663,6 +664,8 @@ static int get_pack(int xd[2], char **pack_lockfile)\n \t}\n \tif (*hdr_arg)\n \t\t*av++ = hdr_arg;\n+\tif (fetch_fsck_objects)\n+\t\t*av++ = \"--strict\";\n \t*av++ = NULL;\n \n \tcmd.in = demux.out;\n@@ -776,6 +779,11 @@ static int fetch_pack_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"fetch.fsckobjects\")) {\n+\t\tfetch_fsck_objects = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(var, value, cb);\n }\n \n-- \n1.7.7.rc0.175.gb3212\n"},{"id":"174839","messageId":"1315189297-19497-3-git-send-email-gitster@pobox.com","threadId":"28279","inReplyTo":"1315189297-19497-1-git-send-email-gitster@pobox.com","subject":"[PATCH 2/3] transfer.fsckobjects: unify fetch/receive.fsckobjects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-05T02:21:36Z","receivedAt":"2011-09-05T02:21:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This single variable can be used to set instead of setting fsckobjects\nvariable for fetch & receive independently.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/config.txt |   11 +++++++++--\n builtin/fetch-pack.c     |   14 ++++++++++++--\n builtin/receive-pack.c   |   17 ++++++++++++++---\n 3 files changed, 35 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 4cbc4b9..d944403 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -824,7 +824,8 @@ fetch.fsckObjects::\n \tIf it is set to true, git-fetch-pack will check all fetched\n \tobjects. It will abort in the case of a malformed object or a\n \tbroken link. The result of an abort are only dangling objects.\n-\tDefaults to false.\n+\tDefaults to false. If not set, the value of `transfer.fsckObjects`\n+\tis used instead.\n \n fetch.unpackLimit::\n \tIf the number of objects fetched over the git native\n@@ -1427,7 +1428,8 @@ receive.fsckObjects::\n \tIf it is set to true, git-receive-pack will check all received\n \tobjects. It will abort in the case of a malformed object or a\n \tbroken link. The result of an abort are only dangling objects.\n-\tDefaults to false.\n+\tDefaults to false. If not set, the value of `transfer.fsckObjects`\n+\tis used instead.\n \n receive.unpackLimit::\n \tIf the number of objects received in a push is below this\n@@ -1616,6 +1618,11 @@ tar.umask::\n \tarchiving user's umask will be used instead.  See umask(2) and\n \tlinkgit:git-archive[1].\n \n+transfer.fsckObjects::\n+\tWhen `fetch.fsckObjects` or `receive.fsckObjects` are\n+\tnot set, the value of this variable is used instead.\n+\tDefaults to false.\n+\n transfer.unpackLimit::\n \tWhen `fetch.unpackLimit` or `receive.unpackLimit` are\n \tnot set, the value of this variable is used instead.\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex df6a8dc..dac3038 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -14,7 +14,8 @@ static int transfer_unpack_limit = -1;\n static int fetch_unpack_limit = -1;\n static int unpack_limit = 100;\n static int prefer_ofs_delta = 1;\n-static int fetch_fsck_objects;\n+static int fetch_fsck_objects = -1;\n+static int transfer_fsck_objects = -1;\n static struct fetch_pack_args args = {\n \t/* .uploadpack = */ \"git-upload-pack\",\n };\n@@ -664,7 +665,11 @@ static int get_pack(int xd[2], char **pack_lockfile)\n \t}\n \tif (*hdr_arg)\n \t\t*av++ = hdr_arg;\n-\tif (fetch_fsck_objects)\n+\tif (fetch_fsck_objects >= 0\n+\t    ? fetch_fsck_objects\n+\t    : transfer_fsck_objects >= 0\n+\t    ? transfer_fsck_objects\n+\t    : 0)\n \t\t*av++ = \"--strict\";\n \t*av++ = NULL;\n \n@@ -784,6 +789,11 @@ static int fetch_pack_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"transfer.fsckobjects\")) {\n+\t\ttransfer_fsck_objects = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \treturn git_default_config(var, value, cb);\n }\n \ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 0559fcc..021ea65 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -23,7 +23,8 @@ static int deny_deletes;\n static int deny_non_fast_forwards;\n static enum deny_action deny_current_branch = DENY_UNCONFIGURED;\n static enum deny_action deny_delete_current = DENY_UNCONFIGURED;\n-static int receive_fsck_objects;\n+static int receive_fsck_objects = -1;\n+static int transfer_fsck_objects = -1;\n static int receive_unpack_limit = -1;\n static int transfer_unpack_limit = -1;\n static int unpack_limit = 100;\n@@ -77,6 +78,11 @@ static int receive_pack_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (strcmp(var, \"transfer.fsckobjects\") == 0) {\n+\t\ttransfer_fsck_objects = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"receive.denycurrentbranch\")) {\n \t\tdeny_current_branch = parse_deny_action(var, value);\n \t\treturn 0;\n@@ -586,6 +592,11 @@ static const char *unpack(void)\n \tstruct pack_header hdr;\n \tconst char *hdr_err;\n \tchar hdr_arg[38];\n+\tint fsck_objects = (receive_fsck_objects >= 0\n+\t\t\t    ? receive_fsck_objects\n+\t\t\t    : transfer_fsck_objects >= 0\n+\t\t\t    ? transfer_fsck_objects\n+\t\t\t    : 0);\n \n \thdr_err = parse_pack_header(&hdr);\n \tif (hdr_err)\n@@ -598,7 +609,7 @@ static const char *unpack(void)\n \t\tint code, i = 0;\n \t\tconst char *unpacker[4];\n \t\tunpacker[i++] = \"unpack-objects\";\n-\t\tif (receive_fsck_objects)\n+\t\tif (fsck_objects)\n \t\t\tunpacker[i++] = \"--strict\";\n \t\tunpacker[i++] = hdr_arg;\n \t\tunpacker[i++] = NULL;\n@@ -618,7 +629,7 @@ static const char *unpack(void)\n \n \t\tkeeper[i++] = \"index-pack\";\n \t\tkeeper[i++] = \"--stdin\";\n-\t\tif (receive_fsck_objects)\n+\t\tif (fsck_objects)\n \t\t\tkeeper[i++] = \"--strict\";\n \t\tkeeper[i++] = \"--fix-thin\";\n \t\tkeeper[i++] = hdr_arg;\n-- \n1.7.7.rc0.175.gb3212\n"},{"id":"174838","messageId":"1315189297-19497-4-git-send-email-gitster@pobox.com","threadId":"28279","inReplyTo":"1315189297-19497-1-git-send-email-gitster@pobox.com","subject":"[PATCH 3/3] test: fetch/receive with fsckobjects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-09-05T02:21:37Z","receivedAt":"2011-09-05T02:21:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Add tests for the new fetch.fsckobjects, and also tests for\nreceive.fsckobjects we have had for quite some time.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t5504-fetch-receive-strict.sh |  104 +++++++++++++++++++++++++++++++++++++++\n 1 files changed, 104 insertions(+), 0 deletions(-)\n create mode 100755 t/t5504-fetch-receive-strict.sh\n\ndiff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\nnew file mode 100755\nindex 0000000..6610012\n--- /dev/null\n+++ b/t/t5504-fetch-receive-strict.sh\n@@ -0,0 +1,104 @@\n+#!/bin/sh\n+\n+test_description='fetch/receive strict mode'\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\techo hello >greetings &&\n+\tgit add greetings &&\n+\tgit commit -m greetings &&\n+\n+\tS=$(git rev-parse :greetings | sed -e \"s|^..|&/|\") &&\n+\tX=$(echo bye | git hash-object -w --stdin | sed -e \"s|^..|&/|\") &&\n+\tmv -f .git/objects/$X .git/objects/$S &&\n+\n+\ttest_must_fail git fsck\n+'\n+\n+test_expect_success 'fetch without strict' '\n+\trm -rf dst &&\n+\tgit init dst &&\n+\t(\n+\t\tcd dst &&\n+\t\tgit config fetch.fsckobjects false &&\n+\t\tgit config transfer.fsckobjects false &&\n+\t\tgit fetch ../.git master\n+\t)\n+'\n+\n+test_expect_success 'fetch with !fetch.fsckobjects' '\n+\trm -rf dst &&\n+\tgit init dst &&\n+\t(\n+\t\tcd dst &&\n+\t\tgit config fetch.fsckobjects false &&\n+\t\tgit config transfer.fsckobjects true &&\n+\t\tgit fetch ../.git master\n+\t)\n+'\n+\n+test_expect_success 'fetch with fetch.fsckobjects' '\n+\trm -rf dst &&\n+\tgit init dst &&\n+\t(\n+\t\tcd dst &&\n+\t\tgit config fetch.fsckobjects true &&\n+\t\tgit config transfer.fsckobjects false &&\n+\t\ttest_must_fail git fetch ../.git master\n+\t)\n+'\n+\n+test_expect_success 'fetch with transfer.fsckobjects' '\n+\trm -rf dst &&\n+\tgit init dst &&\n+\t(\n+\t\tcd dst &&\n+\t\tgit config transfer.fsckobjects true &&\n+\t\ttest_must_fail git fetch ../.git master\n+\t)\n+'\n+\n+test_expect_success 'push without strict' '\n+\trm -rf dst &&\n+\tgit init dst &&\n+\t(\n+\t\tcd dst &&\n+\t\tgit config fetch.fsckobjects false &&\n+\t\tgit config transfer.fsckobjects false\n+\t) &&\n+\tgit push dst master:refs/heads/test\n+'\n+\n+test_expect_success 'push with !receive.fsckobjects' '\n+\trm -rf dst &&\n+\tgit init dst &&\n+\t(\n+\t\tcd dst &&\n+\t\tgit config receive.fsckobjects false &&\n+\t\tgit config transfer.fsckobjects true\n+\t) &&\n+\tgit push dst master:refs/heads/test\n+'\n+\n+test_expect_success 'push with receive.fsckobjects' '\n+\trm -rf dst &&\n+\tgit init dst &&\n+\t(\n+\t\tcd dst &&\n+\t\tgit config receive.fsckobjects true &&\n+\t\tgit config transfer.fsckobjects false\n+\t) &&\n+\ttest_must_fail git push dst master:refs/heads/test\n+'\n+\n+test_expect_success 'push with transfer.fsckobjects' '\n+\trm -rf dst &&\n+\tgit init dst &&\n+\t(\n+\t\tcd dst &&\n+\t\tgit config transfer.fsckobjects true\n+\t) &&\n+\ttest_must_fail git push dst master:refs/heads/test\n+'\n+\n+test_done\n-- \n1.7.7.rc0.175.gb3212\n"}]}