{"thread":{"id":"44604","subject":"[RFC PATCHv2 02/17] submodule: modernize ok_to_remove_submodule to use argv_array","startedAt":"2016-12-03T00:30:55Z","lastAt":"2016-12-06T00:18:35Z","messageCount":30,"participants":["Stefan Beller","Brandon Williams","David Turner"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"306835","messageId":"20161203003022.29797-3-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 02/17] submodule: modernize ok_to_remove_submodule to use argv_array","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:07Z","receivedAt":"2016-12-03T00:30:55Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Instead of constructing the NULL terminated array ourselves, we\nshould make use of the argv_array infrastructure.\n\nSigned-off-by: Stefan Beller \n---\n submodule.c | 14 ++++----------\n 1 file changed, 4 insertions(+), 10 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 66c5ce5a24..eb80b0c5ad 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1022,13 +1022,6 @@ int ok_to_remove_submodule(const char *path)\n {\n \tssize_t len;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n-\tconst char *argv[] = {\n-\t\t\"status\",\n-\t\t\"--porcelain\",\n-\t\t\"-u\",\n-\t\t\"--ignore-submodules=none\",\n-\t\tNULL,\n-\t};\n \tstruct strbuf buf = STRBUF_INIT;\n \tint ok_to_remove = 1;\n \n@@ -1038,14 +1031,15 @@ int ok_to_remove_submodule(const char *path)\n \tif (!submodule_uses_gitfile(path))\n \t\treturn 0;\n \n-\tcp.argv = argv;\n+\targv_array_pushl(&cp.args, \"status\", \"--porcelain\", \"-u\",\n+\t\t\t\t \"--ignore-submodules=none\", NULL);\n \tprepare_submodule_repo_env(&cp.env_array);\n \tcp.git_cmd = 1;\n \tcp.no_stdin = 1;\n \tcp.out = -1;\n \tcp.dir = path;\n \tif (start_command(&cp))\n-\t\tdie(\"Could not run 'git status --porcelain -uall --ignore-submodules=none' in submodule %s\", path);\n+\t\tdie(\"Could not run 'git status --porcelain -u --ignore-submodules=none' in submodule %s\", path);\n \n \tlen = strbuf_read(&buf, cp.out, 1024);\n \tif (len > 2)\n@@ -1053,7 +1047,7 @@ int ok_to_remove_submodule(const char *path)\n \tclose(cp.out);\n \n \tif (finish_command(&cp))\n-\t\tdie(\"'git status --porcelain -uall --ignore-submodules=none' failed in submodule %s\", path);\n+\t\tdie(\"'git status --porcelain -u --ignore-submodules=none' failed in submodule %s\", path);\n \n \tstrbuf_release(&buf);\n \treturn ok_to_remove;\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306836","messageId":"20161203003022.29797-1-sbeller@google.com","threadId":"44604","inReplyTo":null,"subject":"[RFC PATCHv2 00/17] Checkout aware of Submodules!","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:05Z","receivedAt":"2016-12-03T00:30:59Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"v2:\n* based on top of the series sent out an hour ago\n \"[PATCHv4 0/5] submodule embedgitdirs\"\n* Try to embed a submodule if we need to remove it.\n* Strictly do not change behavior if not giving the new flag.\n* I think I missed some review comments from v1, but I'd like to get\n the current state out over the weekend, as a lot has changed so far.\n On Monday I'll go through the previous discussion with a comb to see\n if I missed something.\n \nv1:\nWhen working with submodules, nearly anytime after checking out\na different state of the projects, that has submodules changed\nyou'd run \"git submodule update\" with a current version of Git.\n\nThere are two problems with this approach:\n\n* The \"submodule update\" command is dangerous as it\n doesn't check for work that may be lost in the submodule\n (e.g. a dangling commit).\n* you may forget to run the command as checkout is supposed\n to do all the work for you.\n\nIntegrate updating the submodules into git checkout, with the same\nsafety promises that git-checkout has, i.e. not throw away data unless\nasked to. This is done by first checking if the submodule is at the same\nsha1 as it is recorded in the superproject. If there are changes we stop\nproceeding the checkout just like it is when checking out a file that\nhas local changes.\n\nThe integration happens in the code that is also used in other commands\nsuch that it will be easier in the future to make other commands aware\nof submodule.\n\nThis also solves d/f conflicts in case you replace a file/directory\nwith a submodule or vice versa.\n\nThe patches are still a bit rough, but the overall series seems\npromising enough to me that I want to put it out here.\n\nAny review, specifically on the design level welcome!\n\nThanks,\nStefan\n\n\nStefan Beller (17):\n submodule.h: add extern keyword to functions\n submodule: modernize ok_to_remove_submodule to use argv_array\n update submodules: move up prepare_submodule_repo_env\n update submodules: add is_submodule_populated\n update submodules: add submodule config parsing\n update submodules: add a config option to determine if submodules are\n updated\n update submodules: introduce submodule_is_interesting\n update submodules: add depopulate_submodule\n update submodules: add scheduling to update submodules\n update submodules: is_submodule_checkout_safe\n unpack-trees: teach verify_clean_submodule to inspect submodules\n unpack-trees: remove submodule contents if interesting\n entry: write_entry to write populate submodules\n submodule: teach unpack_trees() to update submodules\n checkout: recurse into submodules if asked to\n completion: add '--recurse-submodules' to checkout\n checkout: add config option to recurse into submodules by default\n\n Documentation/config.txt | 6 +\n Documentation/git-checkout.txt | 8 +\n builtin/checkout.c | 31 +++-\n cache.h | 2 +\n contrib/completion/git-completion.bash | 2 +-\n entry.c | 14 +-\n submodule-config.c | 22 +++\n submodule-config.h | 17 +-\n submodule.c | 292 +++++++++++++++++++++++++++++---\n submodule.h | 74 ++++++---\n t/t2013-checkout-submodule.sh | 293 ++++++++++++++++++++++++++++++++-\n t/t9902-completion.sh | 1 +\n unpack-trees.c | 92 ++++++++---\n unpack-trees.h | 1 +\n 14 files changed, 768 insertions(+), 87 deletions(-)\n\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306837","messageId":"20161203003022.29797-13-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 12/17] unpack-trees: remove submodule contents if interesting","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:17Z","receivedAt":"2016-12-03T00:31:03Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Currently when a unlink_entry() is called on a submodule, this fails\nas remove_or_warn detects it needs to delete a directory via rmdir.\nHowever rmdir only works on empty directories, such that the\n\"_or_warn\" part kicks in, and we get a warning message.\n\nIn case the submodule is of no interest we're not going to delete the\nsubmodule, so a warning like that is useful.\n\nIf the submodule is interesting then we need to depopulate it properly,\nthere is no need to react on its return code the depopulation method\nhandles proper warnings nor do we need to schedule the directory for\nremoval later, such that we can return early.\n\nSigned-off-by: Stefan Beller \n---\n unpack-trees.c | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 22e32eca96..db03293347 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -214,7 +214,12 @@ static void unlink_entry(const struct cache_entry *ce)\n {\n \tif (!check_leading_path(ce->name, ce_namelen(ce)))\n \t\treturn;\n-\tif (remove_or_warn(ce->ce_mode, ce->name))\n+\n+\tif (S_ISGITLINK(ce->ce_mode) &&\n+\t submodule_is_interesting(ce->name)) {\n+\t\tdepopulate_submodule(ce->name);\n+\t\treturn;\n+\t} else if (remove_or_warn(ce->ce_mode, ce->name))\n \t\treturn;\n \tschedule_dir_for_removal(ce->name, ce_namelen(ce));\n }\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306838","messageId":"20161203003022.29797-10-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 09/17] update submodules: add scheduling to update submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:14Z","receivedAt":"2016-12-03T00:31:13Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The walker of a tree is only expected to call `schedule_submodule_for_update`\nand once done, to run `update_submodules`. This avoids directory/file\nconflicts and later we can parallelize all submodule actions if needed.\n\nSigned-off-by: Stefan Beller \n---\n submodule.c | 132 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n submodule.h | 5 +++\n 2 files changed, 137 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex 7bb64d6c69..02c28ef56b 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1348,3 +1348,135 @@ int parallel_submodules(void)\n {\n \treturn parallel_jobs;\n }\n+\n+static struct scheduled_submodules_update_type {\n+\tconst char *path;\n+\tconst struct object_id *oid;\n+\t/*\n+\t * Do we need to perform a complete checkout or just incremental\n+\t * update?\n+\t */\n+\tunsigned is_new:1;\n+} *scheduled_submodules;\n+#define SCHEDULED_SUBMODULES_INIT {NULL, NULL, 0}\n+\n+static int scheduled_submodules_nr, scheduled_submodules_alloc;\n+\n+void schedule_submodule_for_update(const struct cache_entry *ce, int is_new)\n+{\n+\tstruct scheduled_submodules_update_type *ssu;\n+\tALLOC_GROW(scheduled_submodules,\n+\t\t scheduled_submodules_nr + 1,\n+\t\t scheduled_submodules_alloc);\n+\tssu = &scheduled_submodules[scheduled_submodules_nr++];\n+\tssu->path = ce->name;\n+\tssu->oid = &ce->oid;\n+\tssu->is_new = !!is_new;\n+}\n+\n+static int update_submodule(const char *path, const struct object_id *oid,\n+\t\t\t int force, int is_new)\n+{\n+\tconst char *git_dir;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tconst struct submodule *sub = submodule_from_path(null_sha1, path);\n+\n+\tif (!sub || !sub->name)\n+\t\treturn -1;\n+\n+\tgit_dir = resolve_gitdir(git_common_path(\"modules/%s\", sub->name));\n+\n+\tif (!git_dir)\n+\t\treturn -1;\n+\n+\tif (is_new)\n+\t\tconnect_work_tree_and_git_dir(path, git_dir);\n+\n+\t/* update index via `read-tree --reset sha1` */\n+\targv_array_pushl(&cp.args, \"read-tree\",\n+\t\t\t\t force ? \"--reset\" : \"-m\",\n+\t\t\t\t \"-u\", sha1_to_hex(oid->hash), NULL);\n+\tprepare_submodule_repo_env(&cp.env_array);\n+\tcp.git_cmd = 1;\n+\tcp.no_stdin = 1;\n+\tcp.dir = path;\n+\tif (run_command(&cp)) {\n+\t\twarning(_(\"reading the index in submodule '%s' failed\"), path);\n+\t\tchild_process_clear(&cp);\n+\t\treturn -1;\n+\t}\n+\n+\t/* write index to working dir */\n+\tchild_process_clear(&cp);\n+\tchild_process_init(&cp);\n+\targv_array_pushl(&cp.args, \"checkout-index\", \"-a\", NULL);\n+\tcp.git_cmd = 1;\n+\tcp.no_stdin = 1;\n+\tcp.dir = path;\n+\tif (force)\n+\t\targv_array_push(&cp.args, \"-f\");\n+\n+\tif (run_command(&cp)) {\n+\t\twarning(_(\"populating the working directory in submodule '%s' failed\"), path);\n+\t\tchild_process_clear(&cp);\n+\t\treturn -1;\n+\t}\n+\n+\t/* get the HEAD right */\n+\tchild_process_clear(&cp);\n+\tchild_process_init(&cp);\n+\targv_array_pushl(&cp.args, \"checkout\", \"--recurse-submodules\", NULL);\n+\tcp.git_cmd = 1;\n+\tcp.no_stdin = 1;\n+\tcp.dir = path;\n+\tif (force)\n+\t\targv_array_push(&cp.args, \"-f\");\n+\targv_array_push(&cp.args, sha1_to_hex(oid->hash));\n+\n+\tif (run_command(&cp)) {\n+\t\twarning(_(\"setting the HEAD in submodule '%s' failed\"), path);\n+\t\tchild_process_clear(&cp);\n+\t\treturn -1;\n+\t}\n+\n+\tchild_process_clear(&cp);\n+\treturn 0;\n+}\n+\n+int update_submodules(int force)\n+{\n+\tint i;\n+\tgitmodules_config();\n+\n+\t/*\n+\t * NEEDSWORK: As submodule updates can potentially take some\n+\t * time each and they do not overlap (i.e. no d/f conflicts;\n+\t * this can be parallelized using the run_commands.h API.\n+\t */\n+\tfor (i = 0; i < scheduled_submodules_nr; i++) {\n+\t\tstruct submodule *sub;\n+\t\tstruct scheduled_submodules_update_type *ssu =\n+\t\t\t&scheduled_submodules[i];\n+\n+\t\tif (!submodule_is_interesting(ssu->path))\n+\t\t\tcontinue;\n+\n+\t\tsub = submodule_from_path(null_sha1, ssu->path);\n+\n+\t\tswitch (sub->update_strategy) {\n+\t\tcase SM_UPDATE_UNSPECIFIED: /* fall thru */\n+\t\tcase SM_UPDATE_CHECKOUT:\n+\t\t\tupdate_submodule(ssu->path, ssu->oid,\n+\t\t\t\t\t force, ssu->is_new);\n+\t\t\tbreak;\n+\t\tcase SM_UPDATE_REBASE:\n+\t\tcase SM_UPDATE_MERGE:\n+\t\tcase SM_UPDATE_NONE:\n+\t\tcase SM_UPDATE_COMMAND:\n+\t\t\twarning(\"update strategy for submodule '%s' not supported\", ssu->path);\n+\t\t}\n+\t}\n+\n+\tscheduled_submodules_nr = 0;\n+\treturn 0;\n+}\ndiff --git a/submodule.h b/submodule.h\nindex d8bb1d4baf..74df8b93d5 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -90,4 +90,9 @@ extern int parallel_submodules(void);\n * retaining any config in the environment.\n */\n extern void prepare_submodule_repo_env(struct argv_array *out);\n+\n+extern void schedule_submodule_for_update(const struct cache_entry *ce,\n+\t\t\t\t\t int new);\n+extern int update_submodules(int force);\n+\n #endif\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306839","messageId":"20161203003022.29797-8-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 07/17] update submodules: introduce submodule_is_interesting","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:12Z","receivedAt":"2016-12-03T00:31:16Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In later patches we introduce the --recurse-submodule flag for commands\nthat modify the working directory, e.g. git-checkout.\n\nIt is potentially expensive to check if a submodule needs an update,\nbecause a common theme to interact with submodules is to spawn a child\nprocess for each interaction.\n\nSo let's introduce a function that pre checks if a submodule needs\nto be checked for an update.\n\nI am not particular happy with the name `submodule_is_interesting`,\nin internal iterations I had `submodule_requires_check_for_update`\nand `submodule_needs_update`, but I was even less happy with those\nnames. Maybe `submodule_interesting_for_update`?\n\nGenerally this is to answer \"Am I allowed to touch the submodule\nat all?\" or: \"Does the user expect me to touch it?\"\nwhich includes all of creation/deletion/update.\n\nThis patch is based off a prior attempt by Jens Lehmann to add\nsubmodules to checkout.\n\nSigned-off-by: Stefan Beller \n---\n submodule.c | 23 +++++++++++++++++++++++\n submodule.h | 9 +++++++++\n 2 files changed, 32 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex 1ba398ba3b..62e9ef3872 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -516,6 +516,29 @@ void set_config_update_recurse_submodules(int value)\n \tconfig_update_recurse_submodules = value;\n }\n \n+int submodules_interesting_for_update(void)\n+{\n+\t/*\n+\t * Update can't be \"none\", \"merge\" or \"rebase\",\n+\t * treat any value as OFF, except an explicit ON.\n+\t */\n+\treturn config_update_recurse_submodules == RECURSE_SUBMODULES_ON;\n+}\n+\n+int submodule_is_interesting(const char *path)\n+{\n+\tconst struct submodule *sub;\n+\n+\tif (!submodules_interesting_for_update())\n+\t\treturn 0;\n+\n+\tsub = submodule_from_path(null_sha1, path);\n+\tif (!sub)\n+\t\treturn 0;\n+\n+\treturn sub->update_strategy.type != SM_UPDATE_NONE;\n+}\n+\n static int has_remote(const char *refname, const struct object_id *oid,\n \t\t int flags, void *cb_data)\n {\ndiff --git a/submodule.h b/submodule.h\nindex 21236b095c..7d890e0464 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -54,6 +54,15 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,\n \t\tconst struct diff_options *opt);\n extern void set_config_fetch_recurse_submodules(int value);\n extern void set_config_update_recurse_submodules(int value);\n+\n+/**\n+ * When updating the working tree, we need to check if the submodule needs\n+ * updating. We do not require a check if we are already sure that the\n+ * submodule doesn't need updating, e.g. when we are not interested in submodules\n+ * or the submodule is marked uninteresting by being not initialized.\n+ */\n+extern int submodule_is_interesting(const char *path);\n+extern int submodules_interesting_for_update(void);\n extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n extern int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306840","messageId":"20161203003022.29797-9-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 08/17] update submodules: add depopulate_submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:13Z","receivedAt":"2016-12-03T00:31:17Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Implement the functionality needed to enable work tree manipulating\ncommands so that a deleted submodule should not only affect the index\n(leaving all the files of the submodule in the work tree) but also to\nremove the work tree of the superproject (including any untracked\nfiles).\n\nTo do so, we need an equivalent of \"rm -rf\", which is already found in\nentry.c, so expose that and for clarity add a suffix \"_or_dir\" to it.\n\nThat will only work properly when the submodule uses a gitfile instead of\na .git directory and no untracked files are present. Otherwise the removal\nwill fail with a warning (which is just what happened until now).\n\nSigned-off-by: Stefan Beller \n---\n cache.h | 2 ++\n entry.c | 5 +++++\n submodule.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++\n submodule.h | 1 +\n 4 files changed, 54 insertions(+)\n\ndiff --git a/cache.h b/cache.h\nindex a50a61a197..b645ca2f9a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -2018,4 +2018,6 @@ void sleep_millisec(int millisec);\n */\n void safe_create_dir(const char *dir, int share);\n \n+extern void remove_directory_or_die(struct strbuf *path);\n+\n #endif /* CACHE_H */\ndiff --git a/entry.c b/entry.c\nindex c6eea240b6..02c4ac9f22 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -73,6 +73,11 @@ static void remove_subtree(struct strbuf *path)\n \t\tdie_errno(\"cannot rmdir '%s'\", path->buf);\n }\n \n+void remove_directory_or_die(struct strbuf *path)\n+{\n+\tremove_subtree(path);\n+}\n+\n static int create_file(const char *path, unsigned int mode)\n {\n \tmode = (mode & 0100) ? 0777 : 0666;\ndiff --git a/submodule.c b/submodule.c\nindex 62e9ef3872..7bb64d6c69 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -324,6 +324,52 @@ void prepare_submodule_repo_env(struct argv_array *out)\n \targv_array_push(out, \"GIT_DIR=.git\");\n }\n \n+int depopulate_submodule(const char *path)\n+{\n+\tint ret = 0;\n+\tstruct strbuf pathbuf = STRBUF_INIT;\n+\tchar *dot_git = xstrfmt(\"%s/.git\", path);\n+\n+\t/* Is it populated? */\n+\tif (!resolve_gitdir(dot_git))\n+\t\tgoto out;\n+\n+\t/* Does it have a .git directory? */\n+\tif (!submodule_uses_gitfile(path)) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\t\tprepare_submodule_repo_env(&cp.env_array);\n+\t\targv_array_pushl(&cp.args, \"submodule--helper\",\n+\t\t\t\t \"embed-git-dirs\", path, NULL);\n+\t\tcp.git_cmd = 1;\n+\t\tif (run_command(&cp)) {\n+\t\t\twarning(_(\"Cannot remove submodule '%s'\\n\"\n+\t\t\t\t \"because it (or one of its nested submodules) has a git \\n\"\n+\t\t\t\t \"directory in the working tree, which could not be embedded\\n\"\n+\t\t\t\t \"the superprojects git directory automatically.\"), path);\n+\t\t\tret = -1;\n+\t\t\tgoto out;\n+\t\t}\n+\n+\t\tif (!submodule_uses_gitfile(path)) {\n+\t\t\t/*\n+\t\t\t * We should be using a gitfile by now, let's double\n+\t\t\t * check as loosing the git dir would be fatal.\n+\t\t\t */\n+\t\t\tdie(\"BUG: \\\"git submodule--helper embed git-dirs '%s'\\\" \"\n+\t\t\t \"did not embed the git-dirs recursively for '%s'\",\n+\t\t\t path, path);\n+\t\t}\n+\t}\n+\n+\tstrbuf_addstr(&pathbuf, path);\n+\tremove_directory_or_die(&pathbuf);\n+out:\n+\tstrbuf_release(&pathbuf);\n+\tfree(dot_git);\n+\treturn ret;\n+}\n+\n /* Helper function to display the submodule header line prior to the full\n * summary output. If it can locate the submodule objects directory it will\n * attempt to lookup both the left and right commits and put them into the\ndiff --git a/submodule.h b/submodule.h\nindex 7d890e0464..d8bb1d4baf 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -63,6 +63,7 @@ extern void set_config_update_recurse_submodules(int value);\n */\n extern int submodule_is_interesting(const char *path);\n extern int submodules_interesting_for_update(void);\n+extern int depopulate_submodule(const char *path);\n extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n extern int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306841","messageId":"20161203003022.29797-5-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 04/17] update submodules: add is_submodule_populated","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:09Z","receivedAt":"2016-12-03T00:31:18Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"See if a submodule is populated by checking if there\nis a .git file or directory at the given path.\n\nSigned-off-by: Stefan Beller \n---\n submodule.c | 11 +++++++++++\n submodule.h | 1 +\n 2 files changed, 12 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex 78b69b5a55..c29153e9ff 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -930,6 +930,17 @@ int fetch_populated_submodules(const struct argv_array *options,\n \treturn spf.result;\n }\n \n+int is_submodule_populated(const char *path)\n+{\n+\tint retval = 0;\n+\tstruct strbuf gitdir = STRBUF_INIT;\n+\tstrbuf_addf(&gitdir, \"%s/.git\", path);\n+\tif (resolve_gitdir(gitdir.buf))\n+\t\tretval = 1;\n+\tstrbuf_release(&gitdir);\n+\treturn retval;\n+}\n+\n unsigned is_submodule_modified(const char *path, int ignore_untracked)\n {\n \tssize_t len;\ndiff --git a/submodule.h b/submodule.h\nindex 5db0b53b3f..a9eabcc3d0 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -58,6 +58,7 @@ extern int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n \t\t\t int quiet, int max_parallel_jobs);\n extern unsigned is_submodule_modified(const char *path, int ignore_untracked);\n+extern int is_submodule_populated(const char *path);\n extern int submodule_uses_gitfile(const char *path);\n extern int ok_to_remove_submodule(const char *path);\n extern int merge_submodule(unsigned char result[20], const char *path,\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306842","messageId":"20161203003022.29797-11-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 10/17] update submodules: is_submodule_checkout_safe","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:15Z","receivedAt":"2016-12-03T00:31:19Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In later patches we introduce the options and flag for commands\nthat modify the working directory, e.g. git-checkout.\n\nThis piece of code will answer the question:\n\"Is it safe to change the submodule to this new state?\"\ne.g. is it overwriting untracked files or are there local\nchanges that would be overwritten?\n\nSigned-off-by: Stefan Beller \n---\n submodule.c | 37 +++++++++++++++++++++++++++++++++++++\n submodule.h | 2 ++\n 2 files changed, 39 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex 02c28ef56b..4253f7f044 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1155,6 +1155,43 @@ int ok_to_remove_submodule(const char *path)\n \treturn ok_to_remove;\n }\n \n+/**\n+ * Check if a submodule update to a given sha1 is safe.\n+ * Return 1 if it is safe, 0 when it is not.\n+ *\n+ * If the submodule is not populated, we need to check\n+ */\n+int is_submodule_checkout_safe(const char *path,\n+\t\t\t const struct object_id *new_hash)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\targv_array_pushl(&cp.args, \"read-tree\", \"-n\", \"-m\", \"HEAD\",\n+\t\t\tsha1_to_hex(new_hash->hash), NULL);\n+\n+\tif (!is_submodule_populated(path)) {\n+\t\tconst struct submodule *sub;\n+\n+\t\t/* See if we have the submodule configured already: */\n+\t\tsub = submodule_from_path(null_sha1, path);\n+\n+\t\tprepare_submodule_repo_env_no_git_dir(&cp.env_array);\n+\t\targv_array_pushf(&cp.env_array, \"GIT_DIR=%s\",\n+\t\t\t\t sub ? sub->name : path);\n+\t\targv_array_pushf(&cp.env_array, \"GIT_WORK_TREE=%s\", path);\n+\t} else {\n+\t\tprepare_submodule_repo_env(&cp.env_array);\n+\t}\n+\n+\tcp.git_cmd = 1;\n+\tcp.no_stdin = 1;\n+\tcp.no_stdout = 1;\n+\tcp.no_stderr = 1;\n+\tcp.dir = path;\n+\n+\treturn run_command(&cp) == 0;\n+}\n+\n static int find_first_merges(struct object_array *result, const char *path,\n \t\tstruct commit *a, struct commit *b)\n {\ndiff --git a/submodule.h b/submodule.h\nindex 74df8b93d5..1dfcd6939b 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -72,6 +72,8 @@ extern unsigned is_submodule_modified(const char *path, int ignore_untracked);\n extern int is_submodule_populated(const char *path);\n extern int submodule_uses_gitfile(const char *path);\n extern int ok_to_remove_submodule(const char *path);\n+extern int is_submodule_checkout_safe(const char *path,\n+\t\t\t\t const struct object_id *new_hash);\n extern int merge_submodule(unsigned char result[20], const char *path,\n \t\t\t const unsigned char base[20],\n \t\t\t const unsigned char a[20],\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306843","messageId":"20161203003022.29797-6-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 05/17] update submodules: add submodule config parsing","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:10Z","receivedAt":"2016-12-03T00:31:21Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Similar to b33a15b08 (push: add recurseSubmodules config option,\n2015-11-17) and 027771fcb1 (submodule: allow erroneous values for the\nfetchRecurseSubmodules option, 2015-08-17), we add submodule-config code\nthat is later used to parse whether we are interested in updating\nsubmodules.\n\nWe need the `die_on_error` parameter to be able to call this parsing\nfunction for the config file as well, which if incorrect lets Git die.\n\nAs we're just touching the header file, also mark all functions extern.\n\nSigned-off-by: Stefan Beller \n---\n submodule-config.c | 22 ++++++++++++++++++++++\n submodule-config.h | 17 +++++++++--------\n 2 files changed, 31 insertions(+), 8 deletions(-)\n\ndiff --git a/submodule-config.c b/submodule-config.c\nindex 098085be69..4b5297e31d 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -234,6 +234,28 @@ int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg)\n \treturn parse_fetch_recurse(opt, arg, 1);\n }\n \n+static int parse_update_recurse(const char *opt, const char *arg,\n+\t\t\t\tint die_on_error)\n+{\n+\tswitch (git_config_maybe_bool(opt, arg)) {\n+\tcase 1:\n+\t\treturn RECURSE_SUBMODULES_ON;\n+\tcase 0:\n+\t\treturn RECURSE_SUBMODULES_OFF;\n+\tdefault:\n+\t\tif (!strcmp(arg, \"checkout\"))\n+\t\t\treturn RECURSE_SUBMODULES_ON;\n+\t\tif (die_on_error)\n+\t\t\tdie(\"bad %s argument: %s\", opt, arg);\n+\t\treturn RECURSE_SUBMODULES_ERROR;\n+\t}\n+}\n+\n+int parse_update_recurse_submodules_arg(const char *opt, const char *arg)\n+{\n+\treturn parse_update_recurse(opt, arg, 1);\n+}\n+\n static int parse_push_recurse(const char *opt, const char *arg,\n \t\t\t int die_on_error)\n {\ndiff --git a/submodule-config.h b/submodule-config.h\nindex d05c542d2c..992bfbebc0 100644\n--- a/submodule-config.h\n+++ b/submodule-config.h\n@@ -22,13 +22,14 @@ struct submodule {\n \tint recommend_shallow;\n };\n \n-int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n-int parse_push_recurse_submodules_arg(const char *opt, const char *arg);\n-int parse_submodule_config_option(const char *var, const char *value);\n-const struct submodule *submodule_from_name(const unsigned char *commit_sha1,\n-\t\tconst char *name);\n-const struct submodule *submodule_from_path(const unsigned char *commit_sha1,\n-\t\tconst char *path);\n-void submodule_free(void);\n+extern int parse_fetch_recurse_submodules_arg(const char *opt, const char *arg);\n+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 int parse_submodule_config_option(const char *var, const char *value);\n+extern const struct submodule *submodule_from_name(\n+\t\tconst unsigned char *commit_sha1, const char *name);\n+extern const struct submodule *submodule_from_path(\n+\t\tconst unsigned char *commit_sha1, const char *path);\n+extern void submodule_free(void);\n \n #endif /* SUBMODULE_CONFIG_H */\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306844","messageId":"20161203003022.29797-16-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 15/17] checkout: recurse into submodules if asked to","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:20Z","receivedAt":"2016-12-03T00:31:24Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Allow checkout to recurse into submodules via\nthe command line option --[no-]recurse-submodules.\n\nThe flag for recurse-submodules in its current form\ncould be an OPT_BOOL, but eventually we may want to have\nit as:\n\n git checkout --recurse-submodules=rebase|merge| \\\n\t\t\tcherry-pick|checkout|none\n\nwhich resembles the submodule..update options,\nso naturally a value such as\n\"as-configured-or-X-as-fallback\" would also come in handy.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/git-checkout.txt | 7 ++\n builtin/checkout.c | 31 ++++-\n t/t2013-checkout-submodule.sh | 277 +++++++++++++++++++++++++++++++++++++++--\n 3 files changed, 306 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex 8e2c0662dd..a0ea2c5651 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -256,6 +256,13 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode.\n \tout anyway. In other words, the ref can be held by more than one\n \tworktree.\n \n+--[no-]recurse-submodules::\n+\tUsing --recurse-submodules will update the content of all initialized\n+\tsubmodules according to the commit recorded in the superproject. If\n+\tlocal modifications in a submodule would be overwritten the checkout\n+\twill fail until `-f` is used. If nothing (or --no-recurse-submodules)\n+\tis used, the work trees of submodules will not be updated.\n+\n ::\n \tBranch to checkout; if it refers to a branch (i.e., a name that,\n \twhen prepended with \"refs/heads/\", is a valid ref), then that\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 512492aad9..5db0d933d1 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -21,12 +21,31 @@\n #include \"submodule-config.h\"\n #include \"submodule.h\"\n \n+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n+\n static const char * const checkout_usage[] = {\n \tN_(\"git checkout [] \"),\n \tN_(\"git checkout [] [] -- ...\"),\n \tNULL,\n };\n \n+int option_parse_recurse_submodules(const struct option *opt,\n+\t\t\t\t const char *arg, int unset)\n+{\n+\tif (unset) {\n+\t\trecurse_submodules = RECURSE_SUBMODULES_OFF;\n+\t\treturn 0;\n+\t}\n+\tif (arg)\n+\t\trecurse_submodules =\n+\t\t\tparse_update_recurse_submodules_arg(opt->long_name,\n+\t\t\t\t\t\t\t arg);\n+\telse\n+\t\trecurse_submodules = RECURSE_SUBMODULES_ON;\n+\n+\treturn 0;\n+}\n+\n struct checkout_opts {\n \tint patch_mode;\n \tint quiet;\n@@ -826,7 +845,8 @@ static int switch_branches(const struct checkout_opts *opts,\n \t\tparse_commit_or_die(new->commit);\n \t}\n \n-\tret = merge_working_tree(opts, &old, new, &writeout_error);\n+\tret = merge_working_tree(opts, &old, new, &writeout_error) ||\n+\t update_submodules(opts->force);\n \tif (ret) {\n \t\tfree(path_to_free);\n \t\treturn ret;\n@@ -1160,6 +1180,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\t\t\tN_(\"second guess 'git checkout '\")),\n \t\tOPT_BOOL(0, \"ignore-other-worktrees\", &opts.ignore_other_worktrees,\n \t\t\t N_(\"do not check if another worktree is holding the given ref\")),\n+\t\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules,\n+\t\t\t \"checkout\", \"control recursive updating of submodules\",\n+\t\t\t PARSE_OPT_OPTARG, option_parse_recurse_submodules },\n \t\tOPT_BOOL(0, \"progress\", &opts.show_progress, N_(\"force progress reporting\")),\n \t\tOPT_END(),\n \t};\n@@ -1190,6 +1213,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tgit_xmerge_config(\"merge.conflictstyle\", conflict_style, NULL);\n \t}\n \n+\tif (recurse_submodules != RECURSE_SUBMODULES_OFF) {\n+\t\tgit_config(submodule_config, NULL);\n+\t\tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT)\n+\t\t\tset_config_update_recurse_submodules(recurse_submodules);\n+\t}\n+\n \tif ((!!opts.new_branch + !!opts.new_branch_force + !!opts.new_orphan_branch) > 1)\n \t\tdie(_(\"-b, -B and --orphan are mutually exclusive\"));\n \ndiff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh\nindex 6847f75822..33fb2f5de3 100755\n--- a/t/t2013-checkout-submodule.sh\n+++ b/t/t2013-checkout-submodule.sh\n@@ -7,15 +7,21 @@ test_description='checkout can handle submodules'\n \n test_expect_success 'setup' '\n \tmkdir submodule &&\n-\t(cd submodule &&\n-\t git init &&\n-\t test_commit first) &&\n-\tgit add submodule &&\n+\t(\n+\t\tcd submodule &&\n+\t\tgit init &&\n+\t\ttest_commit first\n+\t) &&\n+\techo first >file &&\n+\tgit add file submodule &&\n \ttest_tick &&\n \tgit commit -m superproject &&\n-\t(cd submodule &&\n-\t test_commit second) &&\n-\tgit add submodule &&\n+\t(\n+\t\tcd submodule &&\n+\t\ttest_commit second\n+\t) &&\n+\techo second > file &&\n+\tgit add file submodule &&\n \ttest_tick &&\n \tgit commit -m updated.superproject\n '\n@@ -37,7 +43,8 @@ test_expect_success '\"checkout \" updates the index only' '\n \tgit checkout HEAD^ submodule &&\n \ttest_must_fail git diff-files --quiet &&\n \tgit checkout HEAD submodule &&\n-\tgit diff-files --quiet\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD\n '\n \n test_expect_success '\"checkout \" honors diff.ignoreSubmodules' '\n@@ -63,6 +70,260 @@ test_expect_success '\"checkout \" honors submodule.*.ignore from .git/\n \t! test -s actual\n '\n \n+submodule_creation_must_succeed() {\n+\t# checkout base ($1)\n+\tgit checkout -f --recurse-submodules $1 &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached $1 &&\n+\n+\t# checkout target ($2)\n+\tif test -d submodule; then\n+\t\techo change >>submodule/first.t &&\n+\t\ttest_must_fail git checkout --recurse-submodules $2 &&\n+\t\tgit checkout -f --recurse-submodules $2\n+\telse\n+\t\tgit checkout --recurse-submodules $2\n+\tfi &&\n+\ttest -e submodule/.git &&\n+\ttest -f submodule/first.t &&\n+\ttest -f submodule/second.t &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached $2\n+}\n+\n+test_expect_success 'setup the submodule config' '\n+\tgit config -f .gitmodules submodule.submodule.path submodule &&\n+\tgit config -f .gitmodules submodule.submodule.url ./submodule.bare &&\n+\tgit -C submodule clone --bare . ../submodule.bare &&\n+\techo submodule.bare >>.gitignore &&\n+\tgit add .gitignore .gitmodules submodule &&\n+\tgit commit -m \"submodule registered with a gitmodules file\" &&\n+\tgit config submodule.submodule.url ./submodule.bare\n+'\n+\n+test_expect_success '\"checkout --recurse-submodules\" migrates submodule git dir before deleting' '\n+\tgit checkout -b base &&\n+\tgit checkout -b delete_submodule &&\n+\tgit update-index --force-remove submodule &&\n+\tgit config -f .gitmodules --unset submodule.submodule.path &&\n+\tgit config -f .gitmodules --unset submodule.submodule.url &&\n+\tgit add .gitmodules &&\n+\tgit commit -m \"submodule deleted\" &&\n+\tgit checkout base &&\n+\tgit checkout --recurse-submodules delete_submodule 2>output.err 1>output.out &&\n+\ttest_i18ngrep \"Migrating git directory\" output.out &&\n+\t! test -d submodule\n+'\n+\n+test_expect_success '\"check --recurse-submodules\" removes deleted submodule' '\n+\t# Make sure we have the submodule here and ready.\n+\tgit checkout base &&\n+\tgit submodule embedgitdirs &&\n+\tgit submodule update -f . &&\n+\ttest -e submodule/.git &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached base &&\n+\n+\t# Check if the checkout deletes the submodule.\n+\techo change >>submodule/first.t &&\n+\ttest_must_fail git checkout --recurse-submodules delete_submodule &&\n+\tgit checkout -f --recurse-submodules delete_submodule &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached delete_submodule &&\n+\t! test -d submodule\n+'\n+\n+test_expect_success '\"checkout --recurse-submodules\" repopulates submodule' '\n+\tsubmodule_creation_must_succeed delete_submodule base\n+'\n+\n+test_expect_success '\"checkout --recurse-submodules\" repopulates submodule in existing directory' '\n+\tgit checkout --recurse-submodules delete_submodule &&\n+\tmkdir submodule &&\n+\tsubmodule_creation_must_succeed delete_submodule base\n+'\n+\n+test_expect_success '\"checkout --recurse-submodules\" replaces submodule with files' '\n+\tgit checkout -f base &&\n+\tgit checkout -b replace_submodule_with_file &&\n+\tgit rm -f submodule &&\n+\techo \"file instead\" >submodule &&\n+\tgit add submodule &&\n+\tgit commit -m \"submodule replaced\" &&\n+\tgit checkout -f base &&\n+\tgit submodule update -f . &&\n+\tgit checkout --recurse-submodules replace_submodule_with_file &&\n+\ttest -f submodule\n+'\n+\n+test_expect_success '\"checkout --recurse-submodules\" removes files and repopulates submodule' '\n+\tsubmodule_creation_must_succeed replace_submodule_with_file base\n+'\n+\n+test_expect_success '\"checkout --recurse-submodules\" replaces submodule with a directory' '\n+\tgit checkout -f base &&\n+\tgit checkout -b replace_submodule_with_dir &&\n+\tgit rm -f submodule &&\n+\tmkdir -p submodule/dir &&\n+\techo content >submodule/dir/file &&\n+\tgit add submodule &&\n+\tgit commit -m \"submodule replaced with a directory (file inside)\" &&\n+\tgit checkout -f base &&\n+\tgit submodule update -f . &&\n+\tgit checkout --recurse-submodules replace_submodule_with_dir &&\n+\ttest -d submodule &&\n+\t! test -e submodule/.git &&\n+\t! test -f submodule/first.t &&\n+\t! test -f submodule/second.t &&\n+\ttest -d submodule/dir\n+'\n+\n+test_expect_success '\"checkout --recurse-submodules\" removes the directory and repopulates submodule' '\n+\tsubmodule_creation_must_succeed replace_submodule_with_dir base\n+'\n+\n+test_expect_success SYMLINKS '\"checkout --recurse-submodules\" replaces submodule with a link' '\n+\tgit checkout -f base &&\n+\tgit checkout -b replace_submodule_with_link &&\n+\tgit rm -f submodule &&\n+\tln -s submodule &&\n+\tgit add submodule &&\n+\tgit commit -m \"submodule replaced with a link\" &&\n+\tgit checkout -f base &&\n+\tgit submodule update -f . &&\n+\tgit checkout --recurse-submodules replace_submodule_with_link &&\n+\ttest -L submodule\n+'\n+\n+test_expect_success SYMLINKS '\"checkout --recurse-submodules\" removes the link and repopulates submodule' '\n+\tsubmodule_creation_must_succeed replace_submodule_with_link base\n+'\n+\n+test_expect_success '\"checkout --recurse-submodules\" updates the submodule' '\n+\tgit checkout --recurse-submodules base &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD &&\n+\tgit checkout -b updated_submodule &&\n+\t(\n+\t\tcd submodule &&\n+\t\techo x >>first.t &&\n+\t\tgit add first.t &&\n+\t\ttest_commit third\n+\t) &&\n+\tgit add submodule &&\n+\ttest_tick &&\n+\tgit commit -m updated.superproject &&\n+\tgit checkout --recurse-submodules base &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD\n+'\n+\n+# In 293ab15eea34 we considered untracked ignored files in submodules\n+# expendable, we may want to revisit this decision by adding user as\n+# well as command specific configuration for it.\n+# When building in-tree the untracked ignored files are probably ok to remove\n+# in a case as tested here, but e.g. when git.git is a submodule, then a user\n+# may not want to lose a well crafted (but ignored by default) \"config.mak\"\n+# Commands like \"git rm\" may care less about untracked files in a submodule\n+# as the checkout command that removes a submodule as well.\n+test_expect_failure 'untracked file is not deleted' '\n+\tgit checkout --recurse-submodules base &&\n+\techo important >submodule/untracked &&\n+\ttest_must_fail git checkout --recurse-submodules delete_submodule &&\n+\tgit checkout -f --recurse-submodules delete_submodule\n+'\n+\n+test_expect_success 'ignored file works just fine' '\n+\tgit checkout --recurse-submodules base &&\n+\techo important >submodule/ignored &&\n+\techo ignored >.git/modules/submodule/info/exclude &&\n+\tgit checkout --recurse-submodules delete_submodule\n+'\n+\n+test_expect_success 'dirty file file is not deleted' '\n+\tgit checkout --recurse-submodules base &&\n+\techo important >submodule/first.t &&\n+\ttest_must_fail git checkout --recurse-submodules delete_submodule &&\n+\tgit checkout -f --recurse-submodules delete_submodule\n+'\n+\n+test_expect_success 'added to index is not deleted' '\n+\tgit checkout --recurse-submodules base &&\n+\techo important >submodule/to_index &&\n+\tgit -C submodule add to_index &&\n+\ttest_must_fail git checkout --recurse-submodules delete_submodule &&\n+\tgit checkout -f --recurse-submodules delete_submodule\n+'\n+\n+# This is ok in theory, we just need to make sure\n+# the garbage collection doesn't eat the commit.\n+test_expect_success 'different commit prevents from deleting' '\n+\tgit checkout --recurse-submodules base &&\n+\techo important >submodule/to_index &&\n+\tgit -C submodule add to_index &&\n+\ttest_must_fail git checkout --recurse-submodules delete_submodule &&\n+\tgit checkout -f --recurse-submodules delete_submodule\n+'\n+\n+test_expect_failure '\"checkout --recurse-submodules\" needs -f to update a modifed submodule commit' '\n+\tgit -C submodule checkout --recurse-submodules HEAD^ &&\n+\ttest_must_fail git checkout --recurse-submodules master &&\n+\ttest_must_fail git diff-files --quiet submodule &&\n+\tgit diff-files --quiet file &&\n+\tgit checkout --recurse-submodules -f master &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD\n+'\n+\n+test_expect_failure '\"checkout --recurse-submodules\" needs -f to update modifed submodule content' '\n+\techo modified >submodule/second.t &&\n+\ttest_must_fail git checkout --recurse-submodules HEAD^ &&\n+\ttest_must_fail git diff-files --quiet submodule &&\n+\tgit diff-files --quiet file &&\n+\tgit checkout --recurse-submodules -f HEAD^ &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD &&\n+\tgit checkout --recurse-submodules -f master &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD\n+'\n+\n+test_expect_failure '\"checkout --recurse-submodules\" ignores modified submodule content that would not be changed' '\n+\techo modified >expected &&\n+\tcp expected submodule/first.t &&\n+\tgit checkout --recurse-submodules HEAD^ &&\n+\ttest_cmp expected submodule/first.t &&\n+\ttest_must_fail git diff-files --quiet submodule &&\n+\tgit diff-index --quiet --cached HEAD &&\n+\tgit checkout --recurse-submodules -f master &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached HEAD\n+'\n+\n+test_expect_failure '\"checkout --recurse-submodules\" does not care about untracked submodule content' '\n+\techo untracked >submodule/untracked &&\n+\tgit checkout --recurse-submodules master &&\n+\tgit diff-files --quiet --ignore-submodules=untracked &&\n+\tgit diff-index --quiet --cached HEAD &&\n+\trm submodule/untracked\n+'\n+\n+test_expect_failure '\"checkout --recurse-submodules\" needs -f when submodule commit is not present (but does fail anyway)' '\n+\tgit checkout --recurse-submodules -b bogus_commit master &&\n+\tgit update-index --cacheinfo 160000 0123456789012345678901234567890123456789 submodule &&\n+\tBOGUS_TREE=$(git write-tree) &&\n+\tBOGUS_COMMIT=$(echo \"bogus submodule commit\" | git commit-tree $BOGUS_TREE) &&\n+\tgit commit -m \"bogus submodule commit\" &&\n+\tgit checkout --recurse-submodules -f master &&\n+\ttest_must_fail git checkout --recurse-submodules bogus_commit &&\n+\tgit diff-files --quiet &&\n+\ttest_must_fail git checkout --recurse-submodules -f bogus_commit &&\n+\ttest_must_fail git diff-files --quiet submodule &&\n+\tgit diff-files --quiet file &&\n+\tgit diff-index --quiet --cached HEAD &&\n+\tgit checkout --recurse-submodules -f master\n+'\n+\n test_submodule_switch \"git checkout\"\n \n test_submodule_forced_switch \"git checkout -f\"\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306845","messageId":"20161203003022.29797-4-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 03/17] update submodules: move up prepare_submodule_repo_env","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:08Z","receivedAt":"2016-12-03T00:31:25Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In a later patch we need to prepare the submodule environment with\nanother git directory, so split up the function.\n\nAlso move it up in the file such that we do not need to declare the\nfunction later before using it.\n\nSigned-off-by: Stefan Beller \n---\n submodule.c | 27 ++++++++++++++++-----------\n 1 file changed, 16 insertions(+), 11 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex eb80b0c5ad..78b69b5a55 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -307,6 +307,22 @@ static void print_submodule_summary(struct rev_info *rev, FILE *f,\n \tstrbuf_release(&sb);\n }\n \n+static void prepare_submodule_repo_env_no_git_dir(struct argv_array *out)\n+{\n+\tconst char * const *var;\n+\n+\tfor (var = local_repo_env; *var; var++) {\n+\t\tif (strcmp(*var, CONFIG_DATA_ENVIRONMENT))\n+\t\t\targv_array_push(out, *var);\n+\t}\n+}\n+\n+void prepare_submodule_repo_env(struct argv_array *out)\n+{\n+\tprepare_submodule_repo_env_no_git_dir(out);\n+\targv_array_push(out, \"GIT_DIR=.git\");\n+}\n+\n /* Helper function to display the submodule header line prior to the full\n * summary output. If it can locate the submodule objects directory it will\n * attempt to lookup both the left and right commits and put them into the\n@@ -1246,14 +1262,3 @@ int parallel_submodules(void)\n {\n \treturn parallel_jobs;\n }\n-\n-void prepare_submodule_repo_env(struct argv_array *out)\n-{\n-\tconst char * const *var;\n-\n-\tfor (var = local_repo_env; *var; var++) {\n-\t\tif (strcmp(*var, CONFIG_DATA_ENVIRONMENT))\n-\t\t\targv_array_push(out, *var);\n-\t}\n-\targv_array_push(out, \"GIT_DIR=.git\");\n-}\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306846","messageId":"20161203003022.29797-17-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 16/17] completion: add '--recurse-submodules' to checkout","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:21Z","receivedAt":"2016-12-03T00:31:27Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n contrib/completion/git-completion.bash | 2 +-\n t/t9902-completion.sh | 1 +\n 2 files changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 21016bf8df..28acfdb377 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -1068,7 +1068,7 @@ _git_checkout ()\n \t--*)\n \t\t__gitcomp \"\n \t\t\t--quiet --ours --theirs --track --no-track --merge\n-\t\t\t--conflict= --orphan --patch\n+\t\t\t--conflict= --orphan --patch --recurse-submodules\n \t\t\t\"\n \t\t;;\n \t*)\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 2ba62fbc17..d2d1102a3d 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -447,6 +447,7 @@ test_expect_success 'double dash \"git checkout\"' '\n \t--conflict=\n \t--orphan Z\n \t--patch Z\n+\t--recurse-submodules Z\n \tEOF\n '\n \n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306847","messageId":"20161203003022.29797-2-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 01/17] submodule.h: add extern keyword to functions","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:06Z","receivedAt":"2016-12-03T00:31:30Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"As the upcoming series will add a lot of functions to the submodule\nheader, let's first make the header consistent to the rest of the project\nby adding the extern keyword to functions.\n\nAs per the CodingGuidelines we try to stay below 80 characters per line,\nso adapt all those functions to stay below 80 characters that are already\nusing more than one line. Those function using just one line are better\nkept in one line than breaking them up into multiple lines just for the\ngoal of staying below the character limit as it makes grepping\nfor functions easier if they are one liners.\n\nSigned-off-by: Stefan Beller \n---\n submodule.h | 57 ++++++++++++++++++++++++++++++---------------------------\n 1 file changed, 30 insertions(+), 27 deletions(-)\n\ndiff --git a/submodule.h b/submodule.h\nindex d9e197a948..5db0b53b3f 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -29,50 +29,53 @@ struct submodule_update_strategy {\n };\n #define SUBMODULE_UPDATE_STRATEGY_INIT {SM_UPDATE_UNSPECIFIED, NULL}\n \n-int is_staging_gitmodules_ok(void);\n-int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n-int remove_path_from_gitmodules(const char *path);\n-void stage_updated_gitmodules(void);\n-void set_diffopt_flags_from_submodule_config(struct diff_options *diffopt,\n+extern int is_staging_gitmodules_ok(void);\n+extern int update_path_in_gitmodules(const char *oldpath, const char *newpath);\n+extern int remove_path_from_gitmodules(const char *path);\n+extern void stage_updated_gitmodules(void);\n+extern void set_diffopt_flags_from_submodule_config(struct diff_options *,\n \t\tconst char *path);\n-int submodule_config(const char *var, const char *value, void *cb);\n-void gitmodules_config(void);\n-int parse_submodule_update_strategy(const char *value,\n+extern int submodule_config(const char *var, const char *value, void *cb);\n+extern void gitmodules_config(void);\n+extern int parse_submodule_update_strategy(const char *value,\n \t\tstruct submodule_update_strategy *dst);\n-const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);\n-void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);\n-void show_submodule_summary(FILE *f, const char *path,\n+extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);\n+extern void handle_ignore_submodules_arg(struct diff_options *, const char *);\n+extern void show_submodule_summary(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n \t\tstruct object_id *one, struct object_id *two,\n \t\tunsigned dirty_submodule, const char *meta,\n \t\tconst char *del, const char *add, const char *reset);\n-void show_submodule_inline_diff(FILE *f, const char *path,\n+extern void show_submodule_inline_diff(FILE *f, const char *path,\n \t\tconst char *line_prefix,\n \t\tstruct object_id *one, struct object_id *two,\n \t\tunsigned dirty_submodule, const char *meta,\n \t\tconst char *del, const char *add, const char *reset,\n \t\tconst struct diff_options *opt);\n-void set_config_fetch_recurse_submodules(int value);\n-void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n-int fetch_populated_submodules(const struct argv_array *options,\n+extern void set_config_fetch_recurse_submodules(int value);\n+extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n+extern int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n \t\t\t int quiet, int max_parallel_jobs);\n-unsigned is_submodule_modified(const char *path, int ignore_untracked);\n-int submodule_uses_gitfile(const char *path);\n-int ok_to_remove_submodule(const char *path);\n-int merge_submodule(unsigned char result[20], const char *path, const unsigned char base[20],\n-\t\t const unsigned char a[20], const unsigned char b[20], int search);\n-int find_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name,\n-\t\tstruct string_list *needs_pushing);\n-int push_unpushed_submodules(unsigned char new_sha1[20], const char *remotes_name);\n-void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n-int parallel_submodules(void);\n+extern unsigned is_submodule_modified(const char *path, int ignore_untracked);\n+extern int submodule_uses_gitfile(const char *path);\n+extern int ok_to_remove_submodule(const char *path);\n+extern int merge_submodule(unsigned char result[20], const char *path,\n+\t\t\t const unsigned char base[20],\n+\t\t\t const unsigned char a[20],\n+\t\t\t const unsigned char b[20], int search);\n+extern int find_unpushed_submodules(unsigned char new_sha1[20],\n+\t\t\t\t const char *remotes_name,\n+\t\t\t\t struct string_list *needs_pushing);\n+extern int push_unpushed_submodules(unsigned char new_sha1[20],\n+\t\t\t\t const char *remotes_name);\n+extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);\n+extern int parallel_submodules(void);\n \n /*\n * Prepare the \"env_array\" parameter of a \"struct child_process\" for executing\n * a submodule by clearing any repo-specific envirionment variables, but\n * retaining any config in the environment.\n */\n-void prepare_submodule_repo_env(struct argv_array *out);\n-\n+extern void prepare_submodule_repo_env(struct argv_array *out);\n #endif\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306848","messageId":"20161203003022.29797-15-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 14/17] submodule: teach unpack_trees() to update submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:19Z","receivedAt":"2016-12-03T00:31:31Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n entry.c | 7 +++---\n unpack-trees.c | 76 +++++++++++++++++++++++++++++++++++++++++++++-------------\n unpack-trees.h | 1 +\n 3 files changed, 65 insertions(+), 19 deletions(-)\n\ndiff --git a/entry.c b/entry.c\nindex a668025b8e..3ed885b886 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -267,7 +267,7 @@ int checkout_entry(struct cache_entry *ce,\n \n \tif (!check_path(path.buf, path.len, &st, state->base_dir_len)) {\n \t\tunsigned changed = ce_match_stat(ce, &st, CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE);\n-\t\tif (!changed)\n+\t\tif (!changed && (!S_ISDIR(st.st_mode) || !S_ISGITLINK(ce->ce_mode)))\n \t\t\treturn 0;\n \t\tif (!state->force) {\n \t\t\tif (!state->quiet)\n@@ -284,9 +284,10 @@ int checkout_entry(struct cache_entry *ce,\n \t\t * just do the right thing)\n \t\t */\n \t\tif (S_ISDIR(st.st_mode)) {\n-\t\t\t/* If it is a gitlink, leave it alone! */\n-\t\t\tif (S_ISGITLINK(ce->ce_mode))\n+\t\t\tif (S_ISGITLINK(ce->ce_mode)) {\n+\t\t\t\tschedule_submodule_for_update(ce, 1);\n \t\t\t\treturn 0;\n+\t\t\t}\n \t\t\tif (!state->force)\n \t\t\t\treturn error(\"%s is a directory\", path.buf);\n \t\t\tremove_subtree(&path);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex db03293347..8b0f6dfd1a 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -29,6 +29,9 @@ static const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {\n \t/* ERROR_NOT_UPTODATE_DIR */\n \t\"Updating '%s' would lose untracked files in it\",\n \n+\t/* ERROR_NOT_UPTODATE_SUBMODULE */\n+\t\"Updating submodule '%s' would lose modifications in it\",\n+\n \t/* ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN */\n \t\"Untracked working tree file '%s' would be overwritten by merge.\",\n \n@@ -81,6 +84,9 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,\n \tmsgs[ERROR_NOT_UPTODATE_DIR] =\n \t\t_(\"Updating the following directories would lose untracked files in it:\\n%s\");\n \n+\tmsgs[ERROR_NOT_UPTODATE_SUBMODULE] =\n+\t\t_(\"Updating the following submodules would lose modifications in them:\\n%s\");\n+\n \tif (!strcmp(cmd, \"checkout\"))\n \t\tmsg = advice_commit_before_merge\n \t\t ? _(\"The following untracked working tree files would be removed by checkout:\\n%%s\"\n@@ -1320,19 +1326,17 @@ static int verify_uptodate_1(const struct cache_entry *ce,\n \t\treturn 0;\n \n \tif (!lstat(ce->name, &st)) {\n-\t\tint flags = CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE;\n-\t\tunsigned changed = ie_match_stat(o->src_index, ce, &st, flags);\n-\t\tif (!changed)\n-\t\t\treturn 0;\n-\t\t/*\n-\t\t * NEEDSWORK: the current default policy is to allow\n-\t\t * submodule to be out of sync wrt the superproject\n-\t\t * index. This needs to be tightened later for\n-\t\t * submodules that are marked to be automatically\n-\t\t * checked out.\n-\t\t */\n-\t\tif (S_ISGITLINK(ce->ce_mode))\n-\t\t\treturn 0;\n+\t\tif (S_ISGITLINK(ce->ce_mode)) {\n+\t\t\tif (!submodule_is_interesting(ce->name))\n+\t\t\t\treturn 0;\n+\t\t\tif (ce_stage(ce) ? is_submodule_checkout_safe(ce->name, &ce->oid)\n+\t\t\t : !is_submodule_modified(ce->name, 1))\n+\t\t\t\treturn 0;\n+\t\t} else {\n+\t\t\tint flags = CE_MATCH_IGNORE_VALID | CE_MATCH_IGNORE_SKIP_WORKTREE;\n+\t\t\tif (!ie_match_stat(o->src_index, ce, &st, flags))\n+\t\t\t\treturn 0;\n+\t\t}\n \t\terrno = 0;\n \t}\n \tif (errno == ENOENT)\n@@ -1355,6 +1359,38 @@ static int verify_uptodate_sparse(const struct cache_entry *ce,\n \treturn verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE);\n }\n \n+/*\n+ * When a submodule gets turned into an unmerged entry, we want it to be\n+ * up-to-date regarding the merge changes.\n+ */\n+static int verify_uptodate_submodule(const struct cache_entry *old,\n+\t\t\t\t const struct cache_entry *new,\n+\t\t\t\t struct unpack_trees_options *o)\n+{\n+\tstruct stat st;\n+\n+\tif (o->index_only ||\n+\t (!((old->ce_flags & CE_VALID) || ce_skip_worktree(old)) &&\n+\t (o->reset || ce_uptodate(old))))\n+\t\treturn 0;\n+\n+\tif (!submodule_is_interesting(new->name))\n+\t\treturn 0;\n+\n+\tif (lstat(old->name, &st)) {\n+\t\tif (errno == ENOENT)\n+\t\t\treturn 0;\n+\t\treturn o->gently ? -1 :\n+\t\t\tadd_rejected_path(o, ERROR_NOT_UPTODATE_SUBMODULE,\n+\t\t\t\t\t old->name);\n+\t}\n+\n+\tif (S_ISGITLINK(new->ce_mode))\n+\t\treturn !is_submodule_checkout_safe(new->name, &new->oid);\n+\telse\n+\t\treturn !ok_to_remove_submodule(old->name);\n+}\n+\n static void invalidate_ce_path(const struct cache_entry *ce,\n \t\t\t struct unpack_trees_options *o)\n {\n@@ -1616,9 +1652,17 @@ static int merged_entry(const struct cache_entry *ce,\n \t\t\tcopy_cache_entry(merge, old);\n \t\t\tupdate = 0;\n \t\t} else {\n-\t\t\tif (verify_uptodate(old, o)) {\n-\t\t\t\tfree(merge);\n-\t\t\t\treturn -1;\n+\t\t\tif (S_ISGITLINK(old->ce_mode) ||\n+\t\t\t S_ISGITLINK(merge->ce_mode)) {\n+\t\t\t\tif (verify_uptodate_submodule(old, merge, o)) {\n+\t\t\t\t\tfree(merge);\n+\t\t\t\t\treturn -1;\n+\t\t\t\t}\n+\t\t\t} else {\n+\t\t\t\tif (verify_uptodate(old, o)) {\n+\t\t\t\t\tfree(merge);\n+\t\t\t\t\treturn -1;\n+\t\t\t\t}\n \t\t\t}\n \t\t\t/* Migrate old flags over */\n \t\t\tupdate |= old->ce_flags & (CE_SKIP_WORKTREE | CE_NEW_SKIP_WORKTREE);\ndiff --git a/unpack-trees.h b/unpack-trees.h\nindex 36a73a6d00..bee874088a 100644\n--- a/unpack-trees.h\n+++ b/unpack-trees.h\n@@ -15,6 +15,7 @@ enum unpack_trees_error_types {\n \tERROR_WOULD_OVERWRITE = 0,\n \tERROR_NOT_UPTODATE_FILE,\n \tERROR_NOT_UPTODATE_DIR,\n+\tERROR_NOT_UPTODATE_SUBMODULE,\n \tERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN,\n \tERROR_WOULD_LOSE_UNTRACKED_REMOVED,\n \tERROR_BIND_OVERLAP,\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306849","messageId":"20161203003022.29797-7-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 06/17] update submodules: add a config option to determine if submodules are updated","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:11Z","receivedAt":"2016-12-03T00:31:34Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In later patches we introduce the options and flag for commands\nthat modify the working directory, e.g. git-checkout.\n\nHave a central place to store such settings whether we want to update\na submodule.\n\nSigned-off-by: Stefan Beller \n---\n submodule.c | 6 ++++++\n submodule.h | 1 +\n 2 files changed, 7 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex c29153e9ff..1ba398ba3b 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -16,6 +16,7 @@\n #include \"quote.h\"\n \n static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;\n+static int config_update_recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n static int parallel_jobs = 1;\n static struct string_list changed_submodule_paths = STRING_LIST_INIT_NODUP;\n static int initialized_fetch_ref_tips;\n@@ -510,6 +511,11 @@ void set_config_fetch_recurse_submodules(int value)\n \tconfig_fetch_recurse_submodules = value;\n }\n \n+void set_config_update_recurse_submodules(int value)\n+{\n+\tconfig_update_recurse_submodules = value;\n+}\n+\n static int has_remote(const char *refname, const struct object_id *oid,\n \t\t int flags, void *cb_data)\n {\ndiff --git a/submodule.h b/submodule.h\nindex a9eabcc3d0..21236b095c 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -53,6 +53,7 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,\n \t\tconst char *del, const char *add, const char *reset,\n \t\tconst struct diff_options *opt);\n extern void set_config_fetch_recurse_submodules(int value);\n+extern void set_config_update_recurse_submodules(int value);\n extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n extern int fetch_populated_submodules(const struct argv_array *options,\n \t\t\t const char *prefix, int command_line_option,\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306850","messageId":"20161203003022.29797-14-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 13/17] entry: write_entry to write populate submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:18Z","receivedAt":"2016-12-03T00:31:37Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller \n---\n entry.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/entry.c b/entry.c\nindex 02c4ac9f22..a668025b8e 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -2,6 +2,7 @@\n #include \"blob.h\"\n #include \"dir.h\"\n #include \"streaming.h\"\n+#include \"submodule.h\"\n \n static void create_directories(const char *path, int path_len,\n \t\t\t const struct checkout *state)\n@@ -208,6 +209,7 @@ static int write_entry(struct cache_entry *ce,\n \t\t\treturn error(\"cannot create temporary submodule %s\", path);\n \t\tif (mkdir(path, 0777) < 0)\n \t\t\treturn error(\"cannot create submodule directory %s\", path);\n+\t\tschedule_submodule_for_update(ce, 1);\n \t\tbreak;\n \tdefault:\n \t\treturn error(\"unknown file mode for %s in index\", path);\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306851","messageId":"20161203003022.29797-12-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 11/17] unpack-trees: teach verify_clean_submodule to inspect submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:16Z","receivedAt":"2016-12-03T00:31:40Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"As a later patch will modify submodules, if they are interesting\nwe need to see if the submodule is clean in case they are\ninteresting.\n\nIf they are not interesting, then we do not care about the submodule\nkeeping historic behavior.\n\nSigned-off-by: Stefan Beller \n---\n unpack-trees.c | 9 +++++----\n 1 file changed, 5 insertions(+), 4 deletions(-)\n\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex ea6bdd20e0..22e32eca96 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -9,6 +9,7 @@\n #include \"refs.h\"\n #include \"attr.h\"\n #include \"split-index.h\"\n+#include \"submodule.h\"\n #include \"dir.h\"\n \n /*\n@@ -1361,15 +1362,15 @@ static void invalidate_ce_path(const struct cache_entry *ce,\n /*\n * Check that checking out ce->sha1 in subdir ce->name is not\n * going to overwrite any working files.\n- *\n- * Currently, git does not checkout subprojects during a superproject\n- * checkout, so it is not going to overwrite anything.\n */\n static int verify_clean_submodule(const struct cache_entry *ce,\n \t\t\t\t enum unpack_trees_error_types error_type,\n \t\t\t\t struct unpack_trees_options *o)\n {\n-\treturn 0;\n+\tif (!submodule_is_interesting(ce->name))\n+\t\treturn 0;\n+\n+\treturn !is_submodule_modified(ce->name, 0);\n }\n \n static int verify_clean_subdirectory(const struct cache_entry *ce,\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306852","messageId":"20161203003022.29797-18-sbeller@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-1-sbeller@google.com","subject":"[RFC PATCHv2 17/17] checkout: add config option to recurse into submodules by default","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-03T00:30:22Z","receivedAt":"2016-12-03T00:31:43Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"To make it easier for the user, who doesn't want to give the\n`--recurse-submodules` option whenever they run checkout, have an\noption for to set the default behavior for checkout to recurse into\nsubmodules.\n\nSigned-off-by: Stefan Beller \n---\n Documentation/config.txt | 6 ++++++\n Documentation/git-checkout.txt | 5 +++--\n submodule.c | 6 +++---\n t/t2013-checkout-submodule.sh | 16 ++++++++++++++++\n 4 files changed, 28 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex a0ab66aae7..67e0714358 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -949,6 +949,12 @@ clean.requireForce::\n \tA boolean to make git-clean do nothing unless given -f,\n \t-i or -n. Defaults to true.\n \n+checkout.recurseSubmodules::\n+\tThis option can be set to a boolean value.\n+\tWhen set to true checkout will recurse into submodules and\n+\tupdate them. When set to false, which is the default, checkout\n+\twill leave submodules untouched.\n+\n color.branch::\n \tA boolean to enable/disable color in the output of\n \tlinkgit:git-branch[1]. May be set to `always`,\ndiff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt\nindex a0ea2c5651..819c430b6e 100644\n--- a/Documentation/git-checkout.txt\n+++ b/Documentation/git-checkout.txt\n@@ -260,8 +260,9 @@ section of linkgit:git-add[1] to learn how to operate the `--patch` mode.\n \tUsing --recurse-submodules will update the content of all initialized\n \tsubmodules according to the commit recorded in the superproject. If\n \tlocal modifications in a submodule would be overwritten the checkout\n-\twill fail until `-f` is used. If nothing (or --no-recurse-submodules)\n-\tis used, the work trees of submodules will not be updated.\n+\twill fail until `-f` is used. If `--no-recurse-submodules` is used,\n+\tthe work trees of submodules will not be updated. If no command line\n+\targument is given, `checkout.recurseSubmodules` is used as a default.\n \n ::\n \tBranch to checkout; if it refers to a branch (i.e., a name that,\ndiff --git a/submodule.c b/submodule.c\nindex 4253f7f044..e4e326bce4 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -160,10 +160,10 @@ int submodule_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t} else if (starts_with(var, \"submodule.\"))\n \t\treturn parse_submodule_config_option(var, value);\n-\telse if (!strcmp(var, \"fetch.recursesubmodules\")) {\n+\telse if (!strcmp(var, \"fetch.recursesubmodules\"))\n \t\tconfig_fetch_recurse_submodules = parse_fetch_recurse_submodules_arg(var, value);\n-\t\treturn 0;\n-\t}\n+\telse if (!strcmp(var, \"checkout.recursesubmodules\"))\n+\t\tconfig_update_recurse_submodules = parse_update_recurse_submodules_arg(var, value);\n \treturn 0;\n }\n \ndiff --git a/t/t2013-checkout-submodule.sh b/t/t2013-checkout-submodule.sh\nindex 33fb2f5de3..673021fb80 100755\n--- a/t/t2013-checkout-submodule.sh\n+++ b/t/t2013-checkout-submodule.sh\n@@ -137,6 +137,22 @@ test_expect_success '\"checkout --recurse-submodules\" repopulates submodule' '\n \tsubmodule_creation_must_succeed delete_submodule base\n '\n \n+test_expect_success 'option checkout.recurseSubmodules updates submodule' '\n+\ttest_config checkout.recurseSubmodules 1 &&\n+\tgit checkout base &&\n+\tgit checkout -b advanced-base &&\n+\tgit -C submodule commit --allow-empty -m \"empty commit\" &&\n+\tgit add submodule &&\n+\tgit commit -m \"advance submodule\" &&\n+\tgit checkout base &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached base &&\n+\tgit checkout advanced-base &&\n+\tgit diff-files --quiet &&\n+\tgit diff-index --quiet --cached advanced-base &&\n+\tgit checkout --recurse-submodules base\n+'\n+\n test_expect_success '\"checkout --recurse-submodules\" repopulates submodule in existing directory' '\n \tgit checkout --recurse-submodules delete_submodule &&\n \tmkdir submodule &&\n-- \n2.11.0.rc2.28.g2673dad\n\n"},{"id":"306947","messageId":"20161205192527.GA68588@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-16-sbeller@google.com","subject":"Re: [RFC PATCHv2 15/17] checkout: recurse into submodules if asked to","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2016-12-05T19:25:27Z","receivedAt":"2016-12-05T19:25:36Z","isPatch":false,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 12/02, Stefan Beller wrote:\n> \n> test_expect_success '\"checkout \" honors diff.ignoreSubmodules' '\n> @@ -63,6 +70,260 @@ test_expect_success '\"checkout \" honors submodule.*.ignore from .git/\n> \t! test -s actual\n> '\n\nShould you use test_must_fail and not '!'?\n\n-- \nBrandon Williams\n"},{"id":"306948","messageId":"20161205192918.GB68588@google.com","threadId":"44604","inReplyTo":"20161203003022.29797-18-sbeller@google.com","subject":"Re: [RFC PATCHv2 17/17] checkout: add config option to recurse into submodules by default","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2016-12-05T19:29:18Z","receivedAt":"2016-12-05T19:29:29Z","isPatch":false,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 12/02, Stefan Beller wrote:\n> +test_expect_success 'option checkout.recurseSubmodules updates submodule' '\n> +\ttest_config checkout.recurseSubmodules 1 &&\n> +\tgit checkout base &&\n> +\tgit checkout -b advanced-base &&\n> +\tgit -C submodule commit --allow-empty -m \"empty commit\" &&\n> +\tgit add submodule &&\n> +\tgit commit -m \"advance submodule\" &&\n> +\tgit checkout base &&\n> +\tgit diff-files --quiet &&\n> +\tgit diff-index --quiet --cached base &&\n> +\tgit checkout advanced-base &&\n> +\tgit diff-files --quiet &&\n> +\tgit diff-index --quiet --cached advanced-base &&\n> +\tgit checkout --recurse-submodules base\n> +'\n> +\n\nThis test doesn't look like it looks into the submodule to see if the\nsubmodule has indeed changed. Unless diff-index and diff-files recurse\ninto the submodules?\n-- \nBrandon Williams\n"},{"id":"306949","messageId":"CAGZ79kbEtR7_6ZvBsjkc=8q+nnq9FoPv9HNWdRyuR2CQGqQ2oA@mail.gmail.com","threadId":"44604","inReplyTo":"20161205192527.GA68588@google.com","subject":"Re: [RFC PATCHv2 15/17] checkout: recurse into submodules if asked to","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-05T19:30:57Z","receivedAt":"2016-12-05T19:31:26Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Dec 5, 2016 at 11:25 AM, Brandon Williams wrote:\n> On 12/02, Stefan Beller wrote:\n>>\n>> test_expect_success '\"checkout \" honors diff.ignoreSubmodules' '\n>> @@ -63,6 +70,260 @@ test_expect_success '\"checkout \" honors submodule.*.ignore from .git/\n>> ! test -s actual\n>> '\n>\n> Should you use test_must_fail and not '!'?\n\nWe use test_must_fail for git and '!' for non git thigns (test, grep etc),\nas the test suite is about testing git.\n\nThe test_must_fail expects the command to be run to\n* not reurn 0 (success)\n* not segfault\n* not return some other arbitrary return codes\n indicating abnormal failure (125 IIRC)\n\nSo in a way test_must_fail translates to:\n\"I want to run this git command and it should fail\ngracefully because at this state, it is the best git can do\"\n\nThe '!' however is just inverting the boolean expression.\nWe assume test, grep, et al. to be flawless here. ;)\n\nStefan\n"},{"id":"306950","messageId":"20161205193151.GC68588@google.com","threadId":"44604","inReplyTo":"CAGZ79kbEtR7_6ZvBsjkc=8q+nnq9FoPv9HNWdRyuR2CQGqQ2oA@mail.gmail.com","subject":"Re: [RFC PATCHv2 15/17] checkout: recurse into submodules if asked to","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2016-12-05T19:31:51Z","receivedAt":"2016-12-05T19:31:58Z","isPatch":false,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 12/05, Stefan Beller wrote:\n> On Mon, Dec 5, 2016 at 11:25 AM, Brandon Williams wrote:\n> > On 12/02, Stefan Beller wrote:\n> >>\n> >> test_expect_success '\"checkout \" honors diff.ignoreSubmodules' '\n> >> @@ -63,6 +70,260 @@ test_expect_success '\"checkout \" honors submodule.*.ignore from .git/\n> >> ! test -s actual\n> >> '\n> >\n> > Should you use test_must_fail and not '!'?\n> \n> We use test_must_fail for git and '!' for non git thigns (test, grep etc),\n> as the test suite is about testing git.\n> \n> The test_must_fail expects the command to be run to\n> * not reurn 0 (success)\n> * not segfault\n> * not return some other arbitrary return codes\n> indicating abnormal failure (125 IIRC)\n> \n> So in a way test_must_fail translates to:\n> \"I want to run this git command and it should fail\n> gracefully because at this state, it is the best git can do\"\n> \n> The '!' however is just inverting the boolean expression.\n> We assume test, grep, et al. to be flawless here. ;)\n\nAh, alright. Thanks for the info :)\n\n-- \nBrandon Williams\n"},{"id":"306971","messageId":"CAGZ79kaMXmNUaUCZJaTrcodAoSBPSxXr5JPrZGQeg=m-HrN11w@mail.gmail.com","threadId":"44604","inReplyTo":"20161205192918.GB68588@google.com","subject":"Re: [RFC PATCHv2 17/17] checkout: add config option to recurse into submodules by default","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-05T22:23:25Z","receivedAt":"2016-12-05T22:23:31Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Dec 5, 2016 at 11:29 AM, Brandon Williams wrote:\n> On 12/02, Stefan Beller wrote:\n>> +test_expect_success 'option checkout.recurseSubmodules updates submodule' '\n>> + test_config checkout.recurseSubmodules 1 &&\n>> + git checkout base &&\n>> + git checkout -b advanced-base &&\n>> + git -C submodule commit --allow-empty -m \"empty commit\" &&\n>> + git add submodule &&\n>> + git commit -m \"advance submodule\" &&\n>> + git checkout base &&\n>> + git diff-files --quiet &&\n>> + git diff-index --quiet --cached base &&\n>> + git checkout advanced-base &&\n>> + git diff-files --quiet &&\n>> + git diff-index --quiet --cached advanced-base &&\n>> + git checkout --recurse-submodules base\n>> +'\n>> +\n>\n> This test doesn't look like it looks into the submodule to see if the\n> submodule has indeed changed. Unless diff-index and diff-files recurse\n> into the submodules?\n\nI took the code from Jens once upon a time. Rereading the code, I agree it is\nnot obvious how this checks the submodule state.\n\nHowever `git diff-files --quiet` is perfectly fine, as\nwe have submodule support by default via:\n\n Omitting the --submodule option or specifying --submodule=short,\n uses the short format. This format just shows the names of the commits\n at the beginning and end of the range.\n\nand then we turn it into an exit code via\n\n --quiet\n Disable all output of the program. Implies --exit-code.\n\n --exit-code\n Make the program exit with codes similar to diff(1).\n That is, it exits with 1 if there were differences and 0 means no differences.\n\nSame for diff-index.\n\nThe main purpose of this specific test is to have checkout.recurseSubmodules\n\"to just make it work\" without having to give --recurse-submodules manually.\nAll the other tests with the manual --recurse-submodules should test for\ncorrectness of the behavior within the submodule.\n\nSo maybe I'll need to rewrite submodule_creation_must_succeed() in the previous\npatch to be more obvious. (Well that already has some tests for\nfiles/directories\nin there, so it is a little more.)\n\nBut to be sure we can also add tests here that look more into the submodule.\nI am thinking of \"{new,old}_sub_sha1=$(git -C submodule rev-parse HEAD)\" and\ncomparing them?\n"},{"id":"306974","messageId":"20161205222644.GG68588@google.com","threadId":"44604","inReplyTo":"CAGZ79kaMXmNUaUCZJaTrcodAoSBPSxXr5JPrZGQeg=m-HrN11w@mail.gmail.com","subject":"Re: [RFC PATCHv2 17/17] checkout: add config option to recurse into submodules by default","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2016-12-05T22:26:44Z","receivedAt":"2016-12-05T22:26:58Z","isPatch":false,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 12/05, Stefan Beller wrote:\n> On Mon, Dec 5, 2016 at 11:29 AM, Brandon Williams wrote:\n> > On 12/02, Stefan Beller wrote:\n> >> +test_expect_success 'option checkout.recurseSubmodules updates submodule' '\n> >> + test_config checkout.recurseSubmodules 1 &&\n> >> + git checkout base &&\n> >> + git checkout -b advanced-base &&\n> >> + git -C submodule commit --allow-empty -m \"empty commit\" &&\n> >> + git add submodule &&\n> >> + git commit -m \"advance submodule\" &&\n> >> + git checkout base &&\n> >> + git diff-files --quiet &&\n> >> + git diff-index --quiet --cached base &&\n> >> + git checkout advanced-base &&\n> >> + git diff-files --quiet &&\n> >> + git diff-index --quiet --cached advanced-base &&\n> >> + git checkout --recurse-submodules base\n> >> +'\n> >> +\n> >\n> > This test doesn't look like it looks into the submodule to see if the\n> > submodule has indeed changed. Unless diff-index and diff-files recurse\n> > into the submodules?\n> \n> I took the code from Jens once upon a time. Rereading the code, I agree it is\n> not obvious how this checks the submodule state.\n> \n> However `git diff-files --quiet` is perfectly fine, as\n> we have submodule support by default via:\n> \n> Omitting the --submodule option or specifying --submodule=short,\n> uses the short format. This format just shows the names of the commits\n> at the beginning and end of the range.\n> \n> and then we turn it into an exit code via\n> \n> --quiet\n> Disable all output of the program. Implies --exit-code.\n> \n> --exit-code\n> Make the program exit with codes similar to diff(1).\n> That is, it exits with 1 if there were differences and 0 means no differences.\n> \n> Same for diff-index.\n> \n> The main purpose of this specific test is to have checkout.recurseSubmodules\n> \"to just make it work\" without having to give --recurse-submodules manually.\n> All the other tests with the manual --recurse-submodules should test for\n> correctness of the behavior within the submodule.\n> \n> So maybe I'll need to rewrite submodule_creation_must_succeed() in the previous\n> patch to be more obvious. (Well that already has some tests for\n> files/directories\n> in there, so it is a little more.)\n> \n> But to be sure we can also add tests here that look more into the submodule.\n> I am thinking of \"{new,old}_sub_sha1=$(git -C submodule rev-parse HEAD)\" and\n> comparing them?\n\n\nAh ok, that makes sense now. Its kind of like if I run git status it\nwould show if a submodule is at a different sha1 than the superproject\nhas recorded.\n\n-- \nBrandon Williams\n"},{"id":"306981","messageId":"5d42b3f7bd234d99bd626733594d9a63@exmbdft7.ad.twosigma.com","threadId":"44604","inReplyTo":"20161203003022.29797-16-sbeller@google.com","subject":"RE: [RFC PATCHv2 15/17] checkout: recurse into submodules if asked to","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2016-12-05T23:36:47Z","receivedAt":"2016-12-05T23:36:56Z","isPatch":false,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"> -----Original Message-----\n> From: Stefan Beller [mailto:sbeller@google.com]\n> Sent: Friday, December 02, 2016 7:30 PM\n> To: bmwill@google.com; David Turner\n> Cc: git@vger.kernel.org; sandals@crustytoothpaste.net; hvoigt@hvoigt.net;\n> gitster@pobox.com; Stefan Beller\n> Subject: [RFC PATCHv2 15/17] checkout: recurse into submodules if asked to\n> \n> Allow checkout to recurse into submodules via the command line option --\n> [no-]recurse-submodules.\n\nThis option probably needs to precede 9/17 \"update submodules: add scheduling to update submodules\", since that patch uses --recurse-submodules. \n"},{"id":"306983","messageId":"f19844d15ab4424b8c056cd13837d233@exmbdft7.ad.twosigma.com","threadId":"44604","inReplyTo":"20161203003022.29797-10-sbeller@google.com","subject":"RE: [RFC PATCHv2 09/17] update submodules: add scheduling to update submodules","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2016-12-05T23:37:14Z","receivedAt":"2016-12-05T23:37:20Z","isPatch":false,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"This patch confuses me -- see below.\n\n> -----Original Message-----\n> From: Stefan Beller [mailto:sbeller@google.com]\n> Sent: Friday, December 02, 2016 7:30 PM\n> To: bmwill@google.com; David Turner\n> Cc: git@vger.kernel.org; sandals@crustytoothpaste.net; hvoigt@hvoigt.net;\n> gitster@pobox.com; Stefan Beller\n> Subject: [RFC PATCHv2 09/17] update submodules: add scheduling to update\n> submodules\n[snip] \n> +static int update_submodule(const char *path, const struct object_id\n> *oid,\n> +\t\t\t int force, int is_new)\n> +{\n> +\tconst char *git_dir;\n> +\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\tconst struct submodule *sub = submodule_from_path(null_sha1, path);\n> +\n> +\tif (!sub || !sub->name)\n> +\t\treturn -1;\n> +\n> +\tgit_dir = resolve_gitdir(git_common_path(\"modules/%s\", sub->name));\n> +\n> +\tif (!git_dir)\n> +\t\treturn -1;\n> +\n> +\tif (is_new)\n> +\t\tconnect_work_tree_and_git_dir(path, git_dir);\n> +\n> +\t/* update index via `read-tree --reset sha1` */\n> +\targv_array_pushl(&cp.args, \"read-tree\",\n> +\t\t\t\t force ? \"--reset\" : \"-m\",\n> +\t\t\t\t \"-u\", sha1_to_hex(oid->hash), NULL);\n> +\tprepare_submodule_repo_env(&cp.env_array);\n> +\tcp.git_cmd = 1;\n> +\tcp.no_stdin = 1;\n> +\tcp.dir = path;\n> +\tif (run_command(&cp)) {\n> +\t\twarning(_(\"reading the index in submodule '%s' failed\"),\n> path);\n\nThe error is not (usually) in \"reading the index\" -- it's \"updating the index\" (or the working tree)\n\n> +\t\tchild_process_clear(&cp);\n> +\t\treturn -1;\n> +\t}\n> +\n> +\t/* write index to working dir */\n> +\tchild_process_clear(&cp);\n> +\tchild_process_init(&cp);\n> +\targv_array_pushl(&cp.args, \"checkout-index\", \"-a\", NULL);\n\nI'm confused -- doesn't read-tree -u already do this? And if not, shouldn't we back out the result of the read-tree, to leave the submodule as it was?\n\n> +\tcp.git_cmd = 1;\n> +\tcp.no_stdin = 1;\n> +\tcp.dir = path;\n> +\tif (force)\n> +\t\targv_array_push(&cp.args, \"-f\");\n> +\n> +\tif (run_command(&cp)) {\n> +\t\twarning(_(\"populating the working directory in submodule '%s'\n> failed\"), path);\n> +\t\tchild_process_clear(&cp);\n> +\t\treturn -1;\n> +\t}\n> +\n> +\t/* get the HEAD right */\n> +\tchild_process_clear(&cp);\n> +\tchild_process_init(&cp);\n> +\targv_array_pushl(&cp.args, \"checkout\", \"--recurse-submodules\",\n> NULL);\n\n\nWhy are we running checkout on the submodule when we've already done most of the checkout? The only thing left is to set HEAD and recurse, right? I must be missing something.\n\n"},{"id":"306984","messageId":"c43b64307a3e48188bc9479226058b16@exmbdft7.ad.twosigma.com","threadId":"44604","inReplyTo":"20161203003022.29797-15-sbeller@google.com","subject":"RE: [RFC PATCHv2 14/17] submodule: teach unpack_trees() to update submodules","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2016-12-05T23:37:32Z","receivedAt":"2016-12-05T23:37:38Z","isPatch":false,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"\n> -----Original Message-----\n> From: Stefan Beller [mailto:sbeller@google.com]\n> Sent: Friday, December 02, 2016 7:30 PM\n> To: bmwill@google.com; David Turner\n> Cc: git@vger.kernel.org; sandals@crustytoothpaste.net; hvoigt@hvoigt.net;\n> gitster@pobox.com; Stefan Beller\n> Subject: [RFC PATCHv2 14/17] submodule: teach unpack_trees() to update\n> submodules\n[snip]\n> \tif (!lstat(ce->name, &st)) {\n> -\t\tint flags =\n> CE_MATCH_IGNORE_VALID|CE_MATCH_IGNORE_SKIP_WORKTREE;\n> -\t\tunsigned changed = ie_match_stat(o->src_index, ce, &st,\n> flags);\n> -\t\tif (!changed)\n> -\t\t\treturn 0;\n> -\t\t/*\n> -\t\t * NEEDSWORK: the current default policy is to allow\n> -\t\t * submodule to be out of sync wrt the superproject\n> -\t\t * index. This needs to be tightened later for\n> -\t\t * submodules that are marked to be automatically\n> -\t\t * checked out.\n> -\t\t */\n> -\t\tif (S_ISGITLINK(ce->ce_mode))\n> -\t\t\treturn 0;\n> +\t\tif (S_ISGITLINK(ce->ce_mode)) {\n> +\t\t\tif (!submodule_is_interesting(ce->name))\n> +\t\t\t\treturn 0;\n> +\t\t\tif (ce_stage(ce) ? is_submodule_checkout_safe(ce->name,\n> &ce->oid)\n> +\t\t\t : !is_submodule_modified(ce->name, 1))\n\nThis logic is too convoluted. Do a nested if or something.\n\n> +\t\t\t\treturn 0;\n> +\t\t} else {\n> +\t\t\tint flags = CE_MATCH_IGNORE_VALID |\n> CE_MATCH_IGNORE_SKIP_WORKTREE;\n> +\t\t\tif (!ie_match_stat(o->src_index, ce, &st, flags))\n> +\t\t\t\treturn 0;\n\nNit: I liked the old temp var \"changed\" better -- it made it clear what\nie_match_stat is checking.\n\n> +\t\t}\n> \t\terrno = 0;\n> \t}\n> \tif (errno == ENOENT)\n> @@ -1355,6 +1359,38 @@ static int verify_uptodate_sparse(const struct\n> cache_entry *ce,\n> \treturn verify_uptodate_1(ce, o, ERROR_SPARSE_NOT_UPTODATE_FILE); }\n> \n> +/*\n> + * When a submodule gets turned into an unmerged entry, we want it to\n> +be\n> + * up-to-date regarding the merge changes.\n> + */\n> +static int verify_uptodate_submodule(const struct cache_entry *old,\n> +\t\t\t\t const struct cache_entry *new,\n> +\t\t\t\t struct unpack_trees_options *o) {\n> +\tstruct stat st;\n> +\n> +\tif (o->index_only ||\n> +\t (!((old->ce_flags & CE_VALID) || ce_skip_worktree(old)) &&\n> +\t (o->reset || ce_uptodate(old))))\n> +\t\treturn 0;\n> +\n> +\tif (!submodule_is_interesting(new->name))\n> +\t\treturn 0;\n> +\n> +\tif (lstat(old->name, &st)) {\n\nWe never actually use st here. Should we? If not, is this really the right error message? And should we use access() instead of lstat?\n\n> +\t\tif (errno == ENOENT)\n> +\t\t\treturn 0;\n> +\t\treturn o->gently ? -1 :\n> +\t\t\tadd_rejected_path(o, ERROR_NOT_UPTODATE_SUBMODULE,\n> +\t\t\t\t\t old->name);\n> +\t}\n> +\n\n"},{"id":"306985","messageId":"832dcc3eec0d4237a1e2766e8df690ee@exmbdft7.ad.twosigma.com","threadId":"44604","inReplyTo":"20161203003022.29797-9-sbeller@google.com","subject":"RE: [RFC PATCHv2 08/17] update submodules: add depopulate_submodule","fromName":"David Turner","fromEmail":"david.turner@twosigma.com","sentAt":"2016-12-05T23:37:36Z","receivedAt":"2016-12-05T23:37:42Z","isPatch":false,"sender":{"key":"david.turner@twosigma.com","avatar":null},"body":"> -----Original Message-----\n> From: Stefan Beller [mailto:sbeller@google.com]\n> Sent: Friday, December 02, 2016 7:30 PM\n> To: bmwill@google.com; David Turner\n> Cc: git@vger.kernel.org; sandals@crustytoothpaste.net; hvoigt@hvoigt.net;\n> gitster@pobox.com; Stefan Beller\n> Subject: [RFC PATCHv2 08/17] update submodules: add depopulate_submodule\n> \n> Implement the functionality needed to enable work tree manipulating\n> commands so that a deleted submodule should not only affect the index\n> (leaving all the files of the submodule in the work tree) but also to\n> remove the work tree of the superproject (including any untracked files).\n\n\"including any untracked files\" bothers me, I think. Checkout is not usually willing to overwrite untracked files; it seems odd to me that it would be willing to do so in the submodule case. I would be OK if they were both untracked and gitignored, I think.\n\n> To do so, we need an equivalent of \"rm -rf\", which is already found in\n> entry.c, so expose that and for clarity add a suffix \"_or_dir\" to it.\n> \n> That will only work properly when the submodule uses a gitfile instead of\n> a .git directory and no untracked files are present. Otherwise the removal\n> will fail with a warning (which is just what happened until now).\n> \n> Signed-off-by: Stefan Beller \n> ---\n> cache.h | 2 ++\n> entry.c | 5 +++++\n> submodule.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++\n> submodule.h | 1 +\n> 4 files changed, 54 insertions(+)\n> \n> diff --git a/cache.h b/cache.h\n> index a50a61a197..b645ca2f9a 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -2018,4 +2018,6 @@ void sleep_millisec(int millisec);\n> */\n> void safe_create_dir(const char *dir, int share);\n> \n> +extern void remove_directory_or_die(struct strbuf *path);\n> +\n> #endif /* CACHE_H */\n> diff --git a/entry.c b/entry.c\n> index c6eea240b6..02c4ac9f22 100644\n> --- a/entry.c\n> +++ b/entry.c\n> @@ -73,6 +73,11 @@ static void remove_subtree(struct strbuf *path)\n> \t\tdie_errno(\"cannot rmdir '%s'\", path->buf); }\n> \n> +void remove_directory_or_die(struct strbuf *path) {\n> +\tremove_subtree(path);\n> +}\n> +\n> static int create_file(const char *path, unsigned int mode) {\n> \tmode = (mode & 0100) ? 0777 : 0666;\n> diff --git a/submodule.c b/submodule.c\n> index 62e9ef3872..7bb64d6c69 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -324,6 +324,52 @@ void prepare_submodule_repo_env(struct argv_array\n> *out)\n> \targv_array_push(out, \"GIT_DIR=.git\");\n> }\n> \n> +int depopulate_submodule(const char *path) {\n> +\tint ret = 0;\n> +\tstruct strbuf pathbuf = STRBUF_INIT;\n> +\tchar *dot_git = xstrfmt(\"%s/.git\", path);\n> +\n> +\t/* Is it populated? */\n> +\tif (!resolve_gitdir(dot_git))\n> +\t\tgoto out;\n> +\n> +\t/* Does it have a .git directory? */\n> +\tif (!submodule_uses_gitfile(path)) {\n> +\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n> +\n> +\t\tprepare_submodule_repo_env(&cp.env_array);\n> +\t\targv_array_pushl(&cp.args, \"submodule--helper\",\n> +\t\t\t\t \"embed-git-dirs\", path, NULL);\n> +\t\tcp.git_cmd = 1;\n> +\t\tif (run_command(&cp)) {\n> +\t\t\twarning(_(\"Cannot remove submodule '%s'\\n\"\n> +\t\t\t\t \"because it (or one of its nested submodules)\n> has a git \\n\"\n> +\t\t\t\t \"directory in the working tree, which could not\n> be embedded\\n\"\n> +\t\t\t\t \"the superprojects git directory\n> automatically.\"), path);\n\nWhat if instead it couldn't run the command because you're out of file descriptors or pids or memory or something?\n\nI think this message should be in submodule--helper --embed-git-dirs instead, and we should just pass it through here. Or, perhaps, instead of shelling out here, we should just call the functions directly?\n\n> +\t\t\tret = -1;\n> +\t\t\tgoto out;\n> +\t\t}\n> +\n> +\t\tif (!submodule_uses_gitfile(path)) {\n> +\t\t\t/*\n> +\t\t\t * We should be using a gitfile by now, let's double\n\nComma splice. \n\n> +\t\t\t * check as loosing the git dir would be fatal.\n\ns/loosing/losing/\n\n> +\t\t\t */\n> +\t\t\tdie(\"BUG: \\\"git submodule--helper embed git-dirs '%s'\\\"\n> \"\n> +\t\t\t \"did not embed the git-dirs recursively for '%s'\",\n> +\t\t\t path, path);\n> +\t\t}\n> +\t}\n> +\n> +\tstrbuf_addstr(&pathbuf, path);\n> +\tremove_directory_or_die(&pathbuf);\n> +out:\n> +\tstrbuf_release(&pathbuf);\n> +\tfree(dot_git);\n> +\treturn ret;\n> +}\n> +\n> /* Helper function to display the submodule header line prior to the full\n> * summary output. If it can locate the submodule objects directory it\n> will\n> * attempt to lookup both the left and right commits and put them into\n> the diff --git a/submodule.h b/submodule.h index 7d890e0464..d8bb1d4baf\n> 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -63,6 +63,7 @@ extern void set_config_update_recurse_submodules(int\n> value);\n> */\n> extern int submodule_is_interesting(const char *path); extern int\n> submodules_interesting_for_update(void);\n> +extern int depopulate_submodule(const char *path);\n> extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);\n> extern int fetch_populated_submodules(const struct argv_array *options,\n> \t\t\t const char *prefix, int command_line_option,\n> --\n> 2.11.0.rc2.28.g2673dad\n\n"},{"id":"306986","messageId":"CAGZ79kbbNHL8VQEztcHOea0yoawaitEvoFm6BWt-xi9Uq+8Qqw@mail.gmail.com","threadId":"44604","inReplyTo":"f19844d15ab4424b8c056cd13837d233@exmbdft7.ad.twosigma.com","subject":"Re: [RFC PATCHv2 09/17] update submodules: add scheduling to update submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-05T23:54:47Z","receivedAt":"2016-12-05T23:54:53Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Dec 5, 2016 at 3:37 PM, David Turner wrote:\n> This patch confuses me -- see below.\n>\n>> -----Original Message-----\n>> From: Stefan Beller [mailto:sbeller@google.com]\n>> Sent: Friday, December 02, 2016 7:30 PM\n>> To: bmwill@google.com; David Turner\n>> Cc: git@vger.kernel.org; sandals@crustytoothpaste.net; hvoigt@hvoigt.net;\n>> gitster@pobox.com; Stefan Beller\n>> Subject: [RFC PATCHv2 09/17] update submodules: add scheduling to update\n>> submodules\n> [snip]\n>> +static int update_submodule(const char *path, const struct object_id\n>> *oid,\n>> + int force, int is_new)\n>> +{\n>> + const char *git_dir;\n>> + struct child_process cp = CHILD_PROCESS_INIT;\n>> + const struct submodule *sub = submodule_from_path(null_sha1, path);\n>> +\n>> + if (!sub || !sub->name)\n>> + return -1;\n>> +\n>> + git_dir = resolve_gitdir(git_common_path(\"modules/%s\", sub->name));\n>> +\n>> + if (!git_dir)\n>> + return -1;\n>> +\n>> + if (is_new)\n>> + connect_work_tree_and_git_dir(path, git_dir);\n>> +\n>> + /* update index via `read-tree --reset sha1` */\n>> + argv_array_pushl(&cp.args, \"read-tree\",\n>> + force ? \"--reset\" : \"-m\",\n>> + \"-u\", sha1_to_hex(oid->hash), NULL);\n>> + prepare_submodule_repo_env(&cp.env_array);\n>> + cp.git_cmd = 1;\n>> + cp.no_stdin = 1;\n>> + cp.dir = path;\n>> + if (run_command(&cp)) {\n>> + warning(_(\"reading the index in submodule '%s' failed\"),\n>> path);\n>\n> The error is not (usually) in \"reading the index\" -- it's \"updating the index\" (or the working tree)\n>\n>> + child_process_clear(&cp);\n>> + return -1;\n>> + }\n>> +\n>> + /* write index to working dir */\n>> + child_process_clear(&cp);\n>> + child_process_init(&cp);\n>> + argv_array_pushl(&cp.args, \"checkout-index\", \"-a\", NULL);\n>\n> I'm confused -- doesn't read-tree -u already do this? And if not, shouldn't we back out the result of the read-tree, to leave the submodule as it was?\n>\n>> + cp.git_cmd = 1;\n>> + cp.no_stdin = 1;\n>> + cp.dir = path;\n>> + if (force)\n>> + argv_array_push(&cp.args, \"-f\");\n>> +\n>> + if (run_command(&cp)) {\n>> + warning(_(\"populating the working directory in submodule '%s'\n>> failed\"), path);\n>> + child_process_clear(&cp);\n>> + return -1;\n>> + }\n>> +\n>> + /* get the HEAD right */\n>> + child_process_clear(&cp);\n>> + child_process_init(&cp);\n>> + argv_array_pushl(&cp.args, \"checkout\", \"--recurse-submodules\",\n>> NULL);\n>\n>\n> Why are we running checkout on the submodule when we've already done most of the checkout? The only thing left is to set HEAD and recurse, right? I must be missing something.\n>\n\nYes this is only used to set the HEAD correctly and then recurse down.\nI tried to remove the first 2 calls to ch8ild processes at one point in time,\nwhich did not work out. I should have written in the commit message why\nthat was a problem. So I'll redo that just to see the problem and improve\nthe commit message.\n"},{"id":"306987","messageId":"CAGZ79kaXqOjT_Jd_Q=GkDegbFJjoEQoxHJ-wvvCTkjALndBwFg@mail.gmail.com","threadId":"44604","inReplyTo":"832dcc3eec0d4237a1e2766e8df690ee@exmbdft7.ad.twosigma.com","subject":"Re: [RFC PATCHv2 08/17] update submodules: add depopulate_submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-12-06T00:18:29Z","receivedAt":"2016-12-06T00:18:35Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Dec 5, 2016 at 3:37 PM, David Turner wrote:\n>> -----Original Message-----\n>> From: Stefan Beller [mailto:sbeller@google.com]\n>> Sent: Friday, December 02, 2016 7:30 PM\n>> To: bmwill@google.com; David Turner\n>> Cc: git@vger.kernel.org; sandals@crustytoothpaste.net; hvoigt@hvoigt.net;\n>> gitster@pobox.com; Stefan Beller\n>> Subject: [RFC PATCHv2 08/17] update submodules: add depopulate_submodule\n>>\n>> Implement the functionality needed to enable work tree manipulating\n>> commands so that a deleted submodule should not only affect the index\n>> (leaving all the files of the submodule in the work tree) but also to\n>> remove the work tree of the superproject (including any untracked files).\n>\n> \"including any untracked files\" bothers me, I think. Checkout is not usually willing to overwrite untracked files; it seems odd to me that it would be willing to do so in the submodule case. I would be OK if they were both untracked and gitignored, I think.\n\nI agree on being bothered, this is one of the things I thought how to solve.\nSee the test in \"checkout: recurse into submodules if asked to\", which\ntests for untracked files and is still marked as a failure.\n\nI think to address that issue, I'll add a flag to ok_to_remove_submodule\nwhich let's you specify which files you care about and which you don't.\n\n>> + warning(_(\"Cannot remove submodule '%s'\\n\"\n>> + \"because it (or one of its nested submodules)\n>> has a git \\n\"\n>> + \"directory in the working tree, which could not\n>> be embedded\\n\"\n>> + \"the superprojects git directory\n>> automatically.\"), path);\n>\n> What if instead it couldn't run the command because you're out of file descriptors or pids or memory or something?\n>\n> I think this message should be in submodule--helper --embed-git-dirs instead, and we should just pass it through here. Or, perhaps, instead of shelling out here, we should just call the functions directly?\n\nheh, good point. Will call the function directly.\n\n>\n"}]}