{"thread":{"id":"42228","subject":"[PATCHv5] submodule deinit: require '--all' instead of '.' for all submodules","startedAt":"2016-05-05T19:52:32Z","lastAt":"2016-05-05T20:54:41Z","messageCount":3,"participants":["Stefan Beller","Jonathan Nieder","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"285593","messageId":"1462477952-6669-1-git-send-email-sbeller@google.com","threadId":"42228","inReplyTo":null,"subject":"[PATCHv5] submodule deinit: require '--all' instead of '.' for all submodules","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-05-05T19:52:32Z","receivedAt":"2016-05-05T19:52:32Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The discussion in [1] pointed out that '.' is a faulty suggestion as\nthere is a corner case where it fails:\n\n> \"submodule deinit .\" may have \"worked\" in the sense that you would\n> have at least one path in your tree and avoided this \"nothing\n> matches\" most of the time.  It would have still failed with the\n> exactly same error if run in an empty repository, i.e.\n>\n>        $ E=/var/tmp/x/empty && rm -fr \"$E\" && mkdir -p \"$E\" && cd \"$E\"\n>        $ git init\n>        $ rungit v2.6.6 submodule deinit .\n>        error: pathspec '.' did not match any file(s) known to git.\n>        Did you forget to 'git add'?\n>        $ >file && git add file\n>        $ rungit v2.6.6 submodule deinit .\n>        $ echo $?\n>        0\n\nSo instead of a pathspec add the '--all' option to deinit all submodules\nand add a test to check for the corner case of an empty repository.\n\nThe code only needs to learn about the '--all' option and doesn't\nrequire further changes as `git submodule--helper list \"$@\"` will list\nall submodules when \"$@\" is empty.\n\n[1] http://news.gmane.org/gmane.comp.version-control.git/289535\n\nHelped-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n  * dropped the short -a option\n  * dropped the \"In version 2.8 and before..\" part of the documentation\n  * be concise and give the usage. This doesn't use the new \"die_with_usage\"\n    or \"error_with_usage\" though.\n  * fixed the test\n\n Documentation/git-submodule.txt | 19 +++++++++++++------\n git-submodule.sh                | 15 ++++++++++++---\n t/t7400-submodule-basic.sh      | 24 +++++++++++++++++++++++-\n 3 files changed, 48 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt\nindex 1572f05..ad85183 100644\n--- a/Documentation/git-submodule.txt\n+++ b/Documentation/git-submodule.txt\n@@ -13,7 +13,7 @@ SYNOPSIS\n \t      [--reference <repository>] [--depth <depth>] [--] <repository> [<path>]\n 'git submodule' [--quiet] status [--cached] [--recursive] [--] [<path>...]\n 'git submodule' [--quiet] init [--] [<path>...]\n-'git submodule' [--quiet] deinit [-f|--force] [--] <path>...\n+'git submodule' [--quiet] deinit [-f|--force] (--all|[--] <path>...)\n 'git submodule' [--quiet] update [--init] [--remote] [-N|--no-fetch]\n \t      [-f|--force] [--rebase|--merge] [--reference <repository>]\n \t      [--depth <depth>] [--recursive] [--] [<path>...]\n@@ -140,12 +140,15 @@ deinit::\n \ttree. Further calls to `git submodule update`, `git submodule foreach`\n \tand `git submodule sync` will skip any unregistered submodules until\n \tthey are initialized again, so use this command if you don't want to\n-\thave a local checkout of the submodule in your work tree anymore. If\n+\thave a local checkout of the submodule in your working tree anymore. If\n \tyou really want to remove a submodule from the repository and commit\n \tthat use linkgit:git-rm[1] instead.\n +\n-If `--force` is specified, the submodule's work tree will be removed even if\n-it contains local modifications.\n+When the command is run without pathspec, it errors out,\n+instead of deinit-ing everything, to prevent mistakes.\n++\n+If `--force` is specified, the submodule's working tree will\n+be removed even if it contains local modifications.\n \n update::\n +\n@@ -247,6 +250,10 @@ OPTIONS\n --quiet::\n \tOnly print error messages.\n \n+--all::\n+\tThis option is only valid for the deinit command. Unregister all\n+\tsubmodules in the working tree.\n+\n -b::\n --branch::\n \tBranch of repository to add as submodule.\n@@ -257,8 +264,8 @@ OPTIONS\n --force::\n \tThis option is only valid for add, deinit and update commands.\n \tWhen running add, allow adding an otherwise ignored submodule path.\n-\tWhen running deinit the submodule work trees will be removed even if\n-\tthey contain local changes.\n+\tWhen running deinit the submodule working trees will be removed even\n+\tif they contain local changes.\n \tWhen running update (only effective with the checkout procedure),\n \tthrow away local changes in submodules when switching to a\n \tdifferent commit; and always run a checkout operation in the\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 43c68de..5e9b5c0 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -8,7 +8,7 @@ dashless=$(basename \"$0\" | sed -e 's/-/ /')\n USAGE=\"[--quiet] add [-b <branch>] [-f|--force] [--name <name>] [--reference <repository>] [--] <repository> [<path>]\n    or: $dashless [--quiet] status [--cached] [--recursive] [--] [<path>...]\n    or: $dashless [--quiet] init [--] [<path>...]\n-   or: $dashless [--quiet] deinit [-f|--force] [--] <path>...\n+   or: $dashless [--quiet] deinit [-f|--force] (--all| [--] <path>...)\n    or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--checkout|--merge|--rebase] [--reference <repository>] [--recursive] [--] [<path>...]\n    or: $dashless [--quiet] summary [--cached|--files] [--summary-limit <n>] [commit] [--] [<path>...]\n    or: $dashless [--quiet] foreach [--recursive] <command>\n@@ -521,6 +521,7 @@ cmd_init()\n cmd_deinit()\n {\n \t# parse $args after \"submodule ... deinit\".\n+\tdeinit_all=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -530,6 +531,9 @@ cmd_deinit()\n \t\t-q|--quiet)\n \t\t\tGIT_QUIET=1\n \t\t\t;;\n+\t\t--all)\n+\t\t\tdeinit_all=t\n+\t\t\t;;\n \t\t--)\n \t\t\tshift\n \t\t\tbreak\n@@ -544,9 +548,14 @@ cmd_deinit()\n \t\tshift\n \tdone\n \n-\tif test $# = 0\n+\tif test -n \"$deinit_all\" && test \"$#\" -ne 0\n+\tthen\n+\t\techo >&2 \"$(eval_gettext \"pathspec and --all are incompatible\")\"\n+\t\tusage\n+\tfi\n+\tif test $# = 0 && test -z \"$deinit_all\"\n \tthen\n-\t\tdie \"$(eval_gettext \"Use '.' if you really want to deinitialize all submodules\")\"\n+\t\tdie \"$(eval_gettext \"Use '--all' if you really want to deinitialize all submodules\")\"\n \tfi\n \n \tgit submodule--helper list --prefix \"$wt_prefix\" \"$@\" |\ndiff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh\nindex e1abd19..cc3cca0 100755\n--- a/t/t7400-submodule-basic.sh\n+++ b/t/t7400-submodule-basic.sh\n@@ -11,6 +11,10 @@ subcommands of git submodule.\n \n . ./test-lib.sh\n \n+test_expect_success 'submodule deinit works on empty repository' '\n+\tgit submodule deinit --all\n+'\n+\n test_expect_success 'setup - initial commit' '\n \t>t &&\n \tgit add t &&\n@@ -858,7 +862,8 @@ test_expect_success 'submodule deinit works on repository without submodules' '\n \t\t>file &&\n \t\tgit add file &&\n \t\tgit commit -m \"repo should not be empty\"\n-\t\tgit submodule deinit .\n+\t\tgit submodule deinit . &&\n+\t\tgit submodule deinit --all\n \t)\n '\n \n@@ -900,6 +905,19 @@ test_expect_success 'submodule deinit . deinits all initialized submodules' '\n \trmdir init example2\n '\n \n+test_expect_success 'submodule deinit --all deinits all initialized submodules' '\n+\tgit submodule update --init &&\n+\tgit config submodule.example.foo bar &&\n+\tgit config submodule.example2.frotz nitfol &&\n+\ttest_must_fail git submodule deinit &&\n+\tgit submodule deinit --all >actual &&\n+\ttest -z \"$(git config --get-regexp \"submodule\\.example\\.\")\" &&\n+\ttest -z \"$(git config --get-regexp \"submodule\\.example2\\.\")\" &&\n+\ttest_i18ngrep \"Cleared directory .init\" actual &&\n+\ttest_i18ngrep \"Cleared directory .example2\" actual &&\n+\trmdir init example2\n+'\n+\n test_expect_success 'submodule deinit deinits a submodule when its work tree is missing or empty' '\n \tgit submodule update --init &&\n \trm -rf init example2/* example2/.git &&\n@@ -966,6 +984,10 @@ test_expect_success 'submodule deinit is silent when used on an uninitialized su\n \ttest_i18ngrep ! \"Submodule .example. (.*) unregistered for path .init\" actual &&\n \ttest_i18ngrep ! \"Submodule .example2. (.*) unregistered for path .example2\" actual &&\n \ttest_i18ngrep \"Cleared directory .init\" actual &&\n+\tgit submodule deinit --all >actual &&\n+\ttest_i18ngrep ! \"Submodule .example. (.*) unregistered for path .init\" actual &&\n+\ttest_i18ngrep ! \"Submodule .example2. (.*) unregistered for path .example2\" actual &&\n+\ttest_i18ngrep \"Cleared directory .init\" actual &&\n \trmdir init example2\n '\n \n-- \n2.8.0.1.g3af9c03\n"},{"id":"285599","messageId":"20160505203705.GB22726@google.com","threadId":"42228","inReplyTo":"1462477952-6669-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv5] submodule deinit: require '--all' instead of '.' for all submodules","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2016-05-05T20:37:05Z","receivedAt":"2016-05-05T20:37:05Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Stefan Beller wrote:\n\n>>        $ rungit v2.6.6 submodule deinit .\n>>        error: pathspec '.' did not match any file(s) known to git.\n>>        Did you forget to 'git add'?\n[...]\n> So instead of a pathspec add the '--all' option to deinit all submodules\n> and add a test to check for the corner case of an empty repository.\n[...]\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n\nReviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n"},{"id":"285601","messageId":"CAPc5daU5xLP9XghK=FfpubF96a-t+gqbg4YCBDdWc35Og8EUZA@mail.gmail.com","threadId":"42228","inReplyTo":"20160505203705.GB22726@google.com","subject":"Re: [PATCHv5] submodule deinit: require '--all' instead of '.' for all submodules","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-05T20:54:41Z","receivedAt":"2016-05-05T20:54:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Thu, May 5, 2016 at 1:37 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Stefan Beller wrote:\n>\n>>>        $ rungit v2.6.6 submodule deinit .\n>>>        error: pathspec '.' did not match any file(s) known to git.\n>>>        Did you forget to 'git add'?\n> [...]\n>> So instead of a pathspec add the '--all' option to deinit all submodules\n>> and add a test to check for the corner case of an empty repository.\n> [...]\n>> Signed-off-by: Stefan Beller <sbeller@google.com>\n>\n> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>\n\nThanks, both. Will queue.\n"}]}