[PATCH] git-cvsserver: detect/diagnose write failure, etc.

[Jim Meyering <jim@meyering.net>]

Currently I maintain a CVS repository that mirrors
the primary git repository for the GNU coreutils.
It is kept in sync via a relatively fragile "update" (push) hook
that runs git-cvsexportcommit every time I push to the git repository.

Since it'd be nice to keep cvs support (so as not to *require*
everyone to switch to git right away), without the actual CVS
repository, I'm considering using git-cvsserver.

However, before I switch, I'd like to have a little more
confidence that it is reliable.  I've included a patch below
that makes it more likely to report if/when something goes wrong,
usually just write errors.

Beware: in some contexts (when running as server), one must
not "die", but rather return an "error" indicator to the client.
I did that in the second hunk.  However, I haven't carefully
audited the others, and so, some of the "die" calls I added may
end up killing the server, when a gentler failure is required.

I've just looked, and can confirm that my change to req_Modified
(first hunk) is wrong.  It should not die.  Rather, it should probably
do something like the "print "E ... in hunk#2.  Ideally, someone
would write a test to exercise code like this, to make sure it works.

Jim

diff --git a/git-cvsserver.perl b/git-cvsserver.perl
index 5cbf27e..8d8d6f5 100755
--- a/git-cvsserver.perl
+++ b/git-cvsserver.perl
@@ -644,7 +644,7 @@ sub req_Modified
         $bytesleft -= $blocksize;
     }
 
-    close $fh;
+    close $fh or die "Failed to write temporary, $filename: $!";
 
     # Ensure we have something sensible for the file mode
     if ( $mode =~ /u=(\w+)/ )
@@ -901,8 +901,13 @@ sub req_update
     # projects (heads in this case) to checkout.
     #
     if ($state->{module} eq '') {
+	my $heads_dir = $state->{CVSROOT} . '/refs/heads';
+	if (!opendir HEADS, $heads_dir) {
+	  print "E [server aborted]: Failed to open directory, $heads_dir: $!\n"
+	    . "error\n";
+	  return 0;
+	}
         print "E cvs update: Updating .\n";
-	opendir HEADS, $state->{CVSROOT} . '/refs/heads';
 	while (my $head = readdir(HEADS)) {
 	    if (-f $state->{CVSROOT} . '/refs/heads/' . $head) {
 	        print "E cvs update: New directory `$head'\n";
@@ -1754,7 +1759,9 @@ sub req_annotate
         # git-jsannotate telling us about commits we are hiding
         # from the client.
 
-        open(ANNOTATEHINTS, ">$tmpdir/.annotate_hints") or die "Error opening > $tmpdir/.annotate_hints $!";
+        my $a_hints = "$tmpdir/.annotate_hints";
+        open(ANNOTATEHINTS, '>', $a_hints)
+	  or die "Failed to open '$a_hints' for writing: $!";
         for (my $i=0; $i < @$revisions; $i++)
         {
             print ANNOTATEHINTS $revisions->[$i][2];
@@ -1765,11 +1772,11 @@ sub req_annotate
         }
 
         print ANNOTATEHINTS "\n";
-        close ANNOTATEHINTS;
+        close ANNOTATEHINTS or die "Failed to write $a_hints: $!";
 
-        my $annotatecmd = 'git-annotate';
-        open(ANNOTATE, "-|", $annotatecmd, '-l', '-S', "$tmpdir/.annotate_hints", $filename)
-	    or die "Error invoking $annotatecmd -l -S $tmpdir/.annotate_hints $filename : $!";
+        my @cmd = (qw(git-annotate -l -S), $a_hints, $filename);
+        open(ANNOTATE, "-|", @cmd)
+	    or die "Error invoking ". join(' ',@cmd) .": $!";
         my $metadata = {};
         print "E Annotations for $filename\n";
         print "E ***************\n";
@@ -1996,12 +2003,12 @@ sub transmitfile
         {
             open NEWFILE, ">", $targetfile or die("Couldn't open '$targetfile' for writing : $!");
             print NEWFILE $_ while ( <$fh> );
-            close NEWFILE;
+            close NEWFILE or die("Failed to write '$targetfile': $!");
         } else {
             print "$size\n";
             print while ( <$fh> );
         }
-        close $fh or die ("Couldn't close filehandle for transmitfile()");
+        close $fh or die ("Couldn't close filehandle for transmitfile(): $!");
     } else {
         die("Couldn't execute git-cat-file");
     }
@@ -2501,17 +2508,14 @@ sub update
                     if ($parent eq $lastpicked) {
                         next;
                     }
-                    open my $p, 'git-merge-base '. $lastpicked . ' '
-                    . $parent . '|';
-                    my @output = (<$p>);
-                    close $p;
-                    my $base = join('', @output);
+                    my $base = safe_pipe_capture('git-merge-base',
+						 $lastpicked, $parent);
                     chomp $base;
                     if ($base) {
                         my @merged;
                         # print "want to log between  $base $parent \n";
                         open(GITLOG, '-|', 'git-log', "$base..$parent")
-                        or die "Cannot call git-log: $!";
+			  or die "Cannot call git-log: $!";
                         my $mergedhash;
                         while (<GITLOG>) {
                             chomp;
-- 
1.5.3.rc0.11.ge2b1a-dirty