{"thread":{"id":"17318","subject":"Re: [PATCH] git-cvsserver: run post-update hook *after* update.","startedAt":"2009-01-23T05:43:48Z","lastAt":"2009-01-30T01:32:14Z","messageCount":12,"participants":["Stefan Karpinski","Junio C Hamano","Andy Parkins","Martin Langhoff"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"101612","messageId":"d4bc1a2a0901222143i1a7dd051h1778dcb563120195@mail.gmail.com","threadId":"17318","inReplyTo":"1232144521-21947-2-git-send-email-stefan.karpinski@gmail.com","subject":"Re: [PATCH] git-cvsserver: run post-update hook *after* update.","fromName":"Stefan Karpinski","fromEmail":"stefan.karpinski@gmail.com","sentAt":"2009-01-23T05:43:48Z","receivedAt":"2009-01-23T05:43:48Z","isPatch":true,"sender":{"key":"stefan.karpinski@gmail.com","avatar":"https://gravatar.com/avatar/780cfb8dd7d7dc749d7276a4ca2ec24e7f0482cfce509717c5ddd165fd2cc9d9?d=mp&s=160"},"body":"I know that this and the other patch I sent are completely trivial and\nuninteresting, but they would appear to be correct. Do I need to prod\nmore to get them included or what? Did I submit them incorrectly?\n\nOn Fri, Jan 16, 2009 at 2:22 PM, Stefan Karpinski\n<stefan.karpinski@gmail.com> wrote:\n>\n> CVS server was running the hook before the update\n> action was actually done. This performs the update\n> before the hook is called.\n> ---\n>\n> Unless I'm severely misunderstanding the meaning of\n> a *post-update* hook, I think this is a no-brainer.\n>\n>  git-cvsserver.perl |    4 ++--\n>  1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index c1e09ea..d2e6003 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -1413,14 +1413,14 @@ sub req_ci\n>                close $pipe || die \"bad pipe: $! $?\";\n>        }\n>\n> +    $updater->update();\n> +\n>        ### Then hooks/post-update\n>        $hook = $ENV{GIT_DIR}.'hooks/post-update';\n>        if (-x $hook) {\n>                system($hook, \"refs/heads/$state->{module}\");\n>        }\n>\n> -    $updater->update();\n> -\n>     # foreach file specified on the command line ...\n>     foreach my $filename ( @committedfiles )\n>     {\n> --\n> 1.6.0.3.3.g08dd8\n>\n"},{"id":"101627","messageId":"7viqo61mfq.fsf@gitster.siamese.dyndns.org","threadId":"17318","inReplyTo":"d4bc1a2a0901222143i1a7dd051h1778dcb563120195@mail.gmail.com","subject":"Re: [PATCH] git-cvsserver: run post-update hook *after* update.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-23T08:00:41Z","receivedAt":"2009-01-23T08:00:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Karpinski <stefan.karpinski@gmail.com> writes:\n\n> I know that this and the other patch I sent are completely trivial and\n> uninteresting, but they would appear to be correct. Do I need to prod\n> more to get them included or what? Did I submit them incorrectly?\n\nIf you spend the bandwidth to quote the whole patch, don't quote it, but\nplease use the same bandwidth to resend it --- that way, if the reason\nyour patch left unapplied was because your earlier submission was lost in\nthe noise or too heavy maintainer workload, it can be easily picked up.\n\nUpon my cursory look the patch looks sane, even though it risks breaking\npeople's scripts that relied on the incorrect behaviour of running the\nhook before the update is done, which is slightly worrysome.  Find out who\nare knowledgeable in the area of the code you are touching, and Cc them to\nask their input.  \"git shortlog -s -n git-cvsserver.perl\" may help.\n\nPlease sign your patch.\n\nThanks.\n\nOh, and one more thing.  Please do not top post.\n\n> On Fri, Jan 16, 2009 at 2:22 PM, Stefan Karpinski\n> <stefan.karpinski@gmail.com> wrote:\n>>\n>> CVS server was running the hook before the update\n>> action was actually done. This performs the update\n>> before the hook is called.\n>> ---\n>>\n>> Unless I'm severely misunderstanding the meaning of\n>> a *post-update* hook, I think this is a no-brainer.\n>>\n>>  git-cvsserver.perl |    4 ++--\n>>  1 files changed, 2 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n>> index c1e09ea..d2e6003 100755\n>> --- a/git-cvsserver.perl\n>> +++ b/git-cvsserver.perl\n>> @@ -1413,14 +1413,14 @@ sub req_ci\n>>                close $pipe || die \"bad pipe: $! $?\";\n>>        }\n>>\n>> +    $updater->update();\n>> +\n>>        ### Then hooks/post-update\n>>        $hook = $ENV{GIT_DIR}.'hooks/post-update';\n>>        if (-x $hook) {\n>>                system($hook, \"refs/heads/$state->{module}\");\n>>        }\n>>\n>> -    $updater->update();\n>> -\n>>     # foreach file specified on the command line ...\n>>     foreach my $filename ( @committedfiles )\n>>     {\n>> --\n>> 1.6.0.3.3.g08dd8\n>>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"102505","messageId":"1233264914-7798-1-git-send-email-stefan.karpinski@gmail.com","threadId":"17318","inReplyTo":"1232144521-21947-1-git-send-email-stefan.karpinski@gmail.com","subject":"[PATCH] git-cvsserver: handle CVS 'noop' command.","fromName":"Stefan Karpinski","fromEmail":"stefan.karpinski@gmail.com","sentAt":"2009-01-29T21:35:14Z","receivedAt":"2009-01-29T21:35:14Z","isPatch":true,"sender":{"key":"stefan.karpinski@gmail.com","avatar":"https://gravatar.com/avatar/780cfb8dd7d7dc749d7276a4ca2ec24e7f0482cfce509717c5ddd165fd2cc9d9?d=mp&s=160"},"body":"The implementation is trivial: ignore the 'noop' command\nif it is sent. This command is issued by some CVS clients,\nnotably TortoiseCVS. Without this patch, TortoiseCVS will\nchoke when git-cvsserver complains about the unsupported\ncommand.\n\nSigned-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>\n---\n\nSince this change has no negative impact, is too simple to\nbe wrong, and improves interaction with some clients, it\nseem to me like a no-brainer to apply it.\n\n git-cvsserver.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex fef7faf..c1e09ea 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -188,7 +188,7 @@ while (<STDIN>)\n         # use the $methods hash to call the appropriate sub for this command\n         #$log->info(\"Method : $1\");\n         &{$methods->{$1}}($1,$2);\n-    } else {\n+    } elsif ($1 ne 'noop') {\n         # log fatal because we don't understand this function. If this happens\n         # we're fairly screwed because we don't know if the client is expecting\n         # a response. If it is, the client will hang, we'll hang, and the whole\n-- \n1.6.0.3.3.g08dd8\n"},{"id":"102506","messageId":"1233266282-8010-1-git-send-email-stefan.karpinski@gmail.com","threadId":"17318","inReplyTo":"7viqo61mfq.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] git-cvsserver: run post-update hook *after* update.","fromName":"Stefan Karpinski","fromEmail":"stefan.karpinski@gmail.com","sentAt":"2009-01-29T21:58:02Z","receivedAt":"2009-01-29T21:58:02Z","isPatch":true,"sender":{"key":"stefan.karpinski@gmail.com","avatar":"https://gravatar.com/avatar/780cfb8dd7d7dc749d7276a4ca2ec24e7f0482cfce509717c5ddd165fd2cc9d9?d=mp&s=160"},"body":"CVS server was running the hook before the update action was\nactually done. This performs the update before the hook is called.\n\nThe original commit that introduced the current incorrect behavior\nwas 394d66d \"git-cvsserver runs hooks/post-update\". The error in\nordering of the hook call appears to have gone unnoticed, but since\ngit-cvsserver is supposed to emulate receive-pack, it stands to\nreason that the hook should be run *after* the update. Since this\nbehavior is inconsistent with recieve-pack, users are either:\n\n  1) not using post-update hooks with git-cvsserver;\n  2) using post-update hooks that don't care whether they are\n     called before or after the actual update occurs;\n  3) using post-update hooks *only* with git-cvsserver, and\n     relying on the hook being called just before the update.\n\nThis patch would affect only users in case 3. These users are\ndepending on fairly obviously wrong behavior, and moreover they can\nsimply change their current post-update into post-recieve hooks,\nand their systems will work correctly again.\n\nSigned-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>\n---\nI'm CCing Andy Parkins, Michael Witten, and Junio Hamano, who\nauthored the other three commits implementing or affecting hooks in\ngit-cvsserver (394d66d, cdf6328, b2741f6). If you could please take\na look at this patch and comment on if it's harmful or not, it\nwould be much appreciated.\n\n git-cvsserver.perl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex c1e09ea..d2e6003 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -1413,14 +1413,14 @@ sub req_ci\n \t\tclose $pipe || die \"bad pipe: $! $?\";\n \t}\n \n+    $updater->update();\n+\n \t### Then hooks/post-update\n \t$hook = $ENV{GIT_DIR}.'hooks/post-update';\n \tif (-x $hook) {\n \t\tsystem($hook, \"refs/heads/$state->{module}\");\n \t}\n \n-    $updater->update();\n-\n     # foreach file specified on the command line ...\n     foreach my $filename ( @committedfiles )\n     {\n-- \n1.6.0.3.3.g08dd8\n"},{"id":"102517","messageId":"7v7i4denpg.fsf@gitster.siamese.dyndns.org","threadId":"17318","inReplyTo":"1233264914-7798-1-git-send-email-stefan.karpinski@gmail.com","subject":"Re: [PATCH] git-cvsserver: handle CVS 'noop' command.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-29T22:45:15Z","receivedAt":"2009-01-29T22:45:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Karpinski <stefan.karpinski@gmail.com> writes:\n\n> The implementation is trivial: ignore the 'noop' command\n> if it is sent. This command is issued by some CVS clients,\n> notably TortoiseCVS. Without this patch, TortoiseCVS will\n> choke when git-cvsserver complains about the unsupported\n> command.\n>\n> Signed-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>\n> ---\n>\n> Since this change has no negative impact, is too simple to\n> be wrong, and improves interaction with some clients, it\n> seem to me like a no-brainer to apply it.\n>\n>  git-cvsserver.perl |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index fef7faf..c1e09ea 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -188,7 +188,7 @@ while (<STDIN>)\n>          # use the $methods hash to call the appropriate sub for this command\n>          #$log->info(\"Method : $1\");\n>          &{$methods->{$1}}($1,$2);\n> -    } else {\n> +    } elsif ($1 ne 'noop') {\n>          # log fatal because we don't understand this function. If this happens\n>          # we're fairly screwed because we don't know if the client is expecting\n>          # a response. If it is, the client will hang, we'll hang, and the whole\n> -- \n> 1.6.0.3.3.g08dd8\n\nNot a no-brainer at all, sorry.\n\nImagine what you would do when you discover another request a random other\nclient sends that you would want to ignore just like you did for 'noop'.\nViewed in this light, your patch is a very short sighted one that has a\nbig negative impact on maintainability.\n\nA true no-brainer that has no negative impact would have been something\nlike the attached patch, that adds a method that does not do anything.\n\nEven then, between req_CATCHALL and req_EMPTY, I am not sure which one is\nexpected by the clients, without consulting to the protocol documentation\nfor cvs server/client communication.  In the attached patch, I am guessing\nfrom your patch that at least Tortoise does not expect any response to\nit.\n\n git-cvsserver.perl |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git i/git-cvsserver.perl w/git-cvsserver.perl\nindex fef7faf..ca47e08 100755\n--- i/git-cvsserver.perl\n+++ w/git-cvsserver.perl\n@@ -71,6 +71,7 @@ my $methods = {\n     'log'             => \\&req_log,\n     'rlog'            => \\&req_log,\n     'tag'             => \\&req_CATCHALL,\n+    'noop'            => \\&req_CATCHALL,\n     'status'          => \\&req_status,\n     'admin'           => \\&req_CATCHALL,\n     'history'         => \\&req_CATCHALL,\n"},{"id":"102519","messageId":"7v3af1enkq.fsf@gitster.siamese.dyndns.org","threadId":"17318","inReplyTo":"1233266282-8010-1-git-send-email-stefan.karpinski@gmail.com","subject":"Re: [PATCH] git-cvsserver: run post-update hook *after* update.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-29T22:48:05Z","receivedAt":"2009-01-29T22:48:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Karpinski <stefan.karpinski@gmail.com> writes:\n\n> CVS server was running the hook before the update action was\n> actually done. This performs the update before the hook is called.\n>\n> The original commit that introduced the current incorrect behavior\n> was 394d66d \"git-cvsserver runs hooks/post-update\". The error in\n> ordering of the hook call appears to have gone unnoticed, but since\n> git-cvsserver is supposed to emulate receive-pack, it stands to\n> reason that the hook should be run *after* the update. Since this\n> behavior is inconsistent with recieve-pack, users are either:\n>\n>   1) not using post-update hooks with git-cvsserver;\n>   2) using post-update hooks that don't care whether they are\n>      called before or after the actual update occurs;\n>   3) using post-update hooks *only* with git-cvsserver, and\n>      relying on the hook being called just before the update.\n>\n> This patch would affect only users in case 3. These users are\n> depending on fairly obviously wrong behavior, and moreover they can\n> simply change their current post-update into post-recieve hooks,\n> and their systems will work correctly again.\n>\n> Signed-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>\n> ---\n> I'm CCing Andy Parkins, Michael Witten, and Junio Hamano, who\n> authored the other three commits implementing or affecting hooks in\n> git-cvsserver (394d66d, cdf6328, b2741f6). If you could please take\n> a look at this patch and comment on if it's harmful or not, it\n> would be much appreciated.\n\nI think I've seen this one before and I thought it was a sensible thing to\ndo (and perhaps I even said so here).\n\nIs this a resend?  If so, let's queue it in at least 'next' and see if\nanybody screams ;-).  For a program near the fringe like cvsserver, not\nmany people run it but the small number of people who run it gets hurt\nrather quickly if the updated behaviour breaks their existing practice,\nand sometimes breaking things for them would be the only way to extract\nany response.  Yes, it is very unfortunate.\n\n>  git-cvsserver.perl |    4 ++--\n>  1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index c1e09ea..d2e6003 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -1413,14 +1413,14 @@ sub req_ci\n>  \t\tclose $pipe || die \"bad pipe: $! $?\";\n>  \t}\n>  \n> +    $updater->update();\n> +\n>  \t### Then hooks/post-update\n>  \t$hook = $ENV{GIT_DIR}.'hooks/post-update';\n>  \tif (-x $hook) {\n>  \t\tsystem($hook, \"refs/heads/$state->{module}\");\n>  \t}\n>  \n> -    $updater->update();\n> -\n>      # foreach file specified on the command line ...\n>      foreach my $filename ( @committedfiles )\n>      {\n> -- \n> 1.6.0.3.3.g08dd8\n"},{"id":"102520","messageId":"200901292256.30239.andyparkins@gmail.com","threadId":"17318","inReplyTo":"1233266282-8010-1-git-send-email-stefan.karpinski@gmail.com","subject":"Re: [PATCH] git-cvsserver: run post-update hook *after* update.","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2009-01-29T22:56:29Z","receivedAt":"2009-01-29T22:56:29Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Thursday 29 January 2009 21:58:02 Stefan Karpinski wrote:\n\n> This patch would affect only users in case 3. These users are\n> depending on fairly obviously wrong behavior, and moreover they can\n> simply change their current post-update into post-recieve hooks,\n> and their systems will work correctly again.\n\nQuite right.\n\n>\n> Signed-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>\nAcked-By: Andy Parkins <andyparkins@gmail.com>\n\n-- \nDr Andy Parkins\nandyparkins@gmail.com\n"},{"id":"102526","messageId":"d4bc1a2a0901291526v48e61c1dtde35fa8b77c71560@mail.gmail.com","threadId":"17318","inReplyTo":"7v3af1enkq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsserver: run post-update hook *after* update.","fromName":"Stefan Karpinski","fromEmail":"stefan.karpinski@gmail.com","sentAt":"2009-01-29T23:26:28Z","receivedAt":"2009-01-29T23:26:28Z","isPatch":true,"sender":{"key":"stefan.karpinski@gmail.com","avatar":"https://gravatar.com/avatar/780cfb8dd7d7dc749d7276a4ca2ec24e7f0482cfce509717c5ddd165fd2cc9d9?d=mp&s=160"},"body":"On Thu, Jan 29, 2009 at 2:48 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> I think I've seen this one before and I thought it was a sensible thing to\n> do (and perhaps I even said so here).\n\nYou said it looked sane but that I should resend CCing knowledgable parties.\n\n> Is this a resend?  If so, let's queue it in at least 'next' and see if\n> anybody screams ;-).  For a program near the fringe like cvsserver, not\n> many people run it but the small number of people who run it gets hurt\n> rather quickly if the updated behaviour breaks their existing practice,\n> and sometimes breaking things for them would be the only way to extract\n> any response.  Yes, it is very unfortunate.\n\nYes, it is a resend, but I expanded on the commit message, including\nmy analysis of the potential impact.\n"},{"id":"102527","messageId":"d4bc1a2a0901291539m636f0fc8s5d9280ce9b7d22b2@mail.gmail.com","threadId":"17318","inReplyTo":"7v7i4denpg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsserver: handle CVS 'noop' command.","fromName":"Stefan Karpinski","fromEmail":"stefan.karpinski@gmail.com","sentAt":"2009-01-29T23:39:23Z","receivedAt":"2009-01-29T23:39:23Z","isPatch":true,"sender":{"key":"stefan.karpinski@gmail.com","avatar":"https://gravatar.com/avatar/780cfb8dd7d7dc749d7276a4ca2ec24e7f0482cfce509717c5ddd165fd2cc9d9?d=mp&s=160"},"body":"On Thu, Jan 29, 2009 at 2:45 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Imagine what you would do when you discover another request a random other\n> client sends that you would want to ignore just like you did for 'noop'.\n> Viewed in this light, your patch is a very short sighted one that has a\n> big negative impact on maintainability.\n\nFair enough. I was trying to change the minimal amount that I could\nand still fix the breakage. Your patch is much better. Not to mention\nterser ;-)\n\n> A true no-brainer that has no negative impact would have been something\n> like the attached patch, that adds a method that does not do anything.\n>\n> Even then, between req_CATCHALL and req_EMPTY, I am not sure which one is\n> expected by the clients, without consulting to the protocol documentation\n> for cvs server/client communication.  In the attached patch, I am guessing\n> from your patch that at least Tortoise does not expect any response to\n> it.\n\nI have consulted the CVS protocol documentation (found at\nhttp://www.wandisco.com/techpubs/cvs-protocol.pdf), which states the\nfollowing about the \"noop\" command:\n\n\"Response expected: yes. This request is a null command in the sense\nthat it doesn't do anything, but\nmerely (as with any other requests expecting a response) sends back\nany responses pertaining to\npending errors, pending Notified responses, etc.\"\n\nSo apparently a response *is* expected. I'm not really familiar enough\nwith CVS or git-cvsserver to determine what that means it should do,\nbut I suspect from perusing the code that req_EMPTY is the appropriate\naction.\n\nMoreover, I've moved on from using git-cvsserver myself, having\ninstead convinced my Windows-using compatriots to use msysgit instead.\nSo if you feel that this change is unwarranted, feel free to just drop\nit.\n"},{"id":"102528","messageId":"7vhc3hd6ba.fsf@gitster.siamese.dyndns.org","threadId":"17318","inReplyTo":"d4bc1a2a0901291539m636f0fc8s5d9280ce9b7d22b2@mail.gmail.com","subject":"Re: [PATCH] git-cvsserver: handle CVS 'noop' command.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-01-29T23:46:17Z","receivedAt":"2009-01-29T23:46:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Karpinski <stefan.karpinski@gmail.com> writes:\n\n> So apparently a response *is* expected. I'm not really familiar enough\n> with CVS or git-cvsserver to determine what that means it should do,\n> but I suspect from perusing the code that req_EMPTY is the appropriate\n> action.\n>\n> Moreover, I've moved on from using git-cvsserver myself, having\n> instead convinced my Windows-using compatriots to use msysgit instead.\n> So if you feel that this change is unwarranted, feel free to just drop\n> it.\n\nBecause the issue currently has our attention, and we think we know that\nthe code does not do the right thing currently, and that we are fairly\nsure that the right thing is to do req_EMPTY, I'd rather see a tested fix\napplied so that we can forget about it ;-)\n\nIt's good that you moved your people to native git environment, but if you\nhave an environment where you can test the fix still lying around, I'd\nappreciate a quick test and resubmit.\n"},{"id":"102532","messageId":"1233277947-17175-1-git-send-email-stefan.karpinski@gmail.com","threadId":"17318","inReplyTo":"7vhc3hd6ba.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] git-cvsserver: handle CVS 'noop' command.","fromName":"Stefan Karpinski","fromEmail":"stefan.karpinski@gmail.com","sentAt":"2009-01-30T01:12:27Z","receivedAt":"2009-01-30T01:12:27Z","isPatch":true,"sender":{"key":"stefan.karpinski@gmail.com","avatar":"https://gravatar.com/avatar/780cfb8dd7d7dc749d7276a4ca2ec24e7f0482cfce509717c5ddd165fd2cc9d9?d=mp&s=160"},"body":"The CVS protocol documentation, found at\n\n  http://www.wandisco.com/techpubs/cvs-protocol.pdf\n\nstates the following about the 'noop' command:\n\n  Response expected: yes. This request is a null command\n  in the sense that it doesn't do anything, but merely\n  (as with any other requests expecting a response) sends\n  back any responses pertaining to pending errors, pending\n  Notified responses, etc.\n\nIn accordance with this, the correct way to handle the 'noop'\ncommand, when issued by a client, is to call req_EMPTY.\n\nThe 'noop' command is called by some CVS clients, notably\nTortoiseCVS, thus making it desirable for git-cvsserver to\nrespond to the command rather than choking on it as unknown.\n\nSigned-off-by: Stefan Karpinski <stefan.karpinski@gmail.com>\n---\nOn Thu, Jan 29, 2009 at 3:46 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Because the issue currently has our attention, and we think we know that\n> the code does not do the right thing currently, and that we are fairly\n> sure that the right thing is to do req_EMPTY, I'd rather see a tested fix\n> applied so that we can forget about it ;-)\n>\n> It's good that you moved your people to native git environment, but if you\n> have an environment where you can test the fix still lying around, I'd\n> appreciate a quick test and resubmit.\n\nI've done the best testing I could do under the circumstances. What\nthat means is that the only windows machine I have access to test\nthis on right now is running Vista, which is only partially (read\npoorly) supported by TortoiseCVS. So things seem to work well enough,\nbut TortoiseCVS keeps crapping out for Vista-related reasons rather \nthan git-cvsserver-related reasons. But I did manage to coax it into\nsuccessfully checking out a complete working repository without the\n\"noop\" errors that it used to give.\n\n git-cvsserver.perl |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex fef7faf..277ee4e 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -76,6 +76,7 @@ my $methods = {\n     'history'         => \\&req_CATCHALL,\n     'watchers'        => \\&req_EMPTY,\n     'editors'         => \\&req_EMPTY,\n+    'noop'            => \\&req_EMPTY,\n     'annotate'        => \\&req_annotate,\n     'Global_option'   => \\&req_Globaloption,\n     #'annotate'        => \\&req_CATCHALL,\n-- \n1.6.0.3.3.g08dd8\n"},{"id":"102535","messageId":"46a038f90901291732o56c19387w9debbdc5b2027904@mail.gmail.com","threadId":"17318","inReplyTo":"7v7i4denpg.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] git-cvsserver: handle CVS 'noop' command.","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2009-01-30T01:32:14Z","receivedAt":"2009-01-30T01:32:14Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On Fri, Jan 30, 2009 at 11:45 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Not a no-brainer at all, sorry.\n\n+1 on Junio's notes and patch.\n\nCan someone with a real TortoiseCVS and a real cvs server sniff the\nconnection and catch the noop? (Can TortoiseCVS write debug logs of\nthe conversation with the server?)\n\nHysterical note: the original implementation of cvsserver was done\nreading the output of `cvs -t $opts $cmd`, and ocassionally sniffing\nthe traffic on the wire or ssh connection.\n\nProbably not a major issue for 'noop' though :-)\n\ncheers,\n\n\nm\n-- \n martin.langhoff@gmail.com\n martin@laptop.org -- School Server Architect\n - ask interesting questions\n - don't get distracted with shiny stuff  - working code first\n - http://wiki.laptop.org/go/User:Martinlanghoff\n"}]}