{"thread":{"id":"48169","subject":"[PATCH 0/5] Moving submodules with nested submodules","startedAt":"2018-03-27T21:39:37Z","lastAt":"2018-03-28T23:17:34Z","messageCount":36,"participants":["Stefan Beller","Brandon Williams","Jonathan Tan"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"343171","messageId":"20180327213918.77851-1-sbeller@google.com","threadId":"48169","inReplyTo":null,"subject":"[PATCH 0/5] Moving submodules with nested submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-27T21:39:13Z","receivedAt":"2018-03-27T21:39:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This fixes the bug reported in [1] (\"Bug: moving submodules that have submodules\ninside them causes a fatal error in git status\")\n\n[1] https://public-inbox.org/git/20180306192017.GA5797@riseup.net/\n\nThanks,\nStefan\n\nStefan Beller (5):\n submodule.h: drop declaration of connect_work_tree_and_git_dir\n submodule-config: allow submodule_free to handle arbitrary\n repositories\n submodule-config: add repository argument to submodule_from_{name,\n path}\n submodule-config: remove submodule_from_cache\n submodule: fixup nested submodules after moving the submodule\n\n .../technical/api-submodule-config.txt | 2 +-\n builtin/grep.c | 2 +-\n builtin/mv.c | 6 +-\n builtin/submodule--helper.c | 17 +++--\n dir.c | 70 ++++++++++++++++++-\n dir.h | 12 +++-\n repository.c | 2 +-\n submodule-config.c | 29 +++-----\n submodule-config.h | 9 +--\n submodule.c | 40 ++++++-----\n submodule.h | 1 -\n t/helper/test-submodule-config.c | 8 ++-\n t/t7001-mv.sh | 2 +-\n unpack-trees.c | 2 +-\n 14 files changed, 135 insertions(+), 67 deletions(-)\n\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343172","messageId":"20180327213918.77851-2-sbeller@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-1-sbeller@google.com","subject":"[PATCH 1/5] submodule.h: drop declaration of connect_work_tree_and_git_dir","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-27T21:39:14Z","receivedAt":"2018-03-27T21:39:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The function connect_work_tree_and_git_dir is declared in both submodule.h\nand dir.h, such that one of them is redundant. As the function is\nimplemented in dir.c, drop the declaration from submodule.h\n\nSigned-off-by: Stefan Beller \n---\n submodule.h | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/submodule.h b/submodule.h\nindex 9589f13127..e5526f6aaa 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -105,7 +105,6 @@ extern int push_unpushed_submodules(struct oid_array *commits,\n \t\t\t\t const char **refspec, int refspec_nr,\n \t\t\t\t const struct string_list *push_options,\n \t\t\t\t int dry_run);\n-extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n /*\n * Given a submodule path (as in the index), return the repository\n * path of that submodule in 'buf'. Return -1 on error or when the\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343173","messageId":"20180327213918.77851-4-sbeller@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-1-sbeller@google.com","subject":"[PATCH 3/5] submodule-config: add repository argument to submodule_from_{name, path}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-27T21:39:16Z","receivedAt":"2018-03-27T21:39:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This enables submodule_from_{name, path} to handle arbitrary repositories.\nAll callers just pass in the_repository, a later patch will pass in other\nrepos.\n\nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 14 +++++++-------\n submodule-config.c | 14 ++++++++------\n submodule-config.h | 4 ++--\n submodule.c | 30 ++++++++++++++++--------------\n t/helper/test-submodule-config.c | 6 ++++--\n 5 files changed, 37 insertions(+), 31 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex ee020d4749..a921fbbf56 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -454,7 +454,7 @@ static void init_submodule(const char *path, const char *prefix,\n \n \tdisplaypath = get_submodule_displaypath(path, prefix);\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (!sub)\n \t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n@@ -621,7 +621,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,\n \tstruct rev_info rev;\n \tint diff_files_result;\n \n-\tif (!submodule_from_path(&null_oid, path))\n+\tif (!submodule_from_path(the_repository, &null_oid, path))\n \t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n \t\t path);\n \n@@ -741,7 +741,7 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \tif (argc != 2)\n \t\tusage(_(\"git submodule--helper name \"));\n \n-\tsub = submodule_from_path(&null_oid, argv[1]);\n+\tsub = submodule_from_path(the_repository, &null_oid, argv[1]);\n \n \tif (!sub)\n \t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n@@ -772,7 +772,7 @@ static void sync_submodule(const char *path, const char *prefix,\n \tif (!is_submodule_active(the_repository, path))\n \t\treturn;\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (sub && sub->url) {\n \t\tif (starts_with_dot_dot_slash(sub->url) ||\n@@ -925,7 +925,7 @@ static void deinit_submodule(const char *path, const char *prefix,\n \tstruct strbuf sb_config = STRBUF_INIT;\n \tchar *sub_git_dir = xstrfmt(\"%s/.git\", path);\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (!sub || !sub->name)\n \t\tgoto cleanup;\n@@ -1367,7 +1367,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\tgoto cleanup;\n \t}\n \n-\tsub = submodule_from_path(&null_oid, ce->name);\n+\tsub = submodule_from_path(the_repository, &null_oid, ce->name);\n \n \tif (suc->recursive_prefix)\n \t\tdisplaypath = relative_path(suc->recursive_prefix,\n@@ -1650,7 +1650,7 @@ static const char *remote_submodule_branch(const char *path)\n \tconst char *branch = NULL;\n \tchar *key;\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \tif (!sub)\n \t\treturn NULL;\n \ndiff --git a/submodule-config.c b/submodule-config.c\nindex a3efff1a34..4b7803e6ed 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -619,18 +619,20 @@ static void gitmodules_read_check(struct repository *repo)\n \t\trepo_read_gitmodules(repo);\n }\n \n-const struct submodule *submodule_from_name(const struct object_id *treeish_name,\n+const struct submodule *submodule_from_name(struct repository *r,\n+\t\t\t\t\t const struct object_id *treeish_name,\n \t\tconst char *name)\n {\n-\tgitmodules_read_check(the_repository);\n-\treturn config_from(the_repository->submodule_cache, treeish_name, name, lookup_name);\n+\tgitmodules_read_check(r);\n+\treturn config_from(r->submodule_cache, treeish_name, name, lookup_name);\n }\n \n-const struct submodule *submodule_from_path(const struct object_id *treeish_name,\n+const struct submodule *submodule_from_path(struct repository *r,\n+\t\t\t\t\t const struct object_id *treeish_name,\n \t\tconst char *path)\n {\n-\tgitmodules_read_check(the_repository);\n-\treturn config_from(the_repository->submodule_cache, treeish_name, path, lookup_path);\n+\tgitmodules_read_check(r);\n+\treturn config_from(r->submodule_cache, treeish_name, path, lookup_path);\n }\n \n const struct submodule *submodule_from_cache(struct repository *repo,\ndiff --git a/submodule-config.h b/submodule-config.h\nindex df6bd6e6db..ff3c9e0b5c 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -39,9 +39,9 @@ extern int parse_update_recurse_submodules_arg(const char *opt, const char *arg)\n extern int parse_push_recurse_submodules_arg(const char *opt, const char *arg);\n extern void repo_read_gitmodules(struct repository *repo);\n extern void gitmodules_config_oid(const struct object_id *commit_oid);\n-extern const struct submodule *submodule_from_name(\n+extern const struct submodule *submodule_from_name(struct repository *r,\n \t\tconst struct object_id *commit_or_tree, const char *name);\n-extern const struct submodule *submodule_from_path(\n+extern const struct submodule *submodule_from_path(struct repository *r,\n \t\tconst struct object_id *commit_or_tree, const char *path);\n extern const struct submodule *submodule_from_cache(struct repository *repo,\n \t\t\t\t\t\t const struct object_id *treeish_name,\ndiff --git a/submodule.c b/submodule.c\nindex 12a2503fda..e94b7f9acd 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -95,7 +95,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n \tif (is_gitmodules_unmerged(&the_index))\n \t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n \n-\tsubmodule = submodule_from_path(&null_oid, oldpath);\n+\tsubmodule = submodule_from_path(the_repository, &null_oid, oldpath);\n \tif (!submodule || !submodule->name) {\n \t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), oldpath);\n \t\treturn -1;\n@@ -129,7 +129,7 @@ int remove_path_from_gitmodules(const char *path)\n \tif (is_gitmodules_unmerged(&the_index))\n \t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n \n-\tsubmodule = submodule_from_path(&null_oid, path);\n+\tsubmodule = submodule_from_path(the_repository, &null_oid, path);\n \tif (!submodule || !submodule->name) {\n \t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), path);\n \t\treturn -1;\n@@ -173,7 +173,8 @@ static int add_submodule_odb(const char *path)\n void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\t\t\t\t const char *path)\n {\n-\tconst struct submodule *submodule = submodule_from_path(&null_oid, path);\n+\tconst struct submodule *submodule = submodule_from_path(the_repository,\n+\t\t\t\t\t\t\t\t&null_oid, path);\n \tif (submodule) {\n \t\tconst char *ignore;\n \t\tchar *key;\n@@ -673,7 +674,7 @@ const struct submodule *submodule_from_ce(const struct cache_entry *ce)\n \tif (!should_update_submodules())\n \t\treturn NULL;\n \n-\treturn submodule_from_path(&null_oid, ce->name);\n+\treturn submodule_from_path(the_repository, &null_oid, ce->name);\n }\n \n static struct oid_array *submodule_commits(struct string_list *submodules,\n@@ -730,13 +731,14 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,\n \t\tif (!S_ISGITLINK(p->two->mode))\n \t\t\tcontinue;\n \n-\t\tsubmodule = submodule_from_path(commit_oid, p->two->path);\n+\t\tsubmodule = submodule_from_path(the_repository,\n+\t\t\t\t\t\tcommit_oid, p->two->path);\n \t\tif (submodule)\n \t\t\tname = submodule->name;\n \t\telse {\n \t\t\tname = default_name_or_path(p->two->path);\n \t\t\t/* make sure name does not collide with existing one */\n-\t\t\tsubmodule = submodule_from_name(commit_oid, name);\n+\t\t\tsubmodule = submodule_from_name(the_repository, commit_oid, name);\n \t\t\tif (submodule) {\n \t\t\t\twarning(\"Submodule in commit %s at path: \"\n \t\t\t\t\t\"'%s' collides with a submodule named \"\n@@ -944,7 +946,7 @@ int find_unpushed_submodules(struct oid_array *commits,\n \t\tconst struct submodule *submodule;\n \t\tconst char *path = NULL;\n \n-\t\tsubmodule = submodule_from_name(&null_oid, name->string);\n+\t\tsubmodule = submodule_from_name(the_repository, &null_oid, name->string);\n \t\tif (submodule)\n \t\t\tpath = submodule->path;\n \t\telse\n@@ -1112,7 +1114,7 @@ static void calculate_changed_submodule_paths(void)\n \tconst struct string_list_item *name;\n \n \t/* No need to check if there are no submodules configured */\n-\tif (!submodule_from_path(NULL, NULL))\n+\tif (!submodule_from_path(the_repository, NULL, NULL))\n \t\treturn;\n \n \targv_array_push(&argv, \"--\"); /* argv[0] program name */\n@@ -1133,7 +1135,7 @@ static void calculate_changed_submodule_paths(void)\n \t\tconst struct submodule *submodule;\n \t\tconst char *path = NULL;\n \n-\t\tsubmodule = submodule_from_name(&null_oid, name->string);\n+\t\tsubmodule = submodule_from_name(the_repository, &null_oid, name->string);\n \t\tif (submodule)\n \t\t\tpath = submodule->path;\n \t\telse\n@@ -1161,7 +1163,7 @@ int submodule_touches_in_range(struct object_id *excl_oid,\n \tint ret;\n \n \t/* No need to check if there are no submodules configured */\n-\tif (!submodule_from_path(NULL, NULL))\n+\tif (!submodule_from_path(the_repository, NULL, NULL))\n \t\treturn 0;\n \n \targv_array_push(&args, \"--\"); /* args[0] program name */\n@@ -1603,7 +1605,7 @@ int submodule_move_head(const char *path,\n \tif (old_head && !is_submodule_populated_gently(path, error_code_ptr))\n \t\treturn 0;\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (!sub)\n \t\tdie(\"BUG: could not get submodule information for '%s'\", path);\n@@ -1885,7 +1887,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,\n \n \treal_old_git_dir = real_pathdup(old_git_dir, 1);\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \tif (!sub)\n \t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n \n@@ -1941,7 +1943,7 @@ void absorb_git_dir_into_superproject(const char *prefix,\n \t\t* superproject did not rewrite the git file links yet,\n \t\t* fix it now.\n \t\t*/\n-\t\tsub = submodule_from_path(&null_oid, path);\n+\t\tsub = submodule_from_path(the_repository, &null_oid, path);\n \t\tif (!sub)\n \t\t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n \t\tconnect_work_tree_and_git_dir(path,\n@@ -2087,7 +2089,7 @@ int submodule_to_gitdir(struct strbuf *buf, const char *submodule)\n \t\tstrbuf_addstr(buf, git_dir);\n \t}\n \tif (!is_git_directory(buf->buf)) {\n-\t\tsub = submodule_from_path(&null_oid, submodule);\n+\t\tsub = submodule_from_path(the_repository, &null_oid, submodule);\n \t\tif (!sub) {\n \t\t\tret = -1;\n \t\t\tgoto cleanup;\ndiff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c\nindex 9971c5e9dd..e044871cee 100644\n--- a/t/helper/test-submodule-config.c\n+++ b/t/helper/test-submodule-config.c\n@@ -48,9 +48,11 @@ int cmd_main(int argc, const char **argv)\n \t\t\tdie_usage(argc, argv, \"Commit not found.\");\n \n \t\tif (lookup_name) {\n-\t\t\tsubmodule = submodule_from_name(&commit_oid, path_or_name);\n+\t\t\tsubmodule = submodule_from_name(the_repository,\n+\t\t\t\t\t\t\t&commit_oid, path_or_name);\n \t\t} else\n-\t\t\tsubmodule = submodule_from_path(&commit_oid, path_or_name);\n+\t\t\tsubmodule = submodule_from_path(the_repository,\n+\t\t\t\t\t\t\t&commit_oid, path_or_name);\n \t\tif (!submodule)\n \t\t\tdie_usage(argc, argv, \"Submodule not found.\");\n \n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343174","messageId":"20180327213918.77851-3-sbeller@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-1-sbeller@google.com","subject":"[PATCH 2/5] submodule-config: allow submodule_free to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-27T21:39:15Z","receivedAt":"2018-03-27T21:39:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n Documentation/technical/api-submodule-config.txt | 2 +-\n builtin/grep.c | 2 +-\n submodule-config.c | 6 +++---\n submodule-config.h | 2 +-\n t/helper/test-submodule-config.c | 2 +-\n unpack-trees.c | 2 +-\n 6 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt\nindex ee907c4a82..fb06089393 100644\n--- a/Documentation/technical/api-submodule-config.txt\n+++ b/Documentation/technical/api-submodule-config.txt\n@@ -38,7 +38,7 @@ Data Structures\n Functions\n ---------\n \n-`void submodule_free()`::\n+`void submodule_free(struct repository *r)`::\n \n \tUse these to free the internally cached values.\n \ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 789a89133a..8f04cde18e 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -651,7 +651,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n \n \t\t/* load the gitmodules file for this rev */\n \t\tif (recurse_submodules) {\n-\t\t\tsubmodule_free();\n+\t\t\tsubmodule_free(repo);\n \t\t\tgitmodules_config_oid(&real_obj->oid);\n \t\t}\n \t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].path,\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 602ba8ca8b..a3efff1a34 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -642,8 +642,8 @@ const struct submodule *submodule_from_cache(struct repository *repo,\n \t\t\t key, lookup_path);\n }\n \n-void submodule_free(void)\n+void submodule_free(struct repository *r)\n {\n-\tif (the_repository->submodule_cache)\n-\t\tsubmodule_cache_clear(the_repository->submodule_cache);\n+\tif (r->submodule_cache)\n+\t\tsubmodule_cache_clear(r->submodule_cache);\n }\ndiff --git a/submodule-config.h b/submodule-config.h\nindex a5503a5d17..df6bd6e6db 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -46,6 +46,6 @@ extern const struct submodule *submodule_from_path(\n extern const struct submodule *submodule_from_cache(struct repository *repo,\n \t\t\t\t\t\t const struct object_id *treeish_name,\n \t\t\t\t\t\t const char *key);\n-extern void submodule_free(void);\n+extern void submodule_free(struct repository *r);\n \n #endif /* SUBMODULE_CONFIG_H */\ndiff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c\nindex f23db3b19a..9971c5e9dd 100644\n--- a/t/helper/test-submodule-config.c\n+++ b/t/helper/test-submodule-config.c\n@@ -64,7 +64,7 @@ int cmd_main(int argc, const char **argv)\n \t\targ += 2;\n \t}\n \n-\tsubmodule_free();\n+\tsubmodule_free(the_repository);\n \n \treturn 0;\n }\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex d5685891a5..05e5fa77eb 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -290,7 +290,7 @@ static void load_gitmodules_file(struct index_state *index,\n \t\tif (!state && ce->ce_flags & CE_WT_REMOVE) {\n \t\t\trepo_read_gitmodules(the_repository);\n \t\t} else if (state && (ce->ce_flags & CE_UPDATE)) {\n-\t\t\tsubmodule_free();\n+\t\t\tsubmodule_free(the_repository);\n \t\t\tcheckout_entry(ce, state, NULL);\n \t\t\trepo_read_gitmodules(the_repository);\n \t\t}\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343175","messageId":"20180327213918.77851-6-sbeller@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-1-sbeller@google.com","subject":"[PATCH 5/5] submodule: fixup nested submodules after moving the submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-27T21:39:18Z","receivedAt":"2018-03-27T21:39:48Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"connect_work_tree_and_git_dir is used to connect a submodule worktree with\nits git directory and vice versa after events that require a reconnection\nsuch as moving around the working tree. As submodules can have nested\nsubmoduled themselves, we'd also want to fix the nested submodules when\nasked to. Add an option to recurse into the nested submodules and connect\nthem as well.\n\nAs submodules are identified by their name (which determines their git\ndirectory in relation to their superprojects git directory) internally\nand by their path in the working tree of the superproject, we need to\nmake sure that the mapping of name <-> path is kept intact. We can do\nthat in the git-mv command by writing out the gitmodules file and first\nand then force a reload of the submodule config machinery.\n\nSigned-off-by: Stefan Beller \n---\n builtin/mv.c | 6 ++--\n builtin/submodule--helper.c | 3 +-\n dir.c | 70 +++++++++++++++++++++++++++++++++++--\n dir.h | 12 ++++++-\n submodule.c | 6 ++--\n t/t7001-mv.sh | 2 +-\n 6 files changed, 87 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 6d141f7a53..7a63667d64 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -276,10 +276,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\tdie_errno(_(\"renaming '%s' failed\"), src);\n \t\t}\n \t\tif (submodule_gitfile[i]) {\n-\t\t\tif (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)\n-\t\t\t\tconnect_work_tree_and_git_dir(dst, submodule_gitfile[i]);\n \t\t\tif (!update_path_in_gitmodules(src, dst))\n \t\t\t\tgitmodules_modified = 1;\n+\t\t\tif (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)\n+\t\t\t\tconnect_work_tree_and_git_dir(dst,\n+\t\t\t\t\t\t\t submodule_gitfile[i],\n+\t\t\t\t\t\t\t 1);\n \t\t}\n \n \t\tif (mode == WORKING_DIRECTORY)\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a921fbbf56..05fd657f99 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1259,8 +1259,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_reset(&sb);\n \t}\n \n-\t/* Connect module worktree and git dir */\n-\tconnect_work_tree_and_git_dir(path, sm_gitdir);\n+\tconnect_work_tree_and_git_dir(path, sm_gitdir, 0);\n \n \tp = git_pathdup_submodule(path, \"config\");\n \tif (!p)\ndiff --git a/dir.c b/dir.c\nindex dedbf5d476..313176e291 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -19,6 +19,7 @@\n #include \"varint.h\"\n #include \"ewah/ewok.h\"\n #include \"fsmonitor.h\"\n+#include \"submodule-config.h\"\n \n /*\n * Tells read_directory_recursive how a file or directory should be treated.\n@@ -3010,8 +3011,67 @@ void untracked_cache_add_to_index(struct index_state *istate,\n \tuntracked_cache_invalidate_path(istate, path, 1);\n }\n \n-/* Update gitfile and core.worktree setting to connect work tree and git dir */\n-void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)\n+static void connect_wt_gitdir_in_nested(const char *sub_worktree,\n+\t\t\t\t\tconst char *sub_gitdir,\n+\t\t\t\t\tstruct repository *superproject)\n+{\n+\tint i;\n+\tstruct repository subrepo;\n+\tstruct strbuf sub_wt = STRBUF_INIT;\n+\tstruct strbuf sub_gd = STRBUF_INIT;\n+\tconst struct submodule *sub;\n+\tconst char *super_worktree,\n+\t\t *sub_path; /* path inside the superproject */\n+\n+\t/* subrepo got moved, so superproject has outdated information */\n+\tsubmodule_free(superproject);\n+\n+\tsuper_worktree = real_pathdup(superproject->worktree, 1);\n+\n+\tsub_path = sub_worktree + strlen(super_worktree) + 1;\n+\n+\tif (repo_submodule_init(&subrepo, superproject, sub_path))\n+\t\treturn;\n+\n+\trepo_read_index(&subrepo);\n+\n+\tfor (i = 0; i < subrepo.index->cache_nr; i++) {\n+\t\tconst struct cache_entry *ce = subrepo.index->cache[i];\n+\n+\t\tif (!S_ISGITLINK(ce->ce_mode))\n+\t\t\tcontinue;\n+\n+\t\twhile (i + 1 < subrepo.index->cache_nr &&\n+\t\t !strcmp(ce->name, subrepo.index->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+\n+\t\tsub = submodule_from_path(&subrepo, &null_oid, ce->name);\n+\t\tif (!sub)\n+\t\t\t/* submodule not checked out? */\n+\t\t\tcontinue;\n+\n+\t\tstrbuf_reset(&sub_wt);\n+\t\tstrbuf_addf(&sub_wt, \"%s/%s/.git\", sub_worktree, sub->path);\n+\n+\t\tstrbuf_reset(&sub_gd);\n+\t\tstrbuf_addf(&sub_gd, \"%s/modules/%s\", sub_gitdir, sub->name);\n+\n+\t\tstrbuf_setlen(&sub_wt, sub_wt.len - strlen(\"/.git\"));\n+\n+\t\tif (is_submodule_active(&subrepo, ce->name)) {\n+\t\t\tconnect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 0);\n+\t\t\tconnect_wt_gitdir_in_nested(sub_wt.buf, sub_gd.buf, &subrepo);\n+\t\t}\n+\t}\n+}\n+\n+void connect_work_tree_and_git_dir(const char *work_tree_,\n+\t\t\t\t const char *git_dir_,\n+\t\t\t\t int recurse_into_nested)\n {\n \tstruct strbuf gitfile_sb = STRBUF_INIT;\n \tstruct strbuf cfg_sb = STRBUF_INIT;\n@@ -3041,6 +3101,10 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)\n \tstrbuf_release(&gitfile_sb);\n \tstrbuf_release(&cfg_sb);\n \tstrbuf_release(&rel_path);\n+\n+\tif (recurse_into_nested)\n+\t\tconnect_wt_gitdir_in_nested(work_tree, git_dir, the_repository);\n+\n \tfree(work_tree);\n \tfree(git_dir);\n }\n@@ -3054,5 +3118,5 @@ void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_\n \t\tdie_errno(_(\"could not migrate git directory from '%s' to '%s'\"),\n \t\t\told_git_dir, new_git_dir);\n \n-\tconnect_work_tree_and_git_dir(path, new_git_dir);\n+\tconnect_work_tree_and_git_dir(path, new_git_dir, 0);\n }\ndiff --git a/dir.h b/dir.h\nindex b0758b82a2..3870193e52 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -359,7 +359,17 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);\n void add_untracked_cache(struct index_state *istate);\n void remove_untracked_cache(struct index_state *istate);\n-extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n+\n+/*\n+ * Connect a worktree to a git directory by creating (or overwriting) a\n+ * '.git' file containing the location of the git directory. In the git\n+ * directory set the core.worktree setting to indicate where the worktree is.\n+ * When `recurse_into_nested` is set, recurse into any nested submodules,\n+ * connecting them as well.\n+ */\n+extern void connect_work_tree_and_git_dir(const char *work_tree,\n+\t\t\t\t\t const char *git_dir,\n+\t\t\t\t\t int recurse_into_nested);\n extern void relocate_gitdir(const char *path,\n \t\t\t const char *old_git_dir,\n \t\t\t const char *new_git_dir);\ndiff --git a/submodule.c b/submodule.c\nindex 89d0aee086..c2dac6c00f 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1624,7 +1624,7 @@ int submodule_move_head(const char *path,\n \t\t} else {\n \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n \t\t\t\t get_git_common_dir(), sub->name);\n-\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n+\t\t\tconnect_work_tree_and_git_dir(path, gitdir, 0);\n \t\t\tfree(gitdir);\n \n \t\t\t/* make sure the index is clean as well */\n@@ -1634,7 +1634,7 @@ int submodule_move_head(const char *path,\n \t\tif (old_head && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {\n \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n \t\t\t\t get_git_common_dir(), sub->name);\n-\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n+\t\t\tconnect_work_tree_and_git_dir(path, gitdir, 1);\n \t\t\tfree(gitdir);\n \t\t}\n \t}\n@@ -1947,7 +1947,7 @@ void absorb_git_dir_into_superproject(const char *prefix,\n \t\tif (!sub)\n \t\t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n \t\tconnect_work_tree_and_git_dir(path,\n-\t\t\tgit_path(\"modules/%s\", sub->name));\n+\t\t\tgit_path(\"modules/%s\", sub->name), 0);\n \t} else {\n \t\t/* Is it already absorbed into the superprojects git dir? */\n \t\tchar *real_sub_git_dir = real_pathdup(sub_git_dir, 1);\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex d4e6485a26..ff70244620 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -497,7 +497,7 @@ test_expect_success 'moving a submodule in nested directories' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'moving nested submodules' '\n+test_expect_success 'moving nested submodules' '\n \tgit commit -am \"cleanup commit\" &&\n \tmkdir sub_nested_nested &&\n \t(cd sub_nested_nested &&\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343176","messageId":"20180327213918.77851-5-sbeller@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-1-sbeller@google.com","subject":"[PATCH 4/5] submodule-config: remove submodule_from_cache","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-27T21:39:17Z","receivedAt":"2018-03-27T21:39:52Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This continues the story of bf12fcdf5e (submodule-config: store\nthe_submodule_cache in the_repository, 2017-06-22).\n\nThe previous patch taught submodule_from_path to take a repository into\naccount, such that submodule_from_{path, cache} are the same now.\nRemove submodule_from_cache, migrating all its callers to\nsubmodule_from_path.\n\nSigned-off-by: Stefan Beller \n---\n repository.c | 2 +-\n submodule-config.c | 9 ---------\n submodule-config.h | 3 ---\n submodule.c | 4 ++--\n 4 files changed, 3 insertions(+), 15 deletions(-)\n\ndiff --git a/repository.c b/repository.c\nindex 4ffbe9bc94..fa0a132e22 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -167,7 +167,7 @@ int repo_submodule_init(struct repository *submodule,\n \tstruct strbuf worktree = STRBUF_INIT;\n \tint ret = 0;\n \n-\tsub = submodule_from_cache(superproject, &null_oid, path);\n+\tsub = submodule_from_path(superproject, &null_oid, path);\n \tif (!sub) {\n \t\tret = -1;\n \t\tgoto out;\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 4b7803e6ed..cb65354d4c 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -635,15 +635,6 @@ const struct submodule *submodule_from_path(struct repository *r,\n \treturn config_from(r->submodule_cache, treeish_name, path, lookup_path);\n }\n \n-const struct submodule *submodule_from_cache(struct repository *repo,\n-\t\t\t\t\t const struct object_id *treeish_name,\n-\t\t\t\t\t const char *key)\n-{\n-\tgitmodules_read_check(repo);\n-\treturn config_from(repo->submodule_cache, treeish_name,\n-\t\t\t key, lookup_path);\n-}\n-\n void submodule_free(struct repository *r)\n {\n \tif (r->submodule_cache)\ndiff --git a/submodule-config.h b/submodule-config.h\nindex ff3c9e0b5c..3ae8a1e51b 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -43,9 +43,6 @@ extern const struct submodule *submodule_from_name(struct repository *r,\n \t\tconst struct object_id *commit_or_tree, const char *name);\n extern const struct submodule *submodule_from_path(struct repository *r,\n \t\tconst struct object_id *commit_or_tree, const char *path);\n-extern const struct submodule *submodule_from_cache(struct repository *repo,\n-\t\t\t\t\t\t const struct object_id *treeish_name,\n-\t\t\t\t\t\t const char *key);\n extern void submodule_free(struct repository *r);\n \n #endif /* SUBMODULE_CONFIG_H */\ndiff --git a/submodule.c b/submodule.c\nindex e94b7f9acd..89d0aee086 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -230,7 +230,7 @@ int is_submodule_active(struct repository *repo, const char *path)\n \tconst struct string_list *sl;\n \tconst struct submodule *module;\n \n-\tmodule = submodule_from_cache(repo, &null_oid, path);\n+\tmodule = submodule_from_path(repo, &null_oid, path);\n \n \t/* early return if there isn't a path->module mapping */\n \tif (!module)\n@@ -1235,7 +1235,7 @@ static int get_next_submodule(struct child_process *cp,\n \t\tif (!S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n \n-\t\tsubmodule = submodule_from_cache(spf->r, &null_oid, ce->name);\n+\t\tsubmodule = submodule_from_path(spf->r, &null_oid, ce->name);\n \t\tif (!submodule) {\n \t\t\tconst char *name = default_name_or_path(ce->name);\n \t\t\tif (name) {\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343182","messageId":"20180327225730.GA215480@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-3-sbeller@google.com","subject":"Re: [PATCH 2/5] submodule-config: allow submodule_free to handle arbitrary repositories","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-03-27T22:57:30Z","receivedAt":"2018-03-27T22:57:38Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 03/27, Stefan Beller wrote:\n> Signed-off-by: Stefan Beller \n\nLooks good. At some point we may want to rename this function so that\nit describes what it actually does as 'submodule_free' doesn't quite\ndescribe that this clears a repository's submodule cache. But that's\nbeyond the scope of this.\n\n> ---\n> Documentation/technical/api-submodule-config.txt | 2 +-\n> builtin/grep.c | 2 +-\n> submodule-config.c | 6 +++---\n> submodule-config.h | 2 +-\n> t/helper/test-submodule-config.c | 2 +-\n> unpack-trees.c | 2 +-\n> 6 files changed, 8 insertions(+), 8 deletions(-)\n> \n> diff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt\n> index ee907c4a82..fb06089393 100644\n> --- a/Documentation/technical/api-submodule-config.txt\n> +++ b/Documentation/technical/api-submodule-config.txt\n> @@ -38,7 +38,7 @@ Data Structures\n> Functions\n> ---------\n> \n> -`void submodule_free()`::\n> +`void submodule_free(struct repository *r)`::\n> \n> \tUse these to free the internally cached values.\n> \n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 789a89133a..8f04cde18e 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -651,7 +651,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n> \n> \t\t/* load the gitmodules file for this rev */\n> \t\tif (recurse_submodules) {\n> -\t\t\tsubmodule_free();\n> +\t\t\tsubmodule_free(repo);\n> \t\t\tgitmodules_config_oid(&real_obj->oid);\n> \t\t}\n> \t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].path,\n> diff --git a/submodule-config.c b/submodule-config.c\n> index 602ba8ca8b..a3efff1a34 100644\n> --- a/submodule-config.c\n> +++ b/submodule-config.c\n> @@ -642,8 +642,8 @@ const struct submodule *submodule_from_cache(struct repository *repo,\n> \t\t\t key, lookup_path);\n> }\n> \n> -void submodule_free(void)\n> +void submodule_free(struct repository *r)\n> {\n> -\tif (the_repository->submodule_cache)\n> -\t\tsubmodule_cache_clear(the_repository->submodule_cache);\n> +\tif (r->submodule_cache)\n> +\t\tsubmodule_cache_clear(r->submodule_cache);\n> }\n> diff --git a/submodule-config.h b/submodule-config.h\n> index a5503a5d17..df6bd6e6db 100644\n> --- a/submodule-config.h\n> +++ b/submodule-config.h\n> @@ -46,6 +46,6 @@ extern const struct submodule *submodule_from_path(\n> extern const struct submodule *submodule_from_cache(struct repository *repo,\n> \t\t\t\t\t\t const struct object_id *treeish_name,\n> \t\t\t\t\t\t const char *key);\n> -extern void submodule_free(void);\n> +extern void submodule_free(struct repository *r);\n> \n> #endif /* SUBMODULE_CONFIG_H */\n> diff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c\n> index f23db3b19a..9971c5e9dd 100644\n> --- a/t/helper/test-submodule-config.c\n> +++ b/t/helper/test-submodule-config.c\n> @@ -64,7 +64,7 @@ int cmd_main(int argc, const char **argv)\n> \t\targ += 2;\n> \t}\n> \n> -\tsubmodule_free();\n> +\tsubmodule_free(the_repository);\n> \n> \treturn 0;\n> }\n> diff --git a/unpack-trees.c b/unpack-trees.c\n> index d5685891a5..05e5fa77eb 100644\n> --- a/unpack-trees.c\n> +++ b/unpack-trees.c\n> @@ -290,7 +290,7 @@ static void load_gitmodules_file(struct index_state *index,\n> \t\tif (!state && ce->ce_flags & CE_WT_REMOVE) {\n> \t\t\trepo_read_gitmodules(the_repository);\n> \t\t} else if (state && (ce->ce_flags & CE_UPDATE)) {\n> -\t\t\tsubmodule_free();\n> +\t\t\tsubmodule_free(the_repository);\n> \t\t\tcheckout_entry(ce, state, NULL);\n> \t\t\trepo_read_gitmodules(the_repository);\n> \t\t}\n> -- \n> 2.17.0.rc1.321.gba9d0f2565-goog\n> \n\n-- \nBrandon Williams\n"},{"id":"343183","messageId":"20180327225850.166523-1-jonathantanmy@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-3-sbeller@google.com","subject":"[PATCH] grep: remove \"repo\" arg from non-supporting funcs","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-03-27T22:58:50Z","receivedAt":"2018-03-27T22:59:13Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"As part of commit f9ee2fcdfa (\"grep: recurse in-process using 'struct\nrepository'\", 2017-08-02), many functions in builtin/grep.c were\nconverted to also take \"struct repository *\" arguments. Among them were\ngrep_object() and grep_objects().\n\nHowever, at least grep_objects() was converted incompletely - it calls\ngitmodules_config_oid(), which references the_repository.\n\nBut it turns out that the conversion was extraneous anyway - there has\nbeen no user-visible effect - because grep_objects() is never invoked\nexcept with the_repository.\n\nRevert the changes to grep_objects() and grep_object() (which conversion\nis also extraneous) to show that both these functions do not support\nrepositories other than the_repository.\n\nSigned-off-by: Jonathan Tan \n---\nPatch 1/5 of your series is obviously correct.\n\nI investigated the change to grep_objects() in patch 2/5, and here is a\npatch summarizing my findings. Consider including this patch before 2/5\n(or before 1/5). You'll probably need to write\n\"submodule_free(the_repository);\" instead of what you have currently,\nbut other than that, I don't think this affects your patch set much.\n---\n builtin/grep.c | 12 +++++-------\n 1 file changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 789a89133..f286f2375 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -601,8 +601,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,\n }\n \n static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n-\t\t struct object *obj, const char *name, const char *path,\n-\t\t struct repository *repo)\n+\t\t struct object *obj, const char *name, const char *path)\n {\n \tif (obj->type == OBJ_BLOB)\n \t\treturn grep_oid(opt, &obj->oid, name, 0, path);\n@@ -629,7 +628,7 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n \t\t}\n \t\tinit_tree_desc(&tree, data, size);\n \t\thit = grep_tree(opt, pathspec, &tree, &base, base.len,\n-\t\t\t\tobj->type == OBJ_COMMIT, repo);\n+\t\t\t\tobj->type == OBJ_COMMIT, the_repository);\n \t\tstrbuf_release(&base);\n \t\tfree(data);\n \t\treturn hit;\n@@ -638,7 +637,6 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n }\n \n static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n-\t\t\tstruct repository *repo,\n \t\t\tconst struct object_array *list)\n {\n \tunsigned int i;\n@@ -654,8 +652,8 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n \t\t\tsubmodule_free();\n \t\t\tgitmodules_config_oid(&real_obj->oid);\n \t\t}\n-\t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].path,\n-\t\t\t\trepo)) {\n+\t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name,\n+\t\t\t\tlist->objects[i].path)) {\n \t\t\thit = 1;\n \t\t\tif (opt->status_only)\n \t\t\t\tbreak;\n@@ -1107,7 +1105,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tif (cached)\n \t\t\tdie(_(\"both --cached and trees are given.\"));\n \n-\t\thit = grep_objects(&opt, &pathspec, the_repository, &list);\n+\t\thit = grep_objects(&opt, &pathspec, &list);\n \t}\n \n \tif (num_threads)\n-- \n2.16.0.rc0.35.ga4d78ce26\n\n"},{"id":"343184","messageId":"20180327160443.5034f7db908c08fe3c301ab9@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-4-sbeller@google.com","subject":"Re: [PATCH 3/5] submodule-config: add repository argument to submodule_from_{name, path}","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-03-27T23:04:43Z","receivedAt":"2018-03-27T23:04:51Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 27 Mar 2018 14:39:16 -0700\nStefan Beller wrote:\n\n> -extern const struct submodule *submodule_from_name(\n> +extern const struct submodule *submodule_from_name(struct repository *r,\n> \t\tconst struct object_id *commit_or_tree, const char *name);\n> -extern const struct submodule *submodule_from_path(\n> +extern const struct submodule *submodule_from_path(struct repository *r,\n> \t\tconst struct object_id *commit_or_tree, const char *path);\n\nThere is a recent change to CodingGuidelines that states to not include\n\"extern\" in 89a9f2c862 (\"CodingGuidelines: mention \"static\" and\n\"extern\"\", 2018-02-08), so consider removing \"extern\" since you're\nalready changing this file.\n\nOther than that,\n\nReviewed-by: Jonathan Tan \n"},{"id":"343186","messageId":"20180327160716.eec5ae96c7c1c5135b27b08e@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-5-sbeller@google.com","subject":"Re: [PATCH 4/5] submodule-config: remove submodule_from_cache","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-03-27T23:07:16Z","receivedAt":"2018-03-27T23:07:23Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 27 Mar 2018 14:39:17 -0700\nStefan Beller wrote:\n\n> This continues the story of bf12fcdf5e (submodule-config: store\n> the_submodule_cache in the_repository, 2017-06-22).\n> \n> The previous patch taught submodule_from_path to take a repository into\n> account, such that submodule_from_{path, cache} are the same now.\n> Remove submodule_from_cache, migrating all its callers to\n> submodule_from_path.\n> \n> Signed-off-by: Stefan Beller \n\nObviously correct, since submodule_from_{path,cache} are word-for-word\nidentical (other than parameter names).\n\nReviewed-by: Jonathan Tan \n"},{"id":"343187","messageId":"CAGZ79kY-E5FZRJAg6QG0DX1TzWXgo9LqJ-b7JojpkD6_BdF-wQ@mail.gmail.com","threadId":"48169","inReplyTo":"20180327225850.166523-1-jonathantanmy@google.com","subject":"Re: [PATCH] grep: remove \"repo\" arg from non-supporting funcs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-27T23:20:25Z","receivedAt":"2018-03-27T23:20:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 27, 2018 at 3:58 PM, Jonathan Tan wrote:\n> As part of commit f9ee2fcdfa (\"grep: recurse in-process using 'struct\n> repository'\", 2017-08-02), many functions in builtin/grep.c were\n> converted to also take \"struct repository *\" arguments. Among them were\n> grep_object() and grep_objects().\n>\n> However, at least grep_objects() was converted incompletely - it calls\n> gitmodules_config_oid(), which references the_repository.\n>\n> But it turns out that the conversion was extraneous anyway - there has\n> been no user-visible effect - because grep_objects() is never invoked\n> except with the_repository.\n>\n> Revert the changes to grep_objects() and grep_object() (which conversion\n> is also extraneous) to show that both these functions do not support\n> repositories other than the_repository.\n\nI'd rather convert gitmodules_config_oid instead of reverting the other\nfunctions into a world without an arbitrary repository object.\n\n>\n> Signed-off-by: Jonathan Tan \n> ---\n> Patch 1/5 of your series is obviously correct.\n>\n> I investigated the change to grep_objects() in patch 2/5, and here is a\n> patch summarizing my findings. Consider including this patch before 2/5\n> (or before 1/5). You'll probably need to write\n> \"submodule_free(the_repository);\" instead of what you have currently,\n> but other than that, I don't think this affects your patch set much.\n\nThanks for looking at the patches!\nI'd think this patch is orthogonal to the series, as this is about the effort\nof converting parts of git-grep whereas this series is fixing a bug (by\nconverting parts of the submodule config machinery))?\n\nThanks,\nStefan\n"},{"id":"343188","messageId":"20180327232540.GB215480@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-6-sbeller@google.com","subject":"Re: [PATCH 5/5] submodule: fixup nested submodules after moving the submodule","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-03-27T23:25:40Z","receivedAt":"2018-03-27T23:25:50Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 03/27, Stefan Beller wrote:\n> connect_work_tree_and_git_dir is used to connect a submodule worktree with\n> its git directory and vice versa after events that require a reconnection\n> such as moving around the working tree. As submodules can have nested\n> submoduled themselves, we'd also want to fix the nested submodules when\n> asked to. Add an option to recurse into the nested submodules and connect\n> them as well.\n> \n> As submodules are identified by their name (which determines their git\n> directory in relation to their superprojects git directory) internally\n> and by their path in the working tree of the superproject, we need to\n> make sure that the mapping of name <-> path is kept intact. We can do\n> that in the git-mv command by writing out the gitmodules file and first\n> and then force a reload of the submodule config machinery.\n> \n> Signed-off-by: Stefan Beller \n> ---\n> builtin/mv.c | 6 ++--\n> builtin/submodule--helper.c | 3 +-\n> dir.c | 70 +++++++++++++++++++++++++++++++++++--\n> dir.h | 12 ++++++-\n> submodule.c | 6 ++--\n> t/t7001-mv.sh | 2 +-\n> 6 files changed, 87 insertions(+), 12 deletions(-)\n> \n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 6d141f7a53..7a63667d64 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -276,10 +276,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> \t\t\tdie_errno(_(\"renaming '%s' failed\"), src);\n> \t\t}\n> \t\tif (submodule_gitfile[i]) {\n> -\t\t\tif (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)\n> -\t\t\t\tconnect_work_tree_and_git_dir(dst, submodule_gitfile[i]);\n> \t\t\tif (!update_path_in_gitmodules(src, dst))\n> \t\t\t\tgitmodules_modified = 1;\n> +\t\t\tif (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)\n> +\t\t\t\tconnect_work_tree_and_git_dir(dst,\n> +\t\t\t\t\t\t\t submodule_gitfile[i],\n> +\t\t\t\t\t\t\t 1);\n> \t\t}\n> \n> \t\tif (mode == WORKING_DIRECTORY)\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index a921fbbf56..05fd657f99 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -1259,8 +1259,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \t\tstrbuf_reset(&sb);\n> \t}\n> \n> -\t/* Connect module worktree and git dir */\n> -\tconnect_work_tree_and_git_dir(path, sm_gitdir);\n> +\tconnect_work_tree_and_git_dir(path, sm_gitdir, 0);\n> \n> \tp = git_pathdup_submodule(path, \"config\");\n> \tif (!p)\n> diff --git a/dir.c b/dir.c\n> index dedbf5d476..313176e291 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -19,6 +19,7 @@\n> #include \"varint.h\"\n> #include \"ewah/ewok.h\"\n> #include \"fsmonitor.h\"\n> +#include \"submodule-config.h\"\n> \n> /*\n> * Tells read_directory_recursive how a file or directory should be treated.\n> @@ -3010,8 +3011,67 @@ void untracked_cache_add_to_index(struct index_state *istate,\n> \tuntracked_cache_invalidate_path(istate, path, 1);\n> }\n> \n> -/* Update gitfile and core.worktree setting to connect work tree and git dir */\n> -void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)\n> +static void connect_wt_gitdir_in_nested(const char *sub_worktree,\n> +\t\t\t\t\tconst char *sub_gitdir,\n> +\t\t\t\t\tstruct repository *superproject)\n> +{\n> +\tint i;\n> +\tstruct repository subrepo;\n\nYou never clear this struct which means it leaks the memory it points\nto.\n\n> +\tstruct strbuf sub_wt = STRBUF_INIT;\n> +\tstruct strbuf sub_gd = STRBUF_INIT;\n> +\tconst struct submodule *sub;\n> +\tconst char *super_worktree,\n> +\t\t *sub_path; /* path inside the superproject */\n> +\n> +\t/* subrepo got moved, so superproject has outdated information */\n> +\tsubmodule_free(superproject);\n> +\n> +\tsuper_worktree = real_pathdup(superproject->worktree, 1);\n> +\n> +\tsub_path = sub_worktree + strlen(super_worktree) + 1;\n> +\n> +\tif (repo_submodule_init(&subrepo, superproject, sub_path))\n> +\t\treturn;\n> +\n> +\trepo_read_index(&subrepo);\n\nYou may want to check the return value to see if reading the index was\nsuccessful.\n\n> +\n> +\tfor (i = 0; i < subrepo.index->cache_nr; i++) {\n> +\t\tconst struct cache_entry *ce = subrepo.index->cache[i];\n> +\n> +\t\tif (!S_ISGITLINK(ce->ce_mode))\n> +\t\t\tcontinue;\n> +\n> +\t\twhile (i + 1 < subrepo.index->cache_nr &&\n> +\t\t !strcmp(ce->name, subrepo.index->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> +\n> +\t\tsub = submodule_from_path(&subrepo, &null_oid, ce->name);\n> +\t\tif (!sub)\n> +\t\t\t/* submodule not checked out? */\n> +\t\t\tcontinue;\n> +\n> +\t\tstrbuf_reset(&sub_wt);\n> +\t\tstrbuf_addf(&sub_wt, \"%s/%s/.git\", sub_worktree, sub->path);\n> +\n> +\t\tstrbuf_reset(&sub_gd);\n> +\t\tstrbuf_addf(&sub_gd, \"%s/modules/%s\", sub_gitdir, sub->name);\n> +\n> +\t\tstrbuf_setlen(&sub_wt, sub_wt.len - strlen(\"/.git\"));\n> +\n> +\t\tif (is_submodule_active(&subrepo, ce->name)) {\n> +\t\t\tconnect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 0);\n> +\t\t\tconnect_wt_gitdir_in_nested(sub_wt.buf, sub_gd.buf, &subrepo);\n> +\t\t}\n> +\t}\n> +}\n> +\n> +void connect_work_tree_and_git_dir(const char *work_tree_,\n> +\t\t\t\t const char *git_dir_,\n> +\t\t\t\t int recurse_into_nested)\n> {\n> \tstruct strbuf gitfile_sb = STRBUF_INIT;\n> \tstruct strbuf cfg_sb = STRBUF_INIT;\n> @@ -3041,6 +3101,10 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)\n> \tstrbuf_release(&gitfile_sb);\n> \tstrbuf_release(&cfg_sb);\n> \tstrbuf_release(&rel_path);\n> +\n> +\tif (recurse_into_nested)\n> +\t\tconnect_wt_gitdir_in_nested(work_tree, git_dir, the_repository);\n> +\n> \tfree(work_tree);\n> \tfree(git_dir);\n> }\n> @@ -3054,5 +3118,5 @@ void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_\n> \t\tdie_errno(_(\"could not migrate git directory from '%s' to '%s'\"),\n> \t\t\told_git_dir, new_git_dir);\n> \n> -\tconnect_work_tree_and_git_dir(path, new_git_dir);\n> +\tconnect_work_tree_and_git_dir(path, new_git_dir, 0);\n> }\n> diff --git a/dir.h b/dir.h\n> index b0758b82a2..3870193e52 100644\n> --- a/dir.h\n> +++ b/dir.h\n> @@ -359,7 +359,17 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n> void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);\n> void add_untracked_cache(struct index_state *istate);\n> void remove_untracked_cache(struct index_state *istate);\n> -extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n> +\n> +/*\n> + * Connect a worktree to a git directory by creating (or overwriting) a\n> + * '.git' file containing the location of the git directory. In the git\n> + * directory set the core.worktree setting to indicate where the worktree is.\n> + * When `recurse_into_nested` is set, recurse into any nested submodules,\n> + * connecting them as well.\n> + */\n> +extern void connect_work_tree_and_git_dir(const char *work_tree,\n> +\t\t\t\t\t const char *git_dir,\n> +\t\t\t\t\t int recurse_into_nested);\n> extern void relocate_gitdir(const char *path,\n> \t\t\t const char *old_git_dir,\n> \t\t\t const char *new_git_dir);\n> diff --git a/submodule.c b/submodule.c\n> index 89d0aee086..c2dac6c00f 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -1624,7 +1624,7 @@ int submodule_move_head(const char *path,\n> \t\t} else {\n> \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n> \t\t\t\t get_git_common_dir(), sub->name);\n> -\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n> +\t\t\tconnect_work_tree_and_git_dir(path, gitdir, 0);\n> \t\t\tfree(gitdir);\n> \n> \t\t\t/* make sure the index is clean as well */\n> @@ -1634,7 +1634,7 @@ int submodule_move_head(const char *path,\n> \t\tif (old_head && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {\n> \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n> \t\t\t\t get_git_common_dir(), sub->name);\n> -\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n> +\t\t\tconnect_work_tree_and_git_dir(path, gitdir, 1);\n> \t\t\tfree(gitdir);\n> \t\t}\n> \t}\n> @@ -1947,7 +1947,7 @@ void absorb_git_dir_into_superproject(const char *prefix,\n> \t\tif (!sub)\n> \t\t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n> \t\tconnect_work_tree_and_git_dir(path,\n> -\t\t\tgit_path(\"modules/%s\", sub->name));\n> +\t\t\tgit_path(\"modules/%s\", sub->name), 0);\n> \t} else {\n> \t\t/* Is it already absorbed into the superprojects git dir? */\n> \t\tchar *real_sub_git_dir = real_pathdup(sub_git_dir, 1);\n> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\n> index d4e6485a26..ff70244620 100755\n> --- a/t/t7001-mv.sh\n> +++ b/t/t7001-mv.sh\n> @@ -497,7 +497,7 @@ test_expect_success 'moving a submodule in nested directories' '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_failure 'moving nested submodules' '\n> +test_expect_success 'moving nested submodules' '\n> \tgit commit -am \"cleanup commit\" &&\n> \tmkdir sub_nested_nested &&\n> \t(cd sub_nested_nested &&\n> -- \n> 2.17.0.rc1.321.gba9d0f2565-goog\n> \n\n-- \nBrandon Williams\n"},{"id":"343191","messageId":"CAGZ79kbisNdO_9A_sH0w+FU5q6mhtgynd6BuY8UoWaC-X6twbg@mail.gmail.com","threadId":"48169","inReplyTo":"20180327160443.5034f7db908c08fe3c301ab9@google.com","subject":"Re: [PATCH 3/5] submodule-config: add repository argument to submodule_from_{name, path}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-27T23:53:22Z","receivedAt":"2018-03-27T23:53:27Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 27, 2018 at 4:04 PM, Jonathan Tan wrote:\n> On Tue, 27 Mar 2018 14:39:16 -0700\n> Stefan Beller wrote:\n>\n>> -extern const struct submodule *submodule_from_name(\n>> +extern const struct submodule *submodule_from_name(struct repository *r,\n>> const struct object_id *commit_or_tree, const char *name);\n>> -extern const struct submodule *submodule_from_path(\n>> +extern const struct submodule *submodule_from_path(struct repository *r,\n>> const struct object_id *commit_or_tree, const char *path);\n>\n> There is a recent change to CodingGuidelines that states to not include\n> \"extern\" in 89a9f2c862 (\"CodingGuidelines: mention \"static\" and\n> \"extern\"\", 2018-02-08), so consider removing \"extern\" since you're\n> already changing this file.\n\nGah!\n\nI have that fixed locally; it will be included in the next version\n\n>\n> Other than that,\n>\n> Reviewed-by: Jonathan Tan \n\nThanks!\n"},{"id":"343192","messageId":"20180327170714.81b37f84cb4c544d76f34701@google.com","threadId":"48169","inReplyTo":"20180327213918.77851-6-sbeller@google.com","subject":"Re: [PATCH 5/5] submodule: fixup nested submodules after moving the submodule","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-03-28T00:07:14Z","receivedAt":"2018-03-28T00:07:22Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 27 Mar 2018 14:39:18 -0700\nStefan Beller wrote:\n\n> connect_work_tree_and_git_dir is used to connect a submodule worktree with\n> its git directory and vice versa after events that require a reconnection\n> such as moving around the working tree. As submodules can have nested\n> submoduled themselves, we'd also want to fix the nested submodules when\n\ns/submoduled/submodules\n\n> asked to. Add an option to recurse into the nested submodules and connect\n> them as well.\n> \n> As submodules are identified by their name (which determines their git\n> directory in relation to their superprojects git directory) internally\n\ns/superprojects/superproject's/\n\n> and by their path in the working tree of the superproject, we need to\n> make sure that the mapping of name <-> path is kept intact. We can do\n> that in the git-mv command by writing out the gitmodules file and first\n\ns/and //\n\n> and then force a reload of the submodule config machinery.\n\ns/force/forcing/\n\n> \n> Signed-off-by: Stefan Beller \n\n[snip]\n\n> +static void connect_wt_gitdir_in_nested(const char *sub_worktree,\n> +\t\t\t\t\tconst char *sub_gitdir,\n> +\t\t\t\t\tstruct repository *superproject)\n> +{\n> +\tint i;\n> +\tstruct repository subrepo;\n> +\tstruct strbuf sub_wt = STRBUF_INIT;\n> +\tstruct strbuf sub_gd = STRBUF_INIT;\n> +\tconst struct submodule *sub;\n> +\tconst char *super_worktree,\n> +\t\t *sub_path; /* path inside the superproject */\n> +\n> +\t/* subrepo got moved, so superproject has outdated information */\n> +\tsubmodule_free(superproject);\n> +\n> +\tsuper_worktree = real_pathdup(superproject->worktree, 1);\n> +\n> +\tsub_path = sub_worktree + strlen(super_worktree) + 1;\n> +\n> +\tif (repo_submodule_init(&subrepo, superproject, sub_path))\n> +\t\treturn;\n> +\n> +\trepo_read_index(&subrepo);\n\nFrom the name of this function and its usage in\nconnect_work_tree_and_git_dir(), I expected this function to just\niterate through all the files in its workdir (which it is doing in the\n\"for\" loop below) and connect any nested submodules. Why does it need\naccess to its superproject? (I would think that repo_init() would be\nsufficient here instead of repo_submodule_init().)\n\n> +\n> +\tfor (i = 0; i < subrepo.index->cache_nr; i++) {\n> +\t\tconst struct cache_entry *ce = subrepo.index->cache[i];\n> +\n> +\t\tif (!S_ISGITLINK(ce->ce_mode))\n> +\t\t\tcontinue;\n> +\n> +\t\twhile (i + 1 < subrepo.index->cache_nr &&\n> +\t\t !strcmp(ce->name, subrepo.index->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> +\n> +\t\tsub = submodule_from_path(&subrepo, &null_oid, ce->name);\n> +\t\tif (!sub)\n> +\t\t\t/* submodule not checked out? */\n> +\t\t\tcontinue;\n> +\n> +\t\tstrbuf_reset(&sub_wt);\n> +\t\tstrbuf_addf(&sub_wt, \"%s/%s/.git\", sub_worktree, sub->path);\n> +\n> +\t\tstrbuf_reset(&sub_gd);\n> +\t\tstrbuf_addf(&sub_gd, \"%s/modules/%s\", sub_gitdir, sub->name);\n> +\n> +\t\tstrbuf_setlen(&sub_wt, sub_wt.len - strlen(\"/.git\"));\n> +\n> +\t\tif (is_submodule_active(&subrepo, ce->name)) {\n> +\t\t\tconnect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 0);\n> +\t\t\tconnect_wt_gitdir_in_nested(sub_wt.buf, sub_gd.buf, &subrepo);\n\nThe modifications of sub_wt and sub_gd should probably go here, since\nthey are not used unless this \"if\" block is executed.\n\n> +void connect_work_tree_and_git_dir(const char *work_tree_,\n> +\t\t\t\t const char *git_dir_,\n> +\t\t\t\t int recurse_into_nested)\n\nHow is this function expected to be used? From what I see:\n - if recurse_into_nested is 0, this works regardless of whether the\n work_tree_ and git_dir_ is directly or indirectly a submodule of\n the_repository\n - if recurse_into_nested is 1, work_tree_ and git_dir_ must be directly\n a submodule of the_repository (since it is referenced directly)\n\nThis seems confusing - is this expected?\n"},{"id":"343196","messageId":"20180327172451.34f60c483133df629b1e8ec6@google.com","threadId":"48169","inReplyTo":"CAGZ79kY-E5FZRJAg6QG0DX1TzWXgo9LqJ-b7JojpkD6_BdF-wQ@mail.gmail.com","subject":"Re: [PATCH] grep: remove \"repo\" arg from non-supporting funcs","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-03-28T00:24:51Z","receivedAt":"2018-03-28T00:24:58Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 27 Mar 2018 16:20:25 -0700\nStefan Beller wrote:\n\n> On Tue, Mar 27, 2018 at 3:58 PM, Jonathan Tan wrote:\n> > As part of commit f9ee2fcdfa (\"grep: recurse in-process using 'struct\n> > repository'\", 2017-08-02), many functions in builtin/grep.c were\n> > converted to also take \"struct repository *\" arguments. Among them were\n> > grep_object() and grep_objects().\n> >\n> > However, at least grep_objects() was converted incompletely - it calls\n> > gitmodules_config_oid(), which references the_repository.\n> >\n> > But it turns out that the conversion was extraneous anyway - there has\n> > been no user-visible effect - because grep_objects() is never invoked\n> > except with the_repository.\n> >\n> > Revert the changes to grep_objects() and grep_object() (which conversion\n> > is also extraneous) to show that both these functions do not support\n> > repositories other than the_repository.\n> \n> I'd rather convert gitmodules_config_oid instead of reverting the other\n> functions into a world without an arbitrary repository object.\n\nI don't really think of it as a reversion, since grep_objects() didn't\nfully support repos other than the_repository anyway.\n\nBesides that, that's fine, but then:\n\n (1) You would have to explain both in the gitmodules_config_oid() and\n in this patch why \"gitmodules_config_oid(...)\" ->\n \"gitmodules_config_oid(repo, ...)\" and \"submodule_free()\" ->\n \"submodule_free(repo)\" are safe, since they have different behavior\n upon first glance. (They have the same behavior only because\n grep_objects() is always called with the_repository.) I was trying\n to explain this in as clear a way as possible, by showing (with\n code) that grep_objects() only works with, and is only called with,\n the_repository.\n (2) The code path where repo != the_repository would still never be\n exercised, and I prefer to not have such code paths.\n\nI don't feel too strongly about (1), since they just concern commit\nmessages, of which there are many valid opinions on how to write them. I\nfeel a bit more strongly about (2), but can concede my point if the\nproject is OK with a code path not being exercised.\n\n> Thanks for looking at the patches!\n> I'd think this patch is orthogonal to the series, as this is about the effort\n> of converting parts of git-grep whereas this series is fixing a bug (by\n> converting parts of the submodule config machinery))?\n\nIt is orthogonal in the same way as your patch 1/5, I think - a\npreparatory patch to make your \"real\" patches easier to understand.\n"},{"id":"343197","messageId":"CAGZ79kYb_a3oXCPAyL=140pyQb=CmzSWx2wVVOcyXy2VPSLNZQ@mail.gmail.com","threadId":"48169","inReplyTo":"20180327172451.34f60c483133df629b1e8ec6@google.com","subject":"Re: [PATCH] grep: remove \"repo\" arg from non-supporting funcs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T00:35:04Z","receivedAt":"2018-03-28T00:35:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 27, 2018 at 5:24 PM, Jonathan Tan wrote:\n> On Tue, 27 Mar 2018 16:20:25 -0700\n> Stefan Beller wrote:\n>\n>> On Tue, Mar 27, 2018 at 3:58 PM, Jonathan Tan wrote:\n>> > As part of commit f9ee2fcdfa (\"grep: recurse in-process using 'struct\n>> > repository'\", 2017-08-02), many functions in builtin/grep.c were\n>> > converted to also take \"struct repository *\" arguments. Among them were\n>> > grep_object() and grep_objects().\n>> >\n>> > However, at least grep_objects() was converted incompletely - it calls\n>> > gitmodules_config_oid(), which references the_repository.\n>> >\n>> > But it turns out that the conversion was extraneous anyway - there has\n>> > been no user-visible effect - because grep_objects() is never invoked\n>> > except with the_repository.\n>> >\n>> > Revert the changes to grep_objects() and grep_object() (which conversion\n>> > is also extraneous) to show that both these functions do not support\n>> > repositories other than the_repository.\n>>\n>> I'd rather convert gitmodules_config_oid instead of reverting the other\n>> functions into a world without an arbitrary repository object.\n>\n> I don't really think of it as a reversion, since grep_objects() didn't\n> fully support repos other than the_repository anyway.\n>\n> Besides that, that's fine, but then:\n>\n> (1) You would have to explain both in the gitmodules_config_oid() and\n> in this patch why \"gitmodules_config_oid(...)\" ->\n> \"gitmodules_config_oid(repo, ...)\" and \"submodule_free()\" ->\n> \"submodule_free(repo)\" are safe, since they have different behavior\n> upon first glance. (They have the same behavior only because\n> grep_objects() is always called with the_repository.) I was trying\n> to explain this in as clear a way as possible, by showing (with\n> code) that grep_objects() only works with, and is only called with,\n> the_repository.\n> (2) The code path where repo != the_repository would still never be\n> exercised, and I prefer to not have such code paths.\n>\n> I don't feel too strongly about (1), since they just concern commit\n> messages, of which there are many valid opinions on how to write them. I\n> feel a bit more strongly about (2), but can concede my point if the\n> project is OK with a code path not being exercised.\n\nOk. I admit not having looked at the code beforehand, and I was\njust arguing based on intuition. Turns out I was wrong.\n\nI thought that we'd have to have arbitrary repos for proper submodule\nrecursion, but this specific code is for grepping thru given tree objects,\nwhich for now is assumed that the user can only give trees of\nthe_repository and we'd error out in case of a submodule tree.\nMakes sense.\n\nIn that case, I agree with your patch. Thanks for writing it.\nI'll take it into the series.\n\nThanks,\nStefan\n"},{"id":"343198","messageId":"CAGZ79kaQYjepND8EdgB73meBsShOc5aBporiK2Bo40fqcar0gg@mail.gmail.com","threadId":"48169","inReplyTo":"20180327170714.81b37f84cb4c544d76f34701@google.com","subject":"Re: [PATCH 5/5] submodule: fixup nested submodules after moving the submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T00:42:58Z","receivedAt":"2018-03-28T00:43:04Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Mar 27, 2018 at 5:07 PM, Jonathan Tan wrote:\n\n> s/submoduled/submodules\n\n> s/superprojects/superproject's/\n\n> s/and //\n>\n> s/force/forcing/\n\nAll wording fixed.\n\n>> + sub_path = sub_worktree + strlen(super_worktree) + 1;\n>> +\n>> + if (repo_submodule_init(&subrepo, superproject, sub_path))\n>> + return;\n>> +\n>> + repo_read_index(&subrepo);\n>\n> From the name of this function and its usage in\n> connect_work_tree_and_git_dir(), I expected this function to just\n> iterate through all the files in its workdir (which it is doing in the\n> \"for\" loop below) and connect any nested submodules. Why does it need\n> access to its superproject? (I would think that repo_init() would be\n> sufficient here instead of repo_submodule_init().)\n\nTesting validates your thinking (for now).\n\nIf we ever want to have good error reporting (see bmwills hint to\ncheck for index corruption), we may want to have all the repos constructed\nas submodules from the_repository, as then the error messages might\nbe better (e.g. in the future we could display the\n\"submodule nesting stack\").\n\nI'll remove the superproject argument for now.\n\n>> +\n>> + strbuf_reset(&sub_wt);\n>> + strbuf_addf(&sub_wt, \"%s/%s/.git\", sub_worktree, sub->path);\n>> +\n>> + strbuf_reset(&sub_gd);\n>> + strbuf_addf(&sub_gd, \"%s/modules/%s\", sub_gitdir, sub->name);\n>> +\n>> + strbuf_setlen(&sub_wt, sub_wt.len - strlen(\"/.git\"));\n>> +\n>> + if (is_submodule_active(&subrepo, ce->name)) {\n>> + connect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 0);\n>> + connect_wt_gitdir_in_nested(sub_wt.buf, sub_gd.buf, &subrepo);\n>\n> The modifications of sub_wt and sub_gd should probably go here, since\n> they are not used unless this \"if\" block is executed.\n\nThanks! I also cut out the setlen call by giving the correct format string.\n(The code presented here is not very old, I just fired it off as soon as the\ntest passed)\n\n>\n>> +void connect_work_tree_and_git_dir(const char *work_tree_,\n>> + const char *git_dir_,\n>> + int recurse_into_nested)\n>\n> How is this function expected to be used? From what I see:\n> - if recurse_into_nested is 0, this works regardless of whether the\n> work_tree_ and git_dir_ is directly or indirectly a submodule of\n> the_repository\n> - if recurse_into_nested is 1, work_tree_ and git_dir_ must be directly\n> a submodule of the_repository (since it is referenced directly)\n>\n> This seems confusing - is this expected?\n\nIn the next revision of the series connect_wt_gitdir_in_nested\nwill no longer have a third argument for the repo, as we use repo_init.\n\nThat eases the handling a bit here.\n"},{"id":"343229","messageId":"20180328172449.27012-1-sbeller@google.com","threadId":"48169","inReplyTo":"CAGZ79kaQYjepND8EdgB73meBsShOc5aBporiK2Bo40fqcar0gg@mail.gmail.com","subject":"[PATCHv2 0/6] Moving submodules with nested submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T17:24:43Z","receivedAt":"2018-03-28T17:25:00Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"v2:\n* addressed memleaks and messy code in patch 5\n* removed the extern keyword where applicable\n* extended the commit message, stating we want to rename submodule_free\n in the future.\n* picked up Jonathans patch and added it as a nice finish of the series.\n I did not see the need or aesthetic desire to put that patch earlier\n in the series.\n \nThanks,\nStefan\n\nv1:\n\nThis fixes the bug reported in [1] (\"Bug: moving submodules that have submodules\ninside them causes a fatal error in git status\")\n\n[1] https://public-inbox.org/git/20180306192017.GA5797@riseup.net/\n\nThanks,\nStefan\n\nJonathan Tan (1):\n grep: remove \"repo\" arg from non-supporting funcs\n\nStefan Beller (5):\n submodule.h: drop declaration of connect_work_tree_and_git_dir\n submodule-config: allow submodule_free to handle arbitrary\n repositories\n submodule-config: add repository argument to submodule_from_{name,\n path}\n submodule-config: remove submodule_from_cache\n submodule: fixup nested submodules after moving the submodule\n\n .../technical/api-submodule-config.txt | 2 +-\n builtin/grep.c | 14 ++---\n builtin/mv.c | 6 +-\n builtin/submodule--helper.c | 17 +++--\n dir.c | 63 ++++++++++++++++++-\n dir.h | 12 +++-\n repository.c | 2 +-\n submodule-config.c | 29 ++++-----\n submodule-config.h | 15 +++--\n submodule.c | 40 ++++++------\n submodule.h | 1 -\n t/helper/test-submodule-config.c | 8 ++-\n t/t7001-mv.sh | 2 +-\n unpack-trees.c | 2 +-\n 14 files changed, 137 insertions(+), 76 deletions(-)\n\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343230","messageId":"20180328172449.27012-2-sbeller@google.com","threadId":"48169","inReplyTo":"20180328172449.27012-1-sbeller@google.com","subject":"[PATCH 1/6] submodule.h: drop declaration of connect_work_tree_and_git_dir","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T17:24:44Z","receivedAt":"2018-03-28T17:25:02Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The function connect_work_tree_and_git_dir is declared in both submodule.h\nand dir.h, such that one of them is redundant. As the function is\nimplemented in dir.c, drop the declaration from submodule.h\n\nSigned-off-by: Stefan Beller \n---\n submodule.h | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/submodule.h b/submodule.h\nindex 9589f13127..e5526f6aaa 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -105,7 +105,6 @@ extern int push_unpushed_submodules(struct oid_array *commits,\n \t\t\t\t const char **refspec, int refspec_nr,\n \t\t\t\t const struct string_list *push_options,\n \t\t\t\t int dry_run);\n-extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n /*\n * Given a submodule path (as in the index), return the repository\n * path of that submodule in 'buf'. Return -1 on error or when the\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343231","messageId":"20180328172449.27012-3-sbeller@google.com","threadId":"48169","inReplyTo":"20180328172449.27012-1-sbeller@google.com","subject":"[PATCH 2/6] submodule-config: allow submodule_free to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T17:24:45Z","receivedAt":"2018-03-28T17:25:04Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"At some point we may want to rename the function so that it describes what\nit actually does as 'submodule_free' doesn't quite describe that this\nclears a repository's submodule cache. But that's beyond the scope of\nthis series.\n\nWhile at it remove the extern key word from its declaration.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/technical/api-submodule-config.txt | 2 +-\n builtin/grep.c | 2 +-\n submodule-config.c | 6 +++---\n submodule-config.h | 2 +-\n t/helper/test-submodule-config.c | 2 +-\n unpack-trees.c | 2 +-\n 6 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt\nindex ee907c4a82..fb06089393 100644\n--- a/Documentation/technical/api-submodule-config.txt\n+++ b/Documentation/technical/api-submodule-config.txt\n@@ -38,7 +38,7 @@ Data Structures\n Functions\n ---------\n \n-`void submodule_free()`::\n+`void submodule_free(struct repository *r)`::\n \n \tUse these to free the internally cached values.\n \ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 789a89133a..8f04cde18e 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -651,7 +651,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n \n \t\t/* load the gitmodules file for this rev */\n \t\tif (recurse_submodules) {\n-\t\t\tsubmodule_free();\n+\t\t\tsubmodule_free(repo);\n \t\t\tgitmodules_config_oid(&real_obj->oid);\n \t\t}\n \t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].path,\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 602ba8ca8b..a3efff1a34 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -642,8 +642,8 @@ const struct submodule *submodule_from_cache(struct repository *repo,\n \t\t\t key, lookup_path);\n }\n \n-void submodule_free(void)\n+void submodule_free(struct repository *r)\n {\n-\tif (the_repository->submodule_cache)\n-\t\tsubmodule_cache_clear(the_repository->submodule_cache);\n+\tif (r->submodule_cache)\n+\t\tsubmodule_cache_clear(r->submodule_cache);\n }\ndiff --git a/submodule-config.h b/submodule-config.h\nindex a5503a5d17..6b71a8cd30 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -46,6 +46,6 @@ extern const struct submodule *submodule_from_path(\n extern const struct submodule *submodule_from_cache(struct repository *repo,\n \t\t\t\t\t\t const struct object_id *treeish_name,\n \t\t\t\t\t\t const char *key);\n-extern void submodule_free(void);\n+void submodule_free(struct repository *r);\n \n #endif /* SUBMODULE_CONFIG_H */\ndiff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c\nindex f23db3b19a..9971c5e9dd 100644\n--- a/t/helper/test-submodule-config.c\n+++ b/t/helper/test-submodule-config.c\n@@ -64,7 +64,7 @@ int cmd_main(int argc, const char **argv)\n \t\targ += 2;\n \t}\n \n-\tsubmodule_free();\n+\tsubmodule_free(the_repository);\n \n \treturn 0;\n }\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex d5685891a5..05e5fa77eb 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -290,7 +290,7 @@ static void load_gitmodules_file(struct index_state *index,\n \t\tif (!state && ce->ce_flags & CE_WT_REMOVE) {\n \t\t\trepo_read_gitmodules(the_repository);\n \t\t} else if (state && (ce->ce_flags & CE_UPDATE)) {\n-\t\t\tsubmodule_free();\n+\t\t\tsubmodule_free(the_repository);\n \t\t\tcheckout_entry(ce, state, NULL);\n \t\t\trepo_read_gitmodules(the_repository);\n \t\t}\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343232","messageId":"20180328172449.27012-5-sbeller@google.com","threadId":"48169","inReplyTo":"20180328172449.27012-1-sbeller@google.com","subject":"[PATCH 4/6] submodule-config: remove submodule_from_cache","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T17:24:47Z","receivedAt":"2018-03-28T17:25:07Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This continues the story of bf12fcdf5e (submodule-config: store\nthe_submodule_cache in the_repository, 2017-06-22).\n\nThe previous patch taught submodule_from_path to take a repository into\naccount, such that submodule_from_{path, cache} are the same now.\nRemove submodule_from_cache, migrating all its callers to\nsubmodule_from_path.\n\nReviewed-by: Jonathan Tan \nSigned-off-by: Stefan Beller \n---\n repository.c | 2 +-\n submodule-config.c | 9 ---------\n submodule-config.h | 3 ---\n submodule.c | 4 ++--\n 4 files changed, 3 insertions(+), 15 deletions(-)\n\ndiff --git a/repository.c b/repository.c\nindex 4ffbe9bc94..fa0a132e22 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -167,7 +167,7 @@ int repo_submodule_init(struct repository *submodule,\n \tstruct strbuf worktree = STRBUF_INIT;\n \tint ret = 0;\n \n-\tsub = submodule_from_cache(superproject, &null_oid, path);\n+\tsub = submodule_from_path(superproject, &null_oid, path);\n \tif (!sub) {\n \t\tret = -1;\n \t\tgoto out;\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 4b7803e6ed..cb65354d4c 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -635,15 +635,6 @@ const struct submodule *submodule_from_path(struct repository *r,\n \treturn config_from(r->submodule_cache, treeish_name, path, lookup_path);\n }\n \n-const struct submodule *submodule_from_cache(struct repository *repo,\n-\t\t\t\t\t const struct object_id *treeish_name,\n-\t\t\t\t\t const char *key)\n-{\n-\tgitmodules_read_check(repo);\n-\treturn config_from(repo->submodule_cache, treeish_name,\n-\t\t\t key, lookup_path);\n-}\n-\n void submodule_free(struct repository *r)\n {\n \tif (r->submodule_cache)\ndiff --git a/submodule-config.h b/submodule-config.h\nindex 43dfe7dec0..6f686184e8 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -45,9 +45,6 @@ const struct submodule *submodule_from_name(struct repository *r,\n const struct submodule *submodule_from_path(struct repository *r,\n \t\t\t\t\t const struct object_id *commit_or_tree,\n \t\t\t\t\t const char *path);\n-extern const struct submodule *submodule_from_cache(struct repository *repo,\n-\t\t\t\t\t\t const struct object_id *treeish_name,\n-\t\t\t\t\t\t const char *key);\n void submodule_free(struct repository *r);\n \n #endif /* SUBMODULE_CONFIG_H */\ndiff --git a/submodule.c b/submodule.c\nindex e94b7f9acd..89d0aee086 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -230,7 +230,7 @@ int is_submodule_active(struct repository *repo, const char *path)\n \tconst struct string_list *sl;\n \tconst struct submodule *module;\n \n-\tmodule = submodule_from_cache(repo, &null_oid, path);\n+\tmodule = submodule_from_path(repo, &null_oid, path);\n \n \t/* early return if there isn't a path->module mapping */\n \tif (!module)\n@@ -1235,7 +1235,7 @@ static int get_next_submodule(struct child_process *cp,\n \t\tif (!S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n \n-\t\tsubmodule = submodule_from_cache(spf->r, &null_oid, ce->name);\n+\t\tsubmodule = submodule_from_path(spf->r, &null_oid, ce->name);\n \t\tif (!submodule) {\n \t\t\tconst char *name = default_name_or_path(ce->name);\n \t\t\tif (name) {\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343233","messageId":"20180328172449.27012-6-sbeller@google.com","threadId":"48169","inReplyTo":"20180328172449.27012-1-sbeller@google.com","subject":"[PATCH 5/6] submodule: fixup nested submodules after moving the submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T17:24:48Z","receivedAt":"2018-03-28T17:25:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"connect_work_tree_and_git_dir is used to connect a submodule worktree with\nits git directory and vice versa after events that require a reconnection\nsuch as moving around the working tree. As submodules can have nested\nsubmodules themselves, we'd also want to fix the nested submodules when\nasked to. Add an option to recurse into the nested submodules and connect\nthem as well.\n\nAs submodules are identified by their name (which determines their git\ndirectory in relation to their superproject's git directory) internally\nand by their path in the working tree of the superproject, we need to\nmake sure that the mapping of name <-> path is kept intact. We can do\nthat in the git-mv command by writing out the gitmodules file first\nand then forcing a reload of the submodule config machinery.\n\nSigned-off-by: Stefan Beller \n---\n builtin/mv.c | 6 ++--\n builtin/submodule--helper.c | 3 +-\n dir.c | 63 +++++++++++++++++++++++++++++++++++--\n dir.h | 12 ++++++-\n submodule.c | 6 ++--\n t/t7001-mv.sh | 2 +-\n 6 files changed, 80 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex 6d141f7a53..7a63667d64 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -276,10 +276,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\tdie_errno(_(\"renaming '%s' failed\"), src);\n \t\t}\n \t\tif (submodule_gitfile[i]) {\n-\t\t\tif (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)\n-\t\t\t\tconnect_work_tree_and_git_dir(dst, submodule_gitfile[i]);\n \t\t\tif (!update_path_in_gitmodules(src, dst))\n \t\t\t\tgitmodules_modified = 1;\n+\t\t\tif (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)\n+\t\t\t\tconnect_work_tree_and_git_dir(dst,\n+\t\t\t\t\t\t\t submodule_gitfile[i],\n+\t\t\t\t\t\t\t 1);\n \t\t}\n \n \t\tif (mode == WORKING_DIRECTORY)\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a921fbbf56..05fd657f99 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1259,8 +1259,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_reset(&sb);\n \t}\n \n-\t/* Connect module worktree and git dir */\n-\tconnect_work_tree_and_git_dir(path, sm_gitdir);\n+\tconnect_work_tree_and_git_dir(path, sm_gitdir, 0);\n \n \tp = git_pathdup_submodule(path, \"config\");\n \tif (!p)\ndiff --git a/dir.c b/dir.c\nindex dedbf5d476..71947c0ef3 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -19,6 +19,7 @@\n #include \"varint.h\"\n #include \"ewah/ewok.h\"\n #include \"fsmonitor.h\"\n+#include \"submodule-config.h\"\n \n /*\n * Tells read_directory_recursive how a file or directory should be treated.\n@@ -3010,8 +3011,60 @@ void untracked_cache_add_to_index(struct index_state *istate,\n \tuntracked_cache_invalidate_path(istate, path, 1);\n }\n \n-/* Update gitfile and core.worktree setting to connect work tree and git dir */\n-void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)\n+static void connect_wt_gitdir_in_nested(const char *sub_worktree,\n+\t\t\t\t\tconst char *sub_gitdir)\n+{\n+\tint i;\n+\tstruct repository subrepo;\n+\tstruct strbuf sub_wt = STRBUF_INIT;\n+\tstruct strbuf sub_gd = STRBUF_INIT;\n+\n+\tconst struct submodule *sub;\n+\n+\tif (repo_init(&subrepo, sub_gitdir, sub_worktree))\n+\t\treturn;\n+\n+\tif (repo_read_index(&subrepo) < 0)\n+\t\tdie(\"index file corrupt in repo %s\", subrepo.gitdir);\n+\n+\tfor (i = 0; i < subrepo.index->cache_nr; i++) {\n+\t\tconst struct cache_entry *ce = subrepo.index->cache[i];\n+\n+\t\tif (!S_ISGITLINK(ce->ce_mode))\n+\t\t\tcontinue;\n+\n+\t\twhile (i + 1 < subrepo.index->cache_nr &&\n+\t\t !strcmp(ce->name, subrepo.index->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+\n+\t\tsub = submodule_from_path(&subrepo, &null_oid, ce->name);\n+\t\tif (!sub)\n+\t\t\t/* submodule not checked out? */\n+\t\t\tcontinue;\n+\n+\t\tif (is_submodule_active(&subrepo, ce->name)) {\n+\t\t\tstrbuf_addf(&sub_wt, \"%s/%s\", sub_worktree, sub->path);\n+\t\t\tstrbuf_addf(&sub_gd, \"%s/modules/%s\", sub_gitdir, sub->name);\n+\n+\t\t\tconnect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 0);\n+\t\t\tconnect_wt_gitdir_in_nested(sub_wt.buf, sub_gd.buf);\n+\n+\t\t\tstrbuf_reset(&sub_wt);\n+\t\t\tstrbuf_reset(&sub_gd);\n+\t\t}\n+\t}\n+\tstrbuf_release(&sub_wt);\n+\tstrbuf_release(&sub_gd);\n+\trepo_clear(&subrepo);\n+}\n+\n+void connect_work_tree_and_git_dir(const char *work_tree_,\n+\t\t\t\t const char *git_dir_,\n+\t\t\t\t int recurse_into_nested)\n {\n \tstruct strbuf gitfile_sb = STRBUF_INIT;\n \tstruct strbuf cfg_sb = STRBUF_INIT;\n@@ -3041,6 +3094,10 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)\n \tstrbuf_release(&gitfile_sb);\n \tstrbuf_release(&cfg_sb);\n \tstrbuf_release(&rel_path);\n+\n+\tif (recurse_into_nested)\n+\t\tconnect_wt_gitdir_in_nested(work_tree, git_dir);\n+\n \tfree(work_tree);\n \tfree(git_dir);\n }\n@@ -3054,5 +3111,5 @@ void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_\n \t\tdie_errno(_(\"could not migrate git directory from '%s' to '%s'\"),\n \t\t\told_git_dir, new_git_dir);\n \n-\tconnect_work_tree_and_git_dir(path, new_git_dir);\n+\tconnect_work_tree_and_git_dir(path, new_git_dir, 0);\n }\ndiff --git a/dir.h b/dir.h\nindex b0758b82a2..3870193e52 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -359,7 +359,17 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);\n void add_untracked_cache(struct index_state *istate);\n void remove_untracked_cache(struct index_state *istate);\n-extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n+\n+/*\n+ * Connect a worktree to a git directory by creating (or overwriting) a\n+ * '.git' file containing the location of the git directory. In the git\n+ * directory set the core.worktree setting to indicate where the worktree is.\n+ * When `recurse_into_nested` is set, recurse into any nested submodules,\n+ * connecting them as well.\n+ */\n+extern void connect_work_tree_and_git_dir(const char *work_tree,\n+\t\t\t\t\t const char *git_dir,\n+\t\t\t\t\t int recurse_into_nested);\n extern void relocate_gitdir(const char *path,\n \t\t\t const char *old_git_dir,\n \t\t\t const char *new_git_dir);\ndiff --git a/submodule.c b/submodule.c\nindex 89d0aee086..c2dac6c00f 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1624,7 +1624,7 @@ int submodule_move_head(const char *path,\n \t\t} else {\n \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n \t\t\t\t get_git_common_dir(), sub->name);\n-\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n+\t\t\tconnect_work_tree_and_git_dir(path, gitdir, 0);\n \t\t\tfree(gitdir);\n \n \t\t\t/* make sure the index is clean as well */\n@@ -1634,7 +1634,7 @@ int submodule_move_head(const char *path,\n \t\tif (old_head && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {\n \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n \t\t\t\t get_git_common_dir(), sub->name);\n-\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n+\t\t\tconnect_work_tree_and_git_dir(path, gitdir, 1);\n \t\t\tfree(gitdir);\n \t\t}\n \t}\n@@ -1947,7 +1947,7 @@ void absorb_git_dir_into_superproject(const char *prefix,\n \t\tif (!sub)\n \t\t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n \t\tconnect_work_tree_and_git_dir(path,\n-\t\t\tgit_path(\"modules/%s\", sub->name));\n+\t\t\tgit_path(\"modules/%s\", sub->name), 0);\n \t} else {\n \t\t/* Is it already absorbed into the superprojects git dir? */\n \t\tchar *real_sub_git_dir = real_pathdup(sub_git_dir, 1);\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex d4e6485a26..ff70244620 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -497,7 +497,7 @@ test_expect_success 'moving a submodule in nested directories' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'moving nested submodules' '\n+test_expect_success 'moving nested submodules' '\n \tgit commit -am \"cleanup commit\" &&\n \tmkdir sub_nested_nested &&\n \t(cd sub_nested_nested &&\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343234","messageId":"20180328172449.27012-4-sbeller@google.com","threadId":"48169","inReplyTo":"20180328172449.27012-1-sbeller@google.com","subject":"[PATCH 3/6] submodule-config: add repository argument to submodule_from_{name, path}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T17:24:46Z","receivedAt":"2018-03-28T17:25:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This enables submodule_from_{name, path} to handle arbitrary repositories.\nAll callers just pass in the_repository, a later patch will pass in other\nrepos.\n\nWhile at it remove the extern key word from the declarations.\n\nReviewed-by: Jonathan Tan \nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 14 +++++++-------\n submodule-config.c | 14 ++++++++------\n submodule-config.h | 10 ++++++----\n submodule.c | 30 ++++++++++++++++--------------\n t/helper/test-submodule-config.c | 6 ++++--\n 5 files changed, 41 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex ee020d4749..a921fbbf56 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -454,7 +454,7 @@ static void init_submodule(const char *path, const char *prefix,\n \n \tdisplaypath = get_submodule_displaypath(path, prefix);\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (!sub)\n \t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n@@ -621,7 +621,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,\n \tstruct rev_info rev;\n \tint diff_files_result;\n \n-\tif (!submodule_from_path(&null_oid, path))\n+\tif (!submodule_from_path(the_repository, &null_oid, path))\n \t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n \t\t path);\n \n@@ -741,7 +741,7 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \tif (argc != 2)\n \t\tusage(_(\"git submodule--helper name \"));\n \n-\tsub = submodule_from_path(&null_oid, argv[1]);\n+\tsub = submodule_from_path(the_repository, &null_oid, argv[1]);\n \n \tif (!sub)\n \t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n@@ -772,7 +772,7 @@ static void sync_submodule(const char *path, const char *prefix,\n \tif (!is_submodule_active(the_repository, path))\n \t\treturn;\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (sub && sub->url) {\n \t\tif (starts_with_dot_dot_slash(sub->url) ||\n@@ -925,7 +925,7 @@ static void deinit_submodule(const char *path, const char *prefix,\n \tstruct strbuf sb_config = STRBUF_INIT;\n \tchar *sub_git_dir = xstrfmt(\"%s/.git\", path);\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (!sub || !sub->name)\n \t\tgoto cleanup;\n@@ -1367,7 +1367,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\tgoto cleanup;\n \t}\n \n-\tsub = submodule_from_path(&null_oid, ce->name);\n+\tsub = submodule_from_path(the_repository, &null_oid, ce->name);\n \n \tif (suc->recursive_prefix)\n \t\tdisplaypath = relative_path(suc->recursive_prefix,\n@@ -1650,7 +1650,7 @@ static const char *remote_submodule_branch(const char *path)\n \tconst char *branch = NULL;\n \tchar *key;\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \tif (!sub)\n \t\treturn NULL;\n \ndiff --git a/submodule-config.c b/submodule-config.c\nindex a3efff1a34..4b7803e6ed 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -619,18 +619,20 @@ static void gitmodules_read_check(struct repository *repo)\n \t\trepo_read_gitmodules(repo);\n }\n \n-const struct submodule *submodule_from_name(const struct object_id *treeish_name,\n+const struct submodule *submodule_from_name(struct repository *r,\n+\t\t\t\t\t const struct object_id *treeish_name,\n \t\tconst char *name)\n {\n-\tgitmodules_read_check(the_repository);\n-\treturn config_from(the_repository->submodule_cache, treeish_name, name, lookup_name);\n+\tgitmodules_read_check(r);\n+\treturn config_from(r->submodule_cache, treeish_name, name, lookup_name);\n }\n \n-const struct submodule *submodule_from_path(const struct object_id *treeish_name,\n+const struct submodule *submodule_from_path(struct repository *r,\n+\t\t\t\t\t const struct object_id *treeish_name,\n \t\tconst char *path)\n {\n-\tgitmodules_read_check(the_repository);\n-\treturn config_from(the_repository->submodule_cache, treeish_name, path, lookup_path);\n+\tgitmodules_read_check(r);\n+\treturn config_from(r->submodule_cache, treeish_name, path, lookup_path);\n }\n \n const struct submodule *submodule_from_cache(struct repository *repo,\ndiff --git a/submodule-config.h b/submodule-config.h\nindex 6b71a8cd30..43dfe7dec0 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -39,10 +39,12 @@ extern int parse_update_recurse_submodules_arg(const char *opt, const char *arg)\n extern int parse_push_recurse_submodules_arg(const char *opt, const char *arg);\n extern void repo_read_gitmodules(struct repository *repo);\n extern void gitmodules_config_oid(const struct object_id *commit_oid);\n-extern const struct submodule *submodule_from_name(\n-\t\tconst struct object_id *commit_or_tree, const char *name);\n-extern const struct submodule *submodule_from_path(\n-\t\tconst struct object_id *commit_or_tree, const char *path);\n+const struct submodule *submodule_from_name(struct repository *r,\n+\t\t\t\t\t const struct object_id *commit_or_tree,\n+\t\t\t\t\t const char *name);\n+const struct submodule *submodule_from_path(struct repository *r,\n+\t\t\t\t\t const struct object_id *commit_or_tree,\n+\t\t\t\t\t const char *path);\n extern const struct submodule *submodule_from_cache(struct repository *repo,\n \t\t\t\t\t\t const struct object_id *treeish_name,\n \t\t\t\t\t\t const char *key);\ndiff --git a/submodule.c b/submodule.c\nindex 12a2503fda..e94b7f9acd 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -95,7 +95,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n \tif (is_gitmodules_unmerged(&the_index))\n \t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n \n-\tsubmodule = submodule_from_path(&null_oid, oldpath);\n+\tsubmodule = submodule_from_path(the_repository, &null_oid, oldpath);\n \tif (!submodule || !submodule->name) {\n \t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), oldpath);\n \t\treturn -1;\n@@ -129,7 +129,7 @@ int remove_path_from_gitmodules(const char *path)\n \tif (is_gitmodules_unmerged(&the_index))\n \t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n \n-\tsubmodule = submodule_from_path(&null_oid, path);\n+\tsubmodule = submodule_from_path(the_repository, &null_oid, path);\n \tif (!submodule || !submodule->name) {\n \t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), path);\n \t\treturn -1;\n@@ -173,7 +173,8 @@ static int add_submodule_odb(const char *path)\n void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\t\t\t\t const char *path)\n {\n-\tconst struct submodule *submodule = submodule_from_path(&null_oid, path);\n+\tconst struct submodule *submodule = submodule_from_path(the_repository,\n+\t\t\t\t\t\t\t\t&null_oid, path);\n \tif (submodule) {\n \t\tconst char *ignore;\n \t\tchar *key;\n@@ -673,7 +674,7 @@ const struct submodule *submodule_from_ce(const struct cache_entry *ce)\n \tif (!should_update_submodules())\n \t\treturn NULL;\n \n-\treturn submodule_from_path(&null_oid, ce->name);\n+\treturn submodule_from_path(the_repository, &null_oid, ce->name);\n }\n \n static struct oid_array *submodule_commits(struct string_list *submodules,\n@@ -730,13 +731,14 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,\n \t\tif (!S_ISGITLINK(p->two->mode))\n \t\t\tcontinue;\n \n-\t\tsubmodule = submodule_from_path(commit_oid, p->two->path);\n+\t\tsubmodule = submodule_from_path(the_repository,\n+\t\t\t\t\t\tcommit_oid, p->two->path);\n \t\tif (submodule)\n \t\t\tname = submodule->name;\n \t\telse {\n \t\t\tname = default_name_or_path(p->two->path);\n \t\t\t/* make sure name does not collide with existing one */\n-\t\t\tsubmodule = submodule_from_name(commit_oid, name);\n+\t\t\tsubmodule = submodule_from_name(the_repository, commit_oid, name);\n \t\t\tif (submodule) {\n \t\t\t\twarning(\"Submodule in commit %s at path: \"\n \t\t\t\t\t\"'%s' collides with a submodule named \"\n@@ -944,7 +946,7 @@ int find_unpushed_submodules(struct oid_array *commits,\n \t\tconst struct submodule *submodule;\n \t\tconst char *path = NULL;\n \n-\t\tsubmodule = submodule_from_name(&null_oid, name->string);\n+\t\tsubmodule = submodule_from_name(the_repository, &null_oid, name->string);\n \t\tif (submodule)\n \t\t\tpath = submodule->path;\n \t\telse\n@@ -1112,7 +1114,7 @@ static void calculate_changed_submodule_paths(void)\n \tconst struct string_list_item *name;\n \n \t/* No need to check if there are no submodules configured */\n-\tif (!submodule_from_path(NULL, NULL))\n+\tif (!submodule_from_path(the_repository, NULL, NULL))\n \t\treturn;\n \n \targv_array_push(&argv, \"--\"); /* argv[0] program name */\n@@ -1133,7 +1135,7 @@ static void calculate_changed_submodule_paths(void)\n \t\tconst struct submodule *submodule;\n \t\tconst char *path = NULL;\n \n-\t\tsubmodule = submodule_from_name(&null_oid, name->string);\n+\t\tsubmodule = submodule_from_name(the_repository, &null_oid, name->string);\n \t\tif (submodule)\n \t\t\tpath = submodule->path;\n \t\telse\n@@ -1161,7 +1163,7 @@ int submodule_touches_in_range(struct object_id *excl_oid,\n \tint ret;\n \n \t/* No need to check if there are no submodules configured */\n-\tif (!submodule_from_path(NULL, NULL))\n+\tif (!submodule_from_path(the_repository, NULL, NULL))\n \t\treturn 0;\n \n \targv_array_push(&args, \"--\"); /* args[0] program name */\n@@ -1603,7 +1605,7 @@ int submodule_move_head(const char *path,\n \tif (old_head && !is_submodule_populated_gently(path, error_code_ptr))\n \t\treturn 0;\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (!sub)\n \t\tdie(\"BUG: could not get submodule information for '%s'\", path);\n@@ -1885,7 +1887,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,\n \n \treal_old_git_dir = real_pathdup(old_git_dir, 1);\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \tif (!sub)\n \t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n \n@@ -1941,7 +1943,7 @@ void absorb_git_dir_into_superproject(const char *prefix,\n \t\t* superproject did not rewrite the git file links yet,\n \t\t* fix it now.\n \t\t*/\n-\t\tsub = submodule_from_path(&null_oid, path);\n+\t\tsub = submodule_from_path(the_repository, &null_oid, path);\n \t\tif (!sub)\n \t\t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n \t\tconnect_work_tree_and_git_dir(path,\n@@ -2087,7 +2089,7 @@ int submodule_to_gitdir(struct strbuf *buf, const char *submodule)\n \t\tstrbuf_addstr(buf, git_dir);\n \t}\n \tif (!is_git_directory(buf->buf)) {\n-\t\tsub = submodule_from_path(&null_oid, submodule);\n+\t\tsub = submodule_from_path(the_repository, &null_oid, submodule);\n \t\tif (!sub) {\n \t\t\tret = -1;\n \t\t\tgoto cleanup;\ndiff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c\nindex 9971c5e9dd..e044871cee 100644\n--- a/t/helper/test-submodule-config.c\n+++ b/t/helper/test-submodule-config.c\n@@ -48,9 +48,11 @@ int cmd_main(int argc, const char **argv)\n \t\t\tdie_usage(argc, argv, \"Commit not found.\");\n \n \t\tif (lookup_name) {\n-\t\t\tsubmodule = submodule_from_name(&commit_oid, path_or_name);\n+\t\t\tsubmodule = submodule_from_name(the_repository,\n+\t\t\t\t\t\t\t&commit_oid, path_or_name);\n \t\t} else\n-\t\t\tsubmodule = submodule_from_path(&commit_oid, path_or_name);\n+\t\t\tsubmodule = submodule_from_path(the_repository,\n+\t\t\t\t\t\t\t&commit_oid, path_or_name);\n \t\tif (!submodule)\n \t\t\tdie_usage(argc, argv, \"Submodule not found.\");\n \n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343235","messageId":"20180328172449.27012-7-sbeller@google.com","threadId":"48169","inReplyTo":"20180328172449.27012-1-sbeller@google.com","subject":"[PATCH 6/6] grep: remove \"repo\" arg from non-supporting funcs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T17:24:49Z","receivedAt":"2018-03-28T17:25:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Jonathan Tan \n\nAs part of commit f9ee2fcdfa (\"grep: recurse in-process using 'struct\nrepository'\", 2017-08-02), many functions in builtin/grep.c were\nconverted to also take \"struct repository *\" arguments. Among them were\ngrep_object() and grep_objects().\n\nHowever, at least grep_objects() was converted incompletely - it calls\ngitmodules_config_oid(), which references the_repository.\n\nBut it turns out that the conversion was extraneous anyway - there has\nbeen no user-visible effect - because grep_objects() is never invoked\nexcept with the_repository. This is because grepping through objects\ncannot be done recursively into submodules.\n\nRevert the changes to grep_objects() and grep_object() (which conversion\nis also extraneous) to show that both these functions do not support\nrepositories other than the_repository.\n\nSigned-off-by: Jonathan Tan \nSigned-off-by: Stefan Beller \n---\n builtin/grep.c | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 8f04cde18e..091b3f4cc7 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -601,8 +601,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,\n }\n \n static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n-\t\t struct object *obj, const char *name, const char *path,\n-\t\t struct repository *repo)\n+\t\t struct object *obj, const char *name, const char *path)\n {\n \tif (obj->type == OBJ_BLOB)\n \t\treturn grep_oid(opt, &obj->oid, name, 0, path);\n@@ -629,7 +628,7 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n \t\t}\n \t\tinit_tree_desc(&tree, data, size);\n \t\thit = grep_tree(opt, pathspec, &tree, &base, base.len,\n-\t\t\t\tobj->type == OBJ_COMMIT, repo);\n+\t\t\t\tobj->type == OBJ_COMMIT, the_repository);\n \t\tstrbuf_release(&base);\n \t\tfree(data);\n \t\treturn hit;\n@@ -638,7 +637,6 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n }\n \n static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n-\t\t\tstruct repository *repo,\n \t\t\tconst struct object_array *list)\n {\n \tunsigned int i;\n@@ -651,11 +649,11 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n \n \t\t/* load the gitmodules file for this rev */\n \t\tif (recurse_submodules) {\n-\t\t\tsubmodule_free(repo);\n+\t\t\tsubmodule_free(the_repository);\n \t\t\tgitmodules_config_oid(&real_obj->oid);\n \t\t}\n-\t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].path,\n-\t\t\t\trepo)) {\n+\t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name,\n+\t\t\t\tlist->objects[i].path)) {\n \t\t\thit = 1;\n \t\t\tif (opt->status_only)\n \t\t\t\tbreak;\n@@ -1107,7 +1105,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tif (cached)\n \t\t\tdie(_(\"both --cached and trees are given.\"));\n \n-\t\thit = grep_objects(&opt, &pathspec, the_repository, &list);\n+\t\thit = grep_objects(&opt, &pathspec, &list);\n \t}\n \n \tif (num_threads)\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343236","messageId":"20180328173543.GA159395@google.com","threadId":"48169","inReplyTo":"20180328172449.27012-6-sbeller@google.com","subject":"Re: [PATCH 5/6] submodule: fixup nested submodules after moving the submodule","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-03-28T17:35:43Z","receivedAt":"2018-03-28T17:35:50Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 03/28, Stefan Beller wrote:\n> connect_work_tree_and_git_dir is used to connect a submodule worktree with\n> its git directory and vice versa after events that require a reconnection\n> such as moving around the working tree. As submodules can have nested\n> submodules themselves, we'd also want to fix the nested submodules when\n> asked to. Add an option to recurse into the nested submodules and connect\n> them as well.\n> \n> As submodules are identified by their name (which determines their git\n> directory in relation to their superproject's git directory) internally\n> and by their path in the working tree of the superproject, we need to\n> make sure that the mapping of name <-> path is kept intact. We can do\n> that in the git-mv command by writing out the gitmodules file first\n> and then forcing a reload of the submodule config machinery.\n> \n> Signed-off-by: Stefan Beller \n> ---\n> builtin/mv.c | 6 ++--\n> builtin/submodule--helper.c | 3 +-\n> dir.c | 63 +++++++++++++++++++++++++++++++++++--\n> dir.h | 12 ++++++-\n> submodule.c | 6 ++--\n> t/t7001-mv.sh | 2 +-\n> 6 files changed, 80 insertions(+), 12 deletions(-)\n> \n> diff --git a/builtin/mv.c b/builtin/mv.c\n> index 6d141f7a53..7a63667d64 100644\n> --- a/builtin/mv.c\n> +++ b/builtin/mv.c\n> @@ -276,10 +276,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n> \t\t\tdie_errno(_(\"renaming '%s' failed\"), src);\n> \t\t}\n> \t\tif (submodule_gitfile[i]) {\n> -\t\t\tif (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)\n> -\t\t\t\tconnect_work_tree_and_git_dir(dst, submodule_gitfile[i]);\n> \t\t\tif (!update_path_in_gitmodules(src, dst))\n> \t\t\t\tgitmodules_modified = 1;\n> +\t\t\tif (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)\n> +\t\t\t\tconnect_work_tree_and_git_dir(dst,\n> +\t\t\t\t\t\t\t submodule_gitfile[i],\n> +\t\t\t\t\t\t\t 1);\n> \t\t}\n> \n> \t\tif (mode == WORKING_DIRECTORY)\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index a921fbbf56..05fd657f99 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -1259,8 +1259,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n> \t\tstrbuf_reset(&sb);\n> \t}\n> \n> -\t/* Connect module worktree and git dir */\n> -\tconnect_work_tree_and_git_dir(path, sm_gitdir);\n> +\tconnect_work_tree_and_git_dir(path, sm_gitdir, 0);\n> \n> \tp = git_pathdup_submodule(path, \"config\");\n> \tif (!p)\n> diff --git a/dir.c b/dir.c\n> index dedbf5d476..71947c0ef3 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -19,6 +19,7 @@\n> #include \"varint.h\"\n> #include \"ewah/ewok.h\"\n> #include \"fsmonitor.h\"\n> +#include \"submodule-config.h\"\n> \n> /*\n> * Tells read_directory_recursive how a file or directory should be treated.\n> @@ -3010,8 +3011,60 @@ void untracked_cache_add_to_index(struct index_state *istate,\n> \tuntracked_cache_invalidate_path(istate, path, 1);\n> }\n> \n> -/* Update gitfile and core.worktree setting to connect work tree and git dir */\n> -void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)\n> +static void connect_wt_gitdir_in_nested(const char *sub_worktree,\n> +\t\t\t\t\tconst char *sub_gitdir)\n> +{\n> +\tint i;\n> +\tstruct repository subrepo;\n> +\tstruct strbuf sub_wt = STRBUF_INIT;\n> +\tstruct strbuf sub_gd = STRBUF_INIT;\n> +\n> +\tconst struct submodule *sub;\n> +\n> +\tif (repo_init(&subrepo, sub_gitdir, sub_worktree))\n> +\t\treturn;\n\nNote that in Duy's object-store series he made this function private\n(IIRC) so this will result in some clash of the two series.\n\n> +\n> +\tif (repo_read_index(&subrepo) < 0)\n> +\t\tdie(\"index file corrupt in repo %s\", subrepo.gitdir);\n> +\n> +\tfor (i = 0; i < subrepo.index->cache_nr; i++) {\n> +\t\tconst struct cache_entry *ce = subrepo.index->cache[i];\n> +\n> +\t\tif (!S_ISGITLINK(ce->ce_mode))\n> +\t\t\tcontinue;\n> +\n> +\t\twhile (i + 1 < subrepo.index->cache_nr &&\n> +\t\t !strcmp(ce->name, subrepo.index->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> +\n> +\t\tsub = submodule_from_path(&subrepo, &null_oid, ce->name);\n> +\t\tif (!sub)\n> +\t\t\t/* submodule not checked out? */\n> +\t\t\tcontinue;\n> +\n> +\t\tif (is_submodule_active(&subrepo, ce->name)) {\n> +\t\t\tstrbuf_addf(&sub_wt, \"%s/%s\", sub_worktree, sub->path);\n> +\t\t\tstrbuf_addf(&sub_gd, \"%s/modules/%s\", sub_gitdir, sub->name);\n> +\n> +\t\t\tconnect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 0);\n> +\t\t\tconnect_wt_gitdir_in_nested(sub_wt.buf, sub_gd.buf);\n> +\n> +\t\t\tstrbuf_reset(&sub_wt);\n> +\t\t\tstrbuf_reset(&sub_gd);\n> +\t\t}\n> +\t}\n> +\tstrbuf_release(&sub_wt);\n> +\tstrbuf_release(&sub_gd);\n> +\trepo_clear(&subrepo);\n> +}\n> +\n> +void connect_work_tree_and_git_dir(const char *work_tree_,\n> +\t\t\t\t const char *git_dir_,\n> +\t\t\t\t int recurse_into_nested)\n> {\n> \tstruct strbuf gitfile_sb = STRBUF_INIT;\n> \tstruct strbuf cfg_sb = STRBUF_INIT;\n> @@ -3041,6 +3094,10 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)\n> \tstrbuf_release(&gitfile_sb);\n> \tstrbuf_release(&cfg_sb);\n> \tstrbuf_release(&rel_path);\n> +\n> +\tif (recurse_into_nested)\n> +\t\tconnect_wt_gitdir_in_nested(work_tree, git_dir);\n> +\n> \tfree(work_tree);\n> \tfree(git_dir);\n> }\n> @@ -3054,5 +3111,5 @@ void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_\n> \t\tdie_errno(_(\"could not migrate git directory from '%s' to '%s'\"),\n> \t\t\told_git_dir, new_git_dir);\n> \n> -\tconnect_work_tree_and_git_dir(path, new_git_dir);\n> +\tconnect_work_tree_and_git_dir(path, new_git_dir, 0);\n> }\n> diff --git a/dir.h b/dir.h\n> index b0758b82a2..3870193e52 100644\n> --- a/dir.h\n> +++ b/dir.h\n> @@ -359,7 +359,17 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n> void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);\n> void add_untracked_cache(struct index_state *istate);\n> void remove_untracked_cache(struct index_state *istate);\n> -extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n> +\n> +/*\n> + * Connect a worktree to a git directory by creating (or overwriting) a\n> + * '.git' file containing the location of the git directory. In the git\n> + * directory set the core.worktree setting to indicate where the worktree is.\n> + * When `recurse_into_nested` is set, recurse into any nested submodules,\n> + * connecting them as well.\n> + */\n> +extern void connect_work_tree_and_git_dir(const char *work_tree,\n> +\t\t\t\t\t const char *git_dir,\n> +\t\t\t\t\t int recurse_into_nested);\n> extern void relocate_gitdir(const char *path,\n> \t\t\t const char *old_git_dir,\n> \t\t\t const char *new_git_dir);\n> diff --git a/submodule.c b/submodule.c\n> index 89d0aee086..c2dac6c00f 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -1624,7 +1624,7 @@ int submodule_move_head(const char *path,\n> \t\t} else {\n> \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n> \t\t\t\t get_git_common_dir(), sub->name);\n> -\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n> +\t\t\tconnect_work_tree_and_git_dir(path, gitdir, 0);\n> \t\t\tfree(gitdir);\n> \n> \t\t\t/* make sure the index is clean as well */\n> @@ -1634,7 +1634,7 @@ int submodule_move_head(const char *path,\n> \t\tif (old_head && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {\n> \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n> \t\t\t\t get_git_common_dir(), sub->name);\n> -\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n> +\t\t\tconnect_work_tree_and_git_dir(path, gitdir, 1);\n> \t\t\tfree(gitdir);\n> \t\t}\n> \t}\n> @@ -1947,7 +1947,7 @@ void absorb_git_dir_into_superproject(const char *prefix,\n> \t\tif (!sub)\n> \t\t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n> \t\tconnect_work_tree_and_git_dir(path,\n> -\t\t\tgit_path(\"modules/%s\", sub->name));\n> +\t\t\tgit_path(\"modules/%s\", sub->name), 0);\n> \t} else {\n> \t\t/* Is it already absorbed into the superprojects git dir? */\n> \t\tchar *real_sub_git_dir = real_pathdup(sub_git_dir, 1);\n> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\n> index d4e6485a26..ff70244620 100755\n> --- a/t/t7001-mv.sh\n> +++ b/t/t7001-mv.sh\n> @@ -497,7 +497,7 @@ test_expect_success 'moving a submodule in nested directories' '\n> \ttest_cmp expect actual\n> '\n> \n> -test_expect_failure 'moving nested submodules' '\n> +test_expect_success 'moving nested submodules' '\n> \tgit commit -am \"cleanup commit\" &&\n> \tmkdir sub_nested_nested &&\n> \t(cd sub_nested_nested &&\n> -- \n> 2.17.0.rc1.321.gba9d0f2565-goog\n> \n\n-- \nBrandon Williams\n"},{"id":"343242","messageId":"20180328104657.2396071270905628fd2fdca3@google.com","threadId":"48169","inReplyTo":"20180328172449.27012-6-sbeller@google.com","subject":"Re: [PATCH 5/6] submodule: fixup nested submodules after moving the submodule","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-03-28T17:46:57Z","receivedAt":"2018-03-28T17:47:06Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 28 Mar 2018 10:24:48 -0700\nStefan Beller wrote:\n\n> +static void connect_wt_gitdir_in_nested(const char *sub_worktree,\n> +\t\t\t\t\tconst char *sub_gitdir)\n> +{\n> +\tint i;\n> +\tstruct repository subrepo;\n> +\tstruct strbuf sub_wt = STRBUF_INIT;\n> +\tstruct strbuf sub_gd = STRBUF_INIT;\n> +\n> +\tconst struct submodule *sub;\n> +\n> +\tif (repo_init(&subrepo, sub_gitdir, sub_worktree))\n> +\t\treturn;\n\nIf repo_init() fails, is it because the working tree doesn't exist on\ndisk, so we don't need to perform any connections on submodules? I think\na comment should be added to describe this.\n\n> +\n> +\tif (repo_read_index(&subrepo) < 0)\n> +\t\tdie(\"index file corrupt in repo %s\", subrepo.gitdir);\n> +\n> +\tfor (i = 0; i < subrepo.index->cache_nr; i++) {\n> +\t\tconst struct cache_entry *ce = subrepo.index->cache[i];\n> +\n> +\t\tif (!S_ISGITLINK(ce->ce_mode))\n> +\t\t\tcontinue;\n> +\n> +\t\twhile (i + 1 < subrepo.index->cache_nr &&\n> +\t\t !strcmp(ce->name, subrepo.index->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> +\n> +\t\tsub = submodule_from_path(&subrepo, &null_oid, ce->name);\n> +\t\tif (!sub)\n> +\t\t\t/* submodule not checked out? */\n> +\t\t\tcontinue;\n> +\n> +\t\tif (is_submodule_active(&subrepo, ce->name)) {\n\nOptional: This could be combined with the previous \"if\" block, so that\nthe following lines don't need to be indented.\n\n> +\t\t\tstrbuf_addf(&sub_wt, \"%s/%s\", sub_worktree, sub->path);\n> +\t\t\tstrbuf_addf(&sub_gd, \"%s/modules/%s\", sub_gitdir, sub->name);\n> +\n> +\t\t\tconnect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 0);\n> +\t\t\tconnect_wt_gitdir_in_nested(sub_wt.buf, sub_gd.buf);\n\nWhat's the difference between having this call to\nconnect_wt_gitdir_in_nested() and just passing 1 instead of 0 to\nconnect_work_tree_and_git_dir()? I see that the latter uses absolute\npaths, but that would seem to have the same effect. (If not, I think a\ncomment is warranted.)\n\n> +\n> +\t\t\tstrbuf_reset(&sub_wt);\n> +\t\t\tstrbuf_reset(&sub_gd);\n\nI think we normally write the resets before the strbuf_addf(), so that\nit's clearer that sub_wt and sub_gd are meant to start from scratch in\nevery iteration.\n\nOverall, this version of the patch is clearer - thanks.\n"},{"id":"343243","messageId":"20180328105416.3add54858bac92573d7d1130@google.com","threadId":"48169","inReplyTo":"20180328172449.27012-1-sbeller@google.com","subject":"Re: [PATCHv2 0/6] Moving submodules with nested submodules","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-03-28T17:54:16Z","receivedAt":"2018-03-28T17:56:13Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 28 Mar 2018 10:24:43 -0700\nStefan Beller wrote:\n\n> * picked up Jonathans patch and added it as a nice finish of the series.\n> I did not see the need or aesthetic desire to put that patch earlier\n> in the series.\n\nThanks for picking up my patch. The aesthetic desire is to avoid what's\ncurrently happening in PATCH 2/6 version 2, where we still have the\npotentially confusing submodule_free() -> submodule_free(repo)\nconversion, which is also later redone in PATCH 6/6 version 2\n(submodule_free(repo) -> submodule_free(the_repository)). But I'll leave\nthe final decision to you.\n\nOther than that, besides PATCH 5/6 (which I have already commented on),\neverything looks good.\n"},{"id":"343265","messageId":"CAGZ79kb1z8Ro9XV72s0-8O_D68VbLWbA=KT2EPOvgfi+e2YWCg@mail.gmail.com","threadId":"48169","inReplyTo":"20180328173543.GA159395@google.com","subject":"Re: [PATCH 5/6] submodule: fixup nested submodules after moving the submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T19:08:21Z","receivedAt":"2018-03-28T19:08:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":">> + if (repo_init(&subrepo, sub_gitdir, sub_worktree))\n>> + return;\n>\n> Note that in Duy's object-store series he made this function private\n> (IIRC) so this will result in some clash of the two series.\n>\n\nYes, that is the case.\nI wonder if I'd rather revert to v1 where we only use the\nsubmodule repo init, or if we revert b2f0eceecf (repository:\ninitialize the_repository in main(), 2018-03-03) partially to\nhave repo_init available.\n\nI would think the approach with submodule init is a bit cleaner\nthough has some more lines of code, using just repo_init\nseems easier, but we really have no use case for a separate\nrepo_init unless they are submodules. And here we ought to\ncheck for the repo being a submodule.\n\nNot yet sure which path to take.\n"},{"id":"343289","messageId":"20180328223531.241920-1-sbeller@google.com","threadId":"48169","inReplyTo":"20180328105416.3add54858bac92573d7d1130@google.com","subject":"[PATCHv3 0/6] Moving submodules with nested submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T22:35:25Z","receivedAt":"2018-03-28T22:35:40Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"v3:\n* reordered patches to have Jonathans patch before submodule_free\n* addressed Jonathans comments on patch 5.\n* rebased on origin/sb/object-store to resolve a visibility conflict\n about repo_init being exposed outside of repository.c\n\nv2:\n* addressed memleaks and messy code in patch 5\n* removed the extern keyword where applicable\n* extended the commit message, stating we want to rename submodule_free\n in the future.\n* picked up Jonathans patch and added it as a nice finish of the series.\n I did not see the need or aesthetic desire to put that patch earlier\n in the series.\n \nThanks,\nStefan\n\nv1:\n\nThis fixes the bug reported in [1] (\"Bug: moving submodules that have submodules\ninside them causes a fatal error in git status\")\n\n[1] https://public-inbox.org/git/20180306192017.GA5797@riseup.net/\n\nThanks,\nStefan\nJonathan Tan (1):\n grep: remove \"repo\" arg from non-supporting funcs\n\nStefan Beller (5):\n submodule.h: drop declaration of connect_work_tree_and_git_dir\n submodule-config: allow submodule_free to handle arbitrary\n repositories\n submodule-config: add repository argument to submodule_from_{name,\n path}\n submodule-config: remove submodule_from_cache\n submodule: fixup nested submodules after moving the submodule\n\n .../technical/api-submodule-config.txt | 2 +-\n builtin/grep.c | 14 ++---\n builtin/mv.c | 6 +-\n builtin/submodule--helper.c | 17 +++---\n dir.c | 60 ++++++++++++++++++-\n dir.h | 12 +++-\n repository.c | 8 +--\n repository.h | 3 +\n submodule-config.c | 29 ++++-----\n submodule-config.h | 15 +++--\n submodule.c | 40 +++++++------\n submodule.h | 1 -\n t/helper/test-submodule-config.c | 8 ++-\n t/t7001-mv.sh | 2 +-\n unpack-trees.c | 2 +-\n 15 files changed, 140 insertions(+), 79 deletions(-)\n\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343290","messageId":"20180328223531.241920-2-sbeller@google.com","threadId":"48169","inReplyTo":"20180328223531.241920-1-sbeller@google.com","subject":"[PATCHv3 1/6] submodule.h: drop declaration of connect_work_tree_and_git_dir","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T22:35:26Z","receivedAt":"2018-03-28T22:35:42Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The function connect_work_tree_and_git_dir is declared in both submodule.h\nand dir.h, such that one of them is redundant. As the function is\nimplemented in dir.c, drop the declaration from submodule.h\n\nSigned-off-by: Stefan Beller \n---\n submodule.h | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/submodule.h b/submodule.h\nindex b9b7ef0030..b6130e6287 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -105,7 +105,6 @@ extern int push_unpushed_submodules(struct oid_array *commits,\n \t\t\t\t const char **refspec, int refspec_nr,\n \t\t\t\t const struct string_list *push_options,\n \t\t\t\t int dry_run);\n-extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n /*\n * Given a submodule path (as in the index), return the repository\n * path of that submodule in 'buf'. Return -1 on error or when the\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343291","messageId":"20180328223531.241920-3-sbeller@google.com","threadId":"48169","inReplyTo":"20180328223531.241920-1-sbeller@google.com","subject":"[PATCHv3 2/6] grep: remove \"repo\" arg from non-supporting funcs","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T22:35:27Z","receivedAt":"2018-03-28T22:35:44Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"From: Jonathan Tan \n\nAs part of commit f9ee2fcdfa (\"grep: recurse in-process using 'struct\nrepository'\", 2017-08-02), many functions in builtin/grep.c were\nconverted to also take \"struct repository *\" arguments. Among them were\ngrep_object() and grep_objects().\n\nHowever, at least grep_objects() was converted incompletely - it calls\ngitmodules_config_oid(), which references the_repository.\n\nBut it turns out that the conversion was extraneous anyway - there has\nbeen no user-visible effect - because grep_objects() is never invoked\nexcept with the_repository. This is because grepping through objects\ncannot be done recursively into submodules.\n\nRevert the changes to grep_objects() and grep_object() (which conversion\nis also extraneous) to show that both these functions do not support\nrepositories other than the_repository.\n\nSigned-off-by: Jonathan Tan \nSigned-off-by: Stefan Beller \n---\n builtin/grep.c | 12 +++++-------\n 1 file changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 1e9cdbdf78..754eb6da3b 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -595,8 +595,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,\n }\n \n static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n-\t\t struct object *obj, const char *name, const char *path,\n-\t\t struct repository *repo)\n+\t\t struct object *obj, const char *name, const char *path)\n {\n \tif (obj->type == OBJ_BLOB)\n \t\treturn grep_oid(opt, &obj->oid, name, 0, path);\n@@ -623,7 +622,7 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n \t\t}\n \t\tinit_tree_desc(&tree, data, size);\n \t\thit = grep_tree(opt, pathspec, &tree, &base, base.len,\n-\t\t\t\tobj->type == OBJ_COMMIT, repo);\n+\t\t\t\tobj->type == OBJ_COMMIT, the_repository);\n \t\tstrbuf_release(&base);\n \t\tfree(data);\n \t\treturn hit;\n@@ -632,7 +631,6 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n }\n \n static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n-\t\t\tstruct repository *repo,\n \t\t\tconst struct object_array *list)\n {\n \tunsigned int i;\n@@ -648,8 +646,8 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n \t\t\tsubmodule_free();\n \t\t\tgitmodules_config_oid(&real_obj->oid);\n \t\t}\n-\t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name, list->objects[i].path,\n-\t\t\t\trepo)) {\n+\t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name,\n+\t\t\t\tlist->objects[i].path)) {\n \t\t\thit = 1;\n \t\t\tif (opt->status_only)\n \t\t\t\tbreak;\n@@ -1098,7 +1096,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\tif (cached)\n \t\t\tdie(_(\"both --cached and trees are given.\"));\n \n-\t\thit = grep_objects(&opt, &pathspec, the_repository, &list);\n+\t\thit = grep_objects(&opt, &pathspec, &list);\n \t}\n \n \tif (num_threads)\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343292","messageId":"20180328223531.241920-4-sbeller@google.com","threadId":"48169","inReplyTo":"20180328223531.241920-1-sbeller@google.com","subject":"[PATCHv3 3/6] submodule-config: allow submodule_free to handle arbitrary repositories","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T22:35:28Z","receivedAt":"2018-03-28T22:35:48Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"At some point we may want to rename the function so that it describes what\nit actually does as 'submodule_free' doesn't quite describe that this\nclears a repository's submodule cache. But that's beyond the scope of\nthis series.\n\nWhile at it remove the extern key word from its declaration.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/technical/api-submodule-config.txt | 2 +-\n builtin/grep.c | 2 +-\n submodule-config.c | 6 +++---\n submodule-config.h | 2 +-\n t/helper/test-submodule-config.c | 2 +-\n unpack-trees.c | 2 +-\n 6 files changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/Documentation/technical/api-submodule-config.txt b/Documentation/technical/api-submodule-config.txt\nindex 3dce003fda..44a85bbb8b 100644\n--- a/Documentation/technical/api-submodule-config.txt\n+++ b/Documentation/technical/api-submodule-config.txt\n@@ -38,7 +38,7 @@ Data Structures\n Functions\n ---------\n \n-`void submodule_free()`::\n+`void submodule_free(struct repository *r)`::\n \n \tUse these to free the internally cached values.\n \ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 754eb6da3b..c1f22fb9fb 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -643,7 +643,7 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n \n \t\t/* load the gitmodules file for this rev */\n \t\tif (recurse_submodules) {\n-\t\t\tsubmodule_free();\n+\t\t\tsubmodule_free(the_repository);\n \t\t\tgitmodules_config_oid(&real_obj->oid);\n \t\t}\n \t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name,\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 2aa8a1747f..5b4f0baae8 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -642,8 +642,8 @@ const struct submodule *submodule_from_cache(struct repository *repo,\n \t\t\t key, lookup_path);\n }\n \n-void submodule_free(void)\n+void submodule_free(struct repository *r)\n {\n-\tif (the_repository->submodule_cache)\n-\t\tsubmodule_cache_clear(the_repository->submodule_cache);\n+\tif (r->submodule_cache)\n+\t\tsubmodule_cache_clear(r->submodule_cache);\n }\ndiff --git a/submodule-config.h b/submodule-config.h\nindex a5503a5d17..6b71a8cd30 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -46,6 +46,6 @@ extern const struct submodule *submodule_from_path(\n extern const struct submodule *submodule_from_cache(struct repository *repo,\n \t\t\t\t\t\t const struct object_id *treeish_name,\n \t\t\t\t\t\t const char *key);\n-extern void submodule_free(void);\n+void submodule_free(struct repository *r);\n \n #endif /* SUBMODULE_CONFIG_H */\ndiff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c\nindex f23db3b19a..9971c5e9dd 100644\n--- a/t/helper/test-submodule-config.c\n+++ b/t/helper/test-submodule-config.c\n@@ -64,7 +64,7 @@ int cmd_main(int argc, const char **argv)\n \t\targ += 2;\n \t}\n \n-\tsubmodule_free();\n+\tsubmodule_free(the_repository);\n \n \treturn 0;\n }\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex e6a15bbe44..3a6a28e794 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -290,7 +290,7 @@ static void load_gitmodules_file(struct index_state *index,\n \t\tif (!state && ce->ce_flags & CE_WT_REMOVE) {\n \t\t\trepo_read_gitmodules(the_repository);\n \t\t} else if (state && (ce->ce_flags & CE_UPDATE)) {\n-\t\t\tsubmodule_free();\n+\t\t\tsubmodule_free(the_repository);\n \t\t\tcheckout_entry(ce, state, NULL);\n \t\t\trepo_read_gitmodules(the_repository);\n \t\t}\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343293","messageId":"20180328223531.241920-7-sbeller@google.com","threadId":"48169","inReplyTo":"20180328223531.241920-1-sbeller@google.com","subject":"[PATCHv3 6/6] submodule: fixup nested submodules after moving the submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T22:35:31Z","receivedAt":"2018-03-28T22:35:51Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"connect_work_tree_and_git_dir is used to connect a submodule worktree with\nits git directory and vice versa after events that require a reconnection\nsuch as moving around the working tree. As submodules can have nested\nsubmodules themselves, we'd also want to fix the nested submodules when\nasked to. Add an option to recurse into the nested submodules and connect\nthem as well.\n\nAs submodules are identified by their name (which determines their git\ndirectory in relation to their superproject's git directory) internally\nand by their path in the working tree of the superproject, we need to\nmake sure that the mapping of name <-> path is kept intact. We can do\nthat in the git-mv command by writing out the gitmodules file first\nand then forcing a reload of the submodule config machinery.\n\nSigned-off-by: Stefan Beller \n---\n builtin/mv.c | 6 ++--\n builtin/submodule--helper.c | 3 +-\n dir.c | 60 +++++++++++++++++++++++++++++++++++--\n dir.h | 12 +++++++-\n repository.c | 6 ++--\n repository.h | 3 ++\n submodule.c | 6 ++--\n t/t7001-mv.sh | 2 +-\n 8 files changed, 83 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/mv.c b/builtin/mv.c\nindex cf3684d907..b0c5178e0d 100644\n--- a/builtin/mv.c\n+++ b/builtin/mv.c\n@@ -275,10 +275,12 @@ int cmd_mv(int argc, const char **argv, const char *prefix)\n \t\t\tdie_errno(_(\"renaming '%s' failed\"), src);\n \t\t}\n \t\tif (submodule_gitfile[i]) {\n-\t\t\tif (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)\n-\t\t\t\tconnect_work_tree_and_git_dir(dst, submodule_gitfile[i]);\n \t\t\tif (!update_path_in_gitmodules(src, dst))\n \t\t\t\tgitmodules_modified = 1;\n+\t\t\tif (submodule_gitfile[i] != SUBMODULE_WITH_GITDIR)\n+\t\t\t\tconnect_work_tree_and_git_dir(dst,\n+\t\t\t\t\t\t\t submodule_gitfile[i],\n+\t\t\t\t\t\t\t 1);\n \t\t}\n \n \t\tif (mode == WORKING_DIRECTORY)\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 5551cf19c3..ffdc51f426 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1260,8 +1260,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \t\tstrbuf_reset(&sb);\n \t}\n \n-\t/* Connect module worktree and git dir */\n-\tconnect_work_tree_and_git_dir(path, sm_gitdir);\n+\tconnect_work_tree_and_git_dir(path, sm_gitdir, 0);\n \n \tp = git_pathdup_submodule(path, \"config\");\n \tif (!p)\ndiff --git a/dir.c b/dir.c\nindex ce6e50d2a2..4f401b6a3c 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -19,6 +19,7 @@\n #include \"varint.h\"\n #include \"ewah/ewok.h\"\n #include \"fsmonitor.h\"\n+#include \"submodule-config.h\"\n \n /*\n * Tells read_directory_recursive how a file or directory should be treated.\n@@ -2988,8 +2989,57 @@ void untracked_cache_add_to_index(struct index_state *istate,\n \tuntracked_cache_invalidate_path(istate, path);\n }\n \n-/* Update gitfile and core.worktree setting to connect work tree and git dir */\n-void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)\n+static void connect_wt_gitdir_in_nested(const char *sub_worktree,\n+\t\t\t\t\tconst char *sub_gitdir)\n+{\n+\tint i;\n+\tstruct repository subrepo;\n+\tstruct strbuf sub_wt = STRBUF_INIT;\n+\tstruct strbuf sub_gd = STRBUF_INIT;\n+\n+\tconst struct submodule *sub;\n+\n+\t/* If the submodule has no working tree, we can ignore it. */\n+\tif (repo_init(&subrepo, sub_gitdir, sub_worktree))\n+\t\treturn;\n+\n+\tif (repo_read_index(&subrepo) < 0)\n+\t\tdie(\"index file corrupt in repo %s\", subrepo.gitdir);\n+\n+\tfor (i = 0; i < subrepo.index->cache_nr; i++) {\n+\t\tconst struct cache_entry *ce = subrepo.index->cache[i];\n+\n+\t\tif (!S_ISGITLINK(ce->ce_mode))\n+\t\t\tcontinue;\n+\n+\t\twhile (i + 1 < subrepo.index->cache_nr &&\n+\t\t !strcmp(ce->name, subrepo.index->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+\n+\t\tsub = submodule_from_path(&subrepo, &null_oid, ce->name);\n+\t\tif (!sub || !is_submodule_active(&subrepo, ce->name))\n+\t\t\t/* .gitmodules broken or inactive sub */\n+\t\t\tcontinue;\n+\n+\t\tstrbuf_reset(&sub_wt);\n+\t\tstrbuf_reset(&sub_gd);\n+\t\tstrbuf_addf(&sub_wt, \"%s/%s\", sub_worktree, sub->path);\n+\t\tstrbuf_addf(&sub_gd, \"%s/modules/%s\", sub_gitdir, sub->name);\n+\n+\t\tconnect_work_tree_and_git_dir(sub_wt.buf, sub_gd.buf, 1);\n+\t}\n+\tstrbuf_release(&sub_wt);\n+\tstrbuf_release(&sub_gd);\n+\trepo_clear(&subrepo);\n+}\n+\n+void connect_work_tree_and_git_dir(const char *work_tree_,\n+\t\t\t\t const char *git_dir_,\n+\t\t\t\t int recurse_into_nested)\n {\n \tstruct strbuf gitfile_sb = STRBUF_INIT;\n \tstruct strbuf cfg_sb = STRBUF_INIT;\n@@ -3019,6 +3069,10 @@ void connect_work_tree_and_git_dir(const char *work_tree_, const char *git_dir_)\n \tstrbuf_release(&gitfile_sb);\n \tstrbuf_release(&cfg_sb);\n \tstrbuf_release(&rel_path);\n+\n+\tif (recurse_into_nested)\n+\t\tconnect_wt_gitdir_in_nested(work_tree, git_dir);\n+\n \tfree(work_tree);\n \tfree(git_dir);\n }\n@@ -3032,5 +3086,5 @@ void relocate_gitdir(const char *path, const char *old_git_dir, const char *new_\n \t\tdie_errno(_(\"could not migrate git directory from '%s' to '%s'\"),\n \t\t\told_git_dir, new_git_dir);\n \n-\tconnect_work_tree_and_git_dir(path, new_git_dir);\n+\tconnect_work_tree_and_git_dir(path, new_git_dir, 0);\n }\ndiff --git a/dir.h b/dir.h\nindex 11a047ba48..d2545a7685 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -359,7 +359,17 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long\n void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);\n void add_untracked_cache(struct index_state *istate);\n void remove_untracked_cache(struct index_state *istate);\n-extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n+\n+/*\n+ * Connect a worktree to a git directory by creating (or overwriting) a\n+ * '.git' file containing the location of the git directory. In the git\n+ * directory set the core.worktree setting to indicate where the worktree is.\n+ * When `recurse_into_nested` is set, recurse into any nested submodules,\n+ * connecting them as well.\n+ */\n+extern void connect_work_tree_and_git_dir(const char *work_tree,\n+\t\t\t\t\t const char *git_dir,\n+\t\t\t\t\t int recurse_into_nested);\n extern void relocate_gitdir(const char *path,\n \t\t\t const char *old_git_dir,\n \t\t\t const char *new_git_dir);\ndiff --git a/repository.c b/repository.c\nindex eb5b8e9f5a..beff3caa9e 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -135,9 +135,9 @@ static int read_and_verify_repository_format(struct repository_format *format,\n * Initialize 'repo' based on the provided 'gitdir'.\n * Return 0 upon success and a non-zero value upon failure.\n */\n-static int repo_init(struct repository *repo,\n-\t\t const char *gitdir,\n-\t\t const char *worktree)\n+int repo_init(struct repository *repo,\n+\t const char *gitdir,\n+\t const char *worktree)\n {\n \tstruct repository_format format;\n \tmemset(repo, 0, sizeof(*repo));\ndiff --git a/repository.h b/repository.h\nindex 09df94a472..6041367f08 100644\n--- a/repository.h\n+++ b/repository.h\n@@ -97,6 +97,9 @@ extern void repo_set_gitdir(struct repository *repo,\n extern void repo_set_worktree(struct repository *repo, const char *path);\n extern void repo_set_hash_algo(struct repository *repo, int algo);\n extern void initialize_the_repository(void);\n+extern int repo_init(struct repository *r,\n+\t\t const char *gitdir,\n+\t\t const char *worktree);\n extern int repo_submodule_init(struct repository *submodule,\n \t\t\t struct repository *superproject,\n \t\t\t const char *path);\ndiff --git a/submodule.c b/submodule.c\nindex dac73d10a7..53c45e49d0 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1625,7 +1625,7 @@ int submodule_move_head(const char *path,\n \t\t} else {\n \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n \t\t\t\t get_git_common_dir(), sub->name);\n-\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n+\t\t\tconnect_work_tree_and_git_dir(path, gitdir, 0);\n \t\t\tfree(gitdir);\n \n \t\t\t/* make sure the index is clean as well */\n@@ -1635,7 +1635,7 @@ int submodule_move_head(const char *path,\n \t\tif (old && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {\n \t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n \t\t\t\t get_git_common_dir(), sub->name);\n-\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n+\t\t\tconnect_work_tree_and_git_dir(path, gitdir, 1);\n \t\t\tfree(gitdir);\n \t\t}\n \t}\n@@ -1948,7 +1948,7 @@ void absorb_git_dir_into_superproject(const char *prefix,\n \t\tif (!sub)\n \t\t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n \t\tconnect_work_tree_and_git_dir(path,\n-\t\t\tgit_path(\"modules/%s\", sub->name));\n+\t\t\tgit_path(\"modules/%s\", sub->name), 0);\n \t} else {\n \t\t/* Is it already absorbed into the superprojects git dir? */\n \t\tchar *real_sub_git_dir = real_pathdup(sub_git_dir, 1);\ndiff --git a/t/t7001-mv.sh b/t/t7001-mv.sh\nindex 6e5031f56f..bfe2c427f1 100755\n--- a/t/t7001-mv.sh\n+++ b/t/t7001-mv.sh\n@@ -491,7 +491,7 @@ test_expect_success 'moving a submodule in nested directories' '\n \ttest_cmp expect actual\n '\n \n-test_expect_failure 'moving nested submodules' '\n+test_expect_success 'moving nested submodules' '\n \tgit commit -am \"cleanup commit\" &&\n \tmkdir sub_nested_nested &&\n \t(cd sub_nested_nested &&\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343294","messageId":"20180328223531.241920-5-sbeller@google.com","threadId":"48169","inReplyTo":"20180328223531.241920-1-sbeller@google.com","subject":"[PATCHv3 4/6] submodule-config: add repository argument to submodule_from_{name, path}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T22:35:29Z","receivedAt":"2018-03-28T22:35:56Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This enables submodule_from_{name, path} to handle arbitrary repositories.\nAll callers just pass in the_repository, a later patch will pass in other\nrepos.\n\nWhile at it remove the extern key word from the declarations.\n\nReviewed-by: Jonathan Tan \nSigned-off-by: Stefan Beller \n---\n builtin/submodule--helper.c | 14 +++++++-------\n submodule-config.c | 14 ++++++++------\n submodule-config.h | 10 ++++++----\n submodule.c | 30 ++++++++++++++++--------------\n t/helper/test-submodule-config.c | 6 ++++--\n 5 files changed, 41 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 6d8e002be7..5551cf19c3 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -455,7 +455,7 @@ static void init_submodule(const char *path, const char *prefix,\n \n \tdisplaypath = get_submodule_displaypath(path, prefix);\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (!sub)\n \t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n@@ -622,7 +622,7 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,\n \tstruct rev_info rev;\n \tint diff_files_result;\n \n-\tif (!submodule_from_path(&null_oid, path))\n+\tif (!submodule_from_path(the_repository, &null_oid, path))\n \t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n \t\t path);\n \n@@ -742,7 +742,7 @@ static int module_name(int argc, const char **argv, const char *prefix)\n \tif (argc != 2)\n \t\tusage(_(\"git submodule--helper name \"));\n \n-\tsub = submodule_from_path(&null_oid, argv[1]);\n+\tsub = submodule_from_path(the_repository, &null_oid, argv[1]);\n \n \tif (!sub)\n \t\tdie(_(\"no submodule mapping found in .gitmodules for path '%s'\"),\n@@ -773,7 +773,7 @@ static void sync_submodule(const char *path, const char *prefix,\n \tif (!is_submodule_active(the_repository, path))\n \t\treturn;\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (sub && sub->url) {\n \t\tif (starts_with_dot_dot_slash(sub->url) ||\n@@ -926,7 +926,7 @@ static void deinit_submodule(const char *path, const char *prefix,\n \tstruct strbuf sb_config = STRBUF_INIT;\n \tchar *sub_git_dir = xstrfmt(\"%s/.git\", path);\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (!sub || !sub->name)\n \t\tgoto cleanup;\n@@ -1368,7 +1368,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t\tgoto cleanup;\n \t}\n \n-\tsub = submodule_from_path(&null_oid, ce->name);\n+\tsub = submodule_from_path(the_repository, &null_oid, ce->name);\n \n \tif (suc->recursive_prefix)\n \t\tdisplaypath = relative_path(suc->recursive_prefix,\n@@ -1651,7 +1651,7 @@ static const char *remote_submodule_branch(const char *path)\n \tconst char *branch = NULL;\n \tchar *key;\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \tif (!sub)\n \t\treturn NULL;\n \ndiff --git a/submodule-config.c b/submodule-config.c\nindex 5b4f0baae8..0ea1e927d2 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -619,18 +619,20 @@ static void gitmodules_read_check(struct repository *repo)\n \t\trepo_read_gitmodules(repo);\n }\n \n-const struct submodule *submodule_from_name(const struct object_id *treeish_name,\n+const struct submodule *submodule_from_name(struct repository *r,\n+\t\t\t\t\t const struct object_id *treeish_name,\n \t\tconst char *name)\n {\n-\tgitmodules_read_check(the_repository);\n-\treturn config_from(the_repository->submodule_cache, treeish_name, name, lookup_name);\n+\tgitmodules_read_check(r);\n+\treturn config_from(r->submodule_cache, treeish_name, name, lookup_name);\n }\n \n-const struct submodule *submodule_from_path(const struct object_id *treeish_name,\n+const struct submodule *submodule_from_path(struct repository *r,\n+\t\t\t\t\t const struct object_id *treeish_name,\n \t\tconst char *path)\n {\n-\tgitmodules_read_check(the_repository);\n-\treturn config_from(the_repository->submodule_cache, treeish_name, path, lookup_path);\n+\tgitmodules_read_check(r);\n+\treturn config_from(r->submodule_cache, treeish_name, path, lookup_path);\n }\n \n const struct submodule *submodule_from_cache(struct repository *repo,\ndiff --git a/submodule-config.h b/submodule-config.h\nindex 6b71a8cd30..43dfe7dec0 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -39,10 +39,12 @@ extern int parse_update_recurse_submodules_arg(const char *opt, const char *arg)\n extern int parse_push_recurse_submodules_arg(const char *opt, const char *arg);\n extern void repo_read_gitmodules(struct repository *repo);\n extern void gitmodules_config_oid(const struct object_id *commit_oid);\n-extern const struct submodule *submodule_from_name(\n-\t\tconst struct object_id *commit_or_tree, const char *name);\n-extern const struct submodule *submodule_from_path(\n-\t\tconst struct object_id *commit_or_tree, const char *path);\n+const struct submodule *submodule_from_name(struct repository *r,\n+\t\t\t\t\t const struct object_id *commit_or_tree,\n+\t\t\t\t\t const char *name);\n+const struct submodule *submodule_from_path(struct repository *r,\n+\t\t\t\t\t const struct object_id *commit_or_tree,\n+\t\t\t\t\t const char *path);\n extern const struct submodule *submodule_from_cache(struct repository *repo,\n \t\t\t\t\t\t const struct object_id *treeish_name,\n \t\t\t\t\t\t const char *key);\ndiff --git a/submodule.c b/submodule.c\nindex b03e5f5045..9279cff2d7 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -96,7 +96,7 @@ int update_path_in_gitmodules(const char *oldpath, const char *newpath)\n \tif (is_gitmodules_unmerged(&the_index))\n \t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n \n-\tsubmodule = submodule_from_path(&null_oid, oldpath);\n+\tsubmodule = submodule_from_path(the_repository, &null_oid, oldpath);\n \tif (!submodule || !submodule->name) {\n \t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), oldpath);\n \t\treturn -1;\n@@ -130,7 +130,7 @@ int remove_path_from_gitmodules(const char *path)\n \tif (is_gitmodules_unmerged(&the_index))\n \t\tdie(_(\"Cannot change unmerged .gitmodules, resolve merge conflicts first\"));\n \n-\tsubmodule = submodule_from_path(&null_oid, path);\n+\tsubmodule = submodule_from_path(the_repository, &null_oid, path);\n \tif (!submodule || !submodule->name) {\n \t\twarning(_(\"Could not find section in .gitmodules where path=%s\"), path);\n \t\treturn -1;\n@@ -174,7 +174,8 @@ static int add_submodule_odb(const char *path)\n void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n \t\t\t\t\t const char *path)\n {\n-\tconst struct submodule *submodule = submodule_from_path(&null_oid, path);\n+\tconst struct submodule *submodule = submodule_from_path(the_repository,\n+\t\t\t\t\t\t\t\t&null_oid, path);\n \tif (submodule) {\n \t\tconst char *ignore;\n \t\tchar *key;\n@@ -674,7 +675,7 @@ const struct submodule *submodule_from_ce(const struct cache_entry *ce)\n \tif (!should_update_submodules())\n \t\treturn NULL;\n \n-\treturn submodule_from_path(&null_oid, ce->name);\n+\treturn submodule_from_path(the_repository, &null_oid, ce->name);\n }\n \n static struct oid_array *submodule_commits(struct string_list *submodules,\n@@ -731,13 +732,14 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q,\n \t\tif (!S_ISGITLINK(p->two->mode))\n \t\t\tcontinue;\n \n-\t\tsubmodule = submodule_from_path(commit_oid, p->two->path);\n+\t\tsubmodule = submodule_from_path(the_repository,\n+\t\t\t\t\t\tcommit_oid, p->two->path);\n \t\tif (submodule)\n \t\t\tname = submodule->name;\n \t\telse {\n \t\t\tname = default_name_or_path(p->two->path);\n \t\t\t/* make sure name does not collide with existing one */\n-\t\t\tsubmodule = submodule_from_name(commit_oid, name);\n+\t\t\tsubmodule = submodule_from_name(the_repository, commit_oid, name);\n \t\t\tif (submodule) {\n \t\t\t\twarning(\"Submodule in commit %s at path: \"\n \t\t\t\t\t\"'%s' collides with a submodule named \"\n@@ -945,7 +947,7 @@ int find_unpushed_submodules(struct oid_array *commits,\n \t\tconst struct submodule *submodule;\n \t\tconst char *path = NULL;\n \n-\t\tsubmodule = submodule_from_name(&null_oid, name->string);\n+\t\tsubmodule = submodule_from_name(the_repository, &null_oid, name->string);\n \t\tif (submodule)\n \t\t\tpath = submodule->path;\n \t\telse\n@@ -1113,7 +1115,7 @@ static void calculate_changed_submodule_paths(void)\n \tconst struct string_list_item *name;\n \n \t/* No need to check if there are no submodules configured */\n-\tif (!submodule_from_path(NULL, NULL))\n+\tif (!submodule_from_path(the_repository, NULL, NULL))\n \t\treturn;\n \n \targv_array_push(&argv, \"--\"); /* argv[0] program name */\n@@ -1134,7 +1136,7 @@ static void calculate_changed_submodule_paths(void)\n \t\tconst struct submodule *submodule;\n \t\tconst char *path = NULL;\n \n-\t\tsubmodule = submodule_from_name(&null_oid, name->string);\n+\t\tsubmodule = submodule_from_name(the_repository, &null_oid, name->string);\n \t\tif (submodule)\n \t\t\tpath = submodule->path;\n \t\telse\n@@ -1162,7 +1164,7 @@ int submodule_touches_in_range(struct object_id *excl_oid,\n \tint ret;\n \n \t/* No need to check if there are no submodules configured */\n-\tif (!submodule_from_path(NULL, NULL))\n+\tif (!submodule_from_path(the_repository, NULL, NULL))\n \t\treturn 0;\n \n \targv_array_push(&args, \"--\"); /* args[0] program name */\n@@ -1604,7 +1606,7 @@ int submodule_move_head(const char *path,\n \tif (old && !is_submodule_populated_gently(path, error_code_ptr))\n \t\treturn 0;\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \n \tif (!sub)\n \t\tdie(\"BUG: could not get submodule information for '%s'\", path);\n@@ -1886,7 +1888,7 @@ static void relocate_single_git_dir_into_superproject(const char *prefix,\n \n \treal_old_git_dir = real_pathdup(old_git_dir, 1);\n \n-\tsub = submodule_from_path(&null_oid, path);\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n \tif (!sub)\n \t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n \n@@ -1942,7 +1944,7 @@ void absorb_git_dir_into_superproject(const char *prefix,\n \t\t* superproject did not rewrite the git file links yet,\n \t\t* fix it now.\n \t\t*/\n-\t\tsub = submodule_from_path(&null_oid, path);\n+\t\tsub = submodule_from_path(the_repository, &null_oid, path);\n \t\tif (!sub)\n \t\t\tdie(_(\"could not lookup name for submodule '%s'\"), path);\n \t\tconnect_work_tree_and_git_dir(path,\n@@ -2088,7 +2090,7 @@ int submodule_to_gitdir(struct strbuf *buf, const char *submodule)\n \t\tstrbuf_addstr(buf, git_dir);\n \t}\n \tif (!is_git_directory(buf->buf)) {\n-\t\tsub = submodule_from_path(&null_oid, submodule);\n+\t\tsub = submodule_from_path(the_repository, &null_oid, submodule);\n \t\tif (!sub) {\n \t\t\tret = -1;\n \t\t\tgoto cleanup;\ndiff --git a/t/helper/test-submodule-config.c b/t/helper/test-submodule-config.c\nindex 9971c5e9dd..e044871cee 100644\n--- a/t/helper/test-submodule-config.c\n+++ b/t/helper/test-submodule-config.c\n@@ -48,9 +48,11 @@ int cmd_main(int argc, const char **argv)\n \t\t\tdie_usage(argc, argv, \"Commit not found.\");\n \n \t\tif (lookup_name) {\n-\t\t\tsubmodule = submodule_from_name(&commit_oid, path_or_name);\n+\t\t\tsubmodule = submodule_from_name(the_repository,\n+\t\t\t\t\t\t\t&commit_oid, path_or_name);\n \t\t} else\n-\t\t\tsubmodule = submodule_from_path(&commit_oid, path_or_name);\n+\t\t\tsubmodule = submodule_from_path(the_repository,\n+\t\t\t\t\t\t\t&commit_oid, path_or_name);\n \t\tif (!submodule)\n \t\t\tdie_usage(argc, argv, \"Submodule not found.\");\n \n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343296","messageId":"20180328223531.241920-6-sbeller@google.com","threadId":"48169","inReplyTo":"20180328223531.241920-1-sbeller@google.com","subject":"[PATCHv3 5/6] submodule-config: remove submodule_from_cache","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-03-28T22:35:30Z","receivedAt":"2018-03-28T22:36:00Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This continues the story of bf12fcdf5e (submodule-config: store\nthe_submodule_cache in the_repository, 2017-06-22).\n\nThe previous patch taught submodule_from_path to take a repository into\naccount, such that submodule_from_{path, cache} are the same now.\nRemove submodule_from_cache, migrating all its callers to\nsubmodule_from_path.\n\nReviewed-by: Jonathan Tan \nSigned-off-by: Stefan Beller \n---\n repository.c | 2 +-\n submodule-config.c | 9 ---------\n submodule-config.h | 3 ---\n submodule.c | 4 ++--\n 4 files changed, 3 insertions(+), 15 deletions(-)\n\ndiff --git a/repository.c b/repository.c\nindex a4848c1bd0..eb5b8e9f5a 100644\n--- a/repository.c\n+++ b/repository.c\n@@ -176,7 +176,7 @@ int repo_submodule_init(struct repository *submodule,\n \tstruct strbuf worktree = STRBUF_INIT;\n \tint ret = 0;\n \n-\tsub = submodule_from_cache(superproject, &null_oid, path);\n+\tsub = submodule_from_path(superproject, &null_oid, path);\n \tif (!sub) {\n \t\tret = -1;\n \t\tgoto out;\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 0ea1e927d2..9abe9166d5 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -635,15 +635,6 @@ const struct submodule *submodule_from_path(struct repository *r,\n \treturn config_from(r->submodule_cache, treeish_name, path, lookup_path);\n }\n \n-const struct submodule *submodule_from_cache(struct repository *repo,\n-\t\t\t\t\t const struct object_id *treeish_name,\n-\t\t\t\t\t const char *key)\n-{\n-\tgitmodules_read_check(repo);\n-\treturn config_from(repo->submodule_cache, treeish_name,\n-\t\t\t key, lookup_path);\n-}\n-\n void submodule_free(struct repository *r)\n {\n \tif (r->submodule_cache)\ndiff --git a/submodule-config.h b/submodule-config.h\nindex 43dfe7dec0..6f686184e8 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -45,9 +45,6 @@ const struct submodule *submodule_from_name(struct repository *r,\n const struct submodule *submodule_from_path(struct repository *r,\n \t\t\t\t\t const struct object_id *commit_or_tree,\n \t\t\t\t\t const char *path);\n-extern const struct submodule *submodule_from_cache(struct repository *repo,\n-\t\t\t\t\t\t const struct object_id *treeish_name,\n-\t\t\t\t\t\t const char *key);\n void submodule_free(struct repository *r);\n \n #endif /* SUBMODULE_CONFIG_H */\ndiff --git a/submodule.c b/submodule.c\nindex 9279cff2d7..dac73d10a7 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -231,7 +231,7 @@ int is_submodule_active(struct repository *repo, const char *path)\n \tconst struct string_list *sl;\n \tconst struct submodule *module;\n \n-\tmodule = submodule_from_cache(repo, &null_oid, path);\n+\tmodule = submodule_from_path(repo, &null_oid, path);\n \n \t/* early return if there isn't a path->module mapping */\n \tif (!module)\n@@ -1236,7 +1236,7 @@ static int get_next_submodule(struct child_process *cp,\n \t\tif (!S_ISGITLINK(ce->ce_mode))\n \t\t\tcontinue;\n \n-\t\tsubmodule = submodule_from_cache(spf->r, &null_oid, ce->name);\n+\t\tsubmodule = submodule_from_path(spf->r, &null_oid, ce->name);\n \t\tif (!submodule) {\n \t\t\tconst char *name = default_name_or_path(ce->name);\n \t\t\tif (name) {\n-- \n2.17.0.rc1.321.gba9d0f2565-goog\n\n"},{"id":"343298","messageId":"20180328161727.af10f596dffc8e01205c41dd@google.com","threadId":"48169","inReplyTo":"20180328223531.241920-1-sbeller@google.com","subject":"Re: [PATCHv3 0/6] Moving submodules with nested submodules","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2018-03-28T23:17:27Z","receivedAt":"2018-03-28T23:17:34Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Wed, 28 Mar 2018 15:35:25 -0700\nStefan Beller wrote:\n\n> v3:\n> * reordered patches to have Jonathans patch before submodule_free\n> * addressed Jonathans comments on patch 5.\n> * rebased on origin/sb/object-store to resolve a visibility conflict\n> about repo_init being exposed outside of repository.c\n\nAll 6 patches look good to me, thanks.\n\nReviewed-by: Jonathan Tan \n"}]}