{"thread":{"id":"34936","subject":"[PATCH 1/3] t7406-submodule-update: add missing &&","startedAt":"2013-09-15T17:38:21Z","lastAt":"2013-09-19T10:25:27Z","messageCount":7,"participants":["Tay Ray Chuan","Jens Lehmann","Chris Packham"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"227699","messageId":"1379266703-29808-1-git-send-email-rctay89@gmail.com","threadId":"34936","inReplyTo":null,"subject":"[PATCH 1/3] t7406-submodule-update: add missing &&","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2013-09-15T17:38:21Z","receivedAt":"2013-09-15T17:38:21Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"322bb6e (2011 Aug 11) introduced a new subshell at the end of a test\ncase but omitted a '&&' to join the two; fix this.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n t/t7406-submodule-update.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex b192f93..f0b3305 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -58,7 +58,7 @@ test_expect_success 'setup a submodule tree' '\n \t git submodule add ../merging merging &&\n \t test_tick &&\n \t git commit -m \"rebasing\"\n-\t)\n+\t) &&\n \t(cd super &&\n \t git submodule add ../none none &&\n \t test_tick &&\n-- \n1.8.4.rc4.527.g303b16c\n"},{"id":"227700","messageId":"1379266703-29808-2-git-send-email-rctay89@gmail.com","threadId":"34936","inReplyTo":"1379266703-29808-1-git-send-email-rctay89@gmail.com","subject":"[PATCH 2/3] t7406-submodule-update: demonstrate behaviour when run without init beforehand","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2013-09-15T17:38:22Z","receivedAt":"2013-09-15T17:38:22Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n t/t7406-submodule-update.sh | 20 ++++++++++++++++++++\n 1 file changed, 20 insertions(+)\n\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex f0b3305..00475eb 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -66,6 +66,26 @@ test_expect_success 'setup a submodule tree' '\n \t)\n '\n \n+test_expect_success 'setup a repo with uninitialized submodules' '\n+\tgit clone super super2\n+'\n+\n+test_expect_success 'submodule update <path> warns without init beforehand' '\n+\t(cd super2 &&\n+\t test -n \"$(git submodule update submodule)\"\n+\t)\n+'\n+\n+test_expect_success 'submodule update is silent without init beforehand' '\n+\t(cd super2 &&\n+\t test -z \"$(git submodule update)\"\n+\t)\n+'\n+\n+test_expect_success 'cleanup repo with uninitialized submodules' '\n+\trm -rf super2\n+'\n+\n test_expect_success 'submodule update detaching the HEAD ' '\n \t(cd super/submodule &&\n \t git reset --hard HEAD~1\n-- \n1.8.4.rc4.527.g303b16c\n"},{"id":"227701","messageId":"1379266703-29808-3-git-send-email-rctay89@gmail.com","threadId":"34936","inReplyTo":"1379266703-29808-2-git-send-email-rctay89@gmail.com","subject":"[PATCH 3/3] git submodule update should give notice when run without init beforehand","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2013-09-15T17:38:23Z","receivedAt":"2013-09-15T17:38:23Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"When 'update' is run with no path in a repository with uninitialized\nsubmodules, the program terminates with no output, and zero status code.\nBe more helpful to users by mentioning this.\n\nThis may be controlled by an advice.* option.\n\nSigned-off-by: Tay Ray Chuan <rctay89@gmail.com>\n---\n Documentation/config.txt    |  5 +++++\n git-submodule.sh            | 16 ++++++++++++++--\n t/t7406-submodule-update.sh |  5 ++++-\n 3 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex ec57a15..79313f9 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -202,6 +202,11 @@ advice.*::\n \trmHints::\n \t\tIn case of failure in the output of linkgit:git-rm[1],\n \t\tshow directions on how to proceed from the current state.\n+\tsubmoduleUpdateUninit::\n+\t\tWhen linkgit:git-submodule[1] `update` is run with no `path`\n+\t\targuments in a repository with uninitialized submodules,\n+\t\tmention that uninitalized submodules are indeed present, and\n+\t\tthat they may be initialized with the `--init` option.\n --\n \n core.fileMode::\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2979197..56f3dc2 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -777,6 +777,7 @@ cmd_update()\n \tcloned_modules=\n \tmodule_list \"$@\" | {\n \terr=\n+\thas_uninit=\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n@@ -807,9 +808,13 @@ cmd_update()\n \t\tthen\n \t\t\t# Only mention uninitialized submodules when its\n \t\t\t# path have been specified\n-\t\t\ttest \"$#\" != \"0\" &&\n-\t\t\tsay \"$(eval_gettext \"Submodule path '\\$displaypath' not initialized\n+\t\t\tif test \"$#\" != \"0\"\n+\t\t\tthen\n+\t\t\t\tsay \"$(eval_gettext \"Submodule path '\\$displaypath' not initialized\n Maybe you want to use 'update --init'?\")\"\n+\t\t\telse\n+\t\t\t\thas_uninit=1\n+\t\t\tfi\n \t\t\tcontinue\n \t\tfi\n \n@@ -940,6 +945,13 @@ Maybe you want to use 'update --init'?\")\"\n \t\tIFS=$OIFS\n \t\texit 1\n \tfi\n+\n+\tif test -n \"$has_uninit\" \\\n+\t\t-a \"$(git config --bool --get advice.submoduleUpdateUninit)\" != \"false\"\n+\tthen\n+\t\tsay \"$(eval_gettext \"Uninitialized submodules were detected;\n+Maybe you want to use 'update --init'?\")\"\n+\tfi\n \t}\n }\n \ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex 00475eb..8dbe410 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -76,8 +76,11 @@ test_expect_success 'submodule update <path> warns without init beforehand' '\n \t)\n '\n \n-test_expect_success 'submodule update is silent without init beforehand' '\n+test_expect_success 'submodule update warns without init beforehand' '\n \t(cd super2 &&\n+\t test_must_fail git config --get advice.submoduleUpdateUninit &&\n+\t test -n \"$(git submodule update)\" &&\n+\t git config advice.submoduleUpdateUninit false &&\n \t test -z \"$(git submodule update)\"\n \t)\n '\n-- \n1.8.4.rc4.527.g303b16c\n"},{"id":"227712","messageId":"52373AA2.9050807@web.de","threadId":"34936","inReplyTo":"1379266703-29808-3-git-send-email-rctay89@gmail.com","subject":"Re: [PATCH 3/3] git submodule update should give notice when run without init beforehand","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-09-16T17:06:42Z","receivedAt":"2013-09-16T17:06:42Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 15.09.2013 19:38, schrieb Tay Ray Chuan:\n> When 'update' is run with no path in a repository with uninitialized\n> submodules, the program terminates with no output, and zero status code.\n> Be more helpful to users by mentioning this.\n\nHmm, this patch is changing the default behavior, right? I assume the\nmotivation for this patch is to help people who tend to forget to init\nsubmodules they need to have checked out, which is a good idea. But I\nthink this should not be enabled by default, as in a lot of use cases\none or more uninitialized submodules are present on purpose. In those\nuse cases it would be rather nasty to error out on every submodule\nupdate.\n\nAfter the 'autoinit' configuration (which lets upstream hint that\ncertain submodules should be initialized on clone) has materialzed we\nmight want to enable this error for these specific submodules. But in\nany case the error message should contain a hint on how you can get\nrid of the error in case you know what you are doing ;-).\n\n> This may be controlled by an advice.* option.\n>\n> Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>\n> ---\n>  Documentation/config.txt    |  5 +++++\n>  git-submodule.sh            | 16 ++++++++++++++--\n>  t/t7406-submodule-update.sh |  5 ++++-\n>  3 files changed, 23 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/config.txt b/Documentation/config.txt\n> index ec57a15..79313f9 100644\n> --- a/Documentation/config.txt\n> +++ b/Documentation/config.txt\n> @@ -202,6 +202,11 @@ advice.*::\n>  \trmHints::\n>  \t\tIn case of failure in the output of linkgit:git-rm[1],\n>  \t\tshow directions on how to proceed from the current state.\n> +\tsubmoduleUpdateUninit::\n> +\t\tWhen linkgit:git-submodule[1] `update` is run with no `path`\n> +\t\targuments in a repository with uninitialized submodules,\n> +\t\tmention that uninitalized submodules are indeed present, and\n> +\t\tthat they may be initialized with the `--init` option.\n>  --\n>  \n>  core.fileMode::\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 2979197..56f3dc2 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -777,6 +777,7 @@ cmd_update()\n>  \tcloned_modules=\n>  \tmodule_list \"$@\" | {\n>  \terr=\n> +\thas_uninit=\n>  \twhile read mode sha1 stage sm_path\n>  \tdo\n>  \t\tdie_if_unmatched \"$mode\"\n> @@ -807,9 +808,13 @@ cmd_update()\n>  \t\tthen\n>  \t\t\t# Only mention uninitialized submodules when its\n>  \t\t\t# path have been specified\n> -\t\t\ttest \"$#\" != \"0\" &&\n> -\t\t\tsay \"$(eval_gettext \"Submodule path '\\$displaypath' not initialized\n> +\t\t\tif test \"$#\" != \"0\"\n> +\t\t\tthen\n> +\t\t\t\tsay \"$(eval_gettext \"Submodule path '\\$displaypath' not initialized\n>  Maybe you want to use 'update --init'?\")\"\n> +\t\t\telse\n> +\t\t\t\thas_uninit=1\n> +\t\t\tfi\n>  \t\t\tcontinue\n>  \t\tfi\n>  \n> @@ -940,6 +945,13 @@ Maybe you want to use 'update --init'?\")\"\n>  \t\tIFS=$OIFS\n>  \t\texit 1\n>  \tfi\n> +\n> +\tif test -n \"$has_uninit\" \\\n> +\t\t-a \"$(git config --bool --get advice.submoduleUpdateUninit)\" != \"false\"\n> +\tthen\n> +\t\tsay \"$(eval_gettext \"Uninitialized submodules were detected;\n> +Maybe you want to use 'update --init'?\")\"\n> +\tfi\n>  \t}\n>  }\n>  \n> diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\n> index 00475eb..8dbe410 100755\n> --- a/t/t7406-submodule-update.sh\n> +++ b/t/t7406-submodule-update.sh\n> @@ -76,8 +76,11 @@ test_expect_success 'submodule update <path> warns without init beforehand' '\n>  \t)\n>  '\n>  \n> -test_expect_success 'submodule update is silent without init beforehand' '\n> +test_expect_success 'submodule update warns without init beforehand' '\n>  \t(cd super2 &&\n> +\t test_must_fail git config --get advice.submoduleUpdateUninit &&\n> +\t test -n \"$(git submodule update)\" &&\n> +\t git config advice.submoduleUpdateUninit false &&\n>  \t test -z \"$(git submodule update)\"\n>  \t)\n>  '\n> \n"},{"id":"227832","messageId":"CALUzUxoFLOP=-ub_VYr6LqRYQXOO7Tf1oP95mxuYZo-8_dxAZw@mail.gmail.com","threadId":"34936","inReplyTo":"52373AA2.9050807@web.de","subject":"Re: [PATCH 3/3] git submodule update should give notice when run without init beforehand","fromName":"Tay Ray Chuan","fromEmail":"rctay89@gmail.com","sentAt":"2013-09-18T10:12:42Z","receivedAt":"2013-09-18T10:12:42Z","isPatch":true,"sender":{"key":"rctay89@gmail.com","avatar":"https://avatars.githubusercontent.com/u/61553?v=4"},"body":"On Tue, Sep 17, 2013 at 1:06 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n\nThanks Jens for having a look!\n\n> Am 15.09.2013 19:38, schrieb Tay Ray Chuan:\n>> When 'update' is run with no path in a repository with uninitialized\n>> submodules, the program terminates with no output, and zero status code.\n>> Be more helpful to users by mentioning this.\n>\n> [snip] it would be rather nasty to error out on every submodule\n> update.\n\nJust to be sure we're on the right page, with this patch, the 'update'\ncommand still exits with status code zero (non-error), so this patch\ndoesn't make it error out.\n\n> After the 'autoinit' configuration (which lets upstream hint that\n> certain submodules should be initialized on clone) has materialzed we\n> might want to enable this error for these specific submodules.\n\nThat's cool, I'm looking forward to this. Could you point me to\nsomewhere detailing this?\n\nBut in the meantime, on top of the advice.* config, how about having a\nsubmodule.<name>.ignoreUninit config to disable the message on a\nper-submodule basis?\n\n> But in\n> any case the error message should contain a hint on how you can get\n> rid of the error in case you know what you are doing ;-).\n\nThe message does mention that you can throw in an --init to fix the\nproblem. This \"hint\" is similar to what git-submodule prints when a\n<path> is passed (see region at line 807).\n\n-- \nCheers,\nRay Chuan\n"},{"id":"227857","messageId":"5239FB5E.6060703@web.de","threadId":"34936","inReplyTo":"CALUzUxoFLOP=-ub_VYr6LqRYQXOO7Tf1oP95mxuYZo-8_dxAZw@mail.gmail.com","subject":"Re: [PATCH 3/3] git submodule update should give notice when run without init beforehand","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-09-18T19:13:34Z","receivedAt":"2013-09-18T19:13:34Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 18.09.2013 12:12, schrieb Tay Ray Chuan:\n> On Tue, Sep 17, 2013 at 1:06 AM, Jens Lehmann <Jens.Lehmann@web.de> wrote:\n> \n> Thanks Jens for having a look!\n> \n>> Am 15.09.2013 19:38, schrieb Tay Ray Chuan:\n>>> When 'update' is run with no path in a repository with uninitialized\n>>> submodules, the program terminates with no output, and zero status code.\n>>> Be more helpful to users by mentioning this.\n>>\n>> [snip] it would be rather nasty to error out on every submodule\n>> update.\n> \n> Just to be sure we're on the right page, with this patch, the 'update'\n> command still exits with status code zero (non-error), so this patch\n> doesn't make it error out.\n\nOk, sorry for the confusion. But I still think we should not change\nthe default to print these messages, but make it an opt-in. And the\ncommit message (and maybe the documentation too) should talk about\nthe use cases where this makes sense.\n\n>> After the 'autoinit' configuration (which lets upstream hint that\n>> certain submodules should be initialized on clone) has materialzed we\n>> might want to enable this error for these specific submodules.\n> \n> That's cool, I'm looking forward to this. Could you point me to\n> somewhere detailing this?\n\nNot yet. I'm still wrestling with the autoupdate series, which is\nthe logical step before autoinit. autoupdate enables to configure\nthat initialized submodules are updated on checkout, reset, merge\nand all the other work tree manipulating commands. autoinit then\nalso clones the repos of submodules into .git/modules and inits\nthem in .git/config, so that autoupdate will automagically populate\nthem. Unfortunately autoupdate is a rather largish series touching\nlots of commands and needing tons of tests ...\n\n> But in the meantime, on top of the advice.* config, how about having a\n> submodule.<name>.ignoreUninit config to disable the message on a\n> per-submodule basis?\n\nI'd not add such an option unless users request it because it helps\ntheir not-so-terribly-specific use case.\n\n>> But in\n>> any case the error message should contain a hint on how you can get\n>> rid of the error in case you know what you are doing ;-).\n> \n> The message does mention that you can throw in an --init to fix the\n> problem. This \"hint\" is similar to what git-submodule prints when a\n> <path> is passed (see region at line 807).\n\nBut for a lot of users that isn't a solution, as they never want to\ninit it, they want to ignore it. And if you explicitly ask to update\na special submodule, that message is ok.\n"},{"id":"227881","messageId":"523AD117.20109@gmail.com","threadId":"34936","inReplyTo":"CALUzUxrf+ZGX8nQ0DVqYEyWto3Cos16VLTUfjsX99qnDLa=S6w@mail.gmail.com","subject":"Re: [PATCH 3/3] git submodule update should give notice when run without init beforehand","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2013-09-19T10:25:27Z","receivedAt":"2013-09-19T10:25:27Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"On 18/09/13 22:22, Tay Ray Chuan wrote:\n> Hi Chris,\n> \n> I think you mentioned usability issues with git-submodule before on\n> the git mailing list, so I thought you might be interested in taking a\n> look at this patch. It's attached below, you can also view it at\n> \n>   http://thread.gmane.org/gmane.comp.version-control.git/1379266703-29808-3-git-send-email-rctay89@gmail.com\n> \n> I would be interested in hearing what you think.\n> \n\nThe case I had was that an un-init'd submodule was still a directory on\nthe file system so 'cd submodule' would work and it could go unnoticed\nthat we hadn't entered the submodule.\n\nYour change might have helped somewhat but the bigger problem was that\nthe work-tree representation of an un-init'd submodule is a directory.\nThere was never any thought to actually run 'git submodule update'\nbecause the developer in question thought they already had.\n\nI think Jens' autoinit may be the right direction. It certainly would be\nwhat we'd want for our use-case at $dayjob.\n"}]}