threads / patch / 4351

patchcvsimport: complete the cvsps run before starting the import

Subject: [PATCH] cvsimport: complete the cvsps run before starting the import

## tl;dr

One message between May 30, 2006 and May 30, 2006. Diffs are folded; open one to read it.

replies: 0people: 1as markdown or json

Martin Langhoff· May 30, 2006, 08:08 UTC · lore
On 5/24/06, Linus Torvalds <torvalds@osdl.org> wrote:
> It's entirely possible that the fact that it now seems to work for me is
> purely timing-related, since I also ended up using "-P cvsps-output" to
> avoid having a huge cvsps binary in memory at the same time.

We now capture the output of cvsps to a tempfile, and then read it in. cvsps 2.1 works quite a bit "in memory", and only prints its patchset info once it has finished talking with cvs, but apparently retaining all that memory allocation. With this patch, cvsps is finished and reaped before cvsimport start working (and growing). So the footprint of the whole process is much lower.

Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>
---

I don't particularly like the idea of switching from a safe system() call to this ugly one. But this patch makes a huge difference importing gentoo's repo, and I could not find a way to get system() to do redirection.

Of course, we could do the redirection in Perl. Ugly vs uglier?
---
 git-cvsimport.perl |   32 ++++++++++++++++++++++----------
 1 files changed, 22 insertions(+), 10 deletions(-)
5ce458e0883f39ae774ec67211e6565b65139b7f
Show changes to git-cvsimport.perl +22 −10
diff --git a/git-cvsimport.perl b/git-cvsimport.perl
index 60fc86a..2239c67 100755
--- a/git-cvsimport.perl
+++ b/git-cvsimport.perl
@@ -529,24 +529,36 @@ if ($opt_A) {
 	write_author_info("$git_dir/cvs-authors");
 }
 
-my $pid = open(CVS,"-|");
-die "Cannot fork: $!\n" unless defined $pid;
-unless($pid) {
+#
+# run cvsps into a file unless it's provided already
+#
+my $cvspsfile;
+if ($opt_P) {
+       $cvspsfile = $opt_P;
+} else {
+	my $cvspsfh;
+	($cvspsfh, $cvspsfile) = tempfile('gitXXXXXX', SUFFIX => '.cvsps',
+					  DIR => File::Spec->tmpdir());
+	close ($cvspsfh);
+	my ($cvspserrfh, $cvspserr)  = tempfile('gitXXXXXX', SUFFIX => '.err',
+						DIR => File::Spec->tmpdir());
+	close ($cvspserrfh);
+
 	my @opt;
 	@opt = split(/,/,$opt_p) if defined $opt_p;
 	unshift @opt, '-z', $opt_z if defined $opt_z;
-	unshift @opt, '-q'         unless defined $opt_v;
+	unshift @opt, '-q'	   unless defined $opt_v;
 	unless (defined($opt_p) && $opt_p =~ m/--no-cvs-direct/) {
 		push @opt, '--cvs-direct';
 	}
-	if ($opt_P) {
-	    exec("cat", $opt_P);
-	} else {
-	    exec("cvsps","--norc",@opt,"-u","-A",'--root',$opt_d,$cvs_tree);
-	    die "Could not start cvsps: $!\n";
-	}
+
+	print "Running cvsps\n"		  if $opt_v;
+	system(join(' ', "cvsps","--norc",@opt,"-u","-A",'--root',$opt_d,$cvs_tree, "1>$cvspsfile" ))
+		or die "Error in cvsps: $!\n";
 }
 
+open (CVS, "<$cvspsfile") 
+        or die "Cannot open cvsps output file $cvspsfile: $!\n";
 
 ## cvsps output:
 #---------------------
-- 
1.3.2.g82000

← back to recent threads