Re: [PATCH 2/2] submodule: don't print status output with ignore=all

2013-08-17 Thread brian m. carlson 
On Sun, Aug 11, 2013 at 04:03:17PM +, brian m. carlson wrote:
 On Sat, Aug 03, 2013 at 11:24:20AM -0700, Jonathan Nieder wrote:
  If I just renamed a submodule, will 'module_name $path' do the right
  thing with the old path?
 
 module_name uses whatever's in .gitmodules.  I'm not sure what you mean
 by renamed a submodule, since git mv foo bar fails with:
 
   vauxhall ok % git mv .vim/bundle/ctrlp .vim/bundle/ctrlq
   fatal: source directory is empty, source=.vim/bundle/ctrlp, 
 destination=.vim/bundle/ctrlq

Okay, I've tested this against next and it seems that the code handles
it properly (at least I think it does).  I left the following code

  # Always show modules deleted or type-changed (blob-module)
  test $status = D -o $status = T  echo $sm_path  continue

before the ignore code, so I adjusted the new ignore code so that it
prints both the new and the old path.  I don't know that it's possible
to print neither in that case, since we no longer can look up the old
path in .gitmodules.  I'll be sending my reroll shortly.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature

Re: [PATCH 2/2] submodule: don't print status output with ignore=all

2013-08-11 Thread brian m. carlson 
On Sat, Aug 03, 2013 at 11:24:20AM -0700, Jonathan Nieder wrote:
 If I have '[submodule favorite] ignore = all' and I then replace
 that submodule with a blob, should git submodule status not mention
 that path?

Yes, I think it should.  I'll fix this in the reroll.

 If I just renamed a submodule, will 'module_name $path' do the right
 thing with the old path?

module_name uses whatever's in .gitmodules.  I'm not sure what you mean
by renamed a submodule, since git mv foo bar fails with:

  vauxhall ok % git mv .vim/bundle/ctrlp .vim/bundle/ctrlq
  fatal: source directory is empty, source=.vim/bundle/ctrlp, 
destination=.vim/bundle/ctrlq

Can you provide me a set of steps to reproduce that operation so I can
test it effectively?

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature

Re: [PATCH 2/2] submodule: don't print status output with ignore=all

2013-08-11 Thread Jonathan Nieder 
brian m. carlson wrote:

 module_name uses whatever's in .gitmodules.  I'm not sure what you mean
 by renamed a submodule, since git mv foo bar fails with:

   vauxhall ok % git mv .vim/bundle/ctrlp .vim/bundle/ctrlq
   fatal: source directory is empty, source=.vim/bundle/ctrlp, 
 destination=.vim/bundle/ctrlq

 Can you provide me a set of steps to reproduce that operation so I can
 test it effectively?

Ah, I forgot Jens's submodule-mv patch series has not hit master yet.
You can get it by running

git merge origin/pu^{/jl/submodule-mv}^2

before building git.

Thanks,
Jonathan
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] submodule: don't print status output with ignore=all

2013-08-10 Thread brian m. carlson 
On Sun, Aug 04, 2013 at 08:24:09PM +0200, Jens Lehmann wrote:
 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).

Right, that wasn't the intent.

 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 ;-).

I think I'm going to go this route.  My goal is to fix up the TODO tests
and make them work so I can get more familiar with the code.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature

Re: [PATCH 2/2] submodule: don't print status output with ignore=all

2013-08-04 Thread Jens Lehmann 
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 sand...@crustytoothpaste.net
 ---
  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]* )?16' |
  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 majord...@vger.kernel.org
 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] submodule: don't print status output with ignore=all

2013-08-03 Thread brian m. carlson 
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.

Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
---
 git-submodule.sh  | 2 ++
 t/t7508-status.sh | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 30b7fc1..5694ae6 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -1034,6 +1034,8 @@ cmd_summary() {
sane_egrep '^:([0-7]* )?16' |
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
# 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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] submodule: don't print status output with ignore=all

2013-08-03 Thread 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.

 Signed-off-by: brian m. carlson sand...@crustytoothpaste.net
 ---
  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]* )?16' |
   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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html