Re: git-cvsexportcommit fails for huge commits
On Thu, Dec 13, 2007 at 11:45:54PM -0500, Jeff King wrote:
> So it seems that we could probably go with something more like 64K, and
> then only truly pathological cases should trigger the behavior.
So here is a cleaned up patch. It bumps the maximum size to 64kB, adds scalar support (nobody uses it, but it makes sense for the interface to match that of safe_pipe_capture -- I am even tempted to just replace safe_pipe_capture entirely and convert the few other callers), and cleans up the unused safe_pipe_capture_blob.
-- >8 -- cvsexportcommit: fix massive commits
Because we feed the changed filenames to CVS on the command line, it was possible for massive commits to overflow the system exec limits. Instead, we now do an xargs-like split of the arguments.
This means that we lose some of the atomicity of calling CVS in one shot. Since CVS commits are not atomic, but the CVS protocol is, the possible effects of this are not clear; however, since CVS doesn't provide a different interface, this is our only option for large commits (short of writing a CVS client library).
The argument size limit is arbitrarily set to 64kB. This should be high enough to trigger only in rare cases where it is necessary, so normal-sized commits are not affected by the atomicity change. --- I think the atomicity might matter if you are using cvsexportcommit to talk to a CVS server backed by something besides CVS, like git-cvsserver. Which of course would be useless, but who is to say that other such systems don't exist with CVS as an SCM lingua franca?
git-cvsexportcommit.perl | 37 +++++++++++++++++++++++--------------
1 files changed, 23 insertions(+), 14 deletions(-)
diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index 92e4162..d2e50c3 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -195,11 +195,11 @@ foreach my $f (@files) {
my %cvsstat;
if (@canstatusfiles) {
if ($opt_u) {
- my @updated = safe_pipe_capture(@cvs, 'update', @canstatusfiles);
+ my @updated = xargs_safe_pipe_capture([@cvs, 'update'], @canstatusfiles);
print @updated;
}
my @cvsoutput;
- @cvsoutput= safe_pipe_capture(@cvs, 'status', @canstatusfiles);
+ @cvsoutput = xargs_safe_pipe_capture([@cvs, 'status'], @canstatusfiles);
my $matchcount = 0;
foreach my $l (@cvsoutput) {
chomp $l;
@@ -295,7 +295,7 @@ if ($dirtypatch) {
if ($opt_c) {
print "Autocommit\n $cmd\n";
- print safe_pipe_capture(@cvs, 'commit', '-F', '.msg', @files);
+ print xargs_safe_pipe_capture([@cvs, 'commit', '-F', '.msg'], @files);
if ($?) {
die "Exiting: The commit did not succeed";
}
@@ -335,15 +335,24 @@ sub safe_pipe_capture {
return wantarray ? @output : join('',@output);
}
-sub safe_pipe_capture_blob {
- my $output;
- if (my $pid = open my $child, '-|') {
- local $/;
- undef $/;
- $output = (<$child>);
- close $child or die join(' ',@_).": $! $?";
- } else {
- exec(@_) or die "$! $?"; # exec() can fail the executable can't be found
- }
- return $output;
+sub xargs_safe_pipe_capture {
+ my $MAX_ARG_LENGTH = 65536;
+ my $cmd = shift;
+ my @output;
+ my $output;
+ while(@_) {
+ my @args;
+ my $length = 0;
+ while(@_ && $length < $MAX_ARG_LENGTH) {
+ push @args, shift;
+ $length += length($args[$#args]);
+ }
+ if (wantarray) {
+ push @output, safe_pipe_capture(@$cmd, @args);
+ }
+ else {
+ $output .= safe_pipe_capture(@$cmd, @args);
+ }
+ }
+ return wantarray ? @output : $output;
}
--
1.5.4.rc0.1088.g8a30-dirty
