[PATCH 03/15] lib-submodule-update.sh: define tests for recursing into submodules

[Stefan Beller]

Currently lib-submodule-update.sh provides 2 functions
test_submodule_switch and test_submodule_forced_switch that are used by a
variety of tests to ensure that submodules behave as expected. The current
expected behavior is that submodules are not touched at all (see
42639d2317a for the exact setup).

In the future we want to teach all these commands to recurse
into submodules. To do that, we'll add two testing functions to
submodule-update-lib.sh: test_submodule_switch_recursing and
test_submodule_forced_switch_recursing.

These two functions behave in analogy to the already existing functions
just with a different expectation on submodule behavior. The submodule
in the working tree is expected to be updated to the recorded submodule
version. The behavior is analogous to e.g. the behavior of files in a
nested directory in the working tree, where a change to the working tree
handles any arising directory/file conflicts just fine.

Signed-off-by: Stefan Beller 
---
 t/lib-submodule-update.sh | 485 +-
 1 file changed, 483 insertions(+), 2 deletions(-)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index c0d6325133..0b26f0e20f 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -4,6 +4,7 @@
 # - New submodule (no_submodule => add_sub1)
 # - Removed submodule (add_sub1 => remove_sub1)
 # - Updated submodule (add_sub1 => modify_sub1)
+# - Updated submodule recursively (modify_sub1 => modify_sub1_recursively)
 # - Submodule updated to invalid commit (add_sub1 => invalid_sub1)
 # - Submodule updated from invalid commit (invalid_sub1 => valid_sub1)
 # - Submodule replaced by tracked files in directory (add_sub1 =>
@@ -19,8 +20,8 @@
 #/^
 #   / remove_sub1
 #  /
-#   add_sub1  /---O
-# |  /^
+#   add_sub1  /---O-O
+# |  /^ modify_sub1_recursive
 # | / modify_sub1
 # v/
 #  O--O---O-O
@@ -48,6 +49,17 @@ create_lib_submodule_repo () {
git commit -m "Base inside first submodule" &&
git branch "no_submodule"
) &&
+   git init submodule_update_sub2 &&
+   (
+   cd submodule_update_sub2
+   echo "expect" >>.gitignore &&
+   echo "actual" >>.gitignore &&
+   echo "x" >file1 &&
+   echo "y" >file2 &&
+   git add .gitignore file1 file2 &&
+   git commit -m "nested submodule base" &&
+   git branch "no_submodule"
+   ) &&
git init submodule_update_repo &&
(
cd submodule_update_repo &&
@@ -84,6 +96,14 @@ create_lib_submodule_repo () {
git add sub1 &&
git commit -m "Modify sub1" &&
 
+   git checkout -b modify_sub1_recursively modify_sub1 &&
+   git -C sub1 checkout -b "add_nested_sub" &&
+   git -C sub1 submodule add --branch no_submodule 
../submodule_update_sub2 sub2 &&
+   git -C sub1 commit -a -m "add a nested submodule" &&
+   git add sub1 &&
+   git commit -a -m "update submodule, that updates a nested 
submodule" &&
+   git -C sub1 submodule deinit -f --all &&
+
git checkout -b replace_sub1_with_directory add_sub1 &&
git submodule update &&
git -C sub1 checkout modifications &&
@@ -150,6 +170,15 @@ test_git_directory_is_unchanged () {
)
 }
 
+test_git_directory_exists() {
+   test -e ".git/modules/$1" &&
+   if test -f sub1/.git
+   then
+   # does core.worktree point at the right place?
+   test "$(git -C .git/modules/$1 config core.worktree)" = 
"../../../$1"
+   fi
+}
+
 # Helper function to be executed at the start of every test below, it sets up
 # the submodule repo if it doesn't exist and configures the most problematic
 # settings for diff.ignoreSubmodules.
@@ -180,6 +209,18 @@ reset_work_tree_to () {
)
 }
 
+reset_work_tree_to_interested () {
+   reset_work_tree_to $1 &&
+   # indicate we are interested in the submodule:
+   git -C submodule_update config submodule.sub1.url "bogus" &&
+   # also have it available:
+   if ! test -d submodule_update/.git/modules/sub1
+   then
+   mkdir -p submodule_update/.git/modules &&
+   cp -r submodule_update_repo/.git/modules/sub1 
submodule_update/.git/modules/sub1
+   fi
+}
+
 # Test that the superproject contains the content according to commit "$1"
 # (the work tree must match the index for everything but submodules but the
 # index must exactly match the given commit including any submodule SHA-1s).
@@ -695,3 +736,443 @@ test_submodule_forced_switch () {
)
'
 }
+
+# Test that submodule contents are correctly 

Re: [PATCH 03/15] lib-submodule-update.sh: define tests for recursing into submodules

[Stefan Beller]

On Thu, Feb 16, 2017 at 12:39 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> Currently lib-submodule-update.sh provides 2 functions
>> test_submodule_switch and test_submodule_forced_switch that are used by a
>> variety of tests to ensure that submodules behave as expected. The current
>> expected behavior is that submodules are not touched at all (see
>> 42639d2317a for the exact setup).
>>
>> In the future we want to teach all these commands to properly recurse
>> into submodules. To do that, we'll add two testing functions to
>> submodule-update-lib.sh test_submodule_switch_recursing and
>> test_submodule_forced_switch_recursing.
>
> I'd remove "properly" and insert a colon after "add two ... to
> submodule-update-lib.sh" before the names of two functions.

ok

>
>> +reset_work_tree_to_interested () {
>> + reset_work_tree_to $1 &&
>> + # indicate we are interested in the submodule:
>> + git -C submodule_update config submodule.sub1.url "bogus" &&
>> + # also have it available:
>> + if ! test -d submodule_update/.git/modules/sub1
>> + then
>> + mkdir submodule_update/.git/modules &&
>
> Would we want "mkdir -p" here to be safe?

Yes I cannot think of a downside of being overly cautious here.

>
>> + cp -r submodule_update_repo/.git/modules/sub1 
>> submodule_update/.git/modules/sub1
>
> ... ahh, wouldn't matter that much, we checked that modules/sub1
> does not exist, and as long as nobody creates modules/ or 
> modules/somethingelse
> we are OK.

Well, I'll add the -p

>> +# Test that submodule contents are correctly updated when switching
>> +# between commits that change a submodule.
>> +# Test that the following transitions are correctly handled:
>> +# (These tests are also above in the case where we expect no change
>> +#  in the submodule)
>> +# - Updated submodule
>> +# - New submodule
>> +# - Removed submodule
>> +# - Directory containing tracked files replaced by submodule
>> +# - Submodule replaced by tracked files in directory
>> +# - Submodule replaced by tracked file with the same name
>> +# - tracked file replaced by submodule
>
> These should work without trouble only when the paths involved in
> the operation in the working tree are clean, right?  Just double
> checking.  If they are dirty we should expect a failure, instead of
> silent loss of information.

yes, I'll go over the tests again and add those cases if missing.


>> + command="$1"
>
> The dq-pair is not strictly needed on the RHS of the assignment, but
> it is a good way to signal that we considered that we might receive
> an argument with $IFS in it...
>
>> + $command add_sub1 &&
>
> ... and after doing so, not quoting $command here signals that we
> expect command line splitting to happen.  Am I reading it correctly?
> Without an actual caller appearing in this step, it is rather hard
> to judge.
>

I followed the existing code without thinking about these points, but they are
valid and exactly how we'd expect the code to behave.
$1 / $command will be e.g. "git checkout --recurse-submodules" in
this patch series; but later on we could also have functions.
C.f.  t4137 which defines a function

apply_3way () {
git diff --ignore-submodules=dirty "..$1" | git apply --3way -
}
test_submodule_switch "apply_3way"

We'd want to have a similar thing for the recursing part, e.g.

apply_3way_recursing () {
git diff --submodule=diff "..$1" | git apply
--recurse-submodules --3way -
}
test_submodule_switch_recursing "apply_3way_recursing"

>> + echo sub1 > .git/info/exclude
>
> ">.git/info/exclude"

ok

Re: [PATCH 03/15] lib-submodule-update.sh: define tests for recursing into submodules

[Junio C Hamano]

Stefan Beller  writes:

> Currently lib-submodule-update.sh provides 2 functions
> test_submodule_switch and test_submodule_forced_switch that are used by a
> variety of tests to ensure that submodules behave as expected. The current
> expected behavior is that submodules are not touched at all (see
> 42639d2317a for the exact setup).
>
> In the future we want to teach all these commands to properly recurse
> into submodules. To do that, we'll add two testing functions to
> submodule-update-lib.sh test_submodule_switch_recursing and
> test_submodule_forced_switch_recursing.

I'd remove "properly" and insert a colon after "add two ... to
submodule-update-lib.sh" before the names of two functions.

> +reset_work_tree_to_interested () {
> + reset_work_tree_to $1 &&
> + # indicate we are interested in the submodule:
> + git -C submodule_update config submodule.sub1.url "bogus" &&
> + # also have it available:
> + if ! test -d submodule_update/.git/modules/sub1
> + then
> + mkdir submodule_update/.git/modules &&

Would we want "mkdir -p" here to be safe?

> + cp -r submodule_update_repo/.git/modules/sub1 
> submodule_update/.git/modules/sub1

... ahh, wouldn't matter that much, we checked that modules/sub1
does not exist, and as long as nobody creates modules/ or modules/somethingelse
we are OK.

> + fi
> +}
> +

> @@ -695,3 +736,443 @@ test_submodule_forced_switch () {
>   )
>   '
>  }
> +
> +# Test that submodule contents are correctly updated when switching
> +# between commits that change a submodule.
> +# Test that the following transitions are correctly handled:
> +# (These tests are also above in the case where we expect no change
> +#  in the submodule)
> +# - Updated submodule
> +# - New submodule
> +# - Removed submodule
> +# - Directory containing tracked files replaced by submodule
> +# - Submodule replaced by tracked files in directory
> +# - Submodule replaced by tracked file with the same name
> +# - tracked file replaced by submodule

These should work without trouble only when the paths involved in
the operation in the working tree are clean, right?  Just double
checking.  If they are dirty we should expect a failure, instead of
silent loss of information.

> +# New test cases
> +# - Removing a submodule with a git directory absorbs the submodules
> +#   git directory first into the superproject.
> +
> +test_submodule_switch_recursing () {
> + command="$1"

The dq-pair is not strictly needed on the RHS of the assignment, but
it is a good way to signal that we considered that we might receive
an argument with $IFS in it...

> + # Appearing submodule #
> + # Switching to a commit letting a submodule appear checks it out ...
> + test_expect_success "$command: added submodule is checked out" '
> + prolog &&
> + reset_work_tree_to_interested no_submodule &&
> + (
> + cd submodule_update &&
> + git branch -t add_sub1 origin/add_sub1 &&
> + $command add_sub1 &&

... and after doing so, not quoting $command here signals that we
expect command line splitting to happen.  Am I reading it correctly?
Without an actual caller appearing in this step, it is rather hard
to judge.

> + test_superproject_content origin/add_sub1 &&
> + test_submodule_content sub1 origin/add_sub1
> + )
> ...
> + # ... but an ignored file is fine.
> + test_expect_success "$command: added submodule removes an untracked 
> ignored file" '
> + test_when_finished "rm submodule_update/.git/info/exclude" &&
> + prolog &&
> + reset_work_tree_to_interested no_submodule &&
> + (
> + cd submodule_update &&
> + git branch -t add_sub1 origin/add_sub1 &&
> + : >sub1 &&
> + echo sub1 > .git/info/exclude

">.git/info/exclude"

> + $command add_sub1 &&
> + test_superproject_content origin/add_sub1 &&
> + test_submodule_content sub1 origin/add_sub1
> + )
> + '

[PATCH 03/15] lib-submodule-update.sh: define tests for recursing into submodules

[Stefan Beller]

Currently lib-submodule-update.sh provides 2 functions
test_submodule_switch and test_submodule_forced_switch that are used by a
variety of tests to ensure that submodules behave as expected. The current
expected behavior is that submodules are not touched at all (see
42639d2317a for the exact setup).

In the future we want to teach all these commands to properly recurse
into submodules. To do that, we'll add two testing functions to
submodule-update-lib.sh test_submodule_switch_recursing and
test_submodule_forced_switch_recursing.

These two functions behave in analogy to the already existing functions
just with a different expectation on submodule behavior. The submodule
in the working tree is expected to be updated to the recorded submodule
version. The behavior is analogous to e.g. the behavior of files in a
nested directory in the working tree, where a change to the working tree
handles any arising directory/file conflicts just fine.

Signed-off-by: Stefan Beller 
---
 t/lib-submodule-update.sh | 485 +-
 1 file changed, 483 insertions(+), 2 deletions(-)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index c0d6325133..ea838df028 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -4,6 +4,7 @@
 # - New submodule (no_submodule => add_sub1)
 # - Removed submodule (add_sub1 => remove_sub1)
 # - Updated submodule (add_sub1 => modify_sub1)
+# - Updated submodule recursively (modify_sub1 => modify_sub1_recursively)
 # - Submodule updated to invalid commit (add_sub1 => invalid_sub1)
 # - Submodule updated from invalid commit (invalid_sub1 => valid_sub1)
 # - Submodule replaced by tracked files in directory (add_sub1 =>
@@ -19,8 +20,8 @@
 #/^
 #   / remove_sub1
 #  /
-#   add_sub1  /---O
-# |  /^
+#   add_sub1  /---O-O
+# |  /^ modify_sub1_recursive
 # | / modify_sub1
 # v/
 #  O--O---O-O
@@ -48,6 +49,17 @@ create_lib_submodule_repo () {
git commit -m "Base inside first submodule" &&
git branch "no_submodule"
) &&
+   git init submodule_update_sub2 &&
+   (
+   cd submodule_update_sub2
+   echo "expect" >>.gitignore &&
+   echo "actual" >>.gitignore &&
+   echo "x" >file1 &&
+   echo "y" >file2 &&
+   git add .gitignore file1 file2 &&
+   git commit -m "nested submodule base" &&
+   git branch "no_submodule"
+   ) &&
git init submodule_update_repo &&
(
cd submodule_update_repo &&
@@ -84,6 +96,14 @@ create_lib_submodule_repo () {
git add sub1 &&
git commit -m "Modify sub1" &&
 
+   git checkout -b modify_sub1_recursively modify_sub1 &&
+   git -C sub1 checkout -b "add_nested_sub" &&
+   git -C sub1 submodule add --branch no_submodule 
../submodule_update_sub2 sub2 &&
+   git -C sub1 commit -a -m "add a nested submodule" &&
+   git add sub1 &&
+   git commit -a -m "update submodule, that updates a nested 
submodule" &&
+   git -C sub1 submodule deinit -f --all &&
+
git checkout -b replace_sub1_with_directory add_sub1 &&
git submodule update &&
git -C sub1 checkout modifications &&
@@ -150,6 +170,15 @@ test_git_directory_is_unchanged () {
)
 }
 
+test_git_directory_exists() {
+   test -e ".git/modules/$1" &&
+   if test -f sub1/.git
+   then
+   # does core.worktree point at the right place?
+   test "$(git -C .git/modules/$1 config core.worktree)" = 
"../../../$1"
+   fi
+}
+
 # Helper function to be executed at the start of every test below, it sets up
 # the submodule repo if it doesn't exist and configures the most problematic
 # settings for diff.ignoreSubmodules.
@@ -180,6 +209,18 @@ reset_work_tree_to () {
)
 }
 
+reset_work_tree_to_interested () {
+   reset_work_tree_to $1 &&
+   # indicate we are interested in the submodule:
+   git -C submodule_update config submodule.sub1.url "bogus" &&
+   # also have it available:
+   if ! test -d submodule_update/.git/modules/sub1
+   then
+   mkdir submodule_update/.git/modules &&
+   cp -r submodule_update_repo/.git/modules/sub1 
submodule_update/.git/modules/sub1
+   fi
+}
+
 # Test that the superproject contains the content according to commit "$1"
 # (the work tree must match the index for everything but submodules but the
 # index must exactly match the given commit including any submodule SHA-1s).
@@ -695,3 +736,443 @@ test_submodule_forced_switch () {
)
'
 }
+
+# Test that submodule contents are