threads / patch / 14504

patchtestsuite for cvs co -c

Subject: [PATCH] testsuite for cvs co -c

## tl;dr

11 messages between Jul 17, 2008 and Jul 17, 2008. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

Lars Noschinski· Jul 17, 2008, 10:01 UTC · lore

Some cvs related fixes and enhancements

This patch series
    - fixes a small bug in the cvsimport testsuite
    - adds support for packed-refs to cvsserver
    - adds basic support cvs co -c to cvsserver
---
 git-cvsserver.perl              |   38 ++++++++++++++++++++++----------------
 t/t9400-git-cvsserver-server.sh |   25 +++++++++++++++++++++++++
 t/t9600-cvsimport.sh            |    1 +
 3 files changed, 48 insertions(+), 16 deletions(-)
Lars Noschinski· Jul 17, 2008, 10:01 UTC · re: Lars Noschinski · lore

[PATCH] Testsuite: Unset CVS_SERVER

From: Fabian Emmes <fabian.emmes@rwth-aachen.de>

The CVS_SERVER environment variable cane cause some of the cvsimport tests to fail. So unset this variable at the beginning of the test script.

Signed-off-by: Fabian Emmes <fabian.emmes@rwth-aachen.de>
Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
---
 t/t9600-cvsimport.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
Show changes to t/t9600-cvsimport.sh +1 −0
diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 1e01e5c..0d7786a 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -5,6 +5,7 @@ test_description='git-cvsimport basic tests'
 
 CVSROOT=$(pwd)/cvsroot
 export CVSROOT
+unset CVS_SERVER
 # for clean cvsps cache
 HOME=$(pwd)
 export HOME
-- 
1.5.6.2
Lars Noschinski· Jul 17, 2008, 10:01 UTC · re: Lars Noschinski · lore

[PATCH] cvsserver: Add support for packed refs

req_update still parses /refs/heads manually. Replace this by a call to show-ref.

Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
---
 git-cvsserver.perl |   25 +++++++++----------------
 1 files changed, 9 insertions(+), 16 deletions(-)
Show changes to git-cvsserver.perl +9 −16
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index b00d1c2..0e4f5f9 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -947,24 +947,17 @@ sub req_update
     # projects (heads in this case) to checkout.
     #
     if ($state->{module} eq '') {
-	my $heads_dir = $state->{CVSROOT} . '/refs/heads';
-	if (!opendir HEADS, $heads_dir) {
-	    print "E [server aborted]: Failed to open directory, "
-	      . "$heads_dir: $!\nerror\n";
-	    return 0;
-	}
-        print "E cvs update: Updating .\n";
-	while (my $head = readdir(HEADS)) {
-	    if (-f $state->{CVSROOT} . '/refs/heads/' . $head) {
-	        print "E cvs update: New directory `$head'\n";
-	    }
-	}
-	closedir HEADS;
-	print "ok\n";
-	return 1;
+        my $showref = `git show-ref --heads`;
+        for my $line (split '\n', $showref) {
+            if ( $line =~ m% refs/heads/(.*)$% ) {
+                print "M $1\t$1\n";
+            }
+        }
+        closedir HEADS;
+        print "ok\n";
+        return 1;
     }
 
-
     # Grab a handle to the SQLite db and do any necessary updates
     my $updater = GITCVS::updater->new($state->{CVSROOT}, $state->{module}, $log);
 
-- 
1.5.6.2
Lars Noschinski· Jul 17, 2008, 10:01 UTC · re: Lars Noschinski · lore

[PATCH] cvsserver: Add testsuite for packed refs

From: Fabian Emmes <fabian.emmes@rwth-aachen.de>
Check that req_update shows refs, even if all refs are packed.
Signed-off-by: Fabian Emmes <fabian.emmes@rwth-aachen.de>
Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
---
 t/t9400-git-cvsserver-server.sh |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)
Show changes to t/t9400-git-cvsserver-server.sh +14 −0
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index e97aaa6..d181b5b 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -438,6 +438,20 @@ test_expect_success 'cvs update (-p)' '
     test -z "$(cat failures)"
 '
 
+cd "$WORKDIR"
+cat > get_update_modules <<EOF
+Root $SERVERDIR
+Directory .
+$SERVERDIR
+update
+EOF
+
+test_expect_success 'cvs update (module list supports packed refs)' '
+    git pack-refs --all &&
+    git cvsserver server < get_update_modules > out &&
+    grep "^M master[	 ]\+master$" < out
+'
+
 #------------
 # CVS STATUS
 #------------
-- 
1.5.6.2
Lars Noschinski· Jul 17, 2008, 10:01 UTC · re: Lars Noschinski · lore

[PATCH] cvsserver: Add cvs co -c support

Implement cvs checkout's -c option by returning a list of all "modules". This is more useful than displaying a perl warning if -c is given.

Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
---
 git-cvsserver.perl |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
Show changes to git-cvsserver.perl +13 −0
diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 0e4f5f9..afd9789 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -801,6 +801,19 @@ sub req_co
 
     argsplit("co");
 
+    # Provide list of modules, if -c was used.
+    if (exists $state->{opt}{c}) {
+        my $showref = `git show-ref --heads`;
+        for my $line (split '\n', $showref) {
+            if ( $line =~ m% refs/heads/(.*)$% ) {
+                print "M $1\t$1\n";
+            }
+        }
+        closedir HEADS;
+        print "ok\n";
+        return 1;
+    }
+
     my $module = $state->{args}[0];
     $state->{module} = $module;
     my $checkout_path = $module;
-- 
1.5.6.2
Lars Noschinski· Jul 17, 2008, 10:01 UTC · re: Lars Noschinski · lore
From: Fabian Emmes <fabian.emmes@rwth-aachen.de>
Check that all branches are displayed.
Signed-off-by: Fabian Emmes <fabian.emmes@rwth-aachen.de>
Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
---
 t/t9400-git-cvsserver-server.sh |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
Show changes to t/t9400-git-cvsserver-server.sh +11 −0
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index d181b5b..3a59b9f 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -484,4 +484,15 @@ test_expect_success 'cvs status (no subdirs in header)' '
     ! grep / <../out
 '
 
+#------------
+# CVS CHECKOUT
+#------------
+
+cd "$WORKDIR"
+test_expect_success 'cvs co -c (shows module database)' '
+    GIT_CONFIG="$git_config" cvs co -c > out &&
+    grep "^master[	 ]\+master$" < out &&
+    ! grep -v "^master[	 ]\+master$" < out
+'
+
 test_done
-- 
1.5.6.2
Johannes Schindelin· Jul 17, 2008, 11:54 UTC · re: Lars Noschinski · lore

Re: [PATCH] cvsserver: Add testsuite for packed refs

Hi,
On Thu, 17 Jul 2008, Lars Noschinski wrote:
> +    grep "^M master[	 ]\+master$" < out

As I said, I am not versed enough in the Teachings of CVS to know if this is correct.

If the old behaviour ("E cvs ...") was "more" correct, this needs changing, too.

Ciao, Dscho

Johannes Schindelin· Jul 17, 2008, 11:50 UTC · re: Lars Noschinski · lore

Re: [PATCH] cvsserver: Add support for packed refs

Hi,
On Thu, 17 Jul 2008, Lars Noschinski wrote:
Show 35 quoted lines
> req_update still parses /refs/heads manually. Replace this by
> a call to show-ref.
> 
> Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
> ---
>  git-cvsserver.perl |   25 +++++++++----------------
>  1 files changed, 9 insertions(+), 16 deletions(-)
> 
> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
> index b00d1c2..0e4f5f9 100755
> --- a/git-cvsserver.perl
> +++ b/git-cvsserver.perl
> @@ -947,24 +947,17 @@ sub req_update
>      # projects (heads in this case) to checkout.
>      #
>      if ($state->{module} eq '') {
> -	my $heads_dir = $state->{CVSROOT} . '/refs/heads';
> -	if (!opendir HEADS, $heads_dir) {
> -	    print "E [server aborted]: Failed to open directory, "
> -	      . "$heads_dir: $!\nerror\n";
> -	    return 0;
> -	}
> -        print "E cvs update: Updating .\n";
> -	while (my $head = readdir(HEADS)) {
> -	    if (-f $state->{CVSROOT} . '/refs/heads/' . $head) {
> -	        print "E cvs update: New directory `$head'\n";
> -	    }
> -	}
> -	closedir HEADS;
> -	print "ok\n";
> -	return 1;
> +        my $showref = `git show-ref --heads`;
> +        for my $line (split '\n', $showref) {
> +            if ( $line =~ m% refs/heads/(.*)$% ) {
> +                print "M $1\t$1\n";

In the removed part, I see that this changes behaviour from "E cvs update: ..." to "M ...".

I do not know the CVS protocol well enough to know if that is still correct.

BTW from the removed part, it seems that the indentation was done with tabs formerly, and with spaces now; please use tabs instead.

Thanks, Dscho

Lars Noschinski· Jul 17, 2008, 12:40 UTC · re: Johannes Schindelin · lore

Re: [PATCH] cvsserver: Add support for packed refs

Hello!
* Johannes Schindelin <Johannes.Schindelin@gmx.de> [08-07-17 14:31]:
Show 40 quoted lines
>On Thu, 17 Jul 2008, Lars Noschinski wrote:
>
>> req_update still parses /refs/heads manually. Replace this by
>> a call to show-ref.
>> 
>> Signed-off-by: Lars Noschinski <lars@public.noschinski.de>
>> ---
>>  git-cvsserver.perl |   25 +++++++++----------------
>>  1 files changed, 9 insertions(+), 16 deletions(-)
>> 
>> diff --git a/git-cvsserver.perl b/git-cvsserver.perl
>> index b00d1c2..0e4f5f9 100755
>> --- a/git-cvsserver.perl
>> +++ b/git-cvsserver.perl
>> @@ -947,24 +947,17 @@ sub req_update
>>      # projects (heads in this case) to checkout.
>>      #
>>      if ($state->{module} eq '') {
>> -	my $heads_dir = $state->{CVSROOT} . '/refs/heads';
>> -	if (!opendir HEADS, $heads_dir) {
>> -	    print "E [server aborted]: Failed to open directory, "
>> -	      . "$heads_dir: $!\nerror\n";
>> -	    return 0;
>> -	}
>> -        print "E cvs update: Updating .\n";
>> -	while (my $head = readdir(HEADS)) {
>> -	    if (-f $state->{CVSROOT} . '/refs/heads/' . $head) {
>> -	        print "E cvs update: New directory `$head'\n";
>> -	    }
>> -	}
>> -	closedir HEADS;
>> -	print "ok\n";
>> -	return 1;
>> +        my $showref = `git show-ref --heads`;
>> +        for my $line (split '\n', $showref) {
>> +            if ( $line =~ m% refs/heads/(.*)$% ) {
>> +                print "M $1\t$1\n";
>
>In the removed part, I see that this changes behaviour from "E cvs update: 
>..." to "M ...".

Good catch, this part is indeed somewhat busted. The tested clients did not care, but this should be changed nevertheless to be the same as the old behaviour.

Show 5 quoted lines
>I do not know the CVS protocol well enough to know if that is still 
>correct.
>
>BTW from the removed part, it seems that the indentation was done with 
>tabs formerly, and with spaces now; please use tabs instead.

Actually, the old indetation was wrong. The biggest part of the file (and the surrounding code) is indented by spaces.

Johannes Schindelin· Jul 17, 2008, 11:45 UTC · re: Lars Noschinski · lore

Re: [PATCH] Testsuite: Unset CVS_SERVER

Hi,
On Thu, 17 Jul 2008, Lars Noschinski wrote:
> From: Fabian Emmes <fabian.emmes@rwth-aachen.de>
> 
> The CVS_SERVER environment variable cane cause some of the cvsimport tests
s/cane/can/
Otherwise uncontroversial, I'd say.

Ciao, Dscho

Jeff King· Jul 17, 2008, 17:53 UTC · re: Lars Noschinski · lore

Re: [PATCH] Testsuite: Unset CVS_SERVER

On Thu, Jul 17, 2008 at 12:01:13PM +0200, Lars Noschinski wrote:
> The CVS_SERVER environment variable cane cause some of the cvsimport tests
> to fail. So unset this variable at the beginning of the test script.

This is definitely an improvement. However, the cvs manual lists a number of CVS* variables. Perhaps it would be better to simply scrub the environment of any variable matching that pattern? I don't know how commonly used some of the other ones are.

-Peff

← back to recent threads