Re: [PATCH 1/2] Teach --recursive to submodule sync

2012-10-26 Thread Phil Hord

Jens Lehmann wrote:
 Am 24.10.2012 01:15, schrieb Phil Hord:
 The submodule sync command was somehow left out when
 --recursive was added to the other submodule commands.

 Teach sync to handle the --recursive switch by recursing
 when we're in a submodule we are sync'ing.

 Change the report during sync to show submodule-path
 instead of submodule-name to be consistent with the other
 submodule commands and to help recursed paths make sense.

 Signed-off-by: Phil Hord ho...@cisco.com
 This makes perfect sense to me. Two things though:

 First it would be nice to initialize orig_flags like all the other
 call sites do:

 @@ -1003,6 +1003,7 @@ cmd_status()
  #
  cmd_sync()
  {
 + orig_flags=
   while test $# -ne 0
   do
   case $1 in

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

 diff --git a/git-submodule.sh b/git-submodule.sh
 index ab6b110..6dd2338 100755
 --- a/git-submodule.sh
 +++ b/git-submodule.sh
 @@ -11,7 +11,7 @@ USAGE=[--quiet] add [-b branch] [-f|--force] [--reference 
 repository] [--] r
 or: $dashless [--quiet] update [--init] [-N|--no-fetch] [-f|--force] 
 [--rebase] [--reference repository] [--merge] [--recursive] [--] 
 [path...]
 or: $dashless [--quiet] summary [--cached|--files] [--summary-limit n] 
 [commit] [--] [path...]
 or: $dashless [--quiet] foreach [--recursive] command
 -   or: $dashless [--quiet] sync [--] [path...]
 +   or: $dashless [--quiet] sync [--recursive] [--] [path...]
  OPTIONS_SPEC=
  . git-sh-setup
  . git-sh-i18n
 @@ -1008,7 +1008,9 @@ cmd_sync()
  case $1 in
  -q|--quiet)
  GIT_QUIET=1
 -shift
 +;;
 +--recursive)
 +recursive=1
  ;;
  --)
  shift
 @@ -1021,6 +1023,8 @@ cmd_sync()
  break
  ;;
  esac
 +orig_flags=$orig_flags $(git rev-parse --sq-quote $1)
 +shift
  done
  cd_to_toplevel
  module_list $@ |
 @@ -1051,7 +1055,7 @@ cmd_sync()
  
  if git config submodule.$name.url /dev/null 2/dev/null
  then
 -say $(eval_gettext Synchronizing submodule url for 
 '\$name')
 +say $(eval_gettext Synchronizing submodule url for 
 '\$prefix\$sm_path')
  git config submodule.$name.url $super_config_url
  
  if test -e $sm_path/.git
 @@ -1061,6 +1065,14 @@ cmd_sync()
  cd $sm_path
  remote=$(get_default_remote)
  git config remote.$remote.url 
 $sub_origin_url
 +
 +if test -n $recursive
 +then
 +(
 +prefix=$prefix$sm_path/
 +eval cmd_sync $orig_args
 This should read 'eval cmd_sync $orig_flags'. I think you copied that
 from cmd_status(), where this is also incorrect, I just sent a patch to
 correct that one.

Yes, thanks for catching that.  I think I should add a test for that
except I notice that sync doesn't take any other flags useful for passing.

v2 is on the way.

 +)
 +fi
  )
  fi
  fi


--
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 1/2] Teach --recursive to submodule sync

2012-10-26 Thread Phil Hord
On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord ho...@cisco.com wrote:

 Yes, thanks for catching that.  I think I should add a test for that
 except I notice that sync doesn't take any other flags useful for passing.

Which, of course, suggests that I should not add this
flag-propagating-machinery to submodule-sync at all. yes?

Phil
--
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 1/2] Teach --recursive to submodule sync

2012-10-26 Thread Jens Lehmann
Am 26.10.2012 19:55, schrieb Phil Hord:
 On Fri, Oct 26, 2012 at 1:19 PM, Phil Hord ho...@cisco.com wrote:

 Yes, thanks for catching that.  I think I should add a test for that
 except I notice that sync doesn't take any other flags useful for passing.
 
 Which, of course, suggests that I should not add this
 flag-propagating-machinery to submodule-sync at all. yes?

Nope, the new --recursive option has to be passed on!

To catch that bug in your test you'd need another submodule inside your
sub-submodule. The first level submodule is initialized by sync anyways,
the sub-submodule is initialized by the --recursive logic you added but
the sub-sub-submodule would not have been synced because the option was
dropped. I really can't blame you for not adding that third level of
submodules ;-)
--
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