[PATCH 0/7] Enhance TkCVS interoperability

[PATCH 0/7] Enhance TkCVS interoperability

[Damien Diederen]

This series was developed to improve interoperability between
git-cvsserver and TkCVS, a CVS client that is fairly popular at least
in some corporate environments.

Patches 1-6 are "obvious" improvements, implementing missing features
or fixing incorrect behaviour.  7 is more questionable, but likely to
result in more intelligible log output in a majority of cases.

Bear in mind that my perl-fu is not very high when reviewing this
series; suggestions are more than welcome!

        Damien

Damien Diederen (7):
  cvsserver: Respond to the 'editors' command.
  cvsserver: Only print the file part of filename in status header.
  cvsserver: Do not include status output for subdirectories if -l is
    passed.
  cvsserver: Add a few tests for 'status' command.
  cvsserver: Implemented update -p (print to stdout)
  cvsserver: Added test for update -p
  cvsserver: Use the user part of the email in log and annotate results

 git-cvsserver.perl              |   67 ++++++++++++++++++++++++++++++--------
 t/t9400-git-cvsserver-server.sh |   50 +++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 14 deletions(-)

[PATCH 1/7] cvsserver: Respond to the 'editors' command

[Damien Diederen]

"Cvs editors" lists the users currently working on watched (locked)
files.  This trivial implementation always returns an empty response,
since git-cvsserver does not implement file locking.

Without this, TkCVS hangs at startup, waiting forever for a response.

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 git-cvsserver.perl |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 7f632af..33d30c5 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -74,7 +74,7 @@ my $methods = {
     'admin'           => \&req_CATCHALL,
     'history'         => \&req_CATCHALL,
     'watchers'        => \&req_CATCHALL,
-    'editors'         => \&req_CATCHALL,
+    'editors'         => \&req_editors,
     'annotate'        => \&req_annotate,
     'Global_option'   => \&req_Globaloption,
     #'annotate'        => \&req_CATCHALL,
@@ -1489,6 +1489,11 @@ sub req_status
     print "ok\n";
 }
 
+sub req_editors
+{
+    print "ok\n";
+}
+
 sub req_diff
 {
     my ( $cmd, $data ) = @_;
-- 
1.5.5.rc1.6.gd183

[PATCH 2/7] cvsserver: Only print the file part of the filename in status header

[Damien Diederen]

The "File:" header of CVS status output only includes the basename of
the file, even when generating a recursive listing; do the same.

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 git-cvsserver.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 33d30c5..9101eef 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1466,8 +1466,10 @@ sub req_status
 
         $status ||= "Unknown";
 
+        my ($filepart) = filenamesplit($filename);
+
         print "M ===================================================================\n";
-        print "M File: $filename\tStatus: $status\n";
+        print "M File: $filepart\tStatus: $status\n";
         if ( defined($state->{entries}{$filename}{revision}) )
         {
             print "M Working revision:\t" . $state->{entries}{$filename}{revision} . "\n";
-- 
1.5.5.rc1.6.gd183

[PATCH 3/7] cvsserver: Do not include status output for subdirectories if -l is passed

[Damien Diederen]

This effectively implements the -l switch by pruning the entries whose
filenames contain a path separator.  It was previously ignored.

Without this, TkCVS includes strange "ghost" entries in its directory
listings.

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 git-cvsserver.perl |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 9101eef..073a426 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1423,6 +1423,8 @@ sub req_status
     {
         $filename = filecleanup($filename);
 
+        next if exists($state->{opt}{l}) && index($filename, '/', length($state->{prependdir})) >= 0;
+
         my $meta = $updater->getmeta($filename);
         my $oldmeta = $meta;
 
-- 
1.5.5.rc1.6.gd183

[PATCH 4/7] cvsserver: Add a few tests for 'status' command

[Damien Diederen]

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 t/t9400-git-cvsserver-server.sh |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index b91b151..6168324 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -420,4 +420,36 @@ test_expect_success 'cvs update (merge no-op)' \
     GIT_CONFIG="$git_config" cvs -Q update &&
     diff -q merge ../merge'
 
+#------------
+# CVS STATUS
+#------------
+
+cd "$WORKDIR"
+test_expect_success 'cvs status' '
+    mkdir status.dir &&
+    echo Line > status.dir/status.file &&
+    echo Line > status.file &&
+    git add status.dir status.file &&
+    git commit -q -m "Status test" &&
+    git push gitcvs.git >/dev/null &&
+    cd cvswork &&
+    GIT_CONFIG="$git_config" cvs update &&
+    GIT_CONFIG="$git_config" cvs status | grep "^File: status.file" >../out &&
+    test $(wc -l <../out) = 2
+'
+
+cd "$WORKDIR"
+test_expect_success 'cvs status (nonrecursive)' '
+    cd cvswork &&
+    GIT_CONFIG="$git_config" cvs status -l | grep "^File: status.file" >../out &&
+    test $(wc -l <../out) = 1
+'
+
+cd "$WORKDIR"
+test_expect_success 'cvs status (no subdirs in header)' '
+    cd cvswork &&
+    GIT_CONFIG="$git_config" cvs status | grep ^File: >../out &&
+    ! grep / <../out
+'
+
 test_done
-- 
1.5.5.rc1.6.gd183

[PATCH 5/7] cvsserver: Implemented update -p (print to stdout)

[Damien Diederen]

Cvs update -p -r <rev> <path> is the documented way to retrieve a
specific revision of a file (similar to git show <rev>:<path>).
Without this patch, the -p flag is ignored and status output is
produced, causing clients to interpret it as the contents of the file.

TkCVS uses update -p as a basis for implementing its various "View"
and "Diff" commands.

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 git-cvsserver.perl |   36 ++++++++++++++++++++++++++++--------
 1 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 073a426..3c97226 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -958,6 +958,17 @@ sub req_update
             $meta = $updater->getmeta($filename);
         }
 
+        # If -p was given, "print" the contents of the requested revision.
+        if ( exists ( $state->{opt}{p} ) ) {
+            if ( defined ( $meta->{revision} ) ) {
+                $log->info("Printing '$filename' revision " . $meta->{revision});
+
+                transmitfile($meta->{filehash}, { print => 1 });
+            }
+
+            next;
+        }
+
 	if ( ! defined $meta )
 	{
 	    $meta = {
@@ -1091,9 +1102,9 @@ sub req_update
             my $file_local = $filepart . ".mine";
             system("ln","-s",$state->{entries}{$filename}{modified_filename}, $file_local);
             my $file_old = $filepart . "." . $oldmeta->{revision};
-            transmitfile($oldmeta->{filehash}, $file_old);
+            transmitfile($oldmeta->{filehash}, { targetfile => $file_old });
             my $file_new = $filepart . "." . $meta->{revision};
-            transmitfile($meta->{filehash}, $file_new);
+            transmitfile($meta->{filehash}, { targetfile => $file_new });
 
             # we need to merge with the local changes ( M=successful merge, C=conflict merge )
             $log->info("Merging $file_local, $file_old, $file_new");
@@ -1550,14 +1561,14 @@ sub req_diff
                 print "E File $filename at revision 1.$revision1 doesn't exist\n";
                 next;
             }
-            transmitfile($meta1->{filehash}, $file1);
+            transmitfile($meta1->{filehash}, { targetfile => $file1 });
         }
         # otherwise we just use the working copy revision
         else
         {
             ( undef, $file1 ) = tempfile( DIR => $TEMP_DIR, OPEN => 0 );
             $meta1 = $updater->getmeta($filename, $wrev);
-            transmitfile($meta1->{filehash}, $file1);
+            transmitfile($meta1->{filehash}, { targetfile => $file1 });
         }
 
         # if we have a second -r switch, use it too
@@ -1572,7 +1583,7 @@ sub req_diff
                 next;
             }
 
-            transmitfile($meta2->{filehash}, $file2);
+            transmitfile($meta2->{filehash}, { targetfile => $file2 });
         }
         # otherwise we just use the working copy
         else
@@ -1585,7 +1596,7 @@ sub req_diff
         {
             ( undef, $file2 ) = tempfile( DIR => $TEMP_DIR, OPEN => 0 );
             $meta2 = $updater->getmeta($filename, $wrev);
-            transmitfile($meta2->{filehash}, $file2);
+            transmitfile($meta2->{filehash}, { targetfile => $file2 });
         }
 
         # We need to have retrieved something useful
@@ -2021,7 +2032,7 @@ sub revparse
 sub transmitfile
 {
     my $filehash = shift;
-    my $targetfile = shift;
+    my $options = shift;
 
     if ( defined ( $filehash ) and $filehash eq "deleted" )
     {
@@ -2043,11 +2054,20 @@ sub transmitfile
 
     if ( open my $fh, '-|', "git-cat-file", "blob", $filehash )
     {
-        if ( defined ( $targetfile ) )
+        if ( defined ( $options->{targetfile} ) )
         {
+            my $targetfile = $options->{targetfile};
             open NEWFILE, ">", $targetfile or die("Couldn't open '$targetfile' for writing : $!");
             print NEWFILE $_ while ( <$fh> );
             close NEWFILE or die("Failed to write '$targetfile': $!");
+        } elsif ( defined ( $options->{print} ) && $options->{print} ) {
+            while ( <$fh> ) {
+                if( /\n\z/ ) {
+                    print 'M ', $_;
+                } else {
+                    print 'MT text ', $_, "\n";
+                }
+            }
         } else {
             print "$size\n";
             print while ( <$fh> );
-- 
1.5.5.rc1.6.gd183

[PATCH 6/7] cvsserver: Added test for update -p

[Damien Diederen]

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 t/t9400-git-cvsserver-server.sh |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 6168324..166b43f 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -420,6 +420,24 @@ test_expect_success 'cvs update (merge no-op)' \
     GIT_CONFIG="$git_config" cvs -Q update &&
     diff -q merge ../merge'
 
+cd "$WORKDIR"
+test_expect_success 'cvs update (-p)' '
+    touch really-empty &&
+    echo Line 1 > no-lf &&
+    echo -n Line 2 >> no-lf &&
+    git add really-empty no-lf &&
+    git commit -q -m "Update -p test" &&
+    git push gitcvs.git >/dev/null &&
+    cd cvswork &&
+    GIT_CONFIG="$git_config" cvs update &&
+    rm -f failures &&
+    for i in merge no-lf empty really-empty; do
+        GIT_CONFIG="$git_config" cvs update -p "$i" >$i.out
+        diff $i.out ../$i >>failures 2>&1
+    done &&
+    test -z "$(cat failures)"
+'
+
 #------------
 # CVS STATUS
 #------------
-- 
1.5.5.rc1.6.gd183

[PATCH 7/7] cvsserver: Use the user part of the email in log and annotate results

[Damien Diederen]

Generate the CVS author names by taking the first eight characters of
the user part of the email address.  The resulting names are more
likely to make sense (or at least reduce ambiguities) in "corporate"
environments.

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 git-cvsserver.perl |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 3c97226..9d845c8 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1728,8 +1728,7 @@ sub req_log
             print "M revision 1.$revision->{revision}\n";
             # reformat the date for log output
             $revision->{modified} = sprintf('%04d/%02d/%02d %s', $3, $DATE_LIST->{$2}, $1, $4 ) if ( $revision->{modified} =~ /(\d+)\s+(\w+)\s+(\d+)\s+(\S+)/ and defined($DATE_LIST->{$2}) );
-            $revision->{author} =~ s/\s+.*//;
-            $revision->{author} =~ s/^(.{8}).*/$1/;
+            $revision->{author} = cvs_author($revision->{author});
             print "M date: $revision->{modified};  author: $revision->{author};  state: " . ( $revision->{filehash} eq "deleted" ? "dead" : "Exp" ) . ";  lines: +2 -3\n";
             my $commitmessage = $updater->commitmessage($revision->{commithash});
             $commitmessage =~ s/^/M /mg;
@@ -1844,8 +1843,7 @@ sub req_annotate
                 unless ( defined ( $metadata->{$commithash} ) )
                 {
                     $metadata->{$commithash} = $updater->getmeta($filename, $commithash);
-                    $metadata->{$commithash}{author} =~ s/\s+.*//;
-                    $metadata->{$commithash}{author} =~ s/^(.{8}).*/$1/;
+                    $metadata->{$commithash}{author} = cvs_author($metadata->{$commithash}{author});
                     $metadata->{$commithash}{modified} = sprintf("%02d-%s-%02d", $1, $2, $3) if ( $metadata->{$commithash}{modified} =~ /^(\d+)\s(\w+)\s\d\d(\d\d)/ );
                 }
                 printf("M 1.%-5d      (%-8s %10s): %s\n",
@@ -2136,6 +2134,18 @@ sub kopts_from_path
     }
 }
 
+# Generate a CVS author name from Git author information, by taking
+# the first eight characters of the user part of the email address.
+sub cvs_author
+{
+    my $author = shift;
+
+    $author =~ s/.*<([^>]+)\@[^>]+>$/$1/;
+    $author =~ s/^(.{8}).*/$1/;
+
+    $author;
+}
+
 package GITCVS::log;
 
 ####
-- 
1.5.5.rc1.6.gd183

Re: [PATCH 0/7] Enhance TkCVS interoperability

[Junio C Hamano]

Damien Diederen <dash@foobox.net> writes:

> This series was developed to improve interoperability between
> git-cvsserver and TkCVS, a CVS client that is fairly popular at least
> in some corporate environments.
>
> Patches 1-6 are "obvious" improvements, implementing missing features
> or fixing incorrect behaviour.  7 is more questionable, but likely to
> result in more intelligible log output in a majority of cases.

If anything, I think 7/7 is an improvement that consolidates a few
duplicated code that massage authorship information in the commit object
into CVS form.  I cannot readily tell what is going on from this old code
sequence:

-    $metadata->{$commithash}{author} =~ s/\s+.*//;
-    $metadata->{$commithash}{author} =~ s/^(.{8}).*/$1/;

but I can tell what is going on in the latter even without the help from
the leading comment.

+# Generate a CVS author name from Git author information, by taking
+# the first eight characters of the user part of the email address.
+sub cvs_author
+{
+    my $author = shift;
+
+    $author =~ s/.*<([^>]+)\@[^>]+>$/$1/;
+    $author =~ s/^(.{8}).*/$1/;
+
+    $author;
+}

And 1/7-6/7 looked all good, but this is just from _looking_.  I do not
run cvsserver myself, so people should take this with a moderate amount of
salt.

Martin, Frank?

Re: [PATCH 1/7] cvsserver: Respond to the 'editors' command

[Frank Lichtenheld]

On Mon, Mar 24, 2008 at 11:48:52PM +0100, Damien Diederen wrote:
> "Cvs editors" lists the users currently working on watched (locked)
> files.  This trivial implementation always returns an empty response,
> since git-cvsserver does not implement file locking.

It might be nicer to name the function something like req_dummy_response
and use it for (at least) editors and watchers commands.

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

Re: [PATCH 7/7] cvsserver: Use the user part of the email in log and annotate results

[Frank Lichtenheld]

On Mon, Mar 24, 2008 at 11:50:55PM +0100, Damien Diederen wrote:
> +# Generate a CVS author name from Git author information, by taking
> +# the first eight characters of the user part of the email address.
> +sub cvs_author
> +{
> +    my $author = shift;
> +
> +    $author =~ s/.*<([^>]+)\@[^>]+>$/$1/;
> +    $author =~ s/^(.{8}).*/$1/;

IMHO substr($author, 0, 8) would be easier to read here. (It is also
much faster according to some quick benchmarks I just ran)

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

Re: [PATCH 7/7] cvsserver: Use the user part of the email in log and annotate results

[Rafael Garcia-Suarez]

On 25/03/2008, Frank Lichtenheld <frank@lichtenheld.de> wrote:
> On Mon, Mar 24, 2008 at 11:50:55PM +0100, Damien Diederen wrote:
>  > +# Generate a CVS author name from Git author information, by taking
>  > +# the first eight characters of the user part of the email address.
>  > +sub cvs_author
>  > +{
>  > +    my $author = shift;
>  > +
>  > +    $author =~ s/.*<([^>]+)\@[^>]+>$/$1/;
>  > +    $author =~ s/^(.{8}).*/$1/;
>
>
> IMHO substr($author, 0, 8) would be easier to read here. (It is also
>  much faster according to some quick benchmarks I just ran)

While we're at nitpicking:
Faster, shorter, and probably more robust if no @ appears in the email address:

my $author_line = shift;
(my $author) = $author_line =~ /<([^>@]{1,8})/;

Re: [PATCH 7/7] cvsserver: Use the user part of the email in log and annotate results

[Damien Diederen]

Hi All,

"Rafael Garcia-Suarez" <rgarciasuarez@gmail.com> writes:
> On 25/03/2008, Frank Lichtenheld <frank@lichtenheld.de> wrote:
>> On Mon, Mar 24, 2008 at 11:50:55PM +0100, Damien Diederen wrote:
>>  > +# Generate a CVS author name from Git author information, by taking
>>  > +# the first eight characters of the user part of the email address.
>>  > +sub cvs_author
>>  > +{
>>  > +    my $author = shift;
>>  > +
>>  > +    $author =~ s/.*<([^>]+)\@[^>]+>$/$1/;
>>  > +    $author =~ s/^(.{8}).*/$1/;
>>
>> IMHO substr($author, 0, 8) would be easier to read here. (It is also
>>  much faster according to some quick benchmarks I just ran)
>
> While we're at nitpicking:
> Faster, shorter, and probably more robust if no @ appears in the email address:
>
> my $author_line = shift;
> (my $author) = $author_line =~ /<([^>@]{1,8})/;

Keep 'em coming :)

I agree with all suggestions so far; I will prepare a new series
addressing them (and including a minor documentation update) once the
thread has settled down a bit.

        Damien

-- 
http://foobox.net/~dash/

[PATCH v2 0/7] Enhance TkCVS interoperability

[Damien Diederen]

This series was developed to improve interoperability between
git-cvsserver and TkCVS, a CVS client that is fairly popular at least
in some corporate environments.

This revised version incorporates suggestions from Frank Lichtenheld
and Rafael Garcia-Suarez, plus a small internal documentation update.
I have no outstanding changes planned for the near future.  Junio,
this can go in as far as I am concerned.

        Damien

Damien Diederen (7):
  cvsserver: Respond to the 'editors' and 'watchers' commands
  cvsserver: Only print the file part of the filename in status header
  cvsserver: Do not include status output for subdirectories if -l is
    passed
  cvsserver: Add a few tests for 'status' command
  cvsserver: Implement update -p (print to stdout)
  cvsserver: Add test for update -p
  cvsserver: Use the user part of the email in log and annotate results

 git-cvsserver.perl              |   78 +++++++++++++++++++++++++++++---------
 t/t9400-git-cvsserver-server.sh |   50 +++++++++++++++++++++++++
 2 files changed, 109 insertions(+), 19 deletions(-)

[PATCH v2 1/7] cvsserver: Respond to the 'editors' and 'watchers' commands

[Damien Diederen]

These commands list users editing and watching locked files.  This trivial
implementation always returns an empty response, since git-cvsserver does not
implement file locking.

Without this, TkCVS hangs at startup, waiting forever for a response.

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 git-cvsserver.perl |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 7f632af..2fe0a8a 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -73,8 +73,8 @@ my $methods = {
     'status'          => \&req_status,
     'admin'           => \&req_CATCHALL,
     'history'         => \&req_CATCHALL,
-    'watchers'        => \&req_CATCHALL,
-    'editors'         => \&req_CATCHALL,
+    'watchers'        => \&req_EMPTY,
+    'editors'         => \&req_EMPTY,
     'annotate'        => \&req_annotate,
     'Global_option'   => \&req_Globaloption,
     #'annotate'        => \&req_CATCHALL,
@@ -199,6 +199,11 @@ sub req_CATCHALL
     $log->warn("Unhandled command : req_$cmd : $data");
 }
 
+# This method invariably succeeds with an empty response.
+sub req_EMPTY
+{
+    print "ok\n";
+}
 
 # Root pathname \n
 #     Response expected: no. Tell the server which CVSROOT to use. Note that
-- 
1.5.5.rc1.19.gfe7681

[PATCH v2 2/7] cvsserver: Only print the file part of the filename in status header

[Damien Diederen]

The "File:" header of CVS status output only includes the basename of
the file, even when generating a recursive listing; do the same.

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 git-cvsserver.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 2fe0a8a..444ec0d 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1471,8 +1471,10 @@ sub req_status
 
         $status ||= "Unknown";
 
+        my ($filepart) = filenamesplit($filename);
+
         print "M ===================================================================\n";
-        print "M File: $filename\tStatus: $status\n";
+        print "M File: $filepart\tStatus: $status\n";
         if ( defined($state->{entries}{$filename}{revision}) )
         {
             print "M Working revision:\t" . $state->{entries}{$filename}{revision} . "\n";
-- 
1.5.5.rc1.19.gfe7681

[PATCH v2 3/7] cvsserver: Do not include status output for subdirectories if -l is passed

[Damien Diederen]

This effectively implements the -l switch by pruning the entries whose
filenames contain a path separator.  It was previously ignored.

Without this, TkCVS includes strange "ghost" entries in its directory
listings.

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 git-cvsserver.perl |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 444ec0d..89a4dac 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1428,6 +1428,8 @@ sub req_status
     {
         $filename = filecleanup($filename);
 
+        next if exists($state->{opt}{l}) && index($filename, '/', length($state->{prependdir})) >= 0;
+
         my $meta = $updater->getmeta($filename);
         my $oldmeta = $meta;
 
-- 
1.5.5.rc1.19.gfe7681

[PATCH v2 4/7] cvsserver: Add a few tests for 'status' command

[Damien Diederen]

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 t/t9400-git-cvsserver-server.sh |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index b91b151..6168324 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -420,4 +420,36 @@ test_expect_success 'cvs update (merge no-op)' \
     GIT_CONFIG="$git_config" cvs -Q update &&
     diff -q merge ../merge'
 
+#------------
+# CVS STATUS
+#------------
+
+cd "$WORKDIR"
+test_expect_success 'cvs status' '
+    mkdir status.dir &&
+    echo Line > status.dir/status.file &&
+    echo Line > status.file &&
+    git add status.dir status.file &&
+    git commit -q -m "Status test" &&
+    git push gitcvs.git >/dev/null &&
+    cd cvswork &&
+    GIT_CONFIG="$git_config" cvs update &&
+    GIT_CONFIG="$git_config" cvs status | grep "^File: status.file" >../out &&
+    test $(wc -l <../out) = 2
+'
+
+cd "$WORKDIR"
+test_expect_success 'cvs status (nonrecursive)' '
+    cd cvswork &&
+    GIT_CONFIG="$git_config" cvs status -l | grep "^File: status.file" >../out &&
+    test $(wc -l <../out) = 1
+'
+
+cd "$WORKDIR"
+test_expect_success 'cvs status (no subdirs in header)' '
+    cd cvswork &&
+    GIT_CONFIG="$git_config" cvs status | grep ^File: >../out &&
+    ! grep / <../out
+'
+
 test_done
-- 
1.5.5.rc1.19.gfe7681

[PATCH v2 5/7] cvsserver: Implement update -p (print to stdout)

[Damien Diederen]

Cvs update -p -r <rev> <path> is the documented way to retrieve a
specific revision of a file (similar to git show <rev>:<path>).
Without this patch, the -p flag is ignored and status output is
produced, causing clients to interpret it as the contents of the file.

TkCVS uses update -p as a basis for implementing its various "View"
and "Diff" commands.

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 git-cvsserver.perl |   47 +++++++++++++++++++++++++++++++++++------------
 1 files changed, 35 insertions(+), 12 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 89a4dac..49c0ba2 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -963,6 +963,17 @@ sub req_update
             $meta = $updater->getmeta($filename);
         }
 
+        # If -p was given, "print" the contents of the requested revision.
+        if ( exists ( $state->{opt}{p} ) ) {
+            if ( defined ( $meta->{revision} ) ) {
+                $log->info("Printing '$filename' revision " . $meta->{revision});
+
+                transmitfile($meta->{filehash}, { print => 1 });
+            }
+
+            next;
+        }
+
 	if ( ! defined $meta )
 	{
 	    $meta = {
@@ -1096,9 +1107,9 @@ sub req_update
             my $file_local = $filepart . ".mine";
             system("ln","-s",$state->{entries}{$filename}{modified_filename}, $file_local);
             my $file_old = $filepart . "." . $oldmeta->{revision};
-            transmitfile($oldmeta->{filehash}, $file_old);
+            transmitfile($oldmeta->{filehash}, { targetfile => $file_old });
             my $file_new = $filepart . "." . $meta->{revision};
-            transmitfile($meta->{filehash}, $file_new);
+            transmitfile($meta->{filehash}, { targetfile => $file_new });
 
             # we need to merge with the local changes ( M=successful merge, C=conflict merge )
             $log->info("Merging $file_local, $file_old, $file_new");
@@ -1550,14 +1561,14 @@ sub req_diff
                 print "E File $filename at revision 1.$revision1 doesn't exist\n";
                 next;
             }
-            transmitfile($meta1->{filehash}, $file1);
+            transmitfile($meta1->{filehash}, { targetfile => $file1 });
         }
         # otherwise we just use the working copy revision
         else
         {
             ( undef, $file1 ) = tempfile( DIR => $TEMP_DIR, OPEN => 0 );
             $meta1 = $updater->getmeta($filename, $wrev);
-            transmitfile($meta1->{filehash}, $file1);
+            transmitfile($meta1->{filehash}, { targetfile => $file1 });
         }
 
         # if we have a second -r switch, use it too
@@ -1572,7 +1583,7 @@ sub req_diff
                 next;
             }
 
-            transmitfile($meta2->{filehash}, $file2);
+            transmitfile($meta2->{filehash}, { targetfile => $file2 });
         }
         # otherwise we just use the working copy
         else
@@ -1585,7 +1596,7 @@ sub req_diff
         {
             ( undef, $file2 ) = tempfile( DIR => $TEMP_DIR, OPEN => 0 );
             $meta2 = $updater->getmeta($filename, $wrev);
-            transmitfile($meta2->{filehash}, $file2);
+            transmitfile($meta2->{filehash}, { targetfile => $file2 });
         }
 
         # We need to have retrieved something useful
@@ -2014,14 +2025,17 @@ sub revparse
     return undef;
 }
 
-# This method takes a file hash and does a CVS "file transfer" which transmits the
-# size of the file, and then the file contents.
-# If a second argument $targetfile is given, the file is instead written out to
-# a file by the name of $targetfile
+# This method takes a file hash and does a CVS "file transfer".  Its
+# exact behaviour depends on a second, optional hash table argument:
+# - If $options->{targetfile}, dump the contents to that file;
+# - If $options->{print}, use M/MT to transmit the contents one line
+#   at a time;
+# - Otherwise, transmit the size of the file, followed by the file
+#   contents.
 sub transmitfile
 {
     my $filehash = shift;
-    my $targetfile = shift;
+    my $options = shift;
 
     if ( defined ( $filehash ) and $filehash eq "deleted" )
     {
@@ -2043,11 +2057,20 @@ sub transmitfile
 
     if ( open my $fh, '-|', "git-cat-file", "blob", $filehash )
     {
-        if ( defined ( $targetfile ) )
+        if ( defined ( $options->{targetfile} ) )
         {
+            my $targetfile = $options->{targetfile};
             open NEWFILE, ">", $targetfile or die("Couldn't open '$targetfile' for writing : $!");
             print NEWFILE $_ while ( <$fh> );
             close NEWFILE or die("Failed to write '$targetfile': $!");
+        } elsif ( defined ( $options->{print} ) && $options->{print} ) {
+            while ( <$fh> ) {
+                if( /\n\z/ ) {
+                    print 'M ', $_;
+                } else {
+                    print 'MT text ', $_, "\n";
+                }
+            }
         } else {
             print "$size\n";
             print while ( <$fh> );
-- 
1.5.5.rc1.19.gfe7681

[PATCH v2 6/7] cvsserver: Add test for update -p

[Damien Diederen]

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 t/t9400-git-cvsserver-server.sh |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 6168324..166b43f 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -420,6 +420,24 @@ test_expect_success 'cvs update (merge no-op)' \
     GIT_CONFIG="$git_config" cvs -Q update &&
     diff -q merge ../merge'
 
+cd "$WORKDIR"
+test_expect_success 'cvs update (-p)' '
+    touch really-empty &&
+    echo Line 1 > no-lf &&
+    echo -n Line 2 >> no-lf &&
+    git add really-empty no-lf &&
+    git commit -q -m "Update -p test" &&
+    git push gitcvs.git >/dev/null &&
+    cd cvswork &&
+    GIT_CONFIG="$git_config" cvs update &&
+    rm -f failures &&
+    for i in merge no-lf empty really-empty; do
+        GIT_CONFIG="$git_config" cvs update -p "$i" >$i.out
+        diff $i.out ../$i >>failures 2>&1
+    done &&
+    test -z "$(cat failures)"
+'
+
 #------------
 # CVS STATUS
 #------------
-- 
1.5.5.rc1.19.gfe7681

[PATCH v2 7/7] cvsserver: Use the user part of the email in log and annotate results

[Damien Diederen]

Generate the CVS author names by taking the first eight characters of
the user part of the email address.  The resulting names are more
likely to make sense (or at least reduce ambiguities) in "corporate"
environments.

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 git-cvsserver.perl |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 49c0ba2..dcca4e7 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -1728,8 +1728,7 @@ sub req_log
             print "M revision 1.$revision->{revision}\n";
             # reformat the date for log output
             $revision->{modified} = sprintf('%04d/%02d/%02d %s', $3, $DATE_LIST->{$2}, $1, $4 ) if ( $revision->{modified} =~ /(\d+)\s+(\w+)\s+(\d+)\s+(\S+)/ and defined($DATE_LIST->{$2}) );
-            $revision->{author} =~ s/\s+.*//;
-            $revision->{author} =~ s/^(.{8}).*/$1/;
+            $revision->{author} = cvs_author($revision->{author});
             print "M date: $revision->{modified};  author: $revision->{author};  state: " . ( $revision->{filehash} eq "deleted" ? "dead" : "Exp" ) . ";  lines: +2 -3\n";
             my $commitmessage = $updater->commitmessage($revision->{commithash});
             $commitmessage =~ s/^/M /mg;
@@ -1844,8 +1843,7 @@ sub req_annotate
                 unless ( defined ( $metadata->{$commithash} ) )
                 {
                     $metadata->{$commithash} = $updater->getmeta($filename, $commithash);
-                    $metadata->{$commithash}{author} =~ s/\s+.*//;
-                    $metadata->{$commithash}{author} =~ s/^(.{8}).*/$1/;
+                    $metadata->{$commithash}{author} = cvs_author($metadata->{$commithash}{author});
                     $metadata->{$commithash}{modified} = sprintf("%02d-%s-%02d", $1, $2, $3) if ( $metadata->{$commithash}{modified} =~ /^(\d+)\s(\w+)\s\d\d(\d\d)/ );
                 }
                 printf("M 1.%-5d      (%-8s %10s): %s\n",
@@ -2139,6 +2137,16 @@ sub kopts_from_path
     }
 }
 
+# Generate a CVS author name from Git author information, by taking
+# the first eight characters of the user part of the email address.
+sub cvs_author
+{
+    my $author_line = shift;
+    (my $author) = $author_line =~ /<([^>@]{1,8})/;
+
+    $author;
+}
+
 package GITCVS::log;
 
 ####
-- 
1.5.5.rc1.19.gfe7681

Re: [PATCH v2 0/7] Enhance TkCVS interoperability

[Junio C Hamano]

The diff from the v1 patch all makes sense to me.  Thanks.
I'll queue it for post 1.5.5