Enhance TkCVS interoperability

PATCH Superseded Damien Diederen 4 participants 22 messages Jun 15, 2016

Share & tools

Damien Diederen

author

2016-06-15 22:44 UTC #0 lore raw

cover 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.

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

author

2016-06-15 22:44 UTC #1 lore raw

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 <[email protected]>

---
 git-cvsserver.perl |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

git-cvsserver.perl +6 -1

index 7f632af..33d30c5 100755
@@ -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 ) = @_;

Signature · 1 line

1.5.5.rc1.6.gd183

Frank Lichtenheld

2016-06-15 22:44 UTC #2 lore raw

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

Quoted 3 lines · 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.

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

Gruesse,

Signature · 2 lines

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

Damien Diederen

author

2016-06-15 22:44 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---
 git-cvsserver.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

git-cvsserver.perl +3 -1

index 33d30c5..9101eef 100755
@@ -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";

Signature · 1 line

1.5.5.rc1.6.gd183

Damien Diederen

author

2016-06-15 22:44 UTC #4 lore raw

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

In reply to 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 <[email protected]>

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

git-cvsserver.perl +2 -0

index 9101eef..073a426 100755
@@ -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;

Signature · 1 line

1.5.5.rc1.6.gd183

Junio C Hamano

2016-06-15 22:44 UTC #5 lore raw

In reply to Damien Diederen

Quoted 7 lines · 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.

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

author

2016-06-15 22:44 UTC #6 lore raw

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

In reply to Damien Diederen

Signed-off-by Damien Diederen <[email protected]>

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

t/t9400-git-cvsserver-server.sh +32 -0

index b91b151..6168324 100755
@@ -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

Signature · 1 line

1.5.5.rc1.6.gd183

Damien Diederen

author

2016-06-15 22:44 UTC #7 lore raw

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

In reply to 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 <[email protected]>

---
 git-cvsserver.perl |   36 ++++++++++++++++++++++++++++--------
 1 files changed, 28 insertions(+), 8 deletions(-)

git-cvsserver.perl +28 -8

index 073a426..3c97226 100755
@@ -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> );

Signature · 1 line

1.5.5.rc1.6.gd183

Damien Diederen

author

2016-06-15 22:44 UTC #8 lore raw

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

In reply to Damien Diederen

Signed-off-by Damien Diederen <[email protected]>

---
 t/t9400-git-cvsserver-server.sh |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

t/t9400-git-cvsserver-server.sh +18 -0

index 6168324..166b43f 100755
@@ -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
 #------------

Signature · 1 line

1.5.5.rc1.6.gd183

Damien Diederen

author

2016-06-15 22:44 UTC #9 lore raw

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

In reply to 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 <[email protected]>

---
 git-cvsserver.perl |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

git-cvsserver.perl +14 -4

index 3c97226..9d845c8 100755
@@ -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;
 
 ####

Signature · 1 line

1.5.5.rc1.6.gd183

Frank Lichtenheld

2016-06-15 22:44 UTC #10 lore raw

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

git-cvsserver.perl sub kopts_from_path · line 2144

Quoted 8 lines · Damien Diederen

git-cvsserver.perl +8 -0

+# 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,

Signature · 2 lines

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

Rafael Garcia-Suarez

2016-06-15 22:44 UTC #11 lore raw

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

Quoted 13 lines · Frank Lichtenheld

git-cvsserver.perl sub kopts_from_path · line 2144
Quoted 8 lines · Damien Diederen
git-cvsserver.perl +8 -0
+# 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

author

2016-06-15 22:44 UTC #12 lore raw

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

Hi All,

Quoted 11 lines · Rafael Garcia-Suarez

Quoted 10 lines · Frank Lichtenheld

Quoted 8 lines · Damien Diederen

git-cvsserver.perl +8 -0

+# 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/;

Quoted 8 lines

Quoted 2 lines
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

Signature · 1 line

http://foobox.net/~dash/

Damien Diederen

author

2016-06-15 22:44 UTC #13 lore raw

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

In reply to 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 <[email protected]>

---
 git-cvsserver.perl |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

git-cvsserver.perl +7 -2

index 7f632af..2fe0a8a 100755
@@ -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

Signature · 1 line

1.5.5.rc1.19.gfe7681

Damien Diederen

author

2016-06-15 22:44 UTC #14 lore raw

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

In reply to 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 <[email protected]>

---
 git-cvsserver.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

git-cvsserver.perl +3 -1

index 2fe0a8a..444ec0d 100755
@@ -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";

Signature · 1 line

1.5.5.rc1.19.gfe7681

Damien Diederen

author

2016-06-15 22:44 UTC #15 lore raw

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

In reply to 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 <[email protected]>

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

git-cvsserver.perl +2 -0

index 444ec0d..89a4dac 100755
@@ -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;

Signature · 1 line

1.5.5.rc1.19.gfe7681

Damien Diederen

author

2016-06-15 22:44 UTC #16 lore raw

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

In reply to Damien Diederen

Signed-off-by Damien Diederen <[email protected]>

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

t/t9400-git-cvsserver-server.sh +32 -0

index b91b151..6168324 100755
@@ -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

Signature · 1 line

1.5.5.rc1.19.gfe7681

Damien Diederen

author

2016-06-15 22:44 UTC #17 lore raw

v2 cover Enhance TkCVS interoperability

In reply to 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(-)

Junio C Hamano

2016-06-15 22:44 UTC #18 lore raw

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

Damien Diederen

author

2016-06-15 22:44 UTC #19 lore raw

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

In reply to Damien Diederen

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

Signed-off-by Damien Diederen <[email protected]>

---
 git-cvsserver.perl |   47 +++++++++++++++++++++++++++++++++++------------
 1 files changed, 35 insertions(+), 12 deletions(-)

git-cvsserver.perl +35 ~~-12~~

index 89a4dac..49c0ba2 100755
@@ -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> );

Signature · 1 line

1.5.5.rc1.19.gfe7681

Damien Diederen

author

2016-06-15 22:44 UTC #20 lore raw

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

In reply to Damien Diederen

Signed-off-by Damien Diederen <[email protected]>

---
 t/t9400-git-cvsserver-server.sh |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

t/t9400-git-cvsserver-server.sh +18 -0

index 6168324..166b43f 100755
@@ -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
 #------------

Signature · 1 line

1.5.5.rc1.19.gfe7681

Damien Diederen

author

2016-06-15 22:44 UTC #21 lore raw

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

In reply to Damien Diederen

Signed-off-by Damien Diederen <[email protected]>

---
 git-cvsserver.perl |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

git-cvsserver.perl +12 -4

index 49c0ba2..dcca4e7 100755
@@ -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;
 
 ####

Signature · 1 line

1.5.5.rc1.19.gfe7681