make test failure with latest master

make test failure with latest master

[A Large Angry SCM]

[Resent due to incorrect list address.]

With the latest master, 2221a6757161af1905925c405aded9ff470f70d5, "make 
test" now fails; last successful "make test" was mid-week sometime with 
master d25430. This is on a laptop running Suse 9.3.


*** t9600-cvsimport.sh ***
*   ok 1: setup cvsroot
*   ok 2: setup a cvs module
* FAIL 3: import a trivial module


                 git cvsimport -a -z 0 -C module-git module &&
                 git diff module-cvs/o_fortuna module-git/o_fortuna


*   ok 4: pack refs
*   ok 5: update cvs module
* FAIL 6: update git module


                 cd module-git &&
                 git cvsimport -a -z 0 module &&
                 git merge origin &&
                 cd .. &&
                 git diff module-cvs/o_fortuna module-git/o_fortuna


*   ok 7: update cvs module
* FAIL 8: cvsimport.module config works


                 cd module-git &&
                         git config cvsimport.module module &&
                         git cvsimport -a -z0 &&
                         git merge origin &&
                 cd .. &&
                 git diff module-cvs/tick module-git/tick


* failed 3 among 8 test(s)

Re: make test failure with latest master

[Jakub Narebski]

A Large Angry SCM <gitzilla@gmail.com> writes:

> [Resent due to incorrect list address.]
> 
> With the latest master, 2221a6757161af1905925c405aded9ff470f70d5,
> "make test" now fails; last successful "make test" was mid-week
> sometime with master d25430. This is on a laptop running Suse 9.3.
> 
> 
> *** t9600-cvsimport.sh ***

Trivial question: have you got cvs and cvsps installed?

-- 
Jakub Narebski

Re: make test failure with latest master

[A Large Angry SCM]

Jakub Narebski wrote:
> A Large Angry SCM <gitzilla@gmail.com> writes:
> 
>> [Resent due to incorrect list address.]
>>
>> With the latest master, 2221a6757161af1905925c405aded9ff470f70d5,
>> "make test" now fails; last successful "make test" was mid-week
>> sometime with master d25430. This is on a laptop running Suse 9.3.
>>
>>
>> *** t9600-cvsimport.sh ***
> 
> Trivial question: have you got cvs and cvsps installed?
> 

Yes. With the following (rpm based) versions:

	cvs-1.12.12-2.1
	cvsps-1.99-155.2
	cvsup-16.1h-173
	viewcvs-1.0-11

Re: make test failure with latest master

[Jeff King]

On Sun, Dec 02, 2007 at 07:29:50AM -0500, A Large Angry SCM wrote:

> With the latest master, 2221a6757161af1905925c405aded9ff470f70d5, "make 
> test" now fails; last successful "make test" was mid-week sometime with 
> master d25430. This is on a laptop running Suse 9.3.
>
> *** t9600-cvsimport.sh ***
> *   ok 1: setup cvsroot
> *   ok 2: setup a cvs module
> * FAIL 3: import a trivial module
>
>
>                 git cvsimport -a -z 0 -C module-git module &&
>                 git diff module-cvs/o_fortuna module-git/o_fortuna

Can you please try ./t9600 --verbose? Presumably cvsimport is generating
some kind of error message to tell us what's going on.

-Peff

Re: make test failure with latest master

[A Large Angry SCM]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

Jeff King wrote:
> On Sun, Dec 02, 2007 at 07:29:50AM -0500, A Large Angry SCM wrote:
> 
>> With the latest master, 2221a6757161af1905925c405aded9ff470f70d5, "make 
>> test" now fails; last successful "make test" was mid-week sometime with 
>> master d25430. This is on a laptop running Suse 9.3.
>>
>> *** t9600-cvsimport.sh ***
>> *   ok 1: setup cvsroot
>> *   ok 2: setup a cvs module
>> * FAIL 3: import a trivial module
>>
>>
>>                 git cvsimport -a -z 0 -C module-git module &&
>>                 git diff module-cvs/o_fortuna module-git/o_fortuna
> 
> Can you please try ./t9600 --verbose? Presumably cvsimport is generating
> some kind of error message to tell us what's going on.

Attached is the output of

	./t9600-cvsimport.sh --verbose >/tmp/9600_out.txt 2>&1

[-- Attachment #2: 9600_out.txt --]
[-- Type: text/plain, Size: 5602 bytes --]

* expecting success: cvs init
*   ok 1: setup cvsroot

* expecting success: 

	mkdir $CVSROOT/module &&
	cvs co -d module-cvs module &&
	cd module-cvs &&
	cat <<EOF >o_fortuna &&
O Fortuna
velut luna
statu variabilis,

semper crescis
aut decrescis;
vita detestabilis

nunc obdurat
et tunc curat
ludo mentis aciem,

egestatem,
potestatem
dissolvit ut glaciem.
EOF
	cvs add o_fortuna &&
	cat <<EOF >message &&
add "O Fortuna" lyrics

These public domain lyrics make an excellent sample text.
EOF
	cvs commit -F message &&
	cd ..

cvs checkout: Updating module-cvs
cvs add: scheduling file `o_fortuna' for addition
cvs add: use `cvs commit' to add this file permanently
cvs commit: Examining .
/home/internet/GIT/git/t/trash/cvsroot/module/o_fortuna,v  <--  o_fortuna
initial revision: 1.1
*   ok 2: setup a cvs module

* expecting success: 

	git cvsimport -a -z 0 -C module-git module &&
	git diff module-cvs/o_fortuna module-git/o_fortuna


Initialized empty Git repository in /home/internet/GIT/git/t/trash/module-git/.git/

bad usage: invalid argument -A

Usage: cvsps [-h] [-x] [-u] [-z <fuzz>] [-g] [-s <range>[,<range>]]  
             [-a <author>] [-f <file>] [-d <date1> [-d <date2>]] 
             [-b <branch>]  [-l <regex>] [-r <tag> [-r <tag>]] 
             [-p <directory>] [-v] [-t] [--norc] [--summary-first]
             [--test-log <captured cvs log file>] [--bkcvs]
             [--no-rlog] [--diff-opts <option string>] [--cvs-direct]
             [--debuglvl <bitmask>] [-Z <compression>] [--root <cvsroot>]
             [<repository>] [-q]

Where:
  -h display this informative message
  -x ignore (and rebuild) cvsps.cache file
  -u update cvsps.cache file
  -z <fuzz> set the timestamp fuzz factor for identifying patch sets
  -g generate diffs of the selected patch sets
  -s <patch set>[-[<patch set>]][,<patch set>...] restrict patch sets by id
  -a <author> restrict output to patch sets created by author
  -f <file> restrict output to patch sets involving file
  -d <date1> -d <date2> if just one date specified, show
     revisions newer than date1.  If two dates specified,
     show revisions between two dates.
  -b <branch> restrict output to patch sets affecting history of branch
  -l <regex> restrict output to patch sets matching <regex> in log message
  -r <tag1> -r <tag2> if just one tag specified, show
     revisions since tag1. If two tags specified, show
     revisions between the two tags.
  -p <directory> output patch sets to individual files in <directory>
  -v show very verbose parsing messages
  -t show some brief memory usage statistics
  --norc when invoking cvs, ignore the .cvsrc file
  --summary-first when multiple patch sets are shown, put all summaries first
  --test-log <captured cvs log> supply a captured cvs log for testing
  --diff-opts <option string> supply special set of options to diff
  --bkcvs special hack for parsing the BK -> CVS log format
  --no-rlog disable rlog (it's faulty in some setups)
  --cvs-direct (--no-cvs-direct) enable (disable) built-in cvs client code
  --debuglvl <bitmask> enable various debug channels.
  -Z <compression> A value 1-9 which specifies amount of compression
  --root <cvsroot> specify cvsroot.  overrides env. and working directory
  -q be quiet about warnings
  <repository> apply cvsps to repository.  overrides working directory

cvsps version 2.0rc1

fatal: refs/heads/origin: not a valid SHA1
fatal: master: not a valid SHA1
warning: You appear to be on a branch yet to be born.
warning: Forcing checkout of HEAD.
fatal: just how do you expect me to merge 0 trees?
checkout failed: 256
* FAIL 3: import a trivial module
	
	
		git cvsimport -a -z 0 -C module-git module &&
		git diff module-cvs/o_fortuna module-git/o_fortuna
	
	

* expecting success: cd module-git && git gc && cd ..
Nothing new to pack.
*   ok 4: pack refs

* expecting success: 

	cd module-cvs &&
	cat <<EOF >o_fortuna &&
O Fortune,
like the moon
you are changeable,

ever waxing
and waning;
hateful life

first oppresses
and then soothes
as fancy takes it;

poverty
and power
it melts them like ice.
EOF
	cat <<EOF >message &&
translate to English

My Latin is terrible.
EOF
	cvs commit -F message &&
	cd ..

cvs commit: Examining .
/home/internet/GIT/git/t/trash/cvsroot/module/o_fortuna,v  <--  o_fortuna
new revision: 1.2; previous revision: 1.1
*   ok 5: update cvs module

* expecting success: 

	cd module-git &&
	git cvsimport -a -z 0 module &&
	git merge origin &&
	cd .. &&
	git diff module-cvs/o_fortuna module-git/o_fortuna


fatal: Needed a single revision
Branch 'origin' does not exist.
Either use the correct '-o branch' option,
or import to a new repository.
* FAIL 6: update git module
	
	
		cd module-git &&
		git cvsimport -a -z 0 module &&
		git merge origin &&
		cd .. &&
		git diff module-cvs/o_fortuna module-git/o_fortuna
	
	

* expecting success: 

	cd module-cvs &&
		echo 1 >tick &&
		cvs add tick &&
		cvs commit -m 1
	cd ..


./test-lib.sh: line 182: cd: module-cvs: No such file or directory
*   ok 7: update cvs module

* expecting success: 

	cd module-git &&
		git config cvsimport.module module &&
		git cvsimport -a -z0 &&
		git merge origin &&
	cd .. &&
	git diff module-cvs/tick module-git/tick


fatal: Needed a single revision
Branch 'origin' does not exist.
Either use the correct '-o branch' option,
or import to a new repository.
* FAIL 8: cvsimport.module config works
	
	
		cd module-git &&
			git config cvsimport.module module &&
			git cvsimport -a -z0 &&
			git merge origin &&
		cd .. &&
		git diff module-cvs/tick module-git/tick
	
	

* failed 3 among 8 test(s)

Re: make test failure with latest master

[Johannes Schindelin]

Hi,

On Sun, 2 Dec 2007, A Large Angry SCM wrote:

> Jeff King wrote:
> > On Sun, Dec 02, 2007 at 07:29:50AM -0500, A Large Angry SCM wrote:
> > 
> > > With the latest master, 2221a6757161af1905925c405aded9ff470f70d5, "make
> > > test" now fails; last successful "make test" was mid-week sometime with
> > > master d25430. This is on a laptop running Suse 9.3.
> > > 
> > > *** t9600-cvsimport.sh ***
> > > *   ok 1: setup cvsroot
> > > *   ok 2: setup a cvs module
> > > * FAIL 3: import a trivial module
> > > 
> > > 
> > >                 git cvsimport -a -z 0 -C module-git module &&
> > >                 git diff module-cvs/o_fortuna module-git/o_fortuna
> > 
> > Can you please try ./t9600 --verbose? Presumably cvsimport is generating
> > some kind of error message to tell us what's going on.
> 
> Attached is the output of
> 
> 	./t9600-cvsimport.sh --verbose >/tmp/9600_out.txt 2>&1
> 

Unfortunately you attached the interesting part, so I cannot quote it 
here.  Seems that your cvsps does not understand the "-A" option.  So it 
looks like it is too old.

Hth,
Dscho

Re: make test failure with latest master

[Jeff King]

On Sun, Dec 02, 2007 at 11:41:10PM +0000, Johannes Schindelin wrote:

> > Attached is the output of
> > 
> > 	./t9600-cvsimport.sh --verbose >/tmp/9600_out.txt 2>&1
> > 
> 
> Unfortunately you attached the interesting part, so I cannot quote it 
> here.  Seems that your cvsps does not understand the "-A" option.  So it 
> looks like it is too old.

Yes, the changelog indicates that -A was added in 2.1, and he is running
2.0rc1.

We can do something like this, though it feels a bit hack-ish and will
need updated for new cvsps versions (a less lazy or more clever shell
coder than me could do a nice >= version comparator).

-- >8 --
t9600: require cvsps 2.1 to perform tests

git-cvsimport won't run at all with less than cvsps 2.1, because it
lacks the -A flag. But there's no point in preventing people who have an
old cvsps from running the full testsuite.

Signed-off-by: Jeff King <peff@peff.net>
---

diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 08f0f2a..c646111 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -3,13 +3,29 @@
 test_description='git-cvsimport basic tests'
 . ./test-lib.sh
 
-if ! ( type cvs && type cvsps ) >/dev/null 2>&1
+if ! type cvs >/dev/null 2>&1
 then
-	test_expect_success 'skipping cvsimport tests, cvs/cvsps not found' ''
+	test_expect_success 'skipping cvsimport tests, cvs not found' ''
 	test_done
 	exit
 fi
 
+cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
+case "$cvsps_version" in
+2.1)
+	;;
+'')
+	test_expect_success 'skipping cvsimport tests, cvsps not found' ''
+	test_done
+	exit
+	;;
+*)
+	test_expect_success 'skipping cvsimport tests, cvsps too old' ''
+	test_done
+	exit
+	;;
+esac
+
 CVSROOT=$(pwd)/cvsroot
 export CVSROOT
 # for clean cvsps cache

Re: make test failure with latest master

[Johannes Sixt]

Jeff King schrieb:
> -if ! ( type cvs && type cvsps ) >/dev/null 2>&1
> +if ! type cvs >/dev/null 2>&1
>  then
> -	test_expect_success 'skipping cvsimport tests, cvs/cvsps not found' ''
> +	test_expect_success 'skipping cvsimport tests, cvs not found' ''

FWIW, the idiom to provide informative messages in test cases is

	say 'skipping cvsimport tests, cvs not found'

This gives a distinguishing color, too.

-- Hannes

Re: make test failure with latest master

[Jeff King]

On Mon, Dec 03, 2007 at 08:33:27AM +0100, Johannes Sixt wrote:

>> -	test_expect_success 'skipping cvsimport tests, cvs/cvsps not found' ''
>> +	test_expect_success 'skipping cvsimport tests, cvs not found' ''
>
> FWIW, the idiom to provide informative messages in test cases is
>
> 	say 'skipping cvsimport tests, cvs not found'
>
> This gives a distinguishing color, too.

Ah, I adapted the test from t9400, which uses expect_success. Thanks for
the tip.

-Peff

Re: make test failure with latest master

[A Large Angry SCM]

Jeff King wrote:
> On Sun, Dec 02, 2007 at 11:41:10PM +0000, Johannes Schindelin wrote:
> 
>>> Attached is the output of
>>>
>>> 	./t9600-cvsimport.sh --verbose >/tmp/9600_out.txt 2>&1
>>>
>> Unfortunately you attached the interesting part, so I cannot quote it 
>> here.  Seems that your cvsps does not understand the "-A" option.  So it 
>> looks like it is too old.

[Ignoring gitte's declaration that the core of git is dependent on CVS]

> Yes, the changelog indicates that -A was added in 2.1, and he is running
> 2.0rc1.
> 
> We can do something like this, though it feels a bit hack-ish and will
> need updated for new cvsps versions (a less lazy or more clever shell
> coder than me could do a nice >= version comparator).

The patch "fixes" the issue for me.

> -- >8 --
> t9600: require cvsps 2.1 to perform tests
> 
> git-cvsimport won't run at all with less than cvsps 2.1, because it
> lacks the -A flag. But there's no point in preventing people who have an
> old cvsps from running the full testsuite.
> 
> Signed-off-by: Jeff King <peff@peff.net>

Acked-by: A Large Angry SCM <gitzilla@gmail.com>

Re: make test failure with latest master

[Junio C Hamano]

A Large Angry SCM <gitzilla@gmail.com> writes:

> Jeff King wrote:
>> On Sun, Dec 02, 2007 at 11:41:10PM +0000, Johannes Schindelin wrote:
>>
>>>> Attached is the output of
>>>>
>>>> 	./t9600-cvsimport.sh --verbose >/tmp/9600_out.txt 2>&1
>>>>
>>> Unfortunately you attached the interesting part, so I cannot quote
>>> it here.  Seems that your cvsps does not understand the "-A"
>>> option.  So it looks like it is too old.
>
> [Ignoring gitte's declaration that the core of git is dependent on CVS]
>
>> Yes, the changelog indicates that -A was added in 2.1, and he is running
>> 2.0rc1.
>>
>> We can do something like this, though it feels a bit hack-ish and will
>> need updated for new cvsps versions (a less lazy or more clever shell
>> coder than me could do a nice >= version comparator).
>
> The patch "fixes" the issue for me.
>
>> -- >8 --
>> t9600: require cvsps 2.1 to perform tests
>>
>> git-cvsimport won't run at all with less than cvsps 2.1, because it
>> lacks the -A flag. But there's no point in preventing people who have an
>> old cvsps from running the full testsuite.
>>
>> Signed-off-by: Jeff King <peff@peff.net>
>
> Acked-by: A Large Angry SCM <gitzilla@gmail.com>

It would be Tested-by: in a stricter sense of the word but thanks for
testing.

Jeff, would you want to further amend the patch to use "say" as Hannes
suggested to make it look nicer?

Re: make test failure with latest master

[A Large Angry SCM]

Junio C Hamano wrote:
> A Large Angry SCM <gitzilla@gmail.com> writes:
[...]
>> Acked-by: A Large Angry SCM <gitzilla@gmail.com>
> 
> It would be Tested-by: in a stricter sense of the word but thanks for
> testing.

Tested-by: A Large Angry SCM <gitzilla@gmail.com>

[PATCH] t9600: require cvsps 2.1 to perform tests

[Jeff King]

git-cvsimport won't run at all with less than cvsps 2.1, because it
lacks the -A flag. But there's no point in preventing people who have an
old cvsps from running the full testsuite.

Tested-by: A Large Angry SCM <gitzilla@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
On Mon, Dec 03, 2007 at 03:21:55PM -0800, Junio C Hamano wrote:

> Jeff, would you want to further amend the patch to use "say" as Hannes
> suggested to make it look nicer?

Here it is.

 t/t9600-cvsimport.sh |   20 ++++++++++++++++++--
 1 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 08f0f2a..7706430 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -3,13 +3,29 @@
 test_description='git-cvsimport basic tests'
 . ./test-lib.sh
 
-if ! ( type cvs && type cvsps ) >/dev/null 2>&1
+if ! type cvs >/dev/null 2>&1
 then
-	test_expect_success 'skipping cvsimport tests, cvs/cvsps not found' ''
+	say 'skipping cvsimport tests, cvs not found'
 	test_done
 	exit
 fi
 
+cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
+case "$cvsps_version" in
+2.1)
+	;;
+'')
+	say 'skipping cvsimport tests, cvsps not found'
+	test_done
+	exit
+	;;
+*)
+	say 'skipping cvsimport tests, cvsps too old'
+	test_done
+	exit
+	;;
+esac
+
 CVSROOT=$(pwd)/cvsroot
 export CVSROOT
 # for clean cvsps cache
-- 
1.5.3.7.2070.g88cf2-dirty

Re: [PATCH] t9600: require cvsps 2.1 to perform tests

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> +cvsps_version=`cvsps -h 2>&1 | sed -ne 's/cvsps version //p'`
> +case "$cvsps_version" in
> +2.1)
> +	;;
> +'')
> +	say 'skipping cvsimport tests, cvsps not found'
> +	test_done
> +	exit
> +	;;
> +*)
> +	say 'skipping cvsimport tests, cvsps too old'
> +	test_done
> +	exit
> +	;;
> +esac
> +

I wonder if it is better to grep for ' [-A] ' instead, like:

cvsps_supports_A=$(cvsps -h 2>&1 | sed -ne '/\[-A\]/p')
case "$cvsps_supports_A" in
'')
	say 'skipping cvsimport tests, lacking cvsps that supports -A option'
	test_done
        exit
esac

Re: [PATCH] t9600: require cvsps 2.1 to perform tests

[Jeff King]

On Mon, Dec 03, 2007 at 09:37:54PM -0800, Junio C Hamano wrote:

> I wonder if it is better to grep for ' [-A] ' instead, like:
> 
> cvsps_supports_A=$(cvsps -h 2>&1 | sed -ne '/\[-A\]/p')
> case "$cvsps_supports_A" in
> '')
> 	say 'skipping cvsimport tests, lacking cvsps that supports -A option'
> 	test_done
>         exit
> esac

I am not too opposed to that, but I felt that checking the version was a
little bit more future proof. I.e., I have no idea if it is only -A that
we need, but I know that 2.1 works and prior to 2.1 doesn't.

-Jeff

Re: [PATCH] t9600: require cvsps 2.1 to perform tests

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Dec 03, 2007 at 09:37:54PM -0800, Junio C Hamano wrote:
>
>> I wonder if it is better to grep for ' [-A] ' instead, like:
>> 
>> cvsps_supports_A=$(cvsps -h 2>&1 | sed -ne '/\[-A\]/p')
>> case "$cvsps_supports_A" in
>> '')
>> 	say 'skipping cvsimport tests, lacking cvsps that supports -A option'
>> 	test_done
>>         exit
>> esac
>
> I am not too opposed to that, but I felt that checking the version was a
> little bit more future proof. I.e., I have no idea if it is only -A that
> we need, but I know that 2.1 works and prior to 2.1 doesn't.

You are right about "we may be depending on what 2.1 has other than
-A".  Will apply as-is.

Thanks, both.

The primary reason I asked about '\[-A\]' was what will happen if
somebody uses 2.2.

Re: [PATCH] t9600: require cvsps 2.1 to perform tests

[Jeff King]

On Tue, Dec 04, 2007 at 09:39:59AM -0800, Junio C Hamano wrote:

> You are right about "we may be depending on what 2.1 has other than
> -A".  Will apply as-is.
> 
> Thanks, both.
> 
> The primary reason I asked about '\[-A\]' was what will happen if
> somebody uses 2.2.

Yes, it would be nice to be able to easily check >2.1. GNU expr seems to
handle this ok:

  $ expr 2.2 '>' 2.1
  1
  $ expr 2.0 '>' 2.1
  0

but POSIX seems to mention only integers and string comparison (though
if all are of the form "x.y", string comparison works). I have no idea
how portable this is.

-Peff

Re: [PATCH] t9600: require cvsps 2.1 to perform tests

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Dec 04, 2007 at 09:39:59AM -0800, Junio C Hamano wrote:
>
>> You are right about "we may be depending on what 2.1 has other than
>> -A".  Will apply as-is.
>> 
>> Thanks, both.
>> 
>> The primary reason I asked about '\[-A\]' was what will happen if
>> somebody uses 2.2.
>
> Yes, it would be nice to be able to easily check >2.1. GNU expr seems to
> handle this ok:
>
>   $ expr 2.2 '>' 2.1
>   1
>   $ expr 2.0 '>' 2.1
>   0
>
> but POSIX seems to mention only integers and string comparison (though
> if all are of the form "x.y", string comparison works). I have no idea
> how portable this is.

Yeah, but it is the same thing -- we know 2.1 works, we do not know if
2.2 will break things for us ;-)

We'll worry about it when somebody with 2.2 complains.