Re: [PATCH v2] git-submodule: Don't die when command fails for one submodule

From: Ping Yin <pkufranky@gmail.com>
Date: Mar 5, 2008, 17:20 UTC
Message-ID: <46dff0320803050920l4483d436of9b938936b47bfb4@mail.gmail.com>
In-Reply-To: <alpine.LSU.1.00.0803051742290.15786@racer.site>

On Thu, Mar 6, 2008 at 12:48 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 81 quoted lines

> Hi,
>
>
>  On Wed, 5 Mar 2008, Ping Yin wrote:
>
>  > @@ -221,29 +229,40 @@ cmd_init()
>  >       done
>  >
>  >       git ls-files --stage -- "$@" | grep -e '^160000 ' |
>  > +     {
>  > +     total=0
>  > +     success=0
>  >       while read mode sha1 stage path
>  >       do
>  > +             total=$(( $total + 1 ))
>  >               # Skip already registered paths
>  > -             name=$(module_name "$path") || exit
>  > +             name=$(module_name "$path") || continue
>
>  What about this case?  Was the comment misleading?
>
>
>  >               url=$(git config submodule."$name".url)
>  > -             test -z "$url" || continue
>  > +             test -n "$url" && success=$(( $success + 1 )) && continue
>
>  Why counting?  Why not just 'status=0 && continue'?
>
>
>  >
>  > -             git config submodule."$name".url "$url" ||
>  > -             die "Failed to register url for submodule path '$path'"
>  > -
>  > -             say "Submodule '$name' ($url) registered for path '$path'"
>  > +             if git config submodule."$name".url "$url"
>  > +             then
>  > +                     say "Submodule '$name' ($url) registered for path '$path'"
>  > +             else
>  > +                     say "Failed to register url for submodule path '$path'"
>  > +                     continue
>  > +             fi
>  > +             success=$(( $success + 1 ))
>  >       done
>  > +     test $success = $total
>  > +     }
>  >  }
>  >
>  >  #
>
>  Note: I have not even begun to audit if one of your returns should not
>  have been a "status=0 && continue" instead.
>
>
>  > @@ -358,9 +392,11 @@ cmd_status()
>  >       done
>  >
>  >       git ls-files --stage -- "$@" | grep -e '^160000 ' |
>  > +     {
>  > +     exit_status=0
>  >       while read mode sha1 stage path
>  >       do
>  > -             name=$(module_name "$path") || exit
>  > +             ! name=$(module_name "$path") && exit_status=1 && continue
>  >               url=$(git config submodule."$name".url)
>  >               if test -z "$url" || ! test -d "$path"/.git
>  >               then
>  > @@ -380,6 +416,8 @@ cmd_status()
>  >                       say "+$sha1 $path$revname"
>  >               fi
>  >       done
>  > +     exit $exit_status
>  > +     }
>  >  }
>
>  But here you use the simpler paradigm of setting exit_status?  Why that
>  complicated and ugly "total" and "success" counting before?
>
>  Ciao,
>  Dscho
>
>

'total' and 'success' is a trick which i think is simpler than exit_status=1/exit_status=0 in cmd_update and cmd_init.

In the while loop of these two functions, there are many tests which will continue the loop after a failure. By using exit_stauts, i have to replace 'cmd || continue' as '! cmd && exit_status=1 && continue". I don't want so many exit_status in the while loop. So i use 'total' and 'sucess' which count the total number of loops and the number of successful loops separately to simplify this. The trick is as follows:

'total' will plus 1 when at the beginning of a loop and success will plus 1 at the end of loop. If a loop succeeds and continues in the middle of a loop, success also plus 1. Failed loops never reach the loop end so success will not plus 1.

After leaving all loops, $sucess=$total means all loops succeed and exit status should be 0, or exit status should be 1.

In cmd_status, the 'cmd || continue' only exists in one place, so i use exit_status instead of 'success' and 'total'.

-- 
Ping Yin