Am 03.08.2013 20:24, schrieb Jonathan Nieder:
> brian m. carlson wrote:
>
>> git status prints information for submodules, but it should ignore the status of
>> those which have submodule.<name>.ignore set to all. Fix it so that it does
>> properly ignore those which have that setting either in .git/config or in
>> .gitmodules.
I'm a bit confused. The commit message talks about "git status", but the code
you changed handles "git submodule summary". Looks like you are trying to fix
the output of status when the status.submodulesummary option is set, right?
That's a good thing to do.
But your patch also changes the default behavior of "git submodule summary",
which is a change in behavior as that is currently not configurable via the
ignore option (and I believe it should stay that way for backward compatibility
reasons unless actual users provide sound reasons to change that). So a NACK
on this patch from me (and a note to self that tests are missing that should
have failed due to this change).
As a short term solution you could honor the submodule.<name>.ignore setting
only if --for-status is used, as that is explicitly given by "git status" when
it forks the "git submodule summary" script (to make it prepend "# " to each
line, which it could do easily itself nowadays using recently added code ;-).
If you want to fix that issue and make git status perform a lot better, you
should make the status.submodulesummary use what we already have in the diff
machinery instead of forking the submodule script (which it does for hysterical
raisins). Basically you'd need to run just what "git diff-files" and "git
diff-index HEAD" run when they are given the --submodule option, prepend "# "
to the output and limit it to the amount of lines configured via the
status.submodulesummary option. Then we could get rid of the --for-status
option of submodule summary and move some more functionality from that script
into core git.
I'll be glad to help you fixing this problem either way.
>> Signed-off-by: brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx>
>> ---
>> git-submodule.sh | 2 ++
>> t/t7508-status.sh | 4 ++--
>> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> Thanks. Cc-ing Jens, who wrote that test and knows this code much
> better than I do. :)
>
> [...]
>> --- a/git-submodule.sh
>> +++ b/git-submodule.sh
>> @@ -1034,6 +1034,8 @@ cmd_summary() {
>> sane_egrep '^:([0-7]* )?160000' |
>> while read mod_src mod_dst sha1_src sha1_dst status path
>> do
>> + name=$(module_name "$path")
>> + test $(get_submodule_config "$name" ignore none) = all && continue
>> # Always show modules deleted or type-changed (blob<->module)
>> test $status = D -o $status = T && echo "$path" && continue
>
> I'm not sure what the exact semantics should be here, though that's
> mostly because of my unfamiliarity with submodules in general.
>
> If I have '[submodule "favorite"] ignore = all' and I then replace
> that submodule with a blob, should "git submodule status" not mention
> that path?
>
> If I just renamed a submodule, will 'module_name "$path"' do the right
> thing with the old path?
>
> (rest of the patch kept unsnipped for reference)
>> # Also show added or modified modules which are checked out
>> diff --git a/t/t7508-status.sh b/t/t7508-status.sh
>> index ac3d0fe..fb89fb9 100755
>> --- a/t/t7508-status.sh
>> +++ b/t/t7508-status.sh
>> @@ -1316,7 +1316,7 @@ test_expect_success "--ignore-submodules=all suppresses submodule summary" '
>> test_i18ncmp expect output
>> '
>>
>> -test_expect_failure '.gitmodules ignore=all suppresses submodule summary' '
>> +test_expect_success '.gitmodules ignore=all suppresses submodule summary' '
>> git config --add -f .gitmodules submodule.subname.ignore all &&
>> git config --add -f .gitmodules submodule.subname.path sm &&
>> git status > output &&
>> @@ -1324,7 +1324,7 @@ test_expect_failure '.gitmodules ignore=all suppresses submodule summary' '
>> git config -f .gitmodules --remove-section submodule.subname
>> '
>>
>> -test_expect_failure '.git/config ignore=all suppresses submodule summary' '
>> +test_expect_success '.git/config ignore=all suppresses submodule summary' '
>> git config --add -f .gitmodules submodule.subname.ignore none &&
>> git config --add -f .gitmodules submodule.subname.path sm &&
>> git config --add submodule.subname.ignore all &&
>> --
>> 1.8.4.rc1
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html