{"thread":{"id":"9082","subject":"[PATCH 1/3] Move bundle specific stuff into bundle.[ch]","startedAt":"2007-07-17T22:49:23Z","lastAt":"2007-07-18T16:33:26Z","messageCount":11,"participants":["Johannes Schindelin","Daniel Barkalow","Shawn O. Pearce","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"47681","messageId":"Pine.LNX.4.64.0707172346450.14781@racer.site","threadId":"9082","inReplyTo":null,"subject":"[PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-17T22:49:23Z","receivedAt":"2007-07-17T22:49:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThe transport specific stuff was moved into libgit.a, and the\nbundle specific stuff will not be left behind.\n\nThis is a big code move, with one exception: the function\nunbundle() no longer outputs the list of refs.  You have to call\nlist_bundle_refs() yourself for that.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tFWIW this patch series is based on the bf_series branch of\n\tgit://iabervon.org/~barkalow/git.git, revision a204e4b4.\n\n Makefile         |    2 +-\n builtin-bundle.c |  330 +-----------------------------------------------------\n bundle.c         |  310 ++++++++++++++++++++++++++++++++++++++++++++++++++\n bundle.h         |   25 ++++\n 4 files changed, 342 insertions(+), 325 deletions(-)\n create mode 100644 bundle.c\n create mode 100644 bundle.h\n\ndiff --git a/Makefile b/Makefile\nindex 335be8f..a389ee7 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -318,7 +318,7 @@ LIB_OBJS = \\\n \talloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \\\n \tcolor.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \\\n \tconvert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \\\n-\ttransport.o\n+\ttransport.o bundle.o\n \n BUILTIN_OBJS = \\\n \tbuiltin-add.o \\\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex 306ad29..22d03a3 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -1,10 +1,5 @@\n #include \"cache.h\"\n-#include \"object.h\"\n-#include \"commit.h\"\n-#include \"diff.h\"\n-#include \"revision.h\"\n-#include \"list-objects.h\"\n-#include \"run-command.h\"\n+#include \"bundle.h\"\n \n /*\n  * Basic handler for bundle files to connect repositories via sneakernet.\n@@ -15,320 +10,6 @@\n \n static const char *bundle_usage=\"git-bundle (create <bundle> <git-rev-list args> | verify <bundle> | list-heads <bundle> [refname]... | unbundle <bundle> [refname]... )\";\n \n-static const char bundle_signature[] = \"# v2 git bundle\\n\";\n-\n-struct ref_list {\n-\tunsigned int nr, alloc;\n-\tstruct ref_list_entry {\n-\t\tunsigned char sha1[20];\n-\t\tchar *name;\n-\t} *list;\n-};\n-\n-static void add_to_ref_list(const unsigned char *sha1, const char *name,\n-\t\tstruct ref_list *list)\n-{\n-\tif (list->nr + 1 >= list->alloc) {\n-\t\tlist->alloc = alloc_nr(list->nr + 1);\n-\t\tlist->list = xrealloc(list->list,\n-\t\t\t\tlist->alloc * sizeof(list->list[0]));\n-\t}\n-\tmemcpy(list->list[list->nr].sha1, sha1, 20);\n-\tlist->list[list->nr].name = xstrdup(name);\n-\tlist->nr++;\n-}\n-\n-struct bundle_header {\n-\tstruct ref_list prerequisites;\n-\tstruct ref_list references;\n-};\n-\n-/* this function returns the length of the string */\n-static int read_string(int fd, char *buffer, int size)\n-{\n-\tint i;\n-\tfor (i = 0; i < size - 1; i++) {\n-\t\tssize_t count = xread(fd, buffer + i, 1);\n-\t\tif (count < 0)\n-\t\t\treturn error(\"Read error: %s\", strerror(errno));\n-\t\tif (count == 0) {\n-\t\t\ti--;\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (buffer[i] == '\\n')\n-\t\t\tbreak;\n-\t}\n-\tbuffer[i + 1] = '\\0';\n-\treturn i + 1;\n-}\n-\n-/* returns an fd */\n-static int read_header(const char *path, struct bundle_header *header) {\n-\tchar buffer[1024];\n-\tint fd = open(path, O_RDONLY);\n-\n-\tif (fd < 0)\n-\t\treturn error(\"could not open '%s'\", path);\n-\tif (read_string(fd, buffer, sizeof(buffer)) < 0 ||\n-\t\t\tstrcmp(buffer, bundle_signature)) {\n-\t\tclose(fd);\n-\t\treturn error(\"'%s' does not look like a v2 bundle file\", path);\n-\t}\n-\twhile (read_string(fd, buffer, sizeof(buffer)) > 0\n-\t\t\t&& buffer[0] != '\\n') {\n-\t\tint is_prereq = buffer[0] == '-';\n-\t\tint offset = is_prereq ? 1 : 0;\n-\t\tint len = strlen(buffer);\n-\t\tunsigned char sha1[20];\n-\t\tstruct ref_list *list = is_prereq ? &header->prerequisites\n-\t\t\t: &header->references;\n-\t\tchar delim;\n-\n-\t\tif (buffer[len - 1] == '\\n')\n-\t\t\tbuffer[len - 1] = '\\0';\n-\t\tif (get_sha1_hex(buffer + offset, sha1)) {\n-\t\t\twarning(\"unrecognized header: %s\", buffer);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tdelim = buffer[40 + offset];\n-\t\tif (!isspace(delim) && (delim != '\\0' || !is_prereq))\n-\t\t\tdie (\"invalid header: %s\", buffer);\n-\t\tadd_to_ref_list(sha1, isspace(delim) ?\n-\t\t\t\tbuffer + 41 + offset : \"\", list);\n-\t}\n-\treturn fd;\n-}\n-\n-static int list_refs(struct ref_list *r, int argc, const char **argv)\n-{\n-\tint i;\n-\n-\tfor (i = 0; i < r->nr; i++) {\n-\t\tif (argc > 1) {\n-\t\t\tint j;\n-\t\t\tfor (j = 1; j < argc; j++)\n-\t\t\t\tif (!strcmp(r->list[i].name, argv[j]))\n-\t\t\t\t\tbreak;\n-\t\t\tif (j == argc)\n-\t\t\t\tcontinue;\n-\t\t}\n-\t\tprintf(\"%s %s\\n\", sha1_to_hex(r->list[i].sha1),\n-\t\t\t\tr->list[i].name);\n-\t}\n-\treturn 0;\n-}\n-\n-#define PREREQ_MARK (1u<<16)\n-\n-static int verify_bundle(struct bundle_header *header, int verbose)\n-{\n-\t/*\n-\t * Do fast check, then if any prereqs are missing then go line by line\n-\t * to be verbose about the errors\n-\t */\n-\tstruct ref_list *p = &header->prerequisites;\n-\tstruct rev_info revs;\n-\tconst char *argv[] = {NULL, \"--all\"};\n-\tstruct object_array refs;\n-\tstruct commit *commit;\n-\tint i, ret = 0, req_nr;\n-\tconst char *message = \"Repository lacks these prerequisite commits:\";\n-\n-\tinit_revisions(&revs, NULL);\n-\tfor (i = 0; i < p->nr; i++) {\n-\t\tstruct ref_list_entry *e = p->list + i;\n-\t\tstruct object *o = parse_object(e->sha1);\n-\t\tif (o) {\n-\t\t\to->flags |= PREREQ_MARK;\n-\t\t\tadd_pending_object(&revs, o, e->name);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (++ret == 1)\n-\t\t\terror(message);\n-\t\terror(\"%s %s\", sha1_to_hex(e->sha1), e->name);\n-\t}\n-\tif (revs.pending.nr != p->nr)\n-\t\treturn ret;\n-\treq_nr = revs.pending.nr;\n-\tsetup_revisions(2, argv, &revs, NULL);\n-\n-\tmemset(&refs, 0, sizeof(struct object_array));\n-\tfor (i = 0; i < revs.pending.nr; i++) {\n-\t\tstruct object_array_entry *e = revs.pending.objects + i;\n-\t\tadd_object_array(e->item, e->name, &refs);\n-\t}\n-\n-\tprepare_revision_walk(&revs);\n-\n-\ti = req_nr;\n-\twhile (i && (commit = get_revision(&revs)))\n-\t\tif (commit->object.flags & PREREQ_MARK)\n-\t\t\ti--;\n-\n-\tfor (i = 0; i < req_nr; i++)\n-\t\tif (!(refs.objects[i].item->flags & SHOWN)) {\n-\t\t\tif (++ret == 1)\n-\t\t\t\terror(message);\n-\t\t\terror(\"%s %s\", sha1_to_hex(refs.objects[i].item->sha1),\n-\t\t\t\trefs.objects[i].name);\n-\t\t}\n-\n-\tfor (i = 0; i < refs.nr; i++)\n-\t\tclear_commit_marks((struct commit *)refs.objects[i].item, -1);\n-\n-\tif (verbose) {\n-\t\tstruct ref_list *r;\n-\n-\t\tr = &header->references;\n-\t\tprintf(\"The bundle contains %d ref%s\\n\",\n-\t\t       r->nr, (1 < r->nr) ? \"s\" : \"\");\n-\t\tlist_refs(r, 0, NULL);\n-\t\tr = &header->prerequisites;\n-\t\tprintf(\"The bundle requires these %d ref%s\\n\",\n-\t\t       r->nr, (1 < r->nr) ? \"s\" : \"\");\n-\t\tlist_refs(r, 0, NULL);\n-\t}\n-\treturn ret;\n-}\n-\n-static int list_heads(struct bundle_header *header, int argc, const char **argv)\n-{\n-\treturn list_refs(&header->references, argc, argv);\n-}\n-\n-static int create_bundle(struct bundle_header *header, const char *path,\n-\t\tint argc, const char **argv)\n-{\n-\tint bundle_fd = -1;\n-\tconst char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));\n-\tconst char **argv_pack = xmalloc(5 * sizeof(const char *));\n-\tint i, ref_count = 0;\n-\tchar buffer[1024];\n-\tstruct rev_info revs;\n-\tstruct child_process rls;\n-\n-\tbundle_fd = (!strcmp(path, \"-\") ? 1 :\n-\t\t\topen(path, O_CREAT | O_EXCL | O_WRONLY, 0666));\n-\tif (bundle_fd < 0)\n-\t\treturn error(\"Could not create '%s': %s\", path, strerror(errno));\n-\n-\t/* write signature */\n-\twrite_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));\n-\n-\t/* init revs to list objects for pack-objects later */\n-\tsave_commit_buffer = 0;\n-\tinit_revisions(&revs, NULL);\n-\n-\t/* write prerequisites */\n-\tmemcpy(argv_boundary + 3, argv + 1, argc * sizeof(const char *));\n-\targv_boundary[0] = \"rev-list\";\n-\targv_boundary[1] = \"--boundary\";\n-\targv_boundary[2] = \"--pretty=oneline\";\n-\targv_boundary[argc + 2] = NULL;\n-\tmemset(&rls, 0, sizeof(rls));\n-\trls.argv = argv_boundary;\n-\trls.out = -1;\n-\trls.git_cmd = 1;\n-\tif (start_command(&rls))\n-\t\treturn -1;\n-\twhile ((i = read_string(rls.out, buffer, sizeof(buffer))) > 0) {\n-\t\tunsigned char sha1[20];\n-\t\tif (buffer[0] == '-') {\n-\t\t\twrite_or_die(bundle_fd, buffer, i);\n-\t\t\tif (!get_sha1_hex(buffer + 1, sha1)) {\n-\t\t\t\tstruct object *object = parse_object(sha1);\n-\t\t\t\tobject->flags |= UNINTERESTING;\n-\t\t\t\tadd_pending_object(&revs, object, buffer);\n-\t\t\t}\n-\t\t} else if (!get_sha1_hex(buffer, sha1)) {\n-\t\t\tstruct object *object = parse_object(sha1);\n-\t\t\tobject->flags |= SHOWN;\n-\t\t}\n-\t}\n-\tif (finish_command(&rls))\n-\t\treturn error(\"rev-list died\");\n-\n-\t/* write references */\n-\targc = setup_revisions(argc, argv, &revs, NULL);\n-\tif (argc > 1)\n-\t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n-\n-\tfor (i = 0; i < revs.pending.nr; i++) {\n-\t\tstruct object_array_entry *e = revs.pending.objects + i;\n-\t\tunsigned char sha1[20];\n-\t\tchar *ref;\n-\n-\t\tif (e->item->flags & UNINTERESTING)\n-\t\t\tcontinue;\n-\t\tif (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)\n-\t\t\tcontinue;\n-\t\t/*\n-\t\t * Make sure the refs we wrote out is correct; --max-count and\n-\t\t * other limiting options could have prevented all the tips\n-\t\t * from getting output.\n-\t\t */\n-\t\tif (!(e->item->flags & SHOWN)) {\n-\t\t\twarning(\"ref '%s' is excluded by the rev-list options\",\n-\t\t\t\te->name);\n-\t\t\tcontinue;\n-\t\t}\n-\t\tref_count++;\n-\t\twrite_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);\n-\t\twrite_or_die(bundle_fd, \" \", 1);\n-\t\twrite_or_die(bundle_fd, ref, strlen(ref));\n-\t\twrite_or_die(bundle_fd, \"\\n\", 1);\n-\t\tfree(ref);\n-\t}\n-\tif (!ref_count)\n-\t\tdie (\"Refusing to create empty bundle.\");\n-\n-\t/* end header */\n-\twrite_or_die(bundle_fd, \"\\n\", 1);\n-\n-\t/* write pack */\n-\targv_pack[0] = \"pack-objects\";\n-\targv_pack[1] = \"--all-progress\";\n-\targv_pack[2] = \"--stdout\";\n-\targv_pack[3] = \"--thin\";\n-\targv_pack[4] = NULL;\n-\tmemset(&rls, 0, sizeof(rls));\n-\trls.argv = argv_pack;\n-\trls.in = -1;\n-\trls.out = bundle_fd;\n-\trls.git_cmd = 1;\n-\tif (start_command(&rls))\n-\t\treturn error(\"Could not spawn pack-objects\");\n-\tfor (i = 0; i < revs.pending.nr; i++) {\n-\t\tstruct object *object = revs.pending.objects[i].item;\n-\t\tif (object->flags & UNINTERESTING)\n-\t\t\twrite(rls.in, \"^\", 1);\n-\t\twrite(rls.in, sha1_to_hex(object->sha1), 40);\n-\t\twrite(rls.in, \"\\n\", 1);\n-\t}\n-\tif (finish_command(&rls))\n-\t\treturn error (\"pack-objects died\");\n-\treturn 0;\n-}\n-\n-static int unbundle(struct bundle_header *header, int bundle_fd,\n-\t\tint argc, const char **argv)\n-{\n-\tconst char *argv_index_pack[] = {\"index-pack\",\n-\t\t\"--fix-thin\", \"--stdin\", NULL};\n-\tstruct child_process ip;\n-\n-\tif (verify_bundle(header, 0))\n-\t\treturn -1;\n-\tmemset(&ip, 0, sizeof(ip));\n-\tip.argv = argv_index_pack;\n-\tip.in = bundle_fd;\n-\tip.no_stdout = 1;\n-\tip.git_cmd = 1;\n-\tif (run_command(&ip))\n-\t\treturn error(\"index-pack died\");\n-\treturn list_heads(header, argc, argv);\n-}\n-\n int cmd_bundle(int argc, const char **argv, const char *prefix)\n {\n \tstruct bundle_header header;\n@@ -352,8 +33,8 @@ int cmd_bundle(int argc, const char **argv, const char *prefix)\n \t}\n \n \tmemset(&header, 0, sizeof(header));\n-\tif (strcmp(cmd, \"create\") &&\n-\t\t\t(bundle_fd = read_header(bundle_file, &header)) < 0)\n+\tif (strcmp(cmd, \"create\") && (bundle_fd =\n+\t\t\t\tread_bundle_header(bundle_file, &header)) < 0)\n \t\treturn 1;\n \n \tif (!strcmp(cmd, \"verify\")) {\n@@ -365,7 +46,7 @@ int cmd_bundle(int argc, const char **argv, const char *prefix)\n \t}\n \tif (!strcmp(cmd, \"list-heads\")) {\n \t\tclose(bundle_fd);\n-\t\treturn !!list_heads(&header, argc, argv);\n+\t\treturn !!list_bundle_refs(&header, argc, argv);\n \t}\n \tif (!strcmp(cmd, \"create\")) {\n \t\tif (nongit)\n@@ -374,7 +55,8 @@ int cmd_bundle(int argc, const char **argv, const char *prefix)\n \t} else if (!strcmp(cmd, \"unbundle\")) {\n \t\tif (nongit)\n \t\t\tdie(\"Need a repository to unbundle.\");\n-\t\treturn !!unbundle(&header, bundle_fd, argc, argv);\n+\t\treturn !!unbundle(&header, bundle_fd) ||\n+\t\t\tlist_bundle_refs(&header, argc, argv);\n \t} else\n \t\tusage(bundle_usage);\n }\ndiff --git a/bundle.c b/bundle.c\nnew file mode 100644\nindex 0000000..659b61d\n--- /dev/null\n+++ b/bundle.c\n@@ -0,0 +1,310 @@\n+#include \"cache.h\"\n+#include \"bundle.h\"\n+#include \"object.h\"\n+#include \"commit.h\"\n+#include \"diff.h\"\n+#include \"revision.h\"\n+#include \"list-objects.h\"\n+#include \"run-command.h\"\n+\n+static const char bundle_signature[] = \"# v2 git bundle\\n\";\n+\n+static void add_to_ref_list(const unsigned char *sha1, const char *name,\n+\t\tstruct ref_list *list)\n+{\n+\tif (list->nr + 1 >= list->alloc) {\n+\t\tlist->alloc = alloc_nr(list->nr + 1);\n+\t\tlist->list = xrealloc(list->list,\n+\t\t\t\tlist->alloc * sizeof(list->list[0]));\n+\t}\n+\tmemcpy(list->list[list->nr].sha1, sha1, 20);\n+\tlist->list[list->nr].name = xstrdup(name);\n+\tlist->nr++;\n+}\n+\n+/* this function returns the length of the string */\n+static int read_string(int fd, char *buffer, int size)\n+{\n+\tint i;\n+\tfor (i = 0; i < size - 1; i++) {\n+\t\tssize_t count = xread(fd, buffer + i, 1);\n+\t\tif (count < 0)\n+\t\t\treturn error(\"Read error: %s\", strerror(errno));\n+\t\tif (count == 0) {\n+\t\t\ti--;\n+\t\t\tbreak;\n+\t\t}\n+\t\tif (buffer[i] == '\\n')\n+\t\t\tbreak;\n+\t}\n+\tbuffer[i + 1] = '\\0';\n+\treturn i + 1;\n+}\n+\n+/* returns an fd */\n+int read_bundle_header(const char *path, struct bundle_header *header) {\n+\tchar buffer[1024];\n+\tint fd = open(path, O_RDONLY);\n+\n+\tif (fd < 0)\n+\t\treturn error(\"could not open '%s'\", path);\n+\tif (read_string(fd, buffer, sizeof(buffer)) < 0 ||\n+\t\t\tstrcmp(buffer, bundle_signature)) {\n+\t\tclose(fd);\n+\t\treturn error(\"'%s' does not look like a v2 bundle file\", path);\n+\t}\n+\twhile (read_string(fd, buffer, sizeof(buffer)) > 0\n+\t\t\t&& buffer[0] != '\\n') {\n+\t\tint is_prereq = buffer[0] == '-';\n+\t\tint offset = is_prereq ? 1 : 0;\n+\t\tint len = strlen(buffer);\n+\t\tunsigned char sha1[20];\n+\t\tstruct ref_list *list = is_prereq ? &header->prerequisites\n+\t\t\t: &header->references;\n+\t\tchar delim;\n+\n+\t\tif (buffer[len - 1] == '\\n')\n+\t\t\tbuffer[len - 1] = '\\0';\n+\t\tif (get_sha1_hex(buffer + offset, sha1)) {\n+\t\t\twarning(\"unrecognized header: %s\", buffer);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tdelim = buffer[40 + offset];\n+\t\tif (!isspace(delim) && (delim != '\\0' || !is_prereq))\n+\t\t\tdie (\"invalid header: %s\", buffer);\n+\t\tadd_to_ref_list(sha1, isspace(delim) ?\n+\t\t\t\tbuffer + 41 + offset : \"\", list);\n+\t}\n+\treturn fd;\n+}\n+\n+int create_bundle(struct bundle_header *header, const char *path,\n+\t\tint argc, const char **argv)\n+{\n+\tint bundle_fd = -1;\n+\tconst char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));\n+\tconst char **argv_pack = xmalloc(5 * sizeof(const char *));\n+\tint i, ref_count = 0;\n+\tchar buffer[1024];\n+\tstruct rev_info revs;\n+\tstruct child_process rls;\n+\n+\tbundle_fd = (!strcmp(path, \"-\") ? 1 :\n+\t\t\topen(path, O_CREAT | O_EXCL | O_WRONLY, 0666));\n+\tif (bundle_fd < 0)\n+\t\treturn error(\"Could not create '%s': %s\", path, strerror(errno));\n+\n+\t/* write signature */\n+\twrite_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));\n+\n+\t/* init revs to list objects for pack-objects later */\n+\tsave_commit_buffer = 0;\n+\tinit_revisions(&revs, NULL);\n+\n+\t/* write prerequisites */\n+\tmemcpy(argv_boundary + 3, argv + 1, argc * sizeof(const char *));\n+\targv_boundary[0] = \"rev-list\";\n+\targv_boundary[1] = \"--boundary\";\n+\targv_boundary[2] = \"--pretty=oneline\";\n+\targv_boundary[argc + 2] = NULL;\n+\tmemset(&rls, 0, sizeof(rls));\n+\trls.argv = argv_boundary;\n+\trls.out = -1;\n+\trls.git_cmd = 1;\n+\tif (start_command(&rls))\n+\t\treturn -1;\n+\twhile ((i = read_string(rls.out, buffer, sizeof(buffer))) > 0) {\n+\t\tunsigned char sha1[20];\n+\t\tif (buffer[0] == '-') {\n+\t\t\twrite_or_die(bundle_fd, buffer, i);\n+\t\t\tif (!get_sha1_hex(buffer + 1, sha1)) {\n+\t\t\t\tstruct object *object = parse_object(sha1);\n+\t\t\t\tobject->flags |= UNINTERESTING;\n+\t\t\t\tadd_pending_object(&revs, object, buffer);\n+\t\t\t}\n+\t\t} else if (!get_sha1_hex(buffer, sha1)) {\n+\t\t\tstruct object *object = parse_object(sha1);\n+\t\t\tobject->flags |= SHOWN;\n+\t\t}\n+\t}\n+\tif (finish_command(&rls))\n+\t\treturn error(\"rev-list died\");\n+\n+\t/* write references */\n+\targc = setup_revisions(argc, argv, &revs, NULL);\n+\tif (argc > 1)\n+\t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n+\n+\tfor (i = 0; i < revs.pending.nr; i++) {\n+\t\tstruct object_array_entry *e = revs.pending.objects + i;\n+\t\tunsigned char sha1[20];\n+\t\tchar *ref;\n+\n+\t\tif (e->item->flags & UNINTERESTING)\n+\t\t\tcontinue;\n+\t\tif (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)\n+\t\t\tcontinue;\n+\t\t/*\n+\t\t * Make sure the refs we wrote out is correct; --max-count and\n+\t\t * other limiting options could have prevented all the tips\n+\t\t * from getting output.\n+\t\t */\n+\t\tif (!(e->item->flags & SHOWN)) {\n+\t\t\twarning(\"ref '%s' is excluded by the rev-list options\",\n+\t\t\t\te->name);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tref_count++;\n+\t\twrite_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);\n+\t\twrite_or_die(bundle_fd, \" \", 1);\n+\t\twrite_or_die(bundle_fd, ref, strlen(ref));\n+\t\twrite_or_die(bundle_fd, \"\\n\", 1);\n+\t\tfree(ref);\n+\t}\n+\tif (!ref_count)\n+\t\tdie (\"Refusing to create empty bundle.\");\n+\n+\t/* end header */\n+\twrite_or_die(bundle_fd, \"\\n\", 1);\n+\n+\t/* write pack */\n+\targv_pack[0] = \"pack-objects\";\n+\targv_pack[1] = \"--all-progress\";\n+\targv_pack[2] = \"--stdout\";\n+\targv_pack[3] = \"--thin\";\n+\targv_pack[4] = NULL;\n+\tmemset(&rls, 0, sizeof(rls));\n+\trls.argv = argv_pack;\n+\trls.in = -1;\n+\trls.out = bundle_fd;\n+\trls.git_cmd = 1;\n+\tif (start_command(&rls))\n+\t\treturn error(\"Could not spawn pack-objects\");\n+\tfor (i = 0; i < revs.pending.nr; i++) {\n+\t\tstruct object *object = revs.pending.objects[i].item;\n+\t\tif (object->flags & UNINTERESTING)\n+\t\t\twrite(rls.in, \"^\", 1);\n+\t\twrite(rls.in, sha1_to_hex(object->sha1), 40);\n+\t\twrite(rls.in, \"\\n\", 1);\n+\t}\n+\tif (finish_command(&rls))\n+\t\treturn error (\"pack-objects died\");\n+\treturn 0;\n+}\n+\n+static int list_refs(struct ref_list *r, int argc, const char **argv)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < r->nr; i++) {\n+\t\tif (argc > 1) {\n+\t\t\tint j;\n+\t\t\tfor (j = 1; j < argc; j++)\n+\t\t\t\tif (!strcmp(r->list[i].name, argv[j]))\n+\t\t\t\t\tbreak;\n+\t\t\tif (j == argc)\n+\t\t\t\tcontinue;\n+\t\t}\n+\t\tprintf(\"%s %s\\n\", sha1_to_hex(r->list[i].sha1),\n+\t\t\t\tr->list[i].name);\n+\t}\n+\treturn 0;\n+}\n+\n+int list_bundle_refs(struct bundle_header *header, int argc, const char **argv)\n+{\n+\treturn list_refs(&header->references, argc, argv);\n+}\n+\n+#define PREREQ_MARK (1u<<16)\n+\n+int verify_bundle(struct bundle_header *header, int verbose)\n+{\n+\t/*\n+\t * Do fast check, then if any prereqs are missing then go line by line\n+\t * to be verbose about the errors\n+\t */\n+\tstruct ref_list *p = &header->prerequisites;\n+\tstruct rev_info revs;\n+\tconst char *argv[] = {NULL, \"--all\"};\n+\tstruct object_array refs;\n+\tstruct commit *commit;\n+\tint i, ret = 0, req_nr;\n+\tconst char *message = \"Repository lacks these prerequisite commits:\";\n+\n+\tinit_revisions(&revs, NULL);\n+\tfor (i = 0; i < p->nr; i++) {\n+\t\tstruct ref_list_entry *e = p->list + i;\n+\t\tstruct object *o = parse_object(e->sha1);\n+\t\tif (o) {\n+\t\t\to->flags |= PREREQ_MARK;\n+\t\t\tadd_pending_object(&revs, o, e->name);\n+\t\t\tcontinue;\n+\t\t}\n+\t\tif (++ret == 1)\n+\t\t\terror(message);\n+\t\terror(\"%s %s\", sha1_to_hex(e->sha1), e->name);\n+\t}\n+\tif (revs.pending.nr != p->nr)\n+\t\treturn ret;\n+\treq_nr = revs.pending.nr;\n+\tsetup_revisions(2, argv, &revs, NULL);\n+\n+\tmemset(&refs, 0, sizeof(struct object_array));\n+\tfor (i = 0; i < revs.pending.nr; i++) {\n+\t\tstruct object_array_entry *e = revs.pending.objects + i;\n+\t\tadd_object_array(e->item, e->name, &refs);\n+\t}\n+\n+\tprepare_revision_walk(&revs);\n+\n+\ti = req_nr;\n+\twhile (i && (commit = get_revision(&revs)))\n+\t\tif (commit->object.flags & PREREQ_MARK)\n+\t\t\ti--;\n+\n+\tfor (i = 0; i < req_nr; i++)\n+\t\tif (!(refs.objects[i].item->flags & SHOWN)) {\n+\t\t\tif (++ret == 1)\n+\t\t\t\terror(message);\n+\t\t\terror(\"%s %s\", sha1_to_hex(refs.objects[i].item->sha1),\n+\t\t\t\trefs.objects[i].name);\n+\t\t}\n+\n+\tfor (i = 0; i < refs.nr; i++)\n+\t\tclear_commit_marks((struct commit *)refs.objects[i].item, -1);\n+\n+\tif (verbose) {\n+\t\tstruct ref_list *r;\n+\n+\t\tr = &header->references;\n+\t\tprintf(\"The bundle contains %d ref%s\\n\",\n+\t\t       r->nr, (1 < r->nr) ? \"s\" : \"\");\n+\t\tlist_refs(r, 0, NULL);\n+\t\tr = &header->prerequisites;\n+\t\tprintf(\"The bundle requires these %d ref%s\\n\",\n+\t\t       r->nr, (1 < r->nr) ? \"s\" : \"\");\n+\t\tlist_refs(r, 0, NULL);\n+\t}\n+\treturn ret;\n+}\n+\n+int unbundle(struct bundle_header *header, int bundle_fd)\n+{\n+\tconst char *argv_index_pack[] = {\"index-pack\",\n+\t\t\"--fix-thin\", \"--stdin\", NULL};\n+\tstruct child_process ip;\n+\n+\tif (verify_bundle(header, 0))\n+\t\treturn -1;\n+\tmemset(&ip, 0, sizeof(ip));\n+\tip.argv = argv_index_pack;\n+\tip.in = bundle_fd;\n+\tip.no_stdout = 1;\n+\tip.git_cmd = 1;\n+\tif (run_command(&ip))\n+\t\treturn error(\"index-pack died\");\n+\treturn 0;\n+}\n+\n+\ndiff --git a/bundle.h b/bundle.h\nnew file mode 100644\nindex 0000000..e2aedd6\n--- /dev/null\n+++ b/bundle.h\n@@ -0,0 +1,25 @@\n+#ifndef BUNDLE_H\n+#define BUNDLE_H\n+\n+struct ref_list {\n+\tunsigned int nr, alloc;\n+\tstruct ref_list_entry {\n+\t\tunsigned char sha1[20];\n+\t\tchar *name;\n+\t} *list;\n+};\n+\n+struct bundle_header {\n+\tstruct ref_list prerequisites;\n+\tstruct ref_list references;\n+};\n+\n+int read_bundle_header(const char *path, struct bundle_header *header);\n+int create_bundle(struct bundle_header *header, const char *path,\n+\t\tint argc, const char **argv);\n+int verify_bundle(struct bundle_header *header, int verbose);\n+int unbundle(struct bundle_header *header, int bundle_fd);\n+int list_bundle_refs(struct bundle_header *header,\n+\t\tint argc, const char **argv);\n+\n+#endif\n-- \n1.5.3.rc1.16.g9d6f-dirty\n"},{"id":"47694","messageId":"Pine.LNX.4.64.0707172216420.14596@iabervon.org","threadId":"9082","inReplyTo":"Pine.LNX.4.64.0707172346450.14781@racer.site","subject":"Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-07-18T02:48:05Z","receivedAt":"2007-07-18T02:48:05Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 17 Jul 2007, Johannes Schindelin wrote:\n\n> The transport specific stuff was moved into libgit.a, and the\n> bundle specific stuff will not be left behind.\n> \n> This is a big code move, with one exception: the function\n> unbundle() no longer outputs the list of refs.  You have to call\n> list_bundle_refs() yourself for that.\n\nYou should use -C on this sort of thing, so that the interesting aspects \nof the patch are easier to see. (It actually comes out longer in this \ncase, but it's far easier to tell that the code in the new file is the \nsame as the old code.) Can you tell I've been rearranging a lot of code \nlately and trying to make the patches not look really scary?\n\nAside from presentation, it looks good to me. Shall I stick the bundle \nchanges into my series? I'd like to have them come before the patch to \nswitch to builtin-fetch, so that there aren't any revisions where \"git \nfetch\" doesn't have bundle support.\n\nAnd I think it would be best to take part 3 as a review fix to my final \npatch.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"47697","messageId":"Pine.LNX.4.64.0707172302560.14596@iabervon.org","threadId":"9082","inReplyTo":"Pine.LNX.4.64.0707172216420.14596@iabervon.org","subject":"Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-07-18T03:23:49Z","receivedAt":"2007-07-18T03:23:49Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Tue, 17 Jul 2007, Daniel Barkalow wrote:\n\n> On Tue, 17 Jul 2007, Johannes Schindelin wrote:\n> \n> > The transport specific stuff was moved into libgit.a, and the\n> > bundle specific stuff will not be left behind.\n> > \n> > This is a big code move, with one exception: the function\n> > unbundle() no longer outputs the list of refs.  You have to call\n> > list_bundle_refs() yourself for that.\n> \n> You should use -C on this sort of thing, so that the interesting aspects \n> of the patch are easier to see. (It actually comes out longer in this \n> case, but it's far easier to tell that the code in the new file is the \n> same as the old code.) Can you tell I've been rearranging a lot of code \n> lately and trying to make the patches not look really scary?\n\nActually, I ended up touching this up a tiny bit, too: I ordered the \nfunctions in bundle.c the way they were in builtin-bundle.c (so that the \npatch is more trivial) and removed the blank lines at the end of the file. \nThis makes the \"git diff -C\" output really obvious. \n\n(Someday, I'd like to have a diff that can show that a substantial block \nof '+' lines matches a block of lines from somewhere in the \"before\" \ncontent, so reviewers can verify that the patch reorders code but doesn't \nchange it, or changes it in certain ways. But, of course, that's both hard \nto generate and hard to display usefully.)\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"47698","messageId":"20070718032907.GY32566@spearce.org","threadId":"9082","inReplyTo":"Pine.LNX.4.64.0707172302560.14596@iabervon.org","subject":"Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-07-18T03:29:07Z","receivedAt":"2007-07-18T03:29:07Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Daniel Barkalow <barkalow@iabervon.org> wrote:\n> (Someday, I'd like to have a diff that can show that a substantial block \n> of '+' lines matches a block of lines from somewhere in the \"before\" \n> content, so reviewers can verify that the patch reorders code but doesn't \n> change it, or changes it in certain ways. But, of course, that's both hard \n> to generate and hard to display usefully.)\n\nI just moved a huge block in git-gui so I have a great example...\n\nWhat about a patch format like this?  I doubt people move more than\n26 blocks in the same patch of the same file, so using a single\ncharacter block prefix when the before/after images are identical\nmight work OK.\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 0443129..f13fa80 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\na@@ -632,6 +632,43 a@@ You are using [git-version]:\n  \n  ######################################################################\n  ##\na+## feature option selection\na+\na+if {[regexp {^git-(.+)$} [appname] _junk subcommand]} {\na+\tunset _junk\na+} else {\na+\tset subcommand gui\na+}\na+if {$subcommand eq {gui.sh}} {\na+\tset subcommand gui\na+}\na+if {$subcommand eq {gui} && [llength $argv] > 0} {\na+\tset subcommand [lindex $argv 0]\na+\tset argv [lrange $argv 1 end]\na+}\na+\na+enable_option multicommit\na+enable_option branch\na+enable_option transport\na+\na+switch -- $subcommand {\na+browser -\na+blame {\na+\tdisable_option multicommit\na+\tdisable_option branch\na+\tdisable_option transport\na+}\na+citool {\na+\tenable_option singlecommit\na+\na+\tdisable_option multicommit\na+\tdisable_option branch\na+\tdisable_option transport\na+}\na+}\na+\na+######################################################################\na+##\n  ## repository setup\n  \n  if {[catch {\na@@ -1598,43 +1635,6 a@@ apply_config\n  \n  ######################################################################\n  ##\na-## feature option selection\na-\na-if {[regexp {^git-(.+)$} [appname] _junk subcommand]} {\na-\tunset _junk\na-} else {\na-\tset subcommand gui\na-}\na-if {$subcommand eq {gui.sh}} {\na-\tset subcommand gui\na-}\na-if {$subcommand eq {gui} && [llength $argv] > 0} {\na-\tset subcommand [lindex $argv 0]\na-\tset argv [lrange $argv 1 end]\na-}\na-\na-enable_option multicommit\na-enable_option branch\na-enable_option transport\na-\na-switch -- $subcommand {\na-browser -\na-blame {\na-\tdisable_option multicommit\na-\tdisable_option branch\na-\tdisable_option transport\na-}\na-citool {\na-\tenable_option singlecommit\na-\na-\tdisable_option multicommit\na-\tdisable_option branch\na-\tdisable_option transport\na-}\na-}\na-\na-######################################################################\na-##\n  ## ui construction\n  \n  set ui_comm {}\n-- \n1.5.3.rc1.818.g84b7\n\n\n-- \nShawn.\n"},{"id":"47702","messageId":"7vy7he8cjl.fsf@assigned-by-dhcp.cox.net","threadId":"9082","inReplyTo":"20070718032907.GY32566@spearce.org","subject":"Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-18T06:13:18Z","receivedAt":"2007-07-18T06:13:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n> What about a patch format like this?  I doubt people move more than\n> 26 blocks in the same patch of the same file, so using a single\n> character block prefix when the before/after images are identical\n> might work OK.\n>\n> diff --git a/git-gui.sh b/git-gui.sh\n> index 0443129..f13fa80 100755\n> --- a/git-gui.sh\n> +++ b/git-gui.sh\n> a@@ -632,6 +632,43 a@@ You are using [git-version]:\n>   \n>   ######################################################################\n>   ##\n> a+## feature option selection\n> ...\n> a+\n> a+######################################################################\n> a+##\n>   ## repository setup\n>   \n>   if {[catch {\n> a@@ -1598,43 +1635,6 a@@ apply_config\n>   \n>   ######################################################################\n>   ##\n> a-## feature option selection\n> ....\n> a-##\n>   ## ui construction\n>   \n>   set ui_comm {}\n\nGaah, my eyes, my *eyes*!!\n\n\truns, stays in bathroom for 10 minutes and washes, and\n        comes back...\n\nIt might not be actually so bad.  But I wonder if it would be\nmore obvious if you do not show the whole \"a-\" lines but leave\njust a marker there.  That is (ugliness of \"a@@\" and \"a-\" that\nmade me wash my eyes needs to be fixed, though -- but that is\nonly the syntax):\n\na@@ -1598,43 +1635,6 a@@ apply_config\n  \n  ######################################################################\n  ##\na-<<< Block a was originally here >>>\n  ## ui construction\n  \n  set ui_comm {}\n\nYou are coming up with a new output format that is only used\nwhen it is a straight move and nothing else, so by definition\nthere is really no need to show both removal and addition.\n"},{"id":"47722","messageId":"20070718061931.GZ32566@spearce.org","threadId":"9082","inReplyTo":"7vy7he8cjl.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-07-18T06:19:31Z","receivedAt":"2007-07-18T06:19:31Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano <gitster@pobox.com> wrote:\n> It might not be actually so bad.  But I wonder if it would be\n> more obvious if you do not show the whole \"a-\" lines but leave\n> just a marker there.  That is (ugliness of \"a@@\" and \"a-\" that\n> made me wash my eyes needs to be fixed, though -- but that is\n> only the syntax):\n> \n> a@@ -1598,43 +1635,6 a@@ apply_config\n>   \n>   ######################################################################\n>   ##\n> a-<<< Block a was originally here >>>\n>   ## ui construction\n>   \n>   set ui_comm {}\n> \n> You are coming up with a new output format that is only used\n> when it is a straight move and nothing else, so by definition\n> there is really no need to show both removal and addition.\n\nYea, this I like even better than what I posted.  Now we just need\na suck^H^H^H^Hprogrammer to implement a working prototype and see\nhow folks like more realistic diffs generated with it.  ;-)\n\n-- \nShawn.\n"},{"id":"47709","messageId":"7v644i894x.fsf@assigned-by-dhcp.cox.net","threadId":"9082","inReplyTo":"20070718061931.GZ32566@spearce.org","subject":"Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-18T07:26:54Z","receivedAt":"2007-07-18T07:26:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n> Yea, this I like even better than what I posted.  Now we just need\n> a suck^H^H^H^Hprogrammer to implement a working prototype and see\n> how folks like more realistic diffs generated with it.  ;-)\n\nI have to disgree.\n\nWe on the git list *live* in pretty much git-only world (ok, I\nam ignoring git-svn people for now ;-)), so it might feel it\nbeneficial to add a new output format to git-diff.\n\nBut many users of git generated patch needs to interact with\npatches that are not generated by git.  I think an additional\npostprocessor in patchutils/interdiff toolchest would probably\nmake much more sense.  Then a person who reviews a patch that is\nsupposed to be line movement can use the filter to verify the\nparts that should be only line movements are indeed are\nmovements and nothing else.\n\nSpeaking of \"diff\" output, what I would like to see is a support\nof 'copied context' (i.e. traditional 'context diff' format you\nwould get from \"diff -c\"), in addition to the 'unified context'\nwe support.  Generating copied context may help people who need\nto give patches to others that accept only copied context format\npatches (are there important projects that take only cpied\ncontext format patches?).  Being able to accept copied context\nformat patches in 'git-apply' would also be handy.\n\nOf course, we could use interdiff to mangle copied context\nformat into unified context format, so 'git-apply' is not a big\ndeal, though.\n"},{"id":"47717","messageId":"Pine.LNX.4.64.0707181053130.14781@racer.site","threadId":"9082","inReplyTo":"Pine.LNX.4.64.0707172216420.14596@iabervon.org","subject":"Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-18T09:56:06Z","receivedAt":"2007-07-18T09:56:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Jul 2007, Daniel Barkalow wrote:\n\n> On Tue, 17 Jul 2007, Johannes Schindelin wrote:\n> \n> > The transport specific stuff was moved into libgit.a, and the bundle \n> > specific stuff will not be left behind.\n> > \n> > This is a big code move, with one exception: the function unbundle() \n> > no longer outputs the list of refs.  You have to call \n> > list_bundle_refs() yourself for that.\n> \n> You should use -C on this sort of thing, so that the interesting aspects \n> of the patch are easier to see. (It actually comes out longer in this \n> case, but it's far easier to tell that the code in the new file is the \n> same as the old code.)\n\nOkay, I wanted it to be kept short, since I really get lost easily in \nhundreds of \"-\" lines, with possibly one in the midst being a \"+\".\n\n> Can you tell I've been rearranging a lot of code lately and trying to \n> make the patches not look really scary?\n\nSorry, I did not pay that close attention.\n\n> Aside from presentation, it looks good to me. Shall I stick the bundle \n> changes into my series? I'd like to have them come before the patch to \n> switch to builtin-fetch, so that there aren't any revisions where \"git \n> fetch\" doesn't have bundle support.\n\nLooks fine to me.  Seems like you should add a SOB line, too.\n\n> And I think it would be best to take part 3 as a review fix to my final \n> patch.\n\nYes, definitely.  This shows again (to me, at least), that just looking at \nthe code is not enough, you have to run it, too, to review patches.\n\nCiao,\nDscho\n"},{"id":"47718","messageId":"Pine.LNX.4.64.0707181056370.14781@racer.site","threadId":"9082","inReplyTo":"Pine.LNX.4.64.0707172302560.14596@iabervon.org","subject":"Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-18T10:09:09Z","receivedAt":"2007-07-18T10:09:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Jul 2007, Daniel Barkalow wrote:\n\n> On Tue, 17 Jul 2007, Daniel Barkalow wrote:\n> \n> > On Tue, 17 Jul 2007, Johannes Schindelin wrote:\n> > \n> > > The transport specific stuff was moved into libgit.a, and the bundle \n> > > specific stuff will not be left behind.\n> > > \n> > > This is a big code move, with one exception: the function unbundle() \n> > > no longer outputs the list of refs.  You have to call \n> > > list_bundle_refs() yourself for that.\n> > \n> > You should use -C on this sort of thing, so that the interesting \n> > aspects of the patch are easier to see. (It actually comes out longer \n> > in this case, but it's far easier to tell that the code in the new \n> > file is the same as the old code.) Can you tell I've been rearranging \n> > a lot of code lately and trying to make the patches not look really \n> > scary?\n> \n> Actually, I ended up touching this up a tiny bit, too: I ordered the \n> functions in bundle.c the way they were in builtin-bundle.c (so that the \n> patch is more trivial) and removed the blank lines at the end of the file. \n> This makes the \"git diff -C\" output really obvious. \n\nMakes sense.  Thanks.\n\n> (Someday, I'd like to have a diff that can show that a substantial block \n> of '+' lines matches a block of lines from somewhere in the \"before\" \n> content, so reviewers can verify that the patch reorders code but \n> doesn't change it, or changes it in certain ways. But, of course, that's \n> both hard to generate and hard to display usefully.)\n\nAFAIR from the thread after\n\nhttp://thread.gmane.org/gmane.linux.kernel/484924/focus=37498\n\nthe problem was not so much the displaying as the applying.  You have a \ndiff for builtin-bundle.c.  You want to move code to bundle.c.  git-diff \nhas to rearrange the diff so that bundle.c can copy the code from \nbuiltin-bundle.c, and then it is deleted from builtin-bundle.c.  If you \nmove code criss-cross, it is not possible.\n\nHowever, if you do something like\n\ndiff --git a/builtin-bundle.c:10--20 b/bundle.c:0--30\ncode move\n--- a/builtin-bundle.c:10--20\n+++ b/bundle.c:1--11\ndiff --git ...\n\nit should be possible.  We could allow generation of such a diff with \n--code-moves, which would detect if there is substantial evidence for a \ncode move, and produce such a diff.\n\nNote: this kind of code movement diff has to come before _both_ the diffs \nfor builtin-bundle.c and bundle.c, since chances are that the code had to \nbe touched a little here and there.  And probably you would want a little \ncontext, just to make sure it is the correct code when applying the patch.\n\nCiao,\nDscho\n"},{"id":"47748","messageId":"Pine.LNX.4.64.0707181140450.14596@iabervon.org","threadId":"9082","inReplyTo":"Pine.LNX.4.64.0707181053130.14781@racer.site","subject":"Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2007-07-18T16:25:06Z","receivedAt":"2007-07-18T16:25:06Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Wed, 18 Jul 2007, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Tue, 17 Jul 2007, Daniel Barkalow wrote:\n> \n> > You should use -C on this sort of thing, so that the interesting aspects \n> > of the patch are easier to see. (It actually comes out longer in this \n> > case, but it's far easier to tell that the code in the new file is the \n> > same as the old code.)\n> \n> Okay, I wanted it to be kept short, since I really get lost easily in \n> hundreds of \"-\" lines, with possibly one in the midst being a \"+\".\n\nActually, putting the functions in the original order made the -C diff \nshorter than without -C. In general, a hundred lines of '-' with maybe a \n'+' is hard to read, but I think whole functions of '-' without anything \nelse are easy; scan the left column, and find that the whole thing goes \naway. If a patch is discarding blocks which are exactly whole top-level \ndefinitions, those changes are probably either correct or totally broken \n(depending on whether those blocks were actually used); you just have to \nget suspicious around preprocessor stuff. Your change ended up being \ntrivially what the message described: bunch of blocks not in one or the \nother of the resulting files, some functions make not static, renamed, \nand/or had arguments changed, and the function you have to call after \nunbundle if you want it.\n\n> > Aside from presentation, it looks good to me. Shall I stick the bundle \n> > changes into my series? I'd like to have them come before the patch to \n> > switch to builtin-fetch, so that there aren't any revisions where \"git \n> > fetch\" doesn't have bundle support.\n> \n> Looks fine to me.  Seems like you should add a SOB line, too.\n\nAh, yes. I'll have to see if I'll be the first person in git development \nto have a SOB line that's neither first nor last. :)\n\n> > And I think it would be best to take part 3 as a review fix to my final \n> > patch.\n> \n> Yes, definitely.  This shows again (to me, at least), that just looking at \n> the code is not enough, you have to run it, too, to review patches.\n\nYou caught that by running it? I've been running this code, and I've never \ndone anything with it which caused fetch_refs to fail and then checked the \nresult. I thought you must have found it by looking for missing checks of \nreturn values. Or did you find it when you'd implemented half of bundle \nsupport and it didn't complain?\n"},{"id":"47749","messageId":"Pine.LNX.4.64.0707181731140.14781@racer.site","threadId":"9082","inReplyTo":"Pine.LNX.4.64.0707181140450.14596@iabervon.org","subject":"Re: [PATCH 1/3] Move bundle specific stuff into bundle.[ch]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-18T16:33:26Z","receivedAt":"2007-07-18T16:33:26Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 18 Jul 2007, Daniel Barkalow wrote:\n\n> On Wed, 18 Jul 2007, Johannes Schindelin wrote:\n> \n> > On Tue, 17 Jul 2007, Daniel Barkalow wrote:\n> > \n> > > You should use -C on this sort of thing, so that the interesting aspects \n> > > of the patch are easier to see. (It actually comes out longer in this \n> > > case, but it's far easier to tell that the code in the new file is the \n> > > same as the old code.)\n> > \n> > Okay, I wanted it to be kept short, since I really get lost easily in \n> > hundreds of \"-\" lines, with possibly one in the midst being a \"+\".\n> \n> Actually, putting the functions in the original order made the -C diff \n> shorter than without -C.\n\nTrue enough!\n\n> > > Aside from presentation, it looks good to me. Shall I stick the \n> > > bundle changes into my series? I'd like to have them come before the \n> > > patch to switch to builtin-fetch, so that there aren't any revisions \n> > > where \"git fetch\" doesn't have bundle support.\n> > \n> > Looks fine to me.  Seems like you should add a SOB line, too.\n> \n> Ah, yes. I'll have to see if I'll be the first person in git development \n> to have a SOB line that's neither first nor last. :)\n\nWe have 28 commits ATM in next's history, having 3 SOB lines: \n1e76b702c1e754c7e6df1ced9ce6f1863cb7e092 is the most recent one.\n\n> > > And I think it would be best to take part 3 as a review fix to my \n> > > final patch.\n> > \n> > Yes, definitely.  This shows again (to me, at least), that just \n> > looking at the code is not enough, you have to run it, too, to review \n> > patches.\n> \n> You caught that by running it? I've been running this code, and I've never \n> done anything with it which caused fetch_refs to fail and then checked the \n> result. I thought you must have found it by looking for missing checks of \n> return values. Or did you find it when you'd implemented half of bundle \n> support and it didn't complain?\n\nExactly.  It is the \"bundle 1\" test that failed.\n\nCiao,\nDscho\n"}]}