Am 14.07.2014 03:01, schrieb Junio C Hamano: > Jens Lehmann <Jens.Lehmann@xxxxxx> 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" >>> ) >>> } >>> >>> > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html