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

From: Jens Lehmann <[email-masked]>
Date: Mon, 14 Jul 2014 20:22:37 +0200

Am 14.07.2014 03:01, schrieb Junio C Hamano:
> Jens Lehmann <[email-masked]> writes:
> 
>>> Perhaps squashing this to 7e8e5af9 instead?
>>
>> Yes please, this is much better than my first attempt.
> 
> One thing that I found troubling is the ../../../ "three levels up"
> is hardcoded.  Would it be always true for any value of "$1"?  If
> the submodule is bound to the superproject at sub/dir/, not at dir/,
> for example, would it have to change?

Yes. And the code currently also doesn't handle submodules whose
name and path differ.

> I am not saying that we must support artibrary cases, but if there
> is such a limitation in the implementation, people who will use the
> helper in their new tests want it at least documented, I think.

Ah, I didn't think about other tests reusing that and only wrote
this as a local helper. But you're right, it would make sense to
reuse this function instead of coding that again (even though I'd
prefer to extract the generic helpers to t/lib-submodule.sh for
that purpose).

So what about adding "Currently only submodules living in the
root directory of the superproject with the default name (same
as the path) are supported." to the comment above the function?

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