{"thread":{"id":"17346","subject":"[PATCH 1/2] tree.c: allow read_tree_recursive() to traverse gitlink entries","startedAt":"2009-01-25T00:52:04Z","lastAt":"2009-01-26T00:41:34Z","messageCount":14,"participants":["Lars Hjemli","Nanako Shiraishi","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"101804","messageId":"1232844726-14902-1-git-send-email-hjemli@gmail.com","threadId":"17346","inReplyTo":null,"subject":"[PATCH 0/2] Add submodule-support to git archive","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2009-01-25T00:52:04Z","receivedAt":"2009-01-25T00:52:04Z","isPatch":true,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"This is a cleaned up version of my previous patches which allows git archive\nto include submodule content in the archive output.\n\nThe main difference between this series and the previous ones is that the\nbehaviour of `git archive --submodules` are now predictable; the content\nincluded from submodules is defined by the gitlink entries found when\ntraversing the <tree-ish> specified on the command line, and the set of\nsubmodules to include are defined by specifying either `--submodules=all` or\n`--submodules=checkedout` (which is the default mode of operation, i.e. what\nyou get by only specifying `--submodules`).\n\nTo make the `--submodules` option more userfriendly, any submodule repository\ndiscovered during traversal will be registered as an alternate odb (this\nwill typically be required to make the inter-repository traversal succeed).\n\nFinally, the option `--submodules=group:<name>` is not yet implemented. I\nwanted to get these first two patches published early since they define the\nsemantics of the --submodules option. Adding a 'group' selector on top is\nmostly a question of pulling information out of .gitmodules and .git/config,\ni.e. not very exciting (but it will be done ;-)\n\nLars Hjemli (2):\n  tree.c: allow read_tree_recursive() to traverse gitlink entries\n  archive.c: add support for --submodules[=(all|checkedout)]\n\n Documentation/git-archive.txt |    5 ++\n archive.c                     |   81 +++++++++++++++++++++++++-\n archive.h                     |    4 +\n builtin-ls-tree.c             |    9 +--\n cache.h                       |    1 +\n merge-recursive.c             |    2 +-\n sha1_file.c                   |   11 +++-\n t/t5001-archive-submodules.sh |  129 +++++++++++++++++++++++++++++++++++++++++\n tree.c                        |   28 +++++++++\n 9 files changed, 259 insertions(+), 11 deletions(-)\n create mode 100755 t/t5001-archive-submodules.sh\n"},{"id":"101803","messageId":"1232844726-14902-2-git-send-email-hjemli@gmail.com","threadId":"17346","inReplyTo":"1232844726-14902-1-git-send-email-hjemli@gmail.com","subject":"[PATCH 1/2] tree.c: allow read_tree_recursive() to traverse gitlink entries","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2009-01-25T00:52:05Z","receivedAt":"2009-01-25T00:52:05Z","isPatch":true,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"When the callback function invoked from read_tree_recursive() returns\nthe value `READ_TREE_RECURSIVE` for a gitlink entry, the traversal will\nnow continue into the tree connected to the gitlinked commit. This\nfunctionality can be used to allow inter-repository operations, but\nsince the current users of read_tree_recursive() does not yet support\nsuch operations, they have been modified where necessary to make sure\nthat they never return READ_TREE_RECURSIVE for gitlink entries (hence\nno change in behaviour should be introduces by this patch alone).\n\nSigned-off-by: Lars Hjemli <hjemli@gmail.com>\n---\n archive.c         |    2 +-\n builtin-ls-tree.c |    9 ++-------\n merge-recursive.c |    2 +-\n tree.c            |   28 ++++++++++++++++++++++++++++\n 4 files changed, 32 insertions(+), 9 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 9ac455d..e6de039 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -132,7 +132,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \t\terr = write_entry(args, sha1, path.buf, path.len, mode, NULL, 0);\n \t\tif (err)\n \t\t\treturn err;\n-\t\treturn READ_TREE_RECURSIVE;\n+\t\treturn (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);\n \t}\n \n \tbuffer = sha1_file_to_archive(path_without_prefix, sha1, mode,\ndiff --git a/builtin-ls-tree.c b/builtin-ls-tree.c\nindex 5b63e6e..fca4631 100644\n--- a/builtin-ls-tree.c\n+++ b/builtin-ls-tree.c\n@@ -68,13 +68,8 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,\n \t\t *\n \t\t * Something similar to this incomplete example:\n \t\t *\n-\t\tif (show_subprojects(base, baselen, pathname)) {\n-\t\t\tstruct child_process ls_tree;\n-\n-\t\t\tls_tree.dir = base;\n-\t\t\tls_tree.argv = ls-tree;\n-\t\t\tstart_command(&ls_tree);\n-\t\t}\n+\t\tif (show_subprojects(base, baselen, pathname))\n+\t\t\tretval = READ_TREE_RECURSIVE;\n \t\t *\n \t\t */\n \t\ttype = commit_type;\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex b97026b..ee853b9 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -237,7 +237,7 @@ static int save_files_dirs(const unsigned char *sha1,\n \t\tstring_list_insert(newpath, &o->current_file_set);\n \tfree(newpath);\n \n-\treturn READ_TREE_RECURSIVE;\n+\treturn (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);\n }\n \n static int get_files_dirs(struct merge_options *o, struct tree *tree)\ndiff --git a/tree.c b/tree.c\nindex 03e782a..dfe4d5f 100644\n--- a/tree.c\n+++ b/tree.c\n@@ -131,6 +131,34 @@ int read_tree_recursive(struct tree *tree,\n \t\t\tif (retval)\n \t\t\t\treturn -1;\n \t\t\tcontinue;\n+\t\t} else if (S_ISGITLINK(entry.mode)) {\n+\t\t\tint retval;\n+\t\t\tstruct strbuf path;\n+\t\t\tunsigned int entrylen;\n+\t\t\tstruct commit *commit;\n+\n+\t\t\tentrylen = tree_entry_len(entry.path, entry.sha1);\n+\t\t\tstrbuf_init(&path, baselen + entrylen + 1);\n+\t\t\tstrbuf_add(&path, base, baselen);\n+\t\t\tstrbuf_add(&path, entry.path, entrylen);\n+\t\t\tstrbuf_addch(&path, '/');\n+\n+\t\t\tcommit = lookup_commit(entry.sha1);\n+\t\t\tif (!commit)\n+\t\t\t\tdie(\"Commit %s in submodule path %s not found\",\n+\t\t\t\t    sha1_to_hex(entry.sha1), path.buf);\n+\n+\t\t\tif (parse_commit(commit))\n+\t\t\t\tdie(\"Invalid commit %s in submodule path %s\",\n+\t\t\t\t    sha1_to_hex(entry.sha1), path.buf);\n+\n+\t\t\tretval = read_tree_recursive(commit->tree,\n+\t\t\t\t\t\t     path.buf, path.len,\n+\t\t\t\t\t\t     stage, match, fn, context);\n+\t\t\tstrbuf_release(&path);\n+\t\t\tif (retval)\n+\t\t\t\treturn -1;\n+\t\t\tcontinue;\n \t\t}\n \t}\n \treturn 0;\n-- \n1.6.1.150.g5e733b\n"},{"id":"101805","messageId":"1232844726-14902-3-git-send-email-hjemli@gmail.com","threadId":"17346","inReplyTo":"1232844726-14902-2-git-send-email-hjemli@gmail.com","subject":"[PATCH 2/2] archive.c: add support for --submodules[=(all|checkedout)]","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2009-01-25T00:52:06Z","receivedAt":"2009-01-25T00:52:06Z","isPatch":true,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"The --submodules option uses the enhanced read_tree_recursive() to\nenable inclusion of submodules in the generated archive.\n\nWhen invoked with `--submodules=all` all gitlink entries will be\ntraversed, and when invoked with --submodules=checkedout (the default\noption) only gitlink entries with a git repo (i.e. checked out sub-\nmodules) will be traversed.\n\nWhen a gitlink has been selected for traversal, it is required that all\nobjects necessary to perform this traversal are available in either the\nprimary odb or through an alternate odb. To this end, git archive will\ninsert the object database of the selected gitlink (when checked out)\nas an alternate odb, using the new function add_alt_odb(). And since\nalternates now can be added after parsing of objects/info/alternates,\nthe error message in link_alt_odb_entry() has been updated to not\nmention this file.\n\nSigned-off-by: Lars Hjemli <hjemli@gmail.com>\n---\n Documentation/git-archive.txt |    5 ++\n archive.c                     |   81 +++++++++++++++++++++++++-\n archive.h                     |    4 +\n cache.h                       |    1 +\n sha1_file.c                   |   11 +++-\n t/t5001-archive-submodules.sh |  129 +++++++++++++++++++++++++++++++++++++++++\n 6 files changed, 228 insertions(+), 3 deletions(-)\n create mode 100755 t/t5001-archive-submodules.sh\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex 41cbf9c..6068302 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -47,6 +47,11 @@ OPTIONS\n --prefix=<prefix>/::\n \tPrepend <prefix>/ to each filename in the archive.\n \n+--submodules[=<spec>]::\n+\tInclude the content of submodules in the archive. The specification\n+\tof which submodules to include can be either 'checkedout' (default)\n+\tor 'all'.\n+\n <extra>::\n \tThis can be any options that the archiver backend understand.\n \tSee next section.\ndiff --git a/archive.c b/archive.c\nindex e6de039..bb0c5c8 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -4,6 +4,7 @@\n #include \"attr.h\"\n #include \"archive.h\"\n #include \"parse-options.h\"\n+#include \"refs.h\"\n \n static char const * const archive_usage[] = {\n \t\"git archive [options] <tree-ish> [path...]\",\n@@ -91,6 +92,70 @@ static void setup_archive_check(struct git_attr_check *check)\n \tcheck[1].attr = attr_export_subst;\n }\n \n+static int include_repository(const char *path)\n+{\n+\tstruct stat st;\n+\tconst char *tmp;\n+\n+\t/* Return early if the path does not exist since it is OK to not\n+\t * checkout submodules.\n+\t */\n+\tif (stat(path, &st) && errno == ENOENT)\n+\t\treturn 1;\n+\n+\ttmp = read_gitfile_gently(path);\n+\tif (tmp) {\n+\t\tpath = tmp;\n+\t\tif (stat(path, &st))\n+\t\t\tdie(\"Unable to stat submodule gitdir %s: %s (%d)\",\n+\t\t\t    path, strerror(errno), errno);\n+\t}\n+\n+\tif (!S_ISDIR(st.st_mode))\n+\t\tdie(\"Submodule gitdir %s is not a directory\", path);\n+\n+\tif (add_alt_odb(mkpath(\"%s/objects\", path)))\n+\t\tdie(\"submodule odb %s could not be added as an alternate\",\n+\t\t    path);\n+\n+\treturn 0;\n+}\n+\n+static int check_gitlink(struct archiver_args *args, const unsigned char *sha1,\n+\t\t\t const char *path)\n+{\n+\tswitch (args->submodules) {\n+\tcase 0:\n+\t\treturn 0;\n+\n+\tcase SUBMODULES_ALL:\n+\t\t/* When all submodules are requested, we try to add any\n+\t\t * checked out submodules as alternate odbs. But we don't\n+\t\t * really care whether any particular submodule is checked\n+\t\t * out or not, we are going to try to traverse it anyways.\n+\t\t */\n+\t\tinclude_repository(mkpath(\"%s.git\", path));\n+\t\treturn READ_TREE_RECURSIVE;\n+\n+\tcase SUBMODULES_CHECKEDOUT:\n+\t\t/* If a repo is checked out at the gitlink path, we want to\n+\t\t * traverse into the submodule. But we ignore the current\n+\t\t * HEAD of the checked out submodule and always uses the SHA1\n+\t\t * recorded in the gitlink entry since we want the content\n+\t\t * of the archive to match the content of the <tree-ish>\n+\t\t * specified on the command line.\n+\t\t */\n+\t\tif (!include_repository(mkpath(\"%s.git\", path)))\n+\t\t\treturn READ_TREE_RECURSIVE;\n+\t\telse\n+\t\t\treturn 0;\n+\n+\tdefault:\n+\t\tdie(\"archive.c: invalid value for args->submodules: %d\",\n+\t\t    args->submodules);\n+\t}\n+}\n+\n struct archiver_context {\n \tstruct archiver_args *args;\n \twrite_archive_entry_fn_t write_entry;\n@@ -132,7 +197,8 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \t\terr = write_entry(args, sha1, path.buf, path.len, mode, NULL, 0);\n \t\tif (err)\n \t\t\treturn err;\n-\t\treturn (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);\n+\t\treturn (S_ISDIR(mode) ? READ_TREE_RECURSIVE :\n+\t\t\tcheck_gitlink(args, sha1, path.buf));\n \t}\n \n \tbuffer = sha1_file_to_archive(path_without_prefix, sha1, mode,\n@@ -253,6 +319,7 @@ static int parse_archive_args(int argc, const char **argv,\n \tconst char *base = NULL;\n \tconst char *remote = NULL;\n \tconst char *exec = NULL;\n+\tconst char *submodules = NULL;\n \tint compression_level = -1;\n \tint verbose = 0;\n \tint i;\n@@ -262,6 +329,9 @@ static int parse_archive_args(int argc, const char **argv,\n \t\tOPT_STRING(0, \"format\", &format, \"fmt\", \"archive format\"),\n \t\tOPT_STRING(0, \"prefix\", &base, \"prefix\",\n \t\t\t\"prepend prefix to each pathname in the archive\"),\n+\t\t{OPTION_STRING, 0, \"submodules\", &submodules, \"kind\",\n+\t\t\t\"include submodule content in the archive\",\n+\t\t\tPARSE_OPT_OPTARG, NULL, (intptr_t)\"checkedout\"},\n \t\tOPT__VERBOSE(&verbose),\n \t\tOPT__COMPR('0', &compression_level, \"store only\", 0),\n \t\tOPT__COMPR('1', &compression_level, \"compress faster\", 1),\n@@ -316,6 +386,15 @@ static int parse_archive_args(int argc, const char **argv,\n \t\t\t\t\tformat, compression_level);\n \t\t}\n \t}\n+\n+\tif (!submodules)\n+\t\targs->submodules = 0;\n+\telse if (!strcmp(submodules, \"checkedout\"))\n+\t\targs->submodules = SUBMODULES_CHECKEDOUT;\n+\telse if (!strcmp(submodules, \"all\"))\n+\t\targs->submodules = SUBMODULES_ALL;\n+\telse\n+\t\tdie(\"Invalid submodule kind: %s\", submodules);\n \targs->verbose = verbose;\n \targs->base = base;\n \targs->baselen = strlen(base);\ndiff --git a/archive.h b/archive.h\nindex 0b15b35..2b078b6 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -11,8 +11,12 @@ struct archiver_args {\n \tconst char **pathspec;\n \tunsigned int verbose : 1;\n \tint compression_level;\n+\tint submodules;\n };\n \n+#define SUBMODULES_CHECKEDOUT 1\n+#define SUBMODULES_ALL 2\n+\n typedef int (*write_archive_fn_t)(struct archiver_args *);\n \n typedef int (*write_archive_entry_fn_t)(struct archiver_args *args, const unsigned char *sha1, const char *path, size_t pathlen, unsigned int mode, void *buffer, unsigned long size);\ndiff --git a/cache.h b/cache.h\nindex 8d965b8..ea53e4b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -728,6 +728,7 @@ extern struct alternate_object_database {\n \tchar base[FLEX_ARRAY]; /* more */\n } *alt_odb_list;\n extern void prepare_alt_odb(void);\n+extern int add_alt_odb(const char *path);\n extern void add_to_alternates_file(const char *reference);\n typedef int alt_odb_fn(struct alternate_object_database *, void *);\n extern void foreach_alt_odb(alt_odb_fn, void*);\ndiff --git a/sha1_file.c b/sha1_file.c\nindex 360f7e5..53d8db7 100644\n--- a/sha1_file.c\n+++ b/sha1_file.c\n@@ -285,8 +285,7 @@ static int link_alt_odb_entry(const char * entry, int len, const char * relative\n \n \t/* Detect cases where alternate disappeared */\n \tif (!is_directory(ent->base)) {\n-\t\terror(\"object directory %s does not exist; \"\n-\t\t      \"check .git/objects/info/alternates.\",\n+\t\terror(\"Alternate object directory %s does not exist\",\n \t\t      ent->base);\n \t\tfree(ent);\n \t\treturn -1;\n@@ -2573,3 +2572,11 @@ int read_pack_header(int fd, struct pack_header *header)\n \t\treturn PH_ERROR_PROTOCOL;\n \treturn 0;\n }\n+\n+int add_alt_odb(const char *path)\n+{\n+\tint err = link_alt_odb_entry(path, strlen(path), NULL, 0);\n+\tif (!err)\n+\t\tprepare_packed_git_one((char *)path, 0);\n+\treturn err;\n+}\ndiff --git a/t/t5001-archive-submodules.sh b/t/t5001-archive-submodules.sh\nnew file mode 100755\nindex 0000000..14383b3\n--- /dev/null\n+++ b/t/t5001-archive-submodules.sh\n@@ -0,0 +1,129 @@\n+#!/bin/sh\n+\n+test_description='git archive can include submodule content'\n+\n+. ./test-lib.sh\n+\n+add_file()\n+{\n+\tgit add $1 &&\n+\tgit commit -m \"added $1\"\n+}\n+\n+add_submodule()\n+{\n+\tmkdir $1 && (\n+\t\tcd $1 &&\n+\t\tgit init &&\n+\t\techo \"File $2\" >$2 &&\n+\t\tadd_file $2\n+\t) &&\n+\tadd_file $1\n+}\n+\n+test_expect_success 'by default, submodules are not included' '\n+\techo \"File 1\" >1 &&\n+\tadd_file 1 &&\n+\tadd_submodule 2 3 &&\n+\tadd_submodule 4 5 &&\n+\tcat <<EOF >expected &&\n+1\n+2/\n+4/\n+EOF\n+\tgit archive HEAD >normal.tar &&\n+\ttar -tf normal.tar >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'with --submodules, checked out submodules are  included' '\n+\tcat <<EOF >expected &&\n+1\n+2/\n+2/3\n+4/\n+4/5\n+EOF\n+\tgit archive --submodules HEAD >full.tar &&\n+\ttar -tf full.tar >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'with --submodules=all, all submodules are included' '\n+\tgit archive --submodules=all HEAD >all.tar &&\n+\ttar -tf all.tar >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'submodules in submodules are supported' '\n+\t(cd 4 && add_submodule 6 7) &&\n+\tadd_file 4 &&\n+\tcat <<EOF >expected &&\n+1\n+2/\n+2/3\n+4/\n+4/5\n+4/6/\n+4/6/7\n+EOF\n+\tgit archive --submodules HEAD >recursive.tar &&\n+\ttar -tf recursive.tar >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'packed submodules are supported' '\n+\tmsg=$(cd 2 && git repack -ad && git count-objects) &&\n+\ttest \"$msg\" = \"0 objects, 0 kilobytes\" &&\n+\tgit archive --submodules HEAD >packed.tar &&\n+\ttar -tf packed.tar >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'missing submodule packs triggers an error' '\n+\tmv 2/.git/objects/pack .git/packdir2 &&\n+\ttest_must_fail git archive --submodules HEAD\n+'\n+\n+test_expect_success '--submodules skips non-checked out submodules' '\n+\tcat <<EOF >expected &&\n+1\n+2/\n+4/\n+4/5\n+4/6/\n+4/6/7\n+EOF\n+\trm -rf 2/.git &&\n+\tgit archive --submodules HEAD >partial.tar &&\n+\ttar -tf partial.tar >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success '--submodules=all fails if gitlinked objects are missing' '\n+\ttest_must_fail git archive --submodules=all HEAD\n+'\n+\n+test_expect_success \\\n+\t'--submodules=all does not require submodules to be checked out' '\n+\tcat <<EOF >expected &&\n+1\n+2/\n+2/3\n+4/\n+4/5\n+4/6/\n+4/6/7\n+EOF\n+\tmv .git/packdir2/* .git/objects/pack/ &&\n+\tgit archive --submodules=all HEAD >all2.tar &&\n+\ttar -tf all2.tar >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'missing objects in a submodule triggers an error' '\n+\tfind 4/.git/objects -type f | xargs rm &&\n+\ttest_must_fail git archive --submodules HEAD\n+'\n+\n+test_done\n-- \n1.6.1.150.g5e733b\n"},{"id":"101812","messageId":"20090125135340.6117@nanako3.lavabit.com","threadId":"17346","inReplyTo":"1232844726-14902-1-git-send-email-hjemli@gmail.com","subject":"Re: [PATCH 0/2] Add submodule-support to git archive","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-01-25T04:53:40Z","receivedAt":"2009-01-25T04:53:40Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Lars Hjemli <hjemli@gmail.com>:\n\n> This is a cleaned up version of my previous patches which allows git archive\n> to include submodule content in the archive output.\n>\n> The main difference between this series and the previous ones is that the\n> behaviour of `git archive --submodules` are now predictable; the content\n> included from submodules is defined by the gitlink entries found when\n> traversing the <tree-ish> specified on the command line, and the set of\n> submodules to include are defined by specifying either `--submodules=all` or\n> `--submodules=checkedout` (which is the default mode of operation, i.e. what\n> you get by only specifying `--submodules`).\n\nI wanted to try this because I use submodules in a repository at my work, but your patches conflicted a lot with your previous series of patches that are already in the next branch of Junio's. What would I do to try this new series? Fork a branch from Junio's master branch, apply your new patches, and merge the result to Junio's next?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"101821","messageId":"8c5c35580901250018x6827291cj36e6bcb10afa9b27@mail.gmail.com","threadId":"17346","inReplyTo":"20090125135340.6117@nanako3.lavabit.com","subject":"Re: [PATCH 0/2] Add submodule-support to git archive","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2009-01-25T08:18:06Z","receivedAt":"2009-01-25T08:18:06Z","isPatch":true,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"On Sun, Jan 25, 2009 at 05:53, Nanako Shiraishi <nanako3@lavabit.com> wrote:\n> What would I do to try this new series? Fork a branch from Junio's master branch,\n> apply your new patches, and merge the result to Junio's next?\n\nYes, that sounds right (btw: the series is buildt on top of 5dc1308562\n(Merge branch 'js/patience-diff') and can be pulled from\ngit://hjemli.net/pub/git/git lh/traverse-gitlinks).\n\nBut before merging with 'next', you'll need to `git revert -m 1 bdf31cbc00`.\n\n--\nlarsh\n"},{"id":"101824","messageId":"alpine.DEB.1.00.0901251225250.14855@racer","threadId":"17346","inReplyTo":"1232844726-14902-2-git-send-email-hjemli@gmail.com","subject":"Re: [PATCH 1/2] tree.c: allow read_tree_recursive() to traverse gitlink entries","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-25T11:43:17Z","receivedAt":"2009-01-25T11:43:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 25 Jan 2009, Lars Hjemli wrote:\n\n> When the callback function invoked from read_tree_recursive() returns\n> the value `READ_TREE_RECURSIVE` for a gitlink entry, the traversal will\n> now continue into the tree connected to the gitlinked commit.\n\n\\n\n\n> This functionality can be used to allow inter-repository operations, but \n> since the current users of read_tree_recursive() does not yet support \n> such operations, they have been modified where necessary to make sure \n> that they never return READ_TREE_RECURSIVE for gitlink entries (hence no \n> change in behaviour should be introduces by this patch alone).\n\ns/\\(introduce\\)s/\\1d/\n\n> diff --git a/archive.c b/archive.c\n> index 9ac455d..e6de039 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -132,7 +132,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n>  \t\terr = write_entry(args, sha1, path.buf, path.len, mode, NULL, 0);\n>  \t\tif (err)\n>  \t\t\treturn err;\n> -\t\treturn READ_TREE_RECURSIVE;\n> +\t\treturn (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);\n\nYou do not need the parentheses around the conditional:\n\n\t$ git grep 'return (.*?' *.c | wc -l\n\t14\n\tgene099@racer:~/git (rebase-i-p)$ git grep 'return [^(]*?' *.c | wc -l\n\t41\n\nNote that the 14 matches include 9 false positives.\n\n> diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c\n> index 5b63e6e..fca4631 100644\n> --- a/builtin-ls-tree.c\n> +++ b/builtin-ls-tree.c\n> @@ -68,13 +68,8 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,\n>  \t\t *\n>  \t\t * Something similar to this incomplete example:\n>  \t\t *\n> -\t\tif (show_subprojects(base, baselen, pathname)) {\n> -\t\t\tstruct child_process ls_tree;\n> -\n> -\t\t\tls_tree.dir = base;\n> -\t\t\tls_tree.argv = ls-tree;\n\nI wondered how that could ever have compiled...\n\nUntil I inspected the file (which is different in junio/next from what you \nbased your patch on; your patch is vs junio/master).\n\n> @@ -131,6 +131,34 @@ int read_tree_recursive(struct tree *tree,\n>  \t\t\tif (retval)\n>  \t\t\t\treturn -1;\n>  \t\t\tcontinue;\n> +\t\t} else if (S_ISGITLINK(entry.mode)) {\n> +\t\t\tint retval;\n> +\t\t\tstruct strbuf path;\n\ns/;/ = STRBUF_INIT;/\n\n> +\t\t\tunsigned int entrylen;\n> +\t\t\tstruct commit *commit;\n> +\n> +\t\t\tentrylen = tree_entry_len(entry.path, entry.sha1);\n> +\t\t\tstrbuf_init(&path, baselen + entrylen + 1);\n> +\t\t\tstrbuf_add(&path, base, baselen);\n> +\t\t\tstrbuf_add(&path, entry.path, entrylen);\n> +\t\t\tstrbuf_addch(&path, '/');\n\nWhy not\n\t\t\tstrbuf_addf(&path, \"%.*s%.*s/\", baselen, base, \n\t\t\t\tentrylen, entry.path);\n\n> +\n> +\t\t\tcommit = lookup_commit(entry.sha1);\n> +\t\t\tif (!commit)\n> +\t\t\t\tdie(\"Commit %s in submodule path %s not found\",\n> +\t\t\t\t    sha1_to_hex(entry.sha1), path.buf);\n> +\n> +\t\t\tif (parse_commit(commit))\n> +\t\t\t\tdie(\"Invalid commit %s in submodule path %s\",\n> +\t\t\t\t    sha1_to_hex(entry.sha1), path.buf);\n> +\n> +\t\t\tretval = read_tree_recursive(commit->tree,\n> +\t\t\t\t\t\t     path.buf, path.len,\n> +\t\t\t\t\t\t     stage, match, fn, context);\n> +\t\t\tstrbuf_release(&path);\n> +\t\t\tif (retval)\n> +\t\t\t\treturn -1;\n> +\t\t\tcontinue;\n\nI'd also place a comment above read_tree_recursive() stating that this \nfunction tries to traverse into submodules when READ_TREE_RECURSIVE is \nreturned for submodule entries, but no attempt is made at including \nalternate object directories.  (And it must be that way: think bare \nrepositories -- they cannot just try to include a subdirectory's \n.git/objects/..)\n\nCiao,\nDscho\n"},{"id":"101826","messageId":"alpine.DEB.1.00.0901251247040.14855@racer","threadId":"17346","inReplyTo":"1232844726-14902-3-git-send-email-hjemli@gmail.com","subject":"Re: [PATCH 2/2] archive.c: add support for --submodules[=(all|checkedout)]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-25T11:57:21Z","receivedAt":"2009-01-25T11:57:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 25 Jan 2009, Lars Hjemli wrote:\n\n> The --submodules option uses the enhanced read_tree_recursive() to\n> enable inclusion of submodules in the generated archive.\n> \n> When invoked with `--submodules=all` all gitlink entries will be\n> traversed, and when invoked with --submodules=checkedout (the default\n> option) only gitlink entries with a git repo (i.e. checked out sub-\n> modules) will be traversed.\n\n                               In bare repositories, this means: none.\n\nMy reasoning for \"*\" instead of \"all\" and \"\" instead for \"checkedout\" was \nthat you could allow \"<name1>,<name2>\" at some stage, where <name> would \nfirst be interpreted as a submodule group, and if that fails, as submodule \nname.\n\nThinking about that more, \"\" seems illogical, that should rather mean \n\"none\", i.e. the same as --no-submodules.  The \"checkedout\" could be \".\" \nthen, perhaps?  As in \"what we have checked out in ./, the current \ndirectory\"?\n\n> When a gitlink has been selected for traversal, it is required that all\n> objects necessary to perform this traversal are available in either the\n> primary odb or through an alternate odb. To this end, git archive will\n> insert the object database of the selected gitlink (when checked out)\n> as an alternate odb, using the new function add_alt_odb().\n\n> And since alternates now can be added after parsing of \n> objects/info/alternates, the error message in link_alt_odb_entry() has \n> been updated to not mention this file.\n\nCould you split that part into its own patch again, please?\n\n> @@ -91,6 +92,70 @@ static void setup_archive_check(struct git_attr_check *check)\n>  \tcheck[1].attr = attr_export_subst;\n>  }\n>  \n> +static int include_repository(const char *path)\n> +{\n> +\tstruct stat st;\n> +\tconst char *tmp;\n> +\n> +\t/* Return early if the path does not exist since it is OK to not\n> +\t * checkout submodules.\n> +\t */\n> +\tif (stat(path, &st) && errno == ENOENT)\n> +\t\treturn 1;\n> +\n> +\ttmp = read_gitfile_gently(path);\n\nThis will leak memory, no?\n\n> +\tif (tmp) {\n> +\t\tpath = tmp;\n> +\t\tif (stat(path, &st))\n> +\t\t\tdie(\"Unable to stat submodule gitdir %s: %s (%d)\",\n> +\t\t\t    path, strerror(errno), errno);\n> +\t}\n> +\n> +\tif (!S_ISDIR(st.st_mode))\n> +\t\tdie(\"Submodule gitdir %s is not a directory\", path);\n> +\n> +\tif (add_alt_odb(mkpath(\"%s/objects\", path)))\n> +\t\tdie(\"submodule odb %s could not be added as an alternate\",\n> +\t\t    path);\n> +\n> +\treturn 0;\n> +}\n\nCiao,\nDscho\n"},{"id":"101830","messageId":"8c5c35580901250430q68a09150x863f15438336a0eb@mail.gmail.com","threadId":"17346","inReplyTo":"alpine.DEB.1.00.0901251225250.14855@racer","subject":"Re: [PATCH 1/2] tree.c: allow read_tree_recursive() to traverse gitlink entries","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2009-01-25T12:30:48Z","receivedAt":"2009-01-25T12:30:48Z","isPatch":true,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"On Sun, Jan 25, 2009 at 12:43, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> On Sun, 25 Jan 2009, Lars Hjemli wrote:\n>\n>> This functionality can be used to allow inter-repository operations, but\n>> since the current users of read_tree_recursive() does not yet support\n>> such operations, they have been modified where necessary to make sure\n>> that they never return READ_TREE_RECURSIVE for gitlink entries (hence no\n>> change in behaviour should be introduces by this patch alone).\n>\n> s/\\(introduce\\)s/\\1d/\n\nThanks\n\n>\n>> diff --git a/archive.c b/archive.c\n>> index 9ac455d..e6de039 100644\n>> --- a/archive.c\n>> +++ b/archive.c\n>> @@ -132,7 +132,7 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n>>               err = write_entry(args, sha1, path.buf, path.len, mode, NULL, 0);\n>>               if (err)\n>>                       return err;\n>> -             return READ_TREE_RECURSIVE;\n>> +             return (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);\n>\n> You do not need the parentheses around the conditional:\n>\n>        $ git grep 'return (.*?' *.c | wc -l\n>        14\n>        gene099@racer:~/git (rebase-i-p)$ git grep 'return [^(]*?' *.c | wc -l\n>        41\n>\n> Note that the 14 matches include 9 false positives.\n\nOk, will fix.\n\n>\n>> diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c\n>> index 5b63e6e..fca4631 100644\n>> --- a/builtin-ls-tree.c\n>> +++ b/builtin-ls-tree.c\n>> @@ -68,13 +68,8 @@ static int show_tree(const unsigned char *sha1, const char *base, int baselen,\n>>                *\n>>                * Something similar to this incomplete example:\n>>                *\n>> -             if (show_subprojects(base, baselen, pathname)) {\n>> -                     struct child_process ls_tree;\n>> -\n>> -                     ls_tree.dir = base;\n>> -                     ls_tree.argv = ls-tree;\n>\n> I wondered how that could ever have compiled...\n>\n> Until I inspected the file (which is different in junio/next from what you\n> based your patch on; your patch is vs junio/master).\n\nYes, sorry for not mentioning that.\n\n\n>\n>> @@ -131,6 +131,34 @@ int read_tree_recursive(struct tree *tree,\n>>                       if (retval)\n>>                               return -1;\n>>                       continue;\n>> +             } else if (S_ISGITLINK(entry.mode)) {\n>> +                     int retval;\n>> +                     struct strbuf path;\n>\n> s/;/ = STRBUF_INIT;/\n\nI skipped the STRBUF_INIT since I used strbuf_init() below, but...\n\n\n>\n>> +                     unsigned int entrylen;\n>> +                     struct commit *commit;\n>> +\n>> +                     entrylen = tree_entry_len(entry.path, entry.sha1);\n>> +                     strbuf_init(&path, baselen + entrylen + 1);\n>> +                     strbuf_add(&path, base, baselen);\n>> +                     strbuf_add(&path, entry.path, entrylen);\n>> +                     strbuf_addch(&path, '/');\n>\n> Why not\n>                        strbuf_addf(&path, \"%.*s%.*s/\", baselen, base,\n>                                entrylen, entry.path);\n\n...with this cute fix the STRBUF_INIT is required. Will fix.\n\nThanks for the review.\n\n--\nlarsh\n"},{"id":"101831","messageId":"8c5c35580901250500s667db3f0j608a30541321ac0a@mail.gmail.com","threadId":"17346","inReplyTo":"alpine.DEB.1.00.0901251247040.14855@racer","subject":"Re: [PATCH 2/2] archive.c: add support for --submodules[=(all|checkedout)]","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2009-01-25T13:00:42Z","receivedAt":"2009-01-25T13:00:42Z","isPatch":true,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"On Sun, Jan 25, 2009 at 12:57, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n> On Sun, 25 Jan 2009, Lars Hjemli wrote:\n>\n>> The --submodules option uses the enhanced read_tree_recursive() to\n>> enable inclusion of submodules in the generated archive.\n>>\n>> When invoked with `--submodules=all` all gitlink entries will be\n>> traversed, and when invoked with --submodules=checkedout (the default\n>> option) only gitlink entries with a git repo (i.e. checked out sub-\n>> modules) will be traversed.\n>\n>                               In bare repositories, this means: none.\n>\n> My reasoning for \"*\" instead of \"all\" and \"\" instead for \"checkedout\" was\n> that you could allow \"<name1>,<name2>\" at some stage, where <name> would\n> first be interpreted as a submodule group, and if that fails, as submodule\n> name.\n>\n> Thinking about that more, \"\" seems illogical, that should rather mean\n> \"none\", i.e. the same as --no-submodules.  The \"checkedout\" could be \".\"\n> then, perhaps?  As in \"what we have checked out in ./, the current\n> directory\"?\n\nYes, I think that makes sense, i.e. '--submodules' will include _all_\nsubmodules (making the option behave identically for bare and non-bare\nrepositories), '--submodules=.' will include checked out submodules\n(making the option a no-op in bare repos, which also makes sense) and\n'--submodules=<name>[,<name>...]' will include the named submodules,\nwhere \"named\" could mean groupname, submodule name or submodule path,\nin that order.\n\nBut then we probably also want some (optional) syntax to specify the\nkind of name, e.g. '--submodules=g:foo,n:bar,p:lib/baz' for group foo,\nname bar and path lib/baz. Agree?\n\n>\n>> When a gitlink has been selected for traversal, it is required that all\n>> objects necessary to perform this traversal are available in either the\n>> primary odb or through an alternate odb. To this end, git archive will\n>> insert the object database of the selected gitlink (when checked out)\n>> as an alternate odb, using the new function add_alt_odb().\n>\n>> And since alternates now can be added after parsing of\n>> objects/info/alternates, the error message in link_alt_odb_entry() has\n>> been updated to not mention this file.\n>\n> Could you split that part into its own patch again, please?\n\nSure.\n\n\n>\n>> @@ -91,6 +92,70 @@ static void setup_archive_check(struct git_attr_check *check)\n>>       check[1].attr = attr_export_subst;\n>>  }\n>>\n>> +static int include_repository(const char *path)\n>> +{\n>> +     struct stat st;\n>> +     const char *tmp;\n>> +\n>> +     /* Return early if the path does not exist since it is OK to not\n>> +      * checkout submodules.\n>> +      */\n>> +     if (stat(path, &st) && errno == ENOENT)\n>> +             return 1;\n>> +\n>> +     tmp = read_gitfile_gently(path);\n>\n> This will leak memory, no?\n\nI don't think so: read_gitfile_gently() returns a value obtained by\ncalling make_absolute_path() which returns a static buffer. Also, the\npath argument to include_repository() is obtained by calling mkpath()\nwhich returns another static buffer so I don't see any malloc()'s\nwhich should be free()'d. Is my code-reading flawed?\n\n--\nlarsh\n"},{"id":"101833","messageId":"alpine.DEB.1.00.0901251452530.14855@racer","threadId":"17346","inReplyTo":"8c5c35580901250500s667db3f0j608a30541321ac0a@mail.gmail.com","subject":"Re: [PATCH 2/2] archive.c: add support for --submodules[=(all|checkedout)]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-25T13:55:47Z","receivedAt":"2009-01-25T13:55:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 25 Jan 2009, Lars Hjemli wrote:\n\n> On Sun, Jan 25, 2009 at 12:57, Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n>\n> > My reasoning for \"*\" instead of \"all\" and \"\" instead for \"checkedout\" \n> > was that you could allow \"<name1>,<name2>\" at some stage, where <name> \n> > would first be interpreted as a submodule group, and if that fails, as \n> > submodule name.\n> >\n> > Thinking about that more, \"\" seems illogical, that should rather mean \n> > \"none\", i.e. the same as --no-submodules.  The \"checkedout\" could be \n> > \".\" then, perhaps?  As in \"what we have checked out in ./, the current \n> > directory\"?\n> \n> Yes, I think that makes sense, i.e. '--submodules' will include _all_ \n> submodules (making the option behave identically for bare and non-bare \n> repositories), '--submodules=.' will include checked out submodules \n> (making the option a no-op in bare repos, which also makes sense) and \n> '--submodules=<name>[,<name>...]' will include the named submodules, \n> where \"named\" could mean groupname, submodule name or submodule path, in \n> that order.\n\nWell, I can live with the default of all submodules, even if I think that \n\"git-submodule.sh\" uses the checked out submodules by default.\n\n> But then we probably also want some (optional) syntax to specify the \n> kind of name, e.g. '--submodules=g:foo,n:bar,p:lib/baz' for group foo, \n> name bar and path lib/baz. Agree?\n\nIMO that is overkill.  Anybody naming a submodule group identically to a \nsubmodule deserves what she gets, anyway.\n\n\n> >> @@ -91,6 +92,70 @@ static void setup_archive_check(struct git_attr_check *check)\n> >>       check[1].attr = attr_export_subst;\n> >>  }\n> >>\n> >> +static int include_repository(const char *path)\n> >> +{\n> >> +     struct stat st;\n> >> +     const char *tmp;\n> >> +\n> >> +     /* Return early if the path does not exist since it is OK to not\n> >> +      * checkout submodules.\n> >> +      */\n> >> +     if (stat(path, &st) && errno == ENOENT)\n> >> +             return 1;\n> >> +\n> >> +     tmp = read_gitfile_gently(path);\n> >\n> > This will leak memory, no?\n> \n> I don't think so: read_gitfile_gently() returns a value obtained by\n> calling make_absolute_path() which returns a static buffer. Also, the\n> path argument to include_repository() is obtained by calling mkpath()\n> which returns another static buffer so I don't see any malloc()'s\n> which should be free()'d. Is my code-reading flawed?\n\nNo, your code reading is good.  And you spared me having to read the code \nmyself ;-)  Now, maybe a code comment is in order, to spare others, too?\n\nCiao,\nDscho\n"},{"id":"101869","messageId":"7veiyrdszf.fsf@gitster.siamese.dyndns.org","threadId":"17346","inReplyTo":"8c5c35580901250018x6827291cj36e6bcb10afa9b27@mail.gmail.com","subject":"Re: [PATCH 0/2] Add submodule-support to git archive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-25T20:35:16Z","receivedAt":"2009-01-25T20:35:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Hjemli <hjemli@gmail.com> writes:\n\n> On Sun, Jan 25, 2009 at 05:53, Nanako Shiraishi <nanako3@lavabit.com> wrote:\n>> What would I do to try this new series? Fork a branch from Junio's master branch,\n>> apply your new patches, and merge the result to Junio's next?\n>\n> Yes, that sounds right (btw: the series is buildt on top of 5dc1308562\n> (Merge branch 'js/patience-diff') and can be pulled from\n> git://hjemli.net/pub/git/git lh/traverse-gitlinks).\n>\n> But before merging with 'next', you'll need to `git revert -m 1 bdf31cbc00`.\n\nYuck, that is too much to ask for regular testers and users.\n\nCould we switch to incremental refinements once a series hits next, pretty\nplease?\n"},{"id":"101894","messageId":"8c5c35580901251512q5058dde3rdfae81979c46c36a@mail.gmail.com","threadId":"17346","inReplyTo":"7veiyrdszf.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 0/2] Add submodule-support to git archive","fromName":"Lars Hjemli","fromEmail":"hjemli@gmail.com","sentAt":"2009-01-25T23:12:25Z","receivedAt":"2009-01-25T23:12:25Z","isPatch":true,"sender":{"key":"hjemli@gmail.com","avatar":null},"body":"On Sun, Jan 25, 2009 at 21:35, Junio C Hamano <gitster@pobox.com> wrote:\n> Lars Hjemli <hjemli@gmail.com> writes:\n>\n>> On Sun, Jan 25, 2009 at 05:53, Nanako Shiraishi <nanako3@lavabit.com> wrote:\n>>> What would I do to try this new series? Fork a branch from Junio's master branch,\n>>> apply your new patches, and merge the result to Junio's next?\n>>\n>> Yes, that sounds right (btw: the series is buildt on top of 5dc1308562\n>> (Merge branch 'js/patience-diff') and can be pulled from\n>> git://hjemli.net/pub/git/git lh/traverse-gitlinks).\n>>\n>> But before merging with 'next', you'll need to `git revert -m 1 bdf31cbc00`.\n>\n> Yuck, that is too much to ask for regular testers and users.\n\nSorry about that.\n\n\n> Could we switch to incremental refinements once a series hits next, pretty\n> please?\n\nThe problem in this particular case is that the design has changed so\nmuch since the first iteration that we're not really talking about\nincremental refinements but rather a different approach to the same\nproblem.\n\nIf you want me to build on top of the series in next anyways, would it\nbe acceptable if the first patch on top of ee306d2d59 reverts the\nprevious attempt? I think the rest of the series will be easier to\nreview that way.\n\n--\nlarsh\n"},{"id":"101900","messageId":"alpine.DEB.1.00.0901260024140.14855@racer","threadId":"17346","inReplyTo":"8c5c35580901251512q5058dde3rdfae81979c46c36a@mail.gmail.com","subject":"Re: [PATCH 0/2] Add submodule-support to git archive","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-01-25T23:25:08Z","receivedAt":"2009-01-25T23:25:08Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 26 Jan 2009, Lars Hjemli wrote:\n\n> On Sun, Jan 25, 2009 at 21:35, Junio C Hamano <gitster@pobox.com> wrote:\n> \n> > Could we switch to incremental refinements once a series hits next, \n> > pretty please?\n> \n> The problem in this particular case is that the design has changed so \n> much since the first iteration that we're not really talking about \n> incremental refinements but rather a different approach to the same \n> problem.\n> \n> If you want me to build on top of the series in next anyways, would it\n> be acceptable if the first patch on top of ee306d2d59 reverts the\n> previous attempt? I think the rest of the series will be easier to\n> review that way.\n\nI'd appreciate that.\n\nSince 'next' will be rewound eventually, that first iteration and the \nrevert will disappear at some stage.\n\nCiao,\nDscho\n"},{"id":"101921","messageId":"7vmydedhkx.fsf@gitster.siamese.dyndns.org","threadId":"17346","inReplyTo":"8c5c35580901251512q5058dde3rdfae81979c46c36a@mail.gmail.com","subject":"Re: [PATCH 0/2] Add submodule-support to git archive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-26T00:41:34Z","receivedAt":"2009-01-26T00:41:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Hjemli <hjemli@gmail.com> writes:\n\n> If you want me to build on top of the series in next anyways, would it\n> be acceptable if the first patch on top of ee306d2d59 reverts the\n> previous attempt? I think the rest of the series will be easier to\n> review that way.\n\nOk, then I'll simply revert and then queue the new ones on top of it.\n\nThanks.\n"}]}