[PATCH] cvsserver: Fix req_update to handle packed refs

Frank Lichtenheld· Oct 4, 2007, 19:21 UTC · lore

cvsserver returns a list of existing modules on command 'update' without a module specified (apparently this is used by some clients to get a list of available modules, the CVS cli client doesn't support it).

Fix this code to work correctly in presence of packed refs. (Use git-branch instead of reading refs/heads/)

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.de>
---
 git-cvsserver.perl              |   20 +++++++++-----------
 t/t9400-git-cvsserver-server.sh |   26 ++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 11 deletions(-)

 Found while testing Johannes' patch. Is that a sane use of git-branch?

Show changes to 2 files +35 −11

git-cvsserver.perl, t/t9400-git-cvsserver-server.sh

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 7fff18b..acf2e5f 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -914,19 +914,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;
+	my @branches = `git branch`;
+	if ($?) {
+	    print "E [server aborted]: git branch failed\nerror\n";
+	    return;
 	}
-        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";
-	    }
+	foreach (@branches) {
+	    chomp;
+	    s/^\*//;
+	    s/^\s+//;
+	    print "E cvs update: New directory `$_'\n";
 	}
-	closedir HEADS;
 	print "ok\n";
 	return 1;
     }
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 11def84..106a696 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -447,4 +447,30 @@ test_expect_success 'cvs update (merge no-op)' \
     GIT_CONFIG="$git_config" cvs -Q update &&
     diff -q merge ../merge'
 
+cd "$WORKDIR"
+cat <<EOF >list-modules-cmd
+Root $SERVERDIR
+Valid-responses ok error Valid-requests Force-gzip Referrer Redirect Checked-in New-entry Checksum Copy-file Updated Created Update-existing Merged Patched Rcs-diff Mode Mod-time Removed Remove-entry Set-static-directory Clear-static-directory Set-sticky Clear-sticky Edit-file Template Clear-template Notified Module-expansion Wrapper-rcsOption M Mbinary E F MT
+Global_option -q
+Directory .
+$WORKDIR
+update
+EOF
+
+echo \`master\' >modules-list.exp
+
+# can't do these with the CVS cli client, but others clients
+# use that feature
+test_expect_success 'cvs update (no arg)' \
+   'cat list-modules-cmd | git-cvsserver |
+    sed -n "s/^E cvs update: New directory //p" >modules-list &&
+    diff -q modules-list modules-list.exp'
+
+test_expect_success 'cvs update (no arg/packed refs)' \
+   'GIT_DIR="$SERVERDIR" git gc &&
+    test ! -f "$SERVERDIR/refs/heads/master" &&
+    cat list-modules-cmd | git-cvsserver |
+    sed -n "s/^E cvs update: New directory //p" >modules-list &&
+    diff -q modules-list modules-list.exp'
+    
 test_done

-- 
1.5.3.3

Martin Langhoff· Oct 4, 2007, 20:43 UTC · re: Frank Lichtenheld · lore

Re: [PATCH] cvsserver: Fix req_update to handle packed refs

On 10/5/07, Frank Lichtenheld <frank@lichtenheld.de> wrote:

Show 7 quoted lines

> cvsserver returns a list of existing modules on command
> 'update' without a module specified (apparently this is
> used by some clients to get a list of available modules,
> the CVS cli client doesn't support it).
>
> Fix this code to work correctly in presence of packed refs.
> (Use git-branch instead of reading refs/heads/)

ACK - good stuff - thanks!

There is one minor issue around this I suspect - refs with slashes in them. Without this patch, only refs that literally sit in refs/heads will be returned. With git branches, you could see oldbranches/foo being returned to the client.

IIRC - the behaviour cvsserver supports here is completely unspecified, and clients will probably error out in weird and wacky ways. I'd perhaps filter out any headref with a slash.

cheers,

martin

Frank Lichtenheld· Oct 4, 2007, 20:58 UTC · re: Martin Langhoff · lore

Re: [PATCH] cvsserver: Fix req_update to handle packed refs

On Fri, Oct 05, 2007 at 09:43:55AM +1300, Martin Langhoff wrote:

Show 19 quoted lines

> On 10/5/07, Frank Lichtenheld <frank@lichtenheld.de> wrote:
> > cvsserver returns a list of existing modules on command
> > 'update' without a module specified (apparently this is
> > used by some clients to get a list of available modules,
> > the CVS cli client doesn't support it).
> >
> > Fix this code to work correctly in presence of packed refs.
> > (Use git-branch instead of reading refs/heads/)
> 
> ACK - good stuff - thanks!
> 
> There is one minor issue around this I suspect - refs with slashes in
> them. Without this patch, only refs that literally sit in refs/heads
> will be returned. With git branches, you could see oldbranches/foo
> being returned to the client.
> 
> IIRC - the behaviour cvsserver supports here is completely
> unspecified, and clients will probably error out in weird and wacky
> ways. I'd perhaps filter out any headref with a slash.

cvsserver also horribly breaks for module names with slashs in them. Something that might be good to be fixed, too.

Gruesse,

-- 
Frank Lichtenheld <frank@lichtenheld.de>
www: http://www.djpig.de/

patchcvsserver: Fix req_update to handle packed refs