{"thread":{"id":"34729","subject":"[PATCH v2] submodule: prevent warning in summary output","startedAt":"2013-08-20T00:26:37Z","lastAt":"2013-08-26T19:59:44Z","messageCount":3,"participants":["brian m. carlson","Chris Packham","Jens Lehmann"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"225492","messageId":"1376958397-800967-1-git-send-email-sandals@crustytoothpaste.net","threadId":"34729","inReplyTo":null,"subject":"[PATCH v2] submodule: prevent warning in summary output","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2013-08-20T00:26:37Z","receivedAt":"2013-08-20T00:26:37Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"When git submodule summary is run and there is a deleted submodule, there is an\nwarning from git rev-parse:\n\n  fatal: Not a git repository: '.vim/pathogen/.git'\n\nSilence this warning, since it is fully expected that a deleted submodule will\nnot be a git repository.\n\nSigned-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n---\n\nI hesitated to add the test for $status because it will end up having no effect\nsince we exclude that case later.  However, for correctness, I included it.\n\n git-submodule.sh | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2979197..eec3135 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -1070,7 +1070,10 @@ cmd_summary() {\n \t\tmissing_src=\n \t\tmissing_dst=\n \n+\t\ttest $status = D && missing_src=t\n+\n \t\ttest $mod_src = 160000 &&\n+\t\ttest -e \"$name/.git\" &&\n \t\t! GIT_DIR=\"$name/.git\" git-rev-parse -q --verify $sha1_src^0 >/dev/null &&\n \t\tmissing_src=t\n \n-- \n1.8.4.rc3\n"},{"id":"225892","messageId":"521B114B.2080709@gmail.com","threadId":"34729","inReplyTo":"1376958397-800967-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH v2] submodule: prevent warning in summary output","fromName":"Chris Packham","fromEmail":"judge.packham@gmail.com","sentAt":"2013-08-26T08:26:51Z","receivedAt":"2013-08-26T08:26:51Z","isPatch":true,"sender":{"key":"judge.packham@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155667?v=4"},"body":"Hi Brian,\n\nSorry for the delay.\n\nOn 20/08/13 12:26, brian m. carlson wrote:\n> When git submodule summary is run and there is a deleted submodule, there is an\n> warning from git rev-parse:\n> \n>   fatal: Not a git repository: '.vim/pathogen/.git'\n> \n> Silence this warning, since it is fully expected that a deleted submodule will\n> not be a git repository.\n> \n> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n> ---\n> \n> I hesitated to add the test for $status because it will end up having no effect\n> since we exclude that case later.  However, for correctness, I included it.\n> \n>  git-submodule.sh | 3 +++\n>  1 file changed, 3 insertions(+)\n> \n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 2979197..eec3135 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -1070,7 +1070,10 @@ cmd_summary() {\n>  \t\tmissing_src=\n>  \t\tmissing_dst=\n>  \n> +\t\ttest $status = D && missing_src=t\n\nI tend to agree with you that this line is redundant. I'm not sure that\nit's what Jens was looking for in v1.\n\n> +\n>  \t\ttest $mod_src = 160000 &&\n> +\t\ttest -e \"$name/.git\" &&\n>  \t\t! GIT_DIR=\"$name/.git\" git-rev-parse -q --verify $sha1_src^0 >/dev/null &&\n>  \t\tmissing_src=t\n> \n\nThis part looks good to me.\n"},{"id":"225926","messageId":"521BB3B0.4060908@web.de","threadId":"34729","inReplyTo":"521B114B.2080709@gmail.com","subject":"Re: [PATCH v2] submodule: prevent warning in summary output","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-08-26T19:59:44Z","receivedAt":"2013-08-26T19:59:44Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 26.08.2013 10:26, schrieb Chris Packham:\n> Hi Brian,\n> \n> Sorry for the delay.\n\nSame here.\n\n> On 20/08/13 12:26, brian m. carlson wrote:\n>> When git submodule summary is run and there is a deleted submodule, there is an\n>> warning from git rev-parse:\n>>\n>>   fatal: Not a git repository: '.vim/pathogen/.git'\n>>\n>> Silence this warning, since it is fully expected that a deleted submodule will\n>> not be a git repository.\n>>\n>> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>\n>> ---\n>>\n>> I hesitated to add the test for $status because it will end up having no effect\n>> since we exclude that case later.  However, for correctness, I included it.\n>>\n>>  git-submodule.sh | 3 +++\n>>  1 file changed, 3 insertions(+)\n>>\n>> diff --git a/git-submodule.sh b/git-submodule.sh\n>> index 2979197..eec3135 100755\n>> --- a/git-submodule.sh\n>> +++ b/git-submodule.sh\n>> @@ -1070,7 +1070,10 @@ cmd_summary() {\n>>  \t\tmissing_src=\n>>  \t\tmissing_dst=\n>>  \n>> +\t\ttest $status = D && missing_src=t\n> \n> I tend to agree with you that this line is redundant. I'm not sure that\n> it's what Jens was looking for in v1.\n\nUnfortunately you'll get another two \"fatal: Not a git repository:\" errors\nwhen you drop this. Further down a \"git rev-list\" and a \"git log\" are run\non the submodule unless one of the \"missing\" variables is set. But this\ndoesn't feel quite right as we are misusing the missing_src variable for\nsomething else here ...\n\n>> +\n>>  \t\ttest $mod_src = 160000 &&\n>> +\t\ttest -e \"$name/.git\" &&\n>>  \t\t! GIT_DIR=\"$name/.git\" git-rev-parse -q --verify $sha1_src^0 >/dev/null &&\n>>  \t\tmissing_src=t\n>>\n> \n> This part looks good to me.\n\nI agree that this is much better than just piping all errors to /dev/null,\nbut I was thinking of\n\n+               test $status != D &&\n\ninstead of\n\n+\t\ttest -e \"$name/.git\" &&\n\nabove to still catch errors where the submodule repo is gone but the\nsubmodule wasn't deleted.\n\nThe following diff silences all \"fatal: Not a git repository:\" errors for\nme using the status variable. But now four tests in t7401 are failing\nbecause they disagree about an empty line and/or the \"(0)\" at the end of\na line. But this could be a good starting point (and includes a test for\nwhat we are trying to fix here :-).\n\n----------------------------------8<-------------------------\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 2979197..69f6a1b 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -1071,6 +1071,7 @@ cmd_summary() {\n                missing_dst=\n\n                test $mod_src = 160000 &&\n+               test $status != D &&\n                ! GIT_DIR=\"$name/.git\" git-rev-parse -q --verify $sha1_src^0 >/dev/null &&\n                missing_src=t\n\n@@ -1103,6 +1104,7 @@ cmd_summary() {\n                        else\n                                range=$sha1_dst\n                        fi\n+                       test $status != D &&\n                        GIT_DIR=\"$name/.git\" \\\n                        git rev-list --first-parent $range -- | wc -l\n                        )\n@@ -1143,6 +1145,7 @@ cmd_summary() {\n                                GIT_DIR=\"$name/.git\" \\\n                                git log --pretty='format:  > %s' -1 $sha1_dst\n                        else\n+                               test $status != D &&\n                                GIT_DIR=\"$name/.git\" \\\n                                git log --pretty='format:  < %s' -1 $sha1_src\n                        fi\ndiff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh\nindex ac2434c..66c73f6 100755\n--- a/t/t7401-submodule-summary.sh\n+++ b/t/t7401-submodule-summary.sh\n@@ -215,12 +215,13 @@ EOF\n commit_file sm1 &&\n rm -rf sm1\n test_expect_success 'deleted submodule' \"\n-       git submodule summary >actual &&\n+       git submodule summary >actual 2>err &&\n        cat >expected <<-EOF &&\n * sm1 $head6...0000000:\n\n EOF\n-       test_cmp expected actual\n+       test_cmp expected actual &&\n+       ! test -s err\n \"\n\n test_create_repo sm2 &&\n"}]}