{"thread":{"id":"6281","subject":"[PATCH] cvsimport: skip commits that are too recent","startedAt":"2007-01-08T06:43:39Z","lastAt":"2007-01-11T20:18:09Z","messageCount":5,"participants":["Martin Langhoff","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"31120","messageId":"11682386193246-git-send-email-martin@catalyst.net.nz","threadId":"6281","inReplyTo":null,"subject":"[PATCH] cvsimport: skip commits that are too recent","fromName":"Martin Langhoff","fromEmail":"martin@catalyst.net.nz","sentAt":"2007-01-08T06:43:39Z","receivedAt":"2007-01-08T06:43:39Z","isPatch":true,"sender":{"key":"martin@laptop.org","avatar":null},"body":"With this patch, cvsimport will skip commits made\nin the last 10 minutes. The recent-ness test is of\n5 minutes + cvsps fuzz window (5 minutes default).\n\nTo force recent commits to be imported, pass the\n-a(ll) flag.\n\nWhen working with a CVS repository that is in use,\nimporting commits that are too recent can lead to\npartially incorrect trees. This is mainly due to\n\n - Commits that are within the cvsps fuzz window may later\n   be found to have affected more files.\n\n - When performing incremental imports, clock drift between\n   the systems may lead to skipped commits.\n\nThis commit helps keep incremental imports of in-use\nCVS repositories sane.\n\nSigned-off-by: Martin Langhoff <martin@catalyst.net.nz>\n---\n Documentation/git-cvsimport.txt |    7 ++++++-\n git-cvsimport.perl              |   20 ++++++++++++++++++--\n 2 files changed, 24 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-cvsimport.txt b/Documentation/git-cvsimport.txt\nindex d21d66b..6deee94 100644\n--- a/Documentation/git-cvsimport.txt\n+++ b/Documentation/git-cvsimport.txt\n@@ -90,7 +90,8 @@ If you need to pass multiple options, separate them with a comma.\n \tPrint a short usage message and exit.\n \n -z <fuzz>::\n-        Pass the timestamp fuzz factor to cvsps.\n+\tPass the timestamp fuzz factor to cvsps, in seconds. If unset,\n+\tcvsps defaults to 300s.\n \n -s <subst>::\n \tSubstitute the character \"/\" in branch names with <subst>\n@@ -99,6 +100,10 @@ If you need to pass multiple options, separate them with a comma.\n \tCVS by default uses the unix username when writing its\n \tcommit logs. Using this option and an author-conv-file\n \tin this format\n+\n+-a::\n+\tImport all commits, including recent ones. cvsimport by default\n+\tskips commits that have a timestamp less than 10 minutes ago.\n +\n ---------\n \texon=Andreas Ericsson <ae@op5.se>\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex c5bf2d1..a75aaa3 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -29,7 +29,7 @@ use IPC::Open2;\n $SIG{'PIPE'}=\"IGNORE\";\n $ENV{'TZ'}=\"UTC\";\n \n-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);\n+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);\n my (%conv_author_name, %conv_author_email);\n \n sub usage() {\n@@ -37,7 +37,7 @@ sub usage() {\n Usage: ${\\basename $0}     # fetch/update GIT from CVS\n        [-o branch-for-HEAD] [-h] [-v] [-d CVSROOT] [-A author-conv-file]\n        [-p opts-for-cvsps] [-C GIT_repository] [-z fuzz] [-i] [-k] [-u]\n-       [-s subst] [-m] [-M regex] [-S regex] [CVS_module]\n+       [-s subst] [-a] [-m] [-M regex] [-S regex] [CVS_module]\n END\n \texit(1);\n }\n@@ -105,6 +105,8 @@ if ($opt_d) {\n }\n $opt_o ||= \"origin\";\n $opt_s ||= \"-\";\n+$opt_a ||= 0;\n+\n my $git_tree = $opt_C;\n $git_tree ||= \".\";\n \n@@ -129,6 +131,11 @@ if ($opt_M) {\n \tpush (@mergerx, qr/$opt_M/);\n }\n \n+# Remember UTC of our starting time\n+# we'll want to avoid importing commits\n+# that are too recent\n+our $starttime = time();\n+\n select(STDERR); $|=1; select(STDOUT);\n \n \n@@ -824,6 +831,15 @@ while (<CVS>) {\n \t\t\t$state = 11;\n \t\t\tnext;\n \t\t}\n+\t\tif ( !$opt_a && $starttime - 300 - (defined $opt_z ? $opt_z : 300) <= $date) {\n+\t\t\t# skip if the commit is too recent\n+\t\t\t# that the cvsps default fuzz is 300s, we give ourselves another\n+\t\t\t# 300s just in case -- this also prevents skipping commits\n+\t\t\t# due to server clock drift\n+\t\t\tprint \"skip patchset $patchset: $date too recent\\n\" if $opt_v;\n+\t\t\t$state = 11;\n+\t\t\tnext;\n+\t\t}\n \t\tif (exists $ignorebranch{$branch}) {\n \t\t\tprint STDERR \"Skipping $branch\\n\";\n \t\t\t$state = 11;\n-- \n1.5.0.rc0.g4017-dirty\n"},{"id":"31122","messageId":"46a038f90701072317h9bede00o939d4c078ccd569c@mail.gmail.com","threadId":"6281","inReplyTo":"11682386193246-git-send-email-martin@catalyst.net.nz","subject":"Re: [PATCH] cvsimport: skip commits that are too recent","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2007-01-08T07:17:25Z","receivedAt":"2007-01-08T07:17:25Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 1/8/07, Martin Langhoff <martin@catalyst.net.nz> wrote:\n> With this patch, cvsimport will skip commits made\n> in the last 10 minutes. The recent-ness test is of\n> 5 minutes + cvsps fuzz window (5 minutes default).\n\nHere is the repost with appropriate doco and an override ;-)\n\nIn related news, I am trying to debug an import that consistently\nskips over remote commits... which is bad, bad news. The culprit seems\nto be cvsps -- it skips commits it clearly knows about, and I'm not\nsure why. I do think those were commits that cvsps saw half-baked in\nthe first place.\n\nPassing -x to cvsps does bring those commits back, cvsps with -x can\nafford to rewrite history a little bit. As long as the history being\nrewritten is not too old we are safe. So with this patch, passing -x\nis safer, assuming that 10 minutes is enough of a time window for\ncvsps to change opinion about the project history.\n\n (Before you ask: from a data correctness, this is a fine mess.)\n\nFor this repo, I'll start running cvsimport with -o ' -x ' and see how\nit behaves. Time-wise, the bandwidth usage and cpu times are roughly\nsimilar for me using --cvs-direct. The patch to do it by default in\ncvsimport is trivial, but I'm not entirely happy with the concept just\nnow.\n\nIn any case -- this should be a bit of a warning. cvsps is not\nparticularly reliable (not that cvs data ever is!), and passing -o '\n-x' may help.\n\ncheers,\n\n\nmartin\n"},{"id":"31123","messageId":"46a038f90701080024p6629b1a6t227cd1992ad1418@mail.gmail.com","threadId":"6281","inReplyTo":"46a038f90701072317h9bede00o939d4c078ccd569c@mail.gmail.com","subject":"Re: [PATCH] cvsimport: skip commits that are too recent","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2007-01-08T08:24:43Z","receivedAt":"2007-01-08T08:24:43Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 1/8/07, Martin Langhoff <martin.langhoff@gmail.com> wrote:\n> In any case -- this should be a bit of a warning. cvsps is not\n> particularly reliable (not that cvs data ever is!), and passing -o '\n> -x' may help.\n\nCorrection:  it is -p ' -x ' that you need to pass. Things _are_ saner\nhere with it. YMMV.\n\ncheers,\n\n\nmartin\n"},{"id":"31437","messageId":"200701110922.07997.robin.rosenberg.lists@dewire.com","threadId":"6281","inReplyTo":"11682386193246-git-send-email-martin@catalyst.net.nz","subject":"Re: [PATCH] cvsimport: skip commits that are too recent","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2007-01-11T08:22:07Z","receivedAt":"2007-01-11T08:22:07Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"The idea is nice,  but the downside of this patch is that I (and presumably \nothers) have to rewrite the scripts to invoke cvsps explicitly now. The fix \nshould really be in cvsps, not git-cvsimport (which is the reason I haven't \nfixed this). Running a full cvsps takes two hours and consumes more than a \ngigabyte of memory for me, which makes it impossible to run on all but one \nmachine, wheras the incremental import runs in less than five minutes on any \nmachine.\n\nAdd to that the risk that the buggy nature of cvsps probably increases the \nrisk of errors, so please make the old behaviour the default (import all, \nretain cvsps cache) and make the changed behaviour the result of an explicit \nswitch.\n\n-- robin\n\nmåndag 08 januari 2007 07:43 skrev Martin Langhoff:\n> With this patch, cvsimport will skip commits made\n> in the last 10 minutes. The recent-ness test is of\n> 5 minutes + cvsps fuzz window (5 minutes default).\n>\n> To force recent commits to be imported, pass the\n> -a(ll) flag.\n>\n> When working with a CVS repository that is in use,\n> importing commits that are too recent can lead to\n> partially incorrect trees. This is mainly due to\n>\n>  - Commits that are within the cvsps fuzz window may later\n>    be found to have affected more files.\n>\n>  - When performing incremental imports, clock drift between\n>    the systems may lead to skipped commits.\n>\n> This commit helps keep incremental imports of in-use\n> CVS repositories sane.\n>\n> Signed-off-by: Martin Langhoff <martin@catalyst.net.nz>\n> ---\n>  Documentation/git-cvsimport.txt |    7 ++++++-\n>  git-cvsimport.perl              |   20 ++++++++++++++++++--\n>  2 files changed, 24 insertions(+), 3 deletions(-)\n>\n> diff --git a/Documentation/git-cvsimport.txt\n> b/Documentation/git-cvsimport.txt index d21d66b..6deee94 100644\n> --- a/Documentation/git-cvsimport.txt\n> +++ b/Documentation/git-cvsimport.txt\n> @@ -90,7 +90,8 @@ If you need to pass multiple options, separate them with\n> a comma. Print a short usage message and exit.\n>\n>  -z <fuzz>::\n> -        Pass the timestamp fuzz factor to cvsps.\n> +\tPass the timestamp fuzz factor to cvsps, in seconds. If unset,\n> +\tcvsps defaults to 300s.\n>\n>  -s <subst>::\n>  \tSubstitute the character \"/\" in branch names with <subst>\n> @@ -99,6 +100,10 @@ If you need to pass multiple options, separate them\n> with a comma. CVS by default uses the unix username when writing its\n>  \tcommit logs. Using this option and an author-conv-file\n>  \tin this format\n> +\n> +-a::\n> +\tImport all commits, including recent ones. cvsimport by default\n> +\tskips commits that have a timestamp less than 10 minutes ago.\n>  +\n>  ---------\n>  \texon=Andreas Ericsson <ae@op5.se>\n> diff --git a/git-cvsimport.perl b/git-cvsimport.perl\n> index c5bf2d1..a75aaa3 100755\n> --- a/git-cvsimport.perl\n> +++ b/git-cvsimport.perl\n> @@ -29,7 +29,7 @@ use IPC::Open2;\n>  $SIG{'PIPE'}=\"IGNORE\";\n>  $ENV{'TZ'}=\"UTC\";\n>\n> -our\n> ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt\n>_P, $opt_s,$opt_m,$opt_M,$opt_A,$opt_S,$opt_L); +our\n> ($opt_h,$opt_o,$opt_v,$opt_k,$opt_u,$opt_d,$opt_p,$opt_C,$opt_z,$opt_i,$opt\n>_P, $opt_s,$opt_m,$opt_M,$opt_A,$opt_S,$opt_L, $opt_a); my\n> (%conv_author_name, %conv_author_email);\n>\n>  sub usage() {\n> @@ -37,7 +37,7 @@ sub usage() {\n>  Usage: ${\\basename $0}     # fetch/update GIT from CVS\n>         [-o branch-for-HEAD] [-h] [-v] [-d CVSROOT] [-A author-conv-file]\n>         [-p opts-for-cvsps] [-C GIT_repository] [-z fuzz] [-i] [-k] [-u]\n> -       [-s subst] [-m] [-M regex] [-S regex] [CVS_module]\n> +       [-s subst] [-a] [-m] [-M regex] [-S regex] [CVS_module]\n>  END\n>  \texit(1);\n>  }\n> @@ -105,6 +105,8 @@ if ($opt_d) {\n>  }\n>  $opt_o ||= \"origin\";\n>  $opt_s ||= \"-\";\n> +$opt_a ||= 0;\n> +\n>  my $git_tree = $opt_C;\n>  $git_tree ||= \".\";\n>\n> @@ -129,6 +131,11 @@ if ($opt_M) {\n>  \tpush (@mergerx, qr/$opt_M/);\n>  }\n>\n> +# Remember UTC of our starting time\n> +# we'll want to avoid importing commits\n> +# that are too recent\n> +our $starttime = time();\n> +\n>  select(STDERR); $|=1; select(STDOUT);\n>\n>\n> @@ -824,6 +831,15 @@ while (<CVS>) {\n>  \t\t\t$state = 11;\n>  \t\t\tnext;\n>  \t\t}\n> +\t\tif ( !$opt_a && $starttime - 300 - (defined $opt_z ? $opt_z : 300) <=\n> $date) { +\t\t\t# skip if the commit is too recent\n> +\t\t\t# that the cvsps default fuzz is 300s, we give ourselves another\n> +\t\t\t# 300s just in case -- this also prevents skipping commits\n> +\t\t\t# due to server clock drift\n> +\t\t\tprint \"skip patchset $patchset: $date too recent\\n\" if $opt_v;\n> +\t\t\t$state = 11;\n> +\t\t\tnext;\n> +\t\t}\n>  \t\tif (exists $ignorebranch{$branch}) {\n>  \t\t\tprint STDERR \"Skipping $branch\\n\";\n>  \t\t\t$state = 11;\n\n-- \nTESTMAIL\n"},{"id":"31488","messageId":"45A69B81.40306@catalyst.net.nz","threadId":"6281","inReplyTo":"200701110922.07997.robin.rosenberg.lists@dewire.com","subject":"Re: [PATCH] cvsimport: skip commits that are too recent","fromName":"Martin Langhoff","fromEmail":"martin@catalyst.net.nz","sentAt":"2007-01-11T20:18:09Z","receivedAt":"2007-01-11T20:18:09Z","isPatch":true,"sender":{"key":"martin@laptop.org","avatar":null},"body":"Robin Rosenberg wrote:\n> The idea is nice,  but the downside of this patch is that I (and presumably \n> others) have to rewrite the scripts to invoke cvsps explicitly now. \n\nThis patch did _not_ change how we invoke cvsps at all. It did change\nthat we now ignore the very recent commits (and pick them up in the next\nrun), unless you pass -a.\n\n> The fix\n> should really be in cvsps, not git-cvsimport (which is the reason I haven't \n> fixed this). Running a full cvsps takes two hours and consumes more than a \n> gigabyte of memory for me, which makes it impossible to run on all but one \n> machine, wheras the incremental import runs in less than five minutes on any \n> machine.\n\nMany things would need fixing in cvsps. This aspect [that commits we do\nnot know if recent activty belongs to a finished commit or a commit that\nis still happening], is not cvsps' fault. It is due to the lack of\natomicity in CVS, combined with its rather bad network protocol.\n\n> Add to that the risk that the buggy nature of cvsps probably increases the \n> risk of errors, so please make the old behaviour the default (import all, \n> retain cvsps cache) and make the changed behaviour the result of an explicit \n> switch.\n\nWhat seems to concern you is the \"retain cvsps cache\" -- which we do.\n\nI did comment later in the thread that we should consider rebuilding the\ncvsps cache. The reason for that is that I am seeing LESS breakage than\nmaintaining the cache. Significantly less.\n\nAs you say, however, it is a major change, so I'm still evaluating options.\n\ncheers\n\n\n\nmartin\n"}]}