git-cvsserver: added support for update -p

[PATCH] git-cvsserver: added support for update -p
---
Hi,

Someone in our team uses "cvs update -p [-r rev] file" (somehow invoked through TortoiseCVS). The patch below provides that. I think it is fine, except that I don't know with wich other flags -p can be combined and therefore when exactly this should be tested. Figured out that normal CVS sends the file line-by-line preceeded by "M " using strace on the client to a real CVS server.

Enjoy --- Jan

 git-cvsserver.perl |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)

index 13dbd27..987f4d6 100755
@@ -956,6 +956,21 @@ sub req_update
             $meta = $updater->getmeta($filename);
         }
 
+	# if we have a -p we should just send the file
+        if ( exists ( $state->{opt}{p} ) )
+	{
+	    if ( open my $fh, '-|', "git-cat-file", "blob", $meta->{filehash} )
+	    {   while ( <$fh> )
+		{ print "M " . $_;
+		}
+		close $fh or die ("Couldn't close filehandle for transmitfile(): $!");
+	    } else
+	    { die("Couldn't execute git-cat-file");
+	    }
+
+	    next;
+	}
+
 	if ( ! defined $meta )
 	{
 	    $meta = {

1.5.3.4

Based on a patch by Jan Wielemaker <[email protected]>.

---
 git-cvsserver.perl              |   23 +++++++++++++++++++++++
 t/t9400-git-cvsserver-server.sh |   32 ++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 0 deletions(-)

Test cases added and fixed behaviour for non-existant files.

index 2e112fa..7374875 100755
@@ -973,6 +973,29 @@ sub req_update
             $meta = $updater->getmeta($filename);
         }
 
+	# if we have a -p we should just send the file
+	if ( exists ( $state->{opt}{p} ) )
+	{
+	    if (! defined $meta)
+	    {
+		# non-existant files are ignored
+		print "E cvs update: nothing known about `$filename'\n";
+		next;
+	    }
+	    if ( open my $fh, '-|', "git-cat-file", "blob", $meta->{filehash} )
+	    {
+		while ( <$fh> )
+		{
+		    print "M $_";
+		}
+		close $fh or die ("Couldn't close filehandle: $!");
+	    } else {
+		die("Couldn't execute git-cat-file");
+	    }
+
+	    next;
+	}
+
 	if ( ! defined $meta )
 	{
 	    $meta = {

index ee58c0f..4f45578 100755
@@ -446,6 +446,38 @@ test_expect_success 'cvs update (merge no-op)' \
     diff -q merge ../merge'
 
 cd "$WORKDIR"
+test_expect_success 'cvs update (-p)' \
+  'cd cvswork &&
+   GIT_CONFIG="$git_config" cvs -Q update -p merge non-existant testfile1 empty >log &&
+   cat merge testfile1 empty >../expected &&
+   diff -q log ../expected'
+
+cd "$WORKDIR"
+test_expect_success 'cvs update (-p -r)' \
+  'echo testfile1 >expected &&
+   for i in 1 2 3 4 5 6 7
+   do
+     echo Line $i >>expected
+   done &&
+   echo >>expected &&
+   cd cvswork &&
+   GIT_CONFIG="$git_config" cvs -Q update -p -r1.1 testfile1 merge empty >log &&
+   diff -q log ../expected'
+
+cd "$WORKDIR"
+test_expect_success 'cvs update (-p unclean and out-of-date)' \
+  'echo testfile2 >testfile2 &&
+   echo Line 10 >>merge &&
+   git add testfile2 merge &&
+   git commit -q -m "update -p" &&
+   git push gitcvs.git >/dev/null &&
+   cat testfile2 merge >expected &&
+   cd cvswork &&
+   echo "Line 10 workdir" >>merge
+   GIT_CONFIG="$git_config" cvs -Q update -p testfile2 merge >log &&
+   diff -q log ../expected'
+
+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

1.5.3.4

Thanks. You are a bigger Perl programmer than I :-) Are you also interested in one that makes "cvs diff -c" work? It works, but it does not handle things like "cvs diff -C 5" and I'm a bit lost in Perl-space ... If someone knowing more about the server wants to have a look, I'm happy to post the part I have.

Cheers --- Jan

Hmm, the more half-patches you submit the more I'd rather prefer you learning Perl ;) Or at least write your own testcases.

diff -c doesn't really interest me at all. So I'd really prefer you doing the bulk of the work...

Gruesse,

Frank Lichtenheld <[email protected]>
www: http://www.djpig.de/

Hi,

It is in agreement with the rest of git-cvsserver. I really like the style of the other perl stuff in git better, but I wasn't sure what style takes precedence...

Gruesse,

Frank Lichtenheld <[email protected]>
www: http://www.djpig.de/

That cuddled opening brace hurts mine more.

{ while()\n{ print()...

It's usually a good idea to pick some indentation style that at least *some* tool can create, and when contributing to a project it's usually considered good form to stick to the style already used.

Andreas Ericsson                   [email protected]
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Hi,

Proposed alternative for the commit message:

-- snip --
The cvs subcommand "update -p <file>" is frequently used to see the contents of a given file in HEAD, sort of our "git show <file>". It is not that hard to support it, so here it is.

-- snap --

Remember: having such a commit message already at the beginning of your mail body makes it easier to everyone reading your email, for a small cost (time) of just one person (you).

Ciao,
Dscho

P.S.: Have not reviewed the patch at all, so cannot say anything about the merits of it; will leave it to djpig ;-)

Ok. I'm still a guy of ChangeLog files, which you generally needed for CVS to keep track of a large project :-) As the CVS commit message aren't much good anyway, I kept them short. Also for my own project I'm considering to replace these with larger commit messages and drop the ChangeLog files.

Don't trust my Perl; its just copy and intelligent(-ish) paste :-) Works for me though and this isn't very complicated. Is there a test suite for git-cvsserver?

Cheers --- Jan

Hi,

Yes: t/t9400-git-cvsserver-server.sh

Hth,
Dscho

Thanks. B.t.w. from the main directory:

gollem (git) 21_> make check
for i in *.c; do
sparse -g -O2 -Wall -DSHA1_HEADER='<openssl/sha.h>' -DETC_GITCONFIG='"/home/jan/etc/gitconfig"' -DNO_STRLCPY -D__BIG_ENDIAN__ -D__powerpc__ $i || exit; done
/bin/sh: sparse: command not found
make: *** [check] Error 127

Dunno, but maybe something like this is more appropriate:

echo "See t/README for testing GIT"

Cheers --- Jan

P.s. My modified version passes all tests.

Hi,

make check is to check with the static code analyzer "sparse".

To test, try "make test". Since this is so commonly used to test packages (for example, the vast majority of Perl packages have it), I do not see the need to put a message pointing to "make test" in the "check" target.

Ciao,
Dscho