{"thread":{"id":"35406","subject":"[PATCH] submodule recursion in git-archive","startedAt":"2013-11-26T00:04:14Z","lastAt":"2013-12-03T00:03:37Z","messageCount":14,"participants":["Nick Townsend","René Scharfe","Jens Lehmann","Junio C Hamano","Heiko Voigt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"231120","messageId":"2E636B58-47EB-4712-93CA-39E8D1BA3DB9@mac.com","threadId":"35406","inReplyTo":null,"subject":"[PATCH] submodule recursion in git-archive","fromName":"Nick Townsend","fromEmail":"nick.townsend@mac.com","sentAt":"2013-11-26T00:04:14Z","receivedAt":"2013-11-26T00:04:14Z","isPatch":true,"sender":{"key":"nick.townsend@mac.com","avatar":"https://avatars.githubusercontent.com/u/44087510?v=4"},"body":"All,\nMy first git patch - so shout out if I’ve got the etiquette wrong! Or of course if I’ve missed something.\nI googled around looking for solutions to my problem but just came up with a few shell-scripts\nthat didn’t quite get the functionality I needed.\nThe first patch fixes some typos that crept in to existing doc and declarations. It is required\nfor the second which actually implements the changes.\n\nAll comments gratefully received!\n\nRegards\nNick Townsend\n\nSubject: [PATCH 1/2] submodule: add_submodule_odb() usability\n\nAlthough add_submodule_odb() is documented as being\nexternally usable, it is declared static and also\nhas incorrect documentation.\n\nThis commit fixes those and makes no changes to\nexisting code using them. All tests still pass.\n---\n Documentation/technical/api-ref-iteration.txt | 4 ++--\n submodule.c                                   | 2 +-\n submodule.h                                   | 1 +\n 3 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/technical/api-ref-iteration.txt b/Documentation/technical/api-ref-iteration.txt\nindex aa1c50f..cbee624 100644\n--- a/Documentation/technical/api-ref-iteration.txt\n+++ b/Documentation/technical/api-ref-iteration.txt\n@@ -50,10 +50,10 @@ submodules object database. You can do this by a code-snippet like\n this:\n \n \tconst char *path = \"path/to/submodule\"\n-\tif (!add_submodule_odb(path))\n+\tif (add_submodule_odb(path))\n \t\tdie(\"Error submodule '%s' not populated.\", path);\n \n-`add_submodule_odb()` will return an non-zero value on success. If you\n+`add_submodule_odb()` will return a zero value on success. If you\n do not do this you will get an error for each ref that it does not point\n to a valid object.\n \ndiff --git a/submodule.c b/submodule.c\nindex 1905d75..1ea46be 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -143,7 +143,7 @@ void stage_updated_gitmodules(void)\n \t\tdie(_(\"staging updated .gitmodules failed\"));\n }\n \n-static int add_submodule_odb(const char *path)\n+int add_submodule_odb(const char *path)\n {\n \tstruct strbuf objects_directory = STRBUF_INIT;\n \tstruct alternate_object_database *alt_odb;\ndiff --git a/submodule.h b/submodule.h\nindex 7beec48..3e3cdca 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -41,5 +41,6 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam\n \t\tstruct string_list *needs_pushing);\n int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n+int add_submodule_odb(const char *path);\n \n #endif\n-- \n1.8.3.4 (Apple Git-47)\n\nSubject: [PATCH 2/2] archive: allow submodule recursion on git-archive\n\nWhen using git-archive to produce a dump of a\nrepository, the existing code does not recurse\ninto a submodule when it encounters it in the tree\ntraversal. These changes add a command line flag\nthat permits this.\n\nNote that the submodules must be updated in the\nrepository, otherwise this cannot take place.\n\nThe feature is disabled for remote repositories as\nthe git_work_tree fails. This is a possible future\nenhancement.\n\nTwo additional fields are added to archiver_args:\n  * recurse  - a boolean indicator\n  * treepath - the path part of the tree-ish\n               eg. the 'www' in HEAD:www\n\nThe latter is used within the archive writer to\ndetermin the correct path for the submodule .git\nfile.\n\nSigned-off-by: Nick Townsend <nick.townsend@mac.com>\n---\n Documentation/git-archive.txt |  9 +++++++++\n archive.c                     | 38 ++++++++++++++++++++++++++++++++++++--\n archive.h                     |  2 ++\n 3 files changed, 47 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\nindex b97aaab..b4df735 100644\n--- a/Documentation/git-archive.txt\n+++ b/Documentation/git-archive.txt\n@@ -11,6 +11,7 @@ SYNOPSIS\n [verse]\n 'git archive' [--format=<fmt>] [--list] [--prefix=<prefix>/] [<extra>]\n \t      [-o <file> | --output=<file>] [--worktree-attributes]\n+\t      [--recursive|--recurse-submodules]\n \t      [--remote=<repo> [--exec=<git-upload-archive>]] <tree-ish>\n \t      [<path>...]\n \n@@ -51,6 +52,14 @@ OPTIONS\n --prefix=<prefix>/::\n \tPrepend <prefix>/ to each filename in the archive.\n \n+--recursive::\n+--recurse-submodules::\n+\tArchive entries in submodules. Errors occur if the submodules\n+\thave not been initialized and updated.\n+\tRun `git submodule update --init --recursive` immediately after\n+\tthe clone is finished to avoid this.\n+\tThis option is not available with remote repositories.\n+\n -o <file>::\n --output=<file>::\n \tWrite the archive to <file> instead of stdout.\ndiff --git a/archive.c b/archive.c\nindex 346f3b2..f6313c9 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -5,6 +5,7 @@\n #include \"archive.h\"\n #include \"parse-options.h\"\n #include \"unpack-trees.h\"\n+#include \"submodule.h\"\n \n static char const * const archive_usage[] = {\n \tN_(\"git archive [options] <tree-ish> [<path>...]\"),\n@@ -131,13 +132,32 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n \t\targs->convert = ATTR_TRUE(check[1].value);\n \t}\n \n+\tif (S_ISGITLINK(mode) && args->recurse) {\n+\t\tconst char *work_tree = get_git_work_tree();\n+\t\tif (!work_tree) {\n+\t\t\t  die(\"Can't go recursive when no work dir\");\n+\t\t}\n+\t\tstatic struct strbuf dotgit = STRBUF_INIT;\n+\t\tstrbuf_reset(&dotgit);\n+\t\tstrbuf_grow(&dotgit, PATH_MAX);\n+\t\tstrbuf_addstr(&dotgit, work_tree);\n+\t\tstrbuf_addch(&dotgit, '/');\n+\t\tif (args->treepath) {\n+\t\t\t  strbuf_addstr(&dotgit, args->treepath);\n+\t\t\t  strbuf_addch(&dotgit, '/');\n+\t\t}\n+\t\tstrbuf_add(&dotgit, path_without_prefix,strlen(path_without_prefix)-1);\n+\t\tif (add_submodule_odb(dotgit.buf))\n+\t\t\t  die(\"Can't add submodule: %s\", dotgit.buf);\n+\t\tstrbuf_release(&dotgit);\n+\t}\n \tif (S_ISDIR(mode) || S_ISGITLINK(mode)) {\n \t\tif (args->verbose)\n \t\t\tfprintf(stderr, \"%.*s\\n\", (int)path.len, path.buf);\n \t\terr = write_entry(args, sha1, path.buf, path.len, mode);\n \t\tif (err)\n \t\t\treturn err;\n-\t\treturn (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);\n+\t\treturn (S_ISGITLINK(mode) && !args->recurse) ? 0: READ_TREE_RECURSIVE;\n \t}\n \n \tif (args->verbose)\n@@ -256,10 +276,16 @@ static void parse_treeish_arg(const char **argv,\n \tconst struct commit *commit;\n \tunsigned char sha1[20];\n \n+\tconst char *colon = strchr(name, ':');\n+\n+\t/* Store the path on the ref for later (required for --recursive) */\n+\tchar *treepath = NULL;\n+\tif (colon) {\n+\t\ttreepath = strdup(colon+1);\n+\t}\n \t/* Remotes are only allowed to fetch actual refs */\n \tif (remote) {\n \t\tchar *ref = NULL;\n-\t\tconst char *colon = strchr(name, ':');\n \t\tint refnamelen = colon ? colon - name : strlen(name);\n \n \t\tif (!dwim_ref(name, refnamelen, sha1, &ref))\n@@ -296,9 +322,11 @@ static void parse_treeish_arg(const char **argv,\n \t\ttree = parse_tree_indirect(tree_sha1);\n \t}\n \tar_args->tree = tree;\n+\tar_args->treepath = treepath;\n \tar_args->commit_sha1 = commit_sha1;\n \tar_args->commit = commit;\n \tar_args->time = archive_time;\n+\n }\n \n #define OPT__COMPR(s, v, h, p) \\\n@@ -318,6 +346,7 @@ static int parse_archive_args(int argc, const char **argv,\n \tconst char *exec = NULL;\n \tconst char *output = NULL;\n \tint compression_level = -1;\n+\tint recurse = 0;\n \tint verbose = 0;\n \tint i;\n \tint list = 0;\n@@ -331,6 +360,8 @@ static int parse_archive_args(int argc, const char **argv,\n \t\t\tN_(\"write the archive to this file\")),\n \t\tOPT_BOOL(0, \"worktree-attributes\", &worktree_attributes,\n \t\t\tN_(\"read .gitattributes in working directory\")),\n+\t\tOPT_BOOL(0, \"recursive\", &recurse, N_(\"include submodules in archive\")),\n+\t\tOPT_BOOL(0, \"recurse-submodules\", &recurse, N_(\"include submodules in archive\")),\n \t\tOPT__VERBOSE(&verbose, N_(\"report archived files on stderr\")),\n \t\tOPT__COMPR('0', &compression_level, N_(\"store only\"), 0),\n \t\tOPT__COMPR('1', &compression_level, N_(\"compress faster\"), 1),\n@@ -355,6 +386,8 @@ static int parse_archive_args(int argc, const char **argv,\n \n \targc = parse_options(argc, argv, NULL, opts, archive_usage, 0);\n \n+\tif (is_remote && recurse)\n+\t\tdie(\"Cannot include submodules with option --remote\");\n \tif (remote)\n \t\tdie(\"Unexpected option --remote\");\n \tif (exec)\n@@ -393,6 +426,7 @@ static int parse_archive_args(int argc, const char **argv,\n \t\t\t\t\tformat, compression_level);\n \t\t}\n \t}\n+\targs->recurse = recurse;\n \targs->verbose = verbose;\n \targs->base = base;\n \targs->baselen = strlen(base);\ndiff --git a/archive.h b/archive.h\nindex 4a791e1..577238d 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -7,10 +7,12 @@ struct archiver_args {\n \tconst char *base;\n \tsize_t baselen;\n \tstruct tree *tree;\n+\tconst char *treepath;\n \tconst unsigned char *commit_sha1;\n \tconst struct commit *commit;\n \ttime_t time;\n \tstruct pathspec pathspec;\n+\tunsigned int recurse : 1;\n \tunsigned int verbose : 1;\n \tunsigned int worktree_attributes : 1;\n \tunsigned int convert : 1;\n-- \n1.8.3.4 (Apple Git-47)\n"},{"id":"231137","messageId":"5294BB97.7010707@web.de","threadId":"35406","inReplyTo":"2E636B58-47EB-4712-93CA-39E8D1BA3DB9@mac.com","subject":"Re: [PATCH] submodule recursion in git-archive","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2013-11-26T15:17:43Z","receivedAt":"2013-11-26T15:17:43Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 26.11.2013 01:04, schrieb Nick Townsend:\n> My first git patch - so shout out if I’ve got the etiquette wrong! Or\n> of course if I’ve missed something.\n\nThanks for the patches!  Please send only one per message (the second\none as a reply to the first one, or both as replies to a cover letter),\nthough -- that makes commenting on them much easier.\n\nSide note: Documentation/SubmittingPatches doesn't mention that (yet),\nAFAICS.\n\n> Subject: [PATCH 1/2] submodule: add_submodule_odb() usability\n> \n> Although add_submodule_odb() is documented as being\n> externally usable, it is declared static and also\n> has incorrect documentation.\n> \n> This commit fixes those and makes no changes to\n> existing code using them. All tests still pass.\n\nSign-off missing (see Documentation/SubmittingPatches).\n\n> ---\n>  Documentation/technical/api-ref-iteration.txt | 4 ++--\n>  submodule.c                                   | 2 +-\n>  submodule.h                                   | 1 +\n>  3 files changed, 4 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/technical/api-ref-iteration.txt b/Documentation/technical/api-ref-iteration.txt\n> index aa1c50f..cbee624 100644\n> --- a/Documentation/technical/api-ref-iteration.txt\n> +++ b/Documentation/technical/api-ref-iteration.txt\n> @@ -50,10 +50,10 @@ submodules object database. You can do this by a code-snippet like\n>  this:\n>  \n>  \tconst char *path = \"path/to/submodule\"\n> -\tif (!add_submodule_odb(path))\n> +\tif (add_submodule_odb(path))\n>  \t\tdie(\"Error submodule '%s' not populated.\", path);\n>  \n> -`add_submodule_odb()` will return an non-zero value on success. If you\n> +`add_submodule_odb()` will return a zero value on success. If you\n\n\"return zero on success\" instead?\n\n>  do not do this you will get an error for each ref that it does not point\n>  to a valid object.\n>  \n> diff --git a/submodule.c b/submodule.c\n> index 1905d75..1ea46be 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -143,7 +143,7 @@ void stage_updated_gitmodules(void)\n>  \t\tdie(_(\"staging updated .gitmodules failed\"));\n>  }\n>  \n> -static int add_submodule_odb(const char *path)\n> +int add_submodule_odb(const char *path)\n>  {\n>  \tstruct strbuf objects_directory = STRBUF_INIT;\n>  \tstruct alternate_object_database *alt_odb;\n> diff --git a/submodule.h b/submodule.h\n> index 7beec48..3e3cdca 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -41,5 +41,6 @@ int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_nam\n>  \t\tstruct string_list *needs_pushing);\n>  int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n>  void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n> +int add_submodule_odb(const char *path);\n>  \n>  #endif\n\n> Subject: [PATCH 2/2] archive: allow submodule recursion on git-archive\n> \n> When using git-archive to produce a dump of a\n> repository, the existing code does not recurse\n> into a submodule when it encounters it in the tree\n> traversal. These changes add a command line flag\n> that permits this.\n> \n> Note that the submodules must be updated in the\n> repository, otherwise this cannot take place.\n> \n> The feature is disabled for remote repositories as\n> the git_work_tree fails. This is a possible future\n> enhancement.\n\nHmm, curious.  Why does it fail?  I guess that happens with bare\nrepositories, only, right?  (Which are the most likely kind of remote\nrepos to encounter, of course.)\n\n> Two additional fields are added to archiver_args:\n>   * recurse  - a boolean indicator\n>   * treepath - the path part of the tree-ish\n>                eg. the 'www' in HEAD:www\n> \n> The latter is used within the archive writer to\n> determin the correct path for the submodule .git\n> file.\n> \n> Signed-off-by: Nick Townsend <nick.townsend@mac.com>\n> ---\n>  Documentation/git-archive.txt |  9 +++++++++\n>  archive.c                     | 38 ++++++++++++++++++++++++++++++++++++--\n>  archive.h                     |  2 ++\n>  3 files changed, 47 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\n> index b97aaab..b4df735 100644\n> --- a/Documentation/git-archive.txt\n> +++ b/Documentation/git-archive.txt\n> @@ -11,6 +11,7 @@ SYNOPSIS\n>  [verse]\n>  'git archive' [--format=<fmt>] [--list] [--prefix=<prefix>/] [<extra>]\n>  \t      [-o <file> | --output=<file>] [--worktree-attributes]\n> +\t      [--recursive|--recurse-submodules]\n\nI'd expect git archive --recurse to add subdirectories and their\ncontents, which it does right now, and --no-recurse to only archive the\nspecified objects, which is not implemented.  IAW: I wouldn't normally\nassociate an option with that name with submodules.  Would\n--recurse-submodules alone suffice?\n\nSide note: With only one of the options defined you could shorten them\non the command line to e.g. --rec; with both you'd need to type at least\n--recursi or --recurse to disambiguate -- even though they ultimately do\nthe same.\n\n>  \t      [--remote=<repo> [--exec=<git-upload-archive>]] <tree-ish>\n>  \t      [<path>...]\n>  \n> @@ -51,6 +52,14 @@ OPTIONS\n>  --prefix=<prefix>/::\n>  \tPrepend <prefix>/ to each filename in the archive.\n>  \n> +--recursive::\n> +--recurse-submodules::\n> +\tArchive entries in submodules. Errors occur if the submodules\n> +\thave not been initialized and updated.\n> +\tRun `git submodule update --init --recursive` immediately after\n> +\tthe clone is finished to avoid this.\n> +\tThis option is not available with remote repositories.\n> +\n>  -o <file>::\n>  --output=<file>::\n>  \tWrite the archive to <file> instead of stdout.\n> diff --git a/archive.c b/archive.c\n> index 346f3b2..f6313c9 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -5,6 +5,7 @@\n>  #include \"archive.h\"\n>  #include \"parse-options.h\"\n>  #include \"unpack-trees.h\"\n> +#include \"submodule.h\"\n>  \n>  static char const * const archive_usage[] = {\n>  \tN_(\"git archive [options] <tree-ish> [<path>...]\"),\n> @@ -131,13 +132,32 @@ static int write_archive_entry(const unsigned char *sha1, const char *base,\n>  \t\targs->convert = ATTR_TRUE(check[1].value);\n>  \t}\n>  \n> +\tif (S_ISGITLINK(mode) && args->recurse) {\n> +\t\tconst char *work_tree = get_git_work_tree();\n> +\t\tif (!work_tree) {\n> +\t\t\t  die(\"Can't go recursive when no work dir\");\n> +\t\t}\n\nStyle nit: No curly braces around single-line statements, please.\n\nPerhaps mention submodules in the error message?\n\nIt would be nicer to get_git_work_tree right after the parameters have\nbeen parsed and before any archive contents have been written and error\nout early.\n\n> +\t\tstatic struct strbuf dotgit = STRBUF_INIT;\n\nWe avoid declarations after statements because older compilers don't\nsupport it.\n\nYou release the memory at the end of this block; that means there's no\nadvantage in making this strbuf static.  Allocating and freeing the\nmemory for the path of each submodule shouldn't cause any performance\nissues, so please just drop static from the declaration.\n\n> +\t\tstrbuf_reset(&dotgit);\n\n... and then you don't need to reset anymore.\n\n> +\t\tstrbuf_grow(&dotgit, PATH_MAX);\n\nI'd drop that as well; the number of submodules should be low enough\nthat the possibly avoided reallocations by giving this hint shouldn't be\nnoticeable.\n\n> +\t\tstrbuf_addstr(&dotgit, work_tree);\n> +\t\tstrbuf_addch(&dotgit, '/');\n> +\t\tif (args->treepath) {\n> +\t\t\t  strbuf_addstr(&dotgit, args->treepath);\n> +\t\t\t  strbuf_addch(&dotgit, '/');\n> +\t\t}\n> +\t\tstrbuf_add(&dotgit, path_without_prefix,strlen(path_without_prefix)-1);\n> +\t\tif (add_submodule_odb(dotgit.buf))\n> +\t\t\t  die(\"Can't add submodule: %s\", dotgit.buf);\n\nHmm, I wonder if we can traverse the tree and load all submodule object\ndatabases before traversing it again to actually write file contents.\nThat would spare the user from getting half of an archive together with\nthat error message.\n\n> +\t\tstrbuf_release(&dotgit);\n> +\t}\n>  \tif (S_ISDIR(mode) || S_ISGITLINK(mode)) {\n>  \t\tif (args->verbose)\n>  \t\t\tfprintf(stderr, \"%.*s\\n\", (int)path.len, path.buf);\n>  \t\terr = write_entry(args, sha1, path.buf, path.len, mode);\n>  \t\tif (err)\n>  \t\t\treturn err;\n> -\t\treturn (S_ISDIR(mode) ? READ_TREE_RECURSIVE : 0);\n> +\t\treturn (S_ISGITLINK(mode) && !args->recurse) ? 0: READ_TREE_RECURSIVE;\n\nThis line is longer than 80 characters, which we tend to avoid.  How\nabout this?\n\n\t\tif (ISGITLINK(mode) && !args->recurse)\n\t\t\treturn 0;\n\t\treturn READ_TREE_RECURSIVE;\n\n>  \t}\n>  \n>  \tif (args->verbose)\n> @@ -256,10 +276,16 @@ static void parse_treeish_arg(const char **argv,\n>  \tconst struct commit *commit;\n>  \tunsigned char sha1[20];\n>  \n> +\tconst char *colon = strchr(name, ':');\n> +\n> +\t/* Store the path on the ref for later (required for --recursive) */\n> +\tchar *treepath = NULL;\n> +\tif (colon) {\n> +\t\ttreepath = strdup(colon+1);\n> +\t}\n\nStyle: No curly braces, space around operators, use wrapper functions\nfor simple memory error handling:\n\n\tif (colon)\n\t\ttreepath = xstrdup(colon + 1);\n\n>  \t/* Remotes are only allowed to fetch actual refs */\n>  \tif (remote) {\n>  \t\tchar *ref = NULL;\n> -\t\tconst char *colon = strchr(name, ':');\n>  \t\tint refnamelen = colon ? colon - name : strlen(name);\n>  \n>  \t\tif (!dwim_ref(name, refnamelen, sha1, &ref))\n> @@ -296,9 +322,11 @@ static void parse_treeish_arg(const char **argv,\n>  \t\ttree = parse_tree_indirect(tree_sha1);\n>  \t}\n>  \tar_args->tree = tree;\n> +\tar_args->treepath = treepath;\n>  \tar_args->commit_sha1 = commit_sha1;\n>  \tar_args->commit = commit;\n>  \tar_args->time = archive_time;\n> +\n>  }\n\nPlease don't add empty lines before the end of a block.\n\n>  #define OPT__COMPR(s, v, h, p) \\\n> @@ -318,6 +346,7 @@ static int parse_archive_args(int argc, const char **argv,\n>  \tconst char *exec = NULL;\n>  \tconst char *output = NULL;\n>  \tint compression_level = -1;\n> +\tint recurse = 0;\n>  \tint verbose = 0;\n>  \tint i;\n>  \tint list = 0;\n> @@ -331,6 +360,8 @@ static int parse_archive_args(int argc, const char **argv,\n>  \t\t\tN_(\"write the archive to this file\")),\n>  \t\tOPT_BOOL(0, \"worktree-attributes\", &worktree_attributes,\n>  \t\t\tN_(\"read .gitattributes in working directory\")),\n> +\t\tOPT_BOOL(0, \"recursive\", &recurse, N_(\"include submodules in archive\")),\n> +\t\tOPT_BOOL(0, \"recurse-submodules\", &recurse, N_(\"include submodules in archive\")),\n>  \t\tOPT__VERBOSE(&verbose, N_(\"report archived files on stderr\")),\n>  \t\tOPT__COMPR('0', &compression_level, N_(\"store only\"), 0),\n>  \t\tOPT__COMPR('1', &compression_level, N_(\"compress faster\"), 1),\n> @@ -355,6 +386,8 @@ static int parse_archive_args(int argc, const char **argv,\n>  \n>  \targc = parse_options(argc, argv, NULL, opts, archive_usage, 0);\n>  \n> +\tif (is_remote && recurse)\n> +\t\tdie(\"Cannot include submodules with option --remote\");\n>  \tif (remote)\n>  \t\tdie(\"Unexpected option --remote\");\n>  \tif (exec)\n> @@ -393,6 +426,7 @@ static int parse_archive_args(int argc, const char **argv,\n>  \t\t\t\t\tformat, compression_level);\n>  \t\t}\n>  \t}\n> +\targs->recurse = recurse;\n>  \targs->verbose = verbose;\n>  \targs->base = base;\n>  \targs->baselen = strlen(base);\n> diff --git a/archive.h b/archive.h\n> index 4a791e1..577238d 100644\n> --- a/archive.h\n> +++ b/archive.h\n> @@ -7,10 +7,12 @@ struct archiver_args {\n>  \tconst char *base;\n>  \tsize_t baselen;\n>  \tstruct tree *tree;\n> +\tconst char *treepath;\n>  \tconst unsigned char *commit_sha1;\n>  \tconst struct commit *commit;\n>  \ttime_t time;\n>  \tstruct pathspec pathspec;\n> +\tunsigned int recurse : 1;\n\nName it recurse_submodules?\n\n>  \tunsigned int verbose : 1;\n>  \tunsigned int worktree_attributes : 1;\n>  \tunsigned int convert : 1;\n> -- 1.8.3.4 (Apple Git-47) \n\nA test script (t/t5005-archive-submodules.sh?) would be nice which\nexercises the new option.\n\nRené\n"},{"id":"231140","messageId":"5294EF14.7000204@web.de","threadId":"35406","inReplyTo":"5294BB97.7010707@web.de","subject":"Re: [PATCH] submodule recursion in git-archive","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-11-26T18:57:24Z","receivedAt":"2013-11-26T18:57:24Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 26.11.2013 16:17, schrieb René Scharfe:\n> Am 26.11.2013 01:04, schrieb Nick Townsend:\n>> diff --git a/Documentation/git-archive.txt b/Documentation/git-archive.txt\n>> index b97aaab..b4df735 100644\n>> --- a/Documentation/git-archive.txt\n>> +++ b/Documentation/git-archive.txt\n>> @@ -11,6 +11,7 @@ SYNOPSIS\n>>  [verse]\n>>  'git archive' [--format=<fmt>] [--list] [--prefix=<prefix>/] [<extra>]\n>>  \t      [-o <file> | --output=<file>] [--worktree-attributes]\n>> +\t      [--recursive|--recurse-submodules]\n> \n> I'd expect git archive --recurse to add subdirectories and their\n> contents, which it does right now, and --no-recurse to only archive the\n> specified objects, which is not implemented.  IAW: I wouldn't normally\n> associate an option with that name with submodules.  Would\n> --recurse-submodules alone suffice?\n\nIt should. All new code recursing into submodules should not use\n--recursive but always --recurse-submodules, as --recursive means\ndifferent things for different commands (the only exception being\n\"git submodule\", as --recursive is obvious here, and \"git clone\"\nfor backward compatibility reasons).\n\nBut I really like what these patches are aiming at.\n"},{"id":"231148","messageId":"xmqqmwkqvmck.fsf@gitster.dls.corp.google.com","threadId":"35406","inReplyTo":"5294BB97.7010707@web.de","subject":"Re: [PATCH] submodule recursion in git-archive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-26T22:18:03Z","receivedAt":"2013-11-26T22:18:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Thanks for the patches!  Please send only one per message (the second\n> one as a reply to the first one, or both as replies to a cover letter),\n> though -- that makes commenting on them much easier.\n>\n> Side note: Documentation/SubmittingPatches doesn't mention that (yet),\n> AFAICS.\n\nOK, how about doing this then?\n\n Documentation/SubmittingPatches | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex 7055576..304b3c0 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -140,7 +140,12 @@ comment on the changes you are submitting.  It is important for\n a developer to be able to \"quote\" your changes, using standard\n e-mail tools, so that they may comment on specific portions of\n your code.  For this reason, all patches should be submitted\n-\"inline\".  If your log message (including your name on the\n+\"inline\".  A patch series that consists of N commits is sent as N\n+separate e-mail messages, or a cover letter message (see below) with\n+N separate e-mail messages, each being a response to the cover\n+letter.\n+\n+If your log message (including your name on the\n Signed-off-by line) is not writable in ASCII, make sure that\n you send off a message in the correct encoding.\n \n\n>> The feature is disabled for remote repositories as\n>> the git_work_tree fails. This is a possible future\n>> enhancement.\n>\n> Hmm, curious.  Why does it fail?  I guess that happens with bare\n> repositories, only, right?  (Which are the most likely kind of remote\n> repos to encounter, of course.)\n\nYeah, I do not think of a reason why it should fail in a bare\nrepository, either. \"git archive\" is about writing out the contents\nof an already recorded tree, so there shouldn't be a reason to even\ncall get_git_work_tree() in the first place.\n\nEven if the code is run inside a repository with a working tree,\nwhen producing a tarball out of an ancient commit that had a\nsubmodule not at its current location, --recurse-submodules option\nshould do the right thing, so asking for working tree location of\nthat submodule to find its repository is wrong, I think.  It may\nhappen to find one if the archived revision is close enough to what\nis currently checked out, but that may not necessarily be the case.\n\nAt that point when the code discovers an S_ISGITLINK entry, it\nshould have both a pathname to the submodule relative to the\ntoplevel and the commit object name bound to that submodule\nlocation.  What it should do, when it does not find the repository\nat the given path (maybe because there is no working tree, or the\nsudmodule directory has moved over time) is roughly:\n\n - Read from .gitmodules at the top-level from the tree it is\n   creating the tarball out of;\n\n - Find \"submodule.$name.path\" entry that records that path to the\n   submodule; and then\n\n - Using that $name, find the stashed-away location of the submodule\n   repository in $GIT_DIR/modules/$name.\n\nor something like that.\n\nThis is a related tangent, but when used in a repository that people\noften use as their remote, the repository discovery may have to\ninteract with the relative URL.  People often ship .gitmodules with\n\n\t[submodule \"bar\"]\n        \tURL = ../bar.git\n\t\tpath = barDir\n\nfor a top-level project \"foo\" that can be cloned thusly:\n\n\tgit clone git://site.xz/foo.git\n\nand host bar.git to be clonable with\n\n\tgit clone git://site.xz/bar.git barDir/\n\ninside the working tree of the foo project.  In such a case, when\n\"archive --recurse-submodules\" is running, it would find the\nrepository for the \"bar\" submodule at \"../bar.git\", I would think.\n\nSo this part needs a bit more thought, I am afraid.\n\n>>  'git archive' [--format=<fmt>] [--list] [--prefix=<prefix>/] [<extra>]\n>>  \t      [-o <file> | --output=<file>] [--worktree-attributes]\n>> +\t      [--recursive|--recurse-submodules]\n>\n> I'd expect git archive --recurse to add subdirectories and their\n> contents, which it does right now, and --no-recurse to only archive the\n> specified objects, which is not implemented.  IAW: I wouldn't normally\n> associate an option with that name with submodules.  Would\n> --recurse-submodules alone suffice?\n\nJens already commented on this, and I agree that --recursive should\nbe dropped from this patch.\n"},{"id":"231152","messageId":"20131126223858.GA4774@sandbox-ub","threadId":"35406","inReplyTo":"5294BB97.7010707@web.de","subject":"Re: Re: [PATCH] submodule recursion in git-archive","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-26T22:38:58Z","receivedAt":"2013-11-26T22:38:58Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"Hi,\n\nI like where this is going.\n\nOn Tue, Nov 26, 2013 at 04:17:43PM +0100, René Scharfe wrote:\n> Am 26.11.2013 01:04, schrieb Nick Townsend:\n> > +\t\tstrbuf_addstr(&dotgit, work_tree);\n> > +\t\tstrbuf_addch(&dotgit, '/');\n> > +\t\tif (args->treepath) {\n> > +\t\t\t  strbuf_addstr(&dotgit, args->treepath);\n> > +\t\t\t  strbuf_addch(&dotgit, '/');\n> > +\t\t}\n> > +\t\tstrbuf_add(&dotgit, path_without_prefix,strlen(path_without_prefix)-1);\n> > +\t\tif (add_submodule_odb(dotgit.buf))\n> > +\t\t\t  die(\"Can't add submodule: %s\", dotgit.buf);\n> \n> Hmm, I wonder if we can traverse the tree and load all submodule object\n> databases before traversing it again to actually write file contents.\n> That would spare the user from getting half of an archive together with\n> that error message.\n\nI am not sure whether we should die here. What about submodules that\nhave not been initialized and or cloned? I think that is a quite regular\nuse case for example for libraries that not everyone needs or big media\nsubmodules which only the design team uses. How about skipping them (maybe\nissuing a warning) by returning 0 here and proceeding?\n\nCheers Heiko\n"},{"id":"231153","messageId":"52953CB7.8020300@web.de","threadId":"35406","inReplyTo":"xmqqmwkqvmck.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] submodule recursion in git-archive","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2013-11-27T00:28:39Z","receivedAt":"2013-11-27T00:28:39Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 26.11.2013 23:18, schrieb Junio C Hamano:\n> René Scharfe <l.s.r@web.de> writes:\n> \n>> Thanks for the patches!  Please send only one per message (the second\n>> one as a reply to the first one, or both as replies to a cover letter),\n>> though -- that makes commenting on them much easier.\n>>\n>> Side note: Documentation/SubmittingPatches doesn't mention that (yet),\n>> AFAICS.\n> \n> OK, how about doing this then?\n> \n>  Documentation/SubmittingPatches | 7 ++++++-\n>  1 file changed, 6 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\n> index 7055576..304b3c0 100644\n> --- a/Documentation/SubmittingPatches\n> +++ b/Documentation/SubmittingPatches\n> @@ -140,7 +140,12 @@ comment on the changes you are submitting.  It is important for\n>  a developer to be able to \"quote\" your changes, using standard\n>  e-mail tools, so that they may comment on specific portions of\n>  your code.  For this reason, all patches should be submitted\n> -\"inline\".  If your log message (including your name on the\n> +\"inline\".  A patch series that consists of N commits is sent as N\n> +separate e-mail messages, or a cover letter message (see below) with\n> +N separate e-mail messages, each being a response to the cover\n> +letter.\n> +\n> +If your log message (including your name on the\n>  Signed-off-by line) is not writable in ASCII, make sure that\n>  you send off a message in the correct encoding.\n\nOK, but the repetition of \"cover letter\" and \"e-mail messages\"\nirritates me slightly for some reason.  What about the following?\n\n-- >8 --\nSubject: [PATCH] SubmittingPatches: document how to handle multiple patches\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n Documentation/SubmittingPatches |   11 +++++++++--\n 1 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\nindex 7055576..e6d46ed 100644\n--- a/Documentation/SubmittingPatches\n+++ b/Documentation/SubmittingPatches\n@@ -139,8 +139,15 @@ People on the Git mailing list need to be able to read and\n comment on the changes you are submitting.  It is important for\n a developer to be able to \"quote\" your changes, using standard\n e-mail tools, so that they may comment on specific portions of\n-your code.  For this reason, all patches should be submitted\n-\"inline\".  If your log message (including your name on the\n+your code.  For this reason, each patch should be submitted\n+\"inline\" in a separate message.\n+\n+Multiple related patches should be grouped into their own e-mail\n+thread to help readers find all parts of the series.  To that end,\n+send them as replies to either an additional \"cover letter\" message\n+(see below), the first patch, or the respective preceding patch.\n+\n+If your log message (including your name on the\n Signed-off-by line) is not writable in ASCII, make sure that\n you send off a message in the correct encoding.\n \n-- \n1.7.8\n"},{"id":"231156","messageId":"FE55CF9D-FE21-4DCA-A819-0B3E6D378C57@mac.com","threadId":"35406","inReplyTo":"52953CB7.8020300@web.de","subject":"Re: [PATCH] submodule recursion in git-archive","fromName":"Nick Townsend","fromEmail":"nick.townsend@mac.com","sentAt":"2013-11-27T03:28:45Z","receivedAt":"2013-11-27T03:28:45Z","isPatch":true,"sender":{"key":"nick.townsend@mac.com","avatar":"https://avatars.githubusercontent.com/u/44087510?v=4"},"body":"\nOn 26 Nov 2013, at 16:28, René Scharfe <l.s.r@web.de> wrote:\n\n> Am 26.11.2013 23:18, schrieb Junio C Hamano:\n>> René Scharfe <l.s.r@web.de> writes:\n>> \n>>> Thanks for the patches!  Please send only one per message (the second\n>>> one as a reply to the first one, or both as replies to a cover letter),\n>>> though -- that makes commenting on them much easier.\n>>> \n>>> Side note: Documentation/SubmittingPatches doesn't mention that (yet),\n>>> AFAICS.\n>> \n>> OK, how about doing this then?\n>> \n>> Documentation/SubmittingPatches | 7 ++++++-\n>> 1 file changed, 6 insertions(+), 1 deletion(-)\n>> \n>> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\n>> index 7055576..304b3c0 100644\n>> --- a/Documentation/SubmittingPatches\n>> +++ b/Documentation/SubmittingPatches\n>> @@ -140,7 +140,12 @@ comment on the changes you are submitting.  It is important for\n>> a developer to be able to \"quote\" your changes, using standard\n>> e-mail tools, so that they may comment on specific portions of\n>> your code.  For this reason, all patches should be submitted\n>> -\"inline\".  If your log message (including your name on the\n>> +\"inline\".  A patch series that consists of N commits is sent as N\n>> +separate e-mail messages, or a cover letter message (see below) with\n>> +N separate e-mail messages, each being a response to the cover\n>> +letter.\n>> +\n>> +If your log message (including your name on the\n>> Signed-off-by line) is not writable in ASCII, make sure that\n>> you send off a message in the correct encoding.\n> \n> OK, but the repetition of \"cover letter\" and \"e-mail messages\"\n> irritates me slightly for some reason.  What about the following?\n> \n> -- >8 --\n> Subject: [PATCH] SubmittingPatches: document how to handle multiple patches\n> \n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n> Documentation/SubmittingPatches |   11 +++++++++--\n> 1 files changed, 9 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\n> index 7055576..e6d46ed 100644\n> --- a/Documentation/SubmittingPatches\n> +++ b/Documentation/SubmittingPatches\n> @@ -139,8 +139,15 @@ People on the Git mailing list need to be able to read and\n> comment on the changes you are submitting.  It is important for\n> a developer to be able to \"quote\" your changes, using standard\n> e-mail tools, so that they may comment on specific portions of\n> -your code.  For this reason, all patches should be submitted\n> -\"inline\".  If your log message (including your name on the\n> +your code.  For this reason, each patch should be submitted\n> +\"inline\" in a separate message.\n> +\n> +Multiple related patches should be grouped into their own e-mail\n> +thread to help readers find all parts of the series.  To that end,\n> +send them as replies to either an additional \"cover letter\" message\n> +(see below), the first patch, or the respective preceding patch.\n> +\n> +If your log message (including your name on the\n> Signed-off-by line) is not writable in ASCII, make sure that\n> you send off a message in the correct encoding.\n> \n> -- \n> 1.7.8\n> \n> \nThat seems clear to me.\nAt any rate I’m going to rework this based on the collective input and will submit them again.\nPlease check my other replies as there are some discussion points!\n\nNick"},{"id":"231157","messageId":"8C8E104C-D88D-47C9-A796-6634BABFAB3E@mac.com","threadId":"35406","inReplyTo":"20131126223858.GA4774@sandbox-ub","subject":"Re: [PATCH] submodule recursion in git-archive","fromName":"Nick Townsend","fromEmail":"nick.townsend@mac.com","sentAt":"2013-11-27T03:33:45Z","receivedAt":"2013-11-27T03:33:45Z","isPatch":true,"sender":{"key":"nick.townsend@mac.com","avatar":"https://avatars.githubusercontent.com/u/44087510?v=4"},"body":"\nOn 26 Nov 2013, at 14:38, Heiko Voigt <hvoigt@hvoigt.net> wrote:\n\n> Hi,\n> \n> I like where this is going.\n> \n> On Tue, Nov 26, 2013 at 04:17:43PM +0100, René Scharfe wrote:\n>> Am 26.11.2013 01:04, schrieb Nick Townsend:\n>>> +\t\tstrbuf_addstr(&dotgit, work_tree);\n>>> +\t\tstrbuf_addch(&dotgit, '/');\n>>> +\t\tif (args->treepath) {\n>>> +\t\t\t  strbuf_addstr(&dotgit, args->treepath);\n>>> +\t\t\t  strbuf_addch(&dotgit, '/');\n>>> +\t\t}\n>>> +\t\tstrbuf_add(&dotgit, path_without_prefix,strlen(path_without_prefix)-1);\n>>> +\t\tif (add_submodule_odb(dotgit.buf))\n>>> +\t\t\t  die(\"Can't add submodule: %s\", dotgit.buf);\n>> \n>> Hmm, I wonder if we can traverse the tree and load all submodule object\n>> databases before traversing it again to actually write file contents.\n>> That would spare the user from getting half of an archive together with\n>> that error message.\n> \n> I am not sure whether we should die here. What about submodules that\n> have not been initialized and or cloned? I think that is a quite regular\n> use case for example for libraries that not everyone needs or big media\n> submodules which only the design team uses. How about skipping them (maybe\n> issuing a warning) by returning 0 here and proceeding?\n> \n> Cheers Heiko\n\nI agree that issuing a warning and continuing is best. If the submodule hasn’t been setup\nthen we should respect that and keep the current behaviour (just archive the directory entry).\nThere is some further debate to be had about the extent to which this should work with\nun-initialized submodules which I’ll discuss in other replies.\n\nThanks\nNick"},{"id":"231158","messageId":"9AB10474-6DEF-4FFD-B6B3-ED2AB21424AC@mac.com","threadId":"35406","inReplyTo":"xmqqmwkqvmck.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] submodule recursion in git-archive","fromName":"Nick Townsend","fromEmail":"nick.townsend@mac.com","sentAt":"2013-11-27T03:55:06Z","receivedAt":"2013-11-27T03:55:06Z","isPatch":true,"sender":{"key":"nick.townsend@mac.com","avatar":"https://avatars.githubusercontent.com/u/44087510?v=4"},"body":"\nOn 26 Nov 2013, at 14:18, Junio C Hamano <gitster@pobox.com> wrote:\n\n> René Scharfe <l.s.r@web.de> writes:\n> \n>> Thanks for the patches!  Please send only one per message (the second\n>> one as a reply to the first one, or both as replies to a cover letter),\n>> though -- that makes commenting on them much easier.\n>> \n>> Side note: Documentation/SubmittingPatches doesn't mention that (yet),\n>> AFAICS.\n> \n> OK, how about doing this then?\n> \n> Documentation/SubmittingPatches | 7 ++++++-\n> 1 file changed, 6 insertions(+), 1 deletion(-)\n> \n> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\n> index 7055576..304b3c0 100644\n> --- a/Documentation/SubmittingPatches\n> +++ b/Documentation/SubmittingPatches\n> @@ -140,7 +140,12 @@ comment on the changes you are submitting.  It is important for\n> a developer to be able to \"quote\" your changes, using standard\n> e-mail tools, so that they may comment on specific portions of\n> your code.  For this reason, all patches should be submitted\n> -\"inline\".  If your log message (including your name on the\n> +\"inline\".  A patch series that consists of N commits is sent as N\n> +separate e-mail messages, or a cover letter message (see below) with\n> +N separate e-mail messages, each being a response to the cover\n> +letter.\n> +\n> +If your log message (including your name on the\n> Signed-off-by line) is not writable in ASCII, make sure that\n> you send off a message in the correct encoding.\n> \n> \n>>> The feature is disabled for remote repositories as\n>>> the git_work_tree fails. This is a possible future\n>>> enhancement.\n>> \n>> Hmm, curious.  Why does it fail?  I guess that happens with bare\n>> repositories, only, right?  (Which are the most likely kind of remote\n>> repos to encounter, of course.)\n> \n> Yeah, I do not think of a reason why it should fail in a bare\n> repository, either. \"git archive\" is about writing out the contents\n> of an already recorded tree, so there shouldn't be a reason to even\n> call get_git_work_tree() in the first place.\n> \nSee below for a discussion of why I use the .git file in the work tree to \nload the objects for the submodule. I also thought it should work in a\nremote repository - but I ran it on a properly initialized remote repository and\nit failed. Since I didn’t need it for my immediate use-case I just decided to disable \nit with an error. I can look into this further, but we must decide about the question \nbelow first…\n\n> Even if the code is run inside a repository with a working tree,\n> when producing a tarball out of an ancient commit that had a\n> submodule not at its current location, --recurse-submodules option\n> should do the right thing, so asking for working tree location of\n> that submodule to find its repository is wrong, I think.  It may\n> happen to find one if the archived revision is close enough to what\n> is currently checked out, but that may not necessarily be the case.\n> \n> At that point when the code discovers an S_ISGITLINK entry, it\n> should have both a pathname to the submodule relative to the\n> toplevel and the commit object name bound to that submodule\n> location.  What it should do, when it does not find the repository\n> at the given path (maybe because there is no working tree, or the\n> sudmodule directory has moved over time) is roughly:\n> \n> - Read from .gitmodules at the top-level from the tree it is\n>   creating the tarball out of;\n> \n> - Find \"submodule.$name.path\" entry that records that path to the\n>   submodule; and then\n> \n> - Using that $name, find the stashed-away location of the submodule\n>   repository in $GIT_DIR/modules/$name.\n> \n> or something like that.\n> \n> This is a related tangent, but when used in a repository that people\n> often use as their remote, the repository discovery may have to\n> interact with the relative URL.  People often ship .gitmodules with\n> \n> \t[submodule \"bar\"]\n>        \tURL = ../bar.git\n> \t\tpath = barDir\n> \n> for a top-level project \"foo\" that can be cloned thusly:\n> \n> \tgit clone git://site.xz/foo.git\n> \n> and host bar.git to be clonable with\n> \n> \tgit clone git://site.xz/bar.git barDir/\n> \n> inside the working tree of the foo project.  In such a case, when\n> \"archive --recurse-submodules\" is running, it would find the\n> repository for the \"bar\" submodule at \"../bar.git\", I would think.\n> \n> So this part needs a bit more thought, I am afraid.\n\nI see that there is a lot of potential complexity around setting up a submodule:\n* The .gitmodules file can be dirty (easy to flag, but should we allow archive to proceed?)\n* Users can mess with settings both prior to git submodule init and before git submodule update.\n* What if it’s a raw clone and the user manually changes things between init and update?\n* I’m not a git-internals expert but looking through the code I see that you can add additional object\ndirectories and change paths as you show above.\n\nFor those reasons I deliberately decided not to reproduce the above logic all by myself.\nOn the other hand, what it *did* seem to me is that once you have the .git file\nthen you know you’ve got all that covered. So I just used that. This restricts the function to\nworking only on a properly setup repository - but that is my use case!\n\nIf you think that doing this more extensive setup is even *viable* given the space between\ninit and update then I”m happy to try it. I didn’t want to start off on a fools errand.\n\n> \n>>> 'git archive' [--format=<fmt>] [--list] [--prefix=<prefix>/] [<extra>]\n>>> \t      [-o <file> | --output=<file>] [--worktree-attributes]\n>>> +\t      [--recursive|--recurse-submodules]\n>> \n>> I'd expect git archive --recurse to add subdirectories and their\n>> contents, which it does right now, and --no-recurse to only archive the\n>> specified objects, which is not implemented.  IAW: I wouldn't normally\n>> associate an option with that name with submodules.  Would\n>> --recurse-submodules alone suffice?\n> \n> Jens already commented on this, and I agree that --recursive should\n> be dropped from this patch.\nI only put —recursive because that is what git-clone has for it’s behaviour wrt submodules.\nIf that flag is deprecated then I’m fine with using only —recurse-submodules\nPerhaps a deprecation flag or note in the code would help?\n\n\nOverall I’m impressed by the speed and quality of the responses (and the codebase!) so am glad to\nmove this forward. I look forward to your feedback.\n\nKind Regards\nNick\n"},{"id":"231204","messageId":"xmqq61rdu0li.fsf@gitster.dls.corp.google.com","threadId":"35406","inReplyTo":"52953CB7.8020300@web.de","subject":"Re: [PATCH] submodule recursion in git-archive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-27T19:05:29Z","receivedAt":"2013-11-27T19:05:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> OK, but the repetition of \"cover letter\" and \"e-mail messages\"\n> irritates me slightly for some reason.  What about the following?\n\nLooks good to me; will queue, thanks.\n\n> -- >8 --\n> Subject: [PATCH] SubmittingPatches: document how to handle multiple patches\n>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n> ---\n>  Documentation/SubmittingPatches |   11 +++++++++--\n>  1 files changed, 9 insertions(+), 2 deletions(-)\n>\n> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches\n> index 7055576..e6d46ed 100644\n> --- a/Documentation/SubmittingPatches\n> +++ b/Documentation/SubmittingPatches\n> @@ -139,8 +139,15 @@ People on the Git mailing list need to be able to read and\n>  comment on the changes you are submitting.  It is important for\n>  a developer to be able to \"quote\" your changes, using standard\n>  e-mail tools, so that they may comment on specific portions of\n> -your code.  For this reason, all patches should be submitted\n> -\"inline\".  If your log message (including your name on the\n> +your code.  For this reason, each patch should be submitted\n> +\"inline\" in a separate message.\n> +\n> +Multiple related patches should be grouped into their own e-mail\n> +thread to help readers find all parts of the series.  To that end,\n> +send them as replies to either an additional \"cover letter\" message\n> +(see below), the first patch, or the respective preceding patch.\n> +\n> +If your log message (including your name on the\n>  Signed-off-by line) is not writable in ASCII, make sure that\n>  you send off a message in the correct encoding.\n"},{"id":"231208","messageId":"xmqqzjopsk9b.fsf@gitster.dls.corp.google.com","threadId":"35406","inReplyTo":"9AB10474-6DEF-4FFD-B6B3-ED2AB21424AC@mac.com","subject":"Re: [PATCH] submodule recursion in git-archive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-11-27T19:43:44Z","receivedAt":"2013-11-27T19:43:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nick Townsend <nick.townsend@mac.com> writes:\n\n> On 26 Nov 2013, at 14:18, Junio C Hamano <gitster@pobox.com> wrote:\n>\n>> Even if the code is run inside a repository with a working tree,\n>> when producing a tarball out of an ancient commit that had a\n>> submodule not at its current location, --recurse-submodules option\n>> should do the right thing, so asking for working tree location of\n>> that submodule to find its repository is wrong, I think.  It may\n>> happen to find one if the archived revision is close enough to what\n>> is currently checked out, but that may not necessarily be the case.\n>> \n>> At that point when the code discovers an S_ISGITLINK entry, it\n>> should have both a pathname to the submodule relative to the\n>> toplevel and the commit object name bound to that submodule\n>> location.  What it should do, when it does not find the repository\n>> at the given path (maybe because there is no working tree, or the\n>> sudmodule directory has moved over time) is roughly:\n>> \n>> - Read from .gitmodules at the top-level from the tree it is\n>>   creating the tarball out of;\n>> \n>> - Find \"submodule.$name.path\" entry that records that path to the\n>>   submodule; and then\n>> \n>> - Using that $name, find the stashed-away location of the submodule\n>>   repository in $GIT_DIR/modules/$name.\n>> \n>> or something like that.\n>> \n>> This is a related tangent, but when used in a repository that people\n>> often use as their remote, the repository discovery may have to\n>> interact with the relative URL.  People often ship .gitmodules with\n>> \n>> \t[submodule \"bar\"]\n>>        \tURL = ../bar.git\n>> \t\tpath = barDir\n>> \n>> for a top-level project \"foo\" that can be cloned thusly:\n>> \n>> \tgit clone git://site.xz/foo.git\n>> \n>> and host bar.git to be clonable with\n>> \n>> \tgit clone git://site.xz/bar.git barDir/\n>> \n>> inside the working tree of the foo project.  In such a case, when\n>> \"archive --recurse-submodules\" is running, it would find the\n>> repository for the \"bar\" submodule at \"../bar.git\", I would think.\n>> \n>> So this part needs a bit more thought, I am afraid.\n>\n> I see that there is a lot of potential complexity around setting up a submodule:\n\nNo question about it.\n\n> * The .gitmodules file can be dirty (easy to flag, but should we\n> allow archive to proceed?)\n\nAs we are discussing \"archive\", which takes a tree object from the\ntop-level project that is recorded in the object database, the\ninformation _about_ the submodule in question should come from the\ngiven tree being archived.  There is no reason for the .gitmodules\nfile that happens to be sitting in the working tree of the top-level\nproject to be involved in the decision, so its dirtyness should not\nmatter, I think.  If the tree being archived has a submodule whose\nname is \"kernel\" at path \"linux/\" (relative to the top-level\nproject), its repository should be at .git/modules/kernel in the\nlayout recent git-submodule prepares, and we should find that\npath-and-name mapping from .gitmodules recorded in that tree object\nwe are archiving. The version that happens to be checked out to the\nworking tree may have moved the submodule to a new path \"linux-3.0/\"\nand \"linux-3.0/.git\" may have \"gitdir: .git/modules/kernel\" in it,\nbut when archiving a tree that has the submodule at \"linux/\", it\nwould not help---we would not know to look at \"linux-3.0/.git\" to\nlearn that information anyway because .gitmodules in the working\ntree would say that the submodule at path \"linux-3.0/\" is with name\n\"kernel\", and would not tell us anything about \"linux/\".\n\n> * Users can mess with settings both prior to git submodule init\n> and before git submodule update.\n\nI think this is irrelevant for exactly the same reason as above.\n\nWhat makes this tricker, however, is how to deal with an old-style\nrepository, where the submodule repositories are embedded in the\nworking tree that happens to be checked out.  In that case, we may\nhave to read .gitmodules from two places, i.e.\n\n (1) We are archiving a tree with a submodule at \"linux/\";\n\n (2) We read .gitmodules from that tree and learn that the submodule\n     has name \"kernel\";\n\n (3) There is no \".git/modules/kernel\" because the repository uses\n     the old layout (if the user never was interested in this\n     submodule, .git/modules/kernel may also be missing, and we\n     should tell these two cases apart by checking .git/config to\n     see if a corresponding entry for the \"kernel\" submodule exists\n     there);\n\n (4) In a repository that uses the old layout, there must be the\n     repository somewhere embedded in the current working tree (this\n     inability to remove is why we use the new layout these days).\n     We can learn where it is by looking at .gitmodules in the\n     working tree---map the name \"kernel\" we learned earlier, and\n     map it to the current path (\"linux-3.0/\" if you have been\n     following this example so far).\n\nAnd in that fallback context, I would say that reading from a dirty\n(or \"messed with by the user\") .gitmodules is the right thing to\ndo.  Perhaps the user may be in the process of moving the submodule\nin his working tree with\n\n    $ mv linux-3.0 linux-3.2\n    $ git config -f .gitmodules submodule.kernel.path linux-3.2\n\nbut hasn't committed the change yet.\n\n> For those reasons I deliberately decided not to reproduce the\n> above logic all by myself.\n\nAs I already hinted, I agree that the \"how to find the location of\nsubmodule repository, given a particular tree in the top-level\nproject the submodule belongs to and the path to the submodule in\nquestion\" deserves a separate thread to discuss with area experts.\n"},{"id":"231283","messageId":"20131129223845.GA31636@sandbox-ub","threadId":"35406","inReplyTo":"xmqqzjopsk9b.fsf@gitster.dls.corp.google.com","subject":"Re: Re: [PATCH] submodule recursion in git-archive","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-11-29T22:38:45Z","receivedAt":"2013-11-29T22:38:45Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Wed, Nov 27, 2013 at 11:43:44AM -0800, Junio C Hamano wrote:\n> Nick Townsend <nick.townsend@mac.com> writes:\n> > * The .gitmodules file can be dirty (easy to flag, but should we\n> > allow archive to proceed?)\n> \n> As we are discussing \"archive\", which takes a tree object from the\n> top-level project that is recorded in the object database, the\n> information _about_ the submodule in question should come from the\n> given tree being archived.  There is no reason for the .gitmodules\n> file that happens to be sitting in the working tree of the top-level\n> project to be involved in the decision, so its dirtyness should not\n> matter, I think.  If the tree being archived has a submodule whose\n> name is \"kernel\" at path \"linux/\" (relative to the top-level\n> project), its repository should be at .git/modules/kernel in the\n> layout recent git-submodule prepares, and we should find that\n> path-and-name mapping from .gitmodules recorded in that tree object\n> we are archiving. The version that happens to be checked out to the\n> working tree may have moved the submodule to a new path \"linux-3.0/\"\n> and \"linux-3.0/.git\" may have \"gitdir: .git/modules/kernel\" in it,\n> but when archiving a tree that has the submodule at \"linux/\", it\n> would not help---we would not know to look at \"linux-3.0/.git\" to\n> learn that information anyway because .gitmodules in the working\n> tree would say that the submodule at path \"linux-3.0/\" is with name\n> \"kernel\", and would not tell us anything about \"linux/\".\n> \n> > * Users can mess with settings both prior to git submodule init\n> > and before git submodule update.\n> \n> I think this is irrelevant for exactly the same reason as above.\n> \n> What makes this tricker, however, is how to deal with an old-style\n> repository, where the submodule repositories are embedded in the\n> working tree that happens to be checked out.  In that case, we may\n> have to read .gitmodules from two places, i.e.\n> \n>  (1) We are archiving a tree with a submodule at \"linux/\";\n> \n>  (2) We read .gitmodules from that tree and learn that the submodule\n>      has name \"kernel\";\n> \n>  (3) There is no \".git/modules/kernel\" because the repository uses\n>      the old layout (if the user never was interested in this\n>      submodule, .git/modules/kernel may also be missing, and we\n>      should tell these two cases apart by checking .git/config to\n>      see if a corresponding entry for the \"kernel\" submodule exists\n>      there);\n> \n>  (4) In a repository that uses the old layout, there must be the\n>      repository somewhere embedded in the current working tree (this\n>      inability to remove is why we use the new layout these days).\n>      We can learn where it is by looking at .gitmodules in the\n>      working tree---map the name \"kernel\" we learned earlier, and\n>      map it to the current path (\"linux-3.0/\" if you have been\n>      following this example so far).\n> \n> And in that fallback context, I would say that reading from a dirty\n> (or \"messed with by the user\") .gitmodules is the right thing to\n> do.  Perhaps the user may be in the process of moving the submodule\n> in his working tree with\n> \n>     $ mv linux-3.0 linux-3.2\n>     $ git config -f .gitmodules submodule.kernel.path linux-3.2\n> \n> but hasn't committed the change yet.\n> \n> > For those reasons I deliberately decided not to reproduce the\n> > above logic all by myself.\n> \n> As I already hinted, I agree that the \"how to find the location of\n> submodule repository, given a particular tree in the top-level\n> project the submodule belongs to and the path to the submodule in\n> question\" deserves a separate thread to discuss with area experts.\n\nFYI, I already started to implement this lookup of submodule paths early\nthis year[1] but have not found the time to proceed on that yet. I am\nplanning to continue on that topic soonish. We need it to implement a\ncorrect recursive fetch with clone on-demand as a basis for the future\nrecursive checkout.\n\nDuring the work on this I hit too many open questions. Thats why I am\ncurrently working on a complete plan[2] so we can discuss and define how\nthis needs to be implemented. It is an asciidoc document which I will\nsend out once I am finished with it.\n\nCheers Heiko\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/217020\n[2] https://github.com/hvoigt/git/wiki/submodule-fetch-config\n"},{"id":"231400","messageId":"3651F1C2-741E-4170-9468-0EF07F120CB9@mac.com","threadId":"35406","inReplyTo":"xmqqzjopsk9b.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] submodule recursion in git-archive","fromName":"Nick Townsend","fromEmail":"nick.townsend@mac.com","sentAt":"2013-12-03T00:00:50Z","receivedAt":"2013-12-03T00:00:50Z","isPatch":true,"sender":{"key":"nick.townsend@mac.com","avatar":"https://avatars.githubusercontent.com/u/44087510?v=4"},"body":"\nOn 27 Nov 2013, at 11:43, Junio C Hamano <gitster@pobox.com> wrote:\n\n> Nick Townsend <nick.townsend@mac.com> writes:\n> \n>> On 26 Nov 2013, at 14:18, Junio C Hamano <gitster@pobox.com> wrote:\n>> \n>>> Even if the code is run inside a repository with a working tree,\n>>> when producing a tarball out of an ancient commit that had a\n>>> submodule not at its current location, --recurse-submodules option\n>>> should do the right thing, so asking for working tree location of\n>>> that submodule to find its repository is wrong, I think.  It may\n>>> happen to find one if the archived revision is close enough to what\n>>> is currently checked out, but that may not necessarily be the case.\n>>> \n>>> At that point when the code discovers an S_ISGITLINK entry, it\n>>> should have both a pathname to the submodule relative to the\n>>> toplevel and the commit object name bound to that submodule\n>>> location.  What it should do, when it does not find the repository\n>>> at the given path (maybe because there is no working tree, or the\n>>> sudmodule directory has moved over time) is roughly:\n>>> \n>>> - Read from .gitmodules at the top-level from the tree it is\n>>>  creating the tarball out of;\n>>> \n>>> - Find \"submodule.$name.path\" entry that records that path to the\n>>>  submodule; and then\n>>> \n>>> - Using that $name, find the stashed-away location of the submodule\n>>>  repository in $GIT_DIR/modules/$name.\n>>> \n>>> or something like that.\n>>> \n>>> This is a related tangent, but when used in a repository that people\n>>> often use as their remote, the repository discovery may have to\n>>> interact with the relative URL.  People often ship .gitmodules with\n>>> \n>>> \t[submodule \"bar\"]\n>>>       \tURL = ../bar.git\n>>> \t\tpath = barDir\n>>> \n>>> for a top-level project \"foo\" that can be cloned thusly:\n>>> \n>>> \tgit clone git://site.xz/foo.git\n>>> \n>>> and host bar.git to be clonable with\n>>> \n>>> \tgit clone git://site.xz/bar.git barDir/\n>>> \n>>> inside the working tree of the foo project.  In such a case, when\n>>> \"archive --recurse-submodules\" is running, it would find the\n>>> repository for the \"bar\" submodule at \"../bar.git\", I would think.\n>>> \n>>> So this part needs a bit more thought, I am afraid.\n>> \n>> I see that there is a lot of potential complexity around setting up a submodule:\n> \n> No question about it.\n> \n>> * The .gitmodules file can be dirty (easy to flag, but should we\n>> allow archive to proceed?)\n> \n> As we are discussing \"archive\", which takes a tree object from the\n> top-level project that is recorded in the object database, the\n> information _about_ the submodule in question should come from the\n> given tree being archived.  There is no reason for the .gitmodules\n> file that happens to be sitting in the working tree of the top-level\n> project to be involved in the decision, so its dirtyness should not\n> matter, I think.  If the tree being archived has a submodule whose\n> name is \"kernel\" at path \"linux/\" (relative to the top-level\n> project), its repository should be at .git/modules/kernel in the\n> layout recent git-submodule prepares, and we should find that\n> path-and-name mapping from .gitmodules recorded in that tree object\n> we are archiving. The version that happens to be checked out to the\n> working tree may have moved the submodule to a new path \"linux-3.0/\"\n> and \"linux-3.0/.git\" may have \"gitdir: .git/modules/kernel\" in it,\n> but when archiving a tree that has the submodule at \"linux/\", it\n> would not help---we would not know to look at \"linux-3.0/.git\" to\n> learn that information anyway because .gitmodules in the working\n> tree would say that the submodule at path \"linux-3.0/\" is with name\n> \"kernel\", and would not tell us anything about \"linux/\".\n> \n>> * Users can mess with settings both prior to git submodule init\n>> and before git submodule update.\n> \n> I think this is irrelevant for exactly the same reason as above.\n> \n> What makes this tricker, however, is how to deal with an old-style\n> repository, where the submodule repositories are embedded in the\n> working tree that happens to be checked out.  In that case, we may\n> have to read .gitmodules from two places, i.e.\n> \n> (1) We are archiving a tree with a submodule at \"linux/\";\n> \n> (2) We read .gitmodules from that tree and learn that the submodule\n>     has name \"kernel\";\n> \n> (3) There is no \".git/modules/kernel\" because the repository uses\n>     the old layout (if the user never was interested in this\n>     submodule, .git/modules/kernel may also be missing, and we\n>     should tell these two cases apart by checking .git/config to\n>     see if a corresponding entry for the \"kernel\" submodule exists\n>     there);\n> \n> (4) In a repository that uses the old layout, there must be the\n>     repository somewhere embedded in the current working tree (this\n>     inability to remove is why we use the new layout these days).\n>     We can learn where it is by looking at .gitmodules in the\n>     working tree---map the name \"kernel\" we learned earlier, and\n>     map it to the current path (\"linux-3.0/\" if you have been\n>     following this example so far).\n> \n> And in that fallback context, I would say that reading from a dirty\n> (or \"messed with by the user\") .gitmodules is the right thing to\n> do.  Perhaps the user may be in the process of moving the submodule\n> in his working tree with\n> \n>    $ mv linux-3.0 linux-3.2\n>    $ git config -f .gitmodules submodule.kernel.path linux-3.2\n> \n> but hasn't committed the change yet.\n> \n>> For those reasons I deliberately decided not to reproduce the\n>> above logic all by myself.\n> \n> As I already hinted, I agree that the \"how to find the location of\n> submodule repository, given a particular tree in the top-level\n> project the submodule belongs to and the path to the submodule in\n> question\" deserves a separate thread to discuss with area experts.\n\nAs per my email to Heiko on this thread, I’m happy to start such \na discussion - I’ll use your notes as a starting point. I’m much more comfortable\nusing a wiki for this - is this common or should I start a new mail thread\nwith RFC in the title or similar?\n\nI did complete my work on my version of git-archive (for internal use) and added some regression tests\nfor current behaviour. Also the add_submodule_odb patch should IMHO be incorporated\nanyway. I’ll resubmit those two for consideration in a new thread.\n\nKind Regards\nNick Townsend\n"},{"id":"231401","messageId":"D8D13DC5-0E93-4900-A738-A4A6700BC92F@mac.com","threadId":"35406","inReplyTo":"3651F1C2-741E-4170-9468-0EF07F120CB9@mac.com","subject":"Fwd: [PATCH] submodule recursion in git-archive","fromName":"Nick Townsend","fromEmail":"nick.townsend@mac.com","sentAt":"2013-12-03T00:03:37Z","receivedAt":"2013-12-03T00:03:37Z","isPatch":true,"sender":{"key":"nick.townsend@mac.com","avatar":"https://avatars.githubusercontent.com/u/44087510?v=4"},"body":"\n\nBegin forwarded message:\n\n> From: Nick Townsend <nick.townsend@mac.com>\n> Subject: Re: [PATCH] submodule recursion in git-archive\n> Date: 2 December 2013 16:00:50 GMT-8\n> To: Junio C Hamano <gitster@pobox.com>\n> Cc: René Scharfe <l.s.r@web.de>, Jens Lehmann <Jens.Lehmann@web.de>, git@vger.kernel.org, Jeff King <peff@peff.net>\n> \n> \n> On 27 Nov 2013, at 11:43, Junio C Hamano <gitster@pobox.com> wrote:\n> \n>> Nick Townsend <nick.townsend@mac.com> writes:\n>> \n>>> On 26 Nov 2013, at 14:18, Junio C Hamano <gitster@pobox.com> wrote:\n>>> \n>>>> Even if the code is run inside a repository with a working tree,\n>>>> when producing a tarball out of an ancient commit that had a\n>>>> submodule not at its current location, --recurse-submodules option\n>>>> should do the right thing, so asking for working tree location of\n>>>> that submodule to find its repository is wrong, I think.  It may\n>>>> happen to find one if the archived revision is close enough to what\n>>>> is currently checked out, but that may not necessarily be the case.\n>>>> \n>>>> At that point when the code discovers an S_ISGITLINK entry, it\n>>>> should have both a pathname to the submodule relative to the\n>>>> toplevel and the commit object name bound to that submodule\n>>>> location.  What it should do, when it does not find the repository\n>>>> at the given path (maybe because there is no working tree, or the\n>>>> sudmodule directory has moved over time) is roughly:\n>>>> \n>>>> - Read from .gitmodules at the top-level from the tree it is\n>>>> creating the tarball out of;\n>>>> \n>>>> - Find \"submodule.$name.path\" entry that records that path to the\n>>>> submodule; and then\n>>>> \n>>>> - Using that $name, find the stashed-away location of the submodule\n>>>> repository in $GIT_DIR/modules/$name.\n>>>> \n>>>> or something like that.\n>>>> \n>>>> This is a related tangent, but when used in a repository that people\n>>>> often use as their remote, the repository discovery may have to\n>>>> interact with the relative URL.  People often ship .gitmodules with\n>>>> \n>>>> \t[submodule \"bar\"]\n>>>>      \tURL = ../bar.git\n>>>> \t\tpath = barDir\n>>>> \n>>>> for a top-level project \"foo\" that can be cloned thusly:\n>>>> \n>>>> \tgit clone git://site.xz/foo.git\n>>>> \n>>>> and host bar.git to be clonable with\n>>>> \n>>>> \tgit clone git://site.xz/bar.git barDir/\n>>>> \n>>>> inside the working tree of the foo project.  In such a case, when\n>>>> \"archive --recurse-submodules\" is running, it would find the\n>>>> repository for the \"bar\" submodule at \"../bar.git\", I would think.\n>>>> \n>>>> So this part needs a bit more thought, I am afraid.\n>>> \n>>> I see that there is a lot of potential complexity around setting up a submodule:\n>> \n>> No question about it.\n>> \n>>> * The .gitmodules file can be dirty (easy to flag, but should we\n>>> allow archive to proceed?)\n>> \n>> As we are discussing \"archive\", which takes a tree object from the\n>> top-level project that is recorded in the object database, the\n>> information _about_ the submodule in question should come from the\n>> given tree being archived.  There is no reason for the .gitmodules\n>> file that happens to be sitting in the working tree of the top-level\n>> project to be involved in the decision, so its dirtyness should not\n>> matter, I think.  If the tree being archived has a submodule whose\n>> name is \"kernel\" at path \"linux/\" (relative to the top-level\n>> project), its repository should be at .git/modules/kernel in the\n>> layout recent git-submodule prepares, and we should find that\n>> path-and-name mapping from .gitmodules recorded in that tree object\n>> we are archiving. The version that happens to be checked out to the\n>> working tree may have moved the submodule to a new path \"linux-3.0/\"\n>> and \"linux-3.0/.git\" may have \"gitdir: .git/modules/kernel\" in it,\n>> but when archiving a tree that has the submodule at \"linux/\", it\n>> would not help---we would not know to look at \"linux-3.0/.git\" to\n>> learn that information anyway because .gitmodules in the working\n>> tree would say that the submodule at path \"linux-3.0/\" is with name\n>> \"kernel\", and would not tell us anything about \"linux/\".\n>> \n>>> * Users can mess with settings both prior to git submodule init\n>>> and before git submodule update.\n>> \n>> I think this is irrelevant for exactly the same reason as above.\n>> \n>> What makes this tricker, however, is how to deal with an old-style\n>> repository, where the submodule repositories are embedded in the\n>> working tree that happens to be checked out.  In that case, we may\n>> have to read .gitmodules from two places, i.e.\n>> \n>> (1) We are archiving a tree with a submodule at \"linux/\";\n>> \n>> (2) We read .gitmodules from that tree and learn that the submodule\n>>    has name \"kernel\";\n>> \n>> (3) There is no \".git/modules/kernel\" because the repository uses\n>>    the old layout (if the user never was interested in this\n>>    submodule, .git/modules/kernel may also be missing, and we\n>>    should tell these two cases apart by checking .git/config to\n>>    see if a corresponding entry for the \"kernel\" submodule exists\n>>    there);\n>> \n>> (4) In a repository that uses the old layout, there must be the\n>>    repository somewhere embedded in the current working tree (this\n>>    inability to remove is why we use the new layout these days).\n>>    We can learn where it is by looking at .gitmodules in the\n>>    working tree---map the name \"kernel\" we learned earlier, and\n>>    map it to the current path (\"linux-3.0/\" if you have been\n>>    following this example so far).\n>> \n>> And in that fallback context, I would say that reading from a dirty\n>> (or \"messed with by the user\") .gitmodules is the right thing to\n>> do.  Perhaps the user may be in the process of moving the submodule\n>> in his working tree with\n>> \n>>   $ mv linux-3.0 linux-3.2\n>>   $ git config -f .gitmodules submodule.kernel.path linux-3.2\n>> \n>> but hasn't committed the change yet.\n>> \n>>> For those reasons I deliberately decided not to reproduce the\n>>> above logic all by myself.\n>> \n>> As I already hinted, I agree that the \"how to find the location of\n>> submodule repository, given a particular tree in the top-level\n>> project the submodule belongs to and the path to the submodule in\n>> question\" deserves a separate thread to discuss with area experts.\n> \n> As per my email to Heiko on this thread, I’m happy to start such \n> a discussion - I’ll use your notes as a starting point. I’m much more comfortable\n> using a wiki for this - is this common or should I start a new mail thread\n> with RFC in the title or similar?\n> \n> I did complete my work on my version of git-archive (for internal use) and added some regression tests\n> for current behaviour. Also the add_submodule_odb patch should IMHO be incorporated\n> anyway. I’ll resubmit those two for consideration in a new thread.\n> \n> Kind Regards\n> Nick Townsend\n> \n"}]}