git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Cousineau <eacousineau@gmail.com>
To: Jens Lehmann <Jens.Lehmann@web.de>
Cc: Phil Hord <phil.hord@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Heiko Voigt <hvoigt@hvoigt.net>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	Lars Hjemli <hjemli@gmail.com>
Subject: Re: [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well
Date: Mon, 25 Mar 2013 22:56:18 -0500	[thread overview]
Message-ID: <51511C62.3080704@gmail.com> (raw)
In-Reply-To: <514782D3.5060200@web.de>

On 03/18/2013 04:10 PM, Jens Lehmann wrote:
> Am 12.03.2013 17:01, schrieb Phil Hord:
>> On Sat, Mar 9, 2013 at 1:18 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>>> Am 05.03.2013 22:17, schrieb Phil Hord:
>>>> On Tue, Mar 5, 2013 at 3:51 PM, Jens Lehmann <Jens.Lehmann@web.de> wrote:
>>>>> Am 05.03.2013 19:34, schrieb Junio C Hamano:
>>>>>> Eric Cousineau <eacousineau@gmail.com> writes:
>>>>>>> ...
>>>>>> I am not entirely convinced we would want --include-super in the
>>>>>> first place, though.  It does not belong to "submodule foreach";
>>>>>> it is doing something _outside_ the submoudules.
>>>>>
>>>>> I totally agree with that. First, adding --include-super does not
>>>>> belong into the --post-order patch at all, as that is a different
>>>>> topic (even though it belongs to the same use case Eric has). Also
>>>>> the reason why we are thinking about adding the --post-order option
>>>>> IMO cuts the other way for --include-super: It is so easy to do
>>>>> that yourself I'm not convinced we should add an extra option to
>>>>> foreach for that, especially as it has nothing to do with submodules.
>>>>> So I think we should just drop --include-super.
>>>>
>>>> I agree it should not be part of this commit, but I've often found
>>>> myself in need of an --include-super switch.   To me,
>>>> git-submodule-foreach means "visit all my .git repos in this project
>>>> and execute $cmd".  It's a pity that the super-project is considered a
>>>> second-class citizen in this regard.
>>>
>>> Hmm, for me the super-project is a very natural second-class citizen
>>> to "git *submodule* foreach". But also I understand that sometimes the
>>> user wants to apply a command to superproject and submodules alike (I
>>> just recently did exactly that with "git gc" on our build server).
>>>
>>>> I have to do this sometimes:
>>>>
>>>>     ${cmd} && git submodule foreach --recursive '${cmd}'
>>>>
>>>> I often forget the first part in scripts, though, and I've seen others
>>>> do it too.  I usually create a function for it in git-heavy scripts.
>>>>
>>>> In a shell, it usually goes like this:
>>>>
>>>>     git submodule foreach --recursive '${cmd}'
>>>>     <up><home><del>{30-ish}<end><backspace><enter>
>>>>
>>>> It'd be easier if I could just include a switch for this, and maybe
>>>> even create an alias for it.  But maybe this is different command
>>>> altogether.
>>>
>>> Are you sure you wouldn't forget to provide such a switch too? ;-)
>>
>> No.  However, when I remember to add the switch, my shell history will
>> remember it for me.  This does not happen naturally for me in the
>> "<up><home><del>{30-ish}..." workflow.
> 
> I started to use '&&' in my daily shell work for exactly that reason:
> that the bash history remembers groups of two or more commands for me.
> 
>> I also hope this switch grows up into a configuration option someday.
>> Or maybe a completely different command, like I said before; because I
>> actually think it could be dangerous as a configuration option since
>> it would have drastic consequences for users executing scripts or
>> commands in other users' environments.
> 
> I agree on the possible problems a configuration option introduces.
> 
>>> I'm still not convinced we should add a new switch, as it can easily
>>> be achieved by adding "${cmd} &&" to your scripts. And on the command
>>> line you could use an alias like this one to achieve that:
>>>
>>> [alias]
>>>          recurse = !sh -c \"$@ && git submodule foreach --recursive $@\"
>>

I tried this and the 'recurse-post' alias, but could not get it to function as
it does inside of 'git submodule foreach'. I also tried out some different escaping
methods, but nothing seemed to work. I've added the examples below.

>> Yes, making the feature itself a 2nd-class citizen.  :-)
>>
>> But this alias also denies me the benefit of the --post-order option.
>> For 'git recurse git push', for example, I wouldn't want the
>> superproject push to occur first; I would want it to occur last after
>> the submodules have been successfully pushed.
> 
> [alias]
>           recurse-post = !sh -c \"git submodule foreach --recursive --post-order $@ && $@\"
> ;-)
> 
>> I agree this should go in some other commit, but I do not think it is
>> so trivial it should never be considered as a feature for git.  That's
>> all I'm trying to say.
> 
> I am not against adding such a functionality to Git, I'm just not
> convinced "git submodule foreach" is the right command for that. I
> suspect the "git for-each-repo" Lars proposed earlier this year might
> be a better choice, as that could also recurse into other repos which
> aren't registered as submodules. And a "for-each-repo" to me looks
> like a command which could include the superproject too (at least when
> told to do so with an option).
> 

Here are the aliases I am using:

[alias]
	recurse = !sh -c \"$@ && git submodule foreach --recursive $@\"
	recurse-post = !sh -c \"git submodule foreach --recursive --post-order $@ && $@\"
	fer = !sh -c \"eval \\\"$@\\\" && git submodule foreach --recursive \\\"$@\\\"\"
	ferpo = !sh -c \"git submodule foreach --recursive --post-order \\\"$@\\\" && eval \\\"$@\\\"\"
	fers = !sh -c \"eval '$@' && git submodule foreach --recursive '$@'\"
	ferpos = !sh -c \"git submodule foreach --recursive --post-order '$@' && eval '$@'\"

And these are the results I get with the following example:

$ cmd="echo \"'ello world: \$PWD\""
$ eval "$cmd"
'ello world: /tmp/a
$ git submodule foreach --recursive "$cmd"
Entering 'b'
'ello world: /tmp/a/b
Entering 'b/d'
'ello world: /tmp/a/b/d
Entering 'c'
'ello world: /tmp/a/c
$ git submodule foreach --recursive --post-order "$cmd" "$cmd"
Entering 'b'
'ello world: /tmp/a/b
Entering 'b/d'
'ello world: /tmp/a/b/d
Exiting 'b/d'
'ello world: /tmp/a/b/d
Exiting 'b'
'ello world: /tmp/a/b
Entering 'c'
'ello world: /tmp/a/c
Exiting 'c'
'ello world: /tmp/a/c
$ git recurse "$cmd"
'ello world: /tmp/a
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
Stopping at 'b'; script returned non-zero status.
$ git recurse-post "$cmd"
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
Stopping at 'b'; script returned non-zero status.
$ git fer "$cmd"
ello world: /tmp/a
Entering 'b'
ello world: /tmp/a
Entering 'b/d'
ello world: /tmp/a
Entering 'c'
ello world: /tmp/a
$ git ferpo "$cmd"
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: world:: not found
Stopping at 'b'; script returned non-zero status.
Stopping at 'b'; script returned non-zero status.
$ git fers "$cmd"
ello world: /tmp/a' && git submodule foreach --recursive 'echo ello world: /tmp/a
$ git ferpos "$cmd"
Entering 'b'
/home/eacousineau/local/lib/git/libexec/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
Stopping at 'b'; script returned non-zero status.

The problem is trying to escape with double-quotes, where the single-quotes are evaluated
as a shell token thing and not as a string argument, versus single-quotes, where you cannot (easily) escape single
quotes inside of it (though please correct me if I'm wrong!).
It seems the best solution would be to have it as a script to allow recursion to occur in the scope of one script,
like submodule foreach.

I understand now why it does not fit in the scope of 'git submodule', though, so I could implement it as a *very*
lightweight stand-in for Lars's "git for-each-repo" via some copy-and-paste :P

- Eric

  reply	other threads:[~2013-03-26  3:56 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-04  8:41 [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well Eric Cousineau
2013-03-04 22:15 ` Jens Lehmann
2013-03-04 23:00   ` Junio C Hamano
2013-03-05  5:37     ` Eric Cousineau
2013-03-05  7:59     ` Heiko Voigt
2013-03-05 16:09       ` Junio C Hamano
2013-03-05 16:42         ` Eric Cousineau
2013-03-05 18:34           ` Junio C Hamano
2013-03-05 20:51             ` Jens Lehmann
2013-03-05 21:17               ` Phil Hord
2013-03-09 18:18                 ` Jens Lehmann
2013-03-11 16:46                   ` Heiko Voigt
2013-03-12 16:01                   ` Phil Hord
2013-03-14  6:30                     ` Eric Cousineau
2013-03-18 21:25                       ` Jens Lehmann
2013-03-26  4:03                         ` Eric Cousineau
2013-04-02 20:14                           ` Jens Lehmann
2013-04-13  4:04                             ` [PATCH] submodule foreach: Added in --post-order=<command> and adjusted code per Jens Lehmann's suggestions eacousineau
     [not found]                               ` <CA+aSAWuK9Yhvx-vO1fUteq-K=xOPgxkyeWeHG3UwZuDHsxLzAw@mail.gmail.com>
2013-04-13  4:11                                 ` Eric Cousineau
2013-04-14 18:52                               ` Jens Lehmann
2013-03-18 21:10                     ` [PATCH/RFC] Changing submodule foreach --recursive to be depth-first, --parent option to execute command in supermodule as well Jens Lehmann
2013-03-26  3:56                       ` Eric Cousineau [this message]
2013-03-26  4:36                         ` Eric Cousineau
2013-03-26  5:23                       ` Junio C Hamano
2013-03-26  5:25                         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51511C62.3080704@gmail.com \
    --to=eacousineau@gmail.com \
    --cc=Jens.Lehmann@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hjemli@gmail.com \
    --cc=hvoigt@hvoigt.net \
    --cc=phil.hord@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).