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

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

Am 10.07.2014 22:52, schrieb Junio C Hamano:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Jens Lehmann <Jens.Lehmann@web.de> 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.

Indeed.

> Perhaps squashing this to 7e8e5af9 instead?

Yes please, this is much better than my first attempt.

>  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"
>  	)
>  }
>  
>