{"thread":{"id":"12518","subject":"[PATCH v2] git-submodule: Don't die when command fails for one submodule","startedAt":"2008-03-05T15:21:18Z","lastAt":"2008-03-05T17:20:39Z","messageCount":3,"participants":["Ping Yin","Johannes Schindelin"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"71084","messageId":"1204730478-22027-1-git-send-email-pkufranky@gmail.com","threadId":"12518","inReplyTo":null,"subject":"[PATCH v2] git-submodule: Don't die when command fails for one submodule","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-03-05T15:21:18Z","receivedAt":"2008-03-05T15:21:18Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"When handling multiple modules, init/update/status command will exit\nwhen it fails for one submodule. This patch makes the command continue\nbypassing the failure and keep right exit status.\n\nSigned-off-by: Ping Yin <pkufranky@gmail.com>\n---\n git-submodule.sh |  110 ++++++++++++++++++++++++++++++++++++------------------\n 1 files changed, 74 insertions(+), 36 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 67d3224..3ad7214 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -100,15 +100,18 @@ module_clone()\n \t# succeed but the rmdir will fail. We might want to fix this.\n \tif test -d \"$path\"\n \tthen\n-\t\trmdir \"$path\" 2>/dev/null ||\n-\t\tdie \"Directory '$path' exist, but is neither empty nor a git repository\"\n+\t\t! rmdir \"$path\" 2>/dev/null &&\n+\t\tsay \"Directory '$path' exist, but is neither empty nor a git repository\" &&\n+\t\treturn 1\n \tfi\n \n \ttest -e \"$path\" &&\n-\tdie \"A file already exist at path '$path'\"\n+\tsay \"A file already exist at path '$path'\" &&\n+\treturn 1\n \n-\tgit-clone -n \"$url\" \"$path\" ||\n-\tdie \"Clone of '$url' into submodule path '$path' failed\"\n+\t! git-clone -n \"$url\" \"$path\" &&\n+\tsay \"Clone of '$url' into submodule path '$path' failed\" &&\n+\treturn 1\n }\n \n #\n@@ -156,7 +159,7 @@ cmd_add()\n \tcase \"$repo\" in\n \t./*|../*)\n \t\t# dereference source url relative to parent's url\n-\t\trealrepo=\"$(resolve_relative_url $repo)\" ;;\n+\t\trealrepo=\"$(resolve_relative_url $repo)\" || return 1 ;;\n \t*)\n \t\t# Turn the source into an absolute path if\n \t\t# it is local\n@@ -175,21 +178,26 @@ cmd_add()\n \tfi\n \n \ttest -e \"$path\" &&\n-\tdie \"'$path' already exists\"\n+\tsay \"'$path' already exists\" &&\n+\treturn 1\n \n \tgit ls-files --error-unmatch \"$path\" > /dev/null 2>&1 &&\n-\tdie \"'$path' already exists in the index\"\n+\tsay \"'$path' already exists in the index\" &&\n+\treturn 1\n \n-\tmodule_clone \"$path\" \"$realrepo\" || exit\n-\t(unset GIT_DIR; cd \"$path\" && git checkout -q ${branch:+-b \"$branch\" \"origin/$branch\"}) ||\n-\tdie \"Unable to checkout submodule '$path'\"\n-\tgit add \"$path\" ||\n-\tdie \"Failed to add submodule '$path'\"\n+\tmodule_clone \"$path\" \"$realrepo\" || return 1\n+\t! (unset GIT_DIR; cd \"$path\" && git checkout -q ${branch:+-b \"$branch\" \"origin/$branch\"}) &&\n+\tsay \"Unable to checkout submodule '$path'\" &&\n+\treturn 1\n+\t! git add \"$path\" &&\n+\tsay \"Failed to add submodule '$path'\" &&\n+\treturn 1\n \n \tGIT_CONFIG=.gitmodules git config submodule.\"$path\".path \"$path\" &&\n \tGIT_CONFIG=.gitmodules git config submodule.\"$path\".url \"$repo\" &&\n-\tgit add .gitmodules ||\n-\tdie \"Failed to register submodule '$path'\"\n+\t! git add .gitmodules &&\n+\tsay \"Failed to register submodule '$path'\" &&\n+\treturn 1\n }\n \n #\n@@ -221,29 +229,40 @@ cmd_init()\n \tdone\n \n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n+\t{\n+\ttotal=0\n+\tsuccess=0\n \twhile read mode sha1 stage path\n \tdo\n+\t\ttotal=$(( $total + 1 ))\n \t\t# Skip already registered paths\n-\t\tname=$(module_name \"$path\") || exit\n+\t\tname=$(module_name \"$path\") || continue\n \t\turl=$(git config submodule.\"$name\".url)\n-\t\ttest -z \"$url\" || continue\n+\t\ttest -n \"$url\" && success=$(( $success + 1 )) && continue\n \n \t\turl=$(GIT_CONFIG=.gitmodules git config submodule.\"$name\".url)\n \t\ttest -z \"$url\" &&\n-\t\tdie \"No url found for submodule path '$path' in .gitmodules\"\n+\t\tsay \"No url found for submodule path '$path' in .gitmodules\" &&\n+\t\tcontinue\n \n \t\t# Possibly a url relative to parent\n \t\tcase \"$url\" in\n \t\t./*|../*)\n-\t\t\turl=\"$(resolve_relative_url \"$url\")\"\n+\t\t\turl=\"$(resolve_relative_url \"$url\")\" || continue\n \t\t\t;;\n \t\tesac\n \n-\t\tgit config submodule.\"$name\".url \"$url\" ||\n-\t\tdie \"Failed to register url for submodule path '$path'\"\n-\n-\t\tsay \"Submodule '$name' ($url) registered for path '$path'\"\n+\t\tif git config submodule.\"$name\".url \"$url\"\n+\t\tthen\n+\t\t\tsay \"Submodule '$name' ($url) registered for path '$path'\"\n+\t\telse\n+\t\t\tsay \"Failed to register url for submodule path '$path'\"\n+\t\t\tcontinue\n+\t\tfi\n+\t\tsuccess=$(( $success + 1 ))\n \tdone\n+\ttest $success = $total\n+\t}\n }\n \n #\n@@ -275,38 +294,53 @@ cmd_update()\n \tdone\n \n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n+\t{\n+\ttotal=0\n+\tsuccess=0\n \twhile read mode sha1 stage path\n \tdo\n-\t\tname=$(module_name \"$path\") || exit\n+\t\ttotal=$(( $total + 1 ))\n+\t\tname=$(module_name \"$path\") || continue\n \t\turl=$(git config submodule.\"$name\".url)\n \t\tif test -z \"$url\"\n \t\tthen\n \t\t\t# Only mention uninitialized submodules when its\n \t\t\t# path have been specified\n-\t\t\ttest \"$#\" != \"0\" &&\n-\t\t\tsay \"Submodule path '$path' not initialized\"\n+\t\t\tif test \"$#\" != \"0\"\n+\t\t\tthen\n+\t\t\t\tsay \"Submodule path '$path' not initialized\"\n+\t\t\telse\n+\t\t\t\tsuccess=$(( $success + 1 ))\n+\t\t\tfi\n \t\t\tcontinue\n \t\tfi\n \n \t\tif ! test -d \"$path\"/.git\n \t\tthen\n-\t\t\tmodule_clone \"$path\" \"$url\" || exit\n+\t\t\tmodule_clone \"$path\" \"$url\" || continue\n \t\t\tsubsha1=\n \t\telse\n-\t\t\tsubsha1=$(unset GIT_DIR; cd \"$path\" &&\n-\t\t\t\tgit rev-parse --verify HEAD) ||\n-\t\t\tdie \"Unable to find current revision in submodule path '$path'\"\n+\t\t\t! subsha1=$(unset GIT_DIR; cd \"$path\" &&\n+\t\t\t\tgit rev-parse --verify HEAD 2>/dev/null) &&\n+\t\t\tsay \"Unable to find current revision in submodule path '$path'\" &&\n+\t\t\tcontinue\n \t\tfi\n \n \t\tif test \"$subsha1\" != \"$sha1\"\n \t\tthen\n-\t\t\t(unset GIT_DIR; cd \"$path\" && git-fetch &&\n-\t\t\t\tgit-checkout -q \"$sha1\") ||\n-\t\t\tdie \"Unable to checkout '$sha1' in submodule path '$path'\"\n-\n-\t\t\tsay \"Submodule path '$path': checked out '$sha1'\"\n+\t\t\tif (unset GIT_DIR; cd \"$path\" && git-fetch &&\n+\t\t\t\tgit-checkout -q \"$sha1\")\n+\t\t\tthen\n+\t\t\t\tsay \"Submodule path '$path': checked out '$sha1'\"\n+\t\t\telse\n+\t\t\t\tsay \"Unable to checkout '$sha1' in submodule path '$path'\"\n+\t\t\t\tcontinue\n+\t\t\tfi\n \t\tfi\n+\t\tsuccess=$(( $success + 1 ))\n \tdone\n+\ttest $success = $total\n+\t}\n }\n \n set_name_rev () {\n@@ -358,9 +392,11 @@ cmd_status()\n \tdone\n \n \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n+\t{\n+\texit_status=0\n \twhile read mode sha1 stage path\n \tdo\n-\t\tname=$(module_name \"$path\") || exit\n+\t\t! name=$(module_name \"$path\") && exit_status=1 && continue\n \t\turl=$(git config submodule.\"$name\".url)\n \t\tif test -z \"$url\" || ! test -d \"$path\"/.git\n \t\tthen\n@@ -380,6 +416,8 @@ cmd_status()\n \t\t\tsay \"+$sha1 $path$revname\"\n \t\tfi\n \tdone\n+\texit $exit_status\n+\t}\n }\n \n # This loop parses the command line arguments to find the\n-- \n1.5.4.3.347.g5314c\n\n"},{"id":"71099","messageId":"alpine.LSU.1.00.0803051742290.15786@racer.site","threadId":"12518","inReplyTo":"1204730478-22027-1-git-send-email-pkufranky@gmail.com","subject":"Re: [PATCH v2] git-submodule: Don't die when command fails for one submodule","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-05T16:48:37Z","receivedAt":"2008-03-05T16:48:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 5 Mar 2008, Ping Yin wrote:\n\n> @@ -221,29 +229,40 @@ cmd_init()\n>  \tdone\n>  \n>  \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n> +\t{\n> +\ttotal=0\n> +\tsuccess=0\n>  \twhile read mode sha1 stage path\n>  \tdo\n> +\t\ttotal=$(( $total + 1 ))\n>  \t\t# Skip already registered paths\n> -\t\tname=$(module_name \"$path\") || exit\n> +\t\tname=$(module_name \"$path\") || continue\n\nWhat about this case?  Was the comment misleading?\n\n>  \t\turl=$(git config submodule.\"$name\".url)\n> -\t\ttest -z \"$url\" || continue\n> +\t\ttest -n \"$url\" && success=$(( $success + 1 )) && continue\n\nWhy counting?  Why not just 'status=0 && continue'?\n\n>  \n> -\t\tgit config submodule.\"$name\".url \"$url\" ||\n> -\t\tdie \"Failed to register url for submodule path '$path'\"\n> -\n> -\t\tsay \"Submodule '$name' ($url) registered for path '$path'\"\n> +\t\tif git config submodule.\"$name\".url \"$url\"\n> +\t\tthen\n> +\t\t\tsay \"Submodule '$name' ($url) registered for path '$path'\"\n> +\t\telse\n> +\t\t\tsay \"Failed to register url for submodule path '$path'\"\n> +\t\t\tcontinue\n> +\t\tfi\n> +\t\tsuccess=$(( $success + 1 ))\n>  \tdone\n> +\ttest $success = $total\n> +\t}\n>  }\n>  \n>  #\n\nNote: I have not even begun to audit if one of your returns should not \nhave been a \"status=0 && continue\" instead.\n\n> @@ -358,9 +392,11 @@ cmd_status()\n>  \tdone\n>  \n>  \tgit ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n> +\t{\n> +\texit_status=0\n>  \twhile read mode sha1 stage path\n>  \tdo\n> -\t\tname=$(module_name \"$path\") || exit\n> +\t\t! name=$(module_name \"$path\") && exit_status=1 && continue\n>  \t\turl=$(git config submodule.\"$name\".url)\n>  \t\tif test -z \"$url\" || ! test -d \"$path\"/.git\n>  \t\tthen\n> @@ -380,6 +416,8 @@ cmd_status()\n>  \t\t\tsay \"+$sha1 $path$revname\"\n>  \t\tfi\n>  \tdone\n> +\texit $exit_status\n> +\t}\n>  }\n\nBut here you use the simpler paradigm of setting exit_status?  Why that \ncomplicated and ugly \"total\" and \"success\" counting before?\n\nCiao,\nDscho\n\n"},{"id":"71102","messageId":"46dff0320803050920l4483d436of9b938936b47bfb4@mail.gmail.com","threadId":"12518","inReplyTo":"alpine.LSU.1.00.0803051742290.15786@racer.site","subject":"Re: [PATCH v2] git-submodule: Don't die when command fails for one submodule","fromName":"Ping Yin","fromEmail":"pkufranky@gmail.com","sentAt":"2008-03-05T17:20:39Z","receivedAt":"2008-03-05T17:20:39Z","isPatch":true,"sender":{"key":"pkufranky@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5346?v=4"},"body":"On Thu, Mar 6, 2008 at 12:48 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> Hi,\n>\n>\n>  On Wed, 5 Mar 2008, Ping Yin wrote:\n>\n>  > @@ -221,29 +229,40 @@ cmd_init()\n>  >       done\n>  >\n>  >       git ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n>  > +     {\n>  > +     total=0\n>  > +     success=0\n>  >       while read mode sha1 stage path\n>  >       do\n>  > +             total=$(( $total + 1 ))\n>  >               # Skip already registered paths\n>  > -             name=$(module_name \"$path\") || exit\n>  > +             name=$(module_name \"$path\") || continue\n>\n>  What about this case?  Was the comment misleading?\n>\n>\n>  >               url=$(git config submodule.\"$name\".url)\n>  > -             test -z \"$url\" || continue\n>  > +             test -n \"$url\" && success=$(( $success + 1 )) && continue\n>\n>  Why counting?  Why not just 'status=0 && continue'?\n>\n>\n>  >\n>  > -             git config submodule.\"$name\".url \"$url\" ||\n>  > -             die \"Failed to register url for submodule path '$path'\"\n>  > -\n>  > -             say \"Submodule '$name' ($url) registered for path '$path'\"\n>  > +             if git config submodule.\"$name\".url \"$url\"\n>  > +             then\n>  > +                     say \"Submodule '$name' ($url) registered for path '$path'\"\n>  > +             else\n>  > +                     say \"Failed to register url for submodule path '$path'\"\n>  > +                     continue\n>  > +             fi\n>  > +             success=$(( $success + 1 ))\n>  >       done\n>  > +     test $success = $total\n>  > +     }\n>  >  }\n>  >\n>  >  #\n>\n>  Note: I have not even begun to audit if one of your returns should not\n>  have been a \"status=0 && continue\" instead.\n>\n>\n>  > @@ -358,9 +392,11 @@ cmd_status()\n>  >       done\n>  >\n>  >       git ls-files --stage -- \"$@\" | grep -e '^160000 ' |\n>  > +     {\n>  > +     exit_status=0\n>  >       while read mode sha1 stage path\n>  >       do\n>  > -             name=$(module_name \"$path\") || exit\n>  > +             ! name=$(module_name \"$path\") && exit_status=1 && continue\n>  >               url=$(git config submodule.\"$name\".url)\n>  >               if test -z \"$url\" || ! test -d \"$path\"/.git\n>  >               then\n>  > @@ -380,6 +416,8 @@ cmd_status()\n>  >                       say \"+$sha1 $path$revname\"\n>  >               fi\n>  >       done\n>  > +     exit $exit_status\n>  > +     }\n>  >  }\n>\n>  But here you use the simpler paradigm of setting exit_status?  Why that\n>  complicated and ugly \"total\" and \"success\" counting before?\n>\n>  Ciao,\n>  Dscho\n>\n>\n\n'total' and 'success' is a trick which i think is simpler than\nexit_status=1/exit_status=0 in cmd_update and cmd_init.\n\nIn the while loop of these two functions, there are many tests which\nwill continue the loop after a failure. By using exit_stauts, i have\nto replace 'cmd || continue' as '! cmd && exit_status=1 && continue\".\nI don't want so many exit_status in the while loop. So i use 'total'\nand 'sucess' which count the total number of loops and the number of\nsuccessful loops separately to simplify this. The trick is as follows:\n\n'total' will plus 1 when at the beginning of a loop and success will\nplus 1 at the end of loop. If  a loop succeeds and continues in the\nmiddle of a loop, success also plus 1. Failed loops never reach the\nloop end so success will not plus 1.\n\nAfter leaving all loops,  $sucess=$total means all loops succeed and\nexit status should be 0, or exit status should be 1.\n\nIn cmd_status, the 'cmd || continue' only exists in one place, so i\nuse exit_status instead of 'success' and 'total'.\n\n\n\n\n\n-- \nPing Yin\n"}]}