Subject: Re: [PATCH 00/14] Add submodule test harness

From: Junio C Hamano <[email-masked]>
Date: Thu, 10 Jul 2014 13:52:37 -0700

Junio C Hamano <[email-masked]> writes:

> Jens Lehmann <[email-masked]> writes:
>
>> I agree, but this case is special. The test asserts that nobody
>> added, modified or removed *anything* inside the .git directory.
>> The reason for problem we are seeing here is that I have to
>> remove the core.worktree setting when moving the git directory
>> from .git/modules into the submodule work tree.
>
> Hmph.  Comparing the files with core.worktree removed sounds like a
> workaround that knows too much about the implementation detail of
> what is being tested.  I am just wondering if core.worktree will
> stay forever be the only thing that is special, or there may come
> other things (perhaps as a fallout of integrating things like Duy's
> multiple-worktree stuff).
>
> But perhaps we cannot do better than this.

One thing we should be able to do (and must do) better is to
validate that core.worktree in the relocated config file actually
points at the right place.  Unsetting before comparing may let us
compare the relocated one in .git/modules/$1/config with the one
that is embedded in the working tree (hence no .git/config), but the
way your "how about this?" patch does, we wouldn't catch a possible
breakage to the relocation code to point core.worktree to a bogus
location, I'm afraid.

Perhaps squashing this to 7e8e5af9 instead?

 t/lib-submodule-update.sh | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/t/lib-submodule-update.sh b/t/lib-submodule-update.sh
index e441b98..fc1da84 100755
--- a/t/lib-submodule-update.sh
+++ b/t/lib-submodule-update.sh
@@ -110,18 +110,23 @@ replace_gitfile_with_git_dir () {
 }
 
 # Test that the .git directory in the submodule is unchanged (except for the
-# core.worktree setting, which we temporarily restore). Call this function
-# before test_submodule_content as the latter might write the index file
-# leading to false positive index differences.
+# core.worktree setting, which appears only in $GIT_DIR/modules/$1/config).
+# Call this function before test_submodule_content as the latter might
+# write the index file leading to false positive index differences.
 test_git_directory_is_unchanged () {
 	(
-		cd "$1" &&
-		git config core.worktree "../../../$1"
+		cd ".git/modules/$1" &&
+		# does core.worktree point at the right place?
+		test "$(git config core.worktree)" = "../../../$1" &&
+		# remove it temporarily before comparing, as
+		# "$1/.git/config" lacks it...
+		git config --unset core.worktree
 	) &&
 	diff -r ".git/modules/$1" "$1/.git" &&
 	(
-		cd "$1" &&
-		GIT_WORK_TREE=. git config --unset core.worktree
+		# ... and then restore.
+		cd ".git/modules/$1" &&
+		git config core.worktree "../../../$1"
 	)
 }