{"thread":{"id":"6463","subject":"[PATCH] New files in git weren't being downloaded during CVS update","startedAt":"2007-01-21T14:25:12Z","lastAt":"2007-01-22T11:08:18Z","messageCount":8,"participants":["Andy Parkins","Junio C Hamano","Martin Langhoff","Simon 'corecode' Schubert"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"32208","messageId":"200701211425.12971.andyparkins@gmail.com","threadId":"6463","inReplyTo":null,"subject":"[PATCH] New files in git weren't being downloaded during CVS update","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-01-21T14:25:12Z","receivedAt":"2007-01-21T14:25:12Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"If a repository was checked out via git-cvsserver and then later a new\nfile is added to the git repository via some other method; a CVS update\nwasn't fetching the new file.\n\nIt would be reported as a new file as\n A some/dir/newfile.c\nbut would never appear in the directory.\n\nThe problem (I think) is that when git-cvsserver detected a new file, it\nwas issuing the new file message then skipping the actual file send part\nand moving to the next file its list.  In fact only an updated file\nwould be transmitted.\n\nThe fix is to make the added file section identical to the udpated file\nsection.  This additionally makes git-cvsserver behave like a\ntraditional CVS server and will now output\n U some/dir/newfile.c\nfor an added file.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\n---\nThis is in response to Simon Schubert's suggestion that T_ADDED is an\ninappropriate category for a remotely added file.  Instead this treats\nremotely added files the same as remotely changed files.\n\n git-cvsserver.perl |   10 +++++-----\n 1 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex a33a876..501c182 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -879,11 +879,11 @@ sub req_update\n \t    elsif ( !defined($wrev) || $wrev == 0 )\n \t    {\n \t        $log->info(\"Tell the client the file will be added\");\n-\t\tprint \"MT text A \\n\";\n-                print \"MT fname $filename\\n\";\n-                print \"MT newline\\n\";\n-\t\tnext;\n-\n+\t\t\tprint \"MT +updated\\n\";\n+\t\t\tprint \"MT text U \\n\";\n+\t\t\tprint \"MT fname $filename\\n\";\n+\t\t\tprint \"MT newline\\n\";\n+\t\t\tprint \"MT -updated\\n\";\n \t    }\n \t    else {\n                 $log->info(\"Updating '$filename' $wrev\");\n-- \n1.5.0.rc1.gf4b6c\n"},{"id":"32270","messageId":"7vmz4bwzy4.fsf@assigned-by-dhcp.cox.net","threadId":"6463","inReplyTo":"200701211425.12971.andyparkins@gmail.com","subject":"Re: [PATCH] New files in git weren't being downloaded during CVS update","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-22T02:35:47Z","receivedAt":"2007-01-22T02:35:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> If a repository was checked out via git-cvsserver and then later a new\n> file is added to the git repository via some other method; a CVS update\n> wasn't fetching the new file.\n>\n> It would be reported as a new file as\n>  A some/dir/newfile.c\n> but would never appear in the directory.\n>\n> The problem (I think) is that when git-cvsserver detected a new file, it\n> was issuing the new file message then skipping the actual file send part\n> and moving to the next file its list.  In fact only an updated file\n> would be transmitted.\n>\n> The fix is to make the added file section identical to the udpated file\n> section.  This additionally makes git-cvsserver behave like a\n> traditional CVS server and will now output\n>  U some/dir/newfile.c\n> for an added file.\n>\n> Signed-off-by: Andy Parkins <andyparkins@gmail.com>\n> ---\n> This is in response to Simon Schubert's suggestion that T_ADDED is an\n> inappropriate category for a remotely added file.  Instead this treats\n> remotely added files the same as remotely changed files.\n\nMartin, I think this looks like a sane change.  \n\nI do not have anything other than the real CVS running on a\nLinux box to try this change (most notably I do not do Eclipse\nnor Tortoise) myself and I am reluctant to touch things I cannot\npersonally test at this stage near the release.  I am sure Andy\ntested this for his own use in his environment, but I would\nreally appreciate a third party Ack from an environment\ndifferent from the originator of the patch.\n\n>  git-cvsserver.perl |   10 +++++-----\n>  1 files changed, 5 insertions(+), 5 deletions(-)\n>\n> diff --git a/git-cvsserver.perl b/git-cvsserver.perl\n> index a33a876..501c182 100755\n> --- a/git-cvsserver.perl\n> +++ b/git-cvsserver.perl\n> @@ -879,11 +879,11 @@ sub req_update\n>  \t    elsif ( !defined($wrev) || $wrev == 0 )\n>  \t    {\n>  \t        $log->info(\"Tell the client the file will be added\");\n> -\t\tprint \"MT text A \\n\";\n> -                print \"MT fname $filename\\n\";\n> -                print \"MT newline\\n\";\n> -\t\tnext;\n> -\n> +\t\t\tprint \"MT +updated\\n\";\n> +\t\t\tprint \"MT text U \\n\";\n> +\t\t\tprint \"MT fname $filename\\n\";\n> +\t\t\tprint \"MT newline\\n\";\n> +\t\t\tprint \"MT -updated\\n\";\n>  \t    }\n>  \t    else {\n>                  $log->info(\"Updating '$filename' $wrev\");\n> -- \n> 1.5.0.rc1.gf4b6c\n"},{"id":"32282","messageId":"46a038f90701212302v458019fcxcde72a34aa46e7bd@mail.gmail.com","threadId":"6463","inReplyTo":"7vmz4bwzy4.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] New files in git weren't being downloaded during CVS update","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2007-01-22T07:02:29Z","receivedAt":"2007-01-22T07:02:29Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 1/22/07, Junio C Hamano <junkio@cox.net> wrote:\n\n> > This is in response to Simon Schubert's suggestion that T_ADDED is an\n> > inappropriate category for a remotely added file.  Instead this treats\n> > remotely added files the same as remotely changed files.\n>\n> Martin, I think this looks like a sane change.\n\nIt does -- sorry I've been offline for a few days and catching up with work.\n\n> I do not have anything other than the real CVS running on a\n> Linux box to try this change (most notably I do not do Eclipse\n> nor Tortoise) myself and I am reluctant to touch things I cannot\n> personally test at this stage near the release.\n\nFair enough. I'll give it whirl later tonight. For a quick test, you can always\n\nexport CVS_SERVER=/home/martin/src/git/git-cvsserver.perl\ncvs update\n\n> tested this for his own use in his environment, but I would\n> really appreciate a third party Ack from an environment\n> different from the originator of the patch.\n\nYup. Will test later tonight.\n\n\nmartin\n"},{"id":"32284","messageId":"7v8xfvv8ee.fsf@assigned-by-dhcp.cox.net","threadId":"6463","inReplyTo":"46a038f90701212302v458019fcxcde72a34aa46e7bd@mail.gmail.com","subject":"Re: [PATCH] New files in git weren't being downloaded during CVS update","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-01-22T07:16:09Z","receivedAt":"2007-01-22T07:16:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Martin Langhoff\" <martin.langhoff@gmail.com> writes:\n\n>> I do not have anything other than the real CVS running on a\n>> Linux box to try this change (most notably I do not do Eclipse\n>> nor Tortoise) myself and I am reluctant to touch things I cannot\n>> personally test at this stage near the release.\n>\n> Fair enough. I'll give it whirl later tonight. For a quick test, you can always\n>\n> export CVS_SERVER=/home/martin/src/git/git-cvsserver.perl\n> cvs update\n\nYes, that's what I did (what I meant was I can test with cvs as\nclient but not other clients that talk CVS remote protocols).\n\n>> tested this for his own use in his environment, but I would\n>> really appreciate a third party Ack from an environment\n>> different from the originator of the patch.\n>\n> Yup. Will test later tonight.\n\nThanks.\n"},{"id":"32286","messageId":"200701221044.49467.andyparkins@gmail.com","threadId":"6463","inReplyTo":"200701211425.12971.andyparkins@gmail.com","subject":"Re: [PATCH] New files in git weren't being downloaded during CVS update","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-01-22T10:44:47Z","receivedAt":"2007-01-22T10:44:47Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"On Sunday 2007 January 21 14:25, Andy Parkins wrote:\n\n> This is in response to Simon Schubert's suggestion that T_ADDED is an\n> inappropriate category for a remotely added file.  Instead this treats\n> remotely added files the same as remotely changed files.\n\nI'm still concerned that this fix is not right.  Having thought more about it, \nI suspect that this breaks the other \"added-file\" case, where the file is \nadded locally but not remotely.\n\nThere are three key cases:\n 1. File present locally and remotely.  Modified remotely.  Response \"U\"\n 2. File present locally only.  Response should be \"A\"\n 3. File present remotely only.  Response should be \"U\"\n\nI think the real problem is that both 2 and 3 are being handled in the same \nplace.  Hence, my patch, which has fixed case 3; will have broken case 2.\n\nI need a bit of confirmation from Martin; but I suspect that the correct fix \nis something like this:\n\n    elsif ( (!defined($wrev) || $wrev == 0) && !defined($meta->{revision})\n    {\n        $log->info(\"Tell the client the file will be added\");\n\nIf I'm correct, this would only run the added section when there is no \nmatching revision in the repository - this would be case 2.  Then case 3 \nwould be handled as the else to this if, which handles every other case.\n\nMy testing of this works, but I'd like confirmation that I'm right in this \nthinking?  Patch to follow...\n\n\nAndy\n-- \nDr Andy Parkins, M Eng (hons), MIEE\nandyparkins@gmail.com\n"},{"id":"32287","messageId":"200701221046.12795.andyparkins@gmail.com","threadId":"6463","inReplyTo":"200701221044.49467.andyparkins@gmail.com","subject":"[PATCH] New files in git weren't being downloaded during CVS update","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-01-22T10:46:12Z","receivedAt":"2007-01-22T10:46:12Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"If a repository was checked out via git-cvsserver and then later a new\nfile is added to the git repository via some other method; a CVS update\nwasn't fetching the new file.\n\nIt would be reported as a new file as\n A some/dir/newfile.c\nbut would never appear in the directory.\n\nThe problem seems to be that git-cvsserver was treating these two cases\nidentically, as \"A\" type results.\n\n1. New file in repository\n2. New file locally\n\nIn fact, traditionally, case 1 is treated as a \"U\" result, and case 2\nonly is treated as an \"A\" result.  \"A\", should just report that the file\nis added locally and then skip that file during an update as their is\n(of course) nothing to send.\n\nIn both these cases there is no working revision, so the checking for\n\"is there no working revision\" will return true.  The test for case 2\nneeds refining to say \"if there is no working revision and no upstream\nrevision\".  This patch does just that, leaving case 1 to be handled by\nthe normal \"U\" handler.\n\nI've also updated the log message to more accurately describe the\noperation.  i.e. that \"A\" means that content is scheduled for addition;\nnot that it actually has been added.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\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 a33a876..2479f71 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -876,9 +876,9 @@ sub req_update\n                 print \"MT newline\\n\";\n \t\tnext;\n \t    }\n-\t    elsif ( !defined($wrev) || $wrev == 0 )\n+\t    elsif ( (!defined($wrev) || $wrev == 0) && (!defined($meta->{revision}) || $meta->{revision} == 0) )\n \t    {\n-\t        $log->info(\"Tell the client the file will be added\");\n+\t        $log->info(\"Tell the client the file is scheduled for addition\");\n \t\tprint \"MT text A \\n\";\n                 print \"MT fname $filename\\n\";\n                 print \"MT newline\\n\";\n-- \n1.5.0.rc2.g3f8f2\n"},{"id":"32288","messageId":"200701221056.27599.andyparkins@gmail.com","threadId":"6463","inReplyTo":"200701221044.49467.andyparkins@gmail.com","subject":"[PATCH] New files in git weren't being downloaded during CVS update","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-01-22T10:56:27Z","receivedAt":"2007-01-22T10:56:27Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"If a repository was checked out via git-cvsserver and then later a new\nfile is added to the git repository via some other method; a CVS update\nwasn't fetching the new file.\n\nIt would be reported as a new file as\n A some/dir/newfile.c\nbut would never appear in the directory.\n\nThe problem seems to be that git-cvsserver was treating these two cases\nidentically, as \"A\" type results.\n\n1. New file in repository\n2. New file locally\n\nIn fact, traditionally, case 1 is treated as a \"U\" result, and case 2\nonly is treated as an \"A\" result.  \"A\", should just report that the file\nis added locally and then skip that file during an update as there is\n(of course) nothing to send.\n\nIn both these cases there is no working revision, so the checking for\n\"is there no working revision\" will return true.  The test for case 2\nneeds refining to say \"if there is no working revision and no upstream\nrevision\".  This patch does just that, leaving case 1 to be handled by\nthe normal \"U\" handler.\n\nI've also updated the log message to more accurately describe the\noperation.  i.e. that \"A\" means that content is scheduled for addition;\nnot that it actually has been added.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\n---\n\nApologies; this one is the real replacement.  The previous patch didn't\ninclude the correction to the update log message which would potentially\nuse an undefined variable when adding new content.\n\n git-cvsserver.perl |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-cvsserver.perl b/git-cvsserver.perl\nindex a33a876..e18e901 100755\n--- a/git-cvsserver.perl\n+++ b/git-cvsserver.perl\n@@ -876,9 +876,9 @@ sub req_update\n                 print \"MT newline\\n\";\n \t\tnext;\n \t    }\n-\t    elsif ( !defined($wrev) || $wrev == 0 )\n+\t    elsif ( (!defined($wrev) || $wrev == 0) && (!defined($meta->{revision}) || $meta->{revision} == 0) )\n \t    {\n-\t        $log->info(\"Tell the client the file will be added\");\n+\t        $log->info(\"Tell the client the file is scheduled for addition\");\n \t\tprint \"MT text A \\n\";\n                 print \"MT fname $filename\\n\";\n                 print \"MT newline\\n\";\n@@ -886,7 +886,7 @@ sub req_update\n \n \t    }\n \t    else {\n-                $log->info(\"Updating '$filename' $wrev\");\n+                $log->info(\"Updating '$filename' to \".$meta->{revision});\n                 print \"MT +updated\\n\";\n                 print \"MT text U \\n\";\n                 print \"MT fname $filename\\n\";\n-- \n1.5.0.rc2.g3f8f2\n"},{"id":"32289","messageId":"45B49B22.6050806@fs.ei.tum.de","threadId":"6463","inReplyTo":"200701221056.27599.andyparkins@gmail.com","subject":"Re: [PATCH] New files in git weren't being downloaded during CVS update","fromName":"Simon 'corecode' Schubert","fromEmail":"corecode@fs.ei.tum.de","sentAt":"2007-01-22T11:08:18Z","receivedAt":"2007-01-22T11:08:18Z","isPatch":true,"sender":{"key":"corecode@fs.ei.tum.de","avatar":"https://gravatar.com/avatar/eff9dbf0cdac0d1e6a6cd7ed0e50763edcb376b493b5253a35ff167918ad79e1?d=mp&s=160"},"body":"Andy Parkins wrote:\n> @@ -876,9 +876,9 @@ sub req_update\n>                  print \"MT newline\\n\";\n>  \t\tnext;\n>  \t    }\n> -\t    elsif ( !defined($wrev) || $wrev == 0 )\n> +\t    elsif ( (!defined($wrev) || $wrev == 0) && (!defined($meta->{revision}) || $meta->{revision} == 0) )\n>  \t    {\n> -\t        $log->info(\"Tell the client the file will be added\");\n> +\t        $log->info(\"Tell the client the file is scheduled for addition\");\n\nThis is nice but probably does not handle a collision (cvs output: file was added by a third party) well and will probably overwrite the locally added file.  however it is a step in the right direction.  from skimming the code I couldn't find collision handling anyways.\n\ncheers\n  simon\n\n-- \nServe - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /\"\\\nWork - Mac      +++  space for low €€€ NOW!1  +++      Campaign     \\ /\nParty Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \\\nDude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \\\n\n"}]}