On Fri, Oct 26, 2012 at 9:15 AM, Jeff King <peff@xxxxxxxx> 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. :-\
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.
Here's a fix for the test, though, even though it still does not fail
in the absence of the $orig_flags patch.
-- >8 --
diff --git i/t/t7407-submodule-foreach.sh w/t/t7407-submodule-foreach.sh
index 9b69fe2..8442b32 100755
--- i/t/t7407-submodule-foreach.sh
+++ w/t/t7407-submodule-foreach.sh
@@ -226,14 +226,14 @@ test_expect_success 'test "status --recursive"' '
test_cmp expect actual
'
-sed -e "/nested1 /s/.*/+$nested1sha1 nested1 (file2~1)/;/sub[1-3]/d"
< expect > expect2
+sed -e "/nested2 /s/.*/+$nested2sha1 nested1\/nested2
(file2~1)/;/sub[1-3]/d" < expect > expect2
mv -f expect2 expect
test_expect_success 'ensure "status --cached --recursive" preserves
the --cached flag' '
(
cd clone3 &&
(
- cd nested1 &&
+ cd nested1/nested2 &&
test_commit file2
) &&
git submodule status --cached --recursive -- nested1 > ../actual
@@ -245,6 +245,14 @@ test_expect_success 'ensure "status --cached
--recursive" preserves the --cached
test_cmp expect actual
'
test_expect_success 'use "git clone --recursive" to checkout all submodules' '
git clone --recursive super clone4 &&
(
Phil
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html