{"thread":{"id":"45736","subject":"[PATCHv2 0/4] recursive submodules: git-reset!","startedAt":"2017-04-18T21:37:44Z","lastAt":"2017-04-21T18:06:53Z","messageCount":10,"participants":["Stefan Beller","Junio C Hamano","Brandon Williams"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"317137","messageId":"20170418213725.7901-1-sbeller@google.com","threadId":"45736","inReplyTo":null,"subject":"[PATCHv2 0/4] recursive submodules: git-reset!","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-18T21:37:21Z","receivedAt":"2017-04-18T21:37:44Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"v2:\n* improved commit message to be proper English (Thanks, Philip!)\n* clarified why the patch 2 is so short (i.e. it doesn't matter if the submodule\n  is initialized in the preparation repo, we care about the actual testing repo!\n  Thanks, Brandon)\n* reworded patch 1 (Thanks Jonathan)\n\nThanks,\nStefan\n\nv1: https://public-inbox.org/git/20170411234923.1860-1-sbeller@google.com/\n\nNow that the BIG one has landed, e394fa01d6 (Merge branch\n'sb/checkout-recurse-submodules', 2017-03-28), you would expect that\nteaching to recurse into submodules is easy for all the remaining \nworking tree manipulations?\n\nIt turns out it is. See the last patch how we teach git-reset to recurse\ninto submodules.\n\nHowever when thinking more about what git-reset is expected to do,\nI added tests and some fixes for them (patch 2+3).\n\npatch 1 is a correctness thing, required for patch 3.\n\nThanks,\nStefan\n\nStefan Beller (4):\n  entry.c: submodule recursing: respect force flag correctly\n  submodule.c: uninitialized submodules are ignored in recursive\n    commands\n  submodule.c: submodule_move_head works with broken submodules\n  builtin/reset: add --recurse-submodules switch\n\n builtin/reset.c            | 30 ++++++++++++++++++++++++++++++\n entry.c                    |  8 ++++----\n submodule.c                | 31 +++++++++++++++++++++++++++----\n t/lib-submodule-update.sh  | 24 +++++++++++++++++++++---\n t/t7112-reset-submodule.sh |  8 ++++++++\n unpack-trees.c             |  7 ++++++-\n 6 files changed, 96 insertions(+), 12 deletions(-)\n\n\n"},{"id":"317138","messageId":"20170418213725.7901-2-sbeller@google.com","threadId":"45736","inReplyTo":"20170418213725.7901-1-sbeller@google.com","subject":"[PATCHv2 1/4] entry.c: submodule recursing: respect force flag correctly","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-18T21:37:22Z","receivedAt":"2017-04-18T21:37:46Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In case of a non-forced worktree update, the submodule movement is tested\nin a dry run first, such that it doesn't matter if the actual update is\ndone via the force flag. However for correctness, we want to give the\nflag as specified by the user. All callers have been inspected and updated\nif needed.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n entry.c        | 8 ++++----\n unpack-trees.c | 7 ++++++-\n 2 files changed, 10 insertions(+), 5 deletions(-)\n\ndiff --git a/entry.c b/entry.c\nindex d2b512da90..d6b263f78e 100644\n--- a/entry.c\n+++ b/entry.c\n@@ -208,7 +208,8 @@ static int write_entry(struct cache_entry *ce,\n \t\tsub = submodule_from_ce(ce);\n \t\tif (sub)\n \t\t\treturn submodule_move_head(ce->name,\n-\t\t\t\tNULL, oid_to_hex(&ce->oid), SUBMODULE_MOVE_HEAD_FORCE);\n+\t\t\t\tNULL, oid_to_hex(&ce->oid),\n+\t\t\t\tstate->force ? SUBMODULE_MOVE_HEAD_FORCE : 0);\n \t\tbreak;\n \tdefault:\n \t\treturn error(\"unknown file mode for %s in index\", path);\n@@ -282,12 +283,11 @@ int checkout_entry(struct cache_entry *ce,\n \t\t\t\t\tunlink_or_warn(ce->name);\n \n \t\t\t\treturn submodule_move_head(ce->name,\n-\t\t\t\t\tNULL, oid_to_hex(&ce->oid),\n-\t\t\t\t\tSUBMODULE_MOVE_HEAD_FORCE);\n+\t\t\t\t\tNULL, oid_to_hex(&ce->oid), 0);\n \t\t\t} else\n \t\t\t\treturn submodule_move_head(ce->name,\n \t\t\t\t\t\"HEAD\", oid_to_hex(&ce->oid),\n-\t\t\t\t\tSUBMODULE_MOVE_HEAD_FORCE);\n+\t\t\t\t\tstate->force ? SUBMODULE_MOVE_HEAD_FORCE : 0);\n \t\t}\n \n \t\tif (!changed)\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 6b7356dab2..4b3f9518e5 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -252,14 +252,18 @@ static int check_submodule_move_head(const struct cache_entry *ce,\n \t\t\t\t     const char *new_id,\n \t\t\t\t     struct unpack_trees_options *o)\n {\n+\tunsigned flags = SUBMODULE_MOVE_HEAD_DRY_RUN;\n \tconst struct submodule *sub = submodule_from_ce(ce);\n \tif (!sub)\n \t\treturn 0;\n \n+\tif (o->reset)\n+\t\tflags |= SUBMODULE_MOVE_HEAD_FORCE;\n+\n \tswitch (sub->update_strategy.type) {\n \tcase SM_UPDATE_UNSPECIFIED:\n \tcase SM_UPDATE_CHECKOUT:\n-\t\tif (submodule_move_head(ce->name, old_id, new_id, SUBMODULE_MOVE_HEAD_DRY_RUN))\n+\t\tif (submodule_move_head(ce->name, old_id, new_id, flags))\n \t\t\treturn o->gently ? -1 :\n \t\t\t\tadd_rejected_path(o, ERROR_WOULD_LOSE_SUBMODULE, ce->name);\n \t\treturn 0;\n@@ -308,6 +312,7 @@ static void unlink_entry(const struct cache_entry *ce)\n \t\tcase SM_UPDATE_CHECKOUT:\n \t\tcase SM_UPDATE_REBASE:\n \t\tcase SM_UPDATE_MERGE:\n+\t\t\t/* state.force is set at the caller. */\n \t\t\tsubmodule_move_head(ce->name, \"HEAD\", NULL,\n \t\t\t\t\t    SUBMODULE_MOVE_HEAD_FORCE);\n \t\t\tbreak;\n-- \n2.12.2.642.g1b8cc69eee.dirty\n\n"},{"id":"317139","messageId":"20170418213725.7901-3-sbeller@google.com","threadId":"45736","inReplyTo":"20170418213725.7901-1-sbeller@google.com","subject":"[PATCHv2 2/4] submodule.c: uninitialized submodules are ignored in recursive commands","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-18T21:37:23Z","receivedAt":"2017-04-18T21:37:49Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This was an oversight when working on the working tree modifying commands\nrecursing into submodules.\n\nTo test for uninitialized submodules, introduce another submodule\n\"uninitialized_sub\". Adding it via `submodule add` will activate the\nsubmodule in the preparation area (in create_lib_submodule_repo we\nsetup all the things in submodule_update_repo), but the later tests\nwill use a new testing repo that clones the preparation repo\nin which the new submodule is not initialized.\n\nBy adding it to the branch \"add_sub1\", which is the starting point of\nall other branches, we have wide coverage.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule.c               | 3 +++\n t/lib-submodule-update.sh | 1 +\n 2 files changed, 4 insertions(+)\n\ndiff --git a/submodule.c b/submodule.c\nindex 7c3c4b17fb..ccf8932731 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1333,6 +1333,9 @@ int submodule_move_head(const char *path,\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tconst struct submodule *sub;\n \n+\tif (!is_submodule_initialized(path))\n+\t\treturn 0;\n+\n \tsub = submodule_from_path(null_sha1, path);\n \n \tif (!sub)\ndiff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh\nindex fb4f7b014e..22dd9e060c 100755\n--- a/t/lib-submodule-update.sh\n+++ b/t/lib-submodule-update.sh\n@@ -73,6 +73,7 @@ create_lib_submodule_repo () {\n \n \t\tgit checkout -b \"add_sub1\" &&\n \t\tgit submodule add ../submodule_update_sub1 sub1 &&\n+\t\tgit submodule add ../submodule_update_sub1 uninitialized_sub &&\n \t\tgit config -f .gitmodules submodule.sub1.ignore all &&\n \t\tgit config submodule.sub1.ignore all &&\n \t\tgit add .gitmodules &&\n-- \n2.12.2.642.g1b8cc69eee.dirty\n\n"},{"id":"317140","messageId":"20170418213725.7901-4-sbeller@google.com","threadId":"45736","inReplyTo":"20170418213725.7901-1-sbeller@google.com","subject":"[PATCHv2 3/4] submodule.c: submodule_move_head works with broken submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-18T21:37:24Z","receivedAt":"2017-04-18T21:37:50Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Early on in submodule_move_head just after the check if the submodule is\ninitialized, we need to check if the submodule is populated correctly.\n\nIf the submodule is initialized but doesn't look like it is populated,\nthis is a red flag and can indicate multiple sorts of failures:\n(1) The submodule may be recorded at an object name, that is missing.\n(2) The submodule '.git' file link may be broken and it is not pointing\n    at a repository.\n\nIn both cases we want to complain to the user in the non-forced mode,\nand in the forced mode ignoring the old state and just moving the\nsubmodule into its new state with a fixed '.git' file link.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule.c               | 28 ++++++++++++++++++++++++----\n t/lib-submodule-update.sh | 23 ++++++++++++++++++++---\n 2 files changed, 44 insertions(+), 7 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex ccf8932731..20ed5b5681 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -1332,10 +1332,24 @@ int submodule_move_head(const char *path,\n \tint ret = 0;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tconst struct submodule *sub;\n+\tint *error_code_ptr, error_code;\n \n \tif (!is_submodule_initialized(path))\n \t\treturn 0;\n \n+\tif (flags & SUBMODULE_MOVE_HEAD_FORCE)\n+\t\t/*\n+\t\t * Pass non NULL pointer to is_submodule_populated_gently\n+\t\t * to prevent die()-ing. We'll use connect_work_tree_and_git_dir\n+\t\t * to fixup the submodule in the force case later.\n+\t\t */\n+\t\terror_code_ptr = &error_code;\n+\telse\n+\t\terror_code_ptr = NULL;\n+\n+\tif (old && !is_submodule_populated_gently(path, error_code_ptr))\n+\t\treturn 0;\n+\n \tsub = submodule_from_path(null_sha1, path);\n \n \tif (!sub)\n@@ -1353,15 +1367,21 @@ int submodule_move_head(const char *path,\n \t\t\t\tabsorb_git_dir_into_superproject(\"\", path,\n \t\t\t\t\tABSORB_GITDIR_RECURSE_SUBMODULES);\n \t\t} else {\n-\t\t\tstruct strbuf sb = STRBUF_INIT;\n-\t\t\tstrbuf_addf(&sb, \"%s/modules/%s\",\n+\t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n \t\t\t\t    get_git_common_dir(), sub->name);\n-\t\t\tconnect_work_tree_and_git_dir(path, sb.buf);\n-\t\t\tstrbuf_release(&sb);\n+\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n+\t\t\tfree(gitdir);\n \n \t\t\t/* make sure the index is clean as well */\n \t\t\tsubmodule_reset_index(path);\n \t\t}\n+\n+\t\tif (old && (flags & SUBMODULE_MOVE_HEAD_FORCE)) {\n+\t\t\tchar *gitdir = xstrfmt(\"%s/modules/%s\",\n+\t\t\t\t    get_git_common_dir(), sub->name);\n+\t\t\tconnect_work_tree_and_git_dir(path, gitdir);\n+\t\t\tfree(gitdir);\n+\t\t}\n \t}\n \n \tprepare_submodule_repo_env_no_git_dir(&cp.env_array);\ndiff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh\nindex 22dd9e060c..f0b1b18206 100755\n--- a/t/lib-submodule-update.sh\n+++ b/t/lib-submodule-update.sh\n@@ -1213,14 +1213,31 @@ test_submodule_forced_switch_recursing () {\n \t\t)\n \t'\n \t# Updating a submodule from an invalid sha1 updates\n-\ttest_expect_success \"$command: modified submodule does not update submodule work tree from invalid commit\" '\n+\ttest_expect_success \"$command: modified submodule does update submodule work tree from invalid commit\" '\n \t\tprolog &&\n \t\treset_work_tree_to_interested invalid_sub1 &&\n \t\t(\n \t\t\tcd submodule_update &&\n \t\t\tgit branch -t valid_sub1 origin/valid_sub1 &&\n-\t\t\ttest_must_fail $command valid_sub1 &&\n-\t\t\ttest_superproject_content origin/invalid_sub1\n+\t\t\t$command valid_sub1 &&\n+\t\t\ttest_superproject_content origin/valid_sub1 &&\n+\t\t\ttest_submodule_content sub1 origin/valid_sub1\n+\t\t)\n+\t'\n+\n+\t# Old versions of Git were buggy writing the .git link file\n+\t# (e.g. before f8eaa0ba98b and then moving the superproject repo\n+\t# whose submodules contained absolute paths)\n+\ttest_expect_success \"$command: updating submodules fixes .git links\" '\n+\t\tprolog &&\n+\t\treset_work_tree_to_interested add_sub1 &&\n+\t\t(\n+\t\t\tcd submodule_update &&\n+\t\t\tgit branch -t modify_sub1 origin/modify_sub1 &&\n+\t\t\techo \"gitdir: bogus/path\" >sub1/.git &&\n+\t\t\t$command modify_sub1 &&\n+\t\t\ttest_superproject_content origin/modify_sub1 &&\n+\t\t\ttest_submodule_content sub1 origin/modify_sub1\n \t\t)\n \t'\n }\n-- \n2.12.2.642.g1b8cc69eee.dirty\n\n"},{"id":"317141","messageId":"20170418213725.7901-5-sbeller@google.com","threadId":"45736","inReplyTo":"20170418213725.7901-1-sbeller@google.com","subject":"[PATCHv2 4/4] builtin/reset: add --recurse-submodules switch","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-18T21:37:25Z","receivedAt":"2017-04-18T21:37:56Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"git-reset is yet another working tree manipulator, which should\nbe taught about submodules.\n\nOne use case of \"git-reset\" is to reset to a known good state,\nand dropping commits that did not work as expected.\nIn that case one of the expected outcomes from a hard reset\nwould be to have broken submodules reset to a known good\nstate as well.  A test for this was added in a prior patch.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/reset.c            | 30 ++++++++++++++++++++++++++++++\n t/t7112-reset-submodule.sh |  8 ++++++++\n 2 files changed, 38 insertions(+)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fc3b906c47..5ce27fcaed 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -21,6 +21,27 @@\n #include \"parse-options.h\"\n #include \"unpack-trees.h\"\n #include \"cache-tree.h\"\n+#include \"submodule.h\"\n+#include \"submodule-config.h\"\n+\n+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n+\n+static int option_parse_recurse_submodules(const struct option *opt,\n+\t\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 static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]\"),\n@@ -283,6 +304,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\tN_(\"reset HEAD, index and working tree\"), MERGE),\n \t\tOPT_SET_INT(0, \"keep\", &reset_type,\n \t\t\t\tN_(\"reset HEAD but keep local changes\"), KEEP),\n+\t\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules,\n+\t\t\t    \"reset\", \"control recursive updating of submodules\",\n+\t\t\t    PARSE_OPT_OPTARG, option_parse_recurse_submodules },\n \t\tOPT_BOOL('p', \"patch\", &patch_mode, N_(\"select hunks interactively\")),\n \t\tOPT_BOOL('N', \"intent-to-add\", &intent_to_add,\n \t\t\t\tN_(\"record only the fact that removed paths will be added later\")),\n@@ -295,6 +319,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n \n+\tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {\n+\t\tgitmodules_config();\n+\t\tgit_config(submodule_config, NULL);\n+\t\tset_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);\n+\t}\n+\n \tunborn = !strcmp(rev, \"HEAD\") && get_sha1(\"HEAD\", oid.hash);\n \tif (unborn) {\n \t\t/* reset on unborn branch: treat as reset to empty tree */\ndiff --git a/t/t7112-reset-submodule.sh b/t/t7112-reset-submodule.sh\nindex 2eda6adeb1..f86ccdf215 100755\n--- a/t/t7112-reset-submodule.sh\n+++ b/t/t7112-reset-submodule.sh\n@@ -5,6 +5,14 @@ test_description='reset can handle submodules'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-submodule-update.sh\n \n+KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1\n+KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1\n+KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1\n+\n+test_submodule_switch_recursing \"git reset --recurse-submodules --keep\"\n+\n+test_submodule_forced_switch_recursing \"git reset --hard --recurse-submodules\"\n+\n test_submodule_switch \"git reset --keep\"\n \n test_submodule_switch \"git reset --merge\"\n-- \n2.12.2.642.g1b8cc69eee.dirty\n\n"},{"id":"317186","messageId":"xmqqfuh5av47.fsf@gitster.mtv.corp.google.com","threadId":"45736","inReplyTo":"20170418213725.7901-5-sbeller@google.com","subject":"Re: [PATCHv2 4/4] builtin/reset: add --recurse-submodules switch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-04-19T04:18:00Z","receivedAt":"2017-04-19T04:18:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> git-reset is yet another working tree manipulator, which should\n> be taught about submodules.\n>\n> One use case of \"git-reset\" is to reset to a known good state,\n> and dropping commits that did not work as expected.\n>\n> In that case one of the expected outcomes from a hard reset\n> would be to have broken submodules reset to a known good\n> state as well.  A test for this was added in a prior patch.\n\nWhen \"git reset --hard\" at the top-level superproject updates a\ngitlink in the index to a commit that was different from what was\nchecked out in the working tree of the submodule, what should\nhappen?  Do we reset the tip of the current branch in the submodule\nto point at the commit the index of the top-level records?  Do we\ndetach the HEAD in the submodule to point at the commit?  Something\nelse that is configurable?  Or do we just run \"git reset --hard\"\nin each submodule (which may leave submodule's HEAD different from\nwhat is recorded in the index of the superproject)?\n\n\"... to a known good state as well\" does not help answering the above.\n\n"},{"id":"317263","messageId":"CAGZ79kZ4oJcCD5QttcSzkfTVotust8vnBmSbp0sh5PrpqMQgJw@mail.gmail.com","threadId":"45736","inReplyTo":"xmqqfuh5av47.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv2 4/4] builtin/reset: add --recurse-submodules switch","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-19T19:08:24Z","receivedAt":"2017-04-19T19:08:31Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Apr 18, 2017 at 9:18 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> git-reset is yet another working tree manipulator, which should\n>> be taught about submodules.\n>>\n>> One use case of \"git-reset\" is to reset to a known good state,\n>> and dropping commits that did not work as expected.\n>>\n>> In that case one of the expected outcomes from a hard reset\n>> would be to have broken submodules reset to a known good\n>> state as well.  A test for this was added in a prior patch.\n>\n> When \"git reset --hard\" at the top-level superproject updates a\n> gitlink in the index to a commit that was different from what was\n> checked out in the working tree of the submodule, what should\n> happen?\n\nWe reset the submodule to the commit as recorded in the superproject,\ndetaching its HEAD.\n\n\n>  Do we reset the tip of the current branch in the submodule\n> to point at the commit the index of the top-level records?  Do we\n> detach the HEAD in the submodule to point at the commit?  Something\n> else that is configurable?  Or do we just run \"git reset --hard\"\n> in each submodule (which may leave submodule's HEAD different from\n> what is recorded in the index of the superproject)?\n>\n> \"... to a known good state as well\" does not help answering the above.\n\nI agree.\n\n>\n"},{"id":"317367","messageId":"20170420194130.GF142567@google.com","threadId":"45736","inReplyTo":"20170418213725.7901-1-sbeller@google.com","subject":"Re: [PATCHv2 0/4] recursive submodules: git-reset!","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2017-04-20T19:41:30Z","receivedAt":"2017-04-20T19:41:44Z","isPatch":false,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 04/18, Stefan Beller wrote:\n> v2:\n> * improved commit message to be proper English (Thanks, Philip!)\n> * clarified why the patch 2 is so short (i.e. it doesn't matter if the submodule\n>   is initialized in the preparation repo, we care about the actual testing repo!\n>   Thanks, Brandon)\n\nThat was the only thing I was unsure about in v1. v2 lgtm.\n\n> * reworded patch 1 (Thanks Jonathan)\n> \n> Thanks,\n> Stefan\n> \n> v1: https://public-inbox.org/git/20170411234923.1860-1-sbeller@google.com/\n> \n> Now that the BIG one has landed, e394fa01d6 (Merge branch\n> 'sb/checkout-recurse-submodules', 2017-03-28), you would expect that\n> teaching to recurse into submodules is easy for all the remaining \n> working tree manipulations?\n> \n> It turns out it is. See the last patch how we teach git-reset to recurse\n> into submodules.\n> \n> However when thinking more about what git-reset is expected to do,\n> I added tests and some fixes for them (patch 2+3).\n> \n> patch 1 is a correctness thing, required for patch 3.\n> \n> Thanks,\n> Stefan\n> \n> Stefan Beller (4):\n>   entry.c: submodule recursing: respect force flag correctly\n>   submodule.c: uninitialized submodules are ignored in recursive\n>     commands\n>   submodule.c: submodule_move_head works with broken submodules\n>   builtin/reset: add --recurse-submodules switch\n> \n>  builtin/reset.c            | 30 ++++++++++++++++++++++++++++++\n>  entry.c                    |  8 ++++----\n>  submodule.c                | 31 +++++++++++++++++++++++++++----\n>  t/lib-submodule-update.sh  | 24 +++++++++++++++++++++---\n>  t/t7112-reset-submodule.sh |  8 ++++++++\n>  unpack-trees.c             |  7 ++++++-\n>  6 files changed, 96 insertions(+), 12 deletions(-)\n> \n> \n\n-- \nBrandon Williams\n"},{"id":"317368","messageId":"CAGZ79kZxfAJDQ96QngOCSp1q6kepgcByv7zSkRn_aFL3CLcznQ@mail.gmail.com","threadId":"45736","inReplyTo":"20170420194130.GF142567@google.com","subject":"Re: [PATCHv2 0/4] recursive submodules: git-reset!","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-20T20:00:08Z","receivedAt":"2017-04-20T20:01:04Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Apr 20, 2017 at 12:41 PM, Brandon Williams <bmwill@google.com> wrote:\n> On 04/18, Stefan Beller wrote:\n>> v2:\n>> * improved commit message to be proper English (Thanks, Philip!)\n>> * clarified why the patch 2 is so short (i.e. it doesn't matter if the submodule\n>>   is initialized in the preparation repo, we care about the actual testing repo!\n>>   Thanks, Brandon)\n>\n> That was the only thing I was unsure about in v1. v2 lgtm.\n\nThanks for the review.\n\nWell the last commit still needs a better commit message as Junio\npointed out, so might just resend the last patch with better wording.\n"},{"id":"317495","messageId":"20170421173953.16433-1-sbeller@google.com","threadId":"45736","inReplyTo":"CAGZ79kZ4oJcCD5QttcSzkfTVotust8vnBmSbp0sh5PrpqMQgJw@mail.gmail.com","subject":"[PATCHv3 4/4] builtin/reset: add --recurse-submodules switch","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2017-04-21T17:39:53Z","receivedAt":"2017-04-21T18:06:53Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"git-reset is yet another working tree manipulator, which should\nbe taught about submodules.\n\nWhen a user uses git-reset and requests to recurse into submodules,\nthis will reset the submodules to the object name as recorded in the\nsuperproject, detaching the HEADs.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n  This replaces the topmost patch in sb/reset-recurse-submodules.\n  The only difference is the rewording of the commit message.\n  \n  Thanks,\n  Stefan\n\n builtin/reset.c            | 30 ++++++++++++++++++++++++++++++\n t/t7112-reset-submodule.sh |  8 ++++++++\n 2 files changed, 38 insertions(+)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex fc3b906c47..5ce27fcaed 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -21,6 +21,27 @@\n #include \"parse-options.h\"\n #include \"unpack-trees.h\"\n #include \"cache-tree.h\"\n+#include \"submodule.h\"\n+#include \"submodule-config.h\"\n+\n+static int recurse_submodules = RECURSE_SUBMODULES_DEFAULT;\n+\n+static int option_parse_recurse_submodules(const struct option *opt,\n+\t\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 static const char * const git_reset_usage[] = {\n \tN_(\"git reset [--mixed | --soft | --hard | --merge | --keep] [-q] [<commit>]\"),\n@@ -283,6 +304,9 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\tN_(\"reset HEAD, index and working tree\"), MERGE),\n \t\tOPT_SET_INT(0, \"keep\", &reset_type,\n \t\t\t\tN_(\"reset HEAD but keep local changes\"), KEEP),\n+\t\t{ OPTION_CALLBACK, 0, \"recurse-submodules\", &recurse_submodules,\n+\t\t\t    \"reset\", \"control recursive updating of submodules\",\n+\t\t\t    PARSE_OPT_OPTARG, option_parse_recurse_submodules },\n \t\tOPT_BOOL('p', \"patch\", &patch_mode, N_(\"select hunks interactively\")),\n \t\tOPT_BOOL('N', \"intent-to-add\", &intent_to_add,\n \t\t\t\tN_(\"record only the fact that removed paths will be added later\")),\n@@ -295,6 +319,12 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \tparse_args(&pathspec, argv, prefix, patch_mode, &rev);\n \n+\tif (recurse_submodules != RECURSE_SUBMODULES_DEFAULT) {\n+\t\tgitmodules_config();\n+\t\tgit_config(submodule_config, NULL);\n+\t\tset_config_update_recurse_submodules(RECURSE_SUBMODULES_ON);\n+\t}\n+\n \tunborn = !strcmp(rev, \"HEAD\") && get_sha1(\"HEAD\", oid.hash);\n \tif (unborn) {\n \t\t/* reset on unborn branch: treat as reset to empty tree */\ndiff --git a/t/t7112-reset-submodule.sh b/t/t7112-reset-submodule.sh\nindex 2eda6adeb1..f86ccdf215 100755\n--- a/t/t7112-reset-submodule.sh\n+++ b/t/t7112-reset-submodule.sh\n@@ -5,6 +5,14 @@ test_description='reset can handle submodules'\n . ./test-lib.sh\n . \"$TEST_DIRECTORY\"/lib-submodule-update.sh\n \n+KNOWN_FAILURE_SUBMODULE_RECURSIVE_NESTED=1\n+KNOWN_FAILURE_DIRECTORY_SUBMODULE_CONFLICTS=1\n+KNOWN_FAILURE_SUBMODULE_OVERWRITE_IGNORED_UNTRACKED=1\n+\n+test_submodule_switch_recursing \"git reset --recurse-submodules --keep\"\n+\n+test_submodule_forced_switch_recursing \"git reset --hard --recurse-submodules\"\n+\n test_submodule_switch \"git reset --keep\"\n \n test_submodule_switch \"git reset --merge\"\n-- \n2.13.0.rc0.2.g0d1ae48b0e\n\n"}]}