Re: [PATCH] submodule status: properly pass options with --recursive

[Jens Lehmann <Jens.Lehmann@web.de>]

Am 26.10.2012 21:07, schrieb Phil Hord:
> On Fri, Oct 26, 2012 at 9:15 AM, Jeff King <peff@peff.net> wrote:
>> On Fri, Oct 26, 2012 at 12:20:29AM +0200, Jens Lehmann wrote:
>>
>>> When renaming orig_args to orig_flags in 98dbe63d (submodule: only
>>> preserve flags across recursive status/update invocations) the call site
>>> of the recursive cmd_status was forgotten. At that place orig_args is
>>> still passed into the recursion, which is always empty now. This clears
>>> all options when recursing, as that variable is never set.
>>>
>>> Fix that by renaming orig_args to orig_flags there too and add a test to
>>> catch that bug.
>>
>> Thanks. I back-ported your patch on top of 98dbe63d so it can go to
>> 'maint'. I'm curious, though: why didn't the test right before (which
>> checks recursion for --cached) catch this?
> 
> I was wondering the same thing about why 'git submodule sync
> --recursive --quiet' succeeded, so I checked on it.  The answer is
> that "--quiet" sets GIT_QUIET=1, which is then inherited by the
> recursive call. Indeed, Jens' new test passes even without the
> accompanying fix.  :-\

Dang, you're right! At least that explains why nobody noticed that
so far ... (and that's what you get for skipping the "does the test
fail without your fix?" part because the fix is so obvious :-( )

> The 'status --recursive --cached' test passes for two reasons.  The
> first is that the test is checking for a cached change in a level-1
> submodule, but it is the level-2+ submodules which do not get the
> flags passed down correctly in "$@".  The 2nd reason is that the
> $cached variable is _also_ inherited by the recursive call to
> cmd_status, specifically because it is a call to cmd_status() and not
> to '$SHELL git submodule status', where the latter would have cleared
> the flags at startup, but the former does not.
> 
> I'm a bit wary about "fixing" the flags for the recursion machinery.
> I'm starting to think $orig_flags is moot in almost all cases.  It
> looks like clearing all the flags on each iteration would break 'git
> submodule foreach --recursive --quiet' because it does not use
> $orig_flags at all.  Who knows what other surprises lurk there.

I agree, it looks like ripping out the orig_flags would be the way
to go.