threads / patch / 12850

patch, 7 partsEnhance TkCVS interoperability

Subject: [PATCH 0/7] Enhance TkCVS interoperability

## tl;dr

22 messages between Mar 24, 2008 and Mar 28, 2008. Diffs are folded; open one to read it.

replies: 21people: 4as markdown or json

Damien Diederen· Mar 24, 2008, 22:48 UTC · lore

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(-)
Damien Diederen· Mar 24, 2008, 22:48 UTC · re: Damien Diederen · lore

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

"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(-)
Show changes to git-cvsserver.perl +6 −1
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
Frank Lichtenheld· Mar 25, 2008, 09:03 UTC · re: Damien Diederen · lore

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

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/
Damien Diederen· Mar 24, 2008, 22:49 UTC · re: Damien Diederen · lore

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

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(-)
Show changes to git-cvsserver.perl +3 −1
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
Damien Diederen· Mar 24, 2008, 22:49 UTC · re: Damien Diederen · lore

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

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(-)
Show changes to git-cvsserver.perl +2 −0
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
Damien Diederen· Mar 24, 2008, 22:50 UTC · re: Damien Diederen · lore

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

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 t/t9400-git-cvsserver-server.sh |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)
Show changes to t/t9400-git-cvsserver-server.sh +32 −0
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
Damien Diederen· Mar 24, 2008, 22:50 UTC · re: Damien Diederen · lore

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

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(-)
Show changes to git-cvsserver.perl +28 −8
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
Damien Diederen· Mar 24, 2008, 22:50 UTC · re: Damien Diederen · lore

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

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 t/t9400-git-cvsserver-server.sh |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
Show changes to t/t9400-git-cvsserver-server.sh +18 −0
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
Damien Diederen· Mar 24, 2008, 22:50 UTC · re: Damien Diederen · lore

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

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(-)
Show changes to git-cvsserver.perl +14 −4
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
Frank Lichtenheld· Mar 25, 2008, 09:26 UTC · re: Damien Diederen · lore

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

On Mon, Mar 24, 2008 at 11:50:55PM +0100, Damien Diederen wrote:
Show 8 quoted lines
> +# 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/
Rafael Garcia-Suarez· Mar 25, 2008, 09:39 UTC · re: Frank Lichtenheld · lore

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

On 25/03/2008, Frank Lichtenheld <frank@lichtenheld.de> wrote:
Show 13 quoted lines
> 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})/;

Damien Diederen· Mar 25, 2008, 13:58 UTC · re: Rafael Garcia-Suarez · lore

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

Hi All,
"Rafael Garcia-Suarez" <rgarciasuarez@gmail.com> writes:
Show 19 quoted lines
> 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/
Junio C Hamano· Mar 25, 2008, 01:08 UTC · re: Damien Diederen · lore

Re: [PATCH 0/7] Enhance TkCVS interoperability

Damien Diederen <dash@foobox.net> writes:
Show 7 quoted lines
> 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?
Damien Diederen· Mar 27, 2008, 22:17 UTC · re: Damien Diederen · lore

[PATCH v2 0/7] Enhance TkCVS interoperability

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(-)
Damien Diederen· Mar 27, 2008, 22:17 UTC · re: Damien Diederen · lore

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

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(-)
Show changes to git-cvsserver.perl +7 −2
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
Damien Diederen· Mar 27, 2008, 22:17 UTC · re: Damien Diederen · lore

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

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(-)
Show changes to git-cvsserver.perl +3 −1
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
Damien Diederen· Mar 27, 2008, 22:17 UTC · re: Damien Diederen · lore

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

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(-)
Show changes to git-cvsserver.perl +2 −0
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
Damien Diederen· Mar 27, 2008, 22:18 UTC · re: Damien Diederen · lore

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

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 t/t9400-git-cvsserver-server.sh |   32 ++++++++++++++++++++++++++++++++
 1 files changed, 32 insertions(+), 0 deletions(-)
Show changes to t/t9400-git-cvsserver-server.sh +32 −0
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
Damien Diederen· Mar 27, 2008, 22:18 UTC · re: Damien Diederen · lore

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

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(-)
Show changes to git-cvsserver.perl +35 −12
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
Damien Diederen· Mar 27, 2008, 22:18 UTC · re: Damien Diederen · lore

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

Signed-off-by: Damien Diederen <dash@foobox.net>
---
 t/t9400-git-cvsserver-server.sh |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
Show changes to t/t9400-git-cvsserver-server.sh +18 −0
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
Damien Diederen· Mar 27, 2008, 22:18 UTC · re: Damien Diederen · lore

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

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(-)
Show changes to git-cvsserver.perl +12 −4
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

← back to recent threads