# [PATCH 0/7] Enhance TkCVS interoperability 22 messages from 2008-03-24 to 2008-03-28. Participants: Damien Diederen, Junio C Hamano, Frank Lichtenheld, Rafael Garcia-Suarez. Thread: https://gitlist.dev/t/12850 ## Damien Diederen, 2008-03-24 22:48 Subject: [PATCH 0/7] Enhance TkCVS interoperability Message-ID: URL: https://gitlist.dev/e/cover.1206393086.git.dash%40foobox.net ``` 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, 2008-03-24 22:48 Subject: [PATCH 1/7] cvsserver: Respond to the 'editors' command Message-ID: <8cb3f0ef415bdfe8a25f89fdac3d14d3b8360dcf.1206393086.git.dash@foobox.net> URL: https://gitlist.dev/e/8cb3f0ef415bdfe8a25f89fdac3d14d3b8360dcf.1206393086.git.dash%40foobox.net In-Reply-To: ``` "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 --- 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 ``` ## Damien Diederen, 2008-03-24 22:49 Subject: [PATCH 2/7] cvsserver: Only print the file part of the filename in status header Message-ID: <42e48b57a0e94ca07e9dea28da2fa1f8d8a074c2.1206393086.git.dash@foobox.net> URL: https://gitlist.dev/e/42e48b57a0e94ca07e9dea28da2fa1f8d8a074c2.1206393086.git.dash%40foobox.net In-Reply-To: ``` 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 --- 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 ``` ## Damien Diederen, 2008-03-24 22:49 Subject: [PATCH 3/7] cvsserver: Do not include status output for subdirectories if -l is passed Message-ID: URL: https://gitlist.dev/e/acabbce71725396ac8641b59197525fdfd8e8ae9.1206393086.git.dash%40foobox.net In-Reply-To: ``` 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 --- 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 ``` ## Damien Diederen, 2008-03-24 22:50 Subject: [PATCH 4/7] cvsserver: Add a few tests for 'status' command Message-ID: <96f67c3e4ad8f5fe0c179b97b9f1ae1a7943b106.1206393086.git.dash@foobox.net> URL: https://gitlist.dev/e/96f67c3e4ad8f5fe0c179b97b9f1ae1a7943b106.1206393086.git.dash%40foobox.net In-Reply-To: ``` Signed-off-by: Damien Diederen --- 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 ``` ## Damien Diederen, 2008-03-24 22:50 Subject: [PATCH 5/7] cvsserver: Implemented update -p (print to stdout) Message-ID: <4ba0ce96c3dcd304ca598379dc448ca50d994133.1206393086.git.dash@foobox.net> URL: https://gitlist.dev/e/4ba0ce96c3dcd304ca598379dc448ca50d994133.1206393086.git.dash%40foobox.net In-Reply-To: ``` Cvs update -p -r is the documented way to retrieve a specific revision of a file (similar to git show :). 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 --- 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 ``` ## Damien Diederen, 2008-03-24 22:50 Subject: [PATCH 6/7] cvsserver: Added test for update -p Message-ID: URL: https://gitlist.dev/e/d183368d7796cc28e600a6984954a836dc78d8be.1206393086.git.dash%40foobox.net In-Reply-To: ``` Signed-off-by: Damien Diederen --- 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 ``` ## Damien Diederen, 2008-03-24 22:50 Subject: [PATCH 7/7] cvsserver: Use the user part of the email in log and annotate results Message-ID: <01e5947cde524f4eb97a86785d0e1f28e2d040e0.1206393086.git.dash@foobox.net> URL: https://gitlist.dev/e/01e5947cde524f4eb97a86785d0e1f28e2d040e0.1206393086.git.dash%40foobox.net In-Reply-To: ``` 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 --- 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 ``` ## Junio C Hamano, 2008-03-25 01:08 Subject: Re: [PATCH 0/7] Enhance TkCVS interoperability Message-ID: <7vd4pjr4o1.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd4pjr4o1.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Damien Diederen 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? ``` ## Frank Lichtenheld, 2008-03-25 09:03 Subject: Re: [PATCH 1/7] cvsserver: Respond to the 'editors' command Message-ID: <20080325090306.GM25732@mail-vs.djpig.de> URL: https://gitlist.dev/e/20080325090306.GM25732%40mail-vs.djpig.de In-Reply-To: <8cb3f0ef415bdfe8a25f89fdac3d14d3b8360dcf.1206393086.git.dash@foobox.net> ``` 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 www: http://www.djpig.de/ ``` ## Frank Lichtenheld, 2008-03-25 09:26 Subject: Re: [PATCH 7/7] cvsserver: Use the user part of the email in log and annotate results Message-ID: <20080325092621.GN25732@mail-vs.djpig.de> URL: https://gitlist.dev/e/20080325092621.GN25732%40mail-vs.djpig.de In-Reply-To: <01e5947cde524f4eb97a86785d0e1f28e2d040e0.1206393086.git.dash@foobox.net> ``` 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 www: http://www.djpig.de/ ``` ## Rafael Garcia-Suarez, 2008-03-25 09:39 Subject: Re: [PATCH 7/7] cvsserver: Use the user part of the email in log and annotate results Message-ID: URL: https://gitlist.dev/e/b77c1dce0803250239o249a9059sbaafccf9ddea5e22%40mail.gmail.com In-Reply-To: <20080325092621.GN25732@mail-vs.djpig.de> ``` On 25/03/2008, Frank Lichtenheld 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})/; ``` ## Damien Diederen, 2008-03-25 13:58 Subject: Re: [PATCH 7/7] cvsserver: Use the user part of the email in log and annotate results Message-ID: <87myomykg1.fsf@keem.bcc> URL: https://gitlist.dev/e/87myomykg1.fsf%40keem.bcc In-Reply-To: ``` Hi All, "Rafael Garcia-Suarez" writes: > On 25/03/2008, Frank Lichtenheld 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/ ``` ## Damien Diederen, 2008-03-27 22:17 Subject: [PATCH v2 0/7] Enhance TkCVS interoperability Message-ID: URL: https://gitlist.dev/e/cover.1206654905.git.dash%40foobox.net In-Reply-To: ``` 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, 2008-03-27 22:17 Subject: [PATCH v2 1/7] cvsserver: Respond to the 'editors' and 'watchers' commands Message-ID: URL: https://gitlist.dev/e/ade0303c6104ad25bc4d846af2417a994cf9cf46.1206654905.git.dash%40foobox.net In-Reply-To: ``` 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 --- 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 ``` ## Damien Diederen, 2008-03-27 22:17 Subject: [PATCH v2 2/7] cvsserver: Only print the file part of the filename in status header Message-ID: <333a7cfa26e71fe42a9b10cd4d7c1a6a5085aacd.1206654905.git.dash@foobox.net> URL: https://gitlist.dev/e/333a7cfa26e71fe42a9b10cd4d7c1a6a5085aacd.1206654905.git.dash%40foobox.net In-Reply-To: ``` 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 --- 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 ``` ## Damien Diederen, 2008-03-27 22:17 Subject: [PATCH v2 3/7] cvsserver: Do not include status output for subdirectories if -l is passed Message-ID: URL: https://gitlist.dev/e/e9090852dc2f6e0f9aae5f7418b18be24374a4d9.1206654905.git.dash%40foobox.net In-Reply-To: ``` 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 --- 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 ``` ## Damien Diederen, 2008-03-27 22:18 Subject: [PATCH v2 4/7] cvsserver: Add a few tests for 'status' command Message-ID: <3bc8d6b298f7111d5ddcc5bfbaf8bab557eaec61.1206654905.git.dash@foobox.net> URL: https://gitlist.dev/e/3bc8d6b298f7111d5ddcc5bfbaf8bab557eaec61.1206654905.git.dash%40foobox.net In-Reply-To: ``` Signed-off-by: Damien Diederen --- 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 ``` ## Damien Diederen, 2008-03-27 22:18 Subject: [PATCH v2 5/7] cvsserver: Implement update -p (print to stdout) Message-ID: URL: https://gitlist.dev/e/e95fde851fdc6ef39f32054cba6cde0ae975b8fd.1206654905.git.dash%40foobox.net In-Reply-To: ``` Cvs update -p -r is the documented way to retrieve a specific revision of a file (similar to git show :). 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 --- 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 ``` ## Damien Diederen, 2008-03-27 22:18 Subject: [PATCH v2 6/7] cvsserver: Add test for update -p Message-ID: URL: https://gitlist.dev/e/c1b10ec91d678b58e0c5e65b3fbc77a4ed19453a.1206654905.git.dash%40foobox.net In-Reply-To: ``` Signed-off-by: Damien Diederen --- 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 ``` ## Damien Diederen, 2008-03-27 22:18 Subject: [PATCH v2 7/7] cvsserver: Use the user part of the email in log and annotate results Message-ID: URL: https://gitlist.dev/e/f7869cd4f5ed963cbc298ee64b77a43ebd778b0f.1206654905.git.dash%40foobox.net In-Reply-To: ``` 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 --- 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 ``` ## Junio C Hamano, 2008-03-28 07:52 Subject: Re: [PATCH v2 0/7] Enhance TkCVS interoperability Message-ID: <7vabkj2slx.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vabkj2slx.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` The diff from the v1 patch all makes sense to me. Thanks. I'll queue it for post 1.5.5 ```