{"thread":{"id":"6910","subject":"[PATCH] Add git-bundle: move objects and references by archive","startedAt":"2007-02-22T00:59:14Z","lastAt":"2007-02-23T04:39:06Z","messageCount":28,"participants":["Johannes Schindelin","Nicolas Pitre","Junio C Hamano","Johannes Sixt","Simon 'corecode' Schubert","Mark Levedahl"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"35212","messageId":"Pine.LNX.4.63.0702220157130.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":null,"subject":"[PATCH] Add git-bundle: move objects and references by archive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-22T00:59:14Z","receivedAt":"2007-02-22T00:59:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nSome workflows require use of repositories on machines that cannot be \nconnected, preventing use of git-fetch / git-push to transport objects and \nreferences between the repositories.\n\ngit-bundle provides an alternate transport mechanism, effectively allowing \ngit-fetch and git-pull to operate using sneakernet transport. `git-bundle \ncreate` allows the user to create a bundle containing one or more branches \nor tags, but with specified basis assumed to exist on the target \nrepository. At the receiving end, git-bundle acts like git-fetch-pack, \nallowing the user to invoke git-fetch or git-pull using the bundle file as \nthe URL. git-fetch and git-ls-remote determine they have a bundle URL by \nchecking that the URL points to a file, but are otherwise unchanged in \noperation with bundles.\n\nThe original patch was done by Mark Levedahl .\n\nIt was updated to make git-bundle a builtin, and get rid of the tar \nformat: now, the first line is supposed to say \"# v2 git bundle\", the next \nlines either contain a prerequisite (\"-\" followed by the hash of the \nneeded commit), or a ref (the hash of a commit, followed by the name of \nthe ref), and finally the pack. As a result, the bundle argument can be \n\"-\" now.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tThis patch should apply cleanly to \"next\".\n\n\tMark, could you test on cygwin?\n\n .gitignore | 1 +\n Documentation/cmd-list.perl | 1 +\n Documentation/git-bundle.txt | 139 +++++++++++++++\n Makefile | 1 +\n builtin-bundle.c | 389 ++++++++++++++++++++++++++++++++++++++++++\n builtin.h | 1 +\n git-fetch.sh | 7 +\n git-ls-remote.sh | 7 +-\n git.c | 1 +\n index-pack.c | 4 +-\n t/t5510-fetch.sh | 28 +++-\n 11 files changed, 575 insertions(+), 4 deletions(-)\n create mode 100644 Documentation/git-bundle.txt\n create mode 100644 builtin-bundle.c\n\ndiff --git a/.gitignore b/.gitignore\nindex f15155d..4d3c66d 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -13,6 +13,7 @@ git-archive\n git-bisect\n git-blame\n git-branch\n+git-bundle\n git-cat-file\n git-check-ref-format\n git-checkout\ndiff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl\nindex a2d6268..f61c77a 100755\n--- a/Documentation/cmd-list.perl\n+++ b/Documentation/cmd-list.perl\n@@ -70,6 +70,7 @@ git-archive mainporcelain\n git-bisect mainporcelain\n git-blame ancillaryinterrogators\n git-branch mainporcelain\n+git-bundle mainporcelain\n git-cat-file plumbinginterrogators\n git-checkout-index plumbingmanipulators\n git-checkout mainporcelain\ndiff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt\nnew file mode 100644\nindex 0000000..4ea9e85\n--- /dev/null\n+++ b/Documentation/git-bundle.txt\n@@ -0,0 +1,139 @@\n+git-bundle(1)\n+=============\n+\n+NAME\n+----\n+git-bundle - Move objects and refs by archive\n+\n+\n+SYNOPSIS\n+--------\n+'git-bundle' create [git-rev-list args]\n+'git-bundle' verify \n+'git-bundle' list-heads [refname...]\n+'git-bundle' unbundle [refname...]\n+\n+DESCRIPTION\n+-----------\n+\n+Some workflows require that one or more branches of development on one\n+machine be replicated on another machine, but the two machines cannot\n+be directly connected so the interactive git protocols (git, ssh,\n+rsync, http) cannot be used. This command provides suport for\n+git-fetch and git-pull to operate by packaging objects and references\n+in an archive at the originating machine, then importing those into\n+another repository using gitlink:git-fetch[1] and gitlink:git-pull[1]\n+after moving the archive by some means (i.e., by sneakernet). As no\n+direct connection between repositories exists, the user must specify a\n+basis for the bundle that is held by the destination repository: the\n+bundle assumes that all objects in the basis are already in the\n+destination repository.\n+\n+OPTIONS\n+-------\n+\n+create ::\n+ Used to create a bundle named 'file'. This requires the\n+ git-rev-list arguments to define the bundle contents.\n+\n+verify ::\n+ Used to check that a bundle file is valid and will apply\n+ cleanly to the current repository. This includes checks on the\n+ bundle format itself as well as checking that the prerequisite\n+ commits exist and are fully linked in the current repository.\n+ git-bundle prints a list of missing commits, if any, and exits\n+ with non-zero status.\n+\n+list-heads ::\n+ Lists the references defined in the bundle. If followed by a\n+ list of references, only references matching those given are\n+ printed out.\n+\n+unbundle ::\n+ Passes the objects in the bundle to gitlink:git-index-pack[1]\n+ for storage in the repository, then prints the names of all\n+ defined references. If a reflist is given, only references\n+ matching those in the given list are printed. This command is\n+ really plumbing, intended to be called only by\n+ gitlink:git-fetch[1].\n+\n+[git-rev-list-args...]::\n+ A list of arguments, accepatble to git-rev-parse and\n+ git-rev-list, that specify the specific objects and references\n+ to transport. For example, \"master~10..master\" causes the\n+ current master reference to be packaged along with all objects\n+ added since its 10th ancestor commit. There is no explicit\n+ limit to the number of references and objects that may be\n+ packaged.\n+\n+\n+[refname...]::\n+ A list of references used to limit the references reported as\n+ available. This is principally of use to git-fetch, which\n+ expects to recieve only those references asked for and not\n+ necessarily everything in the pack (in this case, git-bundle is\n+ acting like gitlink:git-fetch-pack[1]).\n+\n+SPECIFYING REFERENCES\n+---------------------\n+\n+git-bundle will only package references that are shown by\n+git-show-ref: this includes heads, tags, and remote heads. References\n+such as master~1 cannot be packaged, but are perfectly suitable for\n+defining the basis. More than one reference may be packaged, and more\n+than one basis can be specified. The objects packaged are those not\n+contained in the union of the given bases. Each basis can be\n+specified explicitly (e.g., ^master~10), or implicitly (e.g.,\n+master~10..master, master --since=10.days.ago).\n+\n+It is very important that the basis used be held by the destination.\n+It is ok to err on the side of conservatism, causing the bundle file\n+to contain objects already in the destination as these are ignored\n+when unpacking at the destination.\n+\n+EXAMPLE\n+-------\n+\n+Assume two repositories exist as R1 on machine A, and R2 on machine B.\n+For whatever reason, direct connection between A and B is not allowed,\n+but we can move data from A to B via some mechanism (CD, email, etc).\n+We want to update R2 with developments made on branch master in R1.\n+We set a tag in R1 (lastR2bundle) after the previous such transport,\n+and move it afterwards to help build the bundle.\n+\n+in R1 on A:\n+$ git-bundle create mybundle master ^lastR2bundle\n+$ git tag -f lastR2bundle master\n+\n+(move mybundle from A to B by some mechanism)\n+\n+in R2 on B:\n+$ git-bundle verify mybundle\n+$ git-fetch mybundle refspec\n+\n+where refspec is refInBundle:localRef\n+\n+\n+Also, with something like this in your config:\n+\n+[remote \"bundle\"]\n+ url = /home/me/tmp/file.bdl\n+ fetch = refs/heads/*:refs/remotes/origin/*\n+\n+You can first sneakernet the bundle file to ~/tmp/file.bdl and\n+then these commands:\n+\n+$ git ls-remote bundle\n+$ git fetch bundle\n+$ git pull bundle\n+\n+would treat it as if it is talking with a remote side over the\n+network.\n+\n+Author\n+------\n+Written by Mark Levedahl \n+\n+GIT\n+---\n+Part of the gitlink:git[7] suite\ndiff --git a/Makefile b/Makefile\nindex 56bb0b8..03a7f4a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -276,6 +276,7 @@ BUILTIN_OBJS = \\\n \tbuiltin-archive.o \\\n \tbuiltin-blame.o \\\n \tbuiltin-branch.o \\\n+\tbuiltin-bundle.o \\\n \tbuiltin-cat-file.o \\\n \tbuiltin-checkout-index.o \\\n \tbuiltin-check-ref-format.o \\\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nnew file mode 100644\nindex 0000000..4bd596a\n--- /dev/null\n+++ b/builtin-bundle.c\n@@ -0,0 +1,389 @@\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 \"exec_cmd.h\"\n+\n+/*\n+ * Basic handler for bundle files to connect repositories via sneakernet.\n+ * Invocation must include action.\n+ * This function can create a bundle or provide information on an existing\n+ * bundle supporting git-fetch, git-pull, and git-ls-remote\n+ */\n+\n+static const char *bundle_usage=\"git-bundle (--create | --verify | --list-heads [refname]... | --unbundle [refname]... )\";\n+\n+static const char bundle_signature[] = \"# v2 git bundle\\n\";\n+\n+struct ref_list {\n+\tunsigned int nr, alloc;\n+\tstruct {\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, 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\tint count = read(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 offset = buffer[0] == '-' ? 1 : 0;\n+\t\tint len = strlen(buffer);\n+\t\tunsigned char sha1[20];\n+\t\tstruct ref_list *list = offset ? &header->prerequisites\n+\t\t\t: &header->references;\n+\t\tif (get_sha1_hex(buffer + offset, sha1)) {\n+\t\t\tclose(fd);\n+\t\t\treturn error(\"invalid SHA1: %s\", buffer);\n+\t\t}\n+\t\tif (buffer[len - 1] == '\\n')\n+\t\t\tbuffer[len - 1] = '\\0';\n+\t\tadd_to_ref_list(sha1, buffer + 41 + offset, list);\n+\t}\n+\treturn fd;\n+}\n+\n+/* if in && *in >= 0, take that as input file descriptor instead */\n+static int fork_with_pipe(const char **argv, int *in, int *out)\n+{\n+\tint needs_in, needs_out;\n+\tint fdin[2], fdout[2], pid;\n+\n+\tneeds_in = in && *in < 0;\n+\tif (needs_in) {\n+\t\tif (pipe(fdin) < 0)\n+\t\t\treturn error(\"could not setup pipe\");\n+\t\t*in = fdin[1];\n+\t}\n+\n+\tneeds_out = out && *out < 0;\n+\tif (needs_out) {\n+\t\tif (pipe(fdout) < 0)\n+\t\t\treturn error(\"could not setup pipe\");\n+\t\t*out = fdout[0];\n+\t}\n+\n+\tif ((pid = fork()) < 0) {\n+\t\tif (needs_in) {\n+\t\t\tclose(fdin[0]);\n+\t\t\tclose(fdin[1]);\n+\t\t}\n+\t\tif (needs_out) {\n+\t\t\tclose(fdout[0]);\n+\t\t\tclose(fdout[1]);\n+\t\t}\n+\t\treturn error(\"could not fork\");\n+\t}\n+\tif (!pid) {\n+\t\tif (needs_in) {\n+\t\t\tdup2(fdin[0], 0);\n+\t\t\tclose(fdin[0]);\n+\t\t\tclose(fdin[1]);\n+\t\t} else if (in)\n+\t\t\tdup2(*in, 0);\n+\t\tif (needs_out) {\n+\t\t\tdup2(fdout[1], 1);\n+\t\t\tclose(fdout[0]);\n+\t\t\tclose(fdout[1]);\n+\t\t} else if (out)\n+\t\t\tdup2(*out, 1);\n+\t\texit(execv_git_cmd(argv));\n+\t}\n+\tif (needs_in)\n+\t\tclose(fdin[0]);\n+\tif (needs_out)\n+\t\tclose(fdout[1]);\n+\treturn pid;\n+}\n+\n+static int verify_bundle(struct bundle_header *header)\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+\tconst char *argv[5] = {\"rev-list\", \"--stdin\", \"--not\", \"--all\", NULL};\n+\tint pid, in, out, i, ret = 0;\n+\tchar buffer[1024];\n+\n+\tin = out = -1;\n+\tpid = fork_with_pipe(argv, &in, &out);\n+\tif (pid < 0)\n+\t\treturn error(\"Could not fork rev-list\");\n+\tif (!fork()) {\n+\t\tfor (i = 0; i < p->nr; i++) {\n+\t\t\twrite(in, sha1_to_hex(p->list[i].sha1), 40);\n+\t\t\twrite(in, \"\\n\", 1);\n+\t\t}\n+\t\tclose(in);\n+\t\texit(0);\n+\t}\n+\tclose(in);\n+\twhile (read_string(out, buffer, sizeof(buffer)) > 0) {\n+\t\tif (ret++ == 0)\n+\t\t\terror (\"The bundle requires the following commits you lack:\");\n+\t\tfprintf(stderr, \"%s\", buffer);\n+\t}\n+\tclose(out);\n+\twhile (waitpid(pid, &i, 0) < 0)\n+\t\tif (errno != EINTR)\n+\t\t\treturn -1;\n+\tif (!ret && (!WIFEXITED(i) || WEXITSTATUS(i)))\n+\t\treturn error(\"At least one prerequisite is lacking.\");\n+\n+\treturn ret;\n+}\n+\n+static int list_heads(struct bundle_header *header, int argc, const char **argv)\n+{\n+\tint i;\n+\tstruct ref_list *r = &header->references;\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+static void show_commit(struct commit *commit)\n+{\n+\twrite(1, sha1_to_hex(commit->object.sha1), 40);\n+\twrite(1, \"\\n\", 1);\n+\tif (commit->parents) {\n+\t\tfree_commit_list(commit->parents);\n+\t\tcommit->parents = NULL;\n+\t}\n+}\n+\n+static void show_object(struct object_array_entry *p)\n+{\n+\t/* An object with name \"foo\\n0000000...\" can be used to\n+\t * * confuse downstream git-pack-objects very badly.\n+\t * */\n+\tconst char *ep = strchr(p->name, '\\n');\n+\tint len = ep ? ep - p->name : strlen(p->name);\n+\twrite(1, sha1_to_hex(p->item->sha1), 40);\n+\twrite(1, \" \", 1);\n+\tif (len)\n+\t\twrite(1, p->name, len);\n+\twrite(1, \"\\n\", 1);\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 + 3) * sizeof(const char *));\n+\tconst char **argv_pack = xmalloc(4 * sizeof(const char *));\n+\tint pid, in, out, i, status;\n+\tchar buffer[1024];\n+\tstruct rev_info revs;\n+\n+\tbundle_fd = (!strcmp(path, \"-\") ? 1 :\n+\t\t\topen(path, O_CREAT | O_WRONLY, 0666));\n+\tif (bundle_fd < 0)\n+\t\treturn error(\"Could not write to '%s'\", path);\n+\n+\t/* write signature */\n+\twrite(bundle_fd, bundle_signature, strlen(bundle_signature));\n+\n+\t/* write prerequisites */\n+\tmemcpy(argv_boundary + 2, argv + 1, argc * sizeof(const char *));\n+\targv_boundary[0] = \"rev-list\";\n+\targv_boundary[1] = \"--boundary\";\n+\targv_boundary[argc + 1] = NULL;\n+\tout = -1;\n+\tpid = fork_with_pipe(argv_boundary, NULL, &out);\n+\tif (pid < 0)\n+\t\treturn -1;\n+\twhile ((i = read_string(out, buffer, sizeof(buffer))) > 0)\n+\t\tif (buffer[0] == '-')\n+\t\t\twrite(bundle_fd, buffer, i);\n+\twhile ((i = waitpid(pid, &status, 0)) < 0)\n+\t\tif (errno != EINTR)\n+\t\t\treturn error(\"rev-list died\");\n+\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n+\t\treturn error(\"rev-list died %d\", WEXITSTATUS(status));\n+\n+\t/* write references */\n+\tsave_commit_buffer = 0;\n+\tinit_revisions(&revs, NULL);\n+\trevs.tag_objects = 1;\n+\trevs.tree_objects = 1;\n+\trevs.blob_objects = 1;\n+\targc = setup_revisions(argc, argv, &revs, NULL);\n+\tif (argc > 1)\n+\t\treturn error(\"unrecognized argument: %s'\", argv[1]);\n+\tfor (i = 0; i < revs.pending.nr; i++) {\n+\t\tstruct object_array_entry *e = revs.pending.objects + i;\n+\t\tif (!(e->item->flags & UNINTERESTING)) {\n+\t\t\tunsigned char sha1[20];\n+\t\t\tchar *ref;\n+\t\t\tif (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)\n+\t\t\t\tcontinue;\n+\t\t\twrite(bundle_fd, sha1_to_hex(e->item->sha1), 40);\n+\t\t\twrite(bundle_fd, \" \", 1);\n+\t\t\twrite(bundle_fd, ref, strlen(ref));\n+\t\t\twrite(bundle_fd, \"\\n\", 1);\n+\t\t\tfree(ref);\n+\t\t}\n+\t}\n+\n+\t/* end header */\n+\twrite(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] = NULL;\n+\tin = -1;\n+\tout = bundle_fd;\n+\tpid = fork_with_pipe(argv_pack, &in, &out);\n+\tif (pid < 0)\n+\t\treturn error(\"Could not spawn pack-objects\");\n+\tclose(1);\n+\tclose(bundle_fd);\n+\tdup2(in, 1);\n+\tclose(in);\n+\tprepare_revision_walk(&revs);\n+\ttraverse_commit_list(&revs, show_commit, show_object);\n+\tclose(1);\n+\twhile (waitpid(pid, &status, 0) < 0)\n+\t\tif (errno != EINTR)\n+\t\t\treturn -1;\n+\tif (!WIFEXITED(status) || WEXITSTATUS(status))\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\", \"--stdin\", NULL};\n+\tint pid, status, dev_null;\n+\n+\tif (verify_bundle(header))\n+\t\treturn -1;\n+\tdev_null = open(\"/dev/null\", O_WRONLY);\n+\tpid = fork_with_pipe(argv_index_pack, &bundle_fd, &dev_null);\n+\tif (pid < 0)\n+\t\treturn error(\"Could not spawn index-pack\");\n+\tclose(bundle_fd);\n+\twhile (waitpid(pid, &status, 0) < 0)\n+\t\tif (errno != EINTR)\n+\t\t\treturn error(\"index-pack died\");\n+\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n+\t\treturn error(\"index-pack exited with status %d\",\n+\t\t\t\tWEXITSTATUS(status));\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+\tint nongit = 0;\n+\tconst char *cmd, *bundle_file;\n+\tint bundle_fd = -1;\n+\tchar buffer[PATH_MAX];\n+\n+\tif (argc < 3)\n+\t\tusage(bundle_usage);\n+\n+\tcmd = argv[1];\n+\tbundle_file = argv[2];\n+\targc -= 2;\n+\targv += 2;\n+\n+\tprefix = setup_git_directory_gently(&nongit);\n+\tif (prefix && bundle_file[0] != '/') {\n+\t\tsnprintf(buffer, sizeof(buffer), \"%s/%s\", prefix, bundle_file);\n+\t\tbundle_file = buffer;\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)))\n+\t\treturn 1;\n+\n+\tif (!strcmp(cmd, \"verify\")) {\n+\t\tclose(bundle_fd);\n+\t\tif (verify_bundle(&header))\n+\t\t\treturn 1;\n+\t\tfprintf(stderr, \"%s is okay\\n\", bundle_file);\n+\t\treturn 0;\n+\t}\n+\tif (!strcmp(cmd, \"list-heads\")) {\n+\t\tclose(bundle_fd);\n+\t\treturn !!list_heads(&header, argc, argv);\n+\t}\n+\tif (!strcmp(cmd, \"create\")) {\n+\t\tif (nongit)\n+\t\t\tdie(\"Need a repository to create a bundle.\");\n+\t\treturn !!create_bundle(&header, bundle_file, argc, argv);\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} else\n+\t\tusage(bundle_usage);\n+}\n+\ndiff --git a/builtin.h b/builtin.h\nindex 57e8741..528074b 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -19,6 +19,7 @@ extern int cmd_apply(int argc, const char **argv, const char *prefix);\n extern int cmd_archive(int argc, const char **argv, const char *prefix);\n extern int cmd_blame(int argc, const char **argv, const char *prefix);\n extern int cmd_branch(int argc, const char **argv, const char *prefix);\n+extern int cmd_bundle(int argc, const char **argv, const char *prefix);\n extern int cmd_cat_file(int argc, const char **argv, const char *prefix);\n extern int cmd_checkout_index(int argc, const char **argv, const char *prefix);\n extern int cmd_check_ref_format(int argc, const char **argv, const char *prefix);\ndiff --git a/git-fetch.sh b/git-fetch.sh\nindex 5d3fec0..5ae0d28 100755\n--- a/git-fetch.sh\n+++ b/git-fetch.sh\n@@ -388,9 +388,16 @@ fetch_main () {\n ( : subshell because we muck with IFS\n IFS=\" \t$LF\"\n (\n+\tif test -f \"$remote\" ; then\n+\t test -n \"$shallow_depth\" &&\n+\t\tdie \"shallow clone with bundle is not supported\"\n+\t git-bundle unbundle \"$remote\" $rref ||\n+\t echo failed \"$remote\"\n+\telse\n \t git-fetch-pack --thin $exec $keep $shallow_depth $no_progress \\\n \t\t\"$remote\" $rref ||\n \t echo failed \"$remote\"\n+\tfi\n ) |\n (\n \ttrap '\ndiff --git a/git-ls-remote.sh b/git-ls-remote.sh\nindex 8ea5c5e..a6ed99a 100755\n--- a/git-ls-remote.sh\n+++ b/git-ls-remote.sh\n@@ -89,8 +89,13 @@ rsync://* )\n \t;;\n \n * )\n-\tgit-peek-remote $exec \"$peek_repo\" ||\n+\tif test -f \"$peek_repo\" ; then\n+\t\tgit bundle list-heads \"$peek_repo\" ||\n \t\techo \"failed\tslurping\"\n+\telse\n+\t\tgit-peek-remote $exec \"$peek_repo\" ||\n+\t\techo \"failed\tslurping\"\n+\tfi\n \t;;\n esac |\n sort -t '\t' -k 2 |\ndiff --git a/git.c b/git.c\nindex 83f3d90..a184d47 100644\n--- a/git.c\n+++ b/git.c\n@@ -229,6 +229,7 @@ static void handle_internal_command(int argc, const char **argv, char **envp)\n \t\t{ \"archive\", cmd_archive },\n \t\t{ \"blame\", cmd_blame, RUN_SETUP },\n \t\t{ \"branch\", cmd_branch, RUN_SETUP },\n+\t\t{ \"bundle\", cmd_bundle },\n \t\t{ \"cat-file\", cmd_cat_file, RUN_SETUP },\n \t\t{ \"checkout-index\", cmd_checkout_index, RUN_SETUP },\n \t\t{ \"check-ref-format\", cmd_check_ref_format },\ndiff --git a/index-pack.c b/index-pack.c\nindex fa9a0e7..5ccf4c4 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n \t/* If input_fd is a file, we should have reached its end now. */\n \tif (fstat(input_fd, &st))\n \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n-\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n-\t\tdie(\"pack has junk at the end\");\n+\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n+\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode, (int)st.st_size, (int)consumed_bytes, input_fd);\n \n \tif (!nr_deltas)\n \t\treturn;\ndiff --git a/t/t5510-fetch.sh b/t/t5510-fetch.sh\nindex 50c6485..fa76662 100755\n--- a/t/t5510-fetch.sh\n+++ b/t/t5510-fetch.sh\n@@ -35,7 +35,9 @@ test_expect_success \"clone and setup child repos\" '\n \t\techo \"URL: ../two/.git/\"\n \t\techo \"Pull: refs/heads/master:refs/heads/two\"\n \t\techo \"Pull: refs/heads/one:refs/heads/one\"\n-\t} >.git/remotes/two\n+\t} >.git/remotes/two &&\n+\tcd .. &&\n+\tgit clone . bundle\n '\n \n test_expect_success \"fetch test\" '\n@@ -81,4 +83,28 @@ test_expect_success 'fetch following tags' '\n \n '\n \n+test_expect_success 'create bundle 1' '\n+\tcd \"$D\" &&\n+\techo >file updated again by origin &&\n+\tgit commit -a -m \"tip\" &&\n+\tgit bundle create bundle1 master^..master\n+'\n+\n+test_expect_success 'create bundle 2' '\n+\tcd \"$D\" &&\n+\tgit bundle create bundle2 master~2..master\n+'\n+\n+test_expect_failure 'unbundle 1' '\n+\tcd \"$D/bundle\" &&\n+\tgit checkout -b some-branch &&\n+\tgit fetch \"$D/bundle1\" master:master\n+'\n+\n+test_expect_success 'unbundle 2' '\n+\tcd \"$D/bundle\" &&\n+\tgit fetch ../bundle2 master:master &&\n+\ttest \"tip\" = \"$(git log -1 --pretty=oneline master | cut -b42-)\"\n+'\n+\n test_done\n-- \n1.5.0.1.616.gc6c4-dirty\n"},{"id":"35213","messageId":"alpine.LRH.0.82.0702212224510.31945@xanadu.home","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702220157130.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-02-22T03:28:05Z","receivedAt":"2007-02-22T03:28:05Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n\n> diff --git a/index-pack.c b/index-pack.c\n> index fa9a0e7..5ccf4c4 100644\n> --- a/index-pack.c\n> +++ b/index-pack.c\n> @@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n> \t/* If input_fd is a file, we should have reached its end now. */\n> \tif (fstat(input_fd, &st))\n> \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n> -\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> -\t\tdie(\"pack has junk at the end\");\n> +\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> +\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode, (int)st.st_size, (int)consumed_bytes, input_fd);\n> \n> \tif (!nr_deltas)\n> \t\treturn;\n\nWhat is this supposed to mean?\n\n\nNicolas\n"},{"id":"35216","messageId":"7v8xeqllxk.fsf@assigned-by-dhcp.cox.net","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702220157130.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-22T06:56:55Z","receivedAt":"2007-02-22T06:56:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> It was updated to make git-bundle a builtin, and get rid of the tar \n> format: now, the first line is supposed to say \"# v2 git bundle\", the next \n> lines either contain a prerequisite (\"-\" followed by the hash of the \n> needed commit), or a ref (the hash of a commit, followed by the name of \n> the ref), and finally the pack. As a result, the bundle argument can be \n> \"-\" now.\n\nThat in BNF would be?\n\n\tbundle = signature prereq* ref* pack\n\tsignature = \"# v2 git bundle\\n\"\n prereq = \"-\" sha1 \" \" \"\\n\"\n ref = sha1 \" \" name \"\\n\"\n\tsha1 = [0-9a-f]{40}\n\tname = \n\tpack = \n\nI suspect that we might want to possibly:\n\n - have checksum protection over the part before pack data?\n\n - give descriptive name to prereq, so that an error message can\n say \"you need v1.4.4\" instead of \"you need e267c2f6\"?\n\n - make the fields extensible without updating \"v2\"?\n\n> diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl\n> index a2d6268..f61c77a 100755\n> --- a/Documentation/cmd-list.perl\n> +++ b/Documentation/cmd-list.perl\n> @@ -70,6 +70,7 @@ git-archive mainporcelain\n> git-bisect mainporcelain\n> git-blame ancillaryinterrogators\n> git-branch mainporcelain\n> +git-bundle mainporcelain\n> git-cat-file plumbinginterrogators\n> git-checkout-index plumbingmanipulators\n> git-checkout mainporcelain\n\nIs this really a mainporcelain?\nI would say ancillarymanipulators (or perhaps synchelpers).\n\n> diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt\n> new file mode 100644\n> index 0000000..4ea9e85\n> ...\n> +Author\n> +------\n> +Written by Mark Levedahl \n\n... and Dscho.\n\n> diff --git a/builtin-bundle.c b/builtin-bundle.c\n> new file mode 100644\n> index 0000000..4bd596a\n> --- /dev/null\n> +++ b/builtin-bundle.c\n> ...\n> +static const char *bundle_usage=\"git-bundle (--create | --verify | --list-heads [refname]... | --unbundle [refname]... )\";\n\nI thought you removed \"--\" prefixes from subcommands.\n\n> +struct bundle_header {\n> +\tstruct ref_list prerequisites, references;\n> +};\n\n(minor style) I find these two members each on its own line\neasier to read, as in:\n\n struct bundle_header {\n struct ref_list prerequisites;\n struct 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\tint count = read(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\nAt least xread() please. I know the reason you read one byte at\na time is because you cannot over-read into pack area, but\nsomehow this makes me go \"hmmmmmm\". It's not performance\ncritical so let's leave it that way. Is bundle supposed to be\nstreamable? Otherwise we could probably seek back.\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 offset = buffer[0] == '-' ? 1 : 0;\n> +\t\tint len = strlen(buffer);\n> +\t\tunsigned char sha1[20];\n> +\t\tstruct ref_list *list = offset ? &header->prerequisites\n> +\t\t\t: &header->references;\n> +\t\tif (get_sha1_hex(buffer + offset, sha1)) {\n> +\t\t\tclose(fd);\n> +\t\t\treturn error(\"invalid SHA1: %s\", buffer);\n> +\t\t}\n> +\t\tif (buffer[len - 1] == '\\n')\n> +\t\t\tbuffer[len - 1] = '\\0';\n> +\t\tadd_to_ref_list(sha1, buffer + 41 + offset, list);\n> +\t}\n> +\treturn fd;\n> +}\n\nDon't you want to look at and validate buffer[40+offset] in any\nway, other than what get_sha1_hex() does (which is issspace())?\n\n> +/* if in && *in >= 0, take that as input file descriptor instead */\n> +static int fork_with_pipe(const char **argv, int *in, int *out)\n> +{\n> +\tint needs_in, needs_out;\n> +\tint fdin[2], fdout[2], pid;\n> +\n> +\tneeds_in = in && *in < 0;\n> +\tif (needs_in) {\n> +\t\tif (pipe(fdin) < 0)\n> +\t\t\treturn error(\"could not setup pipe\");\n> +\t\t*in = fdin[1];\n> +\t}\n> +\n> +\tneeds_out = out && *out < 0;\n> +\tif (needs_out) {\n> +\t\tif (pipe(fdout) < 0)\n> +\t\t\treturn error(\"could not setup pipe\");\n> +\t\t*out = fdout[0];\n> +\t}\n> +\n> +\tif ((pid = fork()) < 0) {\n> +\t\tif (needs_in) {\n> +\t\t\tclose(fdin[0]);\n> +\t\t\tclose(fdin[1]);\n> +\t\t}\n> +\t\tif (needs_out) {\n> +\t\t\tclose(fdout[0]);\n> +\t\t\tclose(fdout[1]);\n> +\t\t}\n> +\t\treturn error(\"could not fork\");\n> +\t}\n> +\tif (!pid) {\n> +\t\tif (needs_in) {\n> +\t\t\tdup2(fdin[0], 0);\n> +\t\t\tclose(fdin[0]);\n> +\t\t\tclose(fdin[1]);\n> +\t\t} else if (in)\n> +\t\t\tdup2(*in, 0);\n> +\t\tif (needs_out) {\n> +\t\t\tdup2(fdout[1], 1);\n> +\t\t\tclose(fdout[0]);\n> +\t\t\tclose(fdout[1]);\n> +\t\t} else if (out)\n> +\t\t\tdup2(*out, 1);\n> +\t\texit(execv_git_cmd(argv));\n> +\t}\n\nDo you need to close *in and *out that are given by the caller\nafter dup2() in the child?\n\n> +static int verify_bundle(struct bundle_header *header)\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> +\tconst char *argv[5] = {\"rev-list\", \"--stdin\", \"--not\", \"--all\", NULL};\n> +\tint pid, in, out, i, ret = 0;\n> +\tchar buffer[1024];\n> +\n> +\tin = out = -1;\n> +\tpid = fork_with_pipe(argv, &in, &out);\n> +\tif (pid < 0)\n> +\t\treturn error(\"Could not fork rev-list\");\n> +\tif (!fork()) {\n> +\t\tfor (i = 0; i < p->nr; i++) {\n> +\t\t\twrite(in, sha1_to_hex(p->list[i].sha1), 40);\n> +\t\t\twrite(in, \"\\n\", 1);\n> +\t\t}\n> +\t\tclose(in);\n> +\t\texit(0);\n> +\t}\n> +\tclose(in);\n\nWhat if write() fails? That can happen when one of the objects\nyou feed here, or its parent objects, is missing from your\nrepository -- receiving rev-list would die() and the writing\nchild would sigpipe.\n\nI also wonder who waits for this child...\n\n> +\twhile (read_string(out, buffer, sizeof(buffer)) > 0) {\n> +\t\tif (ret++ == 0)\n> +\t\t\terror (\"The bundle requires the following commits you lack:\");\n> +\t\tfprintf(stderr, \"%s\", buffer);\n> +\t}\n> +\tclose(out);\n\nI do not think this error message would buy you anything. If\nyou say:\n\n\techo commit | rev-list --stdin --not --all\n\nand you got commit back, that means you _do_ have that commit,\nand that commit reaches some ref you have (objects associated to\nthat commit may be still missing). If commit is truly missing,\nthen you do not get commit output from rev-list as it cannot\neven determine if it is \"--not --all\" or not -- it would error\nout, which is the more important check to see if prereqs are\nlacking.\n\n> +\twhile (waitpid(pid, &i, 0) < 0)\n> +\t\tif (errno != EINTR)\n> +\t\t\treturn -1;\n> +\tif (!ret && (!WIFEXITED(i) || WEXITSTATUS(i)))\n> +\t\treturn error(\"At least one prerequisite is lacking.\");\n\nSo you would want to keep this error, perhaps even make it more\nhelpful by suggesting which ones are missing.\n\n> +static int list_heads(struct bundle_header *header, int argc, const char **argv)\n> +{\n> +\tint i;\n> +\tstruct ref_list *r = &header->references;\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\nI wonder if we want to have a way to list prereqs in similar way.\n\n> +static void show_commit(struct commit *commit)\n> +{\n> +\twrite(1, sha1_to_hex(commit->object.sha1), 40);\n> +\twrite(1, \"\\n\", 1);\n> +\tif (commit->parents) {\n> +\t\tfree_commit_list(commit->parents);\n> +\t\tcommit->parents = NULL;\n> +\t}\n> +}\n\n(everywhere) We would want write_in_full() with error handling\nthat dies even on EPIPE.\n\n> +\n> +static void show_object(struct object_array_entry *p)\n> +{\n> +\t/* An object with name \"foo\\n0000000...\" can be used to\n> +\t * * confuse downstream git-pack-objects very badly.\n> +\t * */\n\nAn interesting way to wrap comments.\n\n> +\t/* write prerequisites */\n> +\tmemcpy(argv_boundary + 2, argv + 1, argc * sizeof(const char *));\n> +\targv_boundary[0] = \"rev-list\";\n> +\targv_boundary[1] = \"--boundary\";\n> +\targv_boundary[argc + 1] = NULL;\n> +\tout = -1;\n> +\tpid = fork_with_pipe(argv_boundary, NULL, &out);\n> +\tif (pid < 0)\n> +\t\treturn -1;\n> +\twhile ((i = read_string(out, buffer, sizeof(buffer))) > 0)\n> +\t\tif (buffer[0] == '-')\n> +\t\t\twrite(bundle_fd, buffer, i);\n\nIt would be helpful for the recipient if you can append output\nfrom git-describe (or name-rev) when the buffer lacks \"name\".\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\", \"--stdin\", NULL};\n> +\tint pid, status, dev_null;\n> +\n> +\tif (verify_bundle(header))\n> +\t\treturn -1;\n> +\tdev_null = open(\"/dev/null\", O_WRONLY);\n> +\tpid = fork_with_pipe(argv_index_pack, &bundle_fd, &dev_null);\n> +\tif (pid < 0)\n> +\t\treturn error(\"Could not spawn index-pack\");\n> +\tclose(bundle_fd);\n> +\twhile (waitpid(pid, &status, 0) < 0)\n> +\t\tif (errno != EINTR)\n> +\t\t\treturn error(\"index-pack died\");\n> +\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n> +\t\treturn error(\"index-pack exited with status %d\",\n> +\t\t\t\tWEXITSTATUS(status));\n> +\treturn list_heads(header, argc, argv);\n> +}\n\nWe might want to later use --keep for this as well...\n\n> diff --git a/index-pack.c b/index-pack.c\n> index fa9a0e7..5ccf4c4 100644\n> --- a/index-pack.c\n> +++ b/index-pack.c\n> @@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n> \t/* If input_fd is a file, we should have reached its end now. */\n> \tif (fstat(input_fd, &st))\n> \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n> -\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> -\t\tdie(\"pack has junk at the end\");\n> +\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> +\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode, (int)st.st_size, (int)consumed_bytes, input_fd);\n> \n> \tif (!nr_deltas)\n> \t\treturn;\n\n??\n"},{"id":"35217","messageId":"7v4ppellev.fsf@assigned-by-dhcp.cox.net","threadId":"6910","inReplyTo":"7v8xeqllxk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-22T07:08:08Z","receivedAt":"2007-02-22T07:08:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> +static int verify_bundle(struct bundle_header *header)\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>> +\tconst char *argv[5] = {\"rev-list\", \"--stdin\", \"--not\", \"--all\", NULL};\n>> +\tint pid, in, out, i, ret = 0;\n>> +\tchar buffer[1024];\n>> +\n>> +\tin = out = -1;\n>> +\tpid = fork_with_pipe(argv, &in, &out);\n>> +\tif (pid < 0)\n>> +\t\treturn error(\"Could not fork rev-list\");\n>> +\tif (!fork()) {\n>> +\t\tfor (i = 0; i < p->nr; i++) {\n>> +\t\t\twrite(in, sha1_to_hex(p->list[i].sha1), 40);\n>> +\t\t\twrite(in, \"\\n\", 1);\n>> +\t\t}\n>> +\t\tclose(in);\n>> +\t\texit(0);\n>> +\t}\n>> +\tclose(in);\n>\n> What if write() fails? That can happen when one of the objects\n> you feed here, or its parent objects, is missing from your\n> repository -- receiving rev-list would die() and the writing\n> child would sigpipe.\n>\n> I also wonder who waits for this child...\n\nIn general, fork() to avoid bidirectional pipe deadlock is a\ngood discipline, but in this particular case I think it would be\neasier to handle errors if you don't (and it would save another\nprocess). The other side \"rev-list --stdin --not --all\" is\nrunning a limited traversal, and would not emit anything until\nyou stop feeding it from --stdin, or until it dies because you\nfed it a commit that does not exist. So as long as you check\nthe error condition from write() for EPIPE to notice the other\nend died, I think you are Ok.\n"},{"id":"35222","messageId":"45DD62DF.9324130F@eudaptics.com","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702220157130.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-02-22T09:31:11Z","receivedAt":"2007-02-22T09:31:11Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin wrote:\n> +/* if in && *in >= 0, take that as input file descriptor instead */\n> +static int fork_with_pipe(const char **argv, int *in, int *out)\n> +{\n> + int needs_in, needs_out;\n> + int fdin[2], fdout[2], pid;\n> +\n> + needs_in = in && *in < 0;\n> + if (needs_in) {\n> + if (pipe(fdin) < 0)\n> + return error(\"could not setup pipe\");\n> + *in = fdin[1];\n> + }\n> +\n> + needs_out = out && *out < 0;\n> + if (needs_out) {\n> + if (pipe(fdout) < 0)\n> + return error(\"could not setup pipe\");\n> + *out = fdout[0];\n> + }\n> +\n> + if ((pid = fork()) < 0) {\n> + if (needs_in) {\n> + close(fdin[0]);\n> + close(fdin[1]);\n> + }\n> + if (needs_out) {\n> + close(fdout[0]);\n> + close(fdout[1]);\n> + }\n> + return error(\"could not fork\");\n> + }\n> + if (!pid) {\n> + if (needs_in) {\n> + dup2(fdin[0], 0);\n> + close(fdin[0]);\n> + close(fdin[1]);\n> + } else if (in)\n> + dup2(*in, 0);\n> + if (needs_out) {\n> + dup2(fdout[1], 1);\n> + close(fdout[0]);\n> + close(fdout[1]);\n> + } else if (out)\n> + dup2(*out, 1);\n> + exit(execv_git_cmd(argv));\n> + }\n> + if (needs_in)\n> + close(fdin[0]);\n> + if (needs_out)\n> + close(fdout[1]);\n> + return pid;\n> +}\n\nThis function looks very similar to spawnvppe, which I use in the MinGW\nport for a number of fork/exec pairs. Do you see a chance to make this\ninto a helper that can be used in more places (so that it reduces the\ndifferences to the MinGW code)?\n\n> + in = out = -1;\n> + pid = fork_with_pipe(argv, &in, &out);\n> + if (pid < 0)\n> + return error(\"Could not fork rev-list\");\n> + if (!fork()) {\n> + for (i = 0; i < p->nr; i++) {\n> + write(in, sha1_to_hex(p->list[i].sha1), 40);\n> + write(in, \"\\n\", 1);\n> + }\n> + close(in);\n> + exit(0);\n> + }\n> + close(in);\n\nOops, fork error check missing?\n\n-- Hannes\n"},{"id":"35230","messageId":"Pine.LNX.4.63.0702221654131.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"alpine.LRH.0.82.0702212224510.31945@xanadu.home","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-22T15:55:29Z","receivedAt":"2007-02-22T15:55:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 21 Feb 2007, Nicolas Pitre wrote:\n\n> On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n> \n> > diff --git a/index-pack.c b/index-pack.c\n> > index fa9a0e7..5ccf4c4 100644\n> > --- a/index-pack.c\n> > +++ b/index-pack.c\n> > @@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n> > \t/* If input_fd is a file, we should have reached its end now. */\n> > \tif (fstat(input_fd, &st))\n> > \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n> > -\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > -\t\tdie(\"pack has junk at the end\");\n> > +\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > +\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode, (int)st.st_size, (int)consumed_bytes, input_fd);\n> > \n> > \tif (!nr_deltas)\n> > \t\treturn;\n> \n> What is this supposed to mean?\n\nThe funny thing is, if you stream part of the bundle file to index-pack, \nS_ISREG(st.st_mode) is true, even if input_fd == 0.\n\nThen, because it is only part of the bundle file, the size check fails.\n\nCiao,\nDscho\n"},{"id":"35233","messageId":"45DDC158.9090902@fs.ei.tum.de","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702221654131.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Simon 'corecode' Schubert","fromEmail":"corecode@fs.ei.tum.de","sentAt":"2007-02-22T16:14:16Z","receivedAt":"2007-02-22T16:14:16Z","isPatch":true,"sender":{"key":"corecode@fs.ei.tum.de","avatar":"https://gravatar.com/avatar/eff9dbf0cdac0d1e6a6cd7ed0e50763edcb376b493b5253a35ff167918ad79e1?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Hi,\n> \n> On Wed, 21 Feb 2007, Nicolas Pitre wrote:\n> \n>> On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n>>\n>>> diff --git a/index-pack.c b/index-pack.c\n>>> index fa9a0e7..5ccf4c4 100644\n>>> --- a/index-pack.c\n>>> +++ b/index-pack.c\n>>> @@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n>>> \t/* If input_fd is a file, we should have reached its end now. */\n>>> \tif (fstat(input_fd, &st))\n>>> \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n>>> -\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n>>> -\t\tdie(\"pack has junk at the end\");\n>>> +\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n>>> +\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode, (int)st.st_size, (int)consumed_bytes, input_fd);\n>>> \n>>> \tif (!nr_deltas)\n>>> \t\treturn;\n>> What is this supposed to mean?\n> \n> The funny thing is, if you stream part of the bundle file to index-pack, \n> S_ISREG(st.st_mode) is true, even if input_fd == 0.\n\nWell, of course: you opened a regular file and pass this as stdin to index-pack.\n\nMaybe something like this would be cleaner:\n\nif (IS_REF(st.st_mode) && lseek(input_fd, 0, SEEK_CUR) != st.st_size)\n\tdie(\"...\");\n\nBecause the point isn't actually that we consumed less bytes than the file's size, but that there is still data available after we are done with handling the pack.\n\nAnyways, is this a reason to die(), or rather just a remark?\n\ncheers\n simon\n\n-- \nServe - BSD +++ RENT this banner advert +++ ASCII Ribbon /\"\\\nWork - Mac +++ space for low €€€ NOW!1 +++ Campaign \\ /\nParty Enjoy Relax | http://dragonflybsd.org Against HTML \\\nDude 2c 2 the max ! http://golden-apple.biz Mail + News / \\\n\n"},{"id":"35234","messageId":"Pine.LNX.4.63.0702221656490.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"7v8xeqllxk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-22T16:17:58Z","receivedAt":"2007-02-22T16:17:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 21 Feb 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > It was updated to make git-bundle a builtin, and get rid of the tar \n> > format: now, the first line is supposed to say \"# v2 git bundle\", the next \n> > lines either contain a prerequisite (\"-\" followed by the hash of the \n> > needed commit), or a ref (the hash of a commit, followed by the name of \n> > the ref), and finally the pack. As a result, the bundle argument can be \n> > \"-\" now.\n> \n> That in BNF would be?\n> \n> \tbundle = signature prereq* ref* pack\n\nmore like\n\n\tbundle = header pack\n\theader = signature prereq* ref* nl\n\n> I suspect that we might want to possibly:\n> \n> - have checksum protection over the part before pack data?\n\nPossibly. But would the sha1's not be sufficient? I.e. if the header is \ncorrupted, chances are that the commits are no longer verified correctly.\n\n> - give descriptive name to prereq, so that an error message can\n> say \"you need v1.4.4\" instead of \"you need e267c2f6\"?\n\nMy idea was to allow --since=. You don't have a name there.\n\n> - make the fields extensible without updating \"v2\"?\n\nYou mean, like, warn about unknown header lines? Yes, that's really easy.\n\n> > diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl\n> > index a2d6268..f61c77a 100755\n> > --- a/Documentation/cmd-list.perl\n> > +++ b/Documentation/cmd-list.perl\n> > @@ -70,6 +70,7 @@ git-archive mainporcelain\n> > git-bisect mainporcelain\n> > git-blame ancillaryinterrogators\n> > git-branch mainporcelain\n> > +git-bundle mainporcelain\n> > git-cat-file plumbinginterrogators\n> > git-checkout-index plumbingmanipulators\n> > git-checkout mainporcelain\n> \n> Is this really a mainporcelain?\n\ngit bundle create ?\n\n> > diff --git a/Documentation/git-bundle.txt b/Documentation/git-bundle.txt\n> > new file mode 100644\n> > index 0000000..4ea9e85\n> > ...\n> > +Author\n> > +------\n> > +Written by Mark Levedahl \n> \n> ... and Dscho.\n\n... not the Documentation (only patched a little to reflect the \"--\" \nchange).\n\n> > diff --git a/builtin-bundle.c b/builtin-bundle.c\n> > new file mode 100644\n> > index 0000000..4bd596a\n> > --- /dev/null\n> > +++ b/builtin-bundle.c\n> > ...\n> > +static const char *bundle_usage=\"git-bundle (--create | --verify | --list-heads [refname]... | --unbundle [refname]... )\";\n> \n> I thought you removed \"--\" prefixes from subcommands.\n\nYes, and I forgot it there. Will fix.\n\n> > +struct bundle_header {\n> > +\tstruct ref_list prerequisites, references;\n> > +};\n> \n> (minor style) I find these two members each on its own line\n> easier to read, as in:\n> \n> struct bundle_header {\n> struct ref_list prerequisites;\n> struct ref_list references;\n> };\n\nWill fix.\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\tint count = read(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> At least xread() please. I know the reason you read one byte at\n> a time is because you cannot over-read into pack area, but\n> somehow this makes me go \"hmmmmmm\". It's not performance\n> critical so let's leave it that way. Is bundle supposed to be\n> streamable? Otherwise we could probably seek back.\n\nI wanted unbundle to be able to read from stdin (I did not yet allow \"git \nfetch - master:master\"). But I somehow forgot about xread(). D'oh. Will \nfix.\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 offset = buffer[0] == '-' ? 1 : 0;\n> > +\t\tint len = strlen(buffer);\n> > +\t\tunsigned char sha1[20];\n> > +\t\tstruct ref_list *list = offset ? &header->prerequisites\n> > +\t\t\t: &header->references;\n> > +\t\tif (get_sha1_hex(buffer + offset, sha1)) {\n> > +\t\t\tclose(fd);\n> > +\t\t\treturn error(\"invalid SHA1: %s\", buffer);\n> > +\t\t}\n> > +\t\tif (buffer[len - 1] == '\\n')\n> > +\t\t\tbuffer[len - 1] = '\\0';\n> > +\t\tadd_to_ref_list(sha1, buffer + 41 + offset, list);\n> > +\t}\n> > +\treturn fd;\n> > +}\n> \n> Don't you want to look at and validate buffer[40+offset] in any\n> way, other than what get_sha1_hex() does (which is issspace())?\n\nRight. Will fix. (Although I will only write two \"s\" instead of three...)\n\n> > +/* if in && *in >= 0, take that as input file descriptor instead */\n> > +static int fork_with_pipe(const char **argv, int *in, int *out)\n> > +{\n> > +\tint needs_in, needs_out;\n> > +\tint fdin[2], fdout[2], pid;\n> > +\n> > +\tneeds_in = in && *in < 0;\n> > +\tif (needs_in) {\n> > +\t\tif (pipe(fdin) < 0)\n> > +\t\t\treturn error(\"could not setup pipe\");\n> > +\t\t*in = fdin[1];\n> > +\t}\n> > +\n> > +\tneeds_out = out && *out < 0;\n> > +\tif (needs_out) {\n> > +\t\tif (pipe(fdout) < 0)\n> > +\t\t\treturn error(\"could not setup pipe\");\n> > +\t\t*out = fdout[0];\n> > +\t}\n> > +\n> > +\tif ((pid = fork()) < 0) {\n> > +\t\tif (needs_in) {\n> > +\t\t\tclose(fdin[0]);\n> > +\t\t\tclose(fdin[1]);\n> > +\t\t}\n> > +\t\tif (needs_out) {\n> > +\t\t\tclose(fdout[0]);\n> > +\t\t\tclose(fdout[1]);\n> > +\t\t}\n> > +\t\treturn error(\"could not fork\");\n> > +\t}\n> > +\tif (!pid) {\n> > +\t\tif (needs_in) {\n> > +\t\t\tdup2(fdin[0], 0);\n> > +\t\t\tclose(fdin[0]);\n> > +\t\t\tclose(fdin[1]);\n> > +\t\t} else if (in)\n> > +\t\t\tdup2(*in, 0);\n> > +\t\tif (needs_out) {\n> > +\t\t\tdup2(fdout[1], 1);\n> > +\t\t\tclose(fdout[0]);\n> > +\t\t\tclose(fdout[1]);\n> > +\t\t} else if (out)\n> > +\t\t\tdup2(*out, 1);\n> > +\t\texit(execv_git_cmd(argv));\n> > +\t}\n> \n> Do you need to close *in and *out that are given by the caller\n> after dup2() in the child?\n\nI probably should. Will fix.\n\n> > +static int verify_bundle(struct bundle_header *header)\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> > +\tconst char *argv[5] = {\"rev-list\", \"--stdin\", \"--not\", \"--all\", NULL};\n> > +\tint pid, in, out, i, ret = 0;\n> > +\tchar buffer[1024];\n> > +\n> > +\tin = out = -1;\n> > +\tpid = fork_with_pipe(argv, &in, &out);\n> > +\tif (pid < 0)\n> > +\t\treturn error(\"Could not fork rev-list\");\n> > +\tif (!fork()) {\n> > +\t\tfor (i = 0; i < p->nr; i++) {\n> > +\t\t\twrite(in, sha1_to_hex(p->list[i].sha1), 40);\n> > +\t\t\twrite(in, \"\\n\", 1);\n> > +\t\t}\n> > +\t\tclose(in);\n> > +\t\texit(0);\n> > +\t}\n> > +\tclose(in);\n> \n> What if write() fails? That can happen when one of the objects\n> you feed here, or its parent objects, is missing from your\n> repository -- receiving rev-list would die() and the writing\n> child would sigpipe.\n\nThe error is caught afterwards, with WEXITSTATUS(), no? It might be \ncleaner to check, but is it really necessary?\n\n> I also wonder who waits for this child...\n\nrev-list. It only exits when that child closes the input.\n\n> > +\twhile (read_string(out, buffer, sizeof(buffer)) > 0) {\n> > +\t\tif (ret++ == 0)\n> > +\t\t\terror (\"The bundle requires the following commits you lack:\");\n> > +\t\tfprintf(stderr, \"%s\", buffer);\n> > +\t}\n> > +\tclose(out);\n> \n> I do not think this error message would buy you anything. If\n> you say:\n> \n> \techo commit | rev-list --stdin --not --all\n> \n> and you got commit back, that means you _do_ have that commit,\n> and that commit reaches some ref you have (objects associated to\n> that commit may be still missing). If commit is truly missing,\n> then you do not get commit output from rev-list as it cannot\n> even determine if it is \"--not --all\" or not -- it would error\n> out, which is the more important check to see if prereqs are\n> lacking.\n\nYes, I realized that rev-list die()s, and does not output the missing \nrevs -- as git-bundle.sh expected it to. When I found out about that, I \nfailed to remove that bogus error. Will fix.\n\n> > +\twhile (waitpid(pid, &i, 0) < 0)\n> > +\t\tif (errno != EINTR)\n> > +\t\t\treturn -1;\n> > +\tif (!ret && (!WIFEXITED(i) || WEXITSTATUS(i)))\n> > +\t\treturn error(\"At least one prerequisite is lacking.\");\n> \n> So you would want to keep this error, perhaps even make it more\n> helpful by suggesting which ones are missing.\n\nrev-list die()s with the first missing. It does not continue, printing \nother missing revs.\n\n> > +static int list_heads(struct bundle_header *header, int argc, const char **argv)\n> > +{\n> > +\tint i;\n> > +\tstruct ref_list *r = &header->references;\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> I wonder if we want to have a way to list prereqs in similar way.\n\nThat is what verify should be for. Maybe add \"-v\" to the \"verify\" \nsubcommand, make it properly builtin, and output non-missing prerequisites \nonly with \"-v\"?\n\n> > +static void show_commit(struct commit *commit)\n> > +{\n> > +\twrite(1, sha1_to_hex(commit->object.sha1), 40);\n> > +\twrite(1, \"\\n\", 1);\n> > +\tif (commit->parents) {\n> > +\t\tfree_commit_list(commit->parents);\n> > +\t\tcommit->parents = NULL;\n> > +\t}\n> > +}\n> \n> (everywhere) We would want write_in_full() with error handling\n> that dies even on EPIPE.\n\nOr write_or_die()? I mean, if write() fails, we cannot sanely continue \nanyway, right?\n\n> > +static void show_object(struct object_array_entry *p)\n> > +{\n> > +\t/* An object with name \"foo\\n0000000...\" can be used to\n> > +\t * * confuse downstream git-pack-objects very badly.\n> > +\t * */\n> \n> An interesting way to wrap comments.\n\nBad, bad vi. Will fix.\n\n> > +\t/* write prerequisites */\n> > +\tmemcpy(argv_boundary + 2, argv + 1, argc * sizeof(const char *));\n> > +\targv_boundary[0] = \"rev-list\";\n> > +\targv_boundary[1] = \"--boundary\";\n> > +\targv_boundary[argc + 1] = NULL;\n> > +\tout = -1;\n> > +\tpid = fork_with_pipe(argv_boundary, NULL, &out);\n> > +\tif (pid < 0)\n> > +\t\treturn -1;\n> > +\twhile ((i = read_string(out, buffer, sizeof(buffer))) > 0)\n> > +\t\tif (buffer[0] == '-')\n> > +\t\t\twrite(bundle_fd, buffer, i);\n> \n> It would be helpful for the recipient if you can append output\n> >from git-describe (or name-rev) when the buffer lacks \"name\".\n\nHmm. This could take a long time, as -describe is not really fast. ATM \nname-rev is not fast either, but I have plans to make it so. So obiously, \nI would prefer name-rev output. Comments?\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\", \"--stdin\", NULL};\n> > +\tint pid, status, dev_null;\n> > +\n> > +\tif (verify_bundle(header))\n> > +\t\treturn -1;\n> > +\tdev_null = open(\"/dev/null\", O_WRONLY);\n> > +\tpid = fork_with_pipe(argv_index_pack, &bundle_fd, &dev_null);\n> > +\tif (pid < 0)\n> > +\t\treturn error(\"Could not spawn index-pack\");\n> > +\tclose(bundle_fd);\n> > +\twhile (waitpid(pid, &status, 0) < 0)\n> > +\t\tif (errno != EINTR)\n> > +\t\t\treturn error(\"index-pack died\");\n> > +\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n> > +\t\treturn error(\"index-pack exited with status %d\",\n> > +\t\t\t\tWEXITSTATUS(status));\n> > +\treturn list_heads(header, argc, argv);\n> > +}\n> \n> We might want to later use --keep for this as well...\n\nYes, later. It is such a low-hanging fruit that I'd prefer somebody else \nto get involved with the code.\n\n> > diff --git a/index-pack.c b/index-pack.c\n> > index fa9a0e7..5ccf4c4 100644\n> > --- a/index-pack.c\n> > +++ b/index-pack.c\n> > @@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n> > \t/* If input_fd is a file, we should have reached its end now. */\n> > \tif (fstat(input_fd, &st))\n> > \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n> > -\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > -\t\tdie(\"pack has junk at the end\");\n> > +\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > +\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode, (int)st.st_size, (int)consumed_bytes, input_fd);\n> > \n> > \tif (!nr_deltas)\n> > \t\treturn;\n> \n> ??\n\nAs I said in my reply to Nico, if input_fd is 0, but really comes from a \nfile, this check will fail (the bundle is _strictly_ larger than the \npack).\n\nWould you like the fixes on top of the big commit, or a replacement patch?\n\nCiao,\nDscho\n"},{"id":"35235","messageId":"Pine.LNX.4.63.0702221719030.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"7v4ppellev.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-22T16:20:22Z","receivedAt":"2007-02-22T16:20:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 21 Feb 2007, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> >> +static int verify_bundle(struct bundle_header *header)\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> >> +\tconst char *argv[5] = {\"rev-list\", \"--stdin\", \"--not\", \"--all\", NULL};\n> >> +\tint pid, in, out, i, ret = 0;\n> >> +\tchar buffer[1024];\n> >> +\n> >> +\tin = out = -1;\n> >> +\tpid = fork_with_pipe(argv, &in, &out);\n> >> +\tif (pid < 0)\n> >> +\t\treturn error(\"Could not fork rev-list\");\n> >> +\tif (!fork()) {\n> >> +\t\tfor (i = 0; i < p->nr; i++) {\n> >> +\t\t\twrite(in, sha1_to_hex(p->list[i].sha1), 40);\n> >> +\t\t\twrite(in, \"\\n\", 1);\n> >> +\t\t}\n> >> +\t\tclose(in);\n> >> +\t\texit(0);\n> >> +\t}\n> >> +\tclose(in);\n> >\n> > What if write() fails? That can happen when one of the objects\n> > you feed here, or its parent objects, is missing from your\n> > repository -- receiving rev-list would die() and the writing\n> > child would sigpipe.\n> >\n> > I also wonder who waits for this child...\n> \n> In general, fork() to avoid bidirectional pipe deadlock is a\n> good discipline, but in this particular case I think it would be\n> easier to handle errors if you don't (and it would save another\n> process). The other side \"rev-list --stdin --not --all\" is\n> running a limited traversal, and would not emit anything until\n> you stop feeding it from --stdin, or until it dies because you\n> fed it a commit that does not exist. So as long as you check\n> the error condition from write() for EPIPE to notice the other\n> end died, I think you are Ok.\n\nThinking about this deeper, I have to say I find my decision to use \n\"--stdin\" rather silly, given that I know the exact number of revisions, \nand their SHA1s.\n\nBut it might make more sense to rewrite the checking part, instead of \nfork()ing it.\n\nCiao,\nDscho\n"},{"id":"35236","messageId":"alpine.LRH.0.82.0702221117080.27932@xanadu.home","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702221654131.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-02-22T16:24:32Z","receivedAt":"2007-02-22T16:24:32Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Wed, 21 Feb 2007, Nicolas Pitre wrote:\n> \n> > On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n> > \n> > > diff --git a/index-pack.c b/index-pack.c\n> > > index fa9a0e7..5ccf4c4 100644\n> > > --- a/index-pack.c\n> > > +++ b/index-pack.c\n> > > @@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n> > > \t/* If input_fd is a file, we should have reached its end now. */\n> > > \tif (fstat(input_fd, &st))\n> > > \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n> > > -\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > > -\t\tdie(\"pack has junk at the end\");\n> > > +\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > > +\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode, (int)st.st_size, (int)consumed_bytes, input_fd);\n> > > \n> > > \tif (!nr_deltas)\n> > > \t\treturn;\n> > \n> > What is this supposed to mean?\n> \n> The funny thing is, if you stream part of the bundle file to index-pack, \n> S_ISREG(st.st_mode) is true, even if input_fd == 0.\n\nHmmmm. indeed..\n\nCould you please make the test, including the call to fstat(), \nconditional on !from_stdin instead?\n\nAlso I don't see the point of displaying the mode since we know that \nS_ISREG(st.st_mode) is true, and input_fd is of little interest as well.\n\n\nNicolas\n"},{"id":"35238","messageId":"alpine.LRH.0.82.0702221126090.27932@xanadu.home","threadId":"6910","inReplyTo":"45DDC158.9090902@fs.ei.tum.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-02-22T16:28:17Z","receivedAt":"2007-02-22T16:28:17Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 22 Feb 2007, Simon 'corecode' Schubert wrote:\n\n> Johannes Schindelin wrote:\n> > Hi,\n> > \n> > On Wed, 21 Feb 2007, Nicolas Pitre wrote:\n> > \n> > > On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n> > > \n> > > > diff --git a/index-pack.c b/index-pack.c\n> > > > index fa9a0e7..5ccf4c4 100644\n> > > > --- a/index-pack.c\n> > > > +++ b/index-pack.c\n> > > > @@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n> > > > \t/* If input_fd is a file, we should have reached its end now. */\n> > > > \tif (fstat(input_fd, &st))\n> > > > \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n> > > > -\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > > > -\t\tdie(\"pack has junk at the end\");\n> > > > +\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > > > +\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode,\n> > > > (int)st.st_size, (int)consumed_bytes, input_fd);\n> > > > \tif (!nr_deltas)\n> > > > \t\treturn;\n> > > What is this supposed to mean?\n> > \n> > The funny thing is, if you stream part of the bundle file to index-pack,\n> > S_ISREG(st.st_mode) is true, even if input_fd == 0.\n> \n> Well, of course: you opened a regular file and pass this as stdin to\n> index-pack.\n> \n> Maybe something like this would be cleaner:\n> \n> if (IS_REF(st.st_mode) && lseek(input_fd, 0, SEEK_CUR) != st.st_size)\n\nNo that won't work because the input is buffered in the fill() function.\n\n\nNicolas\n"},{"id":"35239","messageId":"Pine.LNX.4.63.0702221735130.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"45DDC158.9090902@fs.ei.tum.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-22T16:37:35Z","receivedAt":"2007-02-22T16:37:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 22 Feb 2007, Simon 'corecode' Schubert wrote:\n\n> Johannes Schindelin wrote:\n> > Hi,\n> > \n> > On Wed, 21 Feb 2007, Nicolas Pitre wrote:\n> > \n> > > On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n> > > \n> > > > diff --git a/index-pack.c b/index-pack.c\n> > > > index fa9a0e7..5ccf4c4 100644\n> > > > --- a/index-pack.c\n> > > > +++ b/index-pack.c\n> > > > @@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n> > > > \t/* If input_fd is a file, we should have reached its end now. */\n> > > > \tif (fstat(input_fd, &st))\n> > > > \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n> > > > -\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > > > -\t\tdie(\"pack has junk at the end\");\n> > > > +\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > > > +\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode,\n> > > > (int)st.st_size, (int)consumed_bytes, input_fd);\n> > > > \tif (!nr_deltas)\n> > > > \t\treturn;\n> > > What is this supposed to mean?\n> > \n> > The funny thing is, if you stream part of the bundle file to index-pack,\n> > S_ISREG(st.st_mode) is true, even if input_fd == 0.\n> \n> Well, of course: you opened a regular file and pass this as stdin to\n> index-pack.\n> \n> Maybe something like this would be cleaner:\n> \n> if (IS_REF(st.st_mode) && lseek(input_fd, 0, SEEK_CUR) != st.st_size)\n> \tdie(\"...\");\n\nThe lseek would fail whenever the input is _not_ a file, dying. Since \nindex-pack is called from fetch-pack, with a socket instead of a file, it \nwould fail for the most common user.\n\nMy patch was only a minimal fixup to allow \"--stdin\" even starting in the \nmiddle of a file, and it does not break fetching.\n\nCiao,\nDscho\n"},{"id":"35240","messageId":"45DDC8D7.60902@fs.ei.tum.de","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702221735130.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Simon 'corecode' Schubert","fromEmail":"corecode@fs.ei.tum.de","sentAt":"2007-02-22T16:46:15Z","receivedAt":"2007-02-22T16:46:15Z","isPatch":true,"sender":{"key":"corecode@fs.ei.tum.de","avatar":"https://gravatar.com/avatar/eff9dbf0cdac0d1e6a6cd7ed0e50763edcb376b493b5253a35ff167918ad79e1?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Hi,\n> \n> On Thu, 22 Feb 2007, Simon 'corecode' Schubert wrote:\n> \n>> Johannes Schindelin wrote:\n>>> Hi,\n>>>\n>>> On Wed, 21 Feb 2007, Nicolas Pitre wrote:\n>>>\n>>>> On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n>>>>\n>>>>> diff --git a/index-pack.c b/index-pack.c\n>>>>> index fa9a0e7..5ccf4c4 100644\n>>>>> --- a/index-pack.c\n>>>>> +++ b/index-pack.c\n>>>>> @@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n>>>>> \t/* If input_fd is a file, we should have reached its end now. */\n>>>>> \tif (fstat(input_fd, &st))\n>>>>> \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n>>>>> -\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n>>>>> -\t\tdie(\"pack has junk at the end\");\n>>>>> +\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n>>>>> +\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode,\n>>>>> (int)st.st_size, (int)consumed_bytes, input_fd);\n>>>>> \tif (!nr_deltas)\n>>>>> \t\treturn;\n>>>> What is this supposed to mean?\n>>> The funny thing is, if you stream part of the bundle file to index-pack,\n>>> S_ISREG(st.st_mode) is true, even if input_fd == 0.\n>> Well, of course: you opened a regular file and pass this as stdin to\n>> index-pack.\n>>\n>> Maybe something like this would be cleaner:\n>>\n>> if (IS_REF(st.st_mode) && lseek(input_fd, 0, SEEK_CUR) != st.st_size)\n>> \tdie(\"...\");\n> \n> The lseek would fail whenever the input is _not_ a file, dying. Since \n> index-pack is called from fetch-pack, with a socket instead of a file, it \n> would fail for the most common user.\n\nthat's why i put IS_REF (should read IS_REG) there.\n\nbut as nicolas pointed out, this won't work for read-ahead.\n\ncheers\n simon\n\n-- \nServe - BSD +++ RENT this banner advert +++ ASCII Ribbon /\"\\\nWork - Mac +++ space for low €€€ NOW!1 +++ Campaign \\ /\nParty Enjoy Relax | http://dragonflybsd.org Against HTML \\\nDude 2c 2 the max ! http://golden-apple.biz Mail + News / \\\n\n"},{"id":"35241","messageId":"Pine.LNX.4.63.0702221805070.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"45DDC8D7.60902@fs.ei.tum.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-22T17:09:02Z","receivedAt":"2007-02-22T17:09:02Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 22 Feb 2007, Simon 'corecode' Schubert wrote:\n\n> Johannes Schindelin wrote:\n> > \n> > On Thu, 22 Feb 2007, Simon 'corecode' Schubert wrote:\n> > \n> > > Maybe something like this would be cleaner:\n> > > \n> > > if (IS_REF(st.st_mode) && lseek(input_fd, 0, SEEK_CUR) != st.st_size)\n> > > \tdie(\"...\");\n> > \n> > The lseek would fail whenever the input is _not_ a file, dying. Since\n> > index-pack is called from fetch-pack, with a socket instead of a file, it\n> > would fail for the most common user.\n> \n> that's why i put IS_REF (should read IS_REG) there.\n\nAh, yes, I understand.\n\n> but as nicolas pointed out, this won't work for read-ahead.\n\nWell, you could do this:\n\nif (S_ISREG(st.st_mode) &&\n\t\tlseek(input_fd, 0, SEEK_CUR) - input_len != st.st_size)\n\tdie(\"...\");\n\nOf course, this suggests that a file containing a pack must not have \n_anything_ after the pack. But AFAIR the reading part chokes on \"garbage \nafter pack\" anyway.\n\nCiao,\nDscho\n"},{"id":"35242","messageId":"Pine.LNX.4.63.0702221811130.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"alpine.LRH.0.82.0702221117080.27932@xanadu.home","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-22T17:12:10Z","receivedAt":"2007-02-22T17:12:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 22 Feb 2007, Nicolas Pitre wrote:\n\n> On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n> \n> > Hi,\n> > \n> > On Wed, 21 Feb 2007, Nicolas Pitre wrote:\n> > \n> > > On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n> > > \n> > > > diff --git a/index-pack.c b/index-pack.c\n> > > > index fa9a0e7..5ccf4c4 100644\n> > > > --- a/index-pack.c\n> > > > +++ b/index-pack.c\n> > > > @@ -457,8 +457,8 @@ static void parse_pack_objects(unsigned char *sha1)\n> > > > \t/* If input_fd is a file, we should have reached its end now. */\n> > > > \tif (fstat(input_fd, &st))\n> > > > \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n> > > > -\tif (S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > > > -\t\tdie(\"pack has junk at the end\");\n> > > > +\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n> > > > +\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode, (int)st.st_size, (int)consumed_bytes, input_fd);\n> > > > \n> > > > \tif (!nr_deltas)\n> > > > \t\treturn;\n> > > \n> > > What is this supposed to mean?\n> > \n> > The funny thing is, if you stream part of the bundle file to index-pack, \n> > S_ISREG(st.st_mode) is true, even if input_fd == 0.\n> \n> Hmmmm. indeed..\n> \n> Could you please make the test, including the call to fstat(), \n> conditional on !from_stdin instead?\n\nHow about using Simon's idea instead (subtracting input_len)?\n\n> Also I don't see the point of displaying the mode since we know that \n> S_ISREG(st.st_mode) is true, and input_fd is of little interest as well.\n\nAs you probably guessed, these are debug remnants. Will fix.\n\nCiao,\nDscho\n"},{"id":"35243","messageId":"alpine.LRH.0.82.0702221219370.27932@xanadu.home","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702221811130.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-02-22T17:21:59Z","receivedAt":"2007-02-22T17:21:59Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Thu, 22 Feb 2007, Johannes Schindelin wrote:\n\n> On Thu, 22 Feb 2007, Nicolas Pitre wrote:\n> \n> > Could you please make the test, including the call to fstat(), \n> > conditional on !from_stdin instead?\n> \n> How about using Simon's idea instead (subtracting input_len)?\n\nYes it would work.\n\n\nNicolas\n"},{"id":"35245","messageId":"Pine.LNX.4.63.0702221913250.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702220157130.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"[PATCH] git-bundle: assorted fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-22T18:14:14Z","receivedAt":"2007-02-22T18:14:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThis patch fixes issues mentioned by Junio, Nico and Simon:\n\n- I forgot to convert the usage string when removing the \"--\" from\n the subcommands,\n- a style fix in the bundle_header,\n- use xread() instead of read(),\n- use write_or_die() instead of write(),\n- make the bundle header extensible,\n- fail if the whitespace after a sha1 of a reference is missing,\n- close() the fds passed to a subprocess,\n- in verify_bundle(), do not use \"rev-list --stdin\", but rather\n pass the revs directly (avoiding a fork()),\n- fix a corrupted comment in show_object(), and\n- fix the size check in index_pack.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin-bundle.c | 107 ++++++++++++++++++++++++++++++------------------------\n index-pack.c | 5 ++-\n 2 files changed, 63 insertions(+), 49 deletions(-)\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex 4bd596a..521bbda 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -13,7 +13,7 @@\n * bundle supporting git-fetch, git-pull, and git-ls-remote\n */\n \n-static const char *bundle_usage=\"git-bundle (--create | --verify | --list-heads [refname]... | --unbundle [refname]... )\";\n+static const char *bundle_usage=\"git-bundle (create | verify | list-heads [refname]... | unbundle [refname]... )\";\n \n static const char bundle_signature[] = \"# v2 git bundle\\n\";\n \n@@ -39,7 +39,8 @@ static void add_to_ref_list(const unsigned char *sha1, const char *name,\n }\n \n struct bundle_header {\n-\tstruct ref_list prerequisites, references;\n+\tstruct ref_list prerequisites;\n+\tstruct ref_list references;\n };\n \n /* this function returns the length of the string */\n@@ -47,7 +48,7 @@ static int read_string(int fd, char *buffer, int size)\n {\n \tint i;\n \tfor (i = 0; i < size - 1; i++) {\n-\t\tint count = read(fd, buffer + i, 1);\n+\t\tint 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@@ -75,18 +76,25 @@ static int read_header(const char *path, struct bundle_header *header) {\n \t}\n \twhile (read_string(fd, buffer, sizeof(buffer)) > 0\n \t\t\t&& buffer[0] != '\\n') {\n-\t\tint offset = buffer[0] == '-' ? 1 : 0;\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 = offset ? &header->prerequisites\n+\t\tstruct ref_list *list = is_prereq ? &header->prerequisites\n \t\t\t: &header->references;\n-\t\tif (get_sha1_hex(buffer + offset, sha1)) {\n-\t\t\tclose(fd);\n-\t\t\treturn error(\"invalid SHA1: %s\", buffer);\n-\t\t}\n+\t\tchar delim;\n+\n \t\tif (buffer[len - 1] == '\\n')\n \t\t\tbuffer[len - 1] = '\\0';\n-\t\tadd_to_ref_list(sha1, buffer + 41 + offset, list);\n+\t\tif (get_sha1_hex(buffer + offset, sha1)) {\n+\t\t\twarn(\"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@@ -127,20 +135,28 @@ static int fork_with_pipe(const char **argv, int *in, int *out)\n \t\t\tdup2(fdin[0], 0);\n \t\t\tclose(fdin[0]);\n \t\t\tclose(fdin[1]);\n-\t\t} else if (in)\n+\t\t} else if (in) {\n \t\t\tdup2(*in, 0);\n+\t\t\tclose(*in);\n+\t\t}\n \t\tif (needs_out) {\n \t\t\tdup2(fdout[1], 1);\n \t\t\tclose(fdout[0]);\n \t\t\tclose(fdout[1]);\n-\t\t} else if (out)\n+\t\t} else if (out) {\n \t\t\tdup2(*out, 1);\n+\t\t\tclose(*out);\n+\t\t}\n \t\texit(execv_git_cmd(argv));\n \t}\n \tif (needs_in)\n \t\tclose(fdin[0]);\n+\telse if (in)\n+\t\tclose(*in);\n \tif (needs_out)\n \t\tclose(fdout[1]);\n+\telse if (out)\n+\t\tclose(*out);\n \treturn pid;\n }\n \n@@ -151,29 +167,28 @@ static int verify_bundle(struct bundle_header *header)\n \t * to be verbose about the errors\n \t */\n \tstruct ref_list *p = &header->prerequisites;\n-\tconst char *argv[5] = {\"rev-list\", \"--stdin\", \"--not\", \"--all\", NULL};\n-\tint pid, in, out, i, ret = 0;\n+\tchar **argv;\n+\tint pid, out, i, ret = 0;\n \tchar buffer[1024];\n \n-\tin = out = -1;\n-\tpid = fork_with_pipe(argv, &in, &out);\n+\targv = xmalloc((p->nr + 4) * sizeof(const char *));\n+\targv[0] = \"rev-list\";\n+\targv[1] = \"--not\";\n+\targv[2] = \"--all\";\n+\tfor (i = 0; i < p->nr; i++)\n+\t\targv[i + 3] = xstrdup(sha1_to_hex(p->list[i].sha1));\n+\targv[p->nr + 3] = NULL;\n+\tout = -1;\n+\tpid = fork_with_pipe((const char **)argv, NULL, &out);\n \tif (pid < 0)\n \t\treturn error(\"Could not fork rev-list\");\n-\tif (!fork()) {\n-\t\tfor (i = 0; i < p->nr; i++) {\n-\t\t\twrite(in, sha1_to_hex(p->list[i].sha1), 40);\n-\t\t\twrite(in, \"\\n\", 1);\n-\t\t}\n-\t\tclose(in);\n-\t\texit(0);\n-\t}\n-\tclose(in);\n-\twhile (read_string(out, buffer, sizeof(buffer)) > 0) {\n-\t\tif (ret++ == 0)\n-\t\t\terror (\"The bundle requires the following commits you lack:\");\n-\t\tfprintf(stderr, \"%s\", buffer);\n-\t}\n+\twhile (read_string(out, buffer, sizeof(buffer)) > 0)\n+\t\t; /* do nothing */\n \tclose(out);\n+\tfor (i = 0; i < p->nr; i++)\n+\t\tfree(argv[i + 3]);\n+\tfree(argv);\n+\n \twhile (waitpid(pid, &i, 0) < 0)\n \t\tif (errno != EINTR)\n \t\t\treturn -1;\n@@ -205,8 +220,8 @@ static int list_heads(struct bundle_header *header, int argc, const char **argv)\n \n static void show_commit(struct commit *commit)\n {\n-\twrite(1, sha1_to_hex(commit->object.sha1), 40);\n-\twrite(1, \"\\n\", 1);\n+\twrite_or_die(1, sha1_to_hex(commit->object.sha1), 40);\n+\twrite_or_die(1, \"\\n\", 1);\n \tif (commit->parents) {\n \t\tfree_commit_list(commit->parents);\n \t\tcommit->parents = NULL;\n@@ -216,15 +231,15 @@ static void show_commit(struct commit *commit)\n static void show_object(struct object_array_entry *p)\n {\n \t/* An object with name \"foo\\n0000000...\" can be used to\n-\t * * confuse downstream git-pack-objects very badly.\n-\t * */\n+\t * confuse downstream git-pack-objects very badly.\n+\t */\n \tconst char *ep = strchr(p->name, '\\n');\n \tint len = ep ? ep - p->name : strlen(p->name);\n-\twrite(1, sha1_to_hex(p->item->sha1), 40);\n-\twrite(1, \" \", 1);\n+\twrite_or_die(1, sha1_to_hex(p->item->sha1), 40);\n+\twrite_or_die(1, \" \", 1);\n \tif (len)\n-\t\twrite(1, p->name, len);\n-\twrite(1, \"\\n\", 1);\n+\t\twrite_or_die(1, p->name, len);\n+\twrite_or_die(1, \"\\n\", 1);\n }\n \n static int create_bundle(struct bundle_header *header, const char *path,\n@@ -243,7 +258,7 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t\treturn error(\"Could not write to '%s'\", path);\n \n \t/* write signature */\n-\twrite(bundle_fd, bundle_signature, strlen(bundle_signature));\n+\twrite_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));\n \n \t/* write prerequisites */\n \tmemcpy(argv_boundary + 2, argv + 1, argc * sizeof(const char *));\n@@ -256,7 +271,7 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t\treturn -1;\n \twhile ((i = read_string(out, buffer, sizeof(buffer))) > 0)\n \t\tif (buffer[0] == '-')\n-\t\t\twrite(bundle_fd, buffer, i);\n+\t\t\twrite_or_die(bundle_fd, buffer, i);\n \twhile ((i = waitpid(pid, &status, 0)) < 0)\n \t\tif (errno != EINTR)\n \t\t\treturn error(\"rev-list died\");\n@@ -279,16 +294,16 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \t\t\tchar *ref;\n \t\t\tif (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)\n \t\t\t\tcontinue;\n-\t\t\twrite(bundle_fd, sha1_to_hex(e->item->sha1), 40);\n-\t\t\twrite(bundle_fd, \" \", 1);\n-\t\t\twrite(bundle_fd, ref, strlen(ref));\n-\t\t\twrite(bundle_fd, \"\\n\", 1);\n+\t\t\twrite_or_die(bundle_fd, sha1_to_hex(e->item->sha1), 40);\n+\t\t\twrite_or_die(bundle_fd, \" \", 1);\n+\t\t\twrite_or_die(bundle_fd, ref, strlen(ref));\n+\t\t\twrite_or_die(bundle_fd, \"\\n\", 1);\n \t\t\tfree(ref);\n \t\t}\n \t}\n \n \t/* end header */\n-\twrite(bundle_fd, \"\\n\", 1);\n+\twrite_or_die(bundle_fd, \"\\n\", 1);\n \n \t/* write pack */\n \targv_pack[0] = \"pack-objects\";\n@@ -301,7 +316,6 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \tif (pid < 0)\n \t\treturn error(\"Could not spawn pack-objects\");\n \tclose(1);\n-\tclose(bundle_fd);\n \tdup2(in, 1);\n \tclose(in);\n \tprepare_revision_walk(&revs);\n@@ -327,7 +341,6 @@ static int unbundle(struct bundle_header *header, int bundle_fd,\n \tpid = fork_with_pipe(argv_index_pack, &bundle_fd, &dev_null);\n \tif (pid < 0)\n \t\treturn error(\"Could not spawn index-pack\");\n-\tclose(bundle_fd);\n \twhile (waitpid(pid, &status, 0) < 0)\n \t\tif (errno != EINTR)\n \t\t\treturn error(\"index-pack died\");\ndiff --git a/index-pack.c b/index-pack.c\nindex 5ccf4c4..365f8f3 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -457,8 +457,9 @@ static void parse_pack_objects(unsigned char *sha1)\n \t/* If input_fd is a file, we should have reached its end now. */\n \tif (fstat(input_fd, &st))\n \t\tdie(\"cannot fstat packfile: %s\", strerror(errno));\n-\tif (input_fd && S_ISREG(st.st_mode) && st.st_size != consumed_bytes)\n-\t\tdie(\"pack has junk at the end: 0%o, %d, %d %d\", st.st_mode, (int)st.st_size, (int)consumed_bytes, input_fd);\n+\tif (S_ISREG(st.st_mode) &&\n+\t\t\tlseek(input_fd, 0, SEEK_CUR) - input_len != st.st_size)\n+\t\tdie(\"pack has junk at the end\");\n \n \tif (!nr_deltas)\n \t\treturn;\n-- \n1.5.0.1.2213.g94bc-dirty\n"},{"id":"35248","messageId":"7vr6sif1ox.fsf@assigned-by-dhcp.cox.net","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702221719030.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-22T19:10:38Z","receivedAt":"2007-02-22T19:10:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Thinking about this deeper, I have to say I find my decision to use \n> \"--stdin\" rather silly, given that I know the exact number of revisions, \n> and their SHA1s.\n\nThe only worry I would have is if that exact number is too large\nthat you cannot pass them sensibly in argv[].\n"},{"id":"35250","messageId":"Pine.LNX.4.63.0702222015040.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"7vr6sif1ox.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-22T19:16:34Z","receivedAt":"2007-02-22T19:16:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 22 Feb 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Thinking about this deeper, I have to say I find my decision to use \n> > \"--stdin\" rather silly, given that I know the exact number of revisions, \n> > and their SHA1s.\n> \n> The only worry I would have is if that exact number is too large\n> that you cannot pass them sensibly in argv[].\n\nI thought there is only a limitation in bash?\n\nWell, anyway, I think that it makes sense writing just a little loop to \nfind if we have the revs or not, to be able to warn about them properly.\n\nI'll do that.\n\nCiao,\nDscho\n"},{"id":"35257","messageId":"7vy7mqdklr.fsf@assigned-by-dhcp.cox.net","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702222015040.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-22T20:05:04Z","receivedAt":"2007-02-22T20:05:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n>> The only worry I would have is if that exact number is too large\n>> that you cannot pass them sensibly in argv[].\n>\n> I thought there is only a limitation in bash?\n\nI am sure kernel folks on the list would correct me, but my\nunderstanding is that is execve(2) limitation and you would get\nE2BIG.\n"},{"id":"35260","messageId":"Pine.LNX.4.63.0702222119010.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"7vy7mqdklr.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-22T20:25:41Z","receivedAt":"2007-02-22T20:25:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 22 Feb 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> >> The only worry I would have is if that exact number is too large\n> >> that you cannot pass them sensibly in argv[].\n> >\n> > I thought there is only a limitation in bash?\n> \n> I am sure kernel folks on the list would correct me, but my\n> understanding is that is execve(2) limitation and you would get\n> E2BIG.\n\nOkay. So how about this (on top of my previous fixup):\n\n-- snipsnap --\n[PATCH] git-bundle: avoid fork() in verify_bundle()\n\nWe can use the revision walker easily for checking if the\nprerequisites are met, instead of fork()ing off a rev-list,\nwhich would list only the first unmet prerequisite.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin-bundle.c | 75 ++++++++++++++++++++++++++++++++++-------------------\n 1 files changed, 48 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex 521bbda..4ac06f9 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -19,7 +19,7 @@ static const char bundle_signature[] = \"# v2 git bundle\\n\";\n \n struct ref_list {\n \tunsigned int nr, alloc;\n-\tstruct {\n+\tstruct ref_list_entry {\n \t\tunsigned char sha1[20];\n \t\tchar *name;\n \t} *list;\n@@ -167,33 +167,54 @@ static int verify_bundle(struct bundle_header *header)\n \t * to be verbose about the errors\n \t */\n \tstruct ref_list *p = &header->prerequisites;\n-\tchar **argv;\n-\tint pid, out, i, ret = 0;\n-\tchar buffer[1024];\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 = \"The bundle requires these lacking revs:\";\n \n-\targv = xmalloc((p->nr + 4) * sizeof(const char *));\n-\targv[0] = \"rev-list\";\n-\targv[1] = \"--not\";\n-\targv[2] = \"--all\";\n-\tfor (i = 0; i < p->nr; i++)\n-\t\targv[i + 3] = xstrdup(sha1_to_hex(p->list[i].sha1));\n-\targv[p->nr + 3] = NULL;\n-\tout = -1;\n-\tpid = fork_with_pipe((const char **)argv, NULL, &out);\n-\tif (pid < 0)\n-\t\treturn error(\"Could not fork rev-list\");\n-\twhile (read_string(out, buffer, sizeof(buffer)) > 0)\n-\t\t; /* do nothing */\n-\tclose(out);\n-\tfor (i = 0; i < p->nr; i++)\n-\t\tfree(argv[i + 3]);\n-\tfree(argv);\n-\n-\twhile (waitpid(pid, &i, 0) < 0)\n-\t\tif (errno != EINTR)\n-\t\t\treturn -1;\n-\tif (!ret && (!WIFEXITED(i) || WEXITSTATUS(i)))\n-\t\treturn error(\"At least one prerequisite is lacking.\");\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 |= BOUNDARY_SHOW;\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 == 0)\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 & BOUNDARY_SHOW)\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 \treturn ret;\n }\n-- \n1.5.0.1.2214.gf22e-dirty\n"},{"id":"35286","messageId":"45DE4507.7090206@verizon.net","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702221913250.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: assorted fixes","fromName":"Mark Levedahl","fromEmail":"mdl123@verizon.net","sentAt":"2007-02-23T01:36:07Z","receivedAt":"2007-02-23T01:36:07Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n\nI built this under cygwin, it is running but I find two regressions \ncompared to my git-bundle.sh:\n\n1) git bundle create --all does not record any references, \nwhile it should record all references (heads and tags). From shell, this \nrequires use of both git-rev-parse and git-show-ref:\n gitrevargs=$(git-rev-parse --symbolic --revs-only $args) || exit 1\n\n # find the refs to carry along and get sha1s for each.\n refs=\n fullrevargs=\n for arg in $gitrevargs ; do\n #ignore options and basis refs, get full ref name for things\n # we will transport rejecting anything ambiguous (e.g., user\n # gives master, have heads/master and remotes/origin/master, we\n # keep the former).\n case \"$arg\" in\n -* | ^*) fullrevargs=\"$fullrevargs $arg\";;\n *) ref=$(git-show-ref \"$arg\")\n test \"$(echo $ref | wc -w)\" = \"2\" || die \"Ambigous \nreference: $arg\n$ref\"\n fullrevargs=\"$fullrevargs ${ref#* }\"\n refs=\"$refs $ref\";;\n esac\n done\n\n2) git bundle verify reports only a single sha1 if prerequisites are \nnot met. git-bundle.sh would loop through finding each missing one, \nannotating with the one line commit message. The prerequisites were \nstored using:\n\n (for p in $prereqs ; do\n git-rev-list --pretty=one --max-count=1 $p\n done) > \"$prerequisites\"\n\nand then verify does:\n test -z \"$prereqs\" && return 0\n bad=$(echo \"$prereqs\" | cut -b-40 | git-rev-list --stdin --not --all \n2>&1)\n if test -n \"$bad\" ; then\n test \"$1\" = \"--silent\" && return 1\n echo \"error: $bfile requires the following commits you lack:\"\n echo \"$prereqs\" |\n while read sha1 comment ; do\n missing=$(git-rev-list $sha1 --not --all 2>&1)\n test -n \"$missing\" && echo \"$sha1 $comment\"\n done\n exit 1\n fi\n return 0\n\nThe difference is\n\n fatal: bad object 59e4aa84d4b4f7c6393317c68649ef7db3c4440c\n error: At least one prerequisite is lacking.\n\nvs.\n\n error: /home/mlevedahl/bundle2 requires the following commits you lack:\n 59e4aa84d4b4f7c6393317c68649ef7db3c4440c Merge remote branch \n'rvaas05/gem.418' into lcb.418\n 8eb8c8274c28be9435da8534f8bb598503f4f85a Merge remote branch \n'rvaas05/gem.418' into lcd.419\n\nThis was Junio's suggestion originally, and I think it a very good one. \nThe latter error message is far more helpful to some poor soul trying to \nfix a problem.\n\n\nAlso, I did track down the issue that forced me to use tar (or at least \n*some* archiver) in cygwin: it is a known bug without a planned fix that \nprecludes saving mixed text and binary in one file from bash, and seems \nto be tied deeply into bash, fork, and pipes. Basically, doing\n echo \"some text data\" > file\n echo \"some binary data\" >>file\nis totally confused as the underlying pipe / fork mechanism thinks file \nis text mode on the second operation. This happens regardless of mount \nmode or CYGWIN=binmode. The end result is under bash the binary data \nunconditionally suffers crlf->lf translation. You can look at at \nhttp://www.mail-archive.com/cygwin@cygwin.com/msg76319.html for more \ninformation.\n\nMark\n"},{"id":"35287","messageId":"Pine.LNX.4.63.0702230249340.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"45DE4507.7090206@verizon.net","subject":"Re: [PATCH] git-bundle: assorted fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-23T01:56:31Z","receivedAt":"2007-02-23T01:56:31Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 22 Feb 2007, Mark Levedahl wrote:\n\n> Johannes Schindelin wrote:\n> \n> I built this under cygwin, it is running but I find two regressions compared\n> to my git-bundle.sh:\n> \n> 1) git bundle create --all \n\nAre you sure you did not provide a bundle filename?\n\nBut indeed, my test shows that \"--all\" does not leave any refs. Bad.\n\nThis fixes it:\n\n-- snip --\n revision.c | 2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 4cf697e..823bbd1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -480,7 +480,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1, int flag,\n \tstruct all_refs_cb *cb = cb_data;\n \tstruct object *object = get_reference(cb->all_revs, path, sha1,\n \t\t\t\t\t cb->all_flags);\n-\tadd_pending_object(cb->all_revs, object, \"\");\n+\tadd_pending_object(cb->all_revs, object, path);\n \treturn 0;\n }\n-- snap --\n\nbut I think this adds a memory leak, and I don't know what else is \naffected by it.\n \n> 2) git bundle verify reports only a single sha1 if prerequisites are not\n> met.\n\nWith the two follow-up patches I sent, this issue should be resolved, no?\n\nCiao,\nDscho\n"},{"id":"35288","messageId":"45DE4D85.3000906@verizon.net","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702230249340.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: assorted fixes","fromName":"Mark Levedahl","fromEmail":"mdl123@verizon.net","sentAt":"2007-02-23T02:12:21Z","receivedAt":"2007-02-23T02:12:21Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n> Hi,\n> \n>> 1) git bundle create --all \n>> \n>\n> Are you sure you did not provide a bundle filename?\n> \nI did, you *have* to give a bundle filename..\n> But indeed, my test shows that \"--all\" does not leave any refs. Bad.\n>\n> This fixes it:\n> \nConfirmed - that works.\n> \n> \n>> 2) git bundle verify reports only a single sha1 if prerequisites are not\n>> met.\n>> \n>\n> With the two follow-up patches I sent, this issue should be resolved, no?\n>\n> Ciao,\n> Dscho\n>\n> \nI missed one of your patches, adding that, the problem is incompletely \nfixed: all missing commits are shown, but only as raw hashes (e.g., the \n1-line commit message is not there to describe what is missing).\n\nMark\n"},{"id":"35289","messageId":"Pine.LNX.4.63.0702230316570.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6910","inReplyTo":"45DE4D85.3000906@verizon.net","subject":"Re: [PATCH] git-bundle: assorted fixes","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-23T02:17:51Z","receivedAt":"2007-02-23T02:17:51Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\n\nOn Thu, 22 Feb 2007, Mark Levedahl wrote:\n\n> Johannes Schindelin wrote:\n>\n> > Mark wrote:\n> >\n> > > 2) git bundle verify reports only a single sha1 if prerequisites are \n> > > not met.\n> > \n> > With the two follow-up patches I sent, this issue should be resolved, \n> > no?\n> > \n> I missed one of your patches, adding that, the problem is incompletely \n> fixed: all missing commits are shown, but only as raw hashes (e.g., the \n> 1-line commit message is not there to describe what is missing).\n\nOkay, how about this on top:\n\n-- snipsnap --\n\n builtin-bundle.c | 7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex 4ac06f9..d11435b 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -267,7 +267,7 @@ 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 + 3) * sizeof(const char *));\n+\tconst char **argv_boundary = xmalloc((argc + 4) * sizeof(const char *));\n \tconst char **argv_pack = xmalloc(4 * sizeof(const char *));\n \tint pid, in, out, i, status;\n \tchar buffer[1024];\n@@ -282,10 +282,11 @@ static int create_bundle(struct bundle_header *header, const char *path,\n \twrite_or_die(bundle_fd, bundle_signature, strlen(bundle_signature));\n \n \t/* write prerequisites */\n-\tmemcpy(argv_boundary + 2, argv + 1, argc * sizeof(const char *));\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[argc + 1] = NULL;\n+\targv_boundary[2] = \"--pretty=oneline\";\n+\targv_boundary[argc + 2] = NULL;\n \tout = -1;\n \tpid = fork_with_pipe(argv_boundary, NULL, &out);\n \tif (pid < 0)\n"},{"id":"35290","messageId":"45DE5259.6090502@verizon.net","threadId":"6910","inReplyTo":"7v8xeqllxk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Mark Levedahl","fromEmail":"mdl123@verizon.net","sentAt":"2007-02-23T02:32:57Z","receivedAt":"2007-02-23T02:32:57Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Junio C Hamano wrote:\n>> diff --git a/Documentation/cmd-list.perl b/Documentation/cmd-list.perl\n>> index a2d6268..f61c77a 100755\n>> --- a/Documentation/cmd-list.perl\n>> +++ b/Documentation/cmd-list.perl\n>> @@ -70,6 +70,7 @@ git-archive mainporcelain\n>> git-bisect mainporcelain\n>> git-blame ancillaryinterrogators\n>> git-branch mainporcelain\n>> +git-bundle mainporcelain\n>> git-cat-file plumbinginterrogators\n>> git-checkout-index plumbingmanipulators\n>> git-checkout mainporcelain\n>> \n>\n> Is this really a mainporcelain?\n> I would say ancillarymanipulators (or perhaps synchelpers).\n>\n> \ngit bundle has four commands: create, verify, list-heads, and unbundle. \nThe last two are pure helper functions, basically plumbing. Verify is \nquestionable as to where it lies. But, create is the only way to create \na bundle, is logically equivalent to git push as a user command to move \ndata, so I called it mainporcelain because that is how git push is \nclassified.\n\n\n>> +\t/* write prerequisites */\n>> +\tmemcpy(argv_boundary + 2, argv + 1, argc * sizeof(const char *));\n>> +\targv_boundary[0] = \"rev-list\";\n>> +\targv_boundary[1] = \"--boundary\";\n>> +\targv_boundary[argc + 1] = NULL;\n>> +\tout = -1;\n>> +\tpid = fork_with_pipe(argv_boundary, NULL, &out);\n>> +\tif (pid < 0)\n>> +\t\treturn -1;\n>> +\twhile ((i = read_string(out, buffer, sizeof(buffer))) > 0)\n>> +\t\tif (buffer[0] == '-')\n>> +\t\t\twrite(bundle_fd, buffer, i);\n>> \n>\n> It would be helpful for the recipient if you can append output\n> from git-describe (or name-rev) when the buffer lacks \"name\".\n> \nI found the actual commit summary message (i.e., git-rev-list \n--pretty=one --max-count=1 sha1) the most useful of the various \nsummaries available.\n\n\n\nMark\n"},{"id":"35291","messageId":"45DE6191.5030200@verizon.net","threadId":"6910","inReplyTo":"Pine.LNX.4.63.0702230316570.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-bundle: assorted fixes","fromName":"Mark Levedahl","fromEmail":"mdl123@verizon.net","sentAt":"2007-02-23T03:37:53Z","receivedAt":"2007-02-23T03:37:53Z","isPatch":true,"sender":{"key":"mdl123@verizon.net","avatar":"https://avatars.githubusercontent.com/u/5302462?v=4"},"body":"Johannes Schindelin wrote:\n> \n> On Thu, 22 Feb 2007, Mark Levedahl wrote:\n> \n>> Johannes Schindelin wrote:\n>>\n>>> Mark wrote:\n>>>\n>>>> 2) git bundle verify reports only a single sha1 if prerequisites are \n>>>> not met.\n>>> With the two follow-up patches I sent, this issue should be resolved, \n>>> no?\n>>>\n>> I missed one of your patches, adding that, the problem is incompletely \n>> fixed: all missing commits are shown, but only as raw hashes (e.g., the \n>> 1-line commit message is not there to describe what is missing).\n> \n> Okay, how about this on top:\n> \n>\n\nThat works. I would change your error message slightly, perhaps...\n\ndiff --git a/builtin-bundle.c b/builtin-bundle.c\nindex d74afaa..e115feb 100644\n--- a/builtin-bundle.c\n+++ b/builtin-bundle.c\n@@ -172,7 +172,7 @@ static int verify_bundle(struct bundle_header *header)\n struct object_array refs;\n struct commit *commit;\n int i, ret = 0, req_nr;\n- const char *message = \"The bundle requires these lacking revs:\";\n+ const char *message = \"Your repository lacks these prerequisite \ncommits:\";\n\n init_revisions(&revs, NULL);\n for (i = 0; i < p->nr; i++) {\n\nMark\n"},{"id":"35293","messageId":"7vps81bi8l.fsf@assigned-by-dhcp.cox.net","threadId":"6910","inReplyTo":"45DE5259.6090502@verizon.net","subject":"Re: [PATCH] Add git-bundle: move objects and references by archive","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-23T04:39:06Z","receivedAt":"2007-02-23T04:39:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mark Levedahl writes:\n\n> I found the actual commit summary message (i.e., git-rev-list\n> --pretty=one --max-count=1 sha1) the most useful of the various\n> summaries available.\n\nI would agree that would be the case, and Johannes's follow-up\nseems to do that.\n"}]}