{"thread":{"id":"13515","subject":"[PATCH 1/2] cvsexportcommit: chomp only removes trailing whitespace","startedAt":"2008-05-14T14:27:05Z","lastAt":"2008-05-15T03:21:09Z","messageCount":9,"participants":["Johannes Schindelin","Junio C Hamano","Robin Rosenberg"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"76944","messageId":"alpine.DEB.1.00.0805141526160.30431@racer","threadId":"13515","inReplyTo":null,"subject":"[PATCH 1/2] cvsexportcommit: chomp only removes trailing whitespace","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-14T14:27:05Z","receivedAt":"2008-05-14T14:27:05Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIn commit fef3a7cc(cvsexportcommit: be graceful when \"cvs status\"\nreorders the arguments), caution was taken to get the status even\nfor files with leading or trailing whitespace.\n\nHowever, the author of that commit missed that chomp() removes only\ntrailing whitespace.  But the author realized his mistake.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tReally my fault.\n\n git-cvsexportcommit.perl |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\nindex b6036bd..3b20bd1 100755\n--- a/git-cvsexportcommit.perl\n+++ b/git-cvsexportcommit.perl\n@@ -211,6 +211,7 @@ if (@canstatusfiles) {\n \n \t$basename = \"no file \" . $basename if (exists($added{$basename}));\n \tchomp($basename);\n+\t$basename =~ s/^\\s+//;\n \n \tif (!exists($fullname{$basename})) {\n \t  $fullname{$basename} = $name;\n-- \n1.5.5.1.375.g1becb\n"},{"id":"76946","messageId":"alpine.DEB.1.00.0805141528370.30431@racer","threadId":"13515","inReplyTo":"alpine.DEB.1.00.0805141526160.30431@racer","subject":"[PATCH 2/2] cvsexportcommit: introduce -W for shared working trees (between Git and CVS)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-14T14:29:49Z","receivedAt":"2008-05-14T14:29:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIf you have a CVS checkout, it is easy to import the CVS history by\ncalling \"git cvsimport\".  However, interacting with the CVS repository\nusing \"git cvsexportcommit\" was cumbersome, since that script assumes\nseparate working directories for Git and CVS.\n\nNow, you can call cvsexportcommit with the -W option.  This will\nautomatically discover the GIT_DIR, and it will check out the parent\ncommit before exporting the commit.\n\nThe intended workflow is this:\n\n$ CVSROOT=$URL cvs co module\n$ cd module\n$ git cvsimport\nhack, hack, hack, making two commits, cleaning them up using rebase -i.\n$ git cvsexportcommit -W -c -p -u HEAD^\n$ git cvsexportcommit -W -c -p -u HEAD\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tThis makes cvsexportcommit slightly more usable for me.  It is\n\tnot (yet) a \"git cvs dcommit\", because it does not perform the\n\ttest what to commit (and possibly commit more than one patch),\n\tand it does not rebase either.\n\n Documentation/git-cvsexportcommit.txt |    7 +++++-\n git-cvsexportcommit.perl              |   35 ++++++++++++++++++++++++++++----\n t/t9200-git-cvsexportcommit.sh        |   17 ++++++++++++++++\n 3 files changed, 53 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git-cvsexportcommit.txt b/Documentation/git-cvsexportcommit.txt\nindex 9a47b4c..b78c9f3 100644\n--- a/Documentation/git-cvsexportcommit.txt\n+++ b/Documentation/git-cvsexportcommit.txt\n@@ -8,7 +8,7 @@ git-cvsexportcommit - Export a single commit to a CVS checkout\n \n SYNOPSIS\n --------\n-'git-cvsexportcommit' [-h] [-u] [-v] [-c] [-P] [-p] [-a] [-d cvsroot] [-w cvsworkdir] [-f] [-m msgprefix] [PARENTCOMMIT] COMMITID\n+'git-cvsexportcommit' [-h] [-u] [-v] [-c] [-P] [-p] [-a] [-d cvsroot] [-w cvsworkdir] [-W] [-f] [-m msgprefix] [PARENTCOMMIT] COMMITID\n \n \n DESCRIPTION\n@@ -67,6 +67,11 @@ OPTIONS\n \toption does not require GIT_DIR to be set before execution if the\n \tcurrent directory is within a git repository.\n \n+-W::\n+\tTell cvsexportcommit that the current working directory is not only\n+\ta Git checkout, but also the CVS checkout.  Therefore, Git will\n+\treset the working directory to the parent commit before proceeding.\n+\n -v::\n \tVerbose.\n \ndiff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\nindex 3b20bd1..7105825 100755\n--- a/git-cvsexportcommit.perl\n+++ b/git-cvsexportcommit.perl\n@@ -7,15 +7,15 @@ use Data::Dumper;\n use File::Basename qw(basename dirname);\n use File::Spec;\n \n-our ($opt_h, $opt_P, $opt_p, $opt_v, $opt_c, $opt_f, $opt_a, $opt_m, $opt_d, $opt_u, $opt_w);\n+our ($opt_h, $opt_P, $opt_p, $opt_v, $opt_c, $opt_f, $opt_a, $opt_m, $opt_d, $opt_u, $opt_w, $opt_W);\n \n-getopts('uhPpvcfam:d:w:');\n+getopts('uhPpvcfam:d:w:W');\n \n $opt_h && usage();\n \n die \"Need at least one commit identifier!\" unless @ARGV;\n \n-if ($opt_w) {\n+if ($opt_w || $opt_W) {\n \t# Remember where GIT_DIR is before changing to CVS checkout\n \tunless ($ENV{GIT_DIR}) {\n \t\t# No GIT_DIR set. Figure it out for ourselves\n@@ -25,7 +25,9 @@ if ($opt_w) {\n \t}\n \t# Make sure GIT_DIR is absolute\n \t$ENV{GIT_DIR} = File::Spec->rel2abs($ENV{GIT_DIR});\n+}\n \n+if ($opt_w) {\n \tif (! -d $opt_w.\"/CVS\" ) {\n \t\tdie \"$opt_w is not a CVS checkout\";\n \t}\n@@ -116,6 +118,15 @@ if ($parent) {\n     }\n }\n \n+my $go_back_to = 0;\n+\n+if ($opt_W) {\n+    $opt_v && print \"Resetting to $parent\\n\";\n+    $go_back_to = `git symbolic-ref HEAD 2> /dev/null ||\n+\tgit rev-parse HEAD` || die \"Could not determine current branch\";\n+    system(\"git checkout -q $parent^0\") && die \"Could not check out $parent^0\";\n+}\n+\n $opt_v && print \"Applying to CVS commit $commit from parent $parent\\n\";\n \n # grab the commit message\n@@ -260,7 +271,11 @@ if ($dirty) {\n }\n \n print \"Applying\\n\";\n-`GIT_DIR= git-apply $context --summary --numstat --apply <.cvsexportcommit.diff` || die \"cannot patch\";\n+if ($opt_W) {\n+    system(\"git checkout -q $commit^0\") && die \"cannot patch\";\n+} else {\n+    `GIT_DIR= git-apply $context --summary --numstat --apply <.cvsexportcommit.diff` || die \"cannot patch\";\n+}\n \n print \"Patch applied successfully. Adding new files and directories to CVS\\n\";\n my $dirtypatch = 0;\n@@ -313,7 +328,9 @@ if ($dirtypatch) {\n     print \"using a patch program. After applying the patch and resolving the\\n\";\n     print \"problems you may commit using:\";\n     print \"\\n    cd \\\"$opt_w\\\"\" if $opt_w;\n-    print \"\\n    $cmd\\n\\n\";\n+    print \"\\n    $cmd\\n\";\n+    print \"\\n    git checkout $go_back_to\\n\" if $go_back_to;\n+    print \"\\n\";\n     exit(1);\n }\n \n@@ -333,6 +350,14 @@ if ($opt_c) {\n # clean up\n unlink(\".cvsexportcommit.diff\");\n \n+if ($opt_W) {\n+    system(\"git checkout $go_back_to\") && die \"cannot move back to $go_back_to\";\n+    if (!($go_back_to =~ /^[0-9a-fA-F]{40}$/)) {\n+\tsystem(\"git symbolic-ref HEAD $go_back_to\") &&\n+\t    die \"cannot move back to $go_back_to\";\n+    }\n+}\n+\n # CVS version 1.11.x and 1.12.x sleeps the wrong way to ensure the timestamp\n # used by CVS and the one set by subsequence file modifications are different.\n # If they are not different CVS will not detect changes.\ndiff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\nindex 42b144b..b1dc32d 100755\n--- a/t/t9200-git-cvsexportcommit.sh\n+++ b/t/t9200-git-cvsexportcommit.sh\n@@ -297,4 +297,21 @@ test_expect_success 'commit a file with leading spaces in the name' '\n \n '\n \n+test_expect_success 'use the same checkout for Git and CVS' '\n+\n+\t(mkdir shared &&\n+\t cd shared &&\n+\t unset GIT_DIR &&\n+\t cvs co . &&\n+\t git init &&\n+\t git add \" space\" &&\n+\t git commit -m \"fake initial commit\" &&\n+\t echo Hello >> \" space\" &&\n+\t git commit -m \"Another change\" \" space\" &&\n+\t git cvsexportcommit -W -p -u -c HEAD &&\n+\t grep Hello \" space\" &&\n+\t git diff-files)\n+\n+'\n+\n test_done\n-- \n1.5.5.1.375.g1becb\n"},{"id":"76964","messageId":"7vskwkojhy.fsf@gitster.siamese.dyndns.org","threadId":"13515","inReplyTo":"alpine.DEB.1.00.0805141526160.30431@racer","subject":"Re: [PATCH 1/2] cvsexportcommit: chomp only removes trailing whitespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-14T17:58:33Z","receivedAt":"2008-05-14T17:58:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> In commit fef3a7cc(cvsexportcommit: be graceful when \"cvs status\"\n> reorders the arguments), caution was taken to get the status even\n> for files with leading or trailing whitespace.\n>\n> However, the author of that commit missed that chomp() removes only\n> trailing whitespace.  But the author realized his mistake.\n>\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n>\n> \tReally my fault.\n\nI am not quite sure if I understand what is going on correctly.\n\nIs this about a filename that has leading or trailing whitespace, or\nlazily not parsing a protocol message but attempting to match with\npossible whitespaces around the place where a filename should be?\n\nIf you are saying that the output from cvs status is so unreliable that we\ncan only strip all whitespaces from both ends and hope for the best\n(e.g. files \" a\" (two leading spaces in the name), \"a \" (two trailing\nspaces in the name), and \"a\" (no such funny spaces) cannot be\ndistinguished from cvs status output), then perhaps you would also need to\nremove as many trailing whitespaces as you can?\n\n\"chomp()\" chomps only a single line terminator, and only if one exists.\n\n        sub foo {\n                my ($a) = @_;\n                chomp($a);\n                print join(\" \", map { sprintf \"%02x\", ord($_) } split(//, $a)), \"\\n\";\n        }\n        foo(\"abc\");    # 61 62 63\n        foo(\"def\\n\");  # 64 65 66\n        foo(\"gh \\n\");  # 67 68 20\n        foo(\"ij\\n\\n\"); # 69 6a 0a\n\n>  git-cvsexportcommit.perl |    1 +\n>  1 files changed, 1 insertions(+), 0 deletions(-)\n>\n> diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\n> index b6036bd..3b20bd1 100755\n> --- a/git-cvsexportcommit.perl\n> +++ b/git-cvsexportcommit.perl\n> @@ -211,6 +211,7 @@ if (@canstatusfiles) {\n>  \n>  \t$basename = \"no file \" . $basename if (exists($added{$basename}));\n>  \tchomp($basename);\n> +\t$basename =~ s/^\\s+//;\n>  \n>  \tif (!exists($fullname{$basename})) {\n>  \t  $fullname{$basename} = $name;\n> -- \n> 1.5.5.1.375.g1becb\n"},{"id":"76971","messageId":"alpine.DEB.1.00.0805141936410.30431@racer","threadId":"13515","inReplyTo":"7vskwkojhy.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2] cvsexportcommit: chomp only removes trailing whitespace","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-14T18:38:55Z","receivedAt":"2008-05-14T18:38:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 May 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > In commit fef3a7cc(cvsexportcommit: be graceful when \"cvs status\" \n> > reorders the arguments), caution was taken to get the status even for \n> > files with leading or trailing whitespace.\n> >\n> > However, the author of that commit missed that chomp() removes only \n> > trailing whitespace.  But the author realized his mistake.\n> >\n> > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> > ---\n> >\n> > \tReally my fault.\n> \n> I am not quite sure if I understand what is going on correctly.\n> \n> Is this about a filename that has leading or trailing whitespace, or \n> lazily not parsing a protocol message but attempting to match with \n> possible whitespaces around the place where a filename should be?\n> \n> If you are saying that the output from cvs status is so unreliable that \n> we can only strip all whitespaces from both ends and hope for the best \n> (e.g. files \" a\" (two leading spaces in the name), \"a \" (two trailing \n> spaces in the name), and \"a\" (no such funny spaces) cannot be \n> distinguished from cvs status output), then perhaps you would also need \n> to remove as many trailing whitespaces as you can?\n\nYes, that is the idea.  The point is: there are at least two different \nimplementations of cvs, and I do not want to rely on a particular one.\n\nTo prevent bad things from happening, the status is checked on a set of \nfiles which have unique names with regard to the chomp()ed name (well, \nwhatever we do to the name, really).\n\nSo yes, this patch needs an update.\n\nWill do so in a couple of hours,\nDscho\n"},{"id":"76985","messageId":"200805142206.53242.robin.rosenberg@dewire.com","threadId":"13515","inReplyTo":"alpine.DEB.1.00.0805141936410.30431@racer","subject":"Re: [PATCH 1/2] cvsexportcommit: chomp only removes trailing whitespace","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2008-05-14T20:06:53Z","receivedAt":"2008-05-14T20:06:53Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"> Yes, that is the idea.  The point is: there are at least two different \n> implementations of cvs, and I do not want to rely on a particular one.\n\nDoes CVSNT add extra spaces?\n\nA question arises. Can we use cvs update instead? It can be used to retrieve the\nlatest version (as the -u flag to cvsexportcommit does) and it will tell you what\nstatus each affected file has. cvs  update -n will just give us the status with unambigous\nfile names.\n\n-- robin\n"},{"id":"76995","messageId":"alpine.DEB.1.00.0805142313280.30431@racer","threadId":"13515","inReplyTo":"200805142206.53242.robin.rosenberg@dewire.com","subject":"Re: [PATCH 1/2] cvsexportcommit: chomp only removes trailing whitespace","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-14T22:15:53Z","receivedAt":"2008-05-14T22:15:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 May 2008, Robin Rosenberg wrote:\n\n> > Yes, that is the idea.  The point is: there are at least two different \n> > implementations of cvs, and I do not want to rely on a particular one.\n> \n> Does CVSNT add extra spaces?\n\nProbably.  At least a CR I would expect.\n\n> A question arises. Can we use cvs update instead? It can be used to \n> retrieve the latest version (as the -u flag to cvsexportcommit does) and \n> it will tell you what status each affected file has. cvs update -n will \n> just give us the status with unambigous file names.\n\nWill it?  The thing is: as far as I can see, cvs is only \"porcelain\", i.e. \nthe output is meant for users' consumption, not scripts'.\n\nSo I wanted to be safe, and strip all leading and trailing whitespace, \ncall \"cvs status\" (if necessary, in several runs, so that no ambiguous \nfile names can be returned).\n\nCiao,\nDscho\n"},{"id":"76996","messageId":"alpine.DEB.1.00.0805142327520.30431@racer","threadId":"13515","inReplyTo":"alpine.DEB.1.00.0805141936410.30431@racer","subject":"[PATCH 1/2 v2] cvsexportcommit: chomp only removes trailing whitespace","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-14T22:30:43Z","receivedAt":"2008-05-14T22:30:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIn commit fef3a7cc(cvsexportcommit: be graceful when \"cvs status\"\nreorders the arguments), caution was taken to get the status even\nfor files with leading or trailing whitespace.\n\nHowever, the author of that commit missed that chomp() removes only\ntrailing newlines.  With help of the mailing list, the author realized\nhis mistake and provided this patch.\n\nThe idea is that we do not want to rely on a certain layout of the \noutput of \"cvs status\".  Therefore we only call it with files that are \nunambiguous after stripping leading and trailing whitespace.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tOn Wed, 14 May 2008, Johannes Schindelin wrote:\n\n\t> To prevent bad things from happening, the status is checked on a \n\t> set of files which have unique names with regard to the chomp()ed\n\t> name (well, whatever we do to the name, really).\n\t> \n\t> So yes, this patch needs an update.\n\t> \n\t> Will do so in a couple of hours,\n\n\tAnd so I did.\n\n git-cvsexportcommit.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\nindex b6036bd..52ba7de 100755\n--- a/git-cvsexportcommit.perl\n+++ b/git-cvsexportcommit.perl\n@@ -210,7 +210,7 @@ if (@canstatusfiles) {\n \tmy $basename = basename($name);\n \n \t$basename = \"no file \" . $basename if (exists($added{$basename}));\n-\tchomp($basename);\n+\t$basename =~ s/^\\s+(.*?)\\s*$/$1/;\n \n \tif (!exists($fullname{$basename})) {\n \t  $fullname{$basename} = $name;\n-- \n1.5.5.1.375.g1becb\n"},{"id":"77012","messageId":"7vk5hwi9o4.fsf@gitster.siamese.dyndns.org","threadId":"13515","inReplyTo":"alpine.DEB.1.00.0805142327520.30431@racer","subject":"Re: [PATCH 1/2 v2] cvsexportcommit: chomp only removes trailing whitespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-05-15T02:27:23Z","receivedAt":"2008-05-15T02:27:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> @@ -210,7 +210,7 @@ if (@canstatusfiles) {\n>  \tmy $basename = basename($name);\n>  \n>  \t$basename = \"no file \" . $basename if (exists($added{$basename}));\n> -\tchomp($basename);\n> +\t$basename =~ s/^\\s+(.*?)\\s*$/$1/;\n\nIsn't this no-op for a basename that does not begin with a whitespace?\nDon't you want to still strip trailing whitespaces in such a case?\n"},{"id":"77013","messageId":"alpine.DEB.1.00.0805150421000.30431@racer","threadId":"13515","inReplyTo":"7vk5hwi9o4.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 1/2 v2] cvsexportcommit: chomp only removes trailing whitespace","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-05-15T03:21:09Z","receivedAt":"2008-05-15T03:21:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 14 May 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > @@ -210,7 +210,7 @@ if (@canstatusfiles) {\n> >  \tmy $basename = basename($name);\n> >  \n> >  \t$basename = \"no file \" . $basename if (exists($added{$basename}));\n> > -\tchomp($basename);\n> > +\t$basename =~ s/^\\s+(.*?)\\s*$/$1/;\n> \n> Isn't this no-op for a basename that does not begin with a whitespace?\n> Don't you want to still strip trailing whitespaces in such a case?\n\nD'oh.  Time for bed.\n\nCiao,\nDscho\n"}]}