From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jim Meyering Subject: [PATCH] git-cvsserver: detect/diagnose write failure, etc. Date: Fri, 06 Jul 2007 14:18:42 +0200 Message-ID: <87vecx4tel.fsf@rho.meyering.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii To: git@vger.kernel.org X-From: git-owner@vger.kernel.org Fri Jul 06 14:18:52 2007 connect(): Connection refused Return-path: Envelope-to: gcvg-git@gmane.org Received: from vger.kernel.org ([209.132.176.167]) by lo.gmane.org with esmtp (Exim 4.50) id 1I6mlr-0005BB-C5 for gcvg-git@gmane.org; Fri, 06 Jul 2007 14:18:47 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759710AbXGFMSp (ORCPT ); Fri, 6 Jul 2007 08:18:45 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1759925AbXGFMSp (ORCPT ); Fri, 6 Jul 2007 08:18:45 -0400 Received: from server1.f7.net ([64.34.169.74]:52507 "EHLO f7.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759710AbXGFMSo (ORCPT ); Fri, 6 Jul 2007 08:18:44 -0400 X-Envelope-From: jim@meyering.net Received: from mx.meyering.net (server1.f7.net [64.34.169.74]) by f7.net (8.11.7-20030920/8.11.7) with ESMTP id l66CIhF31312 for ; Fri, 6 Jul 2007 07:18:43 -0500 Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000) id 8E7702F030; Fri, 6 Jul 2007 14:18:42 +0200 (CEST) Sender: git-owner@vger.kernel.org Precedence: bulk X-Mailing-List: git@vger.kernel.org Archived-At: 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 () { chomp; -- 1.5.3.rc0.11.ge2b1a-dirty