{"thread":{"id":"40118","subject":"[PATCH 2/7] submodule: extract functions for config set and lookup","startedAt":"2015-08-18T00:21:56Z","lastAt":"2015-08-21T14:22:12Z","messageCount":19,"participants":["Stefan Beller","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"268241","messageId":"1439857323-21048-1-git-send-email-sbeller@google.com","threadId":"40118","inReplyTo":null,"subject":"[PATCH 0/7] Submodule improvements","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:21:56Z","receivedAt":"2015-08-18T00:21:56Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This series is a reroll consisting of hv/submodule-config and \nsb/submodule-helper and it applies on top of origin/jk/git-path.\n\nOur long term goal is to make submodule handling more scalable\nby parallelizing the submodule code. To write good parallelizable\ncode I'd first want to port it to C, as there are no good portable\nsolutions for shell scripts to run in parallel. This series is a\nfirst step on porting git-submodule.sh to C.\n\nI did not alter the patches of Heiko, except for squashing\n$gmane/275799 (2 cleanup patches I proposed 5 days ago).\n\nThe module_{list, name, clone} functions are a direct translation\nof the shell counter parts. I took way longer than expected for\nmodule_clone, as I was fighting with absolute and relative paths\nfor too long. (Whenever shell is translated to C,\nI estimate two times the number of lines of code which fits\nquite reasonably.)\n\nThanks,\nStefan\n\nHeiko Voigt (4):\n submodule: implement a config API for lookup of .gitmodules values\n submodule: extract functions for config set and lookup\n submodule: use new config API for worktree configurations\n submodule: Allow errornous values for the fetchrecursesubmodules\n option\n\nStefan Beller (3):\n submodule: implement `module_list` as a builtin helper\n submodule: implement `module_name` as a builtin helper\n submodule: implement `module_clone` as a builtin helper\n\n .gitignore | 2 +\n Documentation/technical/api-submodule-config.txt | 62 +++\n Makefile | 3 +\n builtin.h | 1 +\n builtin/checkout.c | 1 +\n builtin/fetch.c | 1 +\n builtin/submodule--helper.c | 299 ++++++++++++++\n diff.c | 1 +\n git-submodule.sh | 164 +-------\n git.c | 1 +\n submodule-config.c | 482 +++++++++++++++++++++++\n submodule-config.h | 29 ++\n submodule.c | 122 ++----\n submodule.h | 4 +-\n t/t7411-submodule-config.sh | 153 +++++++\n test-submodule-config.c | 76 ++++\n 16 files changed, 1154 insertions(+), 247 deletions(-)\n create mode 100644 Documentation/technical/api-submodule-config.txt\n create mode 100644 builtin/submodule--helper.c\n create mode 100644 submodule-config.c\n create mode 100644 submodule-config.h\n create mode 100755 t/t7411-submodule-config.sh\n create mode 100644 test-submodule-config.c\n\n-- \n2.5.0.330.g130be8e.dirty\n"},{"id":"268243","messageId":"1439857323-21048-2-git-send-email-sbeller@google.com","threadId":"40118","inReplyTo":"1439857323-21048-1-git-send-email-sbeller@google.com","subject":"[PATCH 1/7] submodule: implement a config API for lookup of .gitmodules values","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:21:57Z","receivedAt":"2015-08-18T00:21:57Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Heiko Voigt \n\nIn a superproject some commands need to interact with submodules. They\nneed to query values from the .gitmodules file either from the worktree\nof from certain revisions. At the moment this is quite hard since a\ncaller would need to read the .gitmodules file from the history and then\nparse the values. We want to provide an API for this so we have one\nplace to get values from .gitmodules from any revision (including the\nworktree).\n\nThe API is realized as a cache which allows us to lazily read\n.gitmodules configurations by commit into a runtime cache which can then\nbe used to easily lookup values from it. Currently only the values for\npath or name are stored but it can be extended for any value needed.\n\nIt is expected that .gitmodules files do not change often between\ncommits. Thats why we lookup the .gitmodules sha1 from a commit and then\neither lookup an already parsed configuration or parse and cache an\nunknown one for each sha1. The cache is lazily build on demand for each\nrequested commit.\n\nThis cache can be used for all purposes which need knowledge about\nsubmodule configurations. Example use cases are:\n\n * Recursive submodule checkout needs to lookup a submodule name from\n its path when a submodule first appears. This needs be done before\n this configuration exists in the worktree.\n\n * The implementation of submodule support for 'git archive' needs to\n lookup the submodule name to generate the archive when given a\n revision that is not checked out.\n\n * 'git fetch' when given the --recurse-submodules=on-demand option (or\n configuration) needs to lookup submodule names by path from the\n database rather than reading from the worktree. For new submodule it\n needs to lookup the name from its path to allow cloning new\n submodules into the .git folder so they can be checked out without\n any network interaction when the user does a checkout of that\n revision.\n\nSigned-off-by: Heiko Voigt \nSigned-off-by: Stefan Beller \n---\n .gitignore | 1 +\n Documentation/technical/api-submodule-config.txt | 45 +++\n Makefile | 2 +\n submodule-config.c | 443 +++++++++++++++++++++++\n submodule-config.h | 27 ++\n submodule.c | 1 +\n submodule.h | 1 +\n t/t7411-submodule-config.sh | 85 +++++\n test-submodule-config.c | 66 ++++\n 9 files changed, 671 insertions(+)\n create mode 100644 Documentation/technical/api-submodule-config.txt\n create mode 100644 submodule-config.c\n create mode 100644 submodule-config.h\n create mode 100755 t/t7411-submodule-config.sh\n create mode 100644 test-submodule-config.c\n\ndiff --git a/.gitignore b/.gitignore\nindex a685ec1..4fd81ba 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -205,6 +205,7 @@\n /test-sha1-array\n /test-sigchain\n /test-string-list\n+/test-submodule-config\n /test-subprocess\n /test-svn-fe\n /test-urlmatch-normalization\ndiff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt\nnew file mode 100644\nindex 0000000..1fbde41\n--- /dev/null\n+++ b/Documentation/technical/api-submodule-config.txt\n@@ -0,0 +1,45 @@\n+submodule config cache API\n+==========================\n+\n+The submodule config cache API allows to read submodule\n+configurations/information from specified revisions. Internally\n+information is lazily read into a cache that is used to avoid\n+unnecessary parsing of the same .gitmodule files. Lookups can be done by\n+submodule path or name.\n+\n+Usage\n+-----\n+\n+The caller can look up information about submodules by using the\n+`submodule_from_path()` or `submodule_from_name()` functions. They return\n+a `struct submodule` which contains the values. The API automatically\n+initializes and allocates the needed infrastructure on-demand.\n+\n+If the internal cache might grow too big or when the caller is done with\n+the API, all internally cached values can be freed with submodule_free().\n+\n+Data Structures\n+---------------\n+\n+`struct submodule`::\n+\n+\tThis structure is used to return the information about one\n+\tsubmodule for a certain revision. It is returned by the lookup\n+\tfunctions.\n+\n+Functions\n+---------\n+\n+`void submodule_free()`::\n+\n+\tUse these to free the internally cached values.\n+\n+`const struct submodule *submodule_from_path(const unsigned char *commit_sha1, const char *path)`::\n+\n+\tLookup values for one submodule by its commit_sha1 and path.\n+\n+`const struct submodule *submodule_from_name(const unsigned char *commit_sha1, const char *name)`::\n+\n+\tThe same as above but lookup by name.\n+\n+For an example usage see test-submodule-config.c.\ndiff --git a/Makefile b/Makefile\nindex 7efedbe..24b636d 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -594,6 +594,7 @@ TEST_PROGRAMS_NEED_X += test-sha1\n TEST_PROGRAMS_NEED_X += test-sha1-array\n TEST_PROGRAMS_NEED_X += test-sigchain\n TEST_PROGRAMS_NEED_X += test-string-list\n+TEST_PROGRAMS_NEED_X += test-submodule-config\n TEST_PROGRAMS_NEED_X += test-subprocess\n TEST_PROGRAMS_NEED_X += test-svn-fe\n TEST_PROGRAMS_NEED_X += test-urlmatch-normalization\n@@ -785,6 +786,7 @@ LIB_OBJS += strbuf.o\n LIB_OBJS += streaming.o\n LIB_OBJS += string-list.o\n LIB_OBJS += submodule.o\n+LIB_OBJS += submodule-config.o\n LIB_OBJS += symlinks.o\n LIB_OBJS += tag.o\n LIB_OBJS += trace.o\ndiff --git a/submodule-config.c b/submodule-config.c\nnew file mode 100644\nindex 0000000..ea82edb\n--- /dev/null\n+++ b/submodule-config.c\n@@ -0,0 +1,443 @@\n+#include \"cache.h\"\n+#include \"submodule-config.h\"\n+#include \"submodule.h\"\n+#include \"strbuf.h\"\n+\n+/*\n+ * submodule cache lookup structure\n+ * There is one shared set of 'struct submodule' entries which can be\n+ * looked up by their sha1 blob id of the .gitmodule file and either\n+ * using path or name as key.\n+ * for_path stores submodule entries with path as key\n+ * for_name stores submodule entries with name as key\n+ */\n+struct submodule_cache {\n+\tstruct hashmap for_path;\n+\tstruct hashmap for_name;\n+};\n+\n+/*\n+ * thin wrapper struct needed to insert 'struct submodule' entries to\n+ * the hashmap\n+ */\n+struct submodule_entry {\n+\tstruct hashmap_entry ent;\n+\tstruct submodule *config;\n+};\n+\n+enum lookup_type {\n+\tlookup_name,\n+\tlookup_path\n+};\n+\n+static struct submodule_cache cache;\n+static int is_cache_init;\n+\n+static int config_path_cmp(const struct submodule_entry *a,\n+\t\t\t const struct submodule_entry *b,\n+\t\t\t const void *unused)\n+{\n+\treturn strcmp(a->config->path, b->config->path) ||\n+\t hashcmp(a->config->gitmodules_sha1, b->config->gitmodules_sha1);\n+}\n+\n+static int config_name_cmp(const struct submodule_entry *a,\n+\t\t\t const struct submodule_entry *b,\n+\t\t\t const void *unused)\n+{\n+\treturn strcmp(a->config->name, b->config->name) ||\n+\t hashcmp(a->config->gitmodules_sha1, b->config->gitmodules_sha1);\n+}\n+\n+static void cache_init(struct submodule_cache *cache)\n+{\n+\thashmap_init(&cache->for_path, (hashmap_cmp_fn) config_path_cmp, 0);\n+\thashmap_init(&cache->for_name, (hashmap_cmp_fn) config_name_cmp, 0);\n+}\n+\n+static void free_one_config(struct submodule_entry *entry)\n+{\n+\tfree((void *) entry->config->path);\n+\tfree((void *) entry->config->name);\n+\tfree(entry->config);\n+}\n+\n+static void cache_free(struct submodule_cache *cache)\n+{\n+\tstruct hashmap_iter iter;\n+\tstruct submodule_entry *entry;\n+\n+\t/*\n+\t * We iterate over the name hash here to be symmetric with the\n+\t * allocation of struct submodule entries. Each is allocated by\n+\t * their .gitmodule blob sha1 and submodule name.\n+\t */\n+\thashmap_iter_init(&cache->for_name, &iter);\n+\twhile ((entry = hashmap_iter_next(&iter)))\n+\t\tfree_one_config(entry);\n+\n+\thashmap_free(&cache->for_path, 1);\n+\thashmap_free(&cache->for_name, 1);\n+}\n+\n+static unsigned int hash_sha1_string(const unsigned char *sha1,\n+\t\t\t\t const char *string)\n+{\n+\treturn memhash(sha1, 20) + strhash(string);\n+}\n+\n+static void cache_put_path(struct submodule_cache *cache,\n+\t\t\t struct submodule *submodule)\n+{\n+\tunsigned int hash = hash_sha1_string(submodule->gitmodules_sha1,\n+\t\t\t\t\t submodule->path);\n+\tstruct submodule_entry *e = xmalloc(sizeof(*e));\n+\thashmap_entry_init(e, hash);\n+\te->config = submodule;\n+\thashmap_put(&cache->for_path, e);\n+}\n+\n+static void cache_remove_path(struct submodule_cache *cache,\n+\t\t\t struct submodule *submodule)\n+{\n+\tunsigned int hash = hash_sha1_string(submodule->gitmodules_sha1,\n+\t\t\t\t\t submodule->path);\n+\tstruct submodule_entry e;\n+\tstruct submodule_entry *removed;\n+\thashmap_entry_init(&e, hash);\n+\te.config = submodule;\n+\tremoved = hashmap_remove(&cache->for_path, &e, NULL);\n+\tfree(removed);\n+}\n+\n+static void cache_add(struct submodule_cache *cache,\n+\t\t struct submodule *submodule)\n+{\n+\tunsigned int hash = hash_sha1_string(submodule->gitmodules_sha1,\n+\t\t\t\t\t submodule->name);\n+\tstruct submodule_entry *e = xmalloc(sizeof(*e));\n+\thashmap_entry_init(e, hash);\n+\te->config = submodule;\n+\thashmap_add(&cache->for_name, e);\n+}\n+\n+static const struct submodule *cache_lookup_path(struct submodule_cache *cache,\n+\t\tconst unsigned char *gitmodules_sha1, const char *path)\n+{\n+\tstruct submodule_entry *entry;\n+\tunsigned int hash = hash_sha1_string(gitmodules_sha1, path);\n+\tstruct submodule_entry key;\n+\tstruct submodule key_config;\n+\n+\thashcpy(key_config.gitmodules_sha1, gitmodules_sha1);\n+\tkey_config.path = path;\n+\n+\thashmap_entry_init(&key, hash);\n+\tkey.config = &key_config;\n+\n+\tentry = hashmap_get(&cache->for_path, &key, NULL);\n+\tif (entry)\n+\t\treturn entry->config;\n+\treturn NULL;\n+}\n+\n+static struct submodule *cache_lookup_name(struct submodule_cache *cache,\n+\t\tconst unsigned char *gitmodules_sha1, const char *name)\n+{\n+\tstruct submodule_entry *entry;\n+\tunsigned int hash = hash_sha1_string(gitmodules_sha1, name);\n+\tstruct submodule_entry key;\n+\tstruct submodule key_config;\n+\n+\thashcpy(key_config.gitmodules_sha1, gitmodules_sha1);\n+\tkey_config.name = name;\n+\n+\thashmap_entry_init(&key, hash);\n+\tkey.config = &key_config;\n+\n+\tentry = hashmap_get(&cache->for_name, &key, NULL);\n+\tif (entry)\n+\t\treturn entry->config;\n+\treturn NULL;\n+}\n+\n+static int name_and_item_from_var(const char *var, struct strbuf *name,\n+\t\t\t\t struct strbuf *item)\n+{\n+\tconst char *subsection, *key;\n+\tint subsection_len, parse;\n+\tparse = parse_config_key(var, \"submodule\", &subsection,\n+\t\t\t&subsection_len, &key);\n+\tif (parse < 0 || !subsection)\n+\t\treturn 0;\n+\n+\tstrbuf_add(name, subsection, subsection_len);\n+\tstrbuf_addstr(item, key);\n+\n+\treturn 1;\n+}\n+\n+static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n+\t\tconst unsigned char *gitmodules_sha1, const char *name)\n+{\n+\tstruct submodule *submodule;\n+\tstruct strbuf name_buf = STRBUF_INIT;\n+\n+\tsubmodule = cache_lookup_name(cache, gitmodules_sha1, name);\n+\tif (submodule)\n+\t\treturn submodule;\n+\n+\tsubmodule = xmalloc(sizeof(*submodule));\n+\n+\tstrbuf_addstr(&name_buf, name);\n+\tsubmodule->name = strbuf_detach(&name_buf, NULL);\n+\n+\tsubmodule->path = NULL;\n+\tsubmodule->url = NULL;\n+\tsubmodule->fetch_recurse = RECURSE_SUBMODULES_NONE;\n+\tsubmodule->ignore = NULL;\n+\n+\thashcpy(submodule->gitmodules_sha1, gitmodules_sha1);\n+\n+\tcache_add(cache, submodule);\n+\n+\treturn submodule;\n+}\n+\n+static void warn_multiple_config(const unsigned char *commit_sha1,\n+\t\t\t\t const char *name, const char *option)\n+{\n+\tconst char *commit_string = \"WORKTREE\";\n+\tif (commit_sha1)\n+\t\tcommit_string = sha1_to_hex(commit_sha1);\n+\twarning(\"%s:.gitmodules, multiple configurations found for \"\n+\t\t\t\"'submodule.%s.%s'. Skipping second one!\",\n+\t\t\tcommit_string, name, option);\n+}\n+\n+struct parse_config_parameter {\n+\tstruct submodule_cache *cache;\n+\tconst unsigned char *commit_sha1;\n+\tconst unsigned char *gitmodules_sha1;\n+\tint overwrite;\n+};\n+\n+static int parse_config(const char *var, const char *value, void *data)\n+{\n+\tstruct parse_config_parameter *me = data;\n+\tstruct submodule *submodule;\n+\tstruct strbuf name = STRBUF_INIT, item = STRBUF_INIT;\n+\tint ret = 0;\n+\n+\t/* this also ensures that we only parse submodule entries */\n+\tif (!name_and_item_from_var(var, &name, &item))\n+\t\treturn 0;\n+\n+\tsubmodule = lookup_or_create_by_name(me->cache, me->gitmodules_sha1,\n+\t\t\tname.buf);\n+\n+\tif (!strcmp(item.buf, \"path\")) {\n+\t\tstruct strbuf path = STRBUF_INIT;\n+\t\tif (!value) {\n+\t\t\tret = config_error_nonbool(var);\n+\t\t\tgoto release_return;\n+\t\t}\n+\t\tif (!me->overwrite && submodule->path != NULL) {\n+\t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n+\t\t\t\t\t\"path\");\n+\t\t\tgoto release_return;\n+\t\t}\n+\n+\t\tif (submodule->path)\n+\t\t\tcache_remove_path(me->cache, submodule);\n+\t\tfree((void *) submodule->path);\n+\t\tstrbuf_addstr(&path, value);\n+\t\tsubmodule->path = strbuf_detach(&path, NULL);\n+\t\tcache_put_path(me->cache, submodule);\n+\t} else if (!strcmp(item.buf, \"fetchrecursesubmodules\")) {\n+\t\tif (!me->overwrite &&\n+\t\t submodule->fetch_recurse != RECURSE_SUBMODULES_NONE) {\n+\t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n+\t\t\t\t\t\"fetchrecursesubmodules\");\n+\t\t\tgoto release_return;\n+\t\t}\n+\n+\t\tsubmodule->fetch_recurse = parse_fetch_recurse_submodules_arg(var, value);\n+\t} else if (!strcmp(item.buf, \"ignore\")) {\n+\t\tstruct strbuf ignore = STRBUF_INIT;\n+\t\tif (!me->overwrite && submodule->ignore != NULL) {\n+\t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n+\t\t\t\t\t\"ignore\");\n+\t\t\tgoto release_return;\n+\t\t}\n+\t\tif (!value) {\n+\t\t\tret = config_error_nonbool(var);\n+\t\t\tgoto release_return;\n+\t\t}\n+\t\tif (strcmp(value, \"untracked\") && strcmp(value, \"dirty\") &&\n+\t\t strcmp(value, \"all\") && strcmp(value, \"none\")) {\n+\t\t\twarning(\"Invalid parameter '%s' for config option \"\n+\t\t\t\t\t\"'submodule.%s.ignore'\", value, var);\n+\t\t\tgoto release_return;\n+\t\t}\n+\n+\t\tfree((void *) submodule->ignore);\n+\t\tstrbuf_addstr(&ignore, value);\n+\t\tsubmodule->ignore = strbuf_detach(&ignore, NULL);\n+\t} else if (!strcmp(item.buf, \"url\")) {\n+\t\tstruct strbuf url = STRBUF_INIT;\n+\t\tif (!value) {\n+\t\t\tret = config_error_nonbool(var);\n+\t\t\tgoto release_return;\n+\t\t}\n+\t\tif (!me->overwrite && submodule->url != NULL) {\n+\t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n+\t\t\t\t\t\"url\");\n+\t\t\tgoto release_return;\n+\t\t}\n+\n+\t\tfree((void *) submodule->url);\n+\t\tstrbuf_addstr(&url, value);\n+\t\tsubmodule->url = strbuf_detach(&url, NULL);\n+\t}\n+\n+release_return:\n+\tstrbuf_release(&name);\n+\tstrbuf_release(&item);\n+\n+\treturn ret;\n+}\n+\n+static int gitmodule_sha1_from_commit(const unsigned char *commit_sha1,\n+\t\t\t\t unsigned char *gitmodules_sha1)\n+{\n+\tstruct strbuf rev = STRBUF_INIT;\n+\tint ret = 0;\n+\n+\tif (is_null_sha1(commit_sha1)) {\n+\t\thashcpy(gitmodules_sha1, null_sha1);\n+\t\treturn 1;\n+\t}\n+\n+\tstrbuf_addf(&rev, \"%s:.gitmodules\", sha1_to_hex(commit_sha1));\n+\tif (get_sha1(rev.buf, gitmodules_sha1) >= 0)\n+\t\tret = 1;\n+\n+\tstrbuf_release(&rev);\n+\treturn ret;\n+}\n+\n+/* This does a lookup of a submodule configuration by name or by path\n+ * (key) with on-demand reading of the appropriate .gitmodules from\n+ * revisions.\n+ */\n+static const struct submodule *config_from(struct submodule_cache *cache,\n+\t\tconst unsigned char *commit_sha1, const char *key,\n+\t\tenum lookup_type lookup_type)\n+{\n+\tstruct strbuf rev = STRBUF_INIT;\n+\tunsigned long config_size;\n+\tchar *config;\n+\tunsigned char sha1[20];\n+\tenum object_type type;\n+\tconst struct submodule *submodule = NULL;\n+\tstruct parse_config_parameter parameter;\n+\n+\t/*\n+\t * If any parameter except the cache is a NULL pointer just\n+\t * return the first submodule. Can be used to check whether\n+\t * there are any submodules parsed.\n+\t */\n+\tif (!commit_sha1 || !key) {\n+\t\tstruct hashmap_iter iter;\n+\t\tstruct submodule_entry *entry;\n+\n+\t\thashmap_iter_init(&cache->for_name, &iter);\n+\t\tentry = hashmap_iter_next(&iter);\n+\t\tif (!entry)\n+\t\t\treturn NULL;\n+\t\treturn entry->config;\n+\t}\n+\n+\tif (!gitmodule_sha1_from_commit(commit_sha1, sha1))\n+\t\treturn NULL;\n+\n+\tswitch (lookup_type) {\n+\tcase lookup_name:\n+\t\tsubmodule = cache_lookup_name(cache, sha1, key);\n+\t\tbreak;\n+\tcase lookup_path:\n+\t\tsubmodule = cache_lookup_path(cache, sha1, key);\n+\t\tbreak;\n+\t}\n+\tif (submodule)\n+\t\treturn submodule;\n+\n+\tconfig = read_sha1_file(sha1, &type, &config_size);\n+\tif (!config)\n+\t\treturn NULL;\n+\n+\tif (type != OBJ_BLOB) {\n+\t\tfree(config);\n+\t\treturn NULL;\n+\t}\n+\n+\t/* fill the submodule config into the cache */\n+\tparameter.cache = cache;\n+\tparameter.commit_sha1 = commit_sha1;\n+\tparameter.gitmodules_sha1 = sha1;\n+\tparameter.overwrite = 0;\n+\tgit_config_from_buf(parse_config, rev.buf, config, config_size,\n+\t\t\t¶meter);\n+\tfree(config);\n+\n+\tswitch (lookup_type) {\n+\tcase lookup_name:\n+\t\treturn cache_lookup_name(cache, sha1, key);\n+\tcase lookup_path:\n+\t\treturn cache_lookup_path(cache, sha1, key);\n+\tdefault:\n+\t\treturn NULL;\n+\t}\n+}\n+\n+static const struct submodule *config_from_path(struct submodule_cache *cache,\n+\t\tconst unsigned char *commit_sha1, const char *path)\n+{\n+\treturn config_from(cache, commit_sha1, path, lookup_path);\n+}\n+\n+static const struct submodule *config_from_name(struct submodule_cache *cache,\n+\t\tconst unsigned char *commit_sha1, const char *name)\n+{\n+\treturn config_from(cache, commit_sha1, name, lookup_name);\n+}\n+\n+static void ensure_cache_init(void)\n+{\n+\tif (is_cache_init)\n+\t\treturn;\n+\n+\tcache_init(&cache);\n+\tis_cache_init = 1;\n+}\n+\n+const struct submodule *submodule_from_name(const unsigned char *commit_sha1,\n+\t\tconst char *name)\n+{\n+\tensure_cache_init();\n+\treturn config_from_name(&cache, commit_sha1, name);\n+}\n+\n+const struct submodule *submodule_from_path(const unsigned char *commit_sha1,\n+\t\tconst char *path)\n+{\n+\tensure_cache_init();\n+\treturn config_from_path(&cache, commit_sha1, path);\n+}\n+\n+void submodule_free(void)\n+{\n+\tcache_free(&cache);\n+\tis_cache_init = 0;\n+}\ndiff --git a/submodule-config.h b/submodule-config.h\nnew file mode 100644\nindex 0000000..cd68030\n--- /dev/null\n+++ b/submodule-config.h\n@@ -0,0 +1,27 @@\n+#ifndef SUBMODULE_CONFIG_CACHE_H\n+#define SUBMODULE_CONFIG_CACHE_H\n+\n+#include \"hashmap.h\"\n+#include \"strbuf.h\"\n+\n+/*\n+ * Submodule entry containing the information about a certain submodule\n+ * in a certain revision.\n+ */\n+struct submodule {\n+\tconst char *path;\n+\tconst char *name;\n+\tconst char *url;\n+\tint fetch_recurse;\n+\tconst char *ignore;\n+\t/* the sha1 blob id of the responsible .gitmodules file */\n+\tunsigned char gitmodules_sha1[20];\n+};\n+\n+const struct submodule *submodule_from_name(const unsigned char *commit_sha1,\n+\t\tconst char *name);\n+const struct submodule *submodule_from_path(const unsigned char *commit_sha1,\n+\t\tconst char *path);\n+void submodule_free(void);\n+\n+#endif /* SUBMODULE_CONFIG_H */\ndiff --git a/submodule.c b/submodule.c\nindex 700bbf4..17b54e4 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -355,6 +355,7 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)\n \tdefault:\n \t\tif (!strcmp(arg, \"on-demand\"))\n \t\t\treturn RECURSE_SUBMODULES_ON_DEMAND;\n+\t\t/* TODO: remove the die for history parsing here */\n \t\tdie(\"bad %s argument: %s\", opt, arg);\n \t}\n }\ndiff --git a/submodule.h b/submodule.h\nindex 7beec48..920fef3 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -5,6 +5,7 @@ struct diff_options;\n struct argv_array;\n \n enum {\n+\tRECURSE_SUBMODULES_NONE = -2,\n \tRECURSE_SUBMODULES_ON_DEMAND = -1,\n \tRECURSE_SUBMODULES_OFF = 0,\n \tRECURSE_SUBMODULES_DEFAULT = 1,\ndiff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh\nnew file mode 100755\nindex 0000000..2602bc5\n--- /dev/null\n+++ b/t/t7411-submodule-config.sh\n@@ -0,0 +1,85 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2014 Heiko Voigt\n+#\n+\n+test_description='Test submodules config cache infrastructure\n+\n+This test verifies that parsing .gitmodules configuration directly\n+from the database works.\n+'\n+\n+TEST_NO_CREATE_REPO=1\n+. ./test-lib.sh\n+\n+test_expect_success 'submodule config cache setup' '\n+\tmkdir submodule &&\n+\t(cd submodule &&\n+\t\tgit init &&\n+\t\techo a >a &&\n+\t\tgit add . &&\n+\t\tgit commit -ma\n+\t) &&\n+\tmkdir super &&\n+\t(cd super &&\n+\t\tgit init &&\n+\t\tgit submodule add ../submodule &&\n+\t\tgit submodule add ../submodule a &&\n+\t\tgit commit -m \"add as submodule and as a\" &&\n+\t\tgit mv a b &&\n+\t\tgit commit -m \"move a to b\"\n+\t)\n+'\n+\n+cat >super/expect <actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+test_expect_success 'test parsing and lookup of submodule config by name' '\n+\t(cd super &&\n+\t\ttest-submodule-config --name \\\n+\t\t\tHEAD^ a \\\n+\t\t\tHEAD a \\\n+\t\t\tHEAD^ submodule \\\n+\t\t\tHEAD submodule \\\n+\t\t\t\t>actual &&\n+\t\ttest_cmp expect actual\n+\t)\n+'\n+\n+cat >super/expect_error <>.gitmodules &&\n+\t\tgit add .gitmodules &&\n+\t\tmv .gitmodules.bak .gitmodules &&\n+\t\tgit commit -m \"add error\" &&\n+\t\ttest-submodule-config \\\n+\t\t\tHEAD b \\\n+\t\t\tHEAD submodule \\\n+\t\t\t\t>actual &&\n+\t\ttest_cmp expect_error actual\n+\t)\n+'\n+\n+test_done\ndiff --git a/test-submodule-config.c b/test-submodule-config.c\nnew file mode 100644\nindex 0000000..f3c3918\n--- /dev/null\n+++ b/test-submodule-config.c\n@@ -0,0 +1,66 @@\n+#include \"cache.h\"\n+#include \"submodule-config.h\"\n+\n+static void die_usage(int argc, char **argv, const char *msg)\n+{\n+\tfprintf(stderr, \"%s\\n\", msg);\n+\tfprintf(stderr, \"Usage: %s [ ] ...\\n\", argv[0]);\n+\texit(1);\n+}\n+\n+int main(int argc, char **argv)\n+{\n+\tchar **arg = argv;\n+\tint my_argc = argc;\n+\tint output_url = 0;\n+\tint lookup_name = 0;\n+\n+\targ++;\n+\tmy_argc--;\n+\twhile (starts_with(arg[0], \"--\")) {\n+\t\tif (!strcmp(arg[0], \"--url\"))\n+\t\t\toutput_url = 1;\n+\t\tif (!strcmp(arg[0], \"--name\"))\n+\t\t\tlookup_name = 1;\n+\t\targ++;\n+\t\tmy_argc--;\n+\t}\n+\n+\tif (my_argc % 2 != 0)\n+\t\tdie_usage(argc, argv, \"Wrong number of arguments.\");\n+\n+\twhile (*arg) {\n+\t\tunsigned char commit_sha1[20];\n+\t\tconst struct submodule *submodule;\n+\t\tconst char *commit;\n+\t\tconst char *path_or_name;\n+\n+\t\tcommit = arg[0];\n+\t\tpath_or_name = arg[1];\n+\n+\t\tif (commit[0] == '\\0')\n+\t\t\thashcpy(commit_sha1, null_sha1);\n+\t\telse if (get_sha1(commit, commit_sha1) < 0)\n+\t\t\tdie_usage(argc, argv, \"Commit not found.\");\n+\n+\t\tif (lookup_name) {\n+\t\t\tsubmodule = submodule_from_name(commit_sha1, path_or_name);\n+\t\t} else\n+\t\t\tsubmodule = submodule_from_path(commit_sha1, path_or_name);\n+\t\tif (!submodule)\n+\t\t\tdie_usage(argc, argv, \"Submodule not found.\");\n+\n+\t\tif (output_url)\n+\t\t\tprintf(\"Submodule url: '%s' for path '%s'\\n\",\n+\t\t\t\t\tsubmodule->url, submodule->path);\n+\t\telse\n+\t\t\tprintf(\"Submodule name: '%s' for path '%s'\\n\",\n+\t\t\t\t\tsubmodule->name, submodule->path);\n+\n+\t\targ += 2;\n+\t}\n+\n+\tsubmodule_free();\n+\n+\treturn 0;\n+}\n-- \n2.5.0.330.g130be8e.dirty\n"},{"id":"268240","messageId":"1439857323-21048-3-git-send-email-sbeller@google.com","threadId":"40118","inReplyTo":"1439857323-21048-1-git-send-email-sbeller@google.com","subject":"[PATCH 2/7] submodule: extract functions for config set and lookup","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:21:58Z","receivedAt":"2015-08-18T00:21:58Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Heiko Voigt \n\nThis is one step towards using the new configuration API. We just\nextract these functions to make replacing the actual code easier.\n\nSigned-off-by: Heiko Voigt \nSigned-off-by: Stefan Beller \n---\n submodule.c | 142 +++++++++++++++++++++++++++++++++++++++++-------------------\n 1 file changed, 97 insertions(+), 45 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 17b54e4..307fa43 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -41,6 +41,76 @@ static int gitmodules_is_unmerged;\n */\n static int gitmodules_is_modified;\n \n+static const char *get_name_for_path(const char *path)\n+{\n+\tstruct string_list_item *path_option;\n+\tif (path == NULL) {\n+\t\tif (config_name_for_path.nr > 0)\n+\t\t\treturn config_name_for_path.items[0].util;\n+\t\telse\n+\t\t\treturn NULL;\n+\t}\n+\tpath_option = unsorted_string_list_lookup(&config_name_for_path, path);\n+\tif (!path_option)\n+\t\treturn NULL;\n+\treturn path_option->util;\n+}\n+\n+static void set_name_for_path(const char *path, const char *name, int namelen)\n+{\n+\tstruct string_list_item *config;\n+\tconfig = unsorted_string_list_lookup(&config_name_for_path, path);\n+\tif (config)\n+\t\tfree(config->util);\n+\telse\n+\t\tconfig = string_list_append(&config_name_for_path, xstrdup(path));\n+\tconfig->util = xmemdupz(name, namelen);\n+}\n+\n+static const char *get_ignore_for_name(const char *name)\n+{\n+\tstruct string_list_item *ignore_option;\n+\tignore_option = unsorted_string_list_lookup(&config_ignore_for_name, name);\n+\tif (!ignore_option)\n+\t\treturn NULL;\n+\n+\treturn ignore_option->util;\n+}\n+\n+static void set_ignore_for_name(const char *name, int namelen, const char *ignore)\n+{\n+\tstruct string_list_item *config;\n+\tchar *name_cstr = xmemdupz(name, namelen);\n+\tconfig = unsorted_string_list_lookup(&config_ignore_for_name, name_cstr);\n+\tif (config) {\n+\t\tfree(config->util);\n+\t\tfree(name_cstr);\n+\t} else\n+\t\tconfig = string_list_append(&config_ignore_for_name, name_cstr);\n+\tconfig->util = xstrdup(ignore);\n+}\n+\n+static int get_fetch_recurse_for_name(const char *name)\n+{\n+\tstruct string_list_item *fetch_recurse;\n+\tfetch_recurse = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name);\n+\tif (!fetch_recurse)\n+\t\treturn RECURSE_SUBMODULES_NONE;\n+\n+\treturn (intptr_t) fetch_recurse->util;\n+}\n+\n+static void set_fetch_recurse_for_name(const char *name, int namelen, int fetch_recurse)\n+{\n+\tstruct string_list_item *config;\n+\tchar *name_cstr = xmemdupz(name, namelen);\n+\tconfig = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name_cstr);\n+\tif (!config)\n+\t\tconfig = string_list_append(&config_fetch_recurse_submodules_for_name, name_cstr);\n+\telse\n+\t\tfree(name_cstr);\n+\tconfig->util = (void *)(intptr_t) fetch_recurse;\n+}\n \n int is_staging_gitmodules_ok(void)\n {\n@@ -55,7 +125,7 @@ int is_staging_gitmodules_ok(void)\n int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n {\n \tstruct strbuf entry = STRBUF_INIT;\n-\tstruct string_list_item *path_option;\n+\tconst char *path;\n \n \tif (!file_exists(\".gitmodules\")) /* Do nothing without .gitmodules */\n \t\treturn -1;\n@@ -63,13 +133,13 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n \tif (gitmodules_is_unmerged)\n \t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n \n-\tpath_option = unsorted_string_list_lookup(&config_name_for_path, oldpath);\n-\tif (!path_option) {\n+\tpath = get_name_for_path(oldpath);\n+\tif (!path) {\n \t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), oldpath);\n \t\treturn -1;\n \t}\n \tstrbuf_addstr(&entry, \"submodule.\");\n-\tstrbuf_addstr(&entry, path_option->util);\n+\tstrbuf_addstr(&entry, path);\n \tstrbuf_addstr(&entry, \".path\");\n \tif (git_config_set_in_file(\".gitmodules\", entry.buf, newpath) < 0) {\n \t\t/* Maybe the user already did that, don't error out here */\n@@ -89,7 +159,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n int remove_path_from_gitmodules(const char *path)\n {\n \tstruct strbuf sect = STRBUF_INIT;\n-\tstruct string_list_item *path_option;\n+\tconst char *path_option;\n \n \tif (!file_exists(\".gitmodules\")) /* Do nothing without .gitmodules */\n \t\treturn -1;\n@@ -97,13 +167,13 @@ int remove_path_from_gitmodules(const char *path)\n \tif (gitmodules_is_unmerged)\n \t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n \n-\tpath_option = unsorted_string_list_lookup(&config_name_for_path, path);\n+\tpath_option = get_name_for_path(path);\n \tif (!path_option) {\n \t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), path);\n \t\treturn -1;\n \t}\n \tstrbuf_addstr(§, \"submodule.\");\n-\tstrbuf_addstr(§, path_option->util);\n+\tstrbuf_addstr(§, path_option);\n \tif (git_config_rename_section_in_file(\".gitmodules\", sect.buf, NULL) < 0) {\n \t\t/* Maybe the user already did that, don't error out here */\n \t\twarning(_(\"Could not remove .gitmodules entry for %s\"), path);\n@@ -165,12 +235,11 @@ done:\n void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\t\t\t\t const char *path)\n {\n-\tstruct string_list_item *path_option, *ignore_option;\n-\tpath_option = unsorted_string_list_lookup(&config_name_for_path, path);\n-\tif (path_option) {\n-\t\tignore_option = unsorted_string_list_lookup(&config_ignore_for_name, path_option->util);\n-\t\tif (ignore_option)\n-\t\t\thandle_ignore_submodules_arg(diffopt, ignore_option->util);\n+\tconst char *name = get_name_for_path(path);\n+\tif (name) {\n+\t\tconst char *ignore = get_ignore_for_name(name);\n+\t\tif (ignore)\n+\t\t\thandle_ignore_submodules_arg(diffopt, ignore);\n \t\telse if (gitmodules_is_unmerged)\n \t\t\tDIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);\n \t}\n@@ -221,7 +290,6 @@ void gitmodules_config(void)\n \n int parse_submodule_config_option(const char *var, const char *value)\n {\n-\tstruct string_list_item *config;\n \tconst char *name, *key;\n \tint namelen;\n \n@@ -232,22 +300,14 @@ int parse_submodule_config_option(const char *var, const char *value)\n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n \n-\t\tconfig = unsorted_string_list_lookup(&config_name_for_path, value);\n-\t\tif (config)\n-\t\t\tfree(config->util);\n-\t\telse\n-\t\t\tconfig = string_list_append(&config_name_for_path, xstrdup(value));\n-\t\tconfig->util = xmemdupz(name, namelen);\n+\t\tset_name_for_path(value, name, namelen);\n+\n \t} else if (!strcmp(key, \"fetchrecursesubmodules\")) {\n-\t\tchar *name_cstr = xmemdupz(name, namelen);\n-\t\tconfig = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name_cstr);\n-\t\tif (!config)\n-\t\t\tconfig = string_list_append(&config_fetch_recurse_submodules_for_name, name_cstr);\n-\t\telse\n-\t\t\tfree(name_cstr);\n-\t\tconfig->util = (void *)(intptr_t)parse_fetch_recurse_submodules_arg(var, value);\n+\t\tint fetch_recurse = parse_fetch_recurse_submodules_arg(var, value);\n+\n+\t\tset_fetch_recurse_for_name(name, namelen, fetch_recurse);\n+\n \t} else if (!strcmp(key, \"ignore\")) {\n-\t\tchar *name_cstr;\n \n \t\tif (!value)\n \t\t\treturn config_error_nonbool(var);\n@@ -258,14 +318,7 @@ int parse_submodule_config_option(const char *var, const char *value)\n \t\t\treturn 0;\n \t\t}\n \n-\t\tname_cstr = xmemdupz(name, namelen);\n-\t\tconfig = unsorted_string_list_lookup(&config_ignore_for_name, name_cstr);\n-\t\tif (config) {\n-\t\t\tfree(config->util);\n-\t\t\tfree(name_cstr);\n-\t\t} else\n-\t\t\tconfig = string_list_append(&config_ignore_for_name, name_cstr);\n-\t\tconfig->util = xstrdup(value);\n+\t\tset_ignore_for_name(name, namelen, value);\n \t\treturn 0;\n \t}\n \treturn 0;\n@@ -647,7 +700,7 @@ static void calculate_changed_submodule_paths(void)\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n \n \t/* No need to check if there are no submodules configured */\n-\tif (!config_name_for_path.nr)\n+\tif (!get_name_for_path(NULL))\n \t\treturn;\n \n \tinit_revisions(&rev, NULL);\n@@ -694,7 +747,7 @@ int fetch_populated_submodules(const struct argv_array *options,\n \tint i, result = 0;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n-\tstruct string_list_item *name_for_path;\n+\tconst char *name_for_path;\n \tconst char *work_tree = get_git_work_tree();\n \tif (!work_tree)\n \t\tgoto out;\n@@ -725,18 +778,17 @@ int fetch_populated_submodules(const struct argv_array *options,\n \t\t\tcontinue;\n \n \t\tname = ce->name;\n-\t\tname_for_path = unsorted_string_list_lookup(&config_name_for_path, ce->name);\n+\t\tname_for_path = get_name_for_path(ce->name);\n \t\tif (name_for_path)\n-\t\t\tname = name_for_path->util;\n+\t\t\tname = name_for_path;\n \n \t\tdefault_argv = \"yes\";\n \t\tif (command_line_option == RECURSE_SUBMODULES_DEFAULT) {\n-\t\t\tstruct string_list_item *fetch_recurse_submodules_option;\n-\t\t\tfetch_recurse_submodules_option = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name);\n-\t\t\tif (fetch_recurse_submodules_option) {\n-\t\t\t\tif ((intptr_t)fetch_recurse_submodules_option->util == RECURSE_SUBMODULES_OFF)\n+\t\t\tint fetch_recurse_option = get_fetch_recurse_for_name(name);\n+\t\t\tif (fetch_recurse_option != RECURSE_SUBMODULES_NONE) {\n+\t\t\t\tif (fetch_recurse_option == RECURSE_SUBMODULES_OFF)\n \t\t\t\t\tcontinue;\n-\t\t\t\tif ((intptr_t)fetch_recurse_submodules_option->util == RECURSE_SUBMODULES_ON_DEMAND) {\n+\t\t\t\tif (fetch_recurse_option == RECURSE_SUBMODULES_ON_DEMAND) {\n \t\t\t\t\tif (!unsorted_string_list_lookup(&changed_submodule_paths, ce->name))\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\tdefault_argv = \"on-demand\";\n-- \n2.5.0.330.g130be8e.dirty\n"},{"id":"268242","messageId":"1439857323-21048-4-git-send-email-sbeller@google.com","threadId":"40118","inReplyTo":"1439857323-21048-1-git-send-email-sbeller@google.com","subject":"[PATCH 3/7] submodule: use new config API for worktree configurations","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:21:59Z","receivedAt":"2015-08-18T00:21:59Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Heiko Voigt \n\nWe remove the extracted functions and directly parse into and read out\nof the cache. This allows us to have one unified way of accessing\nsubmodule configuration values specific to single submodules. Regardless\nwhether we need to access a configuration from history or from the\nworktree.\n\nSigned-off-by: Heiko Voigt \nSigned-off-by: Junio C Hamano \nSigned-off-by: Stefan Beller \n---\n Documentation/technical/api-submodule-config.txt | 19 ++-\n builtin/checkout.c | 1 +\n diff.c | 1 +\n submodule-config.c | 12 ++\n submodule-config.h | 1 +\n submodule.c | 160 ++++-------------------\n submodule.h | 1 -\n t/t7411-submodule-config.sh | 37 +++++-\n test-submodule-config.c | 10 ++\n 9 files changed, 104 insertions(+), 138 deletions(-)\n\ndiff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt\nindex 1fbde41..941fa17 100644\n--- a/Documentation/technical/api-submodule-config.txt\n+++ b/Documentation/technical/api-submodule-config.txt\n@@ -10,10 +10,18 @@ submodule path or name.\n Usage\n -----\n \n+To initialize the cache with configurations from the worktree the caller\n+typically first calls `gitmodules_config()` to read values from the\n+worktree .gitmodules and then to overlay the local git config values\n+`parse_submodule_config_option()` from the config parsing\n+infrastructure.\n+\n The caller can look up information about submodules by using the\n `submodule_from_path()` or `submodule_from_name()` functions. They return\n a `struct submodule` which contains the values. The API automatically\n-initializes and allocates the needed infrastructure on-demand.\n+initializes and allocates the needed infrastructure on-demand. If the\n+caller does only want to lookup values from revisions the initialization\n+can be skipped.\n \n If the internal cache might grow too big or when the caller is done with\n the API, all internally cached values can be freed with submodule_free().\n@@ -34,6 +42,11 @@ Functions\n \n \tUse these to free the internally cached values.\n \n+`int parse_submodule_config_option(const char *var, const char *value)`::\n+\n+\tCan be passed to the config parsing infrastructure to parse\n+\tlocal (worktree) submodule configurations.\n+\n `const struct submodule *submodule_from_path(const unsigned char *commit_sha1, const char *path)`::\n \n \tLookup values for one submodule by its commit_sha1 and path.\n@@ -42,4 +55,8 @@ Functions\n \n \tThe same as above but lookup by name.\n \n+If given the null_sha1 as commit_sha1 the local configuration of a\n+submodule will be returned (e.g. consolidated values from local git\n+configuration and the .gitmodules file in the worktree).\n+\n For an example usage see test-submodule-config.c.\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 9ff5ca4..7ea533e 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -18,6 +18,7 @@\n #include \"xdiff-interface.h\"\n #include \"ll-merge.h\"\n #include \"resolve-undo.h\"\n+#include \"submodule-config.h\"\n #include \"submodule.h\"\n \n static const char * const checkout_usage[] = {\ndiff --git a/diff.c b/diff.c\nindex 7deac90..8b382c2 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -13,6 +13,7 @@\n #include \"utf8.h\"\n #include \"userdiff.h\"\n #include \"sigchain.h\"\n+#include \"submodule-config.h\"\n #include \"submodule.h\"\n #include \"ll-merge.h\"\n #include \"string-list.h\"\ndiff --git a/submodule-config.c b/submodule-config.c\nindex ea82edb..c64faf3 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -422,6 +422,18 @@ static void ensure_cache_init(void)\n \tis_cache_init = 1;\n }\n \n+int parse_submodule_config_option(const char *var, const char *value)\n+{\n+\tstruct parse_config_parameter parameter;\n+\tparameter.cache = &cache;\n+\tparameter.commit_sha1 = NULL;\n+\tparameter.gitmodules_sha1 = null_sha1;\n+\tparameter.overwrite = 1;\n+\n+\tensure_cache_init();\n+\treturn parse_config(var, value, ¶meter);\n+}\n+\n const struct submodule *submodule_from_name(const unsigned char *commit_sha1,\n \t\tconst char *name)\n {\ndiff --git a/submodule-config.h b/submodule-config.h\nindex cd68030..5fe44ce 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -18,6 +18,7 @@ struct submodule {\n \tunsigned char gitmodules_sha1[20];\n };\n \n+int parse_submodule_config_option(const char *var, const char *value);\n const struct submodule *submodule_from_name(const unsigned char *commit_sha1,\n \t\tconst char *name);\n const struct submodule *submodule_from_path(const unsigned char *commit_sha1,\ndiff --git a/submodule.c b/submodule.c\nindex 307fa43..a382677 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"submodule-config.h\"\n #include \"submodule.h\"\n #include \"dir.h\"\n #include \"diff.h\"\n@@ -12,9 +13,6 @@\n #include \"argv-array.h\"\n #include \"blob.h\"\n \n-static struct string_list config_name_for_path;\n-static struct string_list config_fetch_recurse_submodules_for_name;\n-static struct string_list config_ignore_for_name;\n static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n static struct string_list changed_submodule_paths;\n static int initialized_fetch_ref_tips;\n@@ -41,77 +39,6 @@ static int gitmodules_is_unmerged;\n */\n static int gitmodules_is_modified;\n \n-static const char *get_name_for_path(const char *path)\n-{\n-\tstruct string_list_item *path_option;\n-\tif (path == NULL) {\n-\t\tif (config_name_for_path.nr > 0)\n-\t\t\treturn config_name_for_path.items[0].util;\n-\t\telse\n-\t\t\treturn NULL;\n-\t}\n-\tpath_option = unsorted_string_list_lookup(&config_name_for_path, path);\n-\tif (!path_option)\n-\t\treturn NULL;\n-\treturn path_option->util;\n-}\n-\n-static void set_name_for_path(const char *path, const char *name, int namelen)\n-{\n-\tstruct string_list_item *config;\n-\tconfig = unsorted_string_list_lookup(&config_name_for_path, path);\n-\tif (config)\n-\t\tfree(config->util);\n-\telse\n-\t\tconfig = string_list_append(&config_name_for_path, xstrdup(path));\n-\tconfig->util = xmemdupz(name, namelen);\n-}\n-\n-static const char *get_ignore_for_name(const char *name)\n-{\n-\tstruct string_list_item *ignore_option;\n-\tignore_option = unsorted_string_list_lookup(&config_ignore_for_name, name);\n-\tif (!ignore_option)\n-\t\treturn NULL;\n-\n-\treturn ignore_option->util;\n-}\n-\n-static void set_ignore_for_name(const char *name, int namelen, const char *ignore)\n-{\n-\tstruct string_list_item *config;\n-\tchar *name_cstr = xmemdupz(name, namelen);\n-\tconfig = unsorted_string_list_lookup(&config_ignore_for_name, name_cstr);\n-\tif (config) {\n-\t\tfree(config->util);\n-\t\tfree(name_cstr);\n-\t} else\n-\t\tconfig = string_list_append(&config_ignore_for_name, name_cstr);\n-\tconfig->util = xstrdup(ignore);\n-}\n-\n-static int get_fetch_recurse_for_name(const char *name)\n-{\n-\tstruct string_list_item *fetch_recurse;\n-\tfetch_recurse = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name);\n-\tif (!fetch_recurse)\n-\t\treturn RECURSE_SUBMODULES_NONE;\n-\n-\treturn (intptr_t) fetch_recurse->util;\n-}\n-\n-static void set_fetch_recurse_for_name(const char *name, int namelen, int fetch_recurse)\n-{\n-\tstruct string_list_item *config;\n-\tchar *name_cstr = xmemdupz(name, namelen);\n-\tconfig = unsorted_string_list_lookup(&config_fetch_recurse_submodules_for_name, name_cstr);\n-\tif (!config)\n-\t\tconfig = string_list_append(&config_fetch_recurse_submodules_for_name, name_cstr);\n-\telse\n-\t\tfree(name_cstr);\n-\tconfig->util = (void *)(intptr_t) fetch_recurse;\n-}\n-\n int is_staging_gitmodules_ok(void)\n {\n \treturn !gitmodules_is_modified;\n@@ -125,7 +52,7 @@ int is_staging_gitmodules_ok(void)\n int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n {\n \tstruct strbuf entry = STRBUF_INIT;\n-\tconst char *path;\n+\tconst struct submodule *submodule;\n \n \tif (!file_exists(\".gitmodules\")) /* Do nothing without .gitmodules */\n \t\treturn -1;\n@@ -133,13 +60,13 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n \tif (gitmodules_is_unmerged)\n \t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n \n-\tpath = get_name_for_path(oldpath);\n-\tif (!path) {\n+\tsubmodule = submodule_from_path(null_sha1, oldpath);\n+\tif (!submodule || !submodule->name) {\n \t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), oldpath);\n \t\treturn -1;\n \t}\n \tstrbuf_addstr(&entry, \"submodule.\");\n-\tstrbuf_addstr(&entry, path);\n+\tstrbuf_addstr(&entry, submodule->name);\n \tstrbuf_addstr(&entry, \".path\");\n \tif (git_config_set_in_file(\".gitmodules\", entry.buf, newpath) < 0) {\n \t\t/* Maybe the user already did that, don't error out here */\n@@ -159,7 +86,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n int remove_path_from_gitmodules(const char *path)\n {\n \tstruct strbuf sect = STRBUF_INIT;\n-\tconst char *path_option;\n+\tconst struct submodule *submodule;\n \n \tif (!file_exists(\".gitmodules\")) /* Do nothing without .gitmodules */\n \t\treturn -1;\n@@ -167,13 +94,13 @@ int remove_path_from_gitmodules(const char *path)\n \tif (gitmodules_is_unmerged)\n \t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n \n-\tpath_option = get_name_for_path(path);\n-\tif (!path_option) {\n+\tsubmodule = submodule_from_path(null_sha1, path);\n+\tif (!submodule || !submodule->name) {\n \t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), path);\n \t\treturn -1;\n \t}\n \tstrbuf_addstr(§, \"submodule.\");\n-\tstrbuf_addstr(§, path_option);\n+\tstrbuf_addstr(§, submodule->name);\n \tif (git_config_rename_section_in_file(\".gitmodules\", sect.buf, NULL) < 0) {\n \t\t/* Maybe the user already did that, don't error out here */\n \t\twarning(_(\"Could not remove .gitmodules entry for %s\"), path);\n@@ -235,11 +162,10 @@ done:\n void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\t\t\t\t const char *path)\n {\n-\tconst char *name = get_name_for_path(path);\n-\tif (name) {\n-\t\tconst char *ignore = get_ignore_for_name(name);\n-\t\tif (ignore)\n-\t\t\thandle_ignore_submodules_arg(diffopt, ignore);\n+\tconst struct submodule *submodule = submodule_from_path(null_sha1, path);\n+\tif (submodule) {\n+\t\tif (submodule->ignore)\n+\t\t\thandle_ignore_submodules_arg(diffopt, submodule->ignore);\n \t\telse if (gitmodules_is_unmerged)\n \t\t\tDIFF_OPT_SET(diffopt, IGNORE_SUBMODULES);\n \t}\n@@ -288,42 +214,6 @@ void gitmodules_config(void)\n \t}\n }\n \n-int parse_submodule_config_option(const char *var, const char *value)\n-{\n-\tconst char *name, *key;\n-\tint namelen;\n-\n-\tif (parse_config_key(var, \"submodule\", &name, &namelen, &key) < 0 || !name)\n-\t\treturn 0;\n-\n-\tif (!strcmp(key, \"path\")) {\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\n-\t\tset_name_for_path(value, name, namelen);\n-\n-\t} else if (!strcmp(key, \"fetchrecursesubmodules\")) {\n-\t\tint fetch_recurse = parse_fetch_recurse_submodules_arg(var, value);\n-\n-\t\tset_fetch_recurse_for_name(name, namelen, fetch_recurse);\n-\n-\t} else if (!strcmp(key, \"ignore\")) {\n-\n-\t\tif (!value)\n-\t\t\treturn config_error_nonbool(var);\n-\n-\t\tif (strcmp(value, \"untracked\") && strcmp(value, \"dirty\") &&\n-\t\t strcmp(value, \"all\") && strcmp(value, \"none\")) {\n-\t\t\twarning(\"Invalid parameter \\\"%s\\\" for config option \\\"submodule.%s.ignore\\\"\", value, var);\n-\t\t\treturn 0;\n-\t\t}\n-\n-\t\tset_ignore_for_name(name, namelen, value);\n-\t\treturn 0;\n-\t}\n-\treturn 0;\n-}\n-\n void handle_ignore_submodules_arg(struct diff_options *diffopt,\n \t\t\t\t const char *arg)\n {\n@@ -700,7 +590,7 @@ static void calculate_changed_submodule_paths(void)\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n \n \t/* No need to check if there are no submodules configured */\n-\tif (!get_name_for_path(NULL))\n+\tif (!submodule_from_path(NULL, NULL))\n \t\treturn;\n \n \tinit_revisions(&rev, NULL);\n@@ -747,7 +637,6 @@ int fetch_populated_submodules(const struct argv_array *options,\n \tint i, result = 0;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct argv_array argv = ARGV_ARRAY_INIT;\n-\tconst char *name_for_path;\n \tconst char *work_tree = get_git_work_tree();\n \tif (!work_tree)\n \t\tgoto out;\n@@ -772,23 +661,26 @@ int fetch_populated_submodules(const struct argv_array *options,\n \t\tstruct strbuf submodule_git_dir = STRBUF_INIT;\n \t\tstruct strbuf submodule_prefix = STRBUF_INIT;\n \t\tconst struct cache_entry *ce = active_cache[i];\n-\t\tconst char *git_dir, *name, *default_argv;\n+\t\tconst char *git_dir, *default_argv;\n+\t\tconst struct submodule *submodule;\n \n \t\tif (!S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n \n-\t\tname = ce->name;\n-\t\tname_for_path = get_name_for_path(ce->name);\n-\t\tif (name_for_path)\n-\t\t\tname = name_for_path;\n+\t\tsubmodule = submodule_from_path(null_sha1, ce->name);\n+\t\tif (!submodule)\n+\t\t\tsubmodule = submodule_from_name(null_sha1, ce->name);\n \n \t\tdefault_argv = \"yes\";\n \t\tif (command_line_option == RECURSE_SUBMODULES_DEFAULT) {\n-\t\t\tint fetch_recurse_option = get_fetch_recurse_for_name(name);\n-\t\t\tif (fetch_recurse_option != RECURSE_SUBMODULES_NONE) {\n-\t\t\t\tif (fetch_recurse_option == RECURSE_SUBMODULES_OFF)\n+\t\t\tif (submodule &&\n+\t\t\t submodule->fetch_recurse !=\n+\t\t\t\t\t\tRECURSE_SUBMODULES_NONE) {\n+\t\t\t\tif (submodule->fetch_recurse ==\n+\t\t\t\t\t\tRECURSE_SUBMODULES_OFF)\n \t\t\t\t\tcontinue;\n-\t\t\t\tif (fetch_recurse_option == RECURSE_SUBMODULES_ON_DEMAND) {\n+\t\t\t\tif (submodule->fetch_recurse ==\n+\t\t\t\t\t\tRECURSE_SUBMODULES_ON_DEMAND) {\n \t\t\t\t\tif (!unsorted_string_list_lookup(&changed_submodule_paths, ce->name))\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\tdefault_argv = \"on-demand\";\ndiff --git a/submodule.h b/submodule.h\nindex 920fef3..547219d 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -20,7 +20,6 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\tconst char *path);\n int submodule_config(const char *var, const char *value, void *cb);\n void gitmodules_config(void);\n-int parse_submodule_config_option(const char *var, const char *value);\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n void show_submodule_summary(FILE *f, const char *path,\ndiff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh\nindex 2602bc5..7229978 100755\n--- a/t/t7411-submodule-config.sh\n+++ b/t/t7411-submodule-config.sh\n@@ -5,8 +5,8 @@\n \n test_description='Test submodules config cache infrastructure\n \n-This test verifies that parsing .gitmodules configuration directly\n-from the database works.\n+This test verifies that parsing .gitmodules configurations directly\n+from the database and from the worktree works.\n '\n \n TEST_NO_CREATE_REPO=1\n@@ -82,4 +82,37 @@ test_expect_success 'error in one submodule config lets continue' '\n \t)\n '\n \n+cat >super/expect_url <super/expect_local_path <actual &&\n+\t\ttest_cmp expect_url actual &&\n+\t\tgit config submodule.a.path c &&\n+\t\ttest-submodule-config \\\n+\t\t\t\"\" c \\\n+\t\t\t\"\" submodule \\\n+\t\t\t\t>actual &&\n+\t\ttest_cmp expect_local_path actual &&\n+\t\tgit config submodule.a.url $old_a &&\n+\t\tgit config submodule.submodule.url $old_submodule &&\n+\t\tgit config --unset submodule.a.path c\n+\t)\n+'\n+\n test_done\ndiff --git a/test-submodule-config.c b/test-submodule-config.c\nindex f3c3918..dab8c27 100644\n--- a/test-submodule-config.c\n+++ b/test-submodule-config.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"submodule-config.h\"\n+#include \"submodule.h\"\n \n static void die_usage(int argc, char **argv, const char *msg)\n {\n@@ -8,6 +9,11 @@ static void die_usage(int argc, char **argv, const char *msg)\n \texit(1);\n }\n \n+static int git_test_config(const char *var, const char *value, void *cb)\n+{\n+\treturn parse_submodule_config_option(var, value);\n+}\n+\n int main(int argc, char **argv)\n {\n \tchar **arg = argv;\n@@ -29,6 +35,10 @@ int main(int argc, char **argv)\n \tif (my_argc % 2 != 0)\n \t\tdie_usage(argc, argv, \"Wrong number of arguments.\");\n \n+\tsetup_git_directory();\n+\tgitmodules_config();\n+\tgit_config(git_test_config, NULL);\n+\n \twhile (*arg) {\n \t\tunsigned char commit_sha1[20];\n \t\tconst struct submodule *submodule;\n-- \n2.5.0.330.g130be8e.dirty\n"},{"id":"268244","messageId":"1439857323-21048-5-git-send-email-sbeller@google.com","threadId":"40118","inReplyTo":"1439857323-21048-1-git-send-email-sbeller@google.com","subject":"[PATCH 4/7] submodule: Allow errornous values for the fetchrecursesubmodules option","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:22:00Z","receivedAt":"2015-08-18T00:22:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Heiko Voigt \n\nWe should not die when reading the submodule config cache since the user\nmight not be able to get out of that situation when the configuration is\npart of the history.\n\nWe should handle this condition later when the value is about to be\nused.\n\nSigned-off-by: Heiko Voigt \nSigned-off-by: Stefan Beller \n---\n builtin/fetch.c | 1 +\n submodule-config.c | 29 ++++++++++++++++++++++++++++-\n submodule-config.h | 1 +\n submodule.c | 15 ---------------\n submodule.h | 2 +-\n t/t7411-submodule-config.sh | 35 +++++++++++++++++++++++++++++++++++\n 6 files changed, 66 insertions(+), 17 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex df16d0a..ee1f1a9 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -11,6 +11,7 @@\n #include \"run-command.h\"\n #include \"parse-options.h\"\n #include \"sigchain.h\"\n+#include \"submodule-config.h\"\n #include \"submodule.h\"\n #include \"connected.h\"\n #include \"argv-array.h\"\ndiff --git a/submodule-config.c b/submodule-config.c\nindex c64faf3..393de53 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -204,6 +204,30 @@ static struct submodule *lookup_or_create_by_name(struct submodule_cache *cache,\n \treturn submodule;\n }\n \n+static int parse_fetch_recurse(const char *opt, const char *arg,\n+\t\t\t int die_on_error)\n+{\n+\tswitch (git_config_maybe_bool(opt, arg)) {\n+\tcase 1:\n+\t\treturn RECURSE_SUBMODULES_ON;\n+\tcase 0:\n+\t\treturn RECURSE_SUBMODULES_OFF;\n+\tdefault:\n+\t\tif (!strcmp(arg, \"on-demand\"))\n+\t\t\treturn RECURSE_SUBMODULES_ON_DEMAND;\n+\n+\t\tif (die_on_error)\n+\t\t\tdie(\"bad %s argument: %s\", opt, arg);\n+\t\telse\n+\t\t\treturn RECURSE_SUBMODULES_ERROR;\n+\t}\n+}\n+\n+int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)\n+{\n+\treturn parse_fetch_recurse(opt, arg, 1);\n+}\n+\n static void warn_multiple_config(const unsigned char *commit_sha1,\n \t\t\t\t const char *name, const char *option)\n {\n@@ -255,6 +279,8 @@ static int parse_config(const char *var, const char *value, void *data)\n \t\tsubmodule->path = strbuf_detach(&path, NULL);\n \t\tcache_put_path(me->cache, submodule);\n \t} else if (!strcmp(item.buf, \"fetchrecursesubmodules\")) {\n+\t\t/* when parsing worktree configurations we can die early */\n+\t\tint die_on_error = is_null_sha1(me->gitmodules_sha1);\n \t\tif (!me->overwrite &&\n \t\t submodule->fetch_recurse != RECURSE_SUBMODULES_NONE) {\n \t\t\twarn_multiple_config(me->commit_sha1, submodule->name,\n@@ -262,7 +288,8 @@ static int parse_config(const char *var, const char *value, void *data)\n \t\t\tgoto release_return;\n \t\t}\n \n-\t\tsubmodule->fetch_recurse = parse_fetch_recurse_submodules_arg(var, value);\n+\t\tsubmodule->fetch_recurse = parse_fetch_recurse(var, value,\n+\t\t\t\t\t\t\t\tdie_on_error);\n \t} else if (!strcmp(item.buf, \"ignore\")) {\n \t\tstruct strbuf ignore = STRBUF_INIT;\n \t\tif (!me->overwrite && submodule->ignore != NULL) {\ndiff --git a/submodule-config.h b/submodule-config.h\nindex 5fe44ce..9061e4e 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -18,6 +18,7 @@ struct submodule {\n \tunsigned char gitmodules_sha1[20];\n };\n \n+int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n int parse_submodule_config_option(const char *var, const char *value);\n const struct submodule *submodule_from_name(const unsigned char *commit_sha1,\n \t\tconst char *name);\ndiff --git a/submodule.c b/submodule.c\nindex a382677..9fcc86f 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -288,21 +288,6 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,\n \tstrbuf_release(&sb);\n }\n \n-int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)\n-{\n-\tswitch (git_config_maybe_bool(opt, arg)) {\n-\tcase 1:\n-\t\treturn RECURSE_SUBMODULES_ON;\n-\tcase 0:\n-\t\treturn RECURSE_SUBMODULES_OFF;\n-\tdefault:\n-\t\tif (!strcmp(arg, \"on-demand\"))\n-\t\t\treturn RECURSE_SUBMODULES_ON_DEMAND;\n-\t\t/* TODO: remove the die for history parsing here */\n-\t\tdie(\"bad %s argument: %s\", opt, arg);\n-\t}\n-}\n-\n void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n \t\tunsigned char one[20], unsigned char two[20],\ndiff --git a/submodule.h b/submodule.h\nindex 547219d..5507c3d 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -5,6 +5,7 @@ struct diff_options;\n struct argv_array;\n \n enum {\n+\tRECURSE_SUBMODULES_ERROR = -3,\n \tRECURSE_SUBMODULES_NONE = -2,\n \tRECURSE_SUBMODULES_ON_DEMAND = -1,\n \tRECURSE_SUBMODULES_OFF = 0,\n@@ -21,7 +22,6 @@ void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n int submodule_config(const char *var, const char *value, void *cb);\n void gitmodules_config(void);\n void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n-int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n \t\tunsigned char one[20], unsigned char two[20],\ndiff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh\nindex 7229978..fc97c33 100755\n--- a/t/t7411-submodule-config.sh\n+++ b/t/t7411-submodule-config.sh\n@@ -115,4 +115,39 @@ test_expect_success 'reading of local configuration' '\n \t)\n '\n \n+cat >super/expect_fetchrecurse_die.err <actual.out 2>actual.err &&\n+\t\ttouch expect_fetchrecurse_die.out &&\n+\t\ttest_cmp expect_fetchrecurse_die.out actual.out &&\n+\t\ttest_cmp expect_fetchrecurse_die.err actual.err &&\n+\t\tgit config --unset submodule.submodule.fetchrecursesubmodules\n+\t)\n+'\n+\n+test_expect_success 'error in history in fetchrecursesubmodule lets continue' '\n+\t(cd super &&\n+\t\tgit config -f .gitmodules \\\n+\t\t\tsubmodule.submodule.fetchrecursesubmodules blabla &&\n+\t\tgit add .gitmodules &&\n+\t\tgit config --unset -f .gitmodules \\\n+\t\t\tsubmodule.submodule.fetchrecursesubmodules &&\n+\t\tgit commit -m \"add error in fetchrecursesubmodules\" &&\n+\t\ttest-submodule-config \\\n+\t\t\tHEAD b \\\n+\t\t\tHEAD submodule \\\n+\t\t\t\t>actual &&\n+\t\ttest_cmp expect_error actual &&\n+\t\tgit reset --hard HEAD^\n+\t)\n+'\n+\n test_done\n-- \n2.5.0.330.g130be8e.dirty\n"},{"id":"268247","messageId":"1439857323-21048-6-git-send-email-sbeller@google.com","threadId":"40118","inReplyTo":"1439857323-21048-1-git-send-email-sbeller@google.com","subject":"[PATCH 5/7] submodule: implement `module_list` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:22:01Z","receivedAt":"2015-08-18T00:22:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Most of the submodule operations work on a set of submodules.\nCalculating and using this set is usually done via:\n\n module_list \"$@\" | {\n while read mode sha1 stage sm_path\n do\n # the actual operation\n done\n }\n\nCurrently the function `module_list` is implemented in the\ngit-submodule.sh as a shell script wrapping a perl script.\nThe rewrite is in C, such that it is faster and can later be\neasily adapted when other functions are rewritten in C.\n\ngit-submodule.sh similar to the builtin commands will navigate\nto the top most directory of the repository and keeping the\nsubdirectories as a variable. As the helper is called from\nwithin the git-submodule.sh script, we are already navigated\nto the root level, but the path arguments are stil relative\nto the subdirectory we were in when calling git-submodule.sh.\nThat's why there is a `--prefix` option pointing to an alternative\npath where to anchor relative path arguments.\n\nSigned-off-by: Stefan Beller \nSigned-off-by: Junio C Hamano \n---\n .gitignore | 1 +\n Makefile | 1 +\n builtin.h | 1 +\n builtin/submodule--helper.c | 114 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh | 54 +++------------------\n git.c | 1 +\n 6 files changed, 124 insertions(+), 48 deletions(-)\n create mode 100644 builtin/submodule--helper.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 4fd81ba..1c2f832 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -155,6 +155,7 @@\n /git-status\n /git-stripspace\n /git-submodule\n+/git-submodule--helper\n /git-svn\n /git-symbolic-ref\n /git-tag\ndiff --git a/Makefile b/Makefile\nindex 24b636d..d434e73 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -901,6 +901,7 @@ BUILTIN_OBJS += builtin/shortlog.o\n BUILTIN_OBJS += builtin/show-branch.o\n BUILTIN_OBJS += builtin/show-ref.o\n BUILTIN_OBJS += builtin/stripspace.o\n+BUILTIN_OBJS += builtin/submodule--helper.o\n BUILTIN_OBJS += builtin/symbolic-ref.o\n BUILTIN_OBJS += builtin/tag.o\n BUILTIN_OBJS += builtin/unpack-file.o\ndiff --git a/builtin.h b/builtin.h\nindex 839483d..924e6c4 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -119,6 +119,7 @@ extern int cmd_show(int argc, const char **argv, const char *prefix);\n extern int cmd_show_branch(int argc, const char **argv, const char *prefix);\n extern int cmd_status(int argc, const char **argv, const char *prefix);\n extern int cmd_stripspace(int argc, const char **argv, const char *prefix);\n+extern int cmd_submodule__helper(int argc, const char **argv, const char *prefix);\n extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);\n extern int cmd_tag(int argc, const char **argv, const char *prefix);\n extern int cmd_tar_tree(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nnew file mode 100644\nindex 0000000..610ec51\n--- /dev/null\n+++ b/builtin/submodule--helper.c\n@@ -0,0 +1,114 @@\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"parse-options.h\"\n+#include \"quote.h\"\n+#include \"pathspec.h\"\n+#include \"dir.h\"\n+#include \"utf8.h\"\n+\n+static const struct cache_entry **ce_entries;\n+static int ce_alloc, ce_used;\n+static const char *alternative_path;\n+\n+static int module_list_compute(int argc, const char **argv,\n+\t\t\t\tconst char *prefix,\n+\t\t\t\tstruct pathspec *pathspec)\n+{\n+\tint i;\n+\tchar *max_prefix, *ps_matched = NULL;\n+\tint max_prefix_len;\n+\tparse_pathspec(pathspec, 0,\n+\t\t PATHSPEC_PREFER_FULL |\n+\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n+\t\t prefix, argv);\n+\n+\t/* Find common prefix for all pathspec's */\n+\tmax_prefix = common_prefix(pathspec);\n+\tmax_prefix_len = max_prefix ? strlen(max_prefix) : 0;\n+\n+\tif (pathspec->nr)\n+\t\tps_matched = xcalloc(1, pathspec->nr);\n+\n+\tif (read_cache() < 0)\n+\t\tdie(\"index file corrupt\");\n+\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tconst struct cache_entry *ce = active_cache[i];\n+\n+\t\tif (!match_pathspec(pathspec, ce->name, ce_namelen(ce),\n+\t\t\t\t max_prefix_len, ps_matched,\n+\t\t\t\t S_ISGITLINK(ce->ce_mode) | S_ISDIR(ce->ce_mode)))\n+\t\t\tcontinue;\n+\n+\t\tif (S_ISGITLINK(ce->ce_mode)) {\n+\t\t\tALLOC_GROW(ce_entries, ce_used + 1, ce_alloc);\n+\t\t\tce_entries[ce_used++] = ce;\n+\t\t}\n+\n+\t\twhile (i + 1 < active_nr && !strcmp(ce->name, active_cache[i + 1]->name))\n+\t\t\t/*\n+\t\t\t * Skip entries with the same name in different stages\n+\t\t\t * to make sure an entry is returned only once.\n+\t\t\t */\n+\t\t\ti++;\n+\t}\n+\tfree(max_prefix);\n+\n+\tif (ps_matched && report_path_error(ps_matched, pathspec, prefix))\n+\t\treturn -1;\n+\n+\treturn 0;\n+}\n+\n+static int module_list(int argc, const char **argv, const char *prefix)\n+{\n+\tint i;\n+\tstatic struct pathspec pathspec;\n+\n+\tstruct option module_list_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &alternative_path,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"alternative anchor for relative paths\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tstatic const char * const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper module_list [--prefix=] [...]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_list_options,\n+\t\t\t git_submodule_helper_usage, 0);\n+\n+\tif (module_list_compute(argc, argv, alternative_path\n+\t\t\t\t\t ? alternative_path\n+\t\t\t\t\t : prefix, &pathspec) < 0) {\n+\t\tprintf(\"#unmatched\\n\");\n+\t\treturn 1;\n+\t}\n+\n+\tfor (i = 0; i < ce_used; i++) {\n+\t\tconst struct cache_entry *ce = ce_entries[i];\n+\n+\t\tif (ce_stage(ce)) {\n+\t\t\tprintf(\"%06o %s U\\t\", ce->ce_mode, sha1_to_hex(null_sha1));\n+\t\t} else {\n+\t\t\tprintf(\"%06o %s %d\\t\", ce->ce_mode, sha1_to_hex(ce->sha1), ce_stage(ce));\n+\t\t}\n+\n+\t\tutf8_fprintf(stdout, \"%s\\n\", ce->name);\n+\t}\n+\treturn 0;\n+}\n+\n+int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n+{\n+\tif (argc < 2)\n+\t\tgoto usage;\n+\n+\tif (!strcmp(argv[1], \"module_list\"))\n+\t\treturn module_list(argc - 1, argv + 1, prefix);\n+\n+usage:\n+\tusage(\"git submodule--helper module_list\\n\");\n+}\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 36797c3..af9ecef 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -145,48 +145,6 @@ relative_path ()\n \techo \"$result$target\"\n }\n \n-#\n-# Get submodule info for registered submodules\n-# $@ = path to limit submodule list\n-#\n-module_list()\n-{\n-\teval \"set $(git rev-parse --sq --prefix \"$wt_prefix\" -- \"$@\")\"\n-\t(\n-\t\tgit ls-files -z --error-unmatch --stage -- \"$@\" ||\n-\t\techo \"unmatched pathspec exists\"\n-\t) |\n-\t@@PERL@@ -e '\n-\tmy %unmerged = ();\n-\tmy ($null_sha1) = (\"0\" x 40);\n-\tmy @out = ();\n-\tmy $unmatched = 0;\n-\t$/ = \"\\0\";\n-\twhile () {\n-\t\tif (/^unmatched pathspec/) {\n-\t\t\t$unmatched = 1;\n-\t\t\tnext;\n-\t\t}\n-\t\tchomp;\n-\t\tmy ($mode, $sha1, $stage, $path) =\n-\t\t\t/^([0-7]+) ([0-9a-f]{40}) ([0-3])\\t(.*)$/;\n-\t\tnext unless $mode eq \"160000\";\n-\t\tif ($stage ne \"0\") {\n-\t\t\tif (!$unmerged{$path}++) {\n-\t\t\t\tpush @out, \"$mode $null_sha1 U\\t$path\\n\";\n-\t\t\t}\n-\t\t\tnext;\n-\t\t}\n-\t\tpush @out, \"$_\\n\";\n-\t}\n-\tif ($unmatched) {\n-\t\tprint \"#unmatched\\n\";\n-\t} else {\n-\t\tprint for (@out);\n-\t}\n-\t'\n-}\n-\n die_if_unmatched ()\n {\n \tif test \"$1\" = \"#unmatched\"\n@@ -532,7 +490,7 @@ cmd_foreach()\n \t# command in the subshell (and a recursive call to this function)\n \texec 3<&0\n \n-\tmodule_list |\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\"|\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n@@ -592,7 +550,7 @@ cmd_init()\n \t\tshift\n \tdone\n \n-\tmodule_list \"$@\" |\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\" \"$@\" |\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n@@ -674,7 +632,7 @@ cmd_deinit()\n \t\tdie \"$(eval_gettext \"Use '.' if you really want to deinitialize all submodules\")\"\n \tfi\n \n-\tmodule_list \"$@\" |\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\" \"$@\" |\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n@@ -790,7 +748,7 @@ cmd_update()\n \tfi\n \n \tcloned_modules=\n-\tmodule_list \"$@\" | {\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\" \"$@\" | {\n \terr=\n \twhile read mode sha1 stage sm_path\n \tdo\n@@ -1222,7 +1180,7 @@ cmd_status()\n \t\tshift\n \tdone\n \n-\tmodule_list \"$@\" |\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\" \"$@\" |\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n@@ -1299,7 +1257,7 @@ cmd_sync()\n \t\tesac\n \tdone\n \tcd_to_toplevel\n-\tmodule_list \"$@\" |\n+\tgit submodule--helper module_list --prefix \"$wt_prefix\" \"$@\" |\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\ndiff --git a/git.c b/git.c\nindex 55c327c..deecba0 100644\n--- a/git.c\n+++ b/git.c\n@@ -469,6 +469,7 @@ static struct cmd_struct commands[] = {\n \t{ \"stage\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"stripspace\", cmd_stripspace },\n+\t{ \"submodule--helper\", cmd_submodule__helper, RUN_SETUP },\n \t{ \"symbolic-ref\", cmd_symbolic_ref, RUN_SETUP },\n \t{ \"tag\", cmd_tag, RUN_SETUP },\n \t{ \"unpack-file\", cmd_unpack_file, RUN_SETUP },\n-- \n2.5.0.330.g130be8e.dirty\n"},{"id":"268246","messageId":"1439857323-21048-7-git-send-email-sbeller@google.com","threadId":"40118","inReplyTo":"1439857323-21048-1-git-send-email-sbeller@google.com","subject":"[PATCH 6/7] submodule: implement `module_name` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:22:02Z","receivedAt":"2015-08-18T00:22:02Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This implements the helper `module_name` in C instead of shell,\nyielding a nice performance boost.\n\nBefore this patch, I measured a time (best out of three):\n\n $ time ./t7400-submodule-basic.sh >/dev/null\n real\t0m11.066s\n user\t0m3.348s\n sys\t0m8.534s\n\nWith this patch applied I measured (also best out of three)\n\n $ time ./t7400-submodule-basic.sh >/dev/null\n real\t0m10.063s\n user\t0m3.044s\n sys\t0m7.487s\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 28 +++++++++++++++++++++++++++-\n git-submodule.sh | 32 +++++++-------------------------\n 2 files changed, 34 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 610ec51..80d13f5 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -5,6 +5,9 @@\n #include \"pathspec.h\"\n #include \"dir.h\"\n #include \"utf8.h\"\n+#include \"submodule.h\"\n+#include \"submodule-config.h\"\n+#include \"string-list.h\"\n \n static const struct cache_entry **ce_entries;\n static int ce_alloc, ce_used;\n@@ -101,6 +104,26 @@ static int module_list(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int module_name(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *name;\n+\tconst struct submodule *sub;\n+\n+\tif (argc != 1)\n+\t\tusage(\"git submodule--helper module_name \\n\");\n+\n+\tgitmodules_config();\n+\tsub = submodule_from_path(null_sha1, argv[0]);\n+\n+\tif (!sub)\n+\t\tdie(\"No submodule mapping found in .gitmodules for path '%s'\", argv[0]);\n+\n+\tname = sub->name;\n+\tprintf(\"%s\\n\", name);\n+\n+\treturn 0;\n+}\n+\n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n {\n \tif (argc < 2)\n@@ -109,6 +132,9 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[1], \"module_list\"))\n \t\treturn module_list(argc - 1, argv + 1, prefix);\n \n+\tif (!strcmp(argv[1], \"module_name\"))\n+\t\treturn module_name(argc - 2, argv + 2, prefix);\n+\n usage:\n-\tusage(\"git submodule--helper module_list\\n\");\n+\tusage(\"git submodule--helper [module_list module_name]\\n\");\n }\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex af9ecef..e6ff38d 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -178,24 +178,6 @@ get_submodule_config () {\n \tprintf '%s' \"${value:-$default}\"\n }\n \n-\n-#\n-# Map submodule path to submodule name\n-#\n-# $1 = path\n-#\n-module_name()\n-{\n-\t# Do we have \"submodule..path = $1\" defined in .gitmodules file?\n-\tsm_path=\"$1\"\n-\tre=$(printf '%s\\n' \"$1\" | sed -e 's/[].[^$\\\\*]/\\\\&/g')\n-\tname=$( git config -f .gitmodules --get-regexp '^submodule\\..*\\.path$' |\n-\t\tsed -n -e 's|^submodule\\.\\(.*\\)\\.path '\"$re\"'$|\\1|p' )\n-\ttest -z \"$name\" &&\n-\tdie \"$(eval_gettext \"No submodule mapping found in .gitmodules for path '\\$sm_path'\")\"\n-\tprintf '%s\\n' \"$name\"\n-}\n-\n #\n # Clone a submodule\n #\n@@ -498,7 +480,7 @@ cmd_foreach()\n \t\tthen\n \t\t\tdisplaypath=$(relative_path \"$sm_path\")\n \t\t\tsay \"$(eval_gettext \"Entering '\\$prefix\\$displaypath'\")\"\n-\t\t\tname=$(module_name \"$sm_path\")\n+\t\t\tname=$(git submodule--helper module_name \"$sm_path\")\n \t\t\t(\n \t\t\t\tprefix=\"$prefix$sm_path/\"\n \t\t\t\tclear_local_git_env\n@@ -554,7 +536,7 @@ cmd_init()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \n \t\tdisplaypath=$(relative_path \"$sm_path\")\n \n@@ -636,7 +618,7 @@ cmd_deinit()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \n \t\tdisplaypath=$(relative_path \"$sm_path\")\n \n@@ -758,7 +740,7 @@ cmd_update()\n \t\t\techo >&2 \"Skipping unmerged submodule $prefix$sm_path\"\n \t\t\tcontinue\n \t\tfi\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \t\turl=$(git config submodule.\"$name\".url)\n \t\tbranch=$(get_submodule_config \"$name\" branch master)\n \t\tif ! test -z \"$update\"\n@@ -1022,7 +1004,7 @@ cmd_summary() {\n \t\t\t# Respect the ignore setting for --for-status.\n \t\t\tif test -n \"$for_status\"\n \t\t\tthen\n-\t\t\t\tname=$(module_name \"$sm_path\")\n+\t\t\t\tname=$(git submodule--helper module_name \"$sm_path\")\n \t\t\t\tignore_config=$(get_submodule_config \"$name\" ignore none)\n \t\t\t\ttest $status != A && test $ignore_config = all && continue\n \t\t\tfi\n@@ -1184,7 +1166,7 @@ cmd_status()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\") || exit\n+\t\tname=$(git submodule--helper module_name \"$sm_path\") || exit\n \t\turl=$(git config submodule.\"$name\".url)\n \t\tdisplaypath=$(relative_path \"$prefix$sm_path\")\n \t\tif test \"$stage\" = U\n@@ -1261,7 +1243,7 @@ cmd_sync()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(module_name \"$sm_path\")\n+\t\tname=$(git submodule--helper module_name \"$sm_path\")\n \t\turl=$(git config -f .gitmodules --get submodule.\"$name\".url)\n \n \t\t# Possibly a url relative to parent\n-- \n2.5.0.330.g130be8e.dirty\n"},{"id":"268245","messageId":"1439857323-21048-8-git-send-email-sbeller@google.com","threadId":"40118","inReplyTo":"1439857323-21048-1-git-send-email-sbeller@google.com","subject":"[PATCH 7/7] submodule: implement `module_clone` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:22:03Z","receivedAt":"2015-08-18T00:22:03Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"`module_clone` is part of the update command, which I want to convert\nto C next.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 161 +++++++++++++++++++++++++++++++++++++++++++-\n git-submodule.sh | 78 +--------------------\n 2 files changed, 162 insertions(+), 77 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 80d13f5..44ff935 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -8,6 +8,7 @@\n #include \"submodule.h\"\n #include \"submodule-config.h\"\n #include \"string-list.h\"\n+#include \"run-command.h\"\n \n static const struct cache_entry **ce_entries;\n static int ce_alloc, ce_used;\n@@ -124,6 +125,156 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int clone_submodule(const char *path, const char *gitdir, const char *url,\n+\t\t\t const char *depth, const char *reference, int quiet)\n+{\n+\tstruct child_process cp;\n+\tchild_process_init(&cp);\n+\n+\targv_array_push(&cp.args, \"clone\");\n+\targv_array_push(&cp.args, \"--no-checkout\");\n+\tif (quiet)\n+\t\targv_array_push(&cp.args, \"--quiet\");\n+\tif (depth && strcmp(depth, \"\")) {\n+\t\targv_array_push(&cp.args, \"--depth\");\n+\t\targv_array_push(&cp.args, depth);\n+\t}\n+\tif (reference && strcmp(reference, \"\")) {\n+\t\targv_array_push(&cp.args, \"--reference\");\n+\t\targv_array_push(&cp.args, reference);\n+\t}\n+\tif (gitdir) {\n+\t\targv_array_push(&cp.args, \"--separate-git-dir\");\n+\t\targv_array_push(&cp.args, gitdir);\n+\t}\n+\targv_array_push(&cp.args, url);\n+\targv_array_push(&cp.args, path);\n+\n+\tcp.git_cmd = 1;\n+\tcp.env = local_repo_env;\n+\n+\tcp.no_stdin = 1;\n+\tcp.no_stdout = 1;\n+\tcp.no_stderr = 1;\n+\treturn run_command(&cp);\n+}\n+\n+/*\n+ * Clone a submodule\n+ *\n+ * $1 = submodule path\n+ * $2 = submodule name\n+ * $3 = URL to clone\n+ * $4 = reference repository to reuse (empty for independent)\n+ * $5 = depth argument for shallow clones (empty for deep)\n+ *\n+ * Prior to calling, cmd_update checks that a possibly existing\n+ * path is not a git repository.\n+ * Likewise, cmd_add checks that path does not exist at all,\n+ * since it is the location of a new submodule.\n+ */\n+static int module_clone(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *path = NULL, *name = NULL, *url = NULL, *reference = NULL, *depth = NULL;\n+\tint quiet = 0;\n+\tFILE *submodule_dot_git;\n+\tconst char *sm_gitdir, *p;\n+\tstruct strbuf rel_path = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tstruct option module_update_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &alternative_path,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"alternative anchor for relative paths\")),\n+\t\tOPT_STRING(0, \"path\", &path,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"where the new submodule will be cloned to\")),\n+\t\tOPT_STRING(0, \"name\", &name,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"name of the new submodule\")),\n+\t\tOPT_STRING(0, \"url\", &url,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"url where to clone the submodule from\")),\n+\t\tOPT_STRING(0, \"reference\", &reference,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"depth\", &depth,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"depth for shallow clones\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tstatic const char * const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper update [--prefix=] [--quiet] [--remote] [-N|--no-fetch]\"\n+\t\t \"[-f|--force] [--rebase|--merge] [--reference ]\"\n+\t\t \"[--depth ] [--recursive] [--] [...]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_update_options,\n+\t\t\t git_submodule_helper_usage, 0);\n+\n+\tif (getenv(\"GIT_QUIET\"))\n+\t\tquiet = 1;\n+\n+\tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n+\tsm_gitdir = strbuf_detach(&sb, NULL);\n+\tstrbuf_reset(&sb);\n+\n+\tif (!file_exists(sm_gitdir)) {\n+\t\tsafe_create_leading_directories_const(sm_gitdir);\n+\t\tif (clone_submodule(path, sm_gitdir, url, depth, reference, quiet))\n+\t\t\tdie(N_(\"Clone of '%s' into submodule path '%s' failed\"),\n+\t\t\t url, path);\n+\t} else {\n+\t\tsafe_create_leading_directories_const(path);\n+\t\tunlink(sm_gitdir);\n+\t}\n+\n+\t/* Write a .git file in the submodule to redirect to the superproject. */\n+\tconst char *t;\n+\tif (alternative_path && !strcmp(alternative_path, \"\")) {\n+\t\tt = relative_path(path, alternative_path, &sb);\n+\t\tstrbuf_reset(&sb);\n+\t} else\n+\t\tt = path;\n+\n+\tif (safe_create_leading_directories_const(t) < 0)\n+\t\tdie(\"Could not create directory '%s'\", t);\n+\n+\tstrbuf_addf(&sb, \"%s/.git\", t);\n+\n+\tif (safe_create_leading_directories_const(sb.buf) < 0)\n+\t\tdie(_(\"could not create leading directories of '%s'\"), sb.buf);\n+\tsubmodule_dot_git = fopen(sb.buf, \"w\");\n+\tif (!submodule_dot_git)\n+\t\tdie (\"Cannot open file '%s': %s\", sb.buf, strerror(errno)\t);\n+\n+\tfprintf(submodule_dot_git, \"gitdir: %s\\n\",\n+\t\trelative_path(sm_gitdir, path, &rel_path));\n+\tif (fclose(submodule_dot_git))\n+\t\tdie(\"Could not close file %s\", sb.buf);\n+\tstrbuf_reset(&sb);\n+\n+\t/* Redirect the worktree of the submodule in the superprojects config */\n+\tif (!is_absolute_path(sm_gitdir)) {\n+\t\tchar *s = (char*)sm_gitdir;\n+\t\tstrbuf_addf(&sb, \"%s/%s\", xgetcwd(), sm_gitdir);\n+\t\tsm_gitdir = strbuf_detach(&sb, NULL);\n+\t\tstrbuf_reset(&sb);\n+\t\tfree(s);\n+\t}\n+\tstrbuf_addf(&sb, \"%s/%s\", xgetcwd(), path);\n+\tt = relative_path(sb.buf, sm_gitdir, &rel_path);\n+\n+\tp = git_pathdup_submodule(path, \"config\");\n+\tif (!p)\n+\t\tdie(\"Could not get submodule directory for '%s'\", path);\n+\tgit_config_set_in_file(p, \"core.worktree\", t);\n+\tstrbuf_release(&sb);\n+\treturn 0;\n+}\n+\n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n {\n \tif (argc < 2)\n@@ -135,6 +286,14 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[1], \"module_name\"))\n \t\treturn module_name(argc - 2, argv + 2, prefix);\n \n+\tif (!strcmp(argv[1], \"module_clone\"))\n+\t\treturn module_clone(argc - 1, argv + 1, prefix);\n+\tif (!strcmp(argv[1], \"test\")) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\trelative_path(\".git/modules/example\", \"init\", &sb);\n+\t\treturn 0;\n+\t}\n usage:\n-\tusage(\"git submodule--helper [module_list module_name]\\n\");\n+\tusage(\"git submodule--helper [module_list module_name module_clone]\\n\");\n }\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex e6ff38d..6e8561e 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -178,80 +178,6 @@ get_submodule_config () {\n \tprintf '%s' \"${value:-$default}\"\n }\n \n-#\n-# Clone a submodule\n-#\n-# $1 = submodule path\n-# $2 = submodule name\n-# $3 = URL to clone\n-# $4 = reference repository to reuse (empty for independent)\n-# $5 = depth argument for shallow clones (empty for deep)\n-#\n-# Prior to calling, cmd_update checks that a possibly existing\n-# path is not a git repository.\n-# Likewise, cmd_add checks that path does not exist at all,\n-# since it is the location of a new submodule.\n-#\n-module_clone()\n-{\n-\tsm_path=$1\n-\tname=$2\n-\turl=$3\n-\treference=\"$4\"\n-\tdepth=\"$5\"\n-\tquiet=\n-\tif test -n \"$GIT_QUIET\"\n-\tthen\n-\t\tquiet=-q\n-\tfi\n-\n-\tgitdir=\n-\tgitdir_base=\n-\tbase_name=$(dirname \"$name\")\n-\n-\tgitdir=$(git rev-parse --git-dir)\n-\tgitdir_base=\"$gitdir/modules/$base_name\"\n-\tgitdir=\"$gitdir/modules/$name\"\n-\n-\tif test -d \"$gitdir\"\n-\tthen\n-\t\tmkdir -p \"$sm_path\"\n-\t\trm -f \"$gitdir/index\"\n-\telse\n-\t\tmkdir -p \"$gitdir_base\"\n-\t\t(\n-\t\t\tclear_local_git_env\n-\t\t\tgit clone $quiet ${depth:+\"$depth\"} -n ${reference:+\"$reference\"} \\\n-\t\t\t\t--separate-git-dir \"$gitdir\" \"$url\" \"$sm_path\"\n-\t\t) ||\n-\t\tdie \"$(eval_gettext \"Clone of '\\$url' into submodule path '\\$sm_path' failed\")\"\n-\tfi\n-\n-\t# We already are at the root of the work tree but cd_to_toplevel will\n-\t# resolve any symlinks that might be present in $PWD\n-\ta=$(cd_to_toplevel && cd \"$gitdir\" && pwd)/\n-\tb=$(cd_to_toplevel && cd \"$sm_path\" && pwd)/\n-\t# Remove all common leading directories after a sanity check\n-\tif test \"${a#$b}\" != \"$a\" || test \"${b#$a}\" != \"$b\"; then\n-\t\tdie \"$(eval_gettext \"Gitdir '\\$a' is part of the submodule path '\\$b' or vice versa\")\"\n-\tfi\n-\twhile test \"${a%%/*}\" = \"${b%%/*}\"\n-\tdo\n-\t\ta=${a#*/}\n-\t\tb=${b#*/}\n-\tdone\n-\t# Now chop off the trailing '/'s that were added in the beginning\n-\ta=${a%/}\n-\tb=${b%/}\n-\n-\t# Turn each leading \"*/\" component into \"../\"\n-\trel=$(printf '%s\\n' \"$b\" | sed -e 's|[^/][^/]*|..|g')\n-\tprintf '%s\\n' \"gitdir: $rel/$a\" >\"$sm_path/.git\"\n-\n-\trel=$(printf '%s\\n' \"$a\" | sed -e 's|[^/][^/]*|..|g')\n-\t(clear_local_git_env; cd \"$sm_path\" && GIT_WORK_TREE=. git config core.worktree \"$rel/$b\")\n-}\n-\n isnumber()\n {\n \tn=$(($1 + 0)) 2>/dev/null && test \"$n\" = \"$1\"\n@@ -412,7 +338,7 @@ Use -f if you really want to add it.\" >&2\n \t\t\t\techo \"$(eval_gettext \"Reactivating local git directory for submodule '\\$sm_name'.\")\"\n \t\t\tfi\n \t\tfi\n-\t\tmodule_clone \"$sm_path\" \"$sm_name\" \"$realrepo\" \"$reference\" \"$depth\" || exit\n+\t\tgit submodule--helper module_clone --prefix \"$wt_prefix\" --path \"$sm_path\" --name \"$sm_name\" --url \"$realrepo\" --reference \"$reference_path\" --depth \"$depth\" || exit\n \t\t(\n \t\t\tclear_local_git_env\n \t\t\tcd \"$sm_path\" &&\n@@ -774,7 +700,7 @@ Maybe you want to use 'update --init'?\")\"\n \n \t\tif ! test -d \"$sm_path\"/.git && ! test -f \"$sm_path\"/.git\n \t\tthen\n-\t\t\tmodule_clone \"$sm_path\" \"$name\" \"$url\" \"$reference\" \"$depth\" || exit\n+\t\t\tgit submodule--helper module_clone --prefix \"$prefix\" --path \"$sm_path\" --name \"$name\" --url \"$url\" \"$reference\" \"$depth\" || exit\n \t\t\tcloned_modules=\"$cloned_modules;$name\"\n \t\t\tsubsha1=\n \t\telse\n-- \n2.5.0.330.g130be8e.dirty\n"},{"id":"268248","messageId":"CAGZ79kZYk8_266eAtmW72XWNP7xAvQTkdLS8-XTSj_NioiN2-g@mail.gmail.com","threadId":"40118","inReplyTo":"1439857323-21048-8-git-send-email-sbeller@google.com","subject":"Re: [PATCH 7/7] submodule: implement `module_clone` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:26:11Z","receivedAt":"2015-08-18T00:26:11Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Aug 17, 2015 at 5:22 PM, Stefan Beller wrote:\n> `module_clone` is part of the update command, which I want to convert\n> to C next.\n>\n> Signed-off-by: Stefan Beller \n> ---\n> builtin/submodule--helper.c | 161 +++++++++++++++++++++++++++++++++++++++++++-\n> git-submodule.sh | 78 +--------------------\n> 2 files changed, 162 insertions(+), 77 deletions(-)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 80d13f5..44ff935 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -8,6 +8,7 @@\n> #include \"submodule.h\"\n> #include \"submodule-config.h\"\n> #include \"string-list.h\"\n> +#include \"run-command.h\"\n>\n> static const struct cache_entry **ce_entries;\n> static int ce_alloc, ce_used;\n> @@ -124,6 +125,156 @@ static int module_name(int argc, const char **argv, const char *prefix)\n> return 0;\n> }\n>\n> +static int clone_submodule(const char *path, const char *gitdir, const char *url,\n> + const char *depth, const char *reference, int quiet)\n> +{\n> + struct child_process cp;\n> + child_process_init(&cp);\n> +\n> + argv_array_push(&cp.args, \"clone\");\n> + argv_array_push(&cp.args, \"--no-checkout\");\n> + if (quiet)\n> + argv_array_push(&cp.args, \"--quiet\");\n> + if (depth && strcmp(depth, \"\")) {\n> + argv_array_push(&cp.args, \"--depth\");\n> + argv_array_push(&cp.args, depth);\n> + }\n> + if (reference && strcmp(reference, \"\")) {\n> + argv_array_push(&cp.args, \"--reference\");\n> + argv_array_push(&cp.args, reference);\n> + }\n> + if (gitdir) {\n> + argv_array_push(&cp.args, \"--separate-git-dir\");\n> + argv_array_push(&cp.args, gitdir);\n> + }\n> + argv_array_push(&cp.args, url);\n> + argv_array_push(&cp.args, path);\n> +\n> + cp.git_cmd = 1;\n> + cp.env = local_repo_env;\n> +\n> + cp.no_stdin = 1;\n> + cp.no_stdout = 1;\n> + cp.no_stderr = 1;\n> + return run_command(&cp);\n> +}\n> +\n> +/*\n> + * Clone a submodule\n> + *\n> + * $1 = submodule path\n> + * $2 = submodule name\n> + * $3 = URL to clone\n> + * $4 = reference repository to reuse (empty for independent)\n> + * $5 = depth argument for shallow clones (empty for deep)\n> + *\n> + * Prior to calling, cmd_update checks that a possibly existing\n> + * path is not a git repository.\n> + * Likewise, cmd_add checks that path does not exist at all,\n> + * since it is the location of a new submodule.\n> + */\n> +static int module_clone(int argc, const char **argv, const char *prefix)\n> +{\n> + const char *path = NULL, *name = NULL, *url = NULL, *reference = NULL, *depth = NULL;\n> + int quiet = 0;\n> + FILE *submodule_dot_git;\n> + const char *sm_gitdir, *p;\n> + struct strbuf rel_path = STRBUF_INIT;\n> + struct strbuf sb = STRBUF_INIT;\n> +\n> + struct option module_update_options[] = {\n> + OPT_STRING(0, \"prefix\", &alternative_path,\n> + N_(\"path\"),\n> + N_(\"alternative anchor for relative paths\")),\n> + OPT_STRING(0, \"path\", &path,\n> + N_(\"path\"),\n> + N_(\"where the new submodule will be cloned to\")),\n> + OPT_STRING(0, \"name\", &name,\n> + N_(\"string\"),\n> + N_(\"name of the new submodule\")),\n> + OPT_STRING(0, \"url\", &url,\n> + N_(\"string\"),\n> + N_(\"url where to clone the submodule from\")),\n> + OPT_STRING(0, \"reference\", &reference,\n> + N_(\"string\"),\n> + N_(\"reference repository\")),\n> + OPT_STRING(0, \"depth\", &depth,\n> + N_(\"string\"),\n> + N_(\"depth for shallow clones\")),\n> + OPT_END()\n> + };\n> +\n> + static const char * const git_submodule_helper_usage[] = {\n> + N_(\"git submodule--helper update [--prefix=] [--quiet] [--remote] [-N|--no-fetch]\"\n> + \"[-f|--force] [--rebase|--merge] [--reference ]\"\n> + \"[--depth ] [--recursive] [--] [...]\"),\n> + NULL\n> + };\n> +\n> + argc = parse_options(argc, argv, prefix, module_update_options,\n> + git_submodule_helper_usage, 0);\n> +\n> + if (getenv(\"GIT_QUIET\"))\n> + quiet = 1;\n> +\n> + strbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n> + sm_gitdir = strbuf_detach(&sb, NULL);\n> + strbuf_reset(&sb);\n> +\n> + if (!file_exists(sm_gitdir)) {\n> + safe_create_leading_directories_const(sm_gitdir);\n> + if (clone_submodule(path, sm_gitdir, url, depth, reference, quiet))\n> + die(N_(\"Clone of '%s' into submodule path '%s' failed\"),\n> + url, path);\n> + } else {\n> + safe_create_leading_directories_const(path);\n> + unlink(sm_gitdir);\n> + }\n> +\n> + /* Write a .git file in the submodule to redirect to the superproject. */\n> + const char *t;\n\ndeclaration after statement.\n\n> + if (alternative_path && !strcmp(alternative_path, \"\")) {\n> + t = relative_path(path, alternative_path, &sb);\n> + strbuf_reset(&sb);\n> + } else\n> + t = path;\n> +\n> + if (safe_create_leading_directories_const(t) < 0)\n> + die(\"Could not create directory '%s'\", t);\n> +\n> + strbuf_addf(&sb, \"%s/.git\", t);\n> +\n> + if (safe_create_leading_directories_const(sb.buf) < 0)\n> + die(_(\"could not create leading directories of '%s'\"), sb.buf);\n> + submodule_dot_git = fopen(sb.buf, \"w\");\n> + if (!submodule_dot_git)\n> + die (\"Cannot open file '%s': %s\", sb.buf, strerror(errno) );\n> +\n> + fprintf(submodule_dot_git, \"gitdir: %s\\n\",\n> + relative_path(sm_gitdir, path, &rel_path));\n> + if (fclose(submodule_dot_git))\n> + die(\"Could not close file %s\", sb.buf);\n> + strbuf_reset(&sb);\n> +\n> + /* Redirect the worktree of the submodule in the superprojects config */\n> + if (!is_absolute_path(sm_gitdir)) {\n> + char *s = (char*)sm_gitdir;\n> + strbuf_addf(&sb, \"%s/%s\", xgetcwd(), sm_gitdir);\n> + sm_gitdir = strbuf_detach(&sb, NULL);\n> + strbuf_reset(&sb);\n> + free(s);\n> + }\n> + strbuf_addf(&sb, \"%s/%s\", xgetcwd(), path);\n> + t = relative_path(sb.buf, sm_gitdir, &rel_path);\n> +\n> + p = git_pathdup_submodule(path, \"config\");\n> + if (!p)\n> + die(\"Could not get submodule directory for '%s'\", path);\n> + git_config_set_in_file(p, \"core.worktree\", t);\n> + strbuf_release(&sb);\n> + return 0;\n> +}\n> +\n> int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n> {\n> if (argc < 2)\n> @@ -135,6 +286,14 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n> if (!strcmp(argv[1], \"module_name\"))\n> return module_name(argc - 2, argv + 2, prefix);\n>\n> + if (!strcmp(argv[1], \"module_clone\"))\n> + return module_clone(argc - 1, argv + 1, prefix);\n> + if (!strcmp(argv[1], \"test\")) {\n> + struct strbuf sb = STRBUF_INIT;\n> +\n> + relative_path(\".git/modules/example\", \"init\", &sb);\n> + return 0;\n> + }\n> usage:\n> - usage(\"git submodule--helper [module_list module_name]\\n\");\n> + usage(\"git submodule--helper [module_list module_name module_clone]\\n\");\n> }\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index e6ff38d..6e8561e 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -178,80 +178,6 @@ get_submodule_config () {\n> printf '%s' \"${value:-$default}\"\n> }\n>\n> -#\n> -# Clone a submodule\n> -#\n> -# $1 = submodule path\n> -# $2 = submodule name\n> -# $3 = URL to clone\n> -# $4 = reference repository to reuse (empty for independent)\n> -# $5 = depth argument for shallow clones (empty for deep)\n> -#\n> -# Prior to calling, cmd_update checks that a possibly existing\n> -# path is not a git repository.\n> -# Likewise, cmd_add checks that path does not exist at all,\n> -# since it is the location of a new submodule.\n> -#\n> -module_clone()\n> -{\n> - sm_path=$1\n> - name=$2\n> - url=$3\n> - reference=\"$4\"\n> - depth=\"$5\"\n> - quiet=\n> - if test -n \"$GIT_QUIET\"\n> - then\n> - quiet=-q\n> - fi\n> -\n> - gitdir=\n> - gitdir_base=\n> - base_name=$(dirname \"$name\")\n> -\n> - gitdir=$(git rev-parse --git-dir)\n> - gitdir_base=\"$gitdir/modules/$base_name\"\n> - gitdir=\"$gitdir/modules/$name\"\n> -\n> - if test -d \"$gitdir\"\n> - then\n> - mkdir -p \"$sm_path\"\n> - rm -f \"$gitdir/index\"\n> - else\n> - mkdir -p \"$gitdir_base\"\n> - (\n> - clear_local_git_env\n> - git clone $quiet ${depth:+\"$depth\"} -n ${reference:+\"$reference\"} \\\n> - --separate-git-dir \"$gitdir\" \"$url\" \"$sm_path\"\n> - ) ||\n> - die \"$(eval_gettext \"Clone of '\\$url' into submodule path '\\$sm_path' failed\")\"\n> - fi\n> -\n> - # We already are at the root of the work tree but cd_to_toplevel will\n> - # resolve any symlinks that might be present in $PWD\n> - a=$(cd_to_toplevel && cd \"$gitdir\" && pwd)/\n> - b=$(cd_to_toplevel && cd \"$sm_path\" && pwd)/\n> - # Remove all common leading directories after a sanity check\n> - if test \"${a#$b}\" != \"$a\" || test \"${b#$a}\" != \"$b\"; then\n> - die \"$(eval_gettext \"Gitdir '\\$a' is part of the submodule path '\\$b' or vice versa\")\"\n> - fi\n> - while test \"${a%%/*}\" = \"${b%%/*}\"\n> - do\n> - a=${a#*/}\n> - b=${b#*/}\n> - done\n> - # Now chop off the trailing '/'s that were added in the beginning\n> - a=${a%/}\n> - b=${b%/}\n> -\n> - # Turn each leading \"*/\" component into \"../\"\n> - rel=$(printf '%s\\n' \"$b\" | sed -e 's|[^/][^/]*|..|g')\n> - printf '%s\\n' \"gitdir: $rel/$a\" >\"$sm_path/.git\"\n> -\n> - rel=$(printf '%s\\n' \"$a\" | sed -e 's|[^/][^/]*|..|g')\n> - (clear_local_git_env; cd \"$sm_path\" && GIT_WORK_TREE=. git config core.worktree \"$rel/$b\")\n> -}\n> -\n> isnumber()\n> {\n> n=$(($1 + 0)) 2>/dev/null && test \"$n\" = \"$1\"\n> @@ -412,7 +338,7 @@ Use -f if you really want to add it.\" >&2\n> echo \"$(eval_gettext \"Reactivating local git directory for submodule '\\$sm_name'.\")\"\n> fi\n> fi\n> - module_clone \"$sm_path\" \"$sm_name\" \"$realrepo\" \"$reference\" \"$depth\" || exit\n> + git submodule--helper module_clone --prefix \"$wt_prefix\" --path \"$sm_path\" --name \"$sm_name\" --url \"$realrepo\" --reference \"$reference_path\" --depth \"$depth\" || exit\n> (\n> clear_local_git_env\n> cd \"$sm_path\" &&\n> @@ -774,7 +700,7 @@ Maybe you want to use 'update --init'?\")\"\n>\n> if ! test -d \"$sm_path\"/.git && ! test -f \"$sm_path\"/.git\n> then\n> - module_clone \"$sm_path\" \"$name\" \"$url\" \"$reference\" \"$depth\" || exit\n> + git submodule--helper module_clone --prefix \"$prefix\" --path \"$sm_path\" --name \"$name\" --url \"$url\" \"$reference\" \"$depth\" || exit\n> cloned_modules=\"$cloned_modules;$name\"\n> subsha1=\n> else\n> --\n> 2.5.0.330.g130be8e.dirty\n>\n"},{"id":"268249","messageId":"CAGZ79kYqCLQYt7Aosr57rkfHhqVVttuoWGwT4n-Nx7jU0+L2_A@mail.gmail.com","threadId":"40118","inReplyTo":"1439857323-21048-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 0/7] Submodule improvements","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T00:38:05Z","receivedAt":"2015-08-18T00:38:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"A last minute change in the last patch broke t7400-submodule-basic.sh, test 82\n"},{"id":"268288","messageId":"1439935980-30087-1-git-send-email-sbeller@google.com","threadId":"40118","inReplyTo":"1439857323-21048-8-git-send-email-sbeller@google.com","subject":"[PATCH 7/7] submodule: implement `module_clone` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-18T22:13:00Z","receivedAt":"2015-08-18T22:13:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"`module_clone` is part of the update command, which I want to convert\nto C next.\n\nSigned-off-by: Stefan Beller \n---\n\nThis is the only patch that broke by a last minute fix,\nso this replaces the previous PATCH 7/7.\n\n builtin/submodule--helper.c | 161 +++++++++++++++++++++++++++++++++++++++++++-\n git-submodule.sh | 80 +---------------------\n 2 files changed, 163 insertions(+), 78 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 80d13f5..930aa04 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -8,6 +8,7 @@\n #include \"submodule.h\"\n #include \"submodule-config.h\"\n #include \"string-list.h\"\n+#include \"run-command.h\"\n \n static const struct cache_entry **ce_entries;\n static int ce_alloc, ce_used;\n@@ -124,6 +125,156 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int clone_submodule(const char *path, const char *gitdir, const char *url,\n+\t\t\t const char *depth, const char *reference, int quiet)\n+{\n+\tstruct child_process cp;\n+\tchild_process_init(&cp);\n+\n+\targv_array_push(&cp.args, \"clone\");\n+\targv_array_push(&cp.args, \"--no-checkout\");\n+\tif (quiet)\n+\t\targv_array_push(&cp.args, \"--quiet\");\n+\tif (depth && strcmp(depth, \"\")) {\n+\t\targv_array_push(&cp.args, \"--depth\");\n+\t\targv_array_push(&cp.args, depth);\n+\t}\n+\tif (reference && strcmp(reference, \"\")) {\n+\t\targv_array_push(&cp.args, \"--reference\");\n+\t\targv_array_push(&cp.args, reference);\n+\t}\n+\tif (gitdir) {\n+\t\targv_array_push(&cp.args, \"--separate-git-dir\");\n+\t\targv_array_push(&cp.args, gitdir);\n+\t}\n+\targv_array_push(&cp.args, url);\n+\targv_array_push(&cp.args, path);\n+\n+\tcp.git_cmd = 1;\n+\tcp.env = local_repo_env;\n+\n+\tcp.no_stdin = 1;\n+\tcp.no_stdout = 1;\n+\tcp.no_stderr = 1;\n+\n+\treturn run_command(&cp);\n+}\n+\n+/*\n+ * Clone a submodule\n+ *\n+ * $1 = submodule path\n+ * $2 = submodule name\n+ * $3 = URL to clone\n+ * $4 = reference repository to reuse (empty for independent)\n+ * $5 = depth argument for shallow clones (empty for deep)\n+ *\n+ * Prior to calling, cmd_update checks that a possibly existing\n+ * path is not a git repository.\n+ * Likewise, cmd_add checks that path does not exist at all,\n+ * since it is the location of a new submodule.\n+ */\n+static int module_clone(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *path = NULL, *name = NULL, *url = NULL, *reference = NULL, *depth = NULL;\n+\tint quiet = 0;\n+\tFILE *submodule_dot_git;\n+\tconst char *sm_gitdir, *p;\n+\tstruct strbuf rel_path = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tstruct option module_update_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &alternative_path,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"alternative anchor for relative paths\")),\n+\t\tOPT_STRING(0, \"path\", &path,\n+\t\t\t N_(\"path\"),\n+\t\t\t N_(\"where the new submodule will be cloned to\")),\n+\t\tOPT_STRING(0, \"name\", &name,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"name of the new submodule\")),\n+\t\tOPT_STRING(0, \"url\", &url,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"url where to clone the submodule from\")),\n+\t\tOPT_STRING(0, \"reference\", &reference,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"reference repository\")),\n+\t\tOPT_STRING(0, \"depth\", &depth,\n+\t\t\t N_(\"string\"),\n+\t\t\t N_(\"depth for shallow clones\")),\n+\t\tOPT_END()\n+\t};\n+\n+\tstatic const char * const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper update [--prefix=] [--quiet] [--remote] [-N|--no-fetch]\"\n+\t\t \"[-f|--force] [--rebase|--merge] [--reference ]\"\n+\t\t \"[--depth ] [--recursive] [--] [...]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_update_options,\n+\t\t\t git_submodule_helper_usage, 0);\n+\n+\tif (getenv(\"GIT_QUIET\"))\n+\t\tquiet = 1;\n+\n+\tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n+\tsm_gitdir = strbuf_detach(&sb, NULL);\n+\tstrbuf_reset(&sb);\n+\n+\tif (!file_exists(sm_gitdir)) {\n+\t\tsafe_create_leading_directories_const(sm_gitdir);\n+\t\tif (clone_submodule(path, sm_gitdir, url, depth, reference, quiet))\n+\t\t\tdie(N_(\"Clone of '%s' into submodule path '%s' failed\"),\n+\t\t\t url, path);\n+\t} else {\n+\t\tsafe_create_leading_directories_const(path);\n+\t\tunlink(sm_gitdir);\n+\t}\n+\n+\t/* Write a .git file in the submodule to redirect to the superproject. */\n+\tif (alternative_path && !strcmp(alternative_path, \"\")) {\n+\t\tp = relative_path(path, alternative_path, &sb);\n+\t\tstrbuf_reset(&sb);\n+\t} else\n+\t\tp = path;\n+\n+\tif (safe_create_leading_directories_const(p) < 0)\n+\t\tdie(\"Could not create directory '%s'\", p);\n+\n+\tstrbuf_addf(&sb, \"%s/.git\", p);\n+\n+\tif (safe_create_leading_directories_const(sb.buf) < 0)\n+\t\tdie(_(\"could not create leading directories of '%s'\"), sb.buf);\n+\tsubmodule_dot_git = fopen(sb.buf, \"w\");\n+\tif (!submodule_dot_git)\n+\t\tdie (\"Cannot open file '%s': %s\", sb.buf, strerror(errno));\n+\n+\tfprintf(submodule_dot_git, \"gitdir: %s\\n\",\n+\t\trelative_path(sm_gitdir, path, &rel_path));\n+\tif (fclose(submodule_dot_git))\n+\t\tdie(\"Could not close file %s\", sb.buf);\n+\tstrbuf_reset(&sb);\n+\n+\t/* Redirect the worktree of the submodule in the superprojects config */\n+\tif (!is_absolute_path(sm_gitdir)) {\n+\t\tchar *s = (char*)sm_gitdir;\n+\t\tstrbuf_addf(&sb, \"%s/%s\", xgetcwd(), sm_gitdir);\n+\t\tsm_gitdir = strbuf_detach(&sb, NULL);\n+\t\tstrbuf_reset(&sb);\n+\t\tfree(s);\n+\t}\n+\tstrbuf_addf(&sb, \"%s/%s\", xgetcwd(), path);\n+\n+\tp = git_pathdup_submodule(path, \"config\");\n+\tif (!p)\n+\t\tdie(\"Could not get submodule directory for '%s'\", path);\n+\tgit_config_set_in_file(p, \"core.worktree\",\n+\t\t\t relative_path(sb.buf, sm_gitdir, &rel_path));\n+\tstrbuf_release(&sb);\n+\treturn 0;\n+}\n+\n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n {\n \tif (argc < 2)\n@@ -135,6 +286,14 @@ int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(argv[1], \"module_name\"))\n \t\treturn module_name(argc - 2, argv + 2, prefix);\n \n+\tif (!strcmp(argv[1], \"module_clone\"))\n+\t\treturn module_clone(argc - 1, argv + 1, prefix);\n+\tif (!strcmp(argv[1], \"test\")) {\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\trelative_path(\".git/modules/example\", \"init\", &sb);\n+\t\treturn 0;\n+\t}\n usage:\n-\tusage(\"git submodule--helper [module_list module_name]\\n\");\n+\tusage(\"git submodule--helper [module_list module_name module_clone]\\n\");\n }\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex e6ff38d..e50451b 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -178,80 +178,6 @@ get_submodule_config () {\n \tprintf '%s' \"${value:-$default}\"\n }\n \n-#\n-# Clone a submodule\n-#\n-# $1 = submodule path\n-# $2 = submodule name\n-# $3 = URL to clone\n-# $4 = reference repository to reuse (empty for independent)\n-# $5 = depth argument for shallow clones (empty for deep)\n-#\n-# Prior to calling, cmd_update checks that a possibly existing\n-# path is not a git repository.\n-# Likewise, cmd_add checks that path does not exist at all,\n-# since it is the location of a new submodule.\n-#\n-module_clone()\n-{\n-\tsm_path=$1\n-\tname=$2\n-\turl=$3\n-\treference=\"$4\"\n-\tdepth=\"$5\"\n-\tquiet=\n-\tif test -n \"$GIT_QUIET\"\n-\tthen\n-\t\tquiet=-q\n-\tfi\n-\n-\tgitdir=\n-\tgitdir_base=\n-\tbase_name=$(dirname \"$name\")\n-\n-\tgitdir=$(git rev-parse --git-dir)\n-\tgitdir_base=\"$gitdir/modules/$base_name\"\n-\tgitdir=\"$gitdir/modules/$name\"\n-\n-\tif test -d \"$gitdir\"\n-\tthen\n-\t\tmkdir -p \"$sm_path\"\n-\t\trm -f \"$gitdir/index\"\n-\telse\n-\t\tmkdir -p \"$gitdir_base\"\n-\t\t(\n-\t\t\tclear_local_git_env\n-\t\t\tgit clone $quiet ${depth:+\"$depth\"} -n ${reference:+\"$reference\"} \\\n-\t\t\t\t--separate-git-dir \"$gitdir\" \"$url\" \"$sm_path\"\n-\t\t) ||\n-\t\tdie \"$(eval_gettext \"Clone of '\\$url' into submodule path '\\$sm_path' failed\")\"\n-\tfi\n-\n-\t# We already are at the root of the work tree but cd_to_toplevel will\n-\t# resolve any symlinks that might be present in $PWD\n-\ta=$(cd_to_toplevel && cd \"$gitdir\" && pwd)/\n-\tb=$(cd_to_toplevel && cd \"$sm_path\" && pwd)/\n-\t# Remove all common leading directories after a sanity check\n-\tif test \"${a#$b}\" != \"$a\" || test \"${b#$a}\" != \"$b\"; then\n-\t\tdie \"$(eval_gettext \"Gitdir '\\$a' is part of the submodule path '\\$b' or vice versa\")\"\n-\tfi\n-\twhile test \"${a%%/*}\" = \"${b%%/*}\"\n-\tdo\n-\t\ta=${a#*/}\n-\t\tb=${b#*/}\n-\tdone\n-\t# Now chop off the trailing '/'s that were added in the beginning\n-\ta=${a%/}\n-\tb=${b%/}\n-\n-\t# Turn each leading \"*/\" component into \"../\"\n-\trel=$(printf '%s\\n' \"$b\" | sed -e 's|[^/][^/]*|..|g')\n-\tprintf '%s\\n' \"gitdir: $rel/$a\" >\"$sm_path/.git\"\n-\n-\trel=$(printf '%s\\n' \"$a\" | sed -e 's|[^/][^/]*|..|g')\n-\t(clear_local_git_env; cd \"$sm_path\" && GIT_WORK_TREE=. git config core.worktree \"$rel/$b\")\n-}\n-\n isnumber()\n {\n \tn=$(($1 + 0)) 2>/dev/null && test \"$n\" = \"$1\"\n@@ -301,7 +227,7 @@ cmd_add()\n \t\t\tshift\n \t\t\t;;\n \t\t--depth=*)\n-\t\t\tdepth=$1\n+\t\t\tdepth=\"$1\"\n \t\t\t;;\n \t\t--)\n \t\t\tshift\n@@ -412,7 +338,7 @@ Use -f if you really want to add it.\" >&2\n \t\t\t\techo \"$(eval_gettext \"Reactivating local git directory for submodule '\\$sm_name'.\")\"\n \t\t\tfi\n \t\tfi\n-\t\tmodule_clone \"$sm_path\" \"$sm_name\" \"$realrepo\" \"$reference\" \"$depth\" || exit\n+\t\tgit submodule--helper module_clone --prefix \"$wt_prefix\" --path \"$sm_path\" --name \"$sm_name\" --url \"$realrepo\" --reference \"$reference_path\" --depth \"$depth\" || exit\n \t\t(\n \t\t\tclear_local_git_env\n \t\t\tcd \"$sm_path\" &&\n@@ -774,7 +700,7 @@ Maybe you want to use 'update --init'?\")\"\n \n \t\tif ! test -d \"$sm_path\"/.git && ! test -f \"$sm_path\"/.git\n \t\tthen\n-\t\t\tmodule_clone \"$sm_path\" \"$name\" \"$url\" \"$reference\" \"$depth\" || exit\n+\t\t\tgit submodule--helper module_clone --prefix \"$prefix\" --path \"$sm_path\" --name \"$name\" --url \"$url\" \"$reference\" \"$depth\" || exit\n \t\t\tcloned_modules=\"$cloned_modules;$name\"\n \t\t\tsubsha1=\n \t\telse\n-- \n2.5.0.342.g44e0223\n"},{"id":"268333","messageId":"xmqqtwrv9ml9.fsf@gitster.dls.corp.google.com","threadId":"40118","inReplyTo":"1439857323-21048-5-git-send-email-sbeller@google.com","subject":"Re: [PATCH 4/7] submodule: Allow errornous values for the fetchrecursesubmodules option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-19T18:09:38Z","receivedAt":"2015-08-19T18:09:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> From: Heiko Voigt \n>\n> We should not die when reading the submodule config cache since the user\n> might not be able to get out of that situation when the configuration is\n> part of the history.\n>\n> We should handle this condition later when the value is about to be\n> used.\n>\n> Signed-off-by: Heiko Voigt \n> Signed-off-by: Stefan Beller \n> ---\n\nThe retitle is good but we'd need to typofix erroneous there, I\nthink (will do so while queuing, no need to resend).\n"},{"id":"268334","messageId":"xmqqpp2j9m7g.fsf@gitster.dls.corp.google.com","threadId":"40118","inReplyTo":"1439857323-21048-6-git-send-email-sbeller@google.com","subject":"Re: [PATCH 5/7] submodule: implement `module_list` as a builtin helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-19T18:17:55Z","receivedAt":"2015-08-19T18:17:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> +static int module_list_compute(int argc, const char **argv,\n> +\t\t\t\tconst char *prefix,\n> +\t\t\t\tstruct pathspec *pathspec)\n> +{\n> +\tint i;\n> +\tchar *max_prefix, *ps_matched = NULL;\n> +\tint max_prefix_len;\n> +\tparse_pathspec(pathspec, 0,\n> +\t\t PATHSPEC_PREFER_FULL |\n> +\t\t PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n> +\t\t prefix, argv);\n> +\n> +\t/* Find common prefix for all pathspec's */\n> +\tmax_prefix = common_prefix(pathspec);\n> +\tmax_prefix_len = max_prefix ? strlen(max_prefix) : 0;\n> +\n> +\tif (pathspec->nr)\n> +\t\tps_matched = xcalloc(1, pathspec->nr);\n\nMicronit. Even though multiplication is commutative, the order of\narguments to xcalloc() looks odd. It lets you say \"I want an array\nwith nmemb elements, and each of its is size-bytes long\" by giving\nit nmemb and then size.\n\nNo need to resend; will fix it up while queuing.\n\nThanks.\n"},{"id":"268335","messageId":"xmqqfv3f9m0l.fsf@gitster.dls.corp.google.com","threadId":"40118","inReplyTo":"xmqqpp2j9m7g.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 5/7] submodule: implement `module_list` as a builtin helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-19T18:22:02Z","receivedAt":"2015-08-19T18:22:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Micronit. Even though multiplication is commutative, the order of\n> arguments to xcalloc() looks odd. It lets you say \"I want an array\n> with nmemb elements, and each of its is size-bytes long\" by giving\n> it nmemb and then size.\n\nUnrelated tangent, but while I have output from \"git grep 'calloc.*pathspec'\"\non my screen... ;-)\n\n builtin/checkout.c | 2 +-\n builtin/ls-files.c | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 7ea533e..e3b28e4 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -283,7 +283,7 @@ static int checkout_paths(const struct checkout_opts *opts,\n \tif (opts->source_tree)\n \t\tread_tree_some(opts->source_tree, &opts->pathspec);\n \n-\tps_matched = xcalloc(1, opts->pathspec.nr);\n+\tps_matched = xcalloc(opts->pathspec.nr, 1);\n \n \t/*\n \t * Make sure all pathspecs participated in locating the paths\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 6fa2205..b6a7cb0 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -516,7 +516,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \n \t/* Treat unmatching pathspec elements as errors */\n \tif (pathspec.nr && error_unmatch)\n-\t\tps_matched = xcalloc(1, pathspec.nr);\n+\t\tps_matched = xcalloc(pathspec.nr, 1);\n \n \tif ((dir.flags & DIR_SHOW_IGNORED) && !exc_given)\n \t\tdie(\"ls-files --ignored needs some exclude pattern\");\n"},{"id":"268336","messageId":"CAGZ79kbHVAqOFT5s-4RRtHjfBO=KgvMOmup0Q1OyNZ3kSFfvxw@mail.gmail.com","threadId":"40118","inReplyTo":"xmqqpp2j9m7g.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 5/7] submodule: implement `module_list` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-19T18:23:42Z","receivedAt":"2015-08-19T18:23:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 19, 2015 at 11:17 AM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> +static int module_list_compute(int argc, const char **argv,\n>> + const char *prefix,\n>> + struct pathspec *pathspec)\n>> +{\n>> + int i;\n>> + char *max_prefix, *ps_matched = NULL;\n>> + int max_prefix_len;\n>> + parse_pathspec(pathspec, 0,\n>> + PATHSPEC_PREFER_FULL |\n>> + PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,\n>> + prefix, argv);\n>> +\n>> + /* Find common prefix for all pathspec's */\n>> + max_prefix = common_prefix(pathspec);\n>> + max_prefix_len = max_prefix ? strlen(max_prefix) : 0;\n>> +\n>> + if (pathspec->nr)\n>> + ps_matched = xcalloc(1, pathspec->nr);\n>\n> Micronit. Even though multiplication is commutative, the order of\n> arguments to xcalloc() looks odd. It lets you say \"I want an array\n> with nmemb elements, and each of its is size-bytes long\" by giving\n> it nmemb and then size.\n\nThanks for pointing that out!\nI think this would be an alignment issue in theory, if the items were larger and\nthe number of items not dividable by the size of items.\n\nSo because 1 char has no alignment problems, we can happily allocate\njust one block of the member size here.\n\n>\n> No need to resend; will fix it up while queuing.\n>\n> Thanks.\n"},{"id":"268337","messageId":"CAGZ79kacYnaV-cF3jXzoCHm8OvJUpF305CumTXD0ozE2hhNRwA@mail.gmail.com","threadId":"40118","inReplyTo":"xmqqfv3f9m0l.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 5/7] submodule: implement `module_list` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-19T18:25:05Z","receivedAt":"2015-08-19T18:25:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 19, 2015 at 11:22 AM, Junio C Hamano wrote:\n> Junio C Hamano writes:\n>\n>> Micronit. Even though multiplication is commutative, the order of\n>> arguments to xcalloc() looks odd. It lets you say \"I want an array\n>> with nmemb elements, and each of its is size-bytes long\" by giving\n>> it nmemb and then size.\n>\n> Unrelated tangent, but while I have output from \"git grep 'calloc.*pathspec'\"\n> on my screen... ;-)\n>\n> builtin/checkout.c | 2 +-\n> builtin/ls-files.c | 2 +-\n> 2 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 7ea533e..e3b28e4 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -283,7 +283,7 @@ static int checkout_paths(const struct checkout_opts *opts,\n> if (opts->source_tree)\n> read_tree_some(opts->source_tree, &opts->pathspec);\n>\n> - ps_matched = xcalloc(1, opts->pathspec.nr);\n> + ps_matched = xcalloc(opts->pathspec.nr, 1);\n>\n> /*\n> * Make sure all pathspecs participated in locating the paths\n> diff --git a/builtin/ls-files.c b/builtin/ls-files.c\n> index 6fa2205..b6a7cb0 100644\n> --- a/builtin/ls-files.c\n> +++ b/builtin/ls-files.c\n> @@ -516,7 +516,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n>\n> /* Treat unmatching pathspec elements as errors */\n> if (pathspec.nr && error_unmatch)\n> - ps_matched = xcalloc(1, pathspec.nr);\n> + ps_matched = xcalloc(pathspec.nr, 1);\n>\n> if ((dir.flags & DIR_SHOW_IGNORED) && !exc_given)\n> die(\"ls-files --ignored needs some exclude pattern\");\n\nLooks good to me.\n"},{"id":"268339","messageId":"xmqqy4h785af.fsf@gitster.dls.corp.google.com","threadId":"40118","inReplyTo":"1439935980-30087-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH 7/7] submodule: implement `module_clone` as a builtin helper","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-19T19:08:40Z","receivedAt":"2015-08-19T19:08:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller writes:\n\n> `module_clone` is part of the update command, which I want to convert\n> to C next.\n>\n> Signed-off-by: Stefan Beller \n> ---\n>\n> This is the only patch that broke by a last minute fix,\n> so this replaces the previous PATCH 7/7.\n\n... which still breaks 7400.82, though.\n\nOutput from running it with \"-i -v\" ends like this:\n\nexpecting success: \n mkdir super &&\n pwd=$(pwd) &&\n (\n cd super &&\n git init &&\n git submodule add --depth=1 file://\"$pwd\"/example2 submodule &&\n (\n cd submodule &&\n test 1 = $(git log --oneline | wc -l)\n )\n )\n\nInitialized empty Git repository in /home/gitster/w/git.git/t/trash directory.t7400-submodule-basic/super/.git/\nfatal: Clone of 'file:///home/gitster/w/git.git/t/trash directory.t7400-submodule-basic/example2' into submodule path 'submodule' failed\nnot ok 82 - submodule add clone shallow submodule\n#\n# mkdir super &&\n# pwd=$(pwd) &&\n# (\n# cd super &&\n# git init &&\n# git submodule add --depth=1 file://\"$pwd\"/example2 submodule &&\n# (\n# cd submodule &&\n# test 1 = $(git log --oneline | wc -l)\n# )\n# )\n#\n"},{"id":"268340","messageId":"CAGZ79kYHOb_PJFT3x=RrnfSRFGgizw=jxHG1FhSBBx3dTgZhiQ@mail.gmail.com","threadId":"40118","inReplyTo":"xmqqy4h785af.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 7/7] submodule: implement `module_clone` as a builtin helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-08-19T19:20:51Z","receivedAt":"2015-08-19T19:20:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Aug 19, 2015 at 12:08 PM, Junio C Hamano wrote:\n> Stefan Beller writes:\n>\n>> `module_clone` is part of the update command, which I want to convert\n>> to C next.\n>>\n>> Signed-off-by: Stefan Beller \n>> ---\n>>\n>> This is the only patch that broke by a last minute fix,\n>> so this replaces the previous PATCH 7/7.\n>\n> ... which still breaks 7400.82, though.\n\nbecause I messed up the resending. :(\n\nSquashing in this fixes it:\n---8<---\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex e50451b..e7d221e 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -338,7 +338,7 @@ Use -f if you really want to add it.\" >&2\n echo \"$(eval_gettext \"Reactivating\nlocal git directory for submodule '\\$sm_name'.\")\"\n fi\n fi\n- git submodule--helper module_clone --prefix\n\"$wt_prefix\" --path \"$sm_path\" --name \"$sm_name\" --url \"$realrepo\"\n--reference \"$reference_path\" --depth \"$depth\" || exit\n+ git submodule--helper module_clone --prefix\n\"$wt_prefix\" --path \"$sm_path\" --name \"$sm_name\" --url \"$realrepo\"\n\"$reference_path\" \"$depth\" || exit\n (\n clear_local_git_env\n cd \"$sm_path\" &&\n@@ -634,7 +634,7 @@ cmd_update()\n shift\n ;;\n --depth=*)\n- depth=$1\n+ depth=\"$1\"\n ;;\n --)\n shift\n---\nbecause both depth and reference are either empty string or containing\nboth key and value\n(i.e. $depth will be \"--depth=5\" and we should not pass --depth into\nsubmodule--helper module_clone\ntwice)\n\nI can resend the patch as a whole if you'd like. I'd send it together\n in the next series later today, which will demonstrate a possible\nimplementation of `git submodule foreach-parallel`\n\n\n>\n> Output from running it with \"-i -v\" ends like this:\n>\n> expecting success:\n> mkdir super &&\n> pwd=$(pwd) &&\n> (\n> cd super &&\n> git init &&\n> git submodule add --depth=1 file://\"$pwd\"/example2 submodule &&\n> (\n> cd submodule &&\n> test 1 = $(git log --oneline | wc -l)\n> )\n> )\n>\n> Initialized empty Git repository in /home/gitster/w/git.git/t/trash directory.t7400-submodule-basic/super/.git/\n> fatal: Clone of 'file:///home/gitster/w/git.git/t/trash directory.t7400-submodule-basic/example2' into submodule path 'submodule' failed\n> not ok 82 - submodule add clone shallow submodule\n> #\n> # mkdir super &&\n> # pwd=$(pwd) &&\n> # (\n> # cd super &&\n> # git init &&\n> # git submodule add --depth=1 file://\"$pwd\"/example2 submodule &&\n> # (\n> # cd submodule &&\n> # test 1 = $(git log --oneline | wc -l)\n> # )\n> # )\n> #\n"},{"id":"268422","messageId":"20150821142212.GA26130@sigill.intra.peff.net","threadId":"40118","inReplyTo":"1439857323-21048-8-git-send-email-sbeller@google.com","subject":"Re: [PATCH 7/7] submodule: implement `module_clone` as a builtin helper","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-08-21T14:22:12Z","receivedAt":"2015-08-21T14:22:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 17, 2015 at 05:22:03PM -0700, Stefan Beller wrote:\n\n> +static int module_clone(int argc, const char **argv, const char *prefix)\n> [...]\n> +\t/* Redirect the worktree of the submodule in the superprojects config */\n> +\tif (!is_absolute_path(sm_gitdir)) {\n> +\t\tchar *s = (char*)sm_gitdir;\n> +\t\tstrbuf_addf(&sb, \"%s/%s\", xgetcwd(), sm_gitdir);\n\nCoverity noticed that this is a leak; xgetcwd actually returns a newly\nallocated buffer. I think you just want:\n\n if (strbuf_getcwd(&sb))\n\tdie_errno(\"unable to get current working directory\");\n strbuf_addf(\"/%s\", sm_gitdir);\n\n> +\t\tsm_gitdir = strbuf_detach(&sb, NULL);\n> +\t\tstrbuf_reset(&sb);\n\nThis reset is a noop after you've detached. It's not technically\ndocumented, but I do not think it is reasonable for it to work any other\nway. Maybe strbuf.h should make that promise.\n\n> +\tstrbuf_addf(&sb, \"%s/%s\", xgetcwd(), path);\n\nDitto on the xgetcwd issue here.\n\n-Peff\n"}]}