make test failure with latest master

[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)

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

-Peff

Attached is the output of

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

* 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)

Hi,

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

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.

---

index 08f0f2a..c646111 100755
@@ -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

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

-Peff

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

The patch "fixes" the issue for me.

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?

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.

---

Here it is.

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

index 08f0f2a..7706430 100755
@@ -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

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

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

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.

[...]

Trivial question: have you got cvs and cvsps installed?

Jakub Narebski

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