{"thread":{"id":"26106","subject":"cvsimport still not working with cvsnt","startedAt":"2010-12-20T04:05:00Z","lastAt":"2011-05-01T18:44:00Z","messageCount":32,"participants":["Guy Rouillier","Jonathan Nieder","Emil Medve","Martin Langhoff","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"158381","messageId":"4D0ED5EC.9020402@burntmail.com","threadId":"26106","inReplyTo":null,"subject":"cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2010-12-20T04:05:00Z","receivedAt":"2010-12-20T04:05:00Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"I'm going to try sending this blind, as the mailing list has sent me the \npromised authorization key after 24 hrs.\n\nI'm brand new to git. We'll be moving over from CVS, so I imported a \nsmall part of our CVS repository to start learning git. We use the \nCVSNT server, and git-cvsimport was failing with \"I HATE YOU\". I \nfinally found the problems, both of which were reported in 2008 here:\n\nhttp://kerneltrap.org/mailarchive/git/2008/3/13/1157364\n\nHowever, these changes do not appear in the version 1.7.2.2 that Gentoo \nsupplies. I checked the 1.7.3-rc2 source and the changes are not in \nthere either.\n\nI do see one possible issue with the supplied modifications. At work, \nwe upgraded from CVS to CVSNT. So, my home directory has both .cvspass \n(from the original CVS) and .cvs/cvspass (after the conversion to \nCVSNT.) Sloppy housekeeping on my part, I admit, but probably not \nuncommon. The supplied patch would pick up the original CVS file and \nwould fail. (BTW, this is true only of the git-cvsimport.perl script \nitself; cvsps must shell out to the installed CVS client (in my case, \ncvsnt), because when I invoked that manually, it worked.)\n\nSo, I would advise checking to see if both files exist, and if so exit \nwith an error. Unless cvsimport wants to get real fancy and shell out \nto the installed cvs client to try to figure out what is installed, \nthere is no way to tell which cvspass file is actively being used. I \ndon't recommend trying to figure this out, as the user's intent is unclear.\n\n-- \nGuy Rouillier\n"},{"id":"158415","messageId":"20101220213654.GA24628@burratino","threadId":"26106","inReplyTo":"4D0ED5EC.9020402@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-12-20T21:36:54Z","receivedAt":"2010-12-20T21:36:54Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"(+cc: Emil, some cvsimport people)\n\nGuy Rouillier wrote:\n\n> I'm going to try sending this blind, as the mailing list has sent me\n> the promised authorization key after 24 hrs.\n\nNo problem. Actually a subscription is not required --- the\nconvention on this list is to always reply-to-all.\n\n> I finally found the problems, both of which were reported in 2008\n> here:\n>\n> http://kerneltrap.org/mailarchive/git/2008/3/13/1157364\n\nSeems to have received no replies[1].\n\n> I do see one possible issue with the supplied modifications. At\n> work, we upgraded from CVS to CVSNT. So, my home directory has both\n> .cvspass (from the original CVS) and .cvs/cvspass (after the\n> conversion to CVSNT.) Sloppy housekeeping on my part, I admit, but\n> probably not uncommon. The supplied patch would pick up the\n> original CVS file and would fail. (BTW, this is true only of the\n> git-cvsimport.perl script itself; cvsps must shell out to the\n> installed CVS client (in my case, cvsnt), because when I invoked\n> that manually, it worked.)\n> \n> So, I would advise checking to see if both files exist, and if so\n> exit with an error. Unless cvsimport wants to get real fancy and\n> shell out to the installed cvs client to try to figure out what is\n> installed, there is no way to tell which cvspass file is actively\n> being used. I don't recommend trying to figure this out, as the\n> user's intent is unclear.\n\nThanks, sounds sane to me. Care to write a patch?\n\nRegards,\nJonathan\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/77109\n"},{"id":"158463","messageId":"4D112586.2060904@Freescale.com","threadId":"26106","inReplyTo":"20101220213654.GA24628@burratino","subject":"Re: cvsimport still not working with cvsnt","fromName":"Emil Medve","fromEmail":"emilian.medve@freescale.com","sentAt":"2010-12-21T22:09:10Z","receivedAt":"2010-12-21T22:09:10Z","isPatch":false,"sender":{"key":"emilian.medve@freescale.com","avatar":null},"body":"Hello Guy,\n\n\nOn 12/20/10 15:36, Jonathan Nieder wrote:\n> (+cc: Emil, some cvsimport people)\n> \n> Guy Rouillier wrote:\n\nSometimes, on some particularly nasty CVS repos, I noticed better\nresults when using http://cvs2svn.tigris.org\n\n>> I'm going to try sending this blind, as the mailing list has sent me\n>> the promised authorization key after 24 hrs.\n> \n> No problem. Actually a subscription is not required --- the\n> convention on this list is to always reply-to-all.\n> \n>> I finally found the problems, both of which were reported in 2008\n>> here:\n>>\n>> http://kerneltrap.org/mailarchive/git/2008/3/13/1157364\n> \n> Seems to have received no replies[1].\n\nI don't remember why, but that patch didn't get enough interest\n\n>> I do see one possible issue with the supplied modifications. At\n>> work, we upgraded from CVS to CVSNT. So, my home directory has both\n>> .cvspass (from the original CVS) and .cvs/cvspass (after the\n>> conversion to CVSNT.) Sloppy housekeeping on my part, I admit, but\n>> probably not uncommon. The supplied patch would pick up the\n>> original CVS file and would fail. (BTW, this is true only of the\n>> git-cvsimport.perl script itself; cvsps must shell out to the\n>> installed CVS client (in my case, cvsnt), because when I invoked\n>> that manually, it worked.)\n>>\n>> So, I would advise checking to see if both files exist, and if so\n>> exit with an error. Unless cvsimport wants to get real fancy and\n>> shell out to the installed cvs client to try to figure out what is\n>> installed, there is no way to tell which cvspass file is actively\n>> being used. I don't recommend trying to figure this out, as the\n>> user's intent is unclear.\n> \n> Thanks, sounds sane to me. Care to write a patch?\n\nIf you care enough about this scenario, how about search for the\nrelevant in both files. If you find just one pair or\nif you find a pair in both files and they are \"equal\" then just use it.\nIf you find two pairs, one in each file, use the one from the file with\na newer modified time-stamp. In a migration scenario such as this, you'd\nimaging the \"old\" file will get stale after a while. Not perfect, but\nsome informational messages in case of a duplicate would help the user\nclarify their intentions\n\nAdditionally/Alternatively just add a command line parameter to allow\nthe user to explicitly specify a cvspass file\n\n\nCheers,\nEmil.\n"},{"id":"158473","messageId":"4D119015.6020207@burntmail.com","threadId":"26106","inReplyTo":"4D112586.2060904@Freescale.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2010-12-22T05:43:49Z","receivedAt":"2010-12-22T05:43:49Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 12/21/2010 5:09 PM, Emil Medve wrote:\n> Hello Guy,\n>\n>\n> On 12/20/10 15:36, Jonathan Nieder wrote:\n>> (+cc: Emil, some cvsimport people)\n>>\n>> Guy Rouillier wrote:\n>\n> Sometimes, on some particularly nasty CVS repos, I noticed better\n> results when using http://cvs2svn.tigris.org\n>\n>>> I'm going to try sending this blind, as the mailing list has sent me\n>>> the promised authorization key after 24 hrs.\n>>\n>> No problem. Actually a subscription is not required --- the\n>> convention on this list is to always reply-to-all.\n>>\n>>> I finally found the problems, both of which were reported in 2008\n>>> here:\n>>>\n>>> http://kerneltrap.org/mailarchive/git/2008/3/13/1157364\n>>\n>> Seems to have received no replies[1].\n>\n> I don't remember why, but that patch didn't get enough interest\n>\n>>> I do see one possible issue with the supplied modifications. At\n>>> work, we upgraded from CVS to CVSNT. So, my home directory has both\n>>> .cvspass (from the original CVS) and .cvs/cvspass (after the\n>>> conversion to CVSNT.) Sloppy housekeeping on my part, I admit, but\n>>> probably not uncommon. The supplied patch would pick up the\n>>> original CVS file and would fail. (BTW, this is true only of the\n>>> git-cvsimport.perl script itself; cvsps must shell out to the\n>>> installed CVS client (in my case, cvsnt), because when I invoked\n>>> that manually, it worked.)\n>>>\n>>> So, I would advise checking to see if both files exist, and if so\n>>> exit with an error. Unless cvsimport wants to get real fancy and\n>>> shell out to the installed cvs client to try to figure out what is\n>>> installed, there is no way to tell which cvspass file is actively\n>>> being used. I don't recommend trying to figure this out, as the\n>>> user's intent is unclear.\n>>\n>> Thanks, sounds sane to me. Care to write a patch?\n>\n> If you care enough about this scenario, how about search for the\n> relevant in both files. If you find just one pair or\n> if you find a pair in both files and they are \"equal\" then just use it.\n> If you find two pairs, one in each file, use the one from the file with\n> a newer modified time-stamp. In a migration scenario such as this, you'd\n> imaging the \"old\" file will get stale after a while. Not perfect, but\n> some informational messages in case of a duplicate would help the user\n> clarify their intentions\n>\n> Additionally/Alternatively just add a command line parameter to allow\n> the user to explicitly specify a cvspass file\n\nEmil and Jonathan, thanks for the feedback. Perl is not my strong \npoint, but I'll take a crack at it over the upcoming holidays. I'm \ninclined not to get too fancy and try to second-guess the user's \nenvironment. Perhaps he has both cvs and cvsnt installed for some \nreason (testing one, using the other for regular work); perhaps a tool \ninstalled one or the other and he doesn't even know he has them both. Etc.\n\nSo, at most I can see, as Emil suggested, seeing if the entry exists in \nboth files and is the same in both. If so, or if the entry is only in \none of them, then just use the entry. However, if the entry is in both \nfiles and is different, I'd prefer to just exit with an error and have \nthe user clarify his environment.\n\n-- \nGuy Rouillier\n"},{"id":"159259","messageId":"4D2AB63D.7040803@burntmail.com","threadId":"26106","inReplyTo":"4D119015.6020207@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-01-10T07:33:17Z","receivedAt":"2011-01-10T07:33:17Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 12/22/2010 12:43 AM, Guy Rouillier wrote:\n>\n> Emil and Jonathan, thanks for the feedback. Perl is not my strong point,\n> but I'll take a crack at it over the upcoming holidays. I'm inclined not\n> to get too fancy and try to second-guess the user's environment. Perhaps\n> he has both cvs and cvsnt installed for some reason (testing one, using\n> the other for regular work); perhaps a tool installed one or the other\n> and he doesn't even know he has them both. Etc.\n>\n> So, at most I can see, as Emil suggested, seeing if the entry exists in\n> both files and is the same in both. If so, or if the entry is only in\n> one of them, then just use the entry. However, if the entry is in both\n> files and is different, I'd prefer to just exit with an error and have\n> the user clarify his environment.\n\nHere is my patch for accomplishing the above. As this is my first time\nsubmitting a patch, please let me know the correct procedure if\nsubmitting a diff here is not appropriate. Thanks.\n\n--- git-cvsimport.org\t2011-01-09 03:52:39.000000000 -0500\n+++ git-cvsimport.cvsnt\t2011-01-10 01:42:29.000000000 -0500\n@@ -260,6 +260,8 @@\n \t\tif ($pass) {\n \t\t\t$pass = $self->_scramble($pass);\n \t\t} else {\n+\t\t\t# First try the original CVS location.\n+\n \t\t\topen(H,$ENV{'HOME'}.\"/.cvspass\") and do {\n \t\t\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n@@ -272,7 +274,30 @@\n \t\t\t\t\t}\n \t\t\t\t}\n \t\t\t};\n-\t\t\t$pass = \"A\" unless $pass;\n+\n+\t\t\t# Now try the CVSNT location.\n+\n+\t\t\topen(H,$ENV{'HOME'}.\"/.cvs/cvspass\") and do {\n+\t\t\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n+\t\t\t\t\tchomp;\n+\t\t\t\t\ts/^\\/\\d+\\s+//;\n+\t\t\t\t\tmy ($w,$p) = split(/=/,$_,2);\n+\t\t\t\t\tif ($w eq $rr or $w eq $rr2) {\n+\t\t\t\t\t\tmy $cvsntpass = $p;\n+\n+\t\t\t\t\t\tif (!$pass) {\n+\t\t\t\t\t\t\t$pass = $cvsntpass;\n+\t\t\t\t\t\t} elsif ($pass ne $cvsntpass) {\n+\t\t\t\t\t\t\tdie(\"CVSROOT found in both CVS and CVSNT cvspass files, passwords do not match\\n\");\n+\t\t\t\t\t\t}\n+\t\t\t\t\t\tlast;\n+\t\t\t\t\t}\n+\t\t\t\t}\n+\t\t\t};\n+\n+\n+\t\t\tdie(\"Password not found for CVSROOT: $opt_d\\n\") unless $pass;\n \t\t}\n\n \t\tmy ($s, $rep);\n\n\n\n-- \nGuy Rouillier\n"},{"id":"159269","messageId":"AANLkTikreDJmUPfwNJ2ABivrafjvQNN6WrytNMAcse4A@mail.gmail.com","threadId":"26106","inReplyTo":"4D2AB63D.7040803@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Martin Langhoff","fromEmail":"martin@laptop.org","sentAt":"2011-01-10T15:38:38Z","receivedAt":"2011-01-10T15:38:38Z","isPatch":false,"sender":{"key":"martin@laptop.org","avatar":null},"body":"On Mon, Jan 10, 2011 at 2:33 AM, Guy Rouillier wrote:\n> Here is my patch for accomplishing the above.  As this is my first time\n> submitting a patch, please let me know the correct procedure if\n> submitting a diff here is not appropriate.  Thanks.\n\nThe concept of what the patch is doing is good, but I'd recommend\n\n@cvspasslocations = ($ENV{'HOME'}.\"/cvspass\", $ENV{'HOME'}.\"/.cvs/cvspass\")\n\nforeach $cvspass (@cvspasslocations) {\n open(...\n\nand forgo the \"matching\" test.\n\ncheers,\n\n\nm\n-- \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"},{"id":"159460","messageId":"4D2FEF49.8070205@burntmail.com","threadId":"26106","inReplyTo":"AANLkTikreDJmUPfwNJ2ABivrafjvQNN6WrytNMAcse4A@mail.gmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-01-14T06:38:01Z","receivedAt":"2011-01-14T06:38:01Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 1/10/2011 10:38 AM, Martin Langhoff wrote:\n> On Mon, Jan 10, 2011 at 2:33 AM, Guy Rouillier wrote:\n>> Here is my patch for accomplishing the above. As this is my first time\n>> submitting a patch, please let me know the correct procedure if\n>> submitting a diff here is not appropriate. Thanks.\n>\n> The concept of what the patch is doing is good, but I'd recommend\n>\n> @cvspasslocations = ($ENV{'HOME'}.\"/cvspass\", $ENV{'HOME'}.\"/.cvs/cvspass\")\n>\n> foreach $cvspass (@cvspasslocations) {\n> open(...\n>\n> and forgo the \"matching\" test.\n>\n\nMartin, thanks for the reply. Have you had a chance to read the entire \nthread? The matching test was suggested by Emil.\n\nThis is my first patch submission. What is the process for reaching \nconsensus?\n\n-- \nGuy Rouillier\n"},{"id":"159462","messageId":"20110114074449.GA11175@burratino","threadId":"26106","inReplyTo":"4D2FEF49.8070205@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-01-14T07:44:49Z","receivedAt":"2011-01-14T07:44:49Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Guy Rouillier wrote:\n\n> Martin, thanks for the reply. Have you had a chance to read the\n> entire thread? The matching test was suggested by Emil.\n\nTo summarize, Emil originally (2008)[1] suggested only checking\n~/.cvs/cvspass when ~/.cvspass fails to open. There was no response\nat the time, perhaps because nobody interested saw the message.\n\nGuy, two years later[2], wrote:\n\n| I do see one possible issue with the supplied modifications. At work, \n| we upgraded from CVS to CVSNT. So, my home directory has both\n| .cvspass (from the original CVS) and .cvs/cvspass (after the conversion to \n| CVSNT.) Sloppy housekeeping on my part, I admit, but probably not \n| uncommon. The supplied patch would pick up the original CVS file and \n| would fail. (BTW, this is true only of the git-cvsimport.perl script \n\nand recommended erroring out if both files exist to make this easier\nto diagnose.\n\nEmil's advice: if this is an important use case to you, maybe it would\nbe served better by looking at both files?\n\n> This is my first patch submission. What is the process for reaching\n> consensus?\n\nSee Documentation/SubmittingPatches, \"An ideal patch flow\".\n\nMy take: you learn what you can from others' advice, but ultimately\nthe idea is to just make those changes that make the patch better\n(where better can mean featureful or simpler and more maintainable ---\nthis is not meant to be an excuse for overengineering). In most cases\napparent conflicts are not real conflicts at all but signs of distinct\ndesign goals to be balanced or reconciled.\n\nHope that helps,\nJonathan\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/77109\n[2] http://thread.gmane.org/gmane.comp.version-control.git/163979\n"},{"id":"159510","messageId":"7v8vynnokt.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"20110114074449.GA11175@burratino","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-01-14T21:49:06Z","receivedAt":"2011-01-14T21:49:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> and recommended erroring out if both files exist to make this easier\n> to diagnose.\n>\n> Emil's advice: if this is an important use case to you, maybe it would\n> be served better by looking at both files?\n\nThanks for summarizing two-year's worth of discussion ;-)\n\nTrying both, one after another, in the order that likely favors newer one\nover the older one, is a very valid option but is appropriate only under a\nvery narrow condition. Picking a wrong one must reliably, silently _and_\nquickly fail, and fail without any side effect. The one in .cvspass may\nidentify you as a different user from the user .cvs/cvspass identifies you\nas, and the two users may have different capabilities or default server\nside preferences--in such a case, both may succeed, but in a different and\nunexpected way [*1*].\n\nAs the general principle, in a \"we see two, and we cannot tell which one\nthe user wants to use\" situation like this, I tend to prefer erroring out\nto _force_ the user to fix the configuration once and for all.\n\nUnless the \"try both\" approach is reasonable, we could implement \"we read\nfrom one and when we find one we stop, otherwise we read from the other\"\nand document the order, but it is probably less friendly than the above\ntwo options.\n\n\n[Footnote]\n\n*1* I know .cvspass is a bad example for this, as it records the password\nfor the \ntuple. Once you get in, what you can do is exactly the same because you\nare authenticating as the same user, but think of a case like .ssh/config\nthat is indexed by \"Host\" and allows you to use \"User\" to \n"},{"id":"160044","messageId":"4D450655.5090501@burntmail.com","threadId":"26106","inReplyTo":"7v8vynnokt.fsf@alter.siamese.dyndns.org","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-01-30T06:33:57Z","receivedAt":"2011-01-30T06:33:57Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"Sorry for the delay in following up, work has been busy.\n\nOn 1/14/2011 4:49 PM, Junio C Hamano wrote:\n> As the general principle, in a \"we see two, and we cannot tell which one\n> the user wants to use\" situation like this, I tend to prefer erroring out\n> to _force_ the user to fix the configuration once and for all.\n\nThat was my original inclination. As no other opinions have been posted \nsince your message, here is my amended patch, incorporating Martin's \nideas and dieing if the script finds both CVS and CVSNT password files.\nI don't know why diff got tripped up on brace indentation; I used only\ntabs and everything looks fine in vi.\n\n--- git-cvsimport.org 2011-01-09 03:52:39.000000000 -0500\n+++ git-cvsimport.perl 2011-01-30 00:59:29.000000000 -0500\n@@ -260,19 +260,27 @@\n if ($pass) {\n $pass = $self->_scramble($pass);\n } else {\n- open(H,$ENV{'HOME'}.\"/.cvspass\") and do {\n+ my @cvspasslocations = ($ENV{'HOME'}.\"/.cvspass\", $ENV{'HOME'}.\"/.cvs/cvspass\");\n+ my $filecount = 0;\n+ foreach my $cvspass (@cvspasslocations) {\n+\n+ open(H, $cvspass) and do {\n # :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n chomp;\n s/^\\/\\d+\\s+//;\n- my ($w,$p) = split(/\\s/,$_,2);\n+ my ($w,$p) = split(/[\\s=]/,$_,2);\n if ($w eq $rr or $w eq $rr2) {\n $pass = $p;\n last;\n }\n }\n };\n- $pass = \"A\" unless $pass;\n+ }\n+ \n+ die(\"Two CVS password files found: @cvspasslocations, please remove one\") if $filecount > 1;\n+ die(\"Password not found for CVSROOT: $opt_d\\n\") unless $pass;\n }\n\n my ($s, $rep);\n\n\n-- \nGuy Rouillier\n"},{"id":"160063","messageId":"AANLkTik0Mp=Ww_+ZN_jw6t4gsFwLo1UTw5JOpho8bCd=@mail.gmail.com","threadId":"26106","inReplyTo":"4D450655.5090501@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Martin Langhoff","fromEmail":"martin@laptop.org","sentAt":"2011-01-30T20:19:00Z","receivedAt":"2011-01-30T20:19:00Z","isPatch":false,"sender":{"key":"martin@laptop.org","avatar":null},"body":"On Sat, Jan 29, 2011 at 11:33 PM, Guy Rouillier wrote:\n> That was my original inclination.  As no other opinions have been posted\n> since your message, here is my amended patch, incorporating Martin's\n> ideas and dieing if the script finds both CVS and CVSNT password files.\n\nACK! Thanks!\n\n\nm\n-- \n martin@laptop.org -- Software Architect - OLPC\n - ask interesting questions\n - don't get distracted with shiny stuff  - working code first\n - http://wiki.laptop.org/go/User:Martinlanghoff\n"},{"id":"160841","messageId":"7vhbcb35xk.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"AANLkTik0Mp=Ww_+ZN_jw6t4gsFwLo1UTw5JOpho8bCd=@mail.gmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-10T22:01:27Z","receivedAt":"2011-02-10T22:01:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n> On Sat, Jan 29, 2011 at 11:33 PM, Guy Rouillier wrote:\n>> That was my original inclination.  As no other opinions have been posted\n>> since your message, here is my amended patch, incorporating Martin's\n>> ideas and dieing if the script finds both CVS and CVSNT password files.\n>\n> ACK! Thanks!\n\nCan somebody resubmit an appliable patch with a proper commit message that\ndescribes the problem and the solution please.\n\nThanks.\n"},{"id":"161513","messageId":"4D5E1116.7040501@burntmail.com","threadId":"26106","inReplyTo":"7vhbcb35xk.fsf@alter.siamese.dyndns.org","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-02-18T06:26:30Z","receivedAt":"2011-02-18T06:26:30Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 2/10/2011 5:01 PM, Junio C Hamano wrote:\n> Martin Langhoff writes:\n> \n>> On Sat, Jan 29, 2011 at 11:33 PM, Guy Rouillier wrote:\n>>> That was my original inclination. As no other opinions have been posted\n>>> since your message, here is my amended patch, incorporating Martin's\n>>> ideas and dieing if the script finds both CVS and CVSNT password files.\n>>\n>> ACK! Thanks!\n> \n> Can somebody resubmit an appliable patch with a proper commit message that\n> describes the problem and the solution please.\n> \n> Thanks.\n\nJunio, sorry for the delay in response. I'm new to all this and I thought \nperhaps one of the listed committers had to submit the official patch. \nPerhaps they do. I followed the directions in SubmittingPatches. The result \nis below. Please let me know if I need to do something differently. I \ndidn't simply send the result of format-match because that would have started \na different message thread. Thanks.\n\n>From d3ae7d304ee2b89740225b0433bf7d7e07248f59 Mon Sep 17 00:00:00 2001\nFrom: Guy Rouillier \nDate: Fri, 18 Feb 2011 00:53:10 -0500\nSubject: [PATCH] Look for password in both CVS and CVSNT password files.\n\nThe existing code looks for the CVS reposity password only in\nthe CVS password file in HOME/.cvspass. Accommodate the CVS\nalternative CVSNT by also looking in HOME/.cvs/cvspass. Die\nif both files are found, and ask the user to remove one.\n\nSigned-off-by: Guy Rouillier \n---\n git-cvsimport.perl | 32 ++++++++++++++++++++------------\n 1 files changed, 20 insertions(+), 12 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 8e683e5..76b4765 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -259,19 +259,27 @@ sub conn {\n \t\tif ($pass) {\n \t\t\t$pass = $self->_scramble($pass);\n \t\t} else {\n-\t\t\topen(H,$ENV{'HOME'}.\"/.cvspass\") and do {\n-\t\t\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n-\t\t\t\t\tchomp;\n-\t\t\t\t\ts/^\\/\\d+\\s+//;\n-\t\t\t\t\tmy ($w,$p) = split(/\\s/,$_,2);\n-\t\t\t\t\tif ($w eq $rr or $w eq $rr2) {\n-\t\t\t\t\t\t$pass = $p;\n-\t\t\t\t\t\tlast;\n+\t\t\tmy @cvspasslocations = ($ENV{'HOME'}.\"/.cvspass\", $ENV{'HOME'}.\"/.cvs/cvspass\");\n+\t\t\tmy $filecount = 0;\n+\t\t\tforeach my $cvspass (@cvspasslocations) {\n+\n+\t\t\t\topen(H, $cvspass) and do {\n+\t\t\t\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n+\t\t\t\t\t\tchomp;\n+\t\t\t\t\t\ts/^\\/\\d+\\s+//;\n+\t\t\t\t\t\tmy ($w,$p) = split(/[\\s=]/,$_,2);\n+\t\t\t\t\t\tif ($w eq $rr or $w eq $rr2) {\n+\t\t\t\t\t\t\t$pass = $p;\n+\t\t\t\t\t\t\tlast;\n+\t\t\t\t\t\t}\n \t\t\t\t\t}\n-\t\t\t\t}\n-\t\t\t};\n-\t\t\t$pass = \"A\" unless $pass;\n+\t\t\t\t};\n+\t\t\t}\n+\n+\t\t\tdie(\"Two CVS password files found: @cvspasslocations, please remove one\") if $filecount > 1;\n+\t\t\tdie(\"Password not found for CVSROOT: $opt_d\\n\") unless $pass;\n \t\t}\n\n \t\tmy ($s, $rep);\n--\n1.7.3.4\n\n\n\n-- \nGuy Rouillier\n"},{"id":"161556","messageId":"7voc69p4xu.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"4D5E1116.7040501@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-18T18:34:37Z","receivedAt":"2011-02-18T18:34:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Guy Rouillier writes:\n\n> ... I'm new to all this and I thought \n> perhaps one of the listed committers had to submit the official patch. \n\nThere is no _listed committers_ ;-) I was hoping either you as the original\nauthor of the patch or Martin as the area expert would respond, but as\nlong as the result looks correct and explained well, it doesn't matter\neither way.\n\nJust one hopefully final question.\n\nAfter stripping \"/\" from the beginning of the line\nin order to treat newer .cvspass file format and the original file format\nthe same way, the code splits the remainder into two fields (cvsroot and\nlightly-scrambled password). It used to split only at a whitespace, which\nseems to be in line with the source of CVS 1.12.13 I looked at (it is in\npassword_entry_parseline() function, src/login.c). You new code however\nalso allows '=' to be a delimiter to be used for this split.\n\nIs this change intentional? If so please explain why it is necessary in\nthe commit log message.\n"},{"id":"161587","messageId":"4D5F6E97.4000402@burntmail.com","threadId":"26106","inReplyTo":"7voc69p4xu.fsf@alter.siamese.dyndns.org","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-02-19T07:17:43Z","receivedAt":"2011-02-19T07:17:43Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 2/18/2011 1:34 PM, Junio C Hamano wrote:\n> Guy Rouillier writes:\n> \n>> ... I'm new to all this and I thought \n>> perhaps one of the listed committers had to submit the official patch. \n> \n> There is no _listed committers_ ;-) I was hoping either you as the original\n> author of the patch or Martin as the area expert would respond, but as\n> long as the result looks correct and explained well, it doesn't matter\n> either way.\n> \n> Just one hopefully final question.\n> \n> After stripping \"/\" from the beginning of the line\n> in order to treat newer .cvspass file format and the original file format\n> the same way, the code splits the remainder into two fields (cvsroot and\n> lightly-scrambled password). It used to split only at a whitespace, which\n> seems to be in line with the source of CVS 1.12.13 I looked at (it is in\n> password_entry_parseline() function, src/login.c). You new code however\n> also allows '=' to be a delimiter to be used for this split.\n> \n> Is this change intentional? If so please explain why it is necessary in\n> the commit log message.\n\nThanks to everyone here for the gracious patience with newcomers.\n\nYes, the change is intentional. I've added an additional commit comment to\nexplain why.\n\n>From 0fdfbdc0dbd0a0280d987640890f7b5ff566d6ef Mon Sep 17 00:00:00 2001\nFrom: Guy Rouillier \nDate: Sat, 19 Feb 2011 01:56:15 -0500\nSubject: [PATCH] Look for password in both CVS and CVSNT password files.\n\nThe existing code looks for the CVS reposity password only in\nthe CVS password file in HOME/.cvspass. Accommodate the CVS\nalternative CVSNT by also looking in HOME/.cvs/cvspass. Die\nif both files are found, and ask the user to remove one.\n\nThe two clients use a different delimiter to separate the CVS\nrepository name from the user password. The original CVS\nclient separates the two entries with a space character, while\nCVSNT separates them with an equal (=) character. Hence,\nthe regular expression used to split these two tokens is\naltered to accept either delimiter.\n\nSigned-off-by: Guy Rouillier \n---\n git-cvsimport.perl | 32 ++++++++++++++++++++------------\n 1 files changed, 20 insertions(+), 12 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 8e683e5..76b4765 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -259,19 +259,27 @@ sub conn {\n \t\tif ($pass) {\n \t\t\t$pass = $self->_scramble($pass);\n \t\t} else {\n-\t\t\topen(H,$ENV{'HOME'}.\"/.cvspass\") and do {\n-\t\t\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n-\t\t\t\t\tchomp;\n-\t\t\t\t\ts/^\\/\\d+\\s+//;\n-\t\t\t\t\tmy ($w,$p) = split(/\\s/,$_,2);\n-\t\t\t\t\tif ($w eq $rr or $w eq $rr2) {\n-\t\t\t\t\t\t$pass = $p;\n-\t\t\t\t\t\tlast;\n+\t\t\tmy @cvspasslocations = ($ENV{'HOME'}.\"/.cvspass\", $ENV{'HOME'}.\"/.cvs/cvspass\");\n+\t\t\tmy $filecount = 0;\n+\t\t\tforeach my $cvspass (@cvspasslocations) {\n+\n+\t\t\t\topen(H, $cvspass) and do {\n+\t\t\t\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n+\t\t\t\t\t\tchomp;\n+\t\t\t\t\t\ts/^\\/\\d+\\s+//;\n+\t\t\t\t\t\tmy ($w,$p) = split(/[\\s=]/,$_,2);\n+\t\t\t\t\t\tif ($w eq $rr or $w eq $rr2) {\n+\t\t\t\t\t\t\t$pass = $p;\n+\t\t\t\t\t\t\tlast;\n+\t\t\t\t\t\t}\n \t\t\t\t\t}\n-\t\t\t\t}\n-\t\t\t};\n-\t\t\t$pass = \"A\" unless $pass;\n+\t\t\t\t};\n+\t\t\t}\n+\n+\t\t\tdie(\"Two CVS password files found: @cvspasslocations, please remove one\") if $filecount > 1;\n+\t\t\tdie(\"Password not found for CVSROOT: $opt_d\\n\") unless $pass;\n \t\t}\n\n \t\tmy ($s, $rep);\n--\n1.7.4.rc1.5.ge17aa\n\n-- \nGuy Rouillier\n"},{"id":"161738","messageId":"7vy65bkw72.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"4D5F6E97.4000402@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-20T07:21:37Z","receivedAt":"2011-02-20T07:21:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Guy Rouillier writes:\n\n> The two clients use a different delimiter to separate the CVS\n> repository name from the user password. The original CVS\n> client separates the two entries with a space character, while\n> CVSNT separates them with an equal (=) character. Hence,\n> the regular expression used to split these two tokens is\n> altered to accept either delimiter.\n\nThat sounds like a wrong approach. If there are two clients, one reads\nfrom one location with one syntax, and the other one reads from another\ndifferent location with a different syntax, shouldn't the code using the\noriginal syntax when reading the original file, and the other syntax when\nreading the file for the other client?\n\nI personally don't even like the sloppiness of the original code before\nyour patch that discards the version information (\"/\") and hopes\nthe file format stays the same for some time to come, but \"one uses space\nand the other uses equal, so lets mix them up and split at space-or-equal\nwhen we know we are reading from the file that uses space (iow the one we\nknow we shouldn't be splitting at equal)\" is making it even worse.\n\nIn practice, I would imagine that the cvsroot part wouldn't contain an\nequal sign, so this looser regexp would not hurt in the real life, but it\ndoes feel yucky.\n\nHere is a totally untested patch. I think the original code used\n$pass=\"A\" as a fall-back when it didn't find any password entry, and I\ntried to retain that instead of dying. Also this does not error out if\nyou merely have two cvspass files, as long as you do not have the wanted\nentry for both of them.\n\n git-cvsimport.perl | 52 ++++++++++++++++++++++++++++++++++++++++------------\n 1 files changed, 40 insertions(+), 12 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 8e683e5..0a25926 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -227,6 +227,31 @@ sub new {\n \treturn $self;\n }\n \n+sub find_password_entry {\n+\tmy ($cvspass, @cvsroot) = @_;\n+\tmy ($file, $delim) = @$cvspass;\n+\tmy $pass;\n+\tlocal ($_);\n+\n+\tif (open(my $fh, $file)) {\n+\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n+\t\t\tchomp;\n+\t\t\ts/^\\/\\d+\\s+//;\n+\t\t\tmy ($w, $p) = split($delim,$_,2);\n+\t\t\tfor my $cvsroot (@cvsroot) {\n+\t\t\t\tif ($w eq $cvsroot) {\n+\t\t\t\t\t$pass = $p;\n+\t\t\t\t\tlast CVSPASSFILE;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tclose($fh);\n+\t}\n+\treturn $pass;\n+}\n+\n sub conn {\n \tmy $self = shift;\n \tmy $repo = $self->{'fullrep'};\n@@ -259,19 +284,22 @@ sub conn {\n \t\tif ($pass) {\n \t\t\t$pass = $self->_scramble($pass);\n \t\t} else {\n-\t\t\topen(H,$ENV{'HOME'}.\"/.cvspass\") and do {\n-\t\t\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n-\t\t\t\t\tchomp;\n-\t\t\t\t\ts/^\\/\\d+\\s+//;\n-\t\t\t\t\tmy ($w,$p) = split(/\\s/,$_,2);\n-\t\t\t\t\tif ($w eq $rr or $w eq $rr2) {\n-\t\t\t\t\t\t$pass = $p;\n-\t\t\t\t\t\tlast;\n-\t\t\t\t\t}\n+\t\t\tmy @cvspass = ([$ENV{'HOME'}.\"/.cvspass\", qr/\\s/],\n+\t\t\t\t [$ENV{'HOME'}.\"/.cvs/cvspass\", qr/=/]);\n+\t\t\tmy @loc = ();\n+\t\t\tforeach my $cvspass (@cvspass) {\n+\t\t\t\tmy $p = find_password_entry($cvspass, $rr, $rr2);\n+\t\t\t\tif ($p) {\n+\t\t\t\t\tpush @loc, $cvspass->[0];\n+\t\t\t\t\t$pass = $p;\n \t\t\t\t}\n-\t\t\t};\n-\t\t\t$pass = \"A\" unless $pass;\n+\t\t\t}\n+\t\t\tif (1 < @loc) {\n+\t\t\t\tdie(\"More than one cvs password files have \".\n+\t\t\t\t \"entries for CVSROOT $opt_d: @loc\");\n+\t\t\t} elsif (!$pass) {\n+\t\t\t\t$pass = \"A\";\n+\t\t\t}\n \t\t}\n \n \t\tmy ($s, $rep);\n"},{"id":"161774","messageId":"4D61EA4B.3020708@burntmail.com","threadId":"26106","inReplyTo":"7vy65bkw72.fsf@alter.siamese.dyndns.org","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-02-21T04:30:03Z","receivedAt":"2011-02-21T04:30:03Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 2/20/2011 2:21 AM, Junio C Hamano wrote:\n> Guy Rouillier writes:\n> \n>> The two clients use a different delimiter to separate the CVS\n>> repository name from the user password. The original CVS\n>> client separates the two entries with a space character, while\n>> CVSNT separates them with an equal (=) character. Hence,\n>> the regular expression used to split these two tokens is\n>> altered to accept either delimiter.\n> \n> That sounds like a wrong approach. If there are two clients, one reads\n> from one location with one syntax, and the other one reads from another\n> different location with a different syntax, shouldn't the code using the\n> original syntax when reading the original file, and the other syntax when\n> reading the file for the other client?\n\n...\n\n> In practice, I would imagine that the cvsroot part wouldn't contain an\n> equal sign, so this looser regexp would not hurt in the real life, but it\n> does feel yucky.\n\nWell, this is the important point. I did think of these aspects when\nwriting the code. Sure, writing more precise code is possible, but the\nresults are the same in either case. If you look back at the version I\nwrote in response to Emil's post, I did have two entirely separate \nsections for CVS and CVSNT, and I used only one delimiter in each. \nMartin then suggested I combine the two sections into one, so while\nfollowing that suggestion I had to alter the regular expression.\n\n> Here is a totally untested patch. I think the original code used\n> $pass=\"A\" as a fall-back when it didn't find any password entry, and I\n> tried to retain that instead of dying. Also this does not error out if\n> you merely have two cvspass files, as long as you do not have the wanted\n> entry for both of them.\n\nI'll take a look at the patch later this week when I have some time. I\npurposely took out the code the set the password to \"A\" if the CVS\nrepository is not located in the password file. I was surprised to\nsee that in the original code. I can't think of any situation \nwhere silently making up a password is a good idea.\n\nFinally, after our last round of discussions, I thought we all agreed\nnot to try to do any matching on the contents of the two password files.\nI originally implemented a comparison of the contents of the two files,\nbut you pointed out there are many possible permutations involving\nentries found in both files. So I went back to my earlier approach\nof just erroring out if both files are found.\n\n-- \nGuy Rouillier\n"},{"id":"161845","messageId":"7vtyfxgdz2.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"4D61EA4B.3020708@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-21T23:33:21Z","receivedAt":"2011-02-21T23:33:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Guy Rouillier writes:\n\n> On 2/20/2011 2:21 AM, Junio C Hamano wrote:\n> ...\n>> In practice, I would imagine that the cvsroot part wouldn't contain an\n>> equal sign, so this looser regexp would not hurt in the real life, but it\n>> does feel yucky.\n>\n> Well, this is the important point. I did think of these aspects when\n> writing the code. Sure, writing more precise code is possible, but the\n> results are the same in either case.\n\nIt is probably unlikely to see a SP in the pathname, but I do not think it\nis reasonable to introduce a regression to forbid '=' in the pathname to\nthe repository, which we have been supporting since August 2009, when we\nknow the patch as-is will regress the use case, and especially when we\nalready know a way to code not to regress is not too complex.\n\nThe \"substitute with 'A' when missing\" comes from e481b1d (cvs: initialize\nempty password, 2009-09-17); it makes me worried that the patch is\nremoving the support, _unless_ that commit by Clemens was addressing a\nproblem that does not exist (and if so, I'd like to see a sentence or two in\nthe commit log to explain why it is a sane thing to do to remove it).\n\nThanks.\n"},{"id":"161920","messageId":"7vipwbbrcc.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"7vtyfxgdz2.fsf@alter.siamese.dyndns.org","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-22T23:08:03Z","receivedAt":"2011-02-22T23:08:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> It is probably unlikely to see a SP in the pathname, but I do not think it\n> is reasonable to introduce a regression to forbid '=' in the pathname to\n> the repository, which we have been supporting since August 2009, when we\n> know the patch as-is will regress the use case, and especially when we\n> already know a way to code not to regress is not too complex.\n\nEven though I don't deeply care about what CVSNT does, but I am somewhat\ncurious why this \"change SP to =\" was done when CVSNT writes out its\n$HOME/.cvs/cvspass file.\n\nDoes anybody know why? Only to make things incompatible, perhaps? ;-)\n\n http://www.selenic.com/pipermail/mercurial/2009-April/025095.html\n\nseems to indicate that somebody next door had a similar experience on\nexactly the same issue.\n"},{"id":"161994","messageId":"AANLkTinUtUNGO3NK=JPTqnwcTtPMYjmLw82wJZ5nC-32@mail.gmail.com","threadId":"26106","inReplyTo":"7vipwbbrcc.fsf@alter.siamese.dyndns.org","subject":"Re: cvsimport still not working with cvsnt","fromName":"Martin Langhoff","fromEmail":"martin@laptop.org","sentAt":"2011-02-22T23:50:09Z","receivedAt":"2011-02-22T23:50:09Z","isPatch":false,"sender":{"key":"martin@laptop.org","avatar":null},"body":"On Tue, Feb 22, 2011 at 6:08 PM, Junio C Hamano wrote:\n> Even though I don't deeply care about what CVSNT does...\n...\n> Does anybody know why?  Only to make things incompatible, perhaps? ;-)\n\nA brief googling around shows that it also stores it in the Windows registry.\n\nShould we support that too...? ;-)\n\n\nm\n-- \n martin@laptop.org -- Software Architect - OLPC\n - ask interesting questions\n - don't get distracted with shiny stuff  - working code first\n - http://wiki.laptop.org/go/User:Martinlanghoff\n"},{"id":"161997","messageId":"4D644FEE.5030004@burntmail.com","threadId":"26106","inReplyTo":"AANLkTinUtUNGO3NK=JPTqnwcTtPMYjmLw82wJZ5nC-32@mail.gmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-02-23T00:08:14Z","receivedAt":"2011-02-23T00:08:14Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 2/22/2011 6:50 PM, Martin Langhoff wrote:\n> On Tue, Feb 22, 2011 at 6:08 PM, Junio C Hamano wrote:\n>> Even though I don't deeply care about what CVSNT does...\n> ..\n>> Does anybody know why? Only to make things incompatible, perhaps? ;-)\n>\n> A brief googling around shows that it also stores it in the Windows registry.\n>\n> Should we support that too...? ;-)\n\nOne thing at a time, Martin :). After I get this patch through, I want \nto start working on getting the rest of the Perl script to run under \nWindows. I was almost there; the biggest issue is that Perl \nimplementations (ActiveState, Strawberry) for Windows don't support the \nlist form of open. I converted most of them successfully, but got stuck \non one so decided to submit this patch first. Thank goodness I did this \nseparately :).\n\nTo answer Junio's question, I'm looking at the CVSNT code now \n(GlobalSettings.cpp, if anyone is interested.) The password is stored \nin a general fashion like any other user-specified value. So, the \nauthors elected to use a properties file format of key=value. That is \nas valid a format as any other.\n\n-- \nGuy Rouillier\n"},{"id":"161998","messageId":"7vei6zbmz8.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"AANLkTinUtUNGO3NK=JPTqnwcTtPMYjmLw82wJZ5nC-32@mail.gmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-23T00:42:19Z","receivedAt":"2011-02-23T00:42:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n> On Tue, Feb 22, 2011 at 6:08 PM, Junio C Hamano wrote:\n>> Even though I don't deeply care about what CVSNT does...\n> ...\n>> Does anybody know why? Only to make things incompatible, perhaps? ;-)\n>\n> A brief googling around shows that it also stores it in the Windows registry.\n\nYes, I saw that too. I actually also got the impression that registry is\nthe primary location for cvsnt (hence I suspect .cvs/cvspass support might\nbe secondary and would not be surprised if it were sub-par).\n"},{"id":"161999","messageId":"7vaahnbmu2.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"4D644FEE.5030004@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-23T00:45:25Z","receivedAt":"2011-02-23T00:45:25Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Guy Rouillier writes:\n\n> To answer Junio's question, I'm looking at the CVSNT code now\n> (GlobalSettings.cpp, if anyone is interested.) The password is stored\n> in a general fashion like any other user-specified value. So, the\n> authors elected to use a properties file format of key=value. That is\n> as valid a format as any other.\n\nAs you dug that far, could you find out what happens when cvsroot contains\nan equal-sign character in its path component?\n\nI am starting to suspect that we do need two separate codepaths, and we\nwould need to split out the logic to find matching password entry given a\ncvsroot value into a separate function to keep our sanity after all.\n"},{"id":"162003","messageId":"4D6471E8.4060001@burntmail.com","threadId":"26106","inReplyTo":"7vaahnbmu2.fsf@alter.siamese.dyndns.org","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-02-23T02:33:12Z","receivedAt":"2011-02-23T02:33:12Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 2/22/2011 7:45 PM, Junio C Hamano wrote:\n> Guy Rouillier writes:\n>\n>> To answer Junio's question, I'm looking at the CVSNT code now\n>> (GlobalSettings.cpp, if anyone is interested.) The password is stored\n>> in a general fashion like any other user-specified value. So, the\n>> authors elected to use a properties file format of key=value. That is\n>> as valid a format as any other.\n>\n> As you dug that far, could you find out what happens when cvsroot contains\n> an equal-sign character in its path component?\n>\n> I am starting to suspect that we do need two separate codepaths, and we\n> would need to split out the logic to find matching password entry given a\n> cvsroot value into a separate function to keep our sanity after all.\n>\n\nI'll take a look. I spent a short amount of time with Google looking \nfor \"cvsroot valid characters\" but didn't find anything authoritative. \nNote that this issue is not unique to CVSNT. What does CVS do with \nCVSROOT containing a space character?\n\n-- \nGuy Rouillier\n"},{"id":"162005","messageId":"7vpqqj9vc7.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"4D6471E8.4060001@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-23T05:24:40Z","receivedAt":"2011-02-23T05:24:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Guy Rouillier writes:\n\n> ... Note that this issue is not unique to CVSNT. What does\n> CVS do with CVSROOT containing a space character?\n\nIIRC, the comparison is done against canonicalized cvsroot string, so that\nyou can try to connect to :pserver:Xz.Com:/path/to/repo even after you ran\n\"cvs -d :pserver:xz.com:/path/to/repo login\" and I wouldn't be surprised\nif the canonicalization involved quoting SP. Since August 2009 nobody has\ncomplained with the current code that doesn't do any canonicalization, and\nI take that as a sign that nobody sane so far used a cvsroot with a space\nin it ;-). But that doesn't mean nobody sane has been using a cvsroot\nwith an equal sign in it, so we would need to at least avoid splitting at\nan equal sign when reading from .cvsroot.\n\nIt probably is a good idea to port the cvsroot canonicalization code to\ncvsimport in any case.\n"},{"id":"162103","messageId":"4D65CD12.3070903@burntmail.com","threadId":"26106","inReplyTo":"7vei6zbmz8.fsf@alter.siamese.dyndns.org","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-02-24T03:14:26Z","receivedAt":"2011-02-24T03:14:26Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 2/22/2011 7:42 PM, Junio C Hamano wrote:\n> Martin Langhoff writes:\n>\n>> On Tue, Feb 22, 2011 at 6:08 PM, Junio C Hamano wrote:\n>>> Even though I don't deeply care about what CVSNT does...\n>> ...\n>>> Does anybody know why? Only to make things incompatible, perhaps? ;-)\n>>\n>> A brief googling around shows that it also stores it in the Windows registry.\n>\n> Yes, I saw that too. I actually also got the impression that registry is\n> the primary location for cvsnt (hence I suspect .cvs/cvspass support might\n> be secondary and would not be surprised if it were sub-par).\n\nThere may perhaps be a misunderstanding of CVSNT. CVSNT is a \nmulti-platform client and server. Both parts can run on many platforms, \nincluding Windows, Linux, and Solaris. I don't use Macs so don't know \nabout them.\n\nUse of HOME/.cvs/cvspass is not secondary or sub-par. On any platform \nother than Windows, HOME/.cvs/cvspass is the standard place that CVSNT \nstores repository passwords. And on Windows, you can optionally tell it \nto store repository passwords in HOME/.cvs/cvspass instead of the \nregistry. I have my Windows configured that way for consistency with my \nnumerous Linux accounts.\n\nThe whole reason I resurrected this 2 year old topic is that we are \ntrying to migrate from CVSNT on *Linux* to git.\n\nThanks.\n\n-- \nGuy Rouillier\n"},{"id":"162337","messageId":"4D69DF29.8030701@burntmail.com","threadId":"26106","inReplyTo":"7vaahnbmu2.fsf@alter.siamese.dyndns.org","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-02-27T05:20:41Z","receivedAt":"2011-02-27T05:20:41Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 2/23/2011 12:24 AM, Junio C Hamano wrote:\n> Guy Rouillier writes:\n>\n>> ... Note that this issue is not unique to CVSNT. What does CVS do\n>> with CVSROOT containing a space character?\n>\n> IIRC, the comparison is done against canonicalized cvsroot string, so\n> that you can try to connect to :pserver:Xz.Com:/path/to/repo even\n> after you ran \"cvs -d :pserver:xz.com:/path/to/repo login\" and I\n> wouldn't be surprised if the canonicalization involved quoting SP.\n> Since August 2009 nobody has complained with the current code that\n> doesn't do any canonicalization, and I take that as a sign that\n> nobody sane so far used a cvsroot with a space in it ;-). But that\n> doesn't mean nobody sane has been using a cvsroot with an equal sign\n> in it, so we would need to at least avoid splitting at an equal sign\n> when reading from .cvsroot.\n>\n> It probably is a good idea to port the cvsroot canonicalization code\n> to cvsimport in any case.\n\nAs I suspected after reading how the cvspass file is read and written, \nCVSNT doesn't work with repositories with an equal sign in the \nrepository name. You can init it fine, and you can set up a password \nfor it. But if you try to login things go very wrong:\n\nguyr@gentoo-vm /data $ cvs -d \n\":pserver:guyr@gentoo-vm:2401:/data/cvs\\=repo/cvsroot\" login\nLogging in to :pserver:guyr@gentoo-vm:2401:/data/cvs\\=repo/cvsroot\nCVS Password:\nEmpty password used - try 'cvs login' with a real password\ncvs [login aborted]: /data/cvs\\=repo/cvsroot: no such repository\n\nI tried as many permutations as I could think of, escaping the equal \nsign, not escaping it, etc. None of them worked. I did verify my \nenvironment before running this test by setting up a repository without \nthe equal sign in the name, and everything works fine.\n\nSince CVSNT can't handle a repository with an equal sign in its name, I \nsay we don't worry about this. I say the same about the original CVS \nwith a repository name with embedded spaces. We certainly don't want to \ntry to solve problems the original product doesn't solve.\n\n-- \nGuy Rouillier\n"},{"id":"162341","messageId":"7vvd056fyk.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"4D69DF29.8030701@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-02-27T08:26:27Z","receivedAt":"2011-02-27T08:26:27Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Guy Rouillier writes:\n\n> As I suspected after reading how the cvspass file is read and written,\n> CVSNT doesn't work with repositories with an equal sign in the\n> repository name. You can init it fine, and you can set up a password\n> for it. But if you try to login things go very wrong:\n> ...\n> Since CVSNT can't handle a repository with an equal sign in its name,\n> I say we don't worry about this. I say the same about the original\n> CVS with a repository name with embedded spaces. We certainly don't\n> want to try to solve problems the original product doesn't solve.\n\nThanks; your observation matches my earlier suspicion. So in short:\n\n - CVSNT does not work with repository path with an '=' in it, but does work\n with ones with a SP in it; and\n\n - CVS has trouble with repository path with a SP in it, but works with\n ones with an '=' in it just fine.\n\nHave I summarized it correctly?\n\nSo I agree that cvsimport should not worry about supporting repository\npath with an '=' in it, but we do need to make sure we work with one with\na SP in it, when we are reading from cvspass file for CVSNT.\n\nSimilarly when we are reading from cvspass file for CVS, we should make\nsure we don't break with repository path with an '=' in it.\n\nDo we already have such a solution in the thread? Can somebody conclude\nthe discussion with a final, tested and applyable patch, please?\n"},{"id":"166706","messageId":"4DBA3E14.7090602@burntmail.com","threadId":"26106","inReplyTo":"7vvd056fyk.fsf@alter.siamese.dyndns.org","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-04-29T04:27:00Z","receivedAt":"2011-04-29T04:27:00Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 2/27/2011 3:26 AM, Junio C Hamano wrote:\n> Guy Rouillier writes:\n>\n>> As I suspected after reading how the cvspass file is read and written,\n>> CVSNT doesn't work with repositories with an equal sign in the\n>> repository name. You can init it fine, and you can set up a password\n>> for it. But if you try to login things go very wrong:\n>> ...\n>> Since CVSNT can't handle a repository with an equal sign in its name,\n>> I say we don't worry about this. I say the same about the original\n>> CVS with a repository name with embedded spaces. We certainly don't\n>> want to try to solve problems the original product doesn't solve.\n>\n> Thanks; your observation matches my earlier suspicion. So in short:\n>\n> - CVSNT does not work with repository path with an '=' in it, but does work\n> with ones with a SP in it; and\n>\n> - CVS has trouble with repository path with a SP in it, but works with\n> ones with an '=' in it just fine.\n>\n> Have I summarized it correctly?\n>\n> So I agree that cvsimport should not worry about supporting repository\n> path with an '=' in it, but we do need to make sure we work with one with\n> a SP in it, when we are reading from cvspass file for CVSNT.\n>\n> Similarly when we are reading from cvspass file for CVS, we should make\n> sure we don't break with repository path with an '=' in it.\n>\n> Do we already have such a solution in the thread? Can somebody conclude\n> the discussion with a final, tested and applyable patch, please?\n\nI've integrated the untested version you submitted several iterations ago,\nand tested it with CVSNT. Unfortunately, I don't have a CVS repo to test\nwith, so if anyone else watching this thread has access to CVS, it would\nbe good if they can test with CVS. Here is my hopefully final revision.\n\nNote that I've left this test in, although I still think it is a bad idea:\n\n elsif (!$pass) {\n $pass = \"A\";\n }\n\nI looked back at the revisions around 9/17/2009. That revision puts\nthis test back in because two revisions earlier on 8/14/2009 took it out.\nBut that doesn't explain why it was put in there in the first\nplace. I still say a better idea, if we don't want to allow an empty\npassword, is to error out rather than silently set a bogus password.\n\n---\nFrom bea4854fca07ff3a7034a04ad2f163701f9f581c Mon Sep 17 00:00:00 2001\nFrom: Guy Rouillier \nDate: Fri, 29 Apr 2011 00:01:52 -0400\nSubject: [PATCH] Look for password in both CVS and CVSNT password files.\n\nIn conn, if password is not passed on command line, look for a password\nentry in both the CVS password file and the CVSNT password file. If only\none file is found and the requested repository is in that file, or if both\nfiles are found but the requested repository is found in only one file, use\nthe password from the single file containing the repository entry. If both\nfiles are found and the requested repository is found in both files, then\nproduce an error message.\n\nThe CVS password file separates tokens with a space character, while\nthe CVSNT password file separates tokens with an equal (=) character.\nAdd a sub find_password_entry that accepts the password file name\nand a delimiter to eliminate code duplication.\n---\n git-cvsimport.perl | 52 ++++++++++++++++++++++++++++++++++++++++------------\n 1 files changed, 40 insertions(+), 12 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex bbf327f..abf5759 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -227,6 +227,30 @@ sub new {\n \treturn $self;\n }\n\n+sub find_password_entry {\n+\tmy ($cvspass, @cvsroot) = @_;\n+\tmy ($file, $delim) = @$cvspass;\n+\tmy $pass;\n+\tlocal ($_);\n+\n+\tif (open(my $fh, $file)) {\n+\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n+\t\t\tchomp;\n+\t\t\ts/^\\/\\d+\\s+//;\n+\t\t\tmy ($w, $p) = split($delim,$_,2);\n+\t\t\tfor my $cvsroot (@cvsroot) {\n+\t\t\t\tif ($w eq $cvsroot) {\n+\t\t\t\t\t$pass = $p;\n+\t\t\t\t\tlast;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tclose($fh);\n+\t}\n+\treturn $pass;\n+}\n+\n sub conn {\n \tmy $self = shift;\n \tmy $repo = $self->{'fullrep'};\n@@ -259,19 +283,23 @@ sub conn {\n \t\tif ($pass) {\n \t\t\t$pass = $self->_scramble($pass);\n \t\t} else {\n-\t\t\topen(H,$ENV{'HOME'}.\"/.cvspass\") and do {\n-\t\t\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n-\t\t\t\t\tchomp;\n-\t\t\t\t\ts/^\\/\\d+\\s+//;\n-\t\t\t\t\tmy ($w,$p) = split(/\\s/,$_,2);\n-\t\t\t\t\tif ($w eq $rr or $w eq $rr2) {\n-\t\t\t\t\t\t$pass = $p;\n-\t\t\t\t\t\tlast;\n-\t\t\t\t\t}\n+\t\t\tmy @cvspass = ([$ENV{'HOME'}.\"/.cvspass\", qr/\\s/],\n+\t\t\t\t [$ENV{'HOME'}.\"/.cvs/cvspass\", qr/=/]);\n+\t\t\tmy @loc = ();\n+\t\t\tforeach my $cvspass (@cvspass) {\n+\t\t\t\tmy $p = find_password_entry($cvspass, $rr, $rr2);\n+\t\t\t\tif ($p) {\n+\t\t\t\t\tpush @loc, $cvspass->[0];\n+\t\t\t\t\t$pass = $p;\n \t\t\t\t}\n-\t\t\t};\n-\t\t\t$pass = \"A\" unless $pass;\n+\t\t\t}\n+\n+\t\t\tif (1 < @loc) {\n+\t\t\t\tdie(\"More than one cvs password files have \".\n+\t\t\t\t \"entries for CVSROOT $opt_d: @loc\");\n+\t\t\t} elsif (!$pass) {\n+\t\t\t\t$pass = \"A\";\n+\t\t\t}\t\t\n \t\t}\n\n \t\tmy ($s, $rep);\n--\n1.7.4.rc1.5.ge17aa\n\n-- \nGuy Rouillier\n"},{"id":"166779","messageId":"20110429222729.GB5916@elie","threadId":"26106","inReplyTo":"4DBA3E14.7090602@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2011-04-29T22:27:29Z","receivedAt":"2011-04-29T22:27:29Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Guy Rouillier wrote:\n\n> Note that I've left this test in, although I still think it is a bad idea:\n>\n> elsif (!$pass) {\n> $pass = \"A\";\n> }\n[...]\n> But that doesn't explain why it was put in there in the first\n> place. I still say a better idea, if we don't want to allow an empty\n> password, is to error out rather than silently set a bogus password.\n\nIt might be a good idea after all to do something else in that case\n(as a separate patch :)), but it would require a little investigation.\nIsn't the convention in CVS for anonymous pserver access to accept an\narbitrary password?\n\n> The CVS password file separates tokens with a space character, while\n> the CVSNT password file separates tokens with an equal (=) character.\n> Add a sub find_password_entry that accepts the password file name\n> and a delimiter to eliminate code duplication.\n> ---\n\nSounds sensible to my untrained ears. Sign-off?\n\n[...]\n> +++ b/git-cvsimport.perl\n> @@ -227,6 +227,30 @@ sub new {\n> \treturn $self;\n> }\n> \n> +sub find_password_entry {\n> +\tmy ($cvspass, @cvsroot) = @_;\n> +\tmy ($file, $delim) = @$cvspass;\n> +\tmy $pass;\n> +\tlocal ($_);\n> +\n> +\tif (open(my $fh, $file)) {\n> +\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah +\t\twhile (<$fh>) {\n> +\t\t\tchomp;\n> +\t\t\ts/^\\/\\d+\\s+//;\n> +\t\t\tmy ($w, $p) = split($delim,$_,2);\n> +\t\t\tfor my $cvsroot (@cvsroot) {\n> +\t\t\t\tif ($w eq $cvsroot) {\n> +\t\t\t\t\t$pass = $p;\n> +\t\t\t\t\tlast;\n\nIn the old code, this \"last\" applied to the while loop, while in the\nnew code it applies to the for loop. Intentional?\n\n[...]\n> +\t\t\tif (1 < @loc) {\n> +\t\t\t\tdie(\"More than one cvs password files have \".\n> +\t\t\t\t \"entries for CVSROOT $opt_d: @loc\");\n\nGrammar nit: \"More than one\" is singular (weird, eh?). It might\nbe clearer to say:\n\n\t\"Multiple cvs password files have \" .\n\t\"entries for CVSROOT $opt_d: @loc\"\n\n(or \"Both cvs password files\").\n\nThanks again, and hope that helps.\n\nRegards,\nJonathan\n"},{"id":"166807","messageId":"4DBCF0C0.8080307@burntmail.com","threadId":"26106","inReplyTo":"20110429222729.GB5916@elie","subject":"Re: cvsimport still not working with cvsnt","fromName":"Guy Rouillier","fromEmail":"guyr@burntmail.com","sentAt":"2011-05-01T05:33:52Z","receivedAt":"2011-05-01T05:33:52Z","isPatch":false,"sender":{"key":"guyr@burntmail.com","avatar":null},"body":"On 4/29/2011 6:27 PM, Jonathan Nieder wrote:\n> Guy Rouillier wrote:\n> \n>> Note that I've left this test in, although I still think it is a bad idea:\n>>\n>> elsif (!$pass) {\n>> $pass = \"A\";\n>> }\n> [...]\n>> But that doesn't explain why it was put in there in the first\n>> place. I still say a better idea, if we don't want to allow an empty\n>> password, is to error out rather than silently set a bogus password.\n> \n> It might be a good idea after all to do something else in that case\n> (as a separate patch :)), but it would require a little investigation.\n> Isn't the convention in CVS for anonymous pserver access to accept an\n> arbitrary password?\n> \n>> The CVS password file separates tokens with a space character, while\n>> the CVSNT password file separates tokens with an equal (=) character.\n>> Add a sub find_password_entry that accepts the password file name\n>> and a delimiter to eliminate code duplication.\n>> ---\n> \n> Sounds sensible to my untrained ears. Sign-off?\n> \n> [...]\n>> +++ b/git-cvsimport.perl\n>> @@ -227,6 +227,30 @@ sub new {\n>> \treturn $self;\n>> }\n>>\n>> +sub find_password_entry {\n>> +\tmy ($cvspass, @cvsroot) = @_;\n>> +\tmy ($file, $delim) = @$cvspass;\n>> +\tmy $pass;\n>> +\tlocal ($_);\n>> +\n>> +\tif (open(my $fh, $file)) {\n>> +\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah> +\t\twhile (<$fh>) {\n>> +\t\t\tchomp;\n>> +\t\t\ts/^\\/\\d+\\s+//;\n>> +\t\t\tmy ($w, $p) = split($delim,$_,2);\n>> +\t\t\tfor my $cvsroot (@cvsroot) {\n>> +\t\t\t\tif ($w eq $cvsroot) {\n>> +\t\t\t\t\t$pass = $p;\n>> +\t\t\t\t\tlast;\n> \n> In the old code, this \"last\" applied to the while loop, while in the\n> new code it applies to the for loop. Intentional?\n> \n> [...]\n>> +\t\t\tif (1< @loc) {\n>> +\t\t\t\tdie(\"More than one cvs password files have \".\n>> +\t\t\t\t \"entries for CVSROOT $opt_d: @loc\");\n> \n> Grammar nit: \"More than one\" is singular (weird, eh?). It might\n> be clearer to say:\n> \n> \t\"Multiple cvs password files have \" .\n> \t\"entries for CVSROOT $opt_d: @loc\"\n> \n> (or \"Both cvs password files\").\n> \n> Thanks again, and hope that helps.\n\nJonathan, thanks for reading carefully. I hadn't looked at this in a\ncouple months because I've been busy at work, and Perl is not my strong\npoint. I had removed the last label because I didn't think it was\nnecessary, but you point out that it is.\n\nI concur with addressing that default CVS password with a different\npatch. That logic has been in the code since the original Perl version,\nso perhaps no one has really looked into CVS password requirements in\nany detail.\n\nHere is hopefully my final version:\n---\nFrom a96233ab1112748338e6445ed1e4a5f0e8c1213b Mon Sep 17 00:00:00 2001\nFrom: Guy Rouillier \nDate: Sun, 1 May 2011 01:23:44 -0400\nSubject: [PATCH] Look for password in both CVS and CVSNT password files.\n\nIn conn, if password is not passed on command line, look for a password\nentry in both the CVS password file and the CVSNT password file. If only\none file is found and the requested repository is in that file, or if both\nfiles are found but the requested repository is found in only one file, use\nthe password from the single file containing the repository entry. If both\nfiles are found and the requested repository is found in both files, then\nproduce an error message.\n\nThe CVS password file separates tokens with a space character, while\nthe CVSNT password file separates tokens with an equal (=) character.\nAdd a sub find_password_entry that accepts the password file name\nand a delimiter to eliminate code duplication.\n\nSigned-off-by: Guy Rouillier \n---\n git-cvsimport.perl | 53 ++++++++++++++++++++++++++++++++++++++++-----------\n 1 files changed, 41 insertions(+), 12 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex bbf327f..a01b73d 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -227,6 +227,31 @@ sub new {\n \treturn $self;\n }\n\n+sub find_password_entry {\n+\tmy ($cvspass, @cvsroot) = @_;\n+\tmy ($file, $delim) = @$cvspass;\n+\tmy $pass;\n+\tlocal ($_);\n+\n+\tif (open(my $fh, $file)) {\n+\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n+\t\t\tchomp;\n+\t\t\ts/^\\/\\d+\\s+//;\n+\t\t\tmy ($w, $p) = split($delim,$_,2);\n+\t\t\tfor my $cvsroot (@cvsroot) {\n+\t\t\t\tif ($w eq $cvsroot) {\n+\t\t\t\t\t$pass = $p;\n+\t\t\t\t\tlast CVSPASSFILE;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tclose($fh);\n+\t}\n+\treturn $pass;\n+}\n+\n sub conn {\n \tmy $self = shift;\n \tmy $repo = $self->{'fullrep'};\n@@ -259,19 +284,23 @@ sub conn {\n \t\tif ($pass) {\n \t\t\t$pass = $self->_scramble($pass);\n \t\t} else {\n-\t\t\topen(H,$ENV{'HOME'}.\"/.cvspass\") and do {\n-\t\t\t\t# :pserver:cvs@mea.tmt.tele.fi:/cvsroot/zmailer Ah) {\n-\t\t\t\t\tchomp;\n-\t\t\t\t\ts/^\\/\\d+\\s+//;\n-\t\t\t\t\tmy ($w,$p) = split(/\\s/,$_,2);\n-\t\t\t\t\tif ($w eq $rr or $w eq $rr2) {\n-\t\t\t\t\t\t$pass = $p;\n-\t\t\t\t\t\tlast;\n-\t\t\t\t\t}\n+\t\t\tmy @cvspass = ([$ENV{'HOME'}.\"/.cvspass\", qr/\\s/],\n+\t\t\t\t [$ENV{'HOME'}.\"/.cvs/cvspass\", qr/=/]);\n+\t\t\tmy @loc = ();\n+\t\t\tforeach my $cvspass (@cvspass) {\n+\t\t\t\tmy $p = find_password_entry($cvspass, $rr, $rr2);\n+\t\t\t\tif ($p) {\n+\t\t\t\t\tpush @loc, $cvspass->[0];\n+\t\t\t\t\t$pass = $p;\n \t\t\t\t}\n-\t\t\t};\n-\t\t\t$pass = \"A\" unless $pass;\n+\t\t\t}\n+\n+\t\t\tif (1 < @loc) {\n+\t\t\t\tdie(\"Multiple cvs password files have \".\n+\t\t\t\t \"entries for CVSROOT $opt_d: @loc\");\n+\t\t\t} elsif (!$pass) {\n+\t\t\t\t$pass = \"A\";\n+\t\t\t}\t\t\n \t\t}\n\n \t\tmy ($s, $rep);\n--\n1.7.5.134.gbea48\n\n\n\n\n\n-- \nGuy Rouillier\n"},{"id":"166820","messageId":"7vbozms1lb.fsf@alter.siamese.dyndns.org","threadId":"26106","inReplyTo":"4DBCF0C0.8080307@burntmail.com","subject":"Re: cvsimport still not working with cvsnt","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-01T18:44:00Z","receivedAt":"2011-05-01T18:44:00Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Thanks, will queue.\n"}]}