Re: [RFC PATCHv2 17/17] checkout: add config option to recurse into submodules by default

[Stefan Beller <sbeller@google.com>]

On Mon, Dec 5, 2016 at 11:29 AM, Brandon Williams <bmwill@google.com> wrote:
> On 12/02, Stefan Beller wrote:
>> +test_expect_success 'option checkout.recurseSubmodules updates submodule' '
>> +     test_config checkout.recurseSubmodules 1 &&
>> +     git checkout base &&
>> +     git checkout -b advanced-base &&
>> +     git -C submodule commit --allow-empty -m "empty commit" &&
>> +     git add submodule &&
>> +     git commit -m "advance submodule" &&
>> +     git checkout base &&
>> +     git diff-files --quiet &&
>> +     git diff-index --quiet --cached base &&
>> +     git checkout advanced-base &&
>> +     git diff-files --quiet &&
>> +     git diff-index --quiet --cached advanced-base &&
>> +     git checkout --recurse-submodules base
>> +'
>> +
>
> This test doesn't look like it looks into the submodule to see if the
> submodule has indeed changed.  Unless diff-index and diff-files recurse
> into the submodules?

I took the code from Jens once upon a time. Rereading the code, I agree it is
not obvious how this checks the submodule state.

However `git diff-files --quiet` is perfectly fine, as
we have submodule support by default via:

  Omitting the --submodule option or specifying --submodule=short,
  uses the short format.  This format just shows the names of the commits
  at the beginning and end of the range.

and then we turn it into an exit code via

       --quiet
           Disable all output of the program. Implies --exit-code.

       --exit-code
           Make the program exit with codes similar to diff(1).
  That is, it exits with 1 if there were differences and 0 means no differences.

Same for diff-index.

The main purpose of this specific test is to have checkout.recurseSubmodules
"to just make it work" without having to give --recurse-submodules manually.
All the other tests with the manual --recurse-submodules should test for
correctness of the behavior within the submodule.

So maybe I'll need to rewrite submodule_creation_must_succeed() in the previous
patch to be more obvious. (Well that already has some tests for
files/directories
in there, so it is a little more.)

But to be sure we can also add tests here that look more into the submodule.
I am thinking of "{new,old}_sub_sha1=$(git -C submodule rev-parse HEAD)" and
comparing them?