Re: [PATCH v2] submodule: prevent warning in summary output
Am 26.08.2013 10:26, schrieb Chris Packham:
> Hi Brian,
>
> Sorry for the delay.
Show 30 quoted lines
> 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 ...
Show 8 quoted lines
>> +
>> 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<-------------------------
Show changes to 2 files +6 −2
git-submodule.sh, t/t7401-submodule-summary.sh
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 &&
brian m. carlson
