{"thread":{"id":"33063","subject":"[PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","startedAt":"2013-03-04T08:41:40Z","lastAt":"2013-04-14T18:52:08Z","messageCount":25,"participants":["Eric Cousineau","Jens Lehmann","Junio C Hamano","Heiko Voigt","Phil Hord","eacousineau"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"210571","messageId":"CA+aSAWuoxZkSnRybhefnFr9ngs3tHmt6hAH4o0ebjYKvjJ-vpw@mail.gmail.com","threadId":"33063","inReplyTo":null,"subject":"[PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Eric Cousineau","fromEmail":"eacousineau@gmail.com","sentAt":"2013-03-04T08:41:40Z","receivedAt":"2013-03-04T08:41:40Z","isPatch":true,"sender":{"key":"eacousineau@gmail.com","avatar":null},"body":"In this patch, foreach --recursive acts depth-first, much like the default\nbehavior described in the patch by Imram Yousuf in this\npost .\nChanges were made so that the submodule \"Entering ...\" message was right\nnext to the output generated by the command too.\nIt also adds the --parent option for executing the command in the\nsupermodule as well.\n\nI began by adding a --depth option, to preserve the original --recursive\nbehavior, and the --parent option, and trying to get that to work. However,\nI pretty much confused myself for a while trying to straighten that out, so\nI just ended up modifying the --recursive behavior.\nIf the --recursive behavior should be preserved, I could add the --depth\noption back and only have --parent affect non-recursive and --depth\nrecursive behavior.\n\nI had kind-of implemented this behavior with aliases / bash functions\n(posted to pastebin \n, spurned by a\nquestion I asked in StackOverflow ),\nhowever I would always run into issues with escaping characters when\npassing from the bash functions to git aliases (i.e., putting \"'ello\" as an\ntest commit message). I also tried out mb14's method from the StackOverflow\npost, but I ran into the same issues.\nFigured the best way to avoid that was to cut out the extra layers.\n\nI've attached a test script to generate the tree that VonC suggested with\noutput showing the iteration.\n\n\nFrom 851d65fcfb8f49131428a57fc318af7b56416430 Mon Sep 17 00:00:00 2001\nFrom: eacousineau \nDate: Mon, 4 Mar 2013 01:08:07 -0600\nSubject: [PATCH] area: submodules Make foreach --recursive do depth-first.\n Make foreach --parent [--recursive] execute command in toplevel supermodule\n\nSigned-off-by: Eric Cousineau \n---\n git-submodule.sh | 31 ++++++++++++++++++++++++++-----\n 1 file changed, 26 insertions(+), 5 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 004c034..721c959 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -434,6 +434,9 @@ Use -f if you really want to add it.\" >&2\n cmd_foreach()\n {\n \t# parse $args after \"submodule ... foreach\".\n+\t# Gratuitous local's to prevent recursive bleeding\n+\tlocal parent=\n+\tlocal recursive=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -443,6 +446,10 @@ cmd_foreach()\n \t\t--recursive)\n \t\t\trecursive=1\n \t\t\t;;\n+\t\t--parent)\n+\t\t\t# Execute command in parent, after children commands are executed\n+\t\t\tparent=1\n+\t\t\t;;\n \t\t-*)\n \t\t\tusage\n \t\t\t;;\n@@ -464,8 +471,8 @@ cmd_foreach()\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n \t\tif test -e \"$sm_path\"/.git\n-\t\tthen\n-\t\t\tsay \"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n+\t\tthen \n+\t\t\tlocal message=\"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n \t\t\tname=$(module_name \"$sm_path\")\n \t\t\t(\n \t\t\t\tprefix=\"$prefix$sm_path/\"\n@@ -473,15 +480,29 @@ cmd_foreach()\n \t\t\t\t# we make $path available to scripts ...\n \t\t\t\tpath=$sm_path\n \t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\teval \"$@\" &&\n \t\t\t\tif test -n \"$recursive\"\n \t\t\t\tthen\n \t\t\t\t\tcmd_foreach \"--recursive\" \"$@\"\n-\t\t\t\tfi\n+\t\t\t\tfi &&\n+\t\t\t\t(\n+\t\t\t\t\t# Put message here so it stays somewhat tidy -- hopefully OK since prefixes are included\n+\t\t\t\t\tsay \"$message\"\n+\t\t\t\t\teval \"$@\"\n+\t\t\t\t)\n \t\t\t) <&3 3<&- ||\n \t\t\tdie \"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n \t\tfi\n-\tdone\n+\tdone &&\n+\t(\n+\t\tif test -n \"$parent\"\n+\t\tthen\n+\t\t\tname=$(basename \"$toplevel\")\n+\t\t\tclear_local_git_env\n+\t\t\tpath=.\n+\t\t\tsay \"$(eval_gettext \"Entering '\\$name'\")\" # Not sure of proper thing here\n+\t\t\teval \"$@\" || die \"$(eval_gettext \"Stopping at supermodule; script returned non-zero status.\")\"\n+\t\tfi\n+\t)\n }\n \n #\n-- \n1.8.2.rc1.24.g06d67b8\n\n"},{"id":"210592","messageId":"51351CF5.7010308@web.de","threadId":"33063","inReplyTo":"CA+aSAWuoxZkSnRybhefnFr9ngs3tHmt6hAH4o0ebjYKvjJ-vpw@mail.gmail.com","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-03-04T22:15:17Z","receivedAt":"2013-03-04T22:15:17Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Please don't attach your patches, see Documentation/SubmittingPatches on\nhow to post patches to this list.\n\nAm 04.03.2013 09:41, schrieb Eric Cousineau:\n> In this patch, foreach --recursive acts depth-first, much like the default\n> behavior described in the patch by Imram Yousuf in this\n> post .\n> Changes were made so that the submodule \"Entering ...\" message was right\n> next to the output generated by the command too.\n> It also adds the --parent option for executing the command in the\n> supermodule as well.\n\n>From reading the linked pages I assume a valid use case you have is:\n\n git submodule foreach --recursive 'git add -A && git commit ...'\n\nThis will currently not work because the depth first algorithm of foreach\nwill execute the command /before/ recursing deeper. You'd need it to\nexecute the command /after/ returning from the deeper level (which is what\nyour patch seems to be about).\n\n> I began by adding a --depth option, to preserve the original --recursive\n> behavior, and the --parent option, and trying to get that to work. However,\n> I pretty much confused myself for a while trying to straighten that out, so\n> I just ended up modifying the --recursive behavior.\n> If the --recursive behavior should be preserved, I could add the --depth\n> option back and only have --parent affect non-recursive and --depth\n> recursive behavior.\n\nI would rather not change default behavior without having a *very* good\nreason to do so (and I'm not sure what you need the --depth option for).\n\nWhat we currently get from your example is:\n Entering 'a'\n Entering 'a/b'\n Entering 'a/b/d'\n Entering 'a/c'\n Entering 'b'\n Entering 'b/d'\n Entering 'c'\n Entering 'd'\nMe thinks this is what most users would expect of a recursion, enter each\nlevel before descending into the next.\n\nFor your use case you'd need to have:\n Entering 'a/b/d'\n Entering 'a/b'\n Entering 'a/c'\n Entering 'a'\n Entering 'b/d'\n Entering 'b'\n Entering 'c'\n Entering 'd'\n(Please note that this is still depth-first)\n\nI won't object to adding an option to foreach that will execute the command\nafter recursing (but I'm not convinced --parent is a very good name for that).\n\n> I had kind-of implemented this behavior with aliases / bash functions\n> (posted to pastebin \n> , spurned by a\n> question I asked in StackOverflow ),\n> however I would always run into issues with escaping characters when\n> passing from the bash functions to git aliases (i.e., putting \"'ello\" as an\n> test commit message). I also tried out mb14's method from the StackOverflow\n> post, but I ran into the same issues.\n> Figured the best way to avoid that was to cut out the extra layers.\n\nIt seems to be really hard to do what you have in mind with shell commands\nor aliases, which is a good point for adding such an option to foreach. But\nI don't see a reason why we would want to change the current default, which\nis what your RFC proposes.\n"},{"id":"210595","messageId":"7vhakqwz1e.fsf@alter.siamese.dyndns.org","threadId":"33063","inReplyTo":"51351CF5.7010308@web.de","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-04T23:00:45Z","receivedAt":"2013-03-04T23:00:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> Please don't attach your patches, see Documentation/SubmittingPatches on\n> how to post patches to this list.\n>\n> Am 04.03.2013 09:41, schrieb Eric Cousineau:\n>> In this patch, foreach --recursive acts depth-first, much like the default\n>> behavior described in the patch by Imram Yousuf in this\n>> post .\n>> Changes were made so that the submodule \"Entering ...\" message was right\n>> next to the output generated by the command too.\n>> It also adds the --parent option for executing the command in the\n>> supermodule as well.\n>\n> From reading the linked pages I assume a valid use case you have is:\n>\n> git submodule foreach --recursive 'git add -A && git commit ...'\n>\n> This will currently not work because the depth first algorithm of foreach\n> will execute the command /before/ recursing deeper. You'd need it to\n> execute the command /after/ returning from the deeper level (which is what\n> your patch seems to be about).\n> ...\n> What we currently get from your example is:\n> Entering 'a'\n> Entering 'a/b'\n> Entering 'a/b/d'\n> ...\n> Entering 'c'\n> Entering 'd'\n> Me thinks this is what most users would expect of a recursion, enter each\n> level before descending into the next.\n>\n> For your use case you'd need to have:\n> Entering 'a/b/d'\n> Entering 'a/b'\n> Entering 'a/c'\n> ...\n> Entering 'c'\n> Entering 'd'\n> (Please note that this is still depth-first)\n>\n> I won't object to adding an option to foreach that will execute the command\n> after recursing (but I'm not convinced --parent is a very good name for that).\n\nAre you comparing pre-order vs post-order traversal?\n\nBoth can be useful depending on what you are trying to achieve. You\nneed a pre-order traversal (i.e. you \"visit\" and perform some action\non the node and then descend into its children) if you need to do\nsome preparation before you visit deeper levels; you need a\npost-order traversal (i.e. you \"visit\" and perform some action on\nthe node after you have done all its children) if you know you will\nbe readly only after you are done with all your children.\n\nYou can throw in in-order traversal to the mix (i.e. you \"visit\" and\nperform some action on the node after visiting some but not all of\nyour children and then continue visiting the remainder of your\nchildren), but I do not know what practical value you would get out\nof it.\n\nSo if you want a single boolean to toggle between the current\nbehaviour and the other one, it would be --post-order. But you may\nat least want to consider pros and cons of allowing users to give\ntwo separate commands, one for the pre-order visitation (which is\nthe current \"command\") and the other for the post-order\nvisitation. Being able to run both might turn out to be useful.\n"},{"id":"210606","messageId":"51358488.2080005@gmail.com","threadId":"33063","inReplyTo":"7vhakqwz1e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Eric Cousineau","fromEmail":"eacousineau@gmail.com","sentAt":"2013-03-05T05:37:12Z","receivedAt":"2013-03-05T05:37:12Z","isPatch":true,"sender":{"key":"eacousineau@gmail.com","avatar":null},"body":"git-submodule.sh: In foreach, make '-post-order' yield post-order \ntraversal and\n'--include-super' execute commands at the top-level supermodule, with \nboth of these\noptions compatible with '--recursive'.\n\nSigned-off-by: Eric Cousineau \n---\nSorry about missing the part about not included MIME attachments, hope \nthis is in a better format now.\nJens, I changed the '--parent' option to '--include-super' which is \nhopefully less vague.\nJunio, you made an excellent point about both being useful. In \nparticular, I overlooked the case\nfor doing a submodule pull / update (if, for whatever reason, it is more \nconvenient than a submodule\nupdate, maybe for merging). In that case, you might want to initialize \nnew submodules and ignore the\nold ones, instead of wasting time on them with a post-order traversal pull.\nI've implemented your suggestions to have a boolean '--post-order' \noption, and made the '--include-super'\noption compatible with it. This way, the original behavior of 'foreach' \nis preserved.\n\nI've updated the test and uploaded it to pastebin: \nhttp://pastebin.com/BgZNzFpi\n\n git-submodule.sh | 102 \n+++++++++++++++++++++++++++++++++++++++++--------------\n 1 file changed, 77 insertions(+), 25 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 004c034..652bea0 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -10,7 +10,7 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] \n[--name ] [--reference ...]\n or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] \n[-f|--force] [--rebase] [--reference ] [--merge] \n[--recursive] [--] [...]\n or: $dashless [--quiet] summary [--cached|--files] [--summary-limit \n] [commit] [--] [...]\n- or: $dashless [--quiet] foreach [--recursive] \n+ or: $dashless [--quiet] foreach [--recursive] [--include-super] \n[--post-order] \n or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n OPTIONS_SPEC=\n . git-sh-setup\n@@ -434,6 +434,8 @@ Use -f if you really want to add it.\" >&2\n cmd_foreach()\n {\n # parse $args after \"submodule ... foreach\".\n+ # Gratuitous (empty) local's to prevent recursive bleeding\n+ local include_super= recursive= post_order=\n while test $# -ne 0\n do\n case \"$1\" in\n@@ -443,6 +445,12 @@ cmd_foreach()\n --recursive)\n recursive=1\n ;;\n+ --post-order)\n+ post_order=1\n+ ;;\n+ --include-super)\n+ include_super=1\n+ ;;\n -*)\n usage\n ;;\n@@ -453,35 +461,79 @@ cmd_foreach()\n shift\n done\n\n- toplevel=$(pwd)\n+ if test -n \"$recursive\"\n+ then\n+ local recursive_flags=\"--recursive\"\n+ if test -n \"$post_order\"\n+ then\n+ recursive_flags=\"$recursive_flags --post-order\"\n+ fi\n+ fi\n+\n+ local toplevel=$(pwd)\n\n # dup stdin so that it can be restored when running the external\n # command in the subshell (and a recursive call to this function)\n exec 3<&0\n+\n+ # Use nested functions\n+ super_eval() {\n+ name=$(basename \"$toplevel\")\n+ clear_local_git_env\n+ path=.\n+ say \"$(eval_gettext \"Entering '\\$name'\")\" # Not sure of proper \nthing here\n+ eval \"$@\" || die \"$(eval_gettext \"Stopping at supermodule; \nscript returned non-zero status.\")\"\n+ }\n\n- module_list |\n- while read mode sha1 stage sm_path\n- do\n- die_if_unmatched \"$mode\"\n- if test -e \"$sm_path\"/.git\n- then\n- say \"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n- name=$(module_name \"$sm_path\")\n- (\n- prefix=\"$prefix$sm_path/\"\n- clear_local_git_env\n- # we make $path available to scripts ...\n- path=$sm_path\n- cd \"$sm_path\" &&\n- eval \"$@\" &&\n- if test -n \"$recursive\"\n- then\n- cmd_foreach \"--recursive\" \"$@\"\n- fi\n- ) <&3 3<&- ||\n- die \"$(eval_gettext \"Stopping at '\\$sm_path'; script \nreturned non-zero status.\")\"\n- fi\n- done\n+ if test -n \"$include_super\" -a -z \"$post_order\"\n+ then\n+ super_eval \"$@\"\n+ fi &&\n+ (\n+ module_list |\n+ while read mode sha1 stage sm_path\n+ do\n+ die_if_unmatched \"$mode\"\n+ if test -e \"$sm_path\"/.git\n+ then\n+ local name prefix path message epitaph\n+ message=\"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n+ epitaph=\"$(eval_gettext \"Stopping at '\\$sm_path'; \nscript returned non-zero status.\")\"\n+ name=$(module_name \"$sm_path\")\n+ (\n+ prefix=\"$prefix$sm_path/\"\n+ clear_local_git_env\n+ # we make $path available to scripts ...\n+ path=$sm_path\n+\n+ sm_eval() {\n+ say \"$message\"\n+ eval \"$@\" || die \"$epitaph\"\n+ }\n+\n+ cd \"$sm_path\" &&\n+ if test -z \"$post_order\"\n+ then\n+ sm_eval \"$@\"\n+ fi &&\n+ if test -n \"$recursive\"\n+ then\n+ cmd_foreach $recursive_flags \"$@\"\n+ fi &&\n+ if test -n \"$post_order\"\n+ then\n+ sm_eval \"$@\"\n+ fi\n+ # Since the (...) seems to limit exit's scope, make \nsure to kill things here if something goes awry\n+ # (the `|| exit 1` at the end)\n+ ) <&3 3<&- || exit 1\n+ fi\n+ done\n+ ) &&\n+ if test -n \"$include_super\" -a -n \"$post_order\"\n+ then\n+ super_eval \"$@\"\n+ fi\n }\n\n #\n-- \n1.8.2.rc1.24.g06d67b8.dirty\n"},{"id":"210611","messageId":"20130305075901.GB4677@sandbox-ub","threadId":"33063","inReplyTo":"7vhakqwz1e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-03-05T07:59:02Z","receivedAt":"2013-03-05T07:59:02Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Mon, Mar 04, 2013 at 03:00:45PM -0800, Junio C Hamano wrote:\n> So if you want a single boolean to toggle between the current\n> behaviour and the other one, it would be --post-order. But you may\n> at least want to consider pros and cons of allowing users to give\n> two separate commands, one for the pre-order visitation (which is\n> the current \"command\") and the other for the post-order\n> visitation. Being able to run both might turn out to be useful.\n\nI second that. Having a --post-order= switch will give\nus much more flexibility. For ease of use we could allow --post-order\nwithout command to switch the meaning of the main command.\n\nSo a final solution would have these switches:\n\ngit submodule foreach ... [--pre-order[=]] [--post-order[=]] []\n\nIf only --pre-order without argument is given the command will be\nexecuted pre-order. If only --post-order the command will be executed\npost-order. If both are given its an error and so on...\n\nThere are some combinations we would need to catch as errors but this\ndesign should allow a step by step implementation:\n\n\t1. just the --post-order switch\n\t2. --post-order with argument switch\n\t3. --pre-order (including argument) for symmetry of usage\n\nCheers Heiko\n"},{"id":"210635","messageId":"7vhakpvnex.fsf@alter.siamese.dyndns.org","threadId":"33063","inReplyTo":"20130305075901.GB4677@sandbox-ub","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-05T16:09:26Z","receivedAt":"2013-03-05T16:09:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Heiko Voigt writes:\n\n> On Mon, Mar 04, 2013 at 03:00:45PM -0800, Junio C Hamano wrote:\n>> So if you want a single boolean to toggle between the current\n>> behaviour and the other one, it would be --post-order. But you may\n>> at least want to consider pros and cons of allowing users to give\n>> two separate commands, one for the pre-order visitation (which is\n>> the current \"command\") and the other for the post-order\n>> visitation. Being able to run both might turn out to be useful.\n>\n> I second that. Having a --post-order= switch will give\n> us much more flexibility. For ease of use we could allow --post-order\n> without command to switch the meaning of the main command.\n>\n> So a final solution would have these switches:\n>\n> git submodule foreach ... [--pre-order[=]] [--post-order[=]] []\n>\n> If only --pre-order without argument is given the command will be\n> executed pre-order. If only --post-order the command will be executed\n> post-order. If both are given its an error and so on...\n>\n> There are some combinations we would need to catch as errors but this\n> design should allow a step by step implementation:\n>\n> \t1. just the --post-order switch\n> \t2. --post-order with argument switch\n> \t3. --pre-order (including argument) for symmetry of usage\n\nYeah, I think I can agree with that direction, and Eric's patch\ncould be that first step of the three-step progression, without\npainting us into a corner we cannot get out of when we want to\nadvance to 2 and 3 later.\n\nI was more interested in the design aspect and I didn't look at the\nactual patch text, though.\n"},{"id":"210639","messageId":"CA+aSAWvQj2wuJX3ENNn7n_pMJjA6nauve5BLRugiTuhWN25Ctg@mail.gmail.com","threadId":"33063","inReplyTo":"7vhakpvnex.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Eric Cousineau","fromEmail":"eacousineau@gmail.com","sentAt":"2013-03-05T16:42:05Z","receivedAt":"2013-03-05T16:42:05Z","isPatch":true,"sender":{"key":"eacousineau@gmail.com","avatar":null},"body":"On Tue, Mar 5, 2013 at 10:09 AM, Junio C Hamano wrote:\n> Heiko Voigt writes:\n>\n>> On Mon, Mar 04, 2013 at 03:00:45PM -0800, Junio C Hamano wrote:\n>>> So if you want a single boolean to toggle between the current\n>>> behaviour and the other one, it would be --post-order. But you may\n>>> at least want to consider pros and cons of allowing users to give\n>>> two separate commands, one for the pre-order visitation (which is\n>>> the current \"command\") and the other for the post-order\n>>> visitation. Being able to run both might turn out to be useful.\n>>\n>> I second that. Having a --post-order= switch will give\n>> us much more flexibility. For ease of use we could allow --post-order\n>> without command to switch the meaning of the main command.\n>>\n>> So a final solution would have these switches:\n>>\n>> git submodule foreach ... [--pre-order[=]] [--post-order[=]] []\n>>\n>> If only --pre-order without argument is given the command will be\n>> executed pre-order. If only --post-order the command will be executed\n>> post-order. If both are given its an error and so on...\n>>\n>> There are some combinations we would need to catch as errors but this\n>> design should allow a step by step implementation:\n>>\n>> 1. just the --post-order switch\n>> 2. --post-order with argument switch\n>> 3. --pre-order (including argument) for symmetry of usage\n>\n> Yeah, I think I can agree with that direction, and Eric's patch\n> could be that first step of the three-step progression, without\n> painting us into a corner we cannot get out of when we want to\n> advance to 2 and 3 later.\n>\n> I was more interested in the design aspect and I didn't look at the\n> actual patch text, though.\n\nWould these be the correct behaviors of Heiko's implementation?\n\ngit submodule foreach # Empty command, pre-order\ngit submodule foreach --pre-order # Same behavior\ngit submodule foreach --post-order # Empty command, post-order\ngit submodule foreach 'frotz' # Do 'frotz' pre-order in each submodule\ngit submodule foreach --post-order 'frotz' # Do 'frotz' post-order in\neach submodule\ngit submodule foreach --pre-order='frotz' --post-order='shimmy' # Do\n'frotz' pre-order and 'shimmy' post-order in each submodule\ngit submodule foreach --post-order='shimmy' 'frotz' # Invalid usage of\nthe command\ngit submodule foreach --post-order --pre-order #\n\nIt should not be too hard to have this functionality affect the\n--include-super command as well.\n\nAnd would it be worth it to abstract this traversal to expose it to\nother commands, such as 'update', to consolidate the code some?\nI think Imram was doing something like that in his post.\n\n- Eric\n"},{"id":"210650","messageId":"7vboaxu23y.fsf@alter.siamese.dyndns.org","threadId":"33063","inReplyTo":"CA+aSAWvQj2wuJX3ENNn7n_pMJjA6nauve5BLRugiTuhWN25Ctg@mail.gmail.com","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-05T18:34:57Z","receivedAt":"2013-03-05T18:34:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Cousineau writes:\n\n> Would these be the correct behaviors of Heiko's implementation?\n\nI do not think Heiko already has an implementation, but let's try to\nsee how each example makes sense.\n\n> git submodule foreach # Empty command, pre-order\n> git submodule foreach --pre-order # Same behavior\n> git submodule foreach --post-order # Empty command, post-order\n\nOK. The last one shows \"I am here\" output differently from the\nother two, but otherwise they are all no-op.\n\n> git submodule foreach 'frotz' # Do 'frotz' pre-order in each submodule\n\nOK. And it would be the same if you said either one of:\n\n\tgit submodule foreach --pre-order 'frotz'\n\tgit submodule foreach --pre-order='frotz'\n\n> git submodule foreach --post-order 'frotz' # Do 'frotz' post-order in\n> each submodule\n\nOK.\n\n> git submodule foreach --pre-order='frotz' --post-order='shimmy' # Do\n> 'frotz' pre-order and 'shimmy' post-order in each submodule\n\nOK.\n\n> git submodule foreach --post-order='shimmy' 'frotz' # Invalid usage of\n> the command\n\nI would expect this to behave exactly the same as:\n\n\tgit submodule foreach \\\n \t--post-order=shimmy \\\n --pre-order=frotz\n\n> git submodule foreach --post-order --pre-order #\n\nI expect it to behave exactly the same as:\n\n\tgit submodule foreach --post-order=: --pre-order=:\n\n> It should not be too hard to have this functionality affect the\n> --include-super command as well.\n\nI would assume that\n\n\tgit submodule foreach --pre-order=A --post-order=B --include-super\n\nwould be identical to running\n\n\tA &&\n git submodule foreach --pre-order=A --post-order=B &&\n B\n\nI am not entirely convinced we would want --include-super in the\nfirst place, though. It does not belong to \"submodule foreach\";\nit is doing something _outside_ the submoudules.\n"},{"id":"210654","messageId":"51365AB6.2010602@web.de","threadId":"33063","inReplyTo":"7vboaxu23y.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-03-05T20:51:02Z","receivedAt":"2013-03-05T20:51:02Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 05.03.2013 19:34, schrieb Junio C Hamano:\n> Eric Cousineau writes:\n> \n>> Would these be the correct behaviors of Heiko's implementation?\n> \n> I do not think Heiko already has an implementation, but let's try to\n> see how each example makes sense.\n> \n>> git submodule foreach # Empty command, pre-order\n>> git submodule foreach --pre-order # Same behavior\n>> git submodule foreach --post-order # Empty command, post-order\n> \n> OK. The last one shows \"I am here\" output differently from the\n> other two, but otherwise they are all no-op.\n> \n>> git submodule foreach 'frotz' # Do 'frotz' pre-order in each submodule\n> \n> OK. And it would be the same if you said either one of:\n> \n> \tgit submodule foreach --pre-order 'frotz'\n> \tgit submodule foreach --pre-order='frotz'\n> \n>> git submodule foreach --post-order 'frotz' # Do 'frotz' post-order in\n>> each submodule\n> \n> OK.\n> \n>> git submodule foreach --pre-order='frotz' --post-order='shimmy' # Do\n>> 'frotz' pre-order and 'shimmy' post-order in each submodule\n> \n> OK.\n> \n>> git submodule foreach --post-order='shimmy' 'frotz' # Invalid usage of\n>> the command\n> \n> I would expect this to behave exactly the same as:\n> \n> \tgit submodule foreach \\\n> \t--post-order=shimmy \\\n> --pre-order=frotz\n> \n>> git submodule foreach --post-order --pre-order #\n> \n> I expect it to behave exactly the same as:\n> \n> \tgit submodule foreach --post-order=: --pre-order=:\n\nI'd favor to just drop the --pre-order option and do this:\n\n foreach [--recursive] [--post-order ] []\n\nMe thinks pre-order is a sane default and we shouldn't add an\nexplicit option for that. And even with current Git you can\nsimply give no command at all and it'll show you all the\nsubmodules it enters without doing anything in them, so we'd\nonly need to add the --post-order handling anyway (and fix the\nsynopsis by adding square brackets around the command while at\nit, as that is optional).\n\n>> It should not be too hard to have this functionality affect the\n>> --include-super command as well.\n> \n> I would assume that\n> \n> \tgit submodule foreach --pre-order=A --post-order=B --include-super\n> \n> would be identical to running\n> \n> \tA &&\n> git submodule foreach --pre-order=A --post-order=B &&\n> B\n>\n> I am not entirely convinced we would want --include-super in the\n> first place, though. It does not belong to \"submodule foreach\";\n> it is doing something _outside_ the submoudules.\n\nI totally agree with that. First, adding --include-super does not\nbelong into the --post-order patch at all, as that is a different\ntopic (even though it belongs to the same use case Eric has). Also\nthe reason why we are thinking about adding the --post-order option\nIMO cuts the other way for --include-super: It is so easy to do\nthat yourself I'm not convinced we should add an extra option to\nforeach for that, especially as it has nothing to do with submodules.\nSo I think we should just drop --include-super.\n"},{"id":"210657","messageId":"CABURp0psgofX=ean+KKooN74pF4Ns-gDGt68vc5Exs6NmDRUyw@mail.gmail.com","threadId":"33063","inReplyTo":"51365AB6.2010602@web.de","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-03-05T21:17:05Z","receivedAt":"2013-03-05T21:17:05Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann wrote:\n> Am 05.03.2013 19:34, schrieb Junio C Hamano:\n>> Eric Cousineau writes:\n>>> ...\n>> I am not entirely convinced we would want --include-super in the\n>> first place, though. It does not belong to \"submodule foreach\";\n>> it is doing something _outside_ the submoudules.\n>\n> I totally agree with that. First, adding --include-super does not\n> belong into the --post-order patch at all, as that is a different\n> topic (even though it belongs to the same use case Eric has). Also\n> the reason why we are thinking about adding the --post-order option\n> IMO cuts the other way for --include-super: It is so easy to do\n> that yourself I'm not convinced we should add an extra option to\n> foreach for that, especially as it has nothing to do with submodules.\n> So I think we should just drop --include-super.\n\nI agree it should not be part of this commit, but I've often found\nmyself in need of an --include-super switch. To me,\ngit-submodule-foreach means \"visit all my .git repos in this project\nand execute $cmd\". It's a pity that the super-project is considered a\nsecond-class citizen in this regard.\n\nI have to do this sometimes:\n\n ${cmd} && git submodule foreach --recursive '${cmd}'\n\nI often forget the first part in scripts, though, and I've seen others\ndo it too. I usually create a function for it in git-heavy scripts.\n\nIn a shell, it usually goes like this:\n\n git submodule foreach --recursive '${cmd}'\n {30-ish}\n\nIt'd be easier if I could just include a switch for this, and maybe\neven create an alias for it. But maybe this is different command\naltogether.\n"},{"id":"210903","messageId":"513B7D08.20406@web.de","threadId":"33063","inReplyTo":"CABURp0psgofX=ean+KKooN74pF4Ns-gDGt68vc5Exs6NmDRUyw@mail.gmail.com","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-03-09T18:18:48Z","receivedAt":"2013-03-09T18:18:48Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 05.03.2013 22:17, schrieb Phil Hord:\n> On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann wrote:\n>> Am 05.03.2013 19:34, schrieb Junio C Hamano:\n>>> Eric Cousineau writes:\n>>>> ...\n>>> I am not entirely convinced we would want --include-super in the\n>>> first place, though. It does not belong to \"submodule foreach\";\n>>> it is doing something _outside_ the submoudules.\n>>\n>> I totally agree with that. First, adding --include-super does not\n>> belong into the --post-order patch at all, as that is a different\n>> topic (even though it belongs to the same use case Eric has). Also\n>> the reason why we are thinking about adding the --post-order option\n>> IMO cuts the other way for --include-super: It is so easy to do\n>> that yourself I'm not convinced we should add an extra option to\n>> foreach for that, especially as it has nothing to do with submodules.\n>> So I think we should just drop --include-super.\n> \n> I agree it should not be part of this commit, but I've often found\n> myself in need of an --include-super switch. To me,\n> git-submodule-foreach means \"visit all my .git repos in this project\n> and execute $cmd\". It's a pity that the super-project is considered a\n> second-class citizen in this regard.\n\nHmm, for me the super-project is a very natural second-class citizen\nto \"git *submodule* foreach\". But also I understand that sometimes the\nuser wants to apply a command to superproject and submodules alike (I\njust recently did exactly that with \"git gc\" on our build server).\n\n> I have to do this sometimes:\n> \n> ${cmd} && git submodule foreach --recursive '${cmd}'\n> \n> I often forget the first part in scripts, though, and I've seen others\n> do it too. I usually create a function for it in git-heavy scripts.\n> \n> In a shell, it usually goes like this:\n> \n> git submodule foreach --recursive '${cmd}'\n> {30-ish}\n> \n> It'd be easier if I could just include a switch for this, and maybe\n> even create an alias for it. But maybe this is different command\n> altogether.\n\nAre you sure you wouldn't forget to provide such a switch too? ;-)\n\nI'm still not convinced we should add a new switch, as it can easily\nbe achieved by adding \"${cmd} &&\" to your scripts. And on the command\nline you could use an alias like this one to achieve that:\n\n[alias]\n\trecurse = !sh -c \\\"$@ && git submodule foreach --recursive $@\\\"\n"},{"id":"211056","messageId":"20130311164628.GA4167@sandbox-ub.fritz.box","threadId":"33063","inReplyTo":"513B7D08.20406@web.de","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Heiko Voigt","fromEmail":"hvoigt@hvoigt.net","sentAt":"2013-03-11T16:46:30Z","receivedAt":"2013-03-11T16:46:30Z","isPatch":true,"sender":{"key":"hvoigt@hvoigt.net","avatar":"https://avatars.githubusercontent.com/u/184958?v=4"},"body":"On Sat, Mar 09, 2013 at 07:18:48PM +0100, Jens Lehmann wrote:\n> Am 05.03.2013 22:17, schrieb Phil Hord:\n> > In a shell, it usually goes like this:\n> > \n> > git submodule foreach --recursive '${cmd}'\n> > {30-ish}\n> > \n> > It'd be easier if I could just include a switch for this, and maybe\n> > even create an alias for it. But maybe this is different command\n> > altogether.\n> \n> Are you sure you wouldn't forget to provide such a switch too? ;-)\n> \n> I'm still not convinced we should add a new switch, as it can easily\n> be achieved by adding \"${cmd} &&\" to your scripts. And on the command\n> line you could use an alias like this one to achieve that:\n> \n> [alias]\n> \trecurse = !sh -c \\\"$@ && git submodule foreach --recursive $@\\\"\n\nI also think it would be useful to have a switch (or even configuration)\nto include the superproject.\n\nThe following (quite typical) use cases come to my mind:\n\n# Assuming some not yet existing configuration values\ngit config submodule.recursive true\ngit config submodule.includeSuper true\n\n# commit your work over the whole tree into one branch\ngit submodule foreach git checkout -b hv/my-super-cool-feature\ngit submodule foreach --post-order git commit -a -m \"DRAFT: finished work for today\"\ngit submodule foreach git push hvoigt hv/my-super-cool-feature\n\n# cleanup\ngit submodule foreach git clean -xfd\n\n# reset\ngit submodule foreach git reset --hard\n\n...\n\nAssuming you have a submodule heavy project and you work on multiple\nsubmodules including the superproject. These are quite typical commands\nyou would use during development of your feature I imagine. Once you are\nfinished you need to get your feature upstream by the individual\nsubmodule rules.\n\nOn a feature branch during development there is nothing wrong in simply\ndoing full cross-submodule project commits.\n\nAt some point we will probably extend the above commands with a\n--recurse-submodules switch but until then this is a good substitute so\nwhy not have a --include-super maybe even as a configuration option ?\n\nCheers Heiko\n"},{"id":"211137","messageId":"CABURp0qBA6myf7_SuaxJSrePJHmh2v-nmtLRzKTtgAJxLkJ-tQ@mail.gmail.com","threadId":"33063","inReplyTo":"513B7D08.20406@web.de","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-03-12T16:01:43Z","receivedAt":"2013-03-12T16:01:43Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann wrote:\n> Am 05.03.2013 22:17, schrieb Phil Hord:\n>> On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann wrote:\n>>> Am 05.03.2013 19:34, schrieb Junio C Hamano:\n>>>> Eric Cousineau writes:\n>>>>> ...\n>>>> I am not entirely convinced we would want --include-super in the\n>>>> first place, though. It does not belong to \"submodule foreach\";\n>>>> it is doing something _outside_ the submoudules.\n>>>\n>>> I totally agree with that. First, adding --include-super does not\n>>> belong into the --post-order patch at all, as that is a different\n>>> topic (even though it belongs to the same use case Eric has). Also\n>>> the reason why we are thinking about adding the --post-order option\n>>> IMO cuts the other way for --include-super: It is so easy to do\n>>> that yourself I'm not convinced we should add an extra option to\n>>> foreach for that, especially as it has nothing to do with submodules.\n>>> So I think we should just drop --include-super.\n>>\n>> I agree it should not be part of this commit, but I've often found\n>> myself in need of an --include-super switch. To me,\n>> git-submodule-foreach means \"visit all my .git repos in this project\n>> and execute $cmd\". It's a pity that the super-project is considered a\n>> second-class citizen in this regard.\n>\n> Hmm, for me the super-project is a very natural second-class citizen\n> to \"git *submodule* foreach\". But also I understand that sometimes the\n> user wants to apply a command to superproject and submodules alike (I\n> just recently did exactly that with \"git gc\" on our build server).\n>\n>> I have to do this sometimes:\n>>\n>> ${cmd} && git submodule foreach --recursive '${cmd}'\n>>\n>> I often forget the first part in scripts, though, and I've seen others\n>> do it too. I usually create a function for it in git-heavy scripts.\n>>\n>> In a shell, it usually goes like this:\n>>\n>> git submodule foreach --recursive '${cmd}'\n>> {30-ish}\n>>\n>> It'd be easier if I could just include a switch for this, and maybe\n>> even create an alias for it. But maybe this is different command\n>> altogether.\n>\n> Are you sure you wouldn't forget to provide such a switch too? ;-)\n\nNo. However, when I remember to add the switch, my shell history will\nremember it for me. This does not happen naturally for me in the\n\"{30-ish}...\" workflow.\n\nI also hope this switch grows up into a configuration option someday.\nOr maybe a completely different command, like I said before; because I\nactually think it could be dangerous as a configuration option since\nit would have drastic consequences for users executing scripts or\ncommands in other users' environments.\n\n\n> I'm still not convinced we should add a new switch, as it can easily\n> be achieved by adding \"${cmd} &&\" to your scripts. And on the command\n> line you could use an alias like this one to achieve that:\n>\n> [alias]\n> recurse = !sh -c \\\"$@ && git submodule foreach --recursive $@\\\"\n\nYes, making the feature itself a 2nd-class citizen. :-)\n\nBut this alias also denies me the benefit of the --post-order option.\nFor 'git recurse git push', for example, I wouldn't want the\nsuperproject push to occur first; I would want it to occur last after\nthe submodules have been successfully pushed.\n\nI agree this should go in some other commit, but I do not think it is\nso trivial it should never be considered as a feature for git. That's\nall I'm trying to say.\n\nPhil\n"},{"id":"211275","messageId":"51416E9F.7000604@gmail.com","threadId":"33063","inReplyTo":"CABURp0qBA6myf7_SuaxJSrePJHmh2v-nmtLRzKTtgAJxLkJ-tQ@mail.gmail.com","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Eric Cousineau","fromEmail":"eacousineau@gmail.com","sentAt":"2013-03-14T06:30:55Z","receivedAt":"2013-03-14T06:30:55Z","isPatch":true,"sender":{"key":"eacousineau@gmail.com","avatar":null},"body":" From 59fb432e17a1aae9de26bbaaca7f09cc7f03b471 Mon Sep 17 00:00:00 2001\nFrom: Eric Cousineau \nDate: Thu, 14 Mar 2013 01:19:53 -0500\nSubject: [PATCH] submodule-foreach: Added in --post-order= per Jens\n Lehmann's suggestion\n\nSigned-off-by: Eric Cousineau \n---\nMade the scope of the patch only relate to --post-order.\nWould we want to rename this to just --post= ?\n\nAnywho, here it is running in a test setup, where the structure is:\na\n- b\n- - d\n- c\n\n$ git submodule foreach --recursive --post-order 'echo Post $name' 'echo \nPre $path'\nEntering 'b'\nPre b\nEntering 'b/d'\nPre d\nEntering 'b/d'\nPost d\nEntering 'b'\nPost b\nEntering 'c'\nPre c\nEntering 'c'\nPost c\n\nAn interesting note is that it fails with 'git submodule foreach \n--post-order', but not 'git submodule foreach --post-order=', since it \nsimply interprets that as an empty command.\nIf that is important, I could add in a check for $# when parsing the \nargument for --post-order=*.\n\n git-submodule.sh | 39 ++++++++++++++++++++++++++++++++++-----\n 1 file changed, 34 insertions(+), 5 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 004c034..9b70bc2 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -10,7 +10,7 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] \n[--name ] [--reference ...]\n or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] \n[-f|--force] [--rebase] [--reference ] [--merge] \n[--recursive] [--] [...]\n or: $dashless [--quiet] summary [--cached|--files] [--summary-limit \n] [commit] [--] [...]\n- or: $dashless [--quiet] foreach [--recursive] \n+ or: $dashless [--quiet] foreach [--recursive] \n[--post-order=] \n or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n OPTIONS_SPEC=\n . git-sh-setup\n@@ -434,6 +434,8 @@ Use -f if you really want to add it.\" >&2\n cmd_foreach()\n {\n # parse $args after \"submodule ... foreach\".\n+ # Gratuitous (empty) local's to prevent recursive bleeding\n+ local recursive= post_order=\n while test $# -ne 0\n do\n case \"$1\" in\n@@ -443,6 +445,15 @@ cmd_foreach()\n --recursive)\n recursive=1\n ;;\n+ --post-order)\n+ test \"$#\" = \"1\" && usage\n+ post_order=\"$2\"\n+ shift\n+ ;;\n+ --post-order=*)\n+ # Will skip empty commands\n+ post_order=${1#*=}\n+ ;;\n -*)\n usage\n ;;\n@@ -453,7 +464,7 @@ cmd_foreach()\n shift\n done\n\n- toplevel=$(pwd)\n+ local toplevel=$(pwd)\n\n # dup stdin so that it can be restored when running the external\n # command in the subshell (and a recursive call to this function)\n@@ -465,18 +476,36 @@ cmd_foreach()\n die_if_unmatched \"$mode\"\n if test -e \"$sm_path\"/.git\n then\n- say \"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n+ local name prefix path message epitaph\n+ message=\"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n+ epitaph=\"$(eval_gettext \"Stopping at '\\$sm_path'; script \nreturned non-zero status.\")\"\n name=$(module_name \"$sm_path\")\n (\n prefix=\"$prefix$sm_path/\"\n clear_local_git_env\n # we make $path available to scripts ...\n path=$sm_path\n+\n+ sm_eval() {\n+ say \"$message\"\n+ eval \"$@\" || die \"$epitaph\"\n+ }\n+\n cd \"$sm_path\" &&\n- eval \"$@\" &&\n+ sm_eval \"$@\" &&\n if test -n \"$recursive\"\n then\n- cmd_foreach \"--recursive\" \"$@\"\n+ if test -n \"$post_order\"\n+ then\n+ # Tried keeping flags as a variable, but was \nhaving difficulty\n+ cmd_foreach --recursive --post-order \n\"$post_order\" \"$@\"\n+ else\n+ cmd_foreach --recursive \"$@\"\n+ fi\n+ fi &&\n+ if test -n \"$post_order\"\n+ then\n+ sm_eval \"$post_order\"\n fi\n ) <&3 3<&- ||\n die \"$(eval_gettext \"Stopping at '\\$sm_path'; script \nreturned non-zero status.\")\"\n-- \n1.8.2.rc1.24.g06d67b8.dirty\n"},{"id":"211606","messageId":"514782D3.5060200@web.de","threadId":"33063","inReplyTo":"CABURp0qBA6myf7_SuaxJSrePJHmh2v-nmtLRzKTtgAJxLkJ-tQ@mail.gmail.com","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-03-18T21:10:43Z","receivedAt":"2013-03-18T21:10:43Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 12.03.2013 17:01, schrieb Phil Hord:\n> On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann wrote:\n>> Am 05.03.2013 22:17, schrieb Phil Hord:\n>>> On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann wrote:\n>>>> Am 05.03.2013 19:34, schrieb Junio C Hamano:\n>>>>> Eric Cousineau writes:\n>>>>>> ...\n>>>>> I am not entirely convinced we would want --include-super in the\n>>>>> first place, though. It does not belong to \"submodule foreach\";\n>>>>> it is doing something _outside_ the submoudules.\n>>>>\n>>>> I totally agree with that. First, adding --include-super does not\n>>>> belong into the --post-order patch at all, as that is a different\n>>>> topic (even though it belongs to the same use case Eric has). Also\n>>>> the reason why we are thinking about adding the --post-order option\n>>>> IMO cuts the other way for --include-super: It is so easy to do\n>>>> that yourself I'm not convinced we should add an extra option to\n>>>> foreach for that, especially as it has nothing to do with submodules.\n>>>> So I think we should just drop --include-super.\n>>>\n>>> I agree it should not be part of this commit, but I've often found\n>>> myself in need of an --include-super switch. To me,\n>>> git-submodule-foreach means \"visit all my .git repos in this project\n>>> and execute $cmd\". It's a pity that the super-project is considered a\n>>> second-class citizen in this regard.\n>>\n>> Hmm, for me the super-project is a very natural second-class citizen\n>> to \"git *submodule* foreach\". But also I understand that sometimes the\n>> user wants to apply a command to superproject and submodules alike (I\n>> just recently did exactly that with \"git gc\" on our build server).\n>>\n>>> I have to do this sometimes:\n>>>\n>>> ${cmd} && git submodule foreach --recursive '${cmd}'\n>>>\n>>> I often forget the first part in scripts, though, and I've seen others\n>>> do it too. I usually create a function for it in git-heavy scripts.\n>>>\n>>> In a shell, it usually goes like this:\n>>>\n>>> git submodule foreach --recursive '${cmd}'\n>>> {30-ish}\n>>>\n>>> It'd be easier if I could just include a switch for this, and maybe\n>>> even create an alias for it. But maybe this is different command\n>>> altogether.\n>>\n>> Are you sure you wouldn't forget to provide such a switch too? ;-)\n> \n> No. However, when I remember to add the switch, my shell history will\n> remember it for me. This does not happen naturally for me in the\n> \"{30-ish}...\" workflow.\n\nI started to use '&&' in my daily shell work for exactly that reason:\nthat the bash history remembers groups of two or more commands for me.\n\n> I also hope this switch grows up into a configuration option someday.\n> Or maybe a completely different command, like I said before; because I\n> actually think it could be dangerous as a configuration option since\n> it would have drastic consequences for users executing scripts or\n> commands in other users' environments.\n\nI agree on the possible problems a configuration option introduces.\n\n>> I'm still not convinced we should add a new switch, as it can easily\n>> be achieved by adding \"${cmd} &&\" to your scripts. And on the command\n>> line you could use an alias like this one to achieve that:\n>>\n>> [alias]\n>> recurse = !sh -c \\\"$@ && git submodule foreach --recursive $@\\\"\n> \n> Yes, making the feature itself a 2nd-class citizen. :-)\n> \n> But this alias also denies me the benefit of the --post-order option.\n> For 'git recurse git push', for example, I wouldn't want the\n> superproject push to occur first; I would want it to occur last after\n> the submodules have been successfully pushed.\n\n[alias]\n recurse-post = !sh -c \\\"git submodule foreach --recursive --post-order $@ && $@\\\"\n;-)\n\n> I agree this should go in some other commit, but I do not think it is\n> so trivial it should never be considered as a feature for git. That's\n> all I'm trying to say.\n\nI am not against adding such a functionality to Git, I'm just not\nconvinced \"git submodule foreach\" is the right command for that. I\nsuspect the \"git for-each-repo\" Lars proposed earlier this year might\nbe a better choice, as that could also recurse into other repos which\naren't registered as submodules. And a \"for-each-repo\" to me looks\nlike a command which could include the superproject too (at least when\ntold to do so with an option).\n"},{"id":"211609","messageId":"5147863B.8010104@web.de","threadId":"33063","inReplyTo":"51416E9F.7000604@gmail.com","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-03-18T21:25:15Z","receivedAt":"2013-03-18T21:25:15Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Thanks, just a quick review before I find some time do take a\ndeeper look.\n\nAm 14.03.2013 07:30, schrieb Eric Cousineau:\n> From 59fb432e17a1aae9de26bbaaca7f09cc7f03b471 Mon Sep 17 00:00:00 2001\n> From: Eric Cousineau \n> Date: Thu, 14 Mar 2013 01:19:53 -0500\n> Subject: [PATCH] submodule-foreach: Added in --post-order= per Jens\n> Lehmann's suggestion\n> \n> Signed-off-by: Eric Cousineau \n> ---\n> Made the scope of the patch only relate to --post-order.\n> Would we want to rename this to just --post= ?\n\nHmm, while having no strong preference on that, \"post order\"\nlooks more like the correct term describing what we do here.\n\n> Anywho, here it is running in a test setup, where the structure is:\n> a\n> - b\n> - - d\n> - c\n> \n> $ git submodule foreach --recursive --post-order 'echo Post $name' 'echo Pre $path'\n> Entering 'b'\n> Pre b\n> Entering 'b/d'\n> Pre d\n> Entering 'b/d'\n> Post d\n> Entering 'b'\n> Post b\n> Entering 'c'\n> Pre c\n> Entering 'c'\n> Post c\n\nLooking good.\n\n> An interesting note is that it fails with 'git submodule foreach --post-order', but not 'git submodule foreach --post-order=', since it simply interprets that as an empty command.\n> If that is important, I could add in a check for $# when parsing the argument for --post-order=*.\n> \n> git-submodule.sh | 39 ++++++++++++++++++++++++++++++++++-----\n> 1 file changed, 34 insertions(+), 5 deletions(-)\n> \n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 004c034..9b70bc2 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -10,7 +10,7 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] [--name ] [--reference or: $dashless [--quiet] init [--] [...]\n> or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference ] [--merge] [--recursive] [--] [...]\n> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...]\n> - or: $dashless [--quiet] foreach [--recursive] \n> + or: $dashless [--quiet] foreach [--recursive] [--post-order=] \n\nMaybe drop the '=' from the description (see --reference for an example)?\n\n> or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n> OPTIONS_SPEC=\n> . git-sh-setup\n> @@ -434,6 +434,8 @@ Use -f if you really want to add it.\" >&2\n> cmd_foreach()\n> {\n> # parse $args after \"submodule ... foreach\".\n> + # Gratuitous (empty) local's to prevent recursive bleeding\n> + local recursive= post_order=\n\nWouldn't it be sufficient to add \"post_order=\" to the top of the\nfile where \"recursive\" is already initialized? Or am I missing\nsomething here?\n\n> while test $# -ne 0\n> do\n> case \"$1\" in\n> @@ -443,6 +445,15 @@ cmd_foreach()\n> --recursive)\n> recursive=1\n> ;;\n> + --post-order)\n> + test \"$#\" = \"1\" && usage\n> + post_order=\"$2\"\n> + shift\n> + ;;\n> + --post-order=*)\n> + # Will skip empty commands\n> + post_order=${1#*=}\n> + ;;\n> -*)\n> usage\n> ;;\n> @@ -453,7 +464,7 @@ cmd_foreach()\n> shift\n> done\n> \n> - toplevel=$(pwd)\n> + local toplevel=$(pwd)\n\nWhy do you have to add the \"local\" keyword here?\n\n> # dup stdin so that it can be restored when running the external\n> # command in the subshell (and a recursive call to this function)\n> @@ -465,18 +476,36 @@ cmd_foreach()\n> die_if_unmatched \"$mode\"\n> if test -e \"$sm_path\"/.git\n> then\n> - say \"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n> + local name prefix path message epitaph\n\nSame here?\n\n> + message=\"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n> + epitaph=\"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n> name=$(module_name \"$sm_path\")\n> (\n> prefix=\"$prefix$sm_path/\"\n> clear_local_git_env\n> # we make $path available to scripts ...\n> path=$sm_path\n> +\n> + sm_eval() {\n> + say \"$message\"\n> + eval \"$@\" || die \"$epitaph\"\n> + }\n> +\n> cd \"$sm_path\" &&\n> - eval \"$@\" &&\n> + sm_eval \"$@\" &&\n> if test -n \"$recursive\"\n> then\n> - cmd_foreach \"--recursive\" \"$@\"\n> + if test -n \"$post_order\"\n> + then\n> + # Tried keeping flags as a variable, but was having difficulty\n\nMaybe because you set the \"post_order\" variable to empty at the\nbeginning of this function? If I read that right moving that\ninitialization to the top of the file could get rid of the if\nhere?\n\n> + cmd_foreach --recursive --post-order \"$post_order\" \"$@\"\n> + else\n> + cmd_foreach --recursive \"$@\"\n> + fi\n> + fi &&\n> + if test -n \"$post_order\"\n> + then\n> + sm_eval \"$post_order\"\n> fi\n> ) <&3 3<&- ||\n> die \"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n"},{"id":"212263","messageId":"51511C62.3080704@gmail.com","threadId":"33063","inReplyTo":"514782D3.5060200@web.de","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Eric Cousineau","fromEmail":"eacousineau@gmail.com","sentAt":"2013-03-26T03:56:18Z","receivedAt":"2013-03-26T03:56:18Z","isPatch":true,"sender":{"key":"eacousineau@gmail.com","avatar":null},"body":"On 03/18/2013 04:10 PM, Jens Lehmann wrote:\n> Am 12.03.2013 17:01, schrieb Phil Hord:\n>> On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann wrote:\n>>> Am 05.03.2013 22:17, schrieb Phil Hord:\n>>>> On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann wrote:\n>>>>> Am 05.03.2013 19:34, schrieb Junio C Hamano:\n>>>>>> Eric Cousineau writes:\n>>>>>>> ...\n>>>>>> I am not entirely convinced we would want --include-super in the\n>>>>>> first place, though. It does not belong to \"submodule foreach\";\n>>>>>> it is doing something _outside_ the submoudules.\n>>>>>\n>>>>> I totally agree with that. First, adding --include-super does not\n>>>>> belong into the --post-order patch at all, as that is a different\n>>>>> topic (even though it belongs to the same use case Eric has). Also\n>>>>> the reason why we are thinking about adding the --post-order option\n>>>>> IMO cuts the other way for --include-super: It is so easy to do\n>>>>> that yourself I'm not convinced we should add an extra option to\n>>>>> foreach for that, especially as it has nothing to do with submodules.\n>>>>> So I think we should just drop --include-super.\n>>>>\n>>>> I agree it should not be part of this commit, but I've often found\n>>>> myself in need of an --include-super switch. To me,\n>>>> git-submodule-foreach means \"visit all my .git repos in this project\n>>>> and execute $cmd\". It's a pity that the super-project is considered a\n>>>> second-class citizen in this regard.\n>>>\n>>> Hmm, for me the super-project is a very natural second-class citizen\n>>> to \"git *submodule* foreach\". But also I understand that sometimes the\n>>> user wants to apply a command to superproject and submodules alike (I\n>>> just recently did exactly that with \"git gc\" on our build server).\n>>>\n>>>> I have to do this sometimes:\n>>>>\n>>>> ${cmd} && git submodule foreach --recursive '${cmd}'\n>>>>\n>>>> I often forget the first part in scripts, though, and I've seen others\n>>>> do it too. I usually create a function for it in git-heavy scripts.\n>>>>\n>>>> In a shell, it usually goes like this:\n>>>>\n>>>> git submodule foreach --recursive '${cmd}'\n>>>> {30-ish}\n>>>>\n>>>> It'd be easier if I could just include a switch for this, and maybe\n>>>> even create an alias for it. But maybe this is different command\n>>>> altogether.\n>>>\n>>> Are you sure you wouldn't forget to provide such a switch too? ;-)\n>>\n>> No. However, when I remember to add the switch, my shell history will\n>> remember it for me. This does not happen naturally for me in the\n>> \"{30-ish}...\" workflow.\n> \n> I started to use '&&' in my daily shell work for exactly that reason:\n> that the bash history remembers groups of two or more commands for me.\n> \n>> I also hope this switch grows up into a configuration option someday.\n>> Or maybe a completely different command, like I said before; because I\n>> actually think it could be dangerous as a configuration option since\n>> it would have drastic consequences for users executing scripts or\n>> commands in other users' environments.\n> \n> I agree on the possible problems a configuration option introduces.\n> \n>>> I'm still not convinced we should add a new switch, as it can easily\n>>> be achieved by adding \"${cmd} &&\" to your scripts. And on the command\n>>> line you could use an alias like this one to achieve that:\n>>>\n>>> [alias]\n>>> recurse = !sh -c \\\"$@ && git submodule foreach --recursive $@\\\"\n>>\n\nI tried this and the 'recurse-post' alias, but could not get it to function as\nit does inside of 'git submodule foreach'. I also tried out some different escaping\nmethods, but nothing seemed to work. I've added the examples below.\n\n>> Yes, making the feature itself a 2nd-class citizen. :-)\n>>\n>> But this alias also denies me the benefit of the --post-order option.\n>> For 'git recurse git push', for example, I wouldn't want the\n>> superproject push to occur first; I would want it to occur last after\n>> the submodules have been successfully pushed.\n> \n> [alias]\n> recurse-post = !sh -c \\\"git submodule foreach --recursive --post-order $@ && $@\\\"\n> ;-)\n> \n>> I agree this should go in some other commit, but I do not think it is\n>> so trivial it should never be considered as a feature for git. That's\n>> all I'm trying to say.\n> \n> I am not against adding such a functionality to Git, I'm just not\n> convinced \"git submodule foreach\" is the right command for that. I\n> suspect the \"git for-each-repo\" Lars proposed earlier this year might\n> be a better choice, as that could also recurse into other repos which\n> aren't registered as submodules. And a \"for-each-repo\" to me looks\n> like a command which could include the superproject too (at least when\n> told to do so with an option).\n> \n\nHere are the aliases I am using:\n\n[alias]\n\trecurse = !sh -c \\\"$@ && git submodule foreach --recursive $@\\\"\n\trecurse-post = !sh -c \\\"git submodule foreach --recursive --post-order $@ && $@\\\"\n\tfer = !sh -c \\\"eval \\\\\\\"$@\\\\\\\" && git submodule foreach --recursive \\\\\\\"$@\\\\\\\"\\\"\n\tferpo = !sh -c \\\"git submodule foreach --recursive --post-order \\\\\\\"$@\\\\\\\" && eval \\\\\\\"$@\\\\\\\"\\\"\n\tfers = !sh -c \\\"eval '$@' && git submodule foreach --recursive '$@'\\\"\n\tferpos = !sh -c \\\"git submodule foreach --recursive --post-order '$@' && eval '$@'\\\"\n\nAnd these are the results I get with the following example:\n\n$ cmd=\"echo \\\"'ello world: \\$PWD\\\"\"\n$ eval \"$cmd\"\n'ello world: /tmp/a\n$ git submodule foreach --recursive \"$cmd\"\nEntering 'b'\n'ello world: /tmp/a/b\nEntering 'b/d'\n'ello world: /tmp/a/b/d\nEntering 'c'\n'ello world: /tmp/a/c\n$ git submodule foreach --recursive --post-order \"$cmd\" \"$cmd\"\nEntering 'b'\n'ello world: /tmp/a/b\nEntering 'b/d'\n'ello world: /tmp/a/b/d\nExiting 'b/d'\n'ello world: /tmp/a/b/d\nExiting 'b'\n'ello world: /tmp/a/b\nEntering 'c'\n'ello world: /tmp/a/c\nExiting 'c'\n'ello world: /tmp/a/c\n$ git recurse \"$cmd\"\n'ello world: /tmp/a\nEntering 'b'\n/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string\nStopping at 'b'; script returned non-zero status.\n$ git recurse-post \"$cmd\"\nEntering 'b'\n/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string\nStopping at 'b'; script returned non-zero status.\n$ git fer \"$cmd\"\nello world: /tmp/a\nEntering 'b'\nello world: /tmp/a\nEntering 'b/d'\nello world: /tmp/a\nEntering 'c'\nello world: /tmp/a\n$ git ferpo \"$cmd\"\nEntering 'b'\n/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: world:: not found\nStopping at 'b'; script returned non-zero status.\nStopping at 'b'; script returned non-zero status.\n$ git fers \"$cmd\"\nello world: /tmp/a' && git submodule foreach --recursive 'echo ello world: /tmp/a\n$ git ferpos \"$cmd\"\nEntering 'b'\n/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string\nStopping at 'b'; script returned non-zero status.\n\nThe problem is trying to escape with double-quotes, where the single-quotes are evaluated\nas a shell token thing and not as a string argument, versus single-quotes, where you cannot (easily) escape single\nquotes inside of it (though please correct me if I'm wrong!).\nIt seems the best solution would be to have it as a script to allow recursion to occur in the scope of one script,\nlike submodule foreach.\n\nI understand now why it does not fit in the scope of 'git submodule', though, so I could implement it as a *very*\nlightweight stand-in for Lars's \"git for-each-repo\" via some copy-and-paste :P\n\n- Eric\n"},{"id":"212264","messageId":"51511E04.6070504@gmail.com","threadId":"33063","inReplyTo":"5147863B.8010104@web.de","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Eric Cousineau","fromEmail":"eacousineau@gmail.com","sentAt":"2013-03-26T04:03:16Z","receivedAt":"2013-03-26T04:03:16Z","isPatch":true,"sender":{"key":"eacousineau@gmail.com","avatar":null},"body":" From 2c2923ada809d671828aa58dcda05a1b71222b70 Mon Sep 17 00:00:00 2001\nFrom: Eric Cousineau \nDate: Mon, 25 Mar 2013 22:27:06 -0500\nSubject: [PATCH] submodule-foreach: Added in --post-order= and\n adjusted code per Jens Lehmann's suggestion\n\nSigned-off-by: Eric Cousineau \n---\nUpdated the usage line.\nI had put the locals in there before because I think I was having \ntrouble with resolving some\nof the variables in nested submodules, but now that I've taken them out \nthey seem to work fine.\nI also changed the message for the post-order to say \"Exiting\".\n\nI did not have a chance to look into why I couldn't group the \n--post-order stuff into a string\nwhen passing it on to submodule. I can look at it later on though.\n\nNow the output is as follows:\n\n$ git submodule foreach --recursive --post-order 'echo Post $name' 'echo \nPre $path'\nEntering 'b'\nPre b\nEntering 'b/d'\nPre d\nExiting 'b/d'\nPost d\nExiting 'b'\nPost b\nEntering 'c'\nPre c\nExiting 'c'\nPost c\n\n git-submodule.sh | 35 ++++++++++++++++++++++++++++++-----\n 1 file changed, 30 insertions(+), 5 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 004c034..4c9923a 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -10,7 +10,7 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] \n[--name ] [--reference ...]\n or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] \n[-f|--force] [--rebase] [--reference ] [--merge] \n[--recursive] [--] [...]\n or: $dashless [--quiet] summary [--cached|--files] [--summary-limit \n] [commit] [--] [...]\n- or: $dashless [--quiet] foreach [--recursive] \n+ or: $dashless [--quiet] foreach [--recursive] [--post-order \n] \n or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n OPTIONS_SPEC=\n . git-sh-setup\n@@ -434,6 +434,8 @@ Use -f if you really want to add it.\" >&2\n cmd_foreach()\n {\n \t# parse $args after \"submodule ... foreach\".\n+\trecursive=\n+\tpost_order=\n \twhile test $# -ne 0\n \tdo\n \t\tcase \"$1\" in\n@@ -443,6 +445,15 @@ cmd_foreach()\n \t\t--recursive)\n \t\t\trecursive=1\n \t\t\t;;\n+\t\t--post-order)\n+\t\t\ttest \"$#\" = \"1\" && usage\n+\t\t\tpost_order=\"$2\"\n+\t\t\tshift\n+\t\t\t;;\n+\t\t--post-order=*)\n+\t\t\t# Will skip empty commands\n+\t\t\tpost_order=${1#*=}\n+\t\t\t;;\n \t\t-*)\n \t\t\tusage\n \t\t\t;;\n@@ -465,7 +476,9 @@ cmd_foreach()\n \t\tdie_if_unmatched \"$mode\"\n \t\tif test -e \"$sm_path\"/.git\n \t\tthen\n-\t\t\tsay \"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n+\t\t\tenter_msg=\"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n+\t\t\texit_msg=\"$(eval_gettext \"Exiting '\\$prefix\\$sm_path'\")\"\n+\t\t\tdie_msg=\"$(eval_gettext \"Stopping at '\\$sm_path'; script returned \nnon-zero status.\")\"\n \t\t\tname=$(module_name \"$sm_path\")\n \t\t\t(\n \t\t\t\tprefix=\"$prefix$sm_path/\"\n@@ -473,13 +486,25 @@ cmd_foreach()\n \t\t\t\t# we make $path available to scripts ...\n \t\t\t\tpath=$sm_path\n \t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\teval \"$@\" &&\n+\t\t\t\tsay \"$enter_msg\" &&\n+\t\t\t\teval \"$@\" || die \"$die_msg\" &&\n \t\t\t\tif test -n \"$recursive\"\n \t\t\t\tthen\n-\t\t\t\t\tcmd_foreach \"--recursive\" \"$@\"\n+\t\t\t\t\tif test -n \"$post_order\"\n+\t\t\t\t\tthen\n+\t\t\t\t\t\t# tried keeping flags as a variable, but was having difficulty\n+\t\t\t\t\t\tcmd_foreach --recursive --post-order \"$post_order\" \"$@\"\n+\t\t\t\t\telse\n+\t\t\t\t\t\tcmd_foreach --recursive \"$@\"\n+\t\t\t\t\tfi\n+\t\t\t\tfi &&\n+\t\t\t\tif test -n \"$post_order\"\n+\t\t\t\tthen\n+\t\t\t\t\tsay \"$exit_msg\" &&\n+\t\t\t\t\teval \"$post_order\" || die \"$die_msg\"\n \t\t\t\tfi\n \t\t\t) <&3 3<&- ||\n-\t\t\tdie \"$(eval_gettext \"Stopping at '\\$sm_path'; script returned \nnon-zero status.\")\"\n+\t\t\tdie \"$die_msg\"\n \t\tfi\n \tdone\n }\n-- \n1.7.9.5\n"},{"id":"212265","messageId":"515125CE.3020101@gmail.com","threadId":"33063","inReplyTo":"51511C62.3080704@gmail.com","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Eric Cousineau","fromEmail":"eacousineau@gmail.com","sentAt":"2013-03-26T04:36:30Z","receivedAt":"2013-03-26T04:36:30Z","isPatch":true,"sender":{"key":"eacousineau@gmail.com","avatar":null},"body":"On 03/25/2013 10:56 PM, Eric Cousineau wrote:\n> On 03/18/2013 04:10 PM, Jens Lehmann wrote:\n>> Am 12.03.2013 17:01, schrieb Phil Hord:\n>>> On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann wrote:\n>>>> Am 05.03.2013 22:17, schrieb Phil Hord:\n...\n>>\n>> I agree on the possible problems a configuration option introduces.\n>>\n>>>> I'm still not convinced we should add a new switch, as it can easily\n>>>> be achieved by adding \"${cmd} &&\" to your scripts. And on the command\n>>>> line you could use an alias like this one to achieve that:\n>>>>\n>>>> [alias]\n>>>> recurse = !sh -c \\\"$@ && git submodule foreach --recursive $@\\\"\n>>>\n>\n> I tried this and the 'recurse-post' alias, but could not get it to function as\n> it does inside of 'git submodule foreach'. I also tried out some different escaping\n> methods, but nothing seemed to work. I've added the examples below.\n>\n>>> Yes, making the feature itself a 2nd-class citizen. :-)\n>>>\n>>> But this alias also denies me the benefit of the --post-order option.\n>>> For 'git recurse git push', for example, I wouldn't want the\n>>> superproject push to occur first; I would want it to occur last after\n>>> the submodules have been successfully pushed.\n>>\n>> [alias]\n>> recurse-post = !sh -c \\\"git submodule foreach --recursive --post-order $@ && $@\\\"\n>> ;-)\n>>\n>>> I agree this should go in some other commit, but I do not think it is\n>>> so trivial it should never be considered as a feature for git. That's\n>>> all I'm trying to say.\n>>\n>> I am not against adding such a functionality to Git, I'm just not\n>> convinced \"git submodule foreach\" is the right command for that. I\n>> suspect the \"git for-each-repo\" Lars proposed earlier this year might\n>> be a better choice, as that could also recurse into other repos which\n>> aren't registered as submodules. And a \"for-each-repo\" to me looks\n>> like a command which could include the superproject too (at least when\n>> told to do so with an option).\n>>\n>\n> Here are the aliases I am using:\n>\n> [alias]\n> \trecurse = !sh -c \\\"$@ && git submodule foreach --recursive $@\\\"\n> \trecurse-post = !sh -c \\\"git submodule foreach --recursive --post-order $@ && $@\\\"\n> \tfer = !sh -c \\\"eval \\\\\\\"$@\\\\\\\" && git submodule foreach --recursive \\\\\\\"$@\\\\\\\"\\\"\n> \tferpo = !sh -c \\\"git submodule foreach --recursive --post-order \\\\\\\"$@\\\\\\\" && eval \\\\\\\"$@\\\\\\\"\\\"\n> \tfers = !sh -c \\\"eval '$@' && git submodule foreach --recursive '$@'\\\"\n> \tferpos = !sh -c \\\"git submodule foreach --recursive --post-order '$@' && eval '$@'\\\"\n>\n> And these are the results I get with the following example:\n>\n> $ cmd=\"echo \\\"'ello world: \\$PWD\\\"\"\n> $ eval \"$cmd\"\n> 'ello world: /tmp/a\n> $ git submodule foreach --recursive \"$cmd\"\n> Entering 'b'\n> 'ello world: /tmp/a/b\n> Entering 'b/d'\n> 'ello world: /tmp/a/b/d\n> Entering 'c'\n> 'ello world: /tmp/a/c\n> $ git submodule foreach --recursive --post-order \"$cmd\" \"$cmd\"\n> Entering 'b'\n> 'ello world: /tmp/a/b\n> Entering 'b/d'\n> 'ello world: /tmp/a/b/d\n> Exiting 'b/d'\n> 'ello world: /tmp/a/b/d\n> Exiting 'b'\n> 'ello world: /tmp/a/b\n> Entering 'c'\n> 'ello world: /tmp/a/c\n> Exiting 'c'\n> 'ello world: /tmp/a/c\n> $ git recurse \"$cmd\"\n> 'ello world: /tmp/a\n> Entering 'b'\n> /home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string\n> Stopping at 'b'; script returned non-zero status.\n> $ git recurse-post \"$cmd\"\n> Entering 'b'\n> /home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string\n> Stopping at 'b'; script returned non-zero status.\n> $ git fer \"$cmd\"\n> ello world: /tmp/a\n> Entering 'b'\n> ello world: /tmp/a\n> Entering 'b/d'\n> ello world: /tmp/a\n> Entering 'c'\n> ello world: /tmp/a\n> $ git ferpo \"$cmd\"\n> Entering 'b'\n> /home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: world:: not found\n> Stopping at 'b'; script returned non-zero status.\n> Stopping at 'b'; script returned non-zero status.\n> $ git fers \"$cmd\"\n> ello world: /tmp/a' && git submodule foreach --recursive 'echo ello world: /tmp/a\n> $ git ferpos \"$cmd\"\n> Entering 'b'\n> /home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string\n> Stopping at 'b'; script returned non-zero status.\n>\n> The problem is trying to escape with double-quotes, where the single-quotes are evaluated\n> as a shell token thing and not as a string argument, versus single-quotes, where you cannot (easily) escape single\n> quotes inside of it (though please correct me if I'm wrong!).\n> It seems the best solution would be to have it as a script to allow recursion to occur in the scope of one script,\n> like submodule foreach.\n>\n> I understand now why it does not fit in the scope of 'git submodule', though, so I could implement it as a *very*\n> lightweight stand-in for Lars's \"git for-each-repo\" via some copy-and-paste :P\n>\n> - Eric\n>\n\nPut together a script with the --include-super functionality, named it \n'git-fer.sh' to start.\nPosted as a Gist: https://gist.github.com/eacousineau/5243161\n\nThat test case:\n\n$ git-fer --include-super --recursive --post-order \"$cmd\" \"$cmd\"\nEntering supermodule 'a'\n'ello world: /tmp/a\nEntering 'b'\n'ello world: /tmp/a/b\nEntering 'b/d'\n'ello world: /tmp/a/b/d\nExiting 'b/d'\n'ello world: /tmp/a/b/d\nExiting 'b'\n'ello world: /tmp/a/b\nEntering 'c'\n'ello world: /tmp/a/c\nExiting 'c'\n'ello world: /tmp/a/c\nExiting supermodule 'a'\n'ello world: /tmp/a\n"},{"id":"212266","messageId":"7vhajyviny.fsf@alter.siamese.dyndns.org","threadId":"33063","inReplyTo":"514782D3.5060200@web.de","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-26T05:23:13Z","receivedAt":"2013-03-26T05:23:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jens Lehmann writes:\n\n> Am 12.03.2013 17:01, schrieb Phil Hord:\n>> On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann wrote:\n>>> Am 05.03.2013 22:17, schrieb Phil Hord:\n>>>> On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann wrote:\n>>>>> Am 05.03.2013 19:34, schrieb Junio C Hamano:\n>>>>>> Eric Cousineau writes:\n>>>>>>> ...\n>>>>>> I am not entirely convinced we would want --include-super in the\n>>>>>> first place, though. It does not belong to \"submodule foreach\";\n>>>>>> it is doing something _outside_ the submoudules.\n>>>>>\n>>>>> I totally agree with that. First, adding --include-super does not\n>>>>> belong into the --post-order patch at all, as that is a different\n>>>>> topic (even though it belongs to the same use case Eric has). Also\n>>>>> the reason why we are thinking about adding the --post-order option\n>>>>> IMO cuts the other way for --include-super: It is so easy to do\n>>>>> that yourself I'm not convinced we should add an extra option to\n>>>>> foreach for that, especially as it has nothing to do with submodules.\n>>>>> So I think we should just drop --include-super.\n\nFWIW, after thinking about it a bit more and especially after\nthinking about the nested submodule layout, I changed my mind.\n\nThe reasoning is very simple. In short, your \"top-level\" may be\nsomebody else's submodule.\n\nIf you have a project A, that has a submodule B & C that in turn\nhave submodules D, E & F, G, like this:\n\n\t A\n / \\\n B C\n / \\ / \\\n D E F G\n\nyou may want your \"submodule foreach [--post-order]\" that is run at\nthe top-level to visit B D E C F G (or D E B F G C). A is not a\nsubmodule, and it may be rational to do without --also-toplevel\noption from the point of view of yourself.\n\nBut if \"submodule foreach [--post-order] B\" run at the top-level\nvisits B D E (or D E B), wouldn't it be more natural if you had a\nway to optionally make this\n\n\tcd B && submodule foreach [--post-order]\n\nvisit the same modules in the same way? The story is the same if\nyour top-level project A is bound at a path in somebody else's\nproject as a submodule. His \"submodulle foreach\" will visit your\ntop-level A while visiting the hierarchy of your submodules (and\nother submodules he has as your siblings).\n\nI do not know if foreach should visit your top-level by default;\nchanging that may be too late and too disruptive. But I think an\noptional \"I want this traversal to also visit the top\" would not be\nso _wrong_ even at the conceptual level.\n\nOf course, it may make the implementation simpler, too ;-) foreach\ncould just scan the immediate submodules, chdir into each of them\nand then run the equivalent foreach with --also-toplevel option,\nwith the same --post-order (or --pre-order) option.\n"},{"id":"212267","messageId":"7vd2umvikf.fsf@alter.siamese.dyndns.org","threadId":"33063","inReplyTo":"7vhajyviny.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-26T05:25:20Z","receivedAt":"2013-03-26T05:25:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jens Lehmann writes:\n> ...\n>>>>>> I totally agree with that. First, adding --include-super does not\n>>>>>> belong into the --post-order patch at all, as that is a different\n>>>>>> topic (even though it belongs to the same use case Eric has).\n\nI forgot to say that I haven't changed my mind about this part. \"Do\nwe visit the top-level?\" is an orthogonal and different topic and is\nbetter done in a patch separate from the one for --post-order.\n\nThanks.\n"},{"id":"212983","messageId":"515B3C0E.9000802@web.de","threadId":"33063","inReplyTo":"51511E04.6070504@gmail.com","subject":"Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-04-02T20:14:06Z","receivedAt":"2013-04-02T20:14:06Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Seems were getting closer, some comments from a quick read of your\npatch below.\n\nAm 26.03.2013 05:03, schrieb Eric Cousineau:\n> From 2c2923ada809d671828aa58dcda05a1b71222b70 Mon Sep 17 00:00:00 2001\n> From: Eric Cousineau \n> Date: Mon, 25 Mar 2013 22:27:06 -0500\n> Subject: [PATCH] submodule-foreach: Added in --post-order= and\n> adjusted code per Jens Lehmann's suggestion\n> \n> Signed-off-by: Eric Cousineau \n> ---\n> Updated the usage line.\n> I had put the locals in there before because I think I was having trouble with resolving some\n> of the variables in nested submodules, but now that I've taken them out they seem to work fine.\n> I also changed the message for the post-order to say \"Exiting\".\n\nThat's better than \"Stopping\", but while I'm not a native speaker\nI'd propose to use \"Leaving\" as the opposite of \"Entering\".\n\n> I did not have a chance to look into why I couldn't group the --post-order stuff into a string\n> when passing it on to submodule. I can look at it later on though.\n> \n> Now the output is as follows:\n> \n> $ git submodule foreach --recursive --post-order 'echo Post $name' 'echo Pre $path'\n> Entering 'b'\n> Pre b\n> Entering 'b/d'\n> Pre d\n> Exiting 'b/d'\n> Post d\n> Exiting 'b'\n> Post b\n> Entering 'c'\n> Pre c\n> Exiting 'c'\n> Post c\n> \n> git-submodule.sh | 35 ++++++++++++++++++++++++++++++-----\n> 1 file changed, 30 insertions(+), 5 deletions(-)\n> \n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 004c034..4c9923a 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -10,7 +10,7 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] [--name ] [--reference or: $dashless [--quiet] init [--] [...]\n> or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference ] [--merge] [--recursive] [--] [...]\n> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...]\n> - or: $dashless [--quiet] foreach [--recursive] \n> + or: $dashless [--quiet] foreach [--recursive] [--post-order ] \n> or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n> OPTIONS_SPEC=\n> . git-sh-setup\n> @@ -434,6 +434,8 @@ Use -f if you really want to add it.\" >&2\n> cmd_foreach()\n> {\n> # parse $args after \"submodule ... foreach\".\n> + recursive=\n> + post_order=\n\nI'm still not sure we need that here, in fact the problem you have\nwith the cmd_foreach invocation below might just be because you\nreset these variables here instead of once at the top of this file.\n\n> while test $# -ne 0\n> do\n> case \"$1\" in\n> @@ -443,6 +445,15 @@ cmd_foreach()\n> --recursive)\n> recursive=1\n> ;;\n> + --post-order)\n> + test \"$#\" = \"1\" && usage\n> + post_order=\"$2\"\n> + shift\n> + ;;\n> + --post-order=*)\n> + # Will skip empty commands\n> + post_order=${1#*=}\n> + ;;\n> -*)\n> usage\n> ;;\n> @@ -465,7 +476,9 @@ cmd_foreach()\n> die_if_unmatched \"$mode\"\n> if test -e \"$sm_path\"/.git\n> then\n> - say \"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n> + enter_msg=\"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n> + exit_msg=\"$(eval_gettext \"Exiting '\\$prefix\\$sm_path'\")\"\n> + die_msg=\"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n> name=$(module_name \"$sm_path\")\n> (\n> prefix=\"$prefix$sm_path/\"\n> @@ -473,13 +486,25 @@ cmd_foreach()\n> # we make $path available to scripts ...\n> path=$sm_path\n> cd \"$sm_path\" &&\n> - eval \"$@\" &&\n> + say \"$enter_msg\" &&\n> + eval \"$@\" || die \"$die_msg\" &&\n> if test -n \"$recursive\"\n> then\n> - cmd_foreach \"--recursive\" \"$@\"\n> + if test -n \"$post_order\"\n> + then\n> + # tried keeping flags as a variable, but was having difficulty\n> + cmd_foreach --recursive --post-order \"$post_order\" \"$@\"\n> + else\n> + cmd_foreach --recursive \"$@\"\n> + fi\n> + fi &&\n> + if test -n \"$post_order\"\n> + then\n> + say \"$exit_msg\" &&\n> + eval \"$post_order\" || die \"$die_msg\"\n> fi\n> ) <&3 3<&- ||\n> - die \"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n> + die \"$die_msg\"\n> fi\n> done\n> }\n"},{"id":"214118","messageId":"1365825865-12858-1-git-send-email-eacousineau@gmail.com","threadId":"33063","inReplyTo":"515B3C0E.9000802@web.de","subject":"[PATCH] submodule foreach: Added in --post-order= and adjusted code per Jens Lehmann's suggestions","fromName":"eacousineau","fromEmail":"eacousineau@gmail.com","sentAt":"2013-04-13T04:04:25Z","receivedAt":"2013-04-13T04:04:25Z","isPatch":true,"sender":{"key":"eacousineau@gmail.com","avatar":null},"body":"Signed-off-by: eacousineau \n---\nI see what you meant by the extra variables, so I've fixed that so the\noriginal flags aren't needed with recursion. Also updated it to not\nprint the entering command if there is only a post-order command.\n\nExamples:\n\n$ git submodule foreach --recursive --post-order 'echo Goodbye' \"echo \\\"'ello\\\"\"\nEntering 'b'\n'ello\nEntering 'b/d'\n'ello\nLeaving 'b/d'\nGoodbye\nLeaving 'b'\nGoodbye\nEntering 'c'\n'ello\nLeaving 'c'\nGoodbye\n\n$ git submodule foreach --recursive --post-order :\nLeaving 'b/d'\nLeaving 'b'\nLeaving 'c'\n\n git-submodule.sh | 31 ++++++++++++++++++++++++++-----\n 1 file changed, 26 insertions(+), 5 deletions(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 79bfaac..e08a724 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -11,7 +11,7 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] [--name ] [--reference ...\n or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference ] [--merge] [--recursive] [--] [...]\n or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...]\n- or: $dashless [--quiet] foreach [--recursive] \n+ or: $dashless [--quiet] foreach [--recursive] [--post-order ] \n or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n OPTIONS_SPEC=\n . git-sh-setup\n@@ -449,6 +449,15 @@ cmd_foreach()\n \t\t--recursive)\n \t\t\trecursive=1\n \t\t\t;;\n+\t\t--post-order)\n+\t\t\ttest \"$#\" = \"1\" && usage\n+\t\t\tpost_order=\"$2\"\n+\t\t\tshift\n+\t\t\t;;\n+\t\t--post-order=*)\n+\t\t\t# Will skip empty commands\n+\t\t\tpost_order=${1#*=}\n+\t\t\t;;\n \t\t-*)\n \t\t\tusage\n \t\t\t;;\n@@ -471,7 +480,9 @@ cmd_foreach()\n \t\tdie_if_unmatched \"$mode\"\n \t\tif test -e \"$sm_path\"/.git\n \t\tthen\n-\t\t\tsay \"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n+\t\t\tenter_msg=\"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n+\t\t\tleave_msg=\"$(eval_gettext \"Leaving '\\$prefix\\$sm_path'\")\"\n+\t\t\tdie_msg=\"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n \t\t\tname=$(module_name \"$sm_path\")\n \t\t\t(\n \t\t\t\tprefix=\"$prefix$sm_path/\"\n@@ -479,13 +490,23 @@ cmd_foreach()\n \t\t\t\t# we make $path available to scripts ...\n \t\t\t\tpath=$sm_path\n \t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\teval \"$@\" &&\n+\t\t\t\tif test $# -gt 0 -o -z \"$post_order\"\n+\t\t\t\tthen\n+\t\t\t\t\tsay \"$enter_msg\" &&\n+\t\t\t\t\teval \"$@\" || die \"$die_msg\"\n+\t\t\t\tfi &&\n \t\t\t\tif test -n \"$recursive\"\n \t\t\t\tthen\n-\t\t\t\t\tcmd_foreach \"--recursive\" \"$@\"\n+\t\t\t\t\t# subshell will use parent-scoped values\n+\t\t\t\t\tcmd_foreach \"$@\"\n+\t\t\t\tfi &&\n+\t\t\t\tif test -n \"$post_order\"\n+\t\t\t\tthen\n+\t\t\t\t\tsay \"$leave_msg\" &&\n+\t\t\t\t\teval \"$post_order\" || die \"$die_msg\"\n \t\t\t\tfi\n \t\t\t) <&3 3<&- ||\n-\t\t\tdie \"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n+\t\t\tdie \"$die_msg\"\n \t\tfi\n \tdone\n }\n-- \n1.8.2.1.390.gd4ee029\n"},{"id":"214119","messageId":"CA+aSAWviVWd3+89Wpv-1_6BSL=aN-u+60_oDXcyCG03enBmGTg@mail.gmail.com","threadId":"33063","inReplyTo":"CA+aSAWuK9Yhvx-vO1fUteq-K=xOPgxkyeWeHG3UwZuDHsxLzAw@mail.gmail.com","subject":"Re: [PATCH] submodule foreach: Added in --post-order= and adjusted code per Jens Lehmann's suggestions","fromName":"Eric Cousineau","fromEmail":"eacousineau@gmail.com","sentAt":"2013-04-13T04:11:32Z","receivedAt":"2013-04-13T04:11:32Z","isPatch":true,"sender":{"key":"eacousineau@gmail.com","avatar":null},"body":"Had accidentally sent this as HTML, resending as plain-text.\n\nOn Fri, Apr 12, 2013 at 11:09 PM, Eric Cousineau wrote:\n>\n> Oops... I tried out using git-send-email adding in the Message-Id, but forgot to change the title as well. My bad.\n>\n> This was in response to:\n>\n> [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well\n> Message-Id: <515B3C0E.9000802@web.de>\n>\n> - Eric\n>\n>\n> On Fri, Apr 12, 2013 at 11:04 PM, eacousineau wrote:\n>>\n>> Signed-off-by: eacousineau \n>> ---\n>> I see what you meant by the extra variables, so I've fixed that so the\n>> original flags aren't needed with recursion. Also updated it to not\n>> print the entering command if there is only a post-order command.\n>>\n>> Examples:\n>>\n>> $ git submodule foreach --recursive --post-order 'echo Goodbye' \"echo \\\"'ello\\\"\"\n>> Entering 'b'\n>> 'ello\n>> Entering 'b/d'\n>> 'ello\n>> Leaving 'b/d'\n>> Goodbye\n>> Leaving 'b'\n>> Goodbye\n>> Entering 'c'\n>> 'ello\n>> Leaving 'c'\n>> Goodbye\n>>\n>> $ git submodule foreach --recursive --post-order :\n>> Leaving 'b/d'\n>> Leaving 'b'\n>> Leaving 'c'\n>>\n>> git-submodule.sh | 31 ++++++++++++++++++++++++++-----\n>> 1 file changed, 26 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/git-submodule.sh b/git-submodule.sh\n>> index 79bfaac..e08a724 100755\n>> --- a/git-submodule.sh\n>> +++ b/git-submodule.sh\n>> @@ -11,7 +11,7 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] [--name ] [--reference > or: $dashless [--quiet] deinit [-f|--force] [--] ...\n>> or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference ] [--merge] [--recursive] [--] [...]\n>> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...]\n>> - or: $dashless [--quiet] foreach [--recursive] \n>> + or: $dashless [--quiet] foreach [--recursive] [--post-order ] \n>> or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n>> OPTIONS_SPEC=\n>> . git-sh-setup\n>> @@ -449,6 +449,15 @@ cmd_foreach()\n>> --recursive)\n>> recursive=1\n>> ;;\n>> + --post-order)\n>> + test \"$#\" = \"1\" && usage\n>> + post_order=\"$2\"\n>> + shift\n>> + ;;\n>> + --post-order=*)\n>> + # Will skip empty commands\n>> + post_order=${1#*=}\n>> + ;;\n>> -*)\n>> usage\n>> ;;\n>> @@ -471,7 +480,9 @@ cmd_foreach()\n>> die_if_unmatched \"$mode\"\n>> if test -e \"$sm_path\"/.git\n>> then\n>> - say \"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n>> + enter_msg=\"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n>> + leave_msg=\"$(eval_gettext \"Leaving '\\$prefix\\$sm_path'\")\"\n>> + die_msg=\"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n>> name=$(module_name \"$sm_path\")\n>> (\n>> prefix=\"$prefix$sm_path/\"\n>> @@ -479,13 +490,23 @@ cmd_foreach()\n>> # we make $path available to scripts ...\n>> path=$sm_path\n>> cd \"$sm_path\" &&\n>> - eval \"$@\" &&\n>> + if test $# -gt 0 -o -z \"$post_order\"\n>> + then\n>> + say \"$enter_msg\" &&\n>> + eval \"$@\" || die \"$die_msg\"\n>> + fi &&\n>> if test -n \"$recursive\"\n>> then\n>> - cmd_foreach \"--recursive\" \"$@\"\n>> + # subshell will use parent-scoped values\n>> + cmd_foreach \"$@\"\n>> + fi &&\n>> + if test -n \"$post_order\"\n>> + then\n>> + say \"$leave_msg\" &&\n>> + eval \"$post_order\" || die \"$die_msg\"\n>> fi\n>> ) <&3 3<&- ||\n>> - die \"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n>> + die \"$die_msg\"\n>> fi\n>> done\n>> }\n>> --\n>> 1.8.2.1.390.gd4ee029\n>>\n>\n"},{"id":"214221","messageId":"516AFAD8.2010207@web.de","threadId":"33063","inReplyTo":"1365825865-12858-1-git-send-email-eacousineau@gmail.com","subject":"Re: [PATCH] submodule foreach: Added in --post-order= and adjusted code per Jens Lehmann's suggestions","fromName":"Jens Lehmann","fromEmail":"jens.lehmann@web.de","sentAt":"2013-04-14T18:52:08Z","receivedAt":"2013-04-14T18:52:08Z","isPatch":true,"sender":{"key":"jens.lehmann@web.de","avatar":"https://avatars.githubusercontent.com/u/135220?v=4"},"body":"Am 13.04.2013 06:04, schrieb eacousineau:\n> Signed-off-by: eacousineau \n> ---\n> I see what you meant by the extra variables, so I've fixed that so the\n> original flags aren't needed with recursion.\n\nThanks, the code is looking much better now and you nicely\ndescribed the changes you made since the last version. A few\ncomments though:\n\nI think the subject line should read:\n\n [PATCH v2] submodule foreach: Add --post-order option\n\nWe use the imperative form, also the adjustments are a normal part\nof the review process and don't need to be mentioned explicitly in\nthe title, just show the version of your iteration by adding \"v2\"\nafter the word \"PATCH\" (and of course the next iteration will be\n\"v3\" ;-).\n\nThe commit message is not explaining what you did and why you did\nit, please see the \"Describe your changes well.\" section in\nDocumentation/SubmittingPatches on how to do that.\n\nAnd you'll also want to add the new option to the man page in\nDocumentation/git-submodule.txt.\n\n> Also updated it to not\n> print the entering command if there is only a post-order command.\n\nGood idea.\n\n> Examples:\n> \n> $ git submodule foreach --recursive --post-order 'echo Goodbye' \"echo \\\"'ello\\\"\"\n> Entering 'b'\n> 'ello\n> Entering 'b/d'\n> 'ello\n> Leaving 'b/d'\n> Goodbye\n> Leaving 'b'\n> Goodbye\n> Entering 'c'\n> 'ello\n> Leaving 'c'\n> Goodbye\n> \n> $ git submodule foreach --recursive --post-order :\n> Leaving 'b/d'\n> Leaving 'b'\n> Leaving 'c'\n\nMakes sense to me. These two examples should be getting a test\ncase each in t/t7407-submodule-foreach.sh.\n\n> git-submodule.sh | 31 ++++++++++++++++++++++++++-----\n> 1 file changed, 26 insertions(+), 5 deletions(-)\n> \n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 79bfaac..e08a724 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -11,7 +11,7 @@ USAGE=\"[--quiet] add [-b ] [-f|--force] [--name ] [--reference or: $dashless [--quiet] deinit [-f|--force] [--] ...\n> or: $dashless [--quiet] update [--init] [--remote] [-N|--no-fetch] [-f|--force] [--rebase] [--reference ] [--merge] [--recursive] [--] [...]\n> or: $dashless [--quiet] summary [--cached|--files] [--summary-limit ] [commit] [--] [...]\n> - or: $dashless [--quiet] foreach [--recursive] \n> + or: $dashless [--quiet] foreach [--recursive] [--post-order ] \n> or: $dashless [--quiet] sync [--recursive] [--] [...]\"\n> OPTIONS_SPEC=\n> . git-sh-setup\n> @@ -449,6 +449,15 @@ cmd_foreach()\n> \t\t--recursive)\n> \t\t\trecursive=1\n> \t\t\t;;\n> +\t\t--post-order)\n> +\t\t\ttest \"$#\" = \"1\" && usage\n> +\t\t\tpost_order=\"$2\"\n> +\t\t\tshift\n> +\t\t\t;;\n> +\t\t--post-order=*)\n> +\t\t\t# Will skip empty commands\n> +\t\t\tpost_order=${1#*=}\n> +\t\t\t;;\n> \t\t-*)\n> \t\t\tusage\n> \t\t\t;;\n> @@ -471,7 +480,9 @@ cmd_foreach()\n> \t\tdie_if_unmatched \"$mode\"\n> \t\tif test -e \"$sm_path\"/.git\n> \t\tthen\n> -\t\t\tsay \"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n> +\t\t\tenter_msg=\"$(eval_gettext \"Entering '\\$prefix\\$sm_path'\")\"\n> +\t\t\tleave_msg=\"$(eval_gettext \"Leaving '\\$prefix\\$sm_path'\")\"\n\nI don't think we gain much by putting enter_msg and leave_msg into\ntheir own variables as they are only used once, I'd prefer to see\nthese messages inlined.\n\n> +\t\t\tdie_msg=\"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n\nI think there is a \\$prefix missing in front of the \\$sm_path here\n(see enter_msg and leave_msg). As you only copied that message you\ncan simply say in the commit message \"While at it also fix a missing\nprefix in the die message\" at the end of the last paragraph.\n\n> \t\t\tname=$(module_name \"$sm_path\")\n> \t\t\t(\n> \t\t\t\tprefix=\"$prefix$sm_path/\"\n> @@ -479,13 +490,23 @@ cmd_foreach()\n> \t\t\t\t# we make $path available to scripts ...\n> \t\t\t\tpath=$sm_path\n> \t\t\t\tcd \"$sm_path\" &&\n> -\t\t\t\teval \"$@\" &&\n> +\t\t\t\tif test $# -gt 0 -o -z \"$post_order\"\n> +\t\t\t\tthen\n> +\t\t\t\t\tsay \"$enter_msg\" &&\n> +\t\t\t\t\teval \"$@\" || die \"$die_msg\"\n> +\t\t\t\tfi &&\n> \t\t\t\tif test -n \"$recursive\"\n> \t\t\t\tthen\n> -\t\t\t\t\tcmd_foreach \"--recursive\" \"$@\"\n> +\t\t\t\t\t# subshell will use parent-scoped values\n> +\t\t\t\t\tcmd_foreach \"$@\"\n\nYou should at least state that you dropped the --recursive here\non purpose, just add that to the \"While at it ...\" sentence. And I\nsuspect the comment above is more a reminder for yourself, we\ncould drop that too.\n\n> +\t\t\t\tfi &&\n> +\t\t\t\tif test -n \"$post_order\"\n> +\t\t\t\tthen\n> +\t\t\t\t\tsay \"$leave_msg\" &&\n> +\t\t\t\t\teval \"$post_order\" || die \"$die_msg\"\n> \t\t\t\tfi\n> \t\t\t) <&3 3<&- ||\n> -\t\t\tdie \"$(eval_gettext \"Stopping at '\\$sm_path'; script returned non-zero status.\")\"\n> +\t\t\tdie \"$die_msg\"\n> \t\tfi\n> \tdone\n> }\n> \n"}]}