Subject: Re: [PATCH] cvsimport: skip commits that are too recent

From: Robin Rosenberg <[email-masked]>
Date: Thu, 11 Jan 2007 09:22:07 +0100

The idea is nice,  but the downside of this patch is that I (and presumably 
others) have to rewrite the scripts to invoke cvsps explicitly now. The fix 
should really be in cvsps, not git-cvsimport (which is the reason I haven't 
fixed this). Running a full cvsps takes two hours and consumes more than a 
gigabyte of memory for me, which makes it impossible to run on all but one 
machine, wheras the incremental import runs in less than five minutes on any 
machine.

Add to that the risk that the buggy nature of cvsps probably increases the 
risk of errors, so please make the old behaviour the default (import all, 
retain cvsps cache) and make the changed behaviour the result of an explicit 
switch.

-- robin

måndag 08 januari 2007 07:43 skrev Martin Langhoff:
> With this patch, cvsimport will skip commits made
> in the last 10 minutes. The recent-ness test is of
> 5 minutes + cvsps fuzz window (5 minutes default).
>
> To force recent commits to be imported, pass the
> -a(ll) flag.
>
> When working with a CVS repository that is in use,
> importing commits that are too recent can lead to
> partially incorrect trees. This is mainly due to
>
>  - Commits that are within the cvsps fuzz window may later
>    be found to have affected more files.
>
>  - When performing incremental imports, clock drift between
>    the systems may lead to skipped commits.
>
> This commit helps keep incremental imports of in-use
> CVS repositories sane.
>
> Signed-off-by: Martin Langhoff <[email-masked]>
> ---
>  Documentation/git-cvsimport.txt |    7 ++++++-
>  git-cvsimport.perl              |   20 ++++++++++++++++++--
>  2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-cvsimport.txt
> b/Documentation/git-cvsimport.txt index d21d66b..6deee94 100644
> --- a/Documentation/git-cvsimport.txt
> +++ b/Documentation/git-cvsimport.txt
> @@ -90,7 +90,8 @@ If you need to pass multiple options, separate them with
> a comma. Print a short usage message and exit.
>
>  -z <fuzz>::
> -        Pass the timestamp fuzz factor to cvsps.
> +	Pass the timestamp fuzz factor to cvsps, in seconds. If unset,
> +	cvsps defaults to 300s.
>
>  -s <subst>::
>  	Substitute the character "/" in branch names with <subst>
> @@ -99,6 +100,10 @@ If you need to pass multiple options, separate them
> with a comma. CVS by default uses the unix username when writing its
>  	commit logs. Using this option and an author-conv-file
>  	in this format
> +
> +-a::
> +	Import all commits, including recent ones. cvsimport by default
> +	skips commits that have a timestamp less than 10 minutes ago.
>  +
>  ---------
>  	exon=Andreas Ericsson <[email-masked]>
> diff --git a/git-cvsimport.perl b/git-cvsimport.perl
> index c5bf2d1..a75aaa3 100755
> --- a/git-cvsimport.perl
> +++ b/git-cvsimport.perl
> @@ -29,7 +29,7 @@ use IPC::Open2;
>  $SIG{'PIPE'}="IGNORE";
>  $ENV{'TZ'}="UTC";
>
> -our
> ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt
>_P, $opt_s,$opt_m,$opt_M,$opt_A,$opt_S,$opt_L); +our
> ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt
>_P, $opt_s,$opt_m,$opt_M,$opt_A,$opt_S,$opt_L, $opt_a); my
> (%conv_author_name, %conv_author_email);
>
>  sub usage() {
> @@ -37,7 +37,7 @@ sub usage() {
>  Usage: ${\basename $0}     # fetch/update GIT from CVS
>         [-o branch-for-HEAD] [-h] [-v] [-d CVSROOT] [-A author-conv-file]
>         [-p opts-for-cvsps] [-C GIT_repository] [-z fuzz] [-i] [-k] [-u]
> -       [-s subst] [-m] [-M regex] [-S regex] [CVS_module]
> +       [-s subst] [-a] [-m] [-M regex] [-S regex] [CVS_module]
>  END
>  	exit(1);
>  }
> @@ -105,6 +105,8 @@ if ($opt_d) {
>  }
>  $opt_o ||= "origin";
>  $opt_s ||= "-";
> +$opt_a ||= 0;
> +
>  my $git_tree = $opt_C;
>  $git_tree ||= ".";
>
> @@ -129,6 +131,11 @@ if ($opt_M) {
>  	push (@mergerx, qr/$opt_M/);
>  }
>
> +# Remember UTC of our starting time
> +# we'll want to avoid importing commits
> +# that are too recent
> +our $starttime = time();
> +
>  select(STDERR); $|=1; select(STDOUT);
>
>
> @@ -824,6 +831,15 @@ while (<CVS>) {
>  			$state = 11;
>  			next;
>  		}
> +		if ( !$opt_a && $starttime - 300 - (defined $opt_z ? $opt_z : 300) <=
> $date) { +			# skip if the commit is too recent
> +			# that the cvsps default fuzz is 300s, we give ourselves another
> +			# 300s just in case -- this also prevents skipping commits
> +			# due to server clock drift
> +			print "skip patchset $patchset: $date too recent\n" if $opt_v;
> +			$state = 11;
> +			next;
> +		}
>  		if (exists $ignorebranch{$branch}) {
>  			print STDERR "Skipping $branch\n";
>  			$state = 11;

-- 
TESTMAIL