{"thread":{"id":"17818","subject":"CVS import","startedAt":"2009-02-16T09:17:57Z","lastAt":"2009-02-25T09:03:48Z","messageCount":26,"participants":["Ferry Huberts (Pelagic)","Johannes Schindelin","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"104918","messageId":"19651.77.61.241.211.1234775877.squirrel@hupie.xs4all.nl","threadId":"17818","inReplyTo":null,"subject":"CVS import","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-16T09:17:57Z","receivedAt":"2009-02-16T09:17:57Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"Hi list,\n\nwhen I try to import a CVS repo with:\n\ngit cvsimport -v -i \\\n  -d :pserver:anonymous@javagroups.cvs.sourceforge.net:/cvsroot/javagroups \\\n  JGroups\n\nI'm getting the errors:\n\nNew tests/other/org/jgroups/tests/adapttcp/Test.java: 5914 bytes\nUse of uninitialized value in concatenation (.) or string at /usr/bin/git-cvsimport line 674, <CVS> line 652.\nUse of uninitialized value in concatenation (.) or string at /usr/bin/git-cvsimport line 674, <CVS> line 652.\nfatal: malformed index info 100666 \tsrc/org/jgroups/util/RWLock.java\nunable to write to git-update-index:  at /usr/bin/git-cvsimport line 679, <CVS> line 652.\n\n\nI've seen this before when trying to import other repositories.\nAnd since I'm not good with Perl I was wondering whether this sounds familiar\nand if there's a fix for it.\n\nI'm on Fedora 10 with the following packages:\ngit.x86_64                                                      1.6.0.6-1.fc10\ngit-all.x86_64                                                  1.6.0.6-1.fc10\ngit-arch.x86_64                                                 1.6.0.6-1.fc10\ngit-cvs.x86_64                                                  1.6.0.6-1.fc10\ngit-daemon.x86_64                                               1.6.0.6-1.fc10\ngit-email.x86_64                                                1.6.0.6-1.fc10\ngit-gui.x86_64                                                  1.6.0.6-1.fc10\ngit-svn.x86_64                                                  1.6.0.6-1.fc10\ngitk.x86_64                                                     1.6.0.6-1.fc10\ngitosis.noarch                                                  0.2-6.20080825git.fc10\ngitweb.x86_64                                                   1.6.0.6-1.fc10\n\n\n\nFerry\n"},{"id":"104941","messageId":"7291.77.61.241.211.1234790434.squirrel@hupie.xs4all.nl","threadId":"17818","inReplyTo":"19651.77.61.241.211.1234775877.squirrel@hupie.xs4all.nl","subject":"Re: CVS import [SOLVED]","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-16T13:20:34Z","receivedAt":"2009-02-16T13:20:34Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"I solved it:\n\nit has to do with the\ncore.autocrlf=input\ncore.safecrlf=true\n\nsettings I had in my global config.\nMaybe the manual page should warn against having these defined?\n\nFerry\n\n\nOn Mon, February 16, 2009 10:17, Ferry Huberts (Pelagic) wrote:\n> Hi list,\n>\n> when I try to import a CVS repo with:\n>\n> git cvsimport -v -i \\\n>   -d :pserver:anonymous@javagroups.cvs.sourceforge.net:/cvsroot/javagroups \\\n>   JGroups\n>\n> I'm getting the errors:\n>\n> New tests/other/org/jgroups/tests/adapttcp/Test.java: 5914 bytes\n> Use of uninitialized value in concatenation (.) or string at /usr/bin/git-cvsimport line 674, <CVS> line 652.\n> Use of uninitialized value in concatenation (.) or string at /usr/bin/git-cvsimport line 674, <CVS> line 652.\n> fatal: malformed index info 100666 \tsrc/org/jgroups/util/RWLock.java\n> unable to write to git-update-index:  at /usr/bin/git-cvsimport line 679, <CVS> line 652.\n>\n>\n> I've seen this before when trying to import other repositories.\n> And since I'm not good with Perl I was wondering whether this sounds familiar\n> and if there's a fix for it.\n>\n> I'm on Fedora 10 with the following packages:\n> git.x86_64                                                      1.6.0.6-1.fc10\n> git-all.x86_64                                                  1.6.0.6-1.fc10\n> git-arch.x86_64                                                 1.6.0.6-1.fc10\n> git-cvs.x86_64                                                  1.6.0.6-1.fc10\n> git-daemon.x86_64                                               1.6.0.6-1.fc10\n> git-email.x86_64                                                1.6.0.6-1.fc10\n> git-gui.x86_64                                                  1.6.0.6-1.fc10\n> git-svn.x86_64                                                  1.6.0.6-1.fc10\n> gitk.x86_64                                                     1.6.0.6-1.fc10\n> gitosis.noarch                                                  0.2-6.20080825git.fc10\n> gitweb.x86_64                                                   1.6.0.6-1.fc10\n>\n>\n>\n> Ferry\n>\n"},{"id":"104944","messageId":"alpine.DEB.1.00.0902161443230.6289@intel-tinevez-2-302","threadId":"17818","inReplyTo":"7291.77.61.241.211.1234790434.squirrel@hupie.xs4all.nl","subject":"Re: CVS import [SOLVED]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-16T13:45:09Z","receivedAt":"2009-02-16T13:45:09Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 16 Feb 2009, Ferry Huberts (Pelagic) wrote:\n\n> I solved it:\n> \n> it has to do with the\n> core.autocrlf=input\n> core.safecrlf=true\n> \n> settings I had in my global config.\n\nThanks!\n\n> Maybe the manual page should warn against having these defined?\n\nMaybe it should be solved differently?  As cvsimport needs to operate with \nautocrlf=false, it seems, it could set that variable when it creates a \nrepository, and check the variable otherwise (erroring out if it is set \ninappropriately)?\n\nCiao,\nDscho\n"},{"id":"104946","messageId":"alpine.DEB.1.00.0902161451210.6289@intel-tinevez-2-302","threadId":"17818","inReplyTo":"alpine.DEB.1.00.0902161443230.6289@intel-tinevez-2-302","subject":"Re: CVS import [SOLVED]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-16T13:53:07Z","receivedAt":"2009-02-16T13:53:07Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 16 Feb 2009, Johannes Schindelin wrote:\n\n> On Mon, 16 Feb 2009, Ferry Huberts (Pelagic) wrote:\n> \n> > I solved it:\n> > \n> > it has to do with the\n> > core.autocrlf=input\n> > core.safecrlf=true\n> > \n> > settings I had in my global config.\n> \n> Thanks!\n> \n> > Maybe the manual page should warn against having these defined?\n> \n> Maybe it should be solved differently?  As cvsimport needs to operate with \n> autocrlf=false, it seems, it could set that variable when it creates a \n> repository, and check the variable otherwise (erroring out if it is set \n> inappropriately)?\n\nIOW something like this:\n\n-- snip --\n git-cvsimport.perl |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex e439202..a27cc94 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -562,12 +562,16 @@ my %index; # holds filenames of one index per branch\n unless (-d $git_dir) {\n \tsystem(\"git-init\");\n \tdie \"Cannot init the GIT db at $git_tree: $?\\n\" if $?;\n+\tsystem(\"git-config core.autocrlf false\");\n+\tdie \"Cannot set core.autocrlf false\" if $?;\n \tsystem(\"git-read-tree\");\n \tdie \"Cannot init an empty tree: $?\\n\" if $?;\n \n \t$last_branch = $opt_o;\n \t$orig_branch = \"\";\n } else {\n+\tdie \"Cannot operate with core.autocrlf other than 'false'\"\n+\t\tif (`git-config --bool core.autocrlf` =~ /true|input/);\n \topen(F, \"git-symbolic-ref HEAD |\") or\n \t\tdie \"Cannot run git-symbolic-ref: $!\\n\";\n \tchomp ($last_branch = <F>);\n-- snap --\n\nIf you could add a test to t9600 and a few words to the man page, that \nwould be awesome.\n\nCiao,\nDscho\n\nP.S.: I think the same strategy should be applied to git-svn...\n"},{"id":"104980","messageId":"4999A378.2040905@pelagic.nl","threadId":"17818","inReplyTo":"alpine.DEB.1.00.0902161451210.6289@intel-tinevez-2-302","subject":"Re: CVS import [SOLVED]","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-16T17:33:44Z","receivedAt":"2009-02-16T17:33:44Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Hi,\n>\n> On Mon, 16 Feb 2009, Johannes Schindelin wrote:\n>\n>   \n>> On Mon, 16 Feb 2009, Ferry Huberts (Pelagic) wrote:\n>>\n>>     \n>>> I solved it:\n>>>\n>>> it has to do with the\n>>> core.autocrlf=input\n>>> core.safecrlf=true\n>>>\n>>> settings I had in my global config.\n>>>       \n>> Thanks!\n>>\n>>     \n>>> Maybe the manual page should warn against having these defined?\n>>>       \n>> Maybe it should be solved differently?  As cvsimport needs to operate with \n>> autocrlf=false, it seems, it could set that variable when it creates a \n>> repository, and check the variable otherwise (erroring out if it is set \n>> inappropriately)?\n>>     \n>\n> IOW something like this:\n>\n> -- snip --\n>  git-cvsimport.perl |    4 ++++\n>  1 files changed, 4 insertions(+), 0 deletions(-)\n>\n> diff --git a/git-cvsimport.perl b/git-cvsimport.perl\n> index e439202..a27cc94 100755\n> --- a/git-cvsimport.perl\n> +++ b/git-cvsimport.perl\n> @@ -562,12 +562,16 @@ my %index; # holds filenames of one index per branch\n>  unless (-d $git_dir) {\n>  \tsystem(\"git-init\");\n>  \tdie \"Cannot init the GIT db at $git_tree: $?\\n\" if $?;\n> +\tsystem(\"git-config core.autocrlf false\");\n> +\tdie \"Cannot set core.autocrlf false\" if $?;\n>  \tsystem(\"git-read-tree\");\n>  \tdie \"Cannot init an empty tree: $?\\n\" if $?;\n>  \n>  \t$last_branch = $opt_o;\n>  \t$orig_branch = \"\";\n>  } else {\n> +\tdie \"Cannot operate with core.autocrlf other than 'false'\"\n> +\t\tif (`git-config --bool core.autocrlf` =~ /true|input/);\n>  \topen(F, \"git-symbolic-ref HEAD |\") or\n>  \t\tdie \"Cannot run git-symbolic-ref: $!\\n\";\n>  \tchomp ($last_branch = <F>);\n> -- snap --\n>\n> If you could add a test to t9600 and a few words to the man page, that \n> would be awesome.\n>\n> Ciao,\n> Dscho\n>\n> P.S.: I think the same strategy should be applied to git-svn...\n>   \nI'm willing to give it a stab but I'm not versed on Perl at all. C and \nJava I can do without breaking a sweat though.\nIsn't it a better idea to have the original authors do this? They \nunderstand the code.\nAlso, doing this would constitute my first patch to git. I'm unfamiliar \nwith its codebase and the requirements of the people that contribute to \nit. Willing to learn though :-)\nMy patches would then probably need some review and would take a bit \nlonger to develop. If that's acceptable then I'm willing to try.\n\nFerry\n"},{"id":"104988","messageId":"alpine.DEB.1.00.0902161909440.6289@intel-tinevez-2-302","threadId":"17818","inReplyTo":"4999A378.2040905@pelagic.nl","subject":"Re: CVS import [SOLVED]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-16T18:11:09Z","receivedAt":"2009-02-16T18:11:09Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 16 Feb 2009, Ferry Huberts (Pelagic) wrote:\n\n> Johannes Schindelin wrote:\n>\n> > On Mon, 16 Feb 2009, Johannes Schindelin wrote:\n> >\n> >   \n> > > On Mon, 16 Feb 2009, Ferry Huberts (Pelagic) wrote:\n> > >\n> > >     \n> > > > I solved it:\n> > > >\n> > > > it has to do with the\n> > > > core.autocrlf=input\n> > > > core.safecrlf=true\n> > > >\n> > > > settings I had in my global config.\n> > > >       \n> > > Thanks!\n> > >\n> > >     \n> > > > Maybe the manual page should warn against having these defined?\n> > > >       \n> > > Maybe it should be solved differently?  As cvsimport needs to operate with\n> > > autocrlf=false, it seems, it could set that variable when it creates a\n> > > repository, and check the variable otherwise (erroring out if it is set\n> > > inappropriately)?\n> > >     \n> >\n> > IOW something like this:\n> >\n> > -- snip --\n> >  git-cvsimport.perl |    4 ++++\n> >  1 files changed, 4 insertions(+), 0 deletions(-)\n> >\n> > diff --git a/git-cvsimport.perl b/git-cvsimport.perl\n> > index e439202..a27cc94 100755\n> > --- a/git-cvsimport.perl\n> > +++ b/git-cvsimport.perl\n> > @@ -562,12 +562,16 @@ my %index; # holds filenames of one index per branch\n> >  unless (-d $git_dir) {\n> >   system(\"git-init\");\n> >   die \"Cannot init the GIT db at $git_tree: $?\\n\" if $?;\n> > +\tsystem(\"git-config core.autocrlf false\");\n> > +\tdie \"Cannot set core.autocrlf false\" if $?;\n> >   system(\"git-read-tree\");\n> >   die \"Cannot init an empty tree: $?\\n\" if $?;\n> >  \n> >   $last_branch = $opt_o;\n> >   $orig_branch = \"\";\n> > } else {\n> > +\tdie \"Cannot operate with core.autocrlf other than 'false'\"\n> > +\t\tif (`git-config --bool core.autocrlf` =~ /true|input/);\n> >   open(F, \"git-symbolic-ref HEAD |\") or\n> >   \tdie \"Cannot run git-symbolic-ref: $!\\n\";\n> > \tchomp ($last_branch = <F>);\n> > -- snap --\n> >\n> > If you could add a test to t9600 and a few words to the man page, that would\n> > be awesome.\n> >\n> > Ciao,\n> > Dscho\n> >\n> > P.S.: I think the same strategy should be applied to git-svn...\n> >   \n> I'm willing to give it a stab but I'm not versed on Perl at all.\n\nWell, the tests and the man page changes I asked you to do don't require \nPerl..\n\n> Isn't it a better idea to have the original authors do this? They \n> understand the code.\n\nNo.  This is Open Source, and it is not their itch.\n\n> Also, doing this would constitute my first patch to git. I'm unfamiliar \n> with its codebase and the requirements of the people that contribute to \n> it. Willing to learn though :-)\n\nThat's the best way to start.\n\n> My patches would then probably need some review and would take a bit \n> longer to develop. If that's acceptable then I'm willing to try.\n\nNo problem, I am here (if you want to cope with me, that is).\n\nCiao,\nDscho\n"},{"id":"105000","messageId":"60044.192.168.0.51.1234816333.squirrel@hupie.xs4all.nl","threadId":"17818","inReplyTo":"7291.77.61.241.211.1234790434.squirrel@hupie.xs4all.nl","subject":"Re: CVS import [SOLVED]","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-16T20:32:13Z","receivedAt":"2009-02-16T20:32:13Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"On Mon, February 16, 2009 14:20, Ferry Huberts (Pelagic) wrote:\n> I solved it:\n>\n> it has to do with the\n> core.autocrlf=input\n> core.safecrlf=true\n>\n> settings I had in my global config.\n> Maybe the manual page should warn against having these defined?\n>\n\nI'm working on it now, and did some more testing: it's actually the safecrlf setting, not the autocrlf option.\nWhich leaves me with some questions of what to do exactly:\n\n autocrlf safecrlf\n1 false    false\n2 false     warn\n3 false     true\n4 input    false\n5 input     warn\n6 input     true\n7 true     false\n8 true      warn\n9 true      true\n\n\n1- git ignores the safecrlf flag; obviously acceptable\n2- git ignores the safecrlf flag; so acceptable\n3- git ignores the safecrlf flag, so acceptable\n4- seems acceptable to me\n5- seems acceptable to me\n6- unacceptable\n7- seems acceptable to me\n8- seems acceptable to me\n9- unacceptable\n\nSo, 6 and 9 (safecrlf==true) are definitely unacceptable. 1-3 are definitely acceptable.\nHow about the others?\nShould these produce warnings?\nShould the user use a 'force' option to make the import work (and acknowledge that he's calling for trouble)\nShould we enforce some setting? Which flags/setting?\n\nInput appreciated :-)\n\nFerry\n"},{"id":"105017","messageId":"alpine.DEB.1.00.0902162157080.6289@intel-tinevez-2-302","threadId":"17818","inReplyTo":"60044.192.168.0.51.1234816333.squirrel@hupie.xs4all.nl","subject":"Re: CVS import [SOLVED]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-16T20:59:29Z","receivedAt":"2009-02-16T20:59:29Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 16 Feb 2009, Ferry Huberts (Pelagic) wrote:\n\n> On Mon, February 16, 2009 14:20, Ferry Huberts (Pelagic) wrote:\n> > I solved it:\n> >\n> > it has to do with the\n> > core.autocrlf=input\n> > core.safecrlf=true\n> >\n> > settings I had in my global config.\n> > Maybe the manual page should warn against having these defined?\n> >\n> \n> I'm working on it now, and did some more testing: it's actually the \n> safecrlf setting, not the autocrlf option.\n\nOh.  That probably means that cvsimport gets confused by the extra \nwarnings.\n\nHowever, I think it is not correct to run cvsimport with autocrlf set to \nanything than false anyway (and safecrlf would not trigger then, right?).\n\nSo IMHO the solution is still to force autocrlf off.\n\nCiao,\nDscho\n"},{"id":"105123","messageId":"50713.77.61.241.211.1234869582.squirrel@hupie.xs4all.nl","threadId":"17818","inReplyTo":"alpine.DEB.1.00.0902162157080.6289@intel-tinevez-2-302","subject":"Re: CVS import [SOLVED]","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-17T11:19:42Z","receivedAt":"2009-02-17T11:19:42Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"Ok.\n\nI tested all combinations of autocrlf and safecrlf on an artificial cvs\nrepository with only a dos text file and a Unix text file. Here are my results.\n\n\n autocrlf safecrlf\n1 false    false\n2 false     warn\n3 false     true\n4 input    false\n5 input     warn\n6 input     true\n7 true     false\n8 true      warn\n9 true      true\n\n\n1- correct import, no warnings\n2- correct import, no warnings\n3- correct import, no warnings\n4- correct import, no warnings\n5- correct import, warning on the dos text file\n6- correct import, no warnings\n7- correct import, no warnings\n8- correct import, no warnings\n9- fail:\n\nInitialized empty Git repository in /data/home.f9/ferry/testarea/cvsimport/wc.git.true.true/.git/\nRunning cvsps...\ncvs_direct initialized to CVSROOT /data/home.f9/ferry/testarea/cvsimport/cvs\ncvs rlog: Logging master\n* UNKNOWN LINE * Branches:\nFetching dos.txt   v 1.1\nNew dos.txt: 25 bytes\nFetching unix.txt   v 1.1\nNew unix.txt: 22 bytes\nfatal: LF would be replaced by CRLF in /tmp/gitcvs.RT1XN8\nUse of uninitialized value $sha in scalar chomp at /usr/bin/git-cvsimport line 928.\nUse of uninitialized value in concatenation (.) or string at /usr/bin/git-cvsimport line 674, <CVS> line 14.\nfatal: malformed index info 100666 \tunix.txt\nunable to write to git-update-index:  at /usr/bin/git-cvsimport line 679, <CVS> line 14.\n\n\nSo 9 crashes while 6 does not. Apparently the artificial repo with the 2 text files\ndoesn't give enough coverage: my problem was with 6.\n\nIt seems that the import script does not detect a fatal from git. It seems to\nme that it does not check the return code because it tries to continue.\nMust be here (from line 923):\n\nprint \"\".($init ? \"New\" : \"Update\").\" $fn: $size bytes\\n\" if $opt_v;\nmy $pid = open(my $F, '-|');\ndie $! unless defined $pid;\nif (!$pid) {\n\texec(\"git-hash-object\", \"-w\", $tmpname)\n\tor die \"Cannot create object: $!\\n\";\n}\nmy $sha = <$F>;\nchomp $sha;\nclose $F;\n\n\nI think the culprit here is git-hash-object. Either it does return a non-zero\nexit code or cvsimport does not see the exit code correctly.\nI've traced it in the code to the file convert.c, function\nstatic void check_safe_crlf(const char *path, int action,\n                            struct text_stat *stats, enum safe_crlf checksafe)\n\nIt does do a 'die' which will exit with code 128, but apparently isn't picked\nup by perl. I'm stuck now as I don't know Perl well enough.\n\n\nBack to the issue:\nI think requiring autocrlf = false is too strict. Requiring autocrlf = false\nshould be enough. That combined with a bit of text in the manual page about\nthese settings: autocrlf = false is strongly recommended. Also, safecrlf is\nrequired to be set to false.\n"},{"id":"105152","messageId":"alpine.DEB.1.00.0902171517180.6185@intel-tinevez-2-302","threadId":"17818","inReplyTo":"50713.77.61.241.211.1234869582.squirrel@hupie.xs4all.nl","subject":"Re: CVS import [SOLVED]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-17T14:18:17Z","receivedAt":"2009-02-17T14:18:17Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 17 Feb 2009, Ferry Huberts (Pelagic) wrote:\n\n> 1- correct import, no warnings\n\nWhen you say \"correct import\", do you mean that the import worked, or that \nthe imported file is actually bytewise identical to what is stored in the \nCVS _repository_ (as opposed to the working directory)?\n\nCiao,\nDscho\n"},{"id":"105161","messageId":"499AD4B8.3040505@pelagic.nl","threadId":"17818","inReplyTo":"alpine.DEB.1.00.0902171517180.6185@intel-tinevez-2-302","subject":"Re: CVS import [SOLVED]","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-17T15:16:08Z","receivedAt":"2009-02-17T15:16:08Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"Johannes Schindelin wrote:\n> Hi,\n>\n> On Tue, 17 Feb 2009, Ferry Huberts (Pelagic) wrote:\n>\n>   \n>> 1- correct import, no warnings\n>>     \n>\n> When you say \"correct import\", do you mean that the import worked, or that \n> the imported file is actually bytewise identical to what is stored in the \n> CVS _repository_ (as opposed to the working directory)?\n>\n>   \nas far as i could tell (but i looked at the checkout, not the \nrepository) it means that the files were imported 1:1. no modifications.\n(which is actually a bit weird as for some of the cases the autocrlf \noption was set to true or input)\n\nFerry\n"},{"id":"105619","messageId":"20090220152849.GA3826@coredump.intra.peff.net","threadId":"17818","inReplyTo":"alpine.DEB.1.00.0902162157080.6289@intel-tinevez-2-302","subject":"Re: CVS import [SOLVED]","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-20T15:28:49Z","receivedAt":"2009-02-20T15:28:49Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 16, 2009 at 09:59:29PM +0100, Johannes Schindelin wrote:\n\n> > I'm working on it now, and did some more testing: it's actually the \n> > safecrlf setting, not the autocrlf option.\n> \n> Oh.  That probably means that cvsimport gets confused by the extra \n> warnings.\n> \n> However, I think it is not correct to run cvsimport with autocrlf set to \n> anything than false anyway (and safecrlf would not trigger then, right?).\n> \n> So IMHO the solution is still to force autocrlf off.\n\nI don't think that's right. What is happening is that git-hash-object is\nbarfing, and git-cvsimport is not properly detecting the error.\nsomething like this (untested) would make that better:\n\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex e439202..65e7990 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -926,6 +926,7 @@ while (<CVS>) {\n \t\t\tmy $sha = <$F>;\n \t\t\tchomp $sha;\n \t\t\tclose $F;\n+\t\t\t$? and die \"hash-object reported failure\";\n \t\t\tmy $mode = pmode($cvs->{'mode'});\n \t\t\tpush(@new,[$mode, $sha, $fn]); # may be resurrected!\n \t\t}\n\nBut the problem is not autocrlf. It is that the combination of \"autocrlf\n= input\" and \"safecrlf\" is nonsensical. Just try this:\n\n  $ git init\n  $ git config core.autocrlf input\n  $ git config core.safecrlf true\n  $ printf 'DOS\\r\\n' >file\n  $ git add file\n  fatal: CRLF would be replaced by LF in file.\n\nwhich makes sense. SafeCRLF is about making sure that the file will be\nthe same on checkin and checkout. But it won't, because we are only\ndoing CRLF conversion half the time.\n\nSo the best workaround is disabling safecrlf, which makes no sense with\nhis autocrlf setting. But I also think safecrlf could be smarter by\ntreating autocrlf=input as autocrlf=true. That is, we don't care if in\nour _particular_ config it will come out the same; we care about whether\none could, if so inclined, get the CRLF's back to create a byte-for-byte\nidentical object.\n\n-Peff\n"},{"id":"105621","messageId":"32886.77.61.241.211.1235147143.squirrel@hupie.xs4all.nl","threadId":"17818","inReplyTo":"20090220152849.GA3826@coredump.intra.peff.net","subject":"Re: CVS import [SOLVED]","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-20T16:25:43Z","receivedAt":"2009-02-20T16:25:43Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"I replied in the thread with something comparable:\nhttp://article.gmane.org/gmane.comp.version-control.git/110358\n\nMy suggestion is make sure that safecrlf is set to false (see the end part of the mail)\n\nFerry\n\nOn Fri, February 20, 2009 16:28, Jeff King wrote:\n> On Mon, Feb 16, 2009 at 09:59:29PM +0100, Johannes Schindelin wrote:\n>\n>> > I'm working on it now, and did some more testing: it's actually the\n>> > safecrlf setting, not the autocrlf option.\n>>\n>> Oh.  That probably means that cvsimport gets confused by the extra\n>> warnings.\n>>\n>> However, I think it is not correct to run cvsimport with autocrlf set to\n>> anything than false anyway (and safecrlf would not trigger then, right?).\n>>\n>> So IMHO the solution is still to force autocrlf off.\n>\n> I don't think that's right. What is happening is that git-hash-object is\n> barfing, and git-cvsimport is not properly detecting the error.\n> something like this (untested) would make that better:\n>\n> diff --git a/git-cvsimport.perl b/git-cvsimport.perl\n> index e439202..65e7990 100755\n> --- a/git-cvsimport.perl\n> +++ b/git-cvsimport.perl\n> @@ -926,6 +926,7 @@ while (<CVS>) {\n>  \t\t\tmy $sha = <$F>;\n>  \t\t\tchomp $sha;\n>  \t\t\tclose $F;\n> +\t\t\t$? and die \"hash-object reported failure\";\n>  \t\t\tmy $mode = pmode($cvs->{'mode'});\n>  \t\t\tpush(@new,[$mode, $sha, $fn]); # may be resurrected!\n>  \t\t}\n>\n> But the problem is not autocrlf. It is that the combination of \"autocrlf\n> = input\" and \"safecrlf\" is nonsensical. Just try this:\n>\n>   $ git init\n>   $ git config core.autocrlf input\n>   $ git config core.safecrlf true\n>   $ printf 'DOS\\r\\n' >file\n>   $ git add file\n>   fatal: CRLF would be replaced by LF in file.\n>\n> which makes sense. SafeCRLF is about making sure that the file will be\n> the same on checkin and checkout. But it won't, because we are only\n> doing CRLF conversion half the time.\n>\n> So the best workaround is disabling safecrlf, which makes no sense with\n> his autocrlf setting. But I also think safecrlf could be smarter by\n> treating autocrlf=input as autocrlf=true. That is, we don't care if in\n> our _particular_ config it will come out the same; we care about whether\n> one could, if so inclined, get the CRLF's back to create a byte-for-byte\n> identical object.\n>\n> -Peff\n>\n"},{"id":"105636","messageId":"20090220172918.GB4636@coredump.intra.peff.net","threadId":"17818","inReplyTo":"32886.77.61.241.211.1235147143.squirrel@hupie.xs4all.nl","subject":"autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-20T17:29:18Z","receivedAt":"2009-02-20T17:29:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 20, 2009 at 05:25:43PM +0100, Ferry Huberts (Pelagic) wrote:\n\n> I replied in the thread with something comparable:\n> http://article.gmane.org/gmane.comp.version-control.git/110358\n> \n> My suggestion is make sure that safecrlf is set to false (see the end\n> part of the mail)\n\nOh, sorry, I missed that bit. You said:\n\n> Back to the issue:\n> I think requiring autocrlf = false is too strict.  Requiring autocrlf\n> = false should be enough. That combined with a bit of text in the\n> manual page about these settings: autocrlf = false is strongly\n> recommended. Also, safecrlf is required to be set to false.\n\nAssuming there is a typo and you meant to say \"Requiring safecrlf =\nfalse should be enough\", then yes, I agree. But if you are recommending\nto put that into the \"git cvsimport\" manpage, I'm not sure that makes\nsense. Setting autocrlf to input and turning on safecrlf breaks much\nmore than that; you can't add any file that has a CRLF in it.  So such a\nwarning should probably go in the config description for those options.\n\nI still think safecrlf could probably be made more useful in this case\nto differentiate between \"this will corrupt your data if you do a\ncheckout with your current config settings\" and \"this will corrupt your\ndata forever\".  But I am not a user of either config variable, so maybe\nthere is some subtlety I'm missing.\n\n-Peff\n"},{"id":"105666","messageId":"499F3B9B.3020709@pelagic.nl","threadId":"17818","inReplyTo":"20090220172918.GB4636@coredump.intra.peff.net","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-20T23:24:11Z","receivedAt":"2009-02-20T23:24:11Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"Jeff King wrote:\n> On Fri, Feb 20, 2009 at 05:25:43PM +0100, Ferry Huberts (Pelagic) wrote:\n>\n>   \n>> I replied in the thread with something comparable:\n>> http://article.gmane.org/gmane.comp.version-control.git/110358\n>>\n>> My suggestion is make sure that safecrlf is set to false (see the end\n>> part of the mail)\n>>     \n>\n> Oh, sorry, I missed that bit. You said:\n>\n>   \n>> Back to the issue:\n>> I think requiring autocrlf = false is too strict.  Requiring autocrlf\n>> = false should be enough. That combined with a bit of text in the\n>> manual page about these settings: autocrlf = false is strongly\n>> recommended. Also, safecrlf is required to be set to false.\n>>     \n>\n> Assuming there is a typo and you meant to say \"Requiring safecrlf =\n> false should be enough\", then yes, I agree. But if you are recommending\n>   \nyes that was a typo.\n> to put that into the \"git cvsimport\" manpage, I'm not sure that makes\n> sense. Setting autocrlf to input and turning on safecrlf breaks much\n> more than that; you can't add any file that has a CRLF in it.  So such a\n> warning should probably go in the config description for those options.\n>\n>   \nI meant that I would add a patch that makes sure that a new repository \nis created with that option set to 'off' and that an existing repository \nwould be checked for that option set to 'off'. I suggested to _also_ add \nremarks about this in the man page of cvsimport. Johannes already \nsuggested a patch but that was for the autocrlf option (trivially \nconverted to the safecrlf option)\n> I still think safecrlf could probably be made more useful in this case\n> to differentiate between \"this will corrupt your data if you do a\n> checkout with your current config settings\" and \"this will corrupt your\n> data forever\".  But I am not a user of either config variable, so maybe\n> there is some subtlety I'm missing.\n>\n> -Peff\n>   \nI'm a user of these options myself. I maintain several large \nrepositories that contain data that is used both on Unix and Windows \nplatforms and that have the autocrlf=input and safecrlf=true. This makes \nsure that everything is in Unix format.\nYour remark about corrupting your data is a bit strong for my taste. \nCorruption from one point of view, making sure that everybody handles \nthe same content from another :-)\n\nFerry\n"},{"id":"105830","messageId":"20090223000840.GA20392@coredump.intra.peff.net","threadId":"17818","inReplyTo":"499F3B9B.3020709@pelagic.nl","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-23T00:08:40Z","receivedAt":"2009-02-23T00:08:40Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 21, 2009 at 12:24:11AM +0100, Ferry Huberts (Pelagic) wrote:\n\n>> I still think safecrlf could probably be made more useful in this case\n>> to differentiate between \"this will corrupt your data if you do a\n>> checkout with your current config settings\" and \"this will corrupt your\n>> data forever\".  But I am not a user of either config variable, so maybe\n>> there is some subtlety I'm missing.\n>\n> I'm a user of these options myself. I maintain several large repositories \n> that contain data that is used both on Unix and Windows platforms and that \n> have the autocrlf=input and safecrlf=true. This makes sure that everything \n> is in Unix format.\n\nOK, so there is some value to that combination, then, I suppose. It\nseems like there must be some easier and more obvious way to say \"reject\nall CRLFs\", but I can't think of one besides setting up a hook (which\nwould work at commit time, not add time).\n\n> Your remark about corrupting your data is a bit strong for my taste.  \n> Corruption from one point of view, making sure that everybody handles the \n> same content from another :-)\n\nI'm not sure you understood what I meant. What I meant is that for some\nset of data, applying CRLF->LF conversion is lossy, and will permanently\ndestroy the ability to restore the original data. For example, arbitrary\nbinary data which contains both CRLF and LF will have all CRLF become\nLF, but you don't know which of the resulting LFs were originally CRLFs,\nand which were just LFs. The data is corrupted, there is no way to get\nback the original, and this is what CRLF is about protecting.\n\nHowever, that safecrlf check is implemented by saying \"with the current\nautocrlf settings, would checkin and checkout get the same file?\". In\nthe case of autocrlf=true, that that exactly prevents the data above\nfrom being corrupted. But with autocrlf=input, it prevents _any_ CRs\nfrom being converted, since checkout will not convert them back. So even\nthough your data is not irretrievable (the transformation _is_\nreversible, you just don't have it enabled), safecrlf is still\ntriggering and refusing the content.\n\nAnd I was suggesting that it might be useful to distinguish between\nthose two situations. Because right now, with autocrlf=input you have\ntwo choices:\n\n  - safecrlf=false, in which you will corrupt mixed CRLF/LF data without\n    any warning\n\n  - safecrlf=true, in which case you are not allowed to check in CR at\n    all\n\nBut there is no choice for \"protect me from actual corruption, but\nconvert text files (i.e., all CRLF)\".\n\nI am a bit concerned about a proposal to set safecrlf=false in all\ncvsimported repositories.  You are turning off the protection against\ncorrupting binary files.  _Even if_ the person has put safecrlf=true\ninto their ~/.gitconfig and thinks they are safe.\n\n-Peff\n"},{"id":"105870","messageId":"2944.77.61.241.211.1235371848.squirrel@hupie.xs4all.nl","threadId":"17818","inReplyTo":"20090223000840.GA20392@coredump.intra.peff.net","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-23T06:50:48Z","receivedAt":"2009-02-23T06:50:48Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"Jeff King wrote:\n> On Sat, Feb 21, 2009 at 12:24:11AM +0100, Ferry Huberts (Pelagic) wrote:\n>\n>>> I still think safecrlf could probably be made more useful in this case to differentiate between \"this will corrupt your data if you do a checkout with your current config settings\" and \"this\nwill corrupt your data forever\".  But I am not a user of either config variable, so maybe there is some subtlety I'm missing.\n>> I'm a user of these options myself. I maintain several large repositories  that contain data that is used both on Unix and Windows platforms and that  have the autocrlf=input and safecrlf=true.\nThis makes sure that everything  is in Unix format.\n>\n> OK, so there is some value to that combination, then, I suppose. It seems like there must be some easier and more obvious way to say \"reject all CRLFs\", but I can't think of one besides setting up\na hook (which would work at commit time, not add time).\n>\n>> Your remark about corrupting your data is a bit strong for my taste.   Corruption from one point of view, making sure that everybody handles the  same content from another :-)\n>\n> I'm not sure you understood what I meant. What I meant is that for some set of data, applying CRLF->LF conversion is lossy, and will permanently destroy the ability to restore the original data.\nFor example, arbitrary binary data which contains both CRLF and LF will have all CRLF become LF, but you don't know which of the resulting LFs were originally CRLFs, and which were just LFs. The\ndata is corrupted, there is no way to get back the original, and this is what CRLF is about protecting.\n>\n> However, that safecrlf check is implemented by saying \"with the current autocrlf settings, would checkin and checkout get the same file?\". In the case of autocrlf=true, that that exactly prevents\nthe data above from being corrupted. But with autocrlf=input, it prevents _any_ CRs from being converted, since checkout will not convert them back. So even though your data is not irretrievable\n(the transformation _is_\n> reversible, you just don't have it enabled), safecrlf is still\n> triggering and refusing the content.\n>\n> And I was suggesting that it might be useful to distinguish between those two situations. Because right now, with autocrlf=input you have two choices:\n>\n>   - safecrlf=false, in which you will corrupt mixed CRLF/LF data without\n>     any warning\n>\n>   - safecrlf=true, in which case you are not allowed to check in CR at\n>     all\n>\n> But there is no choice for \"protect me from actual corruption, but convert text files (i.e., all CRLF)\".\n>\n> I am a bit concerned about a proposal to set safecrlf=false in all cvsimported repositories.  You are turning off the protection against corrupting binary files.  _Even if_ the person has put\nsafecrlf=true into their ~/.gitconfig and thinks they are safe.\n>\n> -Peff\n\nOk.\nI follow and agree. Full circle :-)\nWe're back to Johannes' proposal: make sure that autocrlf is set to false. Agree? If so, then I'll try to whip up a patch.\n\nFerry\n"},{"id":"105871","messageId":"20090223065615.GA24807@coredump.intra.peff.net","threadId":"17818","inReplyTo":"2944.77.61.241.211.1235371848.squirrel@hupie.xs4all.nl","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-23T06:56:15Z","receivedAt":"2009-02-23T06:56:15Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"[Can you please wrap your lines? They seem to be about 200 characters,\n and then rewrap the quoted text, but without putting a '>' marker.\n I had a very hard time figuring out what was quoted and what was not.]\n\nOn Mon, Feb 23, 2009 at 07:50:48AM +0100, Ferry Huberts (Pelagic) wrote:\n\n> > I am a bit concerned about a proposal to set safecrlf=false in all\n> > cvsimported repositories.  You are turning off the protection\n> > against corrupting binary files.  _Even if_ the person has put\n> > safecrlf=true into their ~/.gitconfig and thinks they are safe.\n> \n> Ok.  I follow and agree. Full circle :-) We're back to Johannes'\n> proposal: make sure that autocrlf is set to false. Agree? If so, then\n> I'll try to whip up a patch.\n\nBut won't that now import CRLF's into your new git repo? Especially on\nWindows, where (IIRC) cvs gives you files with CRLF by default?\n\n-Peff\n"},{"id":"105873","messageId":"1570.77.61.241.211.1235372943.squirrel@hupie.xs4all.nl","threadId":"17818","inReplyTo":"20090223065615.GA24807@coredump.intra.peff.net","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-23T07:09:03Z","receivedAt":"2009-02-23T07:09:03Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"On Mon, February 23, 2009 07:56, Jeff King wrote:\n> [Can you please wrap your lines? They seem to be about 200 characters,\n>  and then rewrap the quoted text, but without putting a '>' marker.\n>  I had a very hard time figuring out what was quoted and what was not.]\n>\n> On Mon, Feb 23, 2009 at 07:50:48AM +0100, Ferry Huberts (Pelagic) wrote:\n>\n>> > I am a bit concerned about a proposal to set safecrlf=false in all\n>> > cvsimported repositories.  You are turning off the protection\n>> > against corrupting binary files.  _Even if_ the person has put\n>> > safecrlf=true into their ~/.gitconfig and thinks they are safe.\n>>\n>> Ok.  I follow and agree. Full circle :-) We're back to Johannes'\n>> proposal: make sure that autocrlf is set to false. Agree? If so, then\n>> I'll try to whip up a patch.\n>\n> But won't that now import CRLF's into your new git repo? Especially on\n> Windows, where (IIRC) cvs gives you files with CRLF by default?\n>\n> -Peff\n>\n\nYes it would. But sadly that's the only way to make sure that the import\nwill work (without serious manual intervention).\nI found this out the hard way.\n\nI started these discussions to narrow down on what we should actually patch.\nNow it appears that we should do what Johannes originally proposed. Always\nnice to be right, isn't it Johannes? :-)\n\nFerry\n"},{"id":"105874","messageId":"20090223071056.GA29241@coredump.intra.peff.net","threadId":"17818","inReplyTo":"1570.77.61.241.211.1235372943.squirrel@hupie.xs4all.nl","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-23T07:10:56Z","receivedAt":"2009-02-23T07:10:56Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 23, 2009 at 08:09:03AM +0100, Ferry Huberts (Pelagic) wrote:\n\n> > But won't that now import CRLF's into your new git repo? Especially on\n> > Windows, where (IIRC) cvs gives you files with CRLF by default?\n> \n> Yes it would. But sadly that's the only way to make sure that the import\n> will work (without serious manual intervention).\n> I found this out the hard way.\n\nWouldn't setting autocrlf=true, safecrlf=true do the import you want?\nThen you could reset autocrlf to input after import but before checkout\ntime.\n\n-Peff\n"},{"id":"105877","messageId":"59288.77.61.241.211.1235374197.squirrel@hupie.xs4all.nl","threadId":"17818","inReplyTo":"20090223071056.GA29241@coredump.intra.peff.net","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-23T07:29:57Z","receivedAt":"2009-02-23T07:29:57Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"On Mon, February 23, 2009 08:10, Jeff King wrote:\n> On Mon, Feb 23, 2009 at 08:09:03AM +0100, Ferry Huberts (Pelagic) wrote:\n>\n>> > But won't that now import CRLF's into your new git repo? Especially on\n>> > Windows, where (IIRC) cvs gives you files with CRLF by default?\n>>\n>> Yes it would. But sadly that's the only way to make sure that the import\n>> will work (without serious manual intervention).\n>> I found this out the hard way.\n>\n> Wouldn't setting autocrlf=true, safecrlf=true do the import you want?\n> Then you could reset autocrlf to input after import but before checkout\n> time.\n\nNo. As I demonstrated in my testing setup the combination of autocrlf=true\nand safecrlf=true ALWAYS makes the import NOT work (for repositories that\nhave CRLF files). In my own experience I also found that the combination\nof autocrlf=input and safecrlf=true ALWAYS makes the import NOT work for\nPRATICAL repositories. That lead me to the conclusion to require\nsafecrlf=false. From the discussion and arguments from you it appeared\nthat that wouldn't be enough. Therefore I think that we have to require\nautocrlf=false (which makes git ignore the safecrlf setting).\n\nFerry\n"},{"id":"105981","messageId":"20090224061134.GF4615@coredump.intra.peff.net","threadId":"17818","inReplyTo":"59288.77.61.241.211.1235374197.squirrel@hupie.xs4all.nl","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-24T06:11:34Z","receivedAt":"2009-02-24T06:11:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 23, 2009 at 08:29:57AM +0100, Ferry Huberts (Pelagic) wrote:\n\n> > Wouldn't setting autocrlf=true, safecrlf=true do the import you want?\n> > Then you could reset autocrlf to input after import but before checkout\n> > time.\n> \n> No. As I demonstrated in my testing setup the combination of autocrlf=true\n> and safecrlf=true ALWAYS makes the import NOT work (for repositories that\n> have CRLF files). In my own experience I also found that the combination\n\nOK, sorry I missed that before.\n\nIt actually works fine with CRLF files. It breaks on _LF_ files.  Look\nagain at the output you posted, which shows it barfing while working on\nunix.txt.\n\nThis is the flip-side of the CRLF and autocrlf=input problem; safecrlf\nis protecting us from the case where the file would change on checkout,\nin addition to when it would actually be corrupted.\n\nBut both of those checks (CRLF on autocrlf=input and safecrlf=true, and\nLF on autocrlf=output/true and safecrlf=true) aren't useful to us here;\nwe are not coming from the working tree to git, and worried about\ngetting back. We are munging input coming from cvs, and whatever gets\nput into the working tree is fine (as long as it is not binary\ncorruption).\n\nSo I think the right solution is a relaxed safecrlf mode that protects\nagainst corruption, but not these other cases. And then git-cvsimport\nshould use that.\n\nIn the meantime, detecting the situation is not a bad idea.\n\n> of autocrlf=input and safecrlf=true ALWAYS makes the import NOT work for\n> PRATICAL repositories. That lead me to the conclusion to require\n> safecrlf=false. From the discussion and arguments from you it appeared\n> that that wouldn't be enough. Therefore I think that we have to require\n> autocrlf=false (which makes git ignore the safecrlf setting).\n\nSo yes, in some sense it is safecrlf that is broken. I'm just concerned\nabout tweaking the user's options behind their back. The import can\nhappen differently than they expected no matter which of safecrlf or\nautocrlf you tweak. So I think you are better off to complain and die.\n\n-Peff\n"},{"id":"106012","messageId":"14283.77.61.241.211.1235467512.squirrel@hupie.xs4all.nl","threadId":"17818","inReplyTo":"20090224061134.GF4615@coredump.intra.peff.net","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-24T09:25:12Z","receivedAt":"2009-02-24T09:25:12Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"\n> So yes, in some sense it is safecrlf that is broken. I'm just concerned\n> about tweaking the user's options behind their back. The import can\n> happen differently than they expected no matter which of safecrlf or\n> autocrlf you tweak. So I think you are better off to complain and die.\n\nThe plan was:\n- when creating a new git repo for cvs import: setup safecrlf=false\n- when importing into an existing repo: check whether the safecrlf\n  setting is set to false and crash and burn when not :-)\n  (complain before going up in flames)\n"},{"id":"106141","messageId":"20090225065624.GA1678@sigill.intra.peff.net","threadId":"17818","inReplyTo":"14283.77.61.241.211.1235467512.squirrel@hupie.xs4all.nl","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-25T06:56:24Z","receivedAt":"2009-02-25T06:56:24Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 24, 2009 at 10:25:12AM +0100, Ferry Huberts (Pelagic) wrote:\n\n> > So yes, in some sense it is safecrlf that is broken. I'm just concerned\n> > about tweaking the user's options behind their back. The import can\n> > happen differently than they expected no matter which of safecrlf or\n> > autocrlf you tweak. So I think you are better off to complain and die.\n> \n> The plan was:\n> - when creating a new git repo for cvs import: setup safecrlf=false\n> - when importing into an existing repo: check whether the safecrlf\n>   setting is set to false and crash and burn when not :-)\n>   (complain before going up in flames)\n\nWhy is it OK to silently change the settings in the first case, but not\nthe second? Don't both have the potential to screw up the user's import?\n\nAlso, are settings going to be unset after the first import? If so, then\nfurther incremental imports will fail as described in your second case.\nBut if not, then safecrlf is turned off for that repo, even for\nnon-cvsimport commands, overriding anything in the user's ~/.gitconfig.\nFor somebody doing a one-shot import, they are paying that price without\nany benefit.\n\n-Peff\n"},{"id":"106150","messageId":"45197.77.61.241.211.1235548980.squirrel@hupie.xs4all.nl","threadId":"17818","inReplyTo":"20090225065624.GA1678@sigill.intra.peff.net","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Ferry Huberts (Pelagic)","fromEmail":"ferry.huberts@pelagic.nl","sentAt":"2009-02-25T08:03:00Z","receivedAt":"2009-02-25T08:03:00Z","isPatch":false,"sender":{"key":"ferry.huberts@pelagic.nl","avatar":"https://gravatar.com/avatar/9f63c0289ad23cbdef0f7609a0af85ff0f4b3babfd066de9ff58f62d48cfd6f2?d=mp&s=160"},"body":"\n\nOn Wed, February 25, 2009 07:56, Jeff King wrote:\n> On Tue, Feb 24, 2009 at 10:25:12AM +0100, Ferry Huberts (Pelagic) wrote:\n>\n>> > So yes, in some sense it is safecrlf that is broken. I'm just concerned\n>> > about tweaking the user's options behind their back. The import can\n>> > happen differently than they expected no matter which of safecrlf or\n>> > autocrlf you tweak. So I think you are better off to complain and die.\n>>\n>> The plan was:\n>> - when creating a new git repo for cvs import: setup safecrlf=false\n>> - when importing into an existing repo: check whether the safecrlf\n>>   setting is set to false and crash and burn when not :-)\n>>   (complain before going up in flames)\n>\n> Why is it OK to silently change the settings in the first case, but not\n> the second? Don't both have the potential to screw up the user's import?\n>\n\nthe option would be setup for the import repository only, not global nor system\n\n> Also, are settings going to be unset after the first import? If so, then\n> further incremental imports will fail as described in your second case.\n> But if not, then safecrlf is turned off for that repo, even for\n> non-cvsimport commands, overriding anything in the user's ~/.gitconfig.\n> For somebody doing a one-shot import, they are paying that price without\n> any benefit.\n>\nthis actually makes sense to me. I was only thinking about the continuous\nimport use-case. In that light it would be better to just complain and die\nin the script. I guess I'll just implement that in the patch then.\n\nFerry\n"},{"id":"106179","messageId":"20090225090347.GB15919@coredump.intra.peff.net","threadId":"17818","inReplyTo":"45197.77.61.241.211.1235548980.squirrel@hupie.xs4all.nl","subject":"Re: autocrlf=input and safecrlf (was Re: CVS import [SOLVED])","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-25T09:03:48Z","receivedAt":"2009-02-25T09:03:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Feb 25, 2009 at 09:03:00AM +0100, Ferry Huberts (Pelagic) wrote:\n\n> > Also, are settings going to be unset after the first import? If so, then\n> > further incremental imports will fail as described in your second case.\n> > But if not, then safecrlf is turned off for that repo, even for\n> > non-cvsimport commands, overriding anything in the user's ~/.gitconfig.\n> > For somebody doing a one-shot import, they are paying that price without\n> > any benefit.\n> >\n> this actually makes sense to me. I was only thinking about the continuous\n> import use-case. In that light it would be better to just complain and die\n> in the script. I guess I'll just implement that in the patch then.\n\nOK, that sounds reasonable to me.\n\n-Peff\n"}]}