{"thread":{"id":"3373","subject":"git-cvs-import retries","startedAt":"2006-02-17T19:38:05Z","lastAt":"2006-02-18T20:44:20Z","messageCount":5,"participants":["Martin Mares","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"16334","messageId":"mj+md-20060217.193146.10308.albireo@ucw.cz","threadId":"3373","inReplyTo":null,"subject":"git-cvs-import retries","fromName":"Martin Mares","fromEmail":"mj@ucw.cz","sentAt":"2006-02-17T19:38:05Z","receivedAt":"2006-02-17T19:38:05Z","isPatch":false,"sender":{"key":"mj@ucw.cz","avatar":null},"body":"Hello!\n\nI am trying git-cvsimport on a rather huge repository and the CVS server\nsometimes drops the connection and the whole importing aborts, although\nit contains some retrying logic. I've noticed that in the connection closes\nI experience, $res ends up being empty instead of undefined. This is tested\nby the `server went again' check, but not by the retry check a couple of\nlines before.\n\nThis patch extends the retry check and makes the symptoms go away.\nHowever, take it with a grain of salt as I don't understand yet why the\nconnection is aborted.\n\n\t\t\t\tHave a nice fortnight\n-- \nMartin `MJ' Mares   <mj@ucw.cz>   http://atrey.karlin.mff.cuni.cz/~mj/\nFaculty of Math and Physics, Charles University, Prague, Czech Rep., Earth\nA jury consists of 12 persons chosen to decide who has the better lawyer.\n\n\nSigned-Off-By: Martin Mares <mj@ucw.cz>\n\n--- old/git-cvsimport\t2006-02-17 13:02:24.000000000 +0100\n+++ new/git-cvsimport\t2006-02-17 18:13:06.000000000 +0100\n@@ -371,7 +371,7 @@\n \n \t$self->_file($fn,$rev) and $res = $self->_line($fh);\n \n-\tif (!defined $res) {\n+\tif (!defined $res || $res eq '') {\n \t    # retry\n \t    $self->conn();\n \t    $self->_file($fn,$rev)\n"},{"id":"16368","messageId":"7v1wy1t9cb.fsf@assigned-by-dhcp.cox.net","threadId":"3373","inReplyTo":"mj+md-20060217.193146.10308.albireo@ucw.cz","subject":"Re: git-cvs-import retries","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-18T07:27:48Z","receivedAt":"2006-02-18T07:27:48Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Mares <mj@ucw.cz> writes:\n\n> Hello!\n>...\n> This patch extends the retry check and makes the symptoms go away.\n> However, take it with a grain of salt as I don't understand yet why the\n> connection is aborted.\n>\n> \t\t\t\tHave a nice fortnight\n> -- \n> Martin `MJ' Mares   <mj@ucw.cz>   http://atrey.karlin.mff.cuni.cz/~mj/\n> Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth\n> A jury consists of 12 persons chosen to decide who has the better lawyer.\n>\n>\n> Signed-Off-By: Martin Mares <mj@ucw.cz>\n>\n> --- old/git-cvsimport\t2006-02-17 13:02:24.000000000 +0100\n\nFirst, one technicality.  You can see what's wrong with the\nabove, right?  Remember, the top part of your message goes into\nthe commit log, so we do not want \"Hello!\" nor signature.\n\n> +++ new/git-cvsimport\t2006-02-17 18:13:06.000000000 +0100\n> @@ -371,7 +371,7 @@\n>  \n>  \t$self->_file($fn,$rev) and $res = $self->_line($fh);\n>  \n> -\tif (!defined $res) {\n> +\tif (!defined $res || $res eq '') {\n>  \t    # retry\n>  \t    $self->conn();\n>  \t    $self->_file($fn,$rev)\n\nI read _line() three times but its return value is the lexical\nvariable $res which is initialized to 0 and then either reset to\n0 by assignment or updated with $res += somethingelse.  So I do\nnot see how you can get a defined but empty string in there.\nEven when _file() returns false, the $res variable in file()\n(the function you are modifying) is not initialized, so it would\nstay undefined.\n\nMaybe I am missing something very obvious, but I cannot see how\nthis can make any difference.  Please enlighten.\n"},{"id":"16384","messageId":"mj+md-20060218.130645.5680.albireo@ucw.cz","threadId":"3373","inReplyTo":"7v1wy1t9cb.fsf@assigned-by-dhcp.cox.net","subject":"Re: git-cvs-import retries","fromName":"Martin Mares","fromEmail":"mj@ucw.cz","sentAt":"2006-02-18T13:14:07Z","receivedAt":"2006-02-18T13:14:07Z","isPatch":false,"sender":{"key":"mj@ucw.cz","avatar":null},"body":"Hi Junio!\n\n> First, one technicality.  You can see what's wrong with the\n> above, right?  Remember, the top part of your message goes into\n> the commit log, so we do not want \"Hello!\" nor signature.\n\nSorry about that, the patch was intended more for discussion than\nfor applying.\n\n> I read _line() three times but its return value is the lexical\n> variable $res which is initialized to 0 and then either reset to\n> 0 by assignment or updated with $res += somethingelse.  So I do\n> not see how you can get a defined but empty string in there.\n\nYou almost convinced me that my fix couldn't have changed anything :-)\n\nBut it did and I finally understand why: _line() can exit not only\nby return, but also by falling over when readline() returns undef.\nIn this case, something weird is returned (the most recent expression\nevaluated) and it's *sometimes* the empty string.\n\nI will send a new patch.\n\n\t\t\t\tHave a nice fortnight\n-- \nMartin `MJ' Mares   <mj@ucw.cz>   http://atrey.karlin.mff.cuni.cz/~mj/\nFaculty of Math and Physics, Charles University, Prague, Czech Rep., Earth\nThe first myth of management is that it exists.\n"},{"id":"16391","messageId":"7vvevco6e3.fsf@assigned-by-dhcp.cox.net","threadId":"3373","inReplyTo":"mj+md-20060218.130645.5680.albireo@ucw.cz","subject":"Re: git-cvs-import retries","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-02-18T18:42:44Z","receivedAt":"2006-02-18T18:42:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Mares <mj@ucw.cz> writes:\n\n> But it did and I finally understand why: _line() can exit not only\n> by return, but also by falling over when readline() returns undef.\n\nAh, you are right and I feel stupid.  It's been a while since I\nwrote real Perl code the last time, and forgot that \"the last\nevaluation\" rule when I was reading the code.\n\nIt did not help that one of the languages I use in my day-job\n(which I am too ashamed to even name) uses \"if fell off at the\nend return undef\" rule X-<.\n"},{"id":"16393","messageId":"mj+md-20060218.203658.16817.albireo@ucw.cz","threadId":"3373","inReplyTo":"7vvevco6e3.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] Fix retries in git-cvsimport","fromName":"Martin Mares","fromEmail":"mj@ucw.cz","sentAt":"2006-02-18T20:44:20Z","receivedAt":"2006-02-18T20:44:20Z","isPatch":true,"sender":{"key":"mj@ucw.cz","avatar":null},"body":"Fixed a couple of bugs in recovering from broken connections:\n\nThe _line() method now returns undef correctly when the connection\nis broken instead of falling off the function and returning garbage.\n\nRetries are now reported to stderr and the eventual partially\ndownloaded file is discarded instead of being appended to.\n\nThe \"Server gone away\" test has been removed, because it was\nreachable only if the garbage return bug bit.\n\nSigned-Off-By: Martin Mares <mj@ucw.cz>\n\n--- old/git-cvsimport\t2006-02-17 13:02:24.000000000 +0100\n+++ new/git-cvsimport\t2006-02-18 14:16:33.000000000 +0100\n@@ -361,6 +361,7 @@\n \t\t\t}\n \t\t}\n \t}\n+\treturn undef;\n }\n sub file {\n \tmy($self,$fn,$rev) = @_;\n@@ -372,19 +373,15 @@\n \t$self->_file($fn,$rev) and $res = $self->_line($fh);\n \n \tif (!defined $res) {\n-\t    # retry\n+\t    print STDERR \"Server has gone away while fetching $fn $rev, retrying...\\n\";\n+\t    truncate $fh, 0;\n \t    $self->conn();\n-\t    $self->_file($fn,$rev)\n-\t\t    or die \"No file command send\\n\";\n+\t    $self->_file($fn,$rev) or die \"No file command send\";\n \t    $res = $self->_line($fh);\n-\t    die \"No input: $fn $rev\\n\" unless defined $res;\n+\t    die \"Retry failed\" unless defined $res;\n \t}\n \tclose ($fh);\n \n-\tif ($res eq '') {\n-\t    die \"Looks like the server has gone away while fetching $fn $rev -- exiting!\";\n-\t}\n-\n \treturn ($name, $res);\n }\n \n"}]}