{"thread":{"id":"1512","subject":"[PATCH] Add -k kill keyword expansion option to git-cvsimport","startedAt":"2005-08-15T06:51:39Z","lastAt":"2005-08-16T11:29:15Z","messageCount":12,"participants":["Martin Langhoff","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"7266","messageId":"46a038f9050814235140877be7@mail.gmail.com","threadId":"1512","inReplyTo":null,"subject":"[PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2005-08-15T06:51:39Z","receivedAt":"2005-08-15T06:51:39Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"[PATCH] Add -k kill keyword expansion option to git-cvsimport\n\nEarly versions of git-cvsimport defaulted to using preexisting keyword\nexpansion settings. This change preserves compatibility with existing cvs\nimports and allows new repository migrations to kill keyword expansion.\n\nShould improve our chances of detecting merges and reduce imported\nrepository size.\n\nSigned-off: Martin Langhoff \n---\n\n Documentation/git-cvsimport-script.txt | 7 ++++++-\n git-cvsimport-script | 10 ++++++----\n 2 files changed, 12 insertions(+), 5 deletions(-)\n\nf0b33908e48ac75a1e3f08a3e3159de3a9a371d9\ndiff --git a/Documentation/git-cvsimport-script.txt\nb/Documentation/git-cvsimport-script.txt\n--- a/Documentation/git-cvsimport-script.txt\n+++ b/Documentation/git-cvsimport-script.txt\n@@ -11,7 +11,7 @@ SYNOPSIS\n --------\n 'git-cvsimport-script' [ -o ] [ -h ] [ -v ]\n [ -d ] [ -p ]\n- [ -C ] [ -i ] [ ]\n+ [ -C ] [ -i ] [ -k ] [ ]\n\n\n DESCRIPTION\n@@ -34,6 +34,11 @@ OPTIONS\n ensures the working directory and cache remain untouched and will\n not create them if they do not exist.\n\n+-k::\n+ Kill keywords: will extract files with -ko from the CVS archive\n+ to avoid noisy changesets. Highly recommended, but off by default\n+ to preserve compatibility with early imported trees.\n+\n -o ::\n The 'HEAD' branch from CVS is imported to the 'origin' branch within\n the git repository, as 'HEAD' already has a special meaning for git.\ndiff --git a/git-cvsimport-script b/git-cvsimport-script\n--- a/git-cvsimport-script\n+++ b/git-cvsimport-script\n@@ -35,12 +35,12 @@ sub usage() {\n Usage: ${\\basename $0} # fetch/update GIT from CVS\n [ -o branch-for-HEAD ] [ -h ] [ -v ] [ -d CVSROOT ]\n [ -p opts-for-cvsps ] [ -C GIT_repository ] [ -z fuzz ]\n- [ -i ] [ CVS_module ]\n+ [ -i ] [ -k ] [ CVS_module ]\n END\n exit(1);\n }\n\n-getopts(\"hivo:d:p:C:z:\") or usage();\n+getopts(\"hivko:d:p:C:z:\") or usage();\n usage if $opt_h;\n\n @ARGV <= 1 or usage();\n@@ -218,8 +218,10 @@ sub _file {\n my($self,$fn,$rev) = @_;\n $self->{'socketo'}->write(\"Argument -N\\n\") or return undef;\n $self->{'socketo'}->write(\"Argument -P\\n\") or return undef;\n- # $self->{'socketo'}->write(\"Argument -ko\\n\") or return undef;\n- # -ko: Linus' version doesn't use it\n+ # -ko: Linus' version doesn't use it - defaults to off\n+ if ($opt_k) {\n+ $self->{'socketo'}->write(\"Argument -ko\\n\") or return undef;\n+ }\n $self->{'socketo'}->write(\"Argument -r\\n\") or return undef;\n $self->{'socketo'}->write(\"Argument $rev\\n\") or return undef;\n $self->{'socketo'}->write(\"Argument --\\n\") or return undef;\n"},{"id":"7267","messageId":"46a038f9050814235625ef0b0f@mail.gmail.com","threadId":"1512","inReplyTo":"46a038f9050814235140877be7@mail.gmail.com","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2005-08-15T06:56:17Z","receivedAt":"2005-08-15T06:56:17Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 8/15/05, Martin Langhoff wrote:\n> [PATCH] Add -k kill keyword expansion option to git-cvsimport\n\n Bad patch! Please ignore while I fix and resend... \n\napologies.\n\nmartin\n"},{"id":"7270","messageId":"7vk6in65dp.fsf@assigned-by-dhcp.cox.net","threadId":"1512","inReplyTo":"46a038f9050814235140877be7@mail.gmail.com","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-15T07:12:02Z","receivedAt":"2005-08-15T07:12:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n> [PATCH] Add -k kill keyword expansion option to git-cvsimport\n>\n> Early versions of git-cvsimport defaulted to using preexisting keyword\n> expansion settings. This change preserves compatibility with existing cvs\n> imports and allows new repository migrations to kill keyword expansion.\n>\n> Should improve our chances of detecting merges and reduce imported\n> repository size.\n\nThe discussion between you and Linus since you brought this up\nhas kept me wondering if -ko is the only thing people may want\nto do, or sometimes -kk or even -kb or -kv make sense for some\nothers, in which case instead of a -k option that does not allow\nanything but -ko, making it take an optional single letter\no/k/b/v might might more sense. A single -k defaulting to -ko\nis fine by me if you did so, because I think that is the most\nuseful and usual mode of operation while converting to GIT\nrepository.\n\nThoughts?\n"},{"id":"7272","messageId":"46a038f90508150037f128d6@mail.gmail.com","threadId":"1512","inReplyTo":"7vk6in65dp.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2005-08-15T07:37:19Z","receivedAt":"2005-08-15T07:37:19Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 8/15/05, Junio C Hamano wrote:\n> The discussion between you and Linus since you brought this up\n> has kept me wondering if -ko is the only thing people may want\n> to do, or sometimes -kk or even -kb or -kv make sense for some\n\nThe git-cvsimport script requests the full file at a given revision\nstraight from the cvs server daemon it has instantiated. So I suspect\nwe are mixing up cvs client flags with protocol flags. Hmm, reading up\non it ( http://www.elegosoft.com/cvs/cvsclient.html#SEC9 ) the flags\nare carried through, and it sounds like it's pretty broken.\n\nStill, it clearly leaves handling of newlines to the client. The\ndifference between -kb and -ko (we are using -ko ATM) is the newline\nhandling if you are using standard cvs clients. With git-cvsimport, we\nare doing the unix thing, which happens to be right thing to do.\n\nPerhaps repos created with early versions of CVSNT are broken in this\nregard, but tough.\n\nI think -kv is just the wrong thing to do if you are migrating to git.\nAnyway, this script has so far followed cvs's own default... which is\n-kv, and I am generally unwilling to break backwards compatibility.\nThough we could make it default to 'on' and provide '-K' for those\nmasochistic enough to want it.\n\nPeople with repos where cvswrappers was set to mark files as -kb or\n-ko are safe from all this pain and tears.\n\ncheers,\n\n\nmartin\n"},{"id":"7273","messageId":"7vvf273aqb.fsf@assigned-by-dhcp.cox.net","threadId":"1512","inReplyTo":"46a038f90508150037f128d6@mail.gmail.com","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-15T07:44:44Z","receivedAt":"2005-08-15T07:44:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n> I think -kv is just the wrong thing to do if you are migrating to git.\n> Anyway, this script has so far followed cvs's own default... which is\n> -kv, and I am generally unwilling to break backwards compatibility.\n\nIsn't cvs default -kkv?\n"},{"id":"7274","messageId":"46a038f90508150052308cf7a8@mail.gmail.com","threadId":"1512","inReplyTo":"7vvf273aqb.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2005-08-15T07:52:42Z","receivedAt":"2005-08-15T07:52:42Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 8/15/05, Junio C Hamano wrote:\n> Isn't cvs default -kkv?\n\nYou're right, default is -kkv (expand keyword/value every time) and\nnot -kv (expand keyword/value only if previously unexpanded).\n\nThere's something else in the -kb / -ko distinction according to the\nprotocol description I linked before. Using -kb changes the way the\nfile is sent, and this may well break our protocol handling, which is\ncurrently quite happy to deal with the file as a series of lines.\n\nOTOH I am not sure if it breaks when dealing with true binary stuff\nlike images, compressed data, etc.\n\ncheers,\n\n\nmartin\n"},{"id":"7281","messageId":"7vacjj3968.fsf@assigned-by-dhcp.cox.net","threadId":"1512","inReplyTo":"46a038f90508150037f128d6@mail.gmail.com","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-15T08:18:23Z","receivedAt":"2005-08-15T08:18:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n>> ..., in which case instead of a -k option that does not allow\n>> anything but -ko, making it take an optional single letter\n>> o/k/b/v might might more sense. A single -k defaulting to -ko\n>> is fine by me if you did so, because I think that is the most\n>> useful and usual mode of operation while converting to GIT\n>> repository.\n\n> Anyway, this script has so far followed cvs's own default... which is\n> -kv, and I am generally unwilling to break backwards compatibility.\n\nI realize what I wrote was prone to be misunderstood. What I\nmeant about \"single -k defaulting to -ko\" was this:\n\n cvsimport without -k => cvs default\n cvsimport -k == cvsimport -ko => use cvs -ko\n cvsimoprt -kv => use cvs -kv\n"},{"id":"7284","messageId":"7vr7cv1t89.fsf@assigned-by-dhcp.cox.net","threadId":"1512","inReplyTo":"46a038f905081501301bd9a801@mail.gmail.com","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-15T08:48:06Z","receivedAt":"2005-08-15T08:48:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n> Do you want just -kv or you'd like to handle all the modes?\n\nNo, I do not.\n\nI was just wondering if we are limiting options for people who\nwant to convert their own CVS repositories by always using\neither -kkv or -ko and nothing else. Your simply saying \"I do\nnot think so, -ko is what makes the most sense, and any other\noption does not make any sense, but we used to do -kkv so let's\nleave that as the default and have a -k option that does -ko\" is\nenough for me.\n\nIt is getting late for me so I'll merge it and push it out\ntomorrow when I find time; it will be my day-job day.\n"},{"id":"7287","messageId":"46a038f9050815020511574e3d@mail.gmail.com","threadId":"1512","inReplyTo":"7vr7cv1t89.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2005-08-15T09:05:01Z","receivedAt":"2005-08-15T09:05:01Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 8/15/05, Junio C Hamano wrote:\n> I was just wondering if we are limiting options for people who\n> want to convert their own CVS repositories by always using\n> either -kkv or -ko and nothing else. \n\nI think the other modes are relevant in different scenarios. -kv is\nonly meaningful as file mode over the life of the file in the repo.\n-kk is only meaningful when calling cvs update with -j -j parameters\nor cvs diff, and is effectively a synonim of -ko.\n\nIn the position we are, getting file/revisions out of a repo, there\nare 2 possible files we can get: the one that you'll get with -kkv and\nthe one you'll get with -ko/-kb. -kb/-ko should give us exactly the\nsame file, modulo bugs.\n\nI suspect that in practice -kb is more reliable when it comes to\nbinary files. But to support that the _files() method will need to\nhandle a slightly different protocol mode on the socket, and I rather\nnot mess with it unless I can prove its broken. Talking with cvs\nservers on the socket is not my idea of fun, and there's all sorts of\nversion-specific oddities.\n\ncheers,\n\n\nmartin\n"},{"id":"7297","messageId":"46a038f9050815042036616b08@mail.gmail.com","threadId":"1512","inReplyTo":"46a038f9050815020511574e3d@mail.gmail.com","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2005-08-15T11:20:16Z","receivedAt":"2005-08-15T11:20:16Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 8/15/05, Martin Langhoff wrote:\n> I think the other modes are relevant in different scenarios. -kv is\n> only meaningful as file mode over the life of the file in the repo.\n> -kk is only meaningful when calling cvs update with -j -j parameters\n> or cvs diff, and is effectively a synonim of -ko.\n> \n> In the position we are, getting file/revisions out of a repo, there\n> are 2 possible files we can get: the one that you'll get with -kkv and\n> the one you'll get with -ko/-kb. -kb/-ko should give us exactly the\n> same file, modulo bugs.\n\nAfter a few more trial runs, it ends up being that -kb and -ko drop\nthe ball in some instances, and the most reliable flag to send is -kk.\nDon't ask me how or why.\n\nSo this patch is obsolete too. \n\n\n\nmartin\n"},{"id":"7328","messageId":"7vll32viy5.fsf@assigned-by-dhcp.cox.net","threadId":"1512","inReplyTo":"46a038f9050815042036616b08@mail.gmail.com","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-16T00:12:02Z","receivedAt":"2005-08-16T00:12:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Martin Langhoff writes:\n\n> So this patch is obsolete too. \n\nI take it to mean that it should be dropped and replaced with\nthe one you sent today with -kk change.\n\nHowever, the -kk change one is a corrupted patch and does not\napply. Your MUA ate leading whitespaces, perhaps.\n\nI have already slurped in other two patches to cvsimport in the\nproposed updates branch, so could you kindly proofread them (I\nam no expert on cvs networking protocol issues) and rebase the\n-kk patch, and send it without whitespace corruption this time\naround please?\n\n-jc\n"},{"id":"7359","messageId":"46a038f9050816042949c414c7@mail.gmail.com","threadId":"1512","inReplyTo":"7vll32viy5.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Add -k kill keyword expansion option to git-cvsimport","fromName":"Martin Langhoff","fromEmail":"martin.langhoff@gmail.com","sentAt":"2005-08-16T11:29:15Z","receivedAt":"2005-08-16T11:29:15Z","isPatch":true,"sender":{"key":"martin.langhoff@gmail.com","avatar":"https://gravatar.com/avatar/1e3f311b6c4c15836501901ca58f8c0b0667246488084ba524d8bc9867e22fd9?d=mp&s=160"},"body":"On 8/16/05, Junio C Hamano wrote:\n> However, the -kk change one is a corrupted patch and does not\n> apply. Your MUA ate leading whitespaces, perhaps.\n\nI stupidly did a forward. Rebased to your current pu branch and sent.\nFrom now on I'll be sending straight from cmdline.\n \n> I have already slurped in other two patches to cvsimport in the\n> proposed updates branch, so could you kindly proofread them (I\n> am no expert on cvs networking protocol issues)\n\nProofread. I don't claim to have understood 100% but it made sense.\nMost importantly, it imported several repos perfectly.\n\ncheers,\n\n\nmartin\n"}]}