[PATCH v2] submodule: prevent warning in summary output

[PATCH v2] submodule: prevent warning in summary output

[brian m. carlson]

When git submodule summary is run and there is a deleted submodule, there is an
warning from git rev-parse:

  fatal: Not a git repository: '.vim/pathogen/.git'

Silence this warning, since it is fully expected that a deleted submodule will
not be a git repository.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---

I hesitated to add the test for $status because it will end up having no effect
since we exclude that case later.  However, for correctness, I included it.

 git-submodule.sh | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..eec3135 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1070,7 +1070,10 @@ cmd_summary() {
 		missing_src=
 		missing_dst=
 
+		test $status = D && missing_src=t
+
 		test $mod_src = 160000 &&
+		test -e "$name/.git" &&
 		! GIT_DIR="$name/.git" git-rev-parse -q --verify $sha1_src^0 >/dev/null &&
 		missing_src=t
 
-- 
1.8.4.rc3

Re: [PATCH v2] submodule: prevent warning in summary output

[Chris Packham]

Hi Brian,

Sorry for the delay.

On 20/08/13 12:26, brian m. carlson wrote:
> When git submodule summary is run and there is a deleted submodule, there is an
> warning from git rev-parse:
> 
>   fatal: Not a git repository: '.vim/pathogen/.git'
> 
> Silence this warning, since it is fully expected that a deleted submodule will
> not be a git repository.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> 
> I hesitated to add the test for $status because it will end up having no effect
> since we exclude that case later.  However, for correctness, I included it.
> 
>  git-submodule.sh | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/git-submodule.sh b/git-submodule.sh
> index 2979197..eec3135 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -1070,7 +1070,10 @@ cmd_summary() {
>  		missing_src=
>  		missing_dst=
>  
> +		test $status = D && missing_src=t

I tend to agree with you that this line is redundant. I'm not sure that
it's what Jens was looking for in v1.

> +
>  		test $mod_src = 160000 &&
> +		test -e "$name/.git" &&
>  		! GIT_DIR="$name/.git" git-rev-parse -q --verify $sha1_src^0 >/dev/null &&
>  		missing_src=t
> 

This part looks good to me.

Re: [PATCH v2] submodule: prevent warning in summary output

[Jens Lehmann]

Am 26.08.2013 10:26, schrieb Chris Packham:
> Hi Brian,
> 
> Sorry for the delay.

Same here.

> On 20/08/13 12:26, brian m. carlson wrote:
>> When git submodule summary is run and there is a deleted submodule, there is an
>> warning from git rev-parse:
>>
>>   fatal: Not a git repository: '.vim/pathogen/.git'
>>
>> Silence this warning, since it is fully expected that a deleted submodule will
>> not be a git repository.
>>
>> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
>> ---
>>
>> I hesitated to add the test for $status because it will end up having no effect
>> since we exclude that case later.  However, for correctness, I included it.
>>
>>  git-submodule.sh | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/git-submodule.sh b/git-submodule.sh
>> index 2979197..eec3135 100755
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -1070,7 +1070,10 @@ cmd_summary() {
>>  		missing_src=
>>  		missing_dst=
>>  
>> +		test $status = D && missing_src=t
> 
> I tend to agree with you that this line is redundant. I'm not sure that
> it's what Jens was looking for in v1.

Unfortunately you'll get another two "fatal: Not a git repository:" errors
when you drop this. Further down a "git rev-list" and a "git log" are run
on the submodule unless one of the "missing" variables is set. But this
doesn't feel quite right as we are misusing the missing_src variable for
something else here ...

>> +
>>  		test $mod_src = 160000 &&
>> +		test -e "$name/.git" &&
>>  		! GIT_DIR="$name/.git" git-rev-parse -q --verify $sha1_src^0 >/dev/null &&
>>  		missing_src=t
>>
> 
> This part looks good to me.

I agree that this is much better than just piping all errors to /dev/null,
but I was thinking of

+               test $status != D &&

instead of

+		test -e "$name/.git" &&

above to still catch errors where the submodule repo is gone but the
submodule wasn't deleted.

The following diff silences all "fatal: Not a git repository:" errors for
me using the status variable. But now four tests in t7401 are failing
because they disagree about an empty line and/or the "(0)" at the end of
a line. But this could be a good starting point (and includes a test for
what we are trying to fix here :-).

----------------------------------8<-------------------------
diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197..69f6a1b 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1071,6 +1071,7 @@ cmd_summary() {
                missing_dst=

                test $mod_src = 160000 &&
+               test $status != D &&
                ! GIT_DIR="$name/.git" git-rev-parse -q --verify $sha1_src^0 >/dev/null &&
                missing_src=t

@@ -1103,6 +1104,7 @@ cmd_summary() {
                        else
                                range=$sha1_dst
                        fi
+                       test $status != D &&
                        GIT_DIR="$name/.git" \
                        git rev-list --first-parent $range -- | wc -l
                        )
@@ -1143,6 +1145,7 @@ cmd_summary() {
                                GIT_DIR="$name/.git" \
                                git log --pretty='format:  > %s' -1 $sha1_dst
                        else
+                               test $status != D &&
                                GIT_DIR="$name/.git" \
                                git log --pretty='format:  < %s' -1 $sha1_src
                        fi
diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh
index ac2434c..66c73f6 100755
--- a/t/t7401-submodule-summary.sh
+++ b/t/t7401-submodule-summary.sh
@@ -215,12 +215,13 @@ EOF
 commit_file sm1 &&
 rm -rf sm1
 test_expect_success 'deleted submodule' "
-       git submodule summary >actual &&
+       git submodule summary >actual 2>err &&
        cat >expected <<-EOF &&
 * sm1 $head6...0000000:

 EOF
-       test_cmp expected actual
+       test_cmp expected actual &&
+       ! test -s err
 "

 test_create_repo sm2 &&