[PATCH v1 1/1] tests: add Git submodule version consistency test

[Ferry Huberts]

On 19/03/13 21:33, John Keeping wrote:
> On Tue, Mar 19, 2013 at 09:22:06PM +0100, Ferry Huberts wrote:
>>
>>
>> On 19/03/13 21:00, John Keeping wrote:
>>> Subject: [PATCH] tests: check that Git version are in sync
>>>
>>> This ensures that the Git version pointed at by the submodule is the
>>> same as the one that will be fetched using "make get-git".
>>>
>>> Suggested-by: Ferry Huberts <ferry.huberts at pelagic.nl>
>>> Signed-off-by: John Keeping <john at keeping.me.uk>
>>> ---
>>> This is an enhanced version of what I posted earlier, which now checks
>>> that the correct Git version is available even if "make get-git" has
>>> been used to fetch it, by examining Git's GIT-VERSION-FILE.
>>
>> I don't agree with that.
>> IMHO we should check that the submodule SHA as stored in the repository
>> matches the version specified in the Makefile.
>>
>> Now you check 2 versions that might be 'dirty' against each other; the
>> version in the Makefile to whatever someone has put in the git
>> directory. That can easily pass testing locally but fail testing once
>> committed (commit dirty Makefile but forget to adjust submodule SHA to
>> match 'development' git directory unpacked from a random git tar download).
>
> The test using GIT-VERSION-FILE will catch this, that's generated during
> the Git build process to match precisely what is built (including a
> dirty flag).
>

Goes to prove that I don't know enough about the Git build process :-)

>> I'd prefer a situation in which we make sure that we test as close to a
>> clean checkout as possible, which means that we should use committed
>> state (when possible).
>
> The second check is doing that (or close to it, we're checking staged
> state rather than committed state, which is better in the case that
> you've got a commit ready and want to sanity check it before typing "git
> commit").

Fair enough.


>
> I think the combination of these two tests gives what you want, the
> GIT-VERSION-FILE tests that the version of Git being used to build CGit
> matches the Makefile and the "git ls-files --stage -- git" test checks
> that what's in the index matches.  We could change that to "git ls-tree
> HEAD -- git" but I think that is unnecessarily annoying for people
> trying out new Git versions.
>

Agree


Reviewed-by: Ferry Huberts <ferry.huberts at pelagic.nl>


>>>
>>> I also changed the submodule test to check the version of the submodule
>>> in the index rather than whatever happens to be checked out.  I think
>>> this is a more sensible test for people who are experimenting with
>>> different Git versions.
>>>
>>>    tests/t0001-validate-git-versions.sh | 36 ++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 36 insertions(+)
>>>    create mode 100755 tests/t0001-validate-git-versions.sh
>>>
>>> diff --git a/tests/t0001-validate-git-versions.sh b/tests/t0001-validate-git-versions.sh
>>> new file mode 100755
>>> index 0000000..3378358
>>> --- /dev/null
>>> +++ b/tests/t0001-validate-git-versions.sh
>>> @@ -0,0 +1,36 @@
>>> +#!/bin/sh
>>> +
>>> +. ./setup.sh
>>> +
>>> +prepare_tests 'Check Git version is correct'
>>> +
>>> +run_test 'extract Git version from Makefile' '
>>> +	sed -n -e "/^GIT_VER[ 	]*=/ {
>>> +		s/^GIT_VER[ 	]*=[ 	]*//
>>> +		p
>>> +	}" ../Makefile >trash/makefile_version
>>> +'
>>> +
>>> +run_test 'test Git version matches Makefile' '
>>> +	( cat ../git/GIT-VERSION-FILE || echo "No GIT-VERSION-FILE" ) |
>>> +	sed -e "s/GIT_VERSION[ 	]*=[ 	]*//" >trash/git_version &&
>>> +	diff -u trash/git_version trash/makefile_version
>>> +'
>>> +
>>> +run_test 'test submodule version matches Makefile' '
>>> +	if ! test -e ../git/.git
>>> +	then
>>> +		echo "git/ is not a Git repository" >&2
>>> +	else
>>> +		(
>>> +			cd .. &&
>>> +			sm_sha1=$(git ls-files --stage -- git |
>>> +				sed -e "s/^[0-9]* \\([0-9a-f]*\\) [0-9]	.*$/\\1/") &&
>>> +			cd git &&
>>> +			git describe --match "v[0-9]*" $sm_sha1
>>> +		) | sed -e "s/^v//" >trash/sm_version &&
>>> +		diff -u trash/sm_version trash/makefile_version
>>> +	fi
>>> +'
>>> +
>>> +tests_done
>>>
>>
>> --
>> Ferry Huberts
>>
>> _______________________________________________
>> cgit mailing list
>> cgit at hjemli.net
>> http://hjemli.net/mailman/listinfo/cgit

-- 
Ferry Huberts