# [PATCH] cvsimport: skip commits that are too recent

5 messages from 2007-01-08 to 2007-01-11. Participants: Martin Langhoff, Robin Rosenberg.
Thread: https://gitlist.dev/t/6281

## Martin Langhoff, 2007-01-08 06:43

Subject: [PATCH] cvsimport: skip commits that are too recent
Message-ID: <11682386193246-git-send-email-martin@catalyst.net.nz>
URL: https://gitlist.dev/e/11682386193246-git-send-email-martin%40catalyst.net.nz

```
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 <martin@catalyst.net.nz>
---
 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 <ae@op5.se>
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;
-- 
1.5.0.rc0.g4017-dirty

```

## Martin Langhoff, 2007-01-08 07:17

Subject: Re: [PATCH] cvsimport: skip commits that are too recent
Message-ID: <46a038f90701072317h9bede00o939d4c078ccd569c@mail.gmail.com>
URL: https://gitlist.dev/e/46a038f90701072317h9bede00o939d4c078ccd569c%40mail.gmail.com
In-Reply-To: <11682386193246-git-send-email-martin@catalyst.net.nz>

```
On 1/8/07, Martin Langhoff <martin@catalyst.net.nz> wrote:
> 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).

Here is the repost with appropriate doco and an override ;-)

In related news, I am trying to debug an import that consistently
skips over remote commits... which is bad, bad news. The culprit seems
to be cvsps -- it skips commits it clearly knows about, and I'm not
sure why. I do think those were commits that cvsps saw half-baked in
the first place.

Passing -x to cvsps does bring those commits back, cvsps with -x can
afford to rewrite history a little bit. As long as the history being
rewritten is not too old we are safe. So with this patch, passing -x
is safer, assuming that 10 minutes is enough of a time window for
cvsps to change opinion about the project history.

 (Before you ask: from a data correctness, this is a fine mess.)

For this repo, I'll start running cvsimport with -o ' -x ' and see how
it behaves. Time-wise, the bandwidth usage and cpu times are roughly
similar for me using --cvs-direct. The patch to do it by default in
cvsimport is trivial, but I'm not entirely happy with the concept just
now.

In any case -- this should be a bit of a warning. cvsps is not
particularly reliable (not that cvs data ever is!), and passing -o '
-x' may help.

cheers,


martin

```

## Martin Langhoff, 2007-01-08 08:24

Subject: Re: [PATCH] cvsimport: skip commits that are too recent
Message-ID: <46a038f90701080024p6629b1a6t227cd1992ad1418@mail.gmail.com>
URL: https://gitlist.dev/e/46a038f90701080024p6629b1a6t227cd1992ad1418%40mail.gmail.com
In-Reply-To: <46a038f90701072317h9bede00o939d4c078ccd569c@mail.gmail.com>

```
On 1/8/07, Martin Langhoff <martin.langhoff@gmail.com> wrote:
> In any case -- this should be a bit of a warning. cvsps is not
> particularly reliable (not that cvs data ever is!), and passing -o '
> -x' may help.

Correction:  it is -p ' -x ' that you need to pass. Things _are_ saner
here with it. YMMV.

cheers,


martin

```

## Robin Rosenberg, 2007-01-11 08:22

Subject: Re: [PATCH] cvsimport: skip commits that are too recent
Message-ID: <200701110922.07997.robin.rosenberg.lists@dewire.com>
URL: https://gitlist.dev/e/200701110922.07997.robin.rosenberg.lists%40dewire.com
In-Reply-To: <11682386193246-git-send-email-martin@catalyst.net.nz>

```
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 <martin@catalyst.net.nz>
> ---
>  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 <ae@op5.se>
> 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

```

## Martin Langhoff, 2007-01-11 20:18

Subject: Re: [PATCH] cvsimport: skip commits that are too recent
Message-ID: <45A69B81.40306@catalyst.net.nz>
URL: https://gitlist.dev/e/45A69B81.40306%40catalyst.net.nz
In-Reply-To: <200701110922.07997.robin.rosenberg.lists@dewire.com>

```
Robin Rosenberg wrote:
> 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. 

This patch did _not_ change how we invoke cvsps at all. It did change
that we now ignore the very recent commits (and pick them up in the next
run), unless you pass -a.

> 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.

Many things would need fixing in cvsps. This aspect [that commits we do
not know if recent activty belongs to a finished commit or a commit that
is still happening], is not cvsps' fault. It is due to the lack of
atomicity in CVS, combined with its rather bad network protocol.

> 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.

What seems to concern you is the "retain cvsps cache" -- which we do.

I did comment later in the thread that we should consider rebuilding the
cvsps cache. The reason for that is that I am seeing LESS breakage than
maintaining the cache. Significantly less.

As you say, however, it is a major change, so I'm still evaluating options.

cheers



martin

```