{"thread":{"id":"5650","subject":"[PATCH] Make cvsexportcommit work with filenames containing spaces.","startedAt":"2006-09-22T22:35:06Z","lastAt":"2006-09-29T07:25:10Z","messageCount":7,"participants":["Robin Rosenberg","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"27421","messageId":"20060922223506.3377.34859.stgit@lathund.dewire.com","threadId":"5650","inReplyTo":null,"subject":"[PATCH] Make cvsexportcommit work with filenames containing spaces.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2006-09-22T22:35:06Z","receivedAt":"2006-09-22T22:35:06Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"From: Robin Rosenberg <robin.rosenberg@dewire.com>\n\nBinary files are except to this so far.\n---\n\n git-cvsexportcommit.perl       |   24 +++++++++++++++--\n t/t9200-git-cvsexportcommit.sh |   55 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 76 insertions(+), 3 deletions(-)\n\ndiff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\nindex 99b3dc3..d78100c 100755\n--- a/git-cvsexportcommit.perl\n+++ b/git-cvsexportcommit.perl\n@@ -121,7 +121,14 @@ #print @files;\n $? && die \"Error in git-diff-tree\";\n foreach my $f (@files) {\n     chomp $f;\n-    my @fields = split(m!\\s+!, $f);\n+    $f =~ m/^(\\S+) (\\S+) (\\S+) (\\S+) (\\S+)\t(.*)/;\n+    my @fields = ();\n+    $fields[++$#fields] = $1;\n+    $fields[++$#fields] = $2;\n+    $fields[++$#fields] = $3;\n+    $fields[++$#fields] = $4;\n+    $fields[++$#fields] = $5;\n+    $fields[++$#fields] = $6;\n     if ($fields[4] eq 'A') {\n         my $path = $fields[5];\n \tpush @afiles, $path;\n@@ -217,7 +224,7 @@ foreach my $f (@bfiles) {\n     }\n \n     # replace with the new file\n-     `git-cat-file blob $blob > $f`;\n+     `git-cat-file blob $blob > \"$f\"`;\n \n     # TODO: something smart with file modes\n \n@@ -231,7 +238,18 @@ ## apply non-binary changes\n my $fuzz = $opt_p ? 0 : 2;\n \n print \"Patching non-binary files\\n\";\n-print `(git-diff-tree -p $parent -p $commit | patch -p1 -F $fuzz ) 2>&1`;\n+\n+my $saveslash = $/;\n+undef $/;\n+\n+open DIFF, \"git-diff-tree -p $parent -p $commit|\" || die \"Cannot diff\";\n+open PATCH, \"|patch -p1 -F $fuzz\" || die \"Cannot patch\";\n+my $delta = <DIFF>;\n+close DIFF || die \"Could not diff\";\n+$delta =~ s/\\n(index [^\\n]*)\\n(--- [^\\n]*)\\n(\\+\\+\\+ [^\\n]*)\\n(@@[^\\n]*@@)\\n/$1\\n$2\\t\\n$3\\t\\n$4\\n/sg;\n+print PATCH $delta;\n+close PATCH || die \"Could not patch\";\n+$/ = $saveslash;\n \n my $dirtypatch = 0;\n if (($? >> 8) == 2) {\ndiff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\nnew file mode 100755\nindex 0000000..1041bb6\n--- /dev/null\n+++ b/t/t9200-git-cvsexportcommit.sh\n@@ -0,0 +1,55 @@\n+#!/bin/bash\n+\n+test_description='CVS export comit. \n+\n+These tests are ad-hoc ones made to test\n+some changes, not a complete test.'\n+\n+. ./test-lib.sh\n+\n+export CVSROOT=$(dirname $(pwd))/cvsroot\n+export CVSWORK=$(dirname $(pwd))/cvswork\n+rm -rf $CVSROOT $CVSWORK\n+mkdir $CVSROOT\n+cvs init\n+cvs -Q co -d $CVSWORK .\n+export GIT_DIR=$(pwd)/.git\n+\n+echo >empty &&\n+git add empty &&\n+git commit -a -m \"Initial\"\n+\n+test_expect_success \\\n+    'New file' \\\n+    'echo hello >newfile.txt &&\n+     git add newfile.txt &&\n+     git commit -a -m \"Hello\" &&\n+     id=$(git rev-list --max-count=1 HEAD) &&\n+     (cd $CVSWORK &&\n+     git cvsexportcommit -c $id &&\n+     test $(cat CVS/Entries|wc -l) = 2\n+     )'\n+\n+test_expect_success \\\n+     'New file with spaces in file name' \\\n+     'echo ok then >\"with spaces.txt\" &&\n+      git add \"with spaces.txt\" && \\\n+      git commit -a -m \"With spaces\" &&\n+      id=$(git rev-list --max-count=1 HEAD) &&\n+      (cd $CVSWORK &&\n+      git-cvsexportcommit.perl -c $id &&\n+      test $(cat CVS/Entries|wc -l) = 3\n+      )'\n+\n+test_expect_success \\\n+     'Update file with spaces in file name' \\\n+     'echo Ok then >>\"with spaces.txt\" &&\n+      git add \"with spaces.txt\" &&\n+      git commit -a -m \"Update with spaces\" &&\n+      id=$(git rev-list --max-count=1 HEAD) &&\n+      (cd $CVSWORK &&\n+      git-cvsexportcommit.perl -c $id &&\n+      test $(cat CVS/Entries|wc -l) = 3\n+      )'\n+     \n+test_done\n"},{"id":"27444","messageId":"7vvenfw727.fsf@assigned-by-dhcp.cox.net","threadId":"5650","inReplyTo":"20060922223506.3377.34859.stgit@lathund.dewire.com","subject":"Re: [PATCH] Make cvsexportcommit work with filenames containing spaces.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-23T04:17:04Z","receivedAt":"2006-09-23T04:17:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> writes:\n\n> Binary files are except to this so far.\n\nFunny.  This works on my home machine (CVS 1.12.13) while it\ndoes not pass test #2 \"with spaces\" on another machine that has\nCVS 1.11.22.\n\n> +. ./test-lib.sh\n> +\n> +export CVSROOT=$(dirname $(pwd))/cvsroot\n> +export CVSWORK=$(dirname $(pwd))/cvswork\n\nYou are creating t/{cvsroot,cvswork} directories.  Do not\ncontaminate outside the test/trash directory your tests is\nstarted in.\n\n> +rm -rf $CVSROOT $CVSWORK\n\nPeople's $(pwd) can contain shell $IFS characters, especially on\nCygwin.   Be careful and quote them when in doubt.\n"},{"id":"27469","messageId":"200609231427.55721.robin.rosenberg@dewire.com","threadId":"5650","inReplyTo":"7vvenfw727.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Make cvsexportcommit work with filenames containing spaces.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2006-09-23T12:27:55Z","receivedAt":"2006-09-23T12:27:55Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"lördag 23 september 2006 06:17 skrev Junio C Hamano:\n> Robin Rosenberg <robin.rosenberg@dewire.com> writes:\n> > Binary files are except to this so far.\n>\n> Funny.  This works on my home machine (CVS 1.12.13) while it\n> does not pass test #2 \"with spaces\" on another machine that has\n> CVS 1.11.22.\nThat must be something else. I've tried with various cvs versions here \n(including plain 1.11.22) on Mandriva and SLES and it works on all with the \nlastest git on master (and 1.4.2.1). \n\nCould you give me some more info, like the output from -v on the failing \nmachine and your perl and patch version numbers and locale? I'm just guessing \nwhat could matter here.  \n\nWhy patch? Well this patch works around (i.e. not perfect) a mismatch between \nwhat patch eats and git submits. They are not totally, compatible, and I'm \nnot sure who to blame yet.  git emits diff's without timestamps, and what \nmatters to patch, without a TAB before the file timestamp. When patch sees a \nheader like \"+++ filename with spaces.txt\" it patches \"filename\". When it \nsees \"+++ filename with spaces.txt<TAB>\" if patches \"filename with \nspaces.txt\". The real fix would ofcourse be in git diff or patch sometime in \nthe future.\n\n> > +. ./test-lib.sh\n> > +\n> > +export CVSROOT=$(dirname $(pwd))/cvsroot\n> > +export CVSWORK=$(dirname $(pwd))/cvswork\nacknowledged\n\n> You are creating t/{cvsroot,cvswork} directories.  Do not\n> contaminate outside the test/trash directory your tests is\n> started in.\n>\n> > +rm -rf $CVSROOT $CVSWORK\n>\n> People's $(pwd) can contain shell $IFS characters, especially on\n> Cygwin.   Be careful and quote them when in doubt.\n\nConsidering this is a patch that should improve on whitespace handlung... ok. \nI'll resubmit later.\n\n-- robin\n"},{"id":"27498","messageId":"7v4puytnh0.fsf@assigned-by-dhcp.cox.net","threadId":"5650","inReplyTo":"200609231427.55721.robin.rosenberg@dewire.com","subject":"Re: [PATCH] Make cvsexportcommit work with filenames containing spaces.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-23T19:03:07Z","receivedAt":"2006-09-23T19:03:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Robin Rosenberg <robin.rosenberg@dewire.com> writes:\n\n> Why patch? Well this patch works around (i.e. not perfect) a mismatch between \n> what patch eats and git submits. They are not totally, compatible, and I'm \n> not sure who to blame yet.  git emits diff's without timestamps, and what \n> matters to patch, without a TAB before the file timestamp. When patch sees a \n> header like \"+++ filename with spaces.txt\" it patches \"filename\". When it \n> sees \"+++ filename with spaces.txt<TAB>\" if patches \"filename with \n> spaces.txt\". The real fix would ofcourse be in git diff or patch sometime in \n> the future.\n\nAh, it might be because our diff output do not have trailing TAB\n(and timestamp) and if CVS uses GNU patch that would be confused\nand not detect the file being patched.\n\nInterestingly, I did a two-patch series to address that issue\nseparately.  But that is not even in \"next\" yet.\n\nMore interestingly, the version of \"GNU patch\" that cannot grok\nour patch (without trailing TAB) for a file that has SP in its\nname is on my home machine where your patched cvsexportcommit\nworks.  I do not know if GNU patch on the other machine does.\n\nWill collect necessary info and follow up.\n"},{"id":"27951","messageId":"200609290128.56217.robin.rosenberg@dewire.com","threadId":"5650","inReplyTo":"7vvenfw727.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Make cvsexportcommit work with filenames containing spaces.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2006-09-28T23:28:55Z","receivedAt":"2006-09-28T23:28:55Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"\nJounio. Thanks for the feedback. It is your fedora patch 2.5.4 that doesn't work. \nIt doesn't seem to handle spaces in filenames in any way. Not much I can do about that.\n\nBtw, 2.5.4 is nine years old, but strangely that's the one that's around everywhere on\nsource mirrors, but most (=the few ones I checked) Linux distros have 2.5.9 (except RH).\n\n-- robin\n\n\nMake cvsexportcommit work with filenames containing spaces.\n\nFrom: Robin Rosenberg <robin.rosenberg@dewire.com>\n\nBinary files are except to this so far.\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n\n git-cvsexportcommit.perl       |   27 +++++++++++++++++---\n t/t9200-git-cvsexportcommit.sh |   55 ++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 79 insertions(+), 3 deletions(-)\n\ndiff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\nindex 99b3dc3..ecded35 100755\n--- a/git-cvsexportcommit.perl\n+++ b/git-cvsexportcommit.perl\n@@ -5,6 +5,7 @@ # - cannot add or remove binary files\n # - does not propagate permissions\n # - tells \"ready for commit\" even when things could not be completed\n #   (eg addition of a binary file)\n+# - Fedora/RHEL uses patch 2.5.4 which doesn't handles spaces in file names\n \n use strict;\n use Getopt::Std;\n@@ -121,7 +122,14 @@ #print @files;\n $? && die \"Error in git-diff-tree\";\n foreach my $f (@files) {\n     chomp $f;\n-    my @fields = split(m!\\s+!, $f);\n+    $f =~ m/^(\\S+) (\\S+) (\\S+) (\\S+) (\\S+)\t(.*)/;\n+    my @fields = ();\n+    $fields[++$#fields] = $1;\n+    $fields[++$#fields] = $2;\n+    $fields[++$#fields] = $3;\n+    $fields[++$#fields] = $4;\n+    $fields[++$#fields] = $5;\n+    $fields[++$#fields] = $6;\n     if ($fields[4] eq 'A') {\n         my $path = $fields[5];\n \tpush @afiles, $path;\n@@ -217,7 +225,7 @@ foreach my $f (@bfiles) {\n     }\n \n     # replace with the new file\n-     `git-cat-file blob $blob > $f`;\n+     `git-cat-file blob $blob > \"$f\"`;\n \n     # TODO: something smart with file modes\n \n@@ -231,7 +239,20 @@ ## apply non-binary changes\n my $fuzz = $opt_p ? 0 : 2;\n \n print \"Patching non-binary files\\n\";\n-print `(git-diff-tree -p $parent -p $commit | patch -p1 -F $fuzz ) 2>&1`;\n+\n+my $saveslash = $/;\n+undef $/;\n+\n+open DIFF, \"git-diff-tree -p $parent -p $commit|\" || die \"Cannot diff\";\n+open PATCH, \"|patch -p1 -F $fuzz\" || die \"Cannot patch\";\n+my $delta = <DIFF>;\n+close DIFF || die \"Could not diff\";\n+unless (defined $ENV{'GIT_CVSEXPORTCOMMIT_NO_SPACES'}) {\n+  $delta =~ s/\\n(index [^\\n]*)\\n(--- [^\\n]*)\\n(\\+\\+\\+ [^\\n]*)\\n(@@[^\\n]*@@)\\n/$1\\n$2\\t\\n$3\\t\\n$4\\n/sg\n+}\n+print PATCH $delta;\n+close PATCH || die \"Could not patch\";\n+$/ = $saveslash;\n \n my $dirtypatch = 0;\n if (($? >> 8) == 2) {\ndiff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\nnew file mode 100755\nindex 0000000..548e329\n--- /dev/null\n+++ b/t/t9200-git-cvsexportcommit.sh\n@@ -0,0 +1,55 @@\n+#!/bin/bash\n+\n+test_description='CVS export comit. \n+\n+These tests are ad-hoc ones made to test\n+some changes, not a complete test.'\n+\n+. ./test-lib.sh\n+\n+export CVSROOT=$(pwd)/cvsroot\n+export CVSWORK=$(pwd)/cvswork\n+rm -rf \"$CVSROOT\" \"$CVSWORK\"\n+mkdir \"$CVSROOT\" &&\n+cvs init &&\n+cvs -Q co -d \"$CVSWORK\" . &&\n+export GIT_DIR=$(pwd)/.git &&\n+echo >empty &&\n+git add empty &&\n+git commit -a -m \"Initial\" 2>/dev/null ||\n+exit 1\n+\n+test_expect_success \\\n+    'New file' \\\n+    'echo hello >newfile.txt &&\n+     git add newfile.txt &&\n+     git commit -a -m \"Hello\" &&\n+     id=$(git rev-list --max-count=1 HEAD) &&\n+     (cd \"$CVSWORK\" &&\n+     git cvsexportcommit -c $id &&\n+     test $(cat CVS/Entries|wc -l) = 2\n+     )'\n+\n+test_expect_success \\\n+     'New file with spaces in file name' \\\n+     'echo ok then >\"with spaces.txt\" &&\n+      git add \"with spaces.txt\" && \\\n+      git commit -a -m \"With spaces\" &&\n+      id=$(git rev-list --max-count=1 HEAD) &&\n+      (cd \"$CVSWORK\" &&\n+      git-cvsexportcommit -c $id &&\n+      test $(cat CVS/Entries|wc -l) = 3\n+      )'\n+\n+test_expect_success \\\n+     'Update file with spaces in file name' \\\n+     'echo Ok then >>\"with spaces.txt\" &&\n+      git add \"with spaces.txt\" &&\n+      git commit -a -m \"Update with spaces\" &&\n+      id=$(git rev-list --max-count=1 HEAD) &&\n+      (cd \"$CVSWORK\" &&\n+      git-cvsexportcommit -c $id &&\n+      test $(cat CVS/Entries|wc -l) = 3\n+      )'\n+     \n+test_done\n\n\nMake cvsexportcommit remove files.\n\nFrom: Robin Rosenberg <robin.rosenberg@dewire.com>\n\nSigned-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>\n---\n\n git-cvsexportcommit.perl       |    2 +-\n t/t9200-git-cvsexportcommit.sh |   12 ++++++++++++\n 2 files changed, 13 insertions(+), 1 deletions(-)\n\ndiff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl\nindex ecded35..4fb55a6 100755\n--- a/git-cvsexportcommit.perl\n+++ b/git-cvsexportcommit.perl\n@@ -143,7 +143,7 @@ foreach my $f (@files) {\n     if ($fields[4] eq 'M') {\n \tpush @mfiles, $fields[5];\n     }\n-    if ($fields[4] eq 'R') {\n+    if ($fields[4] eq 'D') {\n \tpush @dfiles, $fields[5];\n     }\n }\ndiff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\nindex 548e329..6f28da6 100755\n--- a/t/t9200-git-cvsexportcommit.sh\n+++ b/t/t9200-git-cvsexportcommit.sh\n@@ -52,4 +52,16 @@ test_expect_success \\\n       test $(cat CVS/Entries|wc -l) = 3\n       )'\n      \n+test_expect_success \\\n+     'Remove file with spaces in file name' \\\n+     'echo Ok then >\"with spaces.txt\" &&\n+      rm -v \"with spaces.txt\" && \\\n+      git rm \"with spaces.txt\" && \\\n+      git commit -a -m \"Remove file\" &&\n+      id=$(git rev-list --max-count=1 HEAD) &&\n+      (cd \"$CVSWORK\" &&\n+      git-cvsexportcommit -v -c $id &&\n+      test $(cat CVS/Entries|wc -l) = 2\n+      )'\n+     \n test_done\n"},{"id":"27979","messageId":"7vy7s3npo9.fsf@assigned-by-dhcp.cox.net","threadId":"5650","inReplyTo":"200609290128.56217.robin.rosenberg@dewire.com","subject":"Re: [PATCH] Make cvsexportcommit work with filenames containing spaces.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-09-29T06:37:58Z","receivedAt":"2006-09-29T06:37:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Please do not do more than one patch per e-mail; I'll have to\nsave the attachment in separate files and manually make commits,\nwhich is more work.\n\nThe build procedure for the release to build rpms runs testsuite\nas part of it, so I need to think a bit how to proceed with this\npatch.  Leaving the test failing on FC5 means I won't be able to\ncut binary releases.  Checking early in the test script to see\nif \"patch\" can grok a diff for a file with whitespaces, and\nskipping the whitespace test if we have a bad \"patch\", seems to\nthe best workaround.\n\nBTW, the addition of TAB to the patch tail is done by \"git diff\"  \nautomatically for files with whitespace in them in the proposed\nupdates version in \"pu\" (and it is done conditionally only for\nfiles whose names have whitespace), so I think your fix will\nbecome redundant when it graduates to \"master\".\n"},{"id":"27983","messageId":"200609290925.11909.robin.rosenberg@dewire.com","threadId":"5650","inReplyTo":"7vy7s3npo9.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Make cvsexportcommit work with filenames containing spaces.","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg@dewire.com","sentAt":"2006-09-29T07:25:10Z","receivedAt":"2006-09-29T07:25:10Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"fredag 29 september 2006 08:37 skrev Junio C Hamano:\n> Please do not do more than one patch per e-mail; I'll have to\n> save the attachment in separate files and manually make commits,\n> which is more work.\n>\n> The build procedure for the release to build rpms runs testsuite\n> as part of it, so I need to think a bit how to proceed with this\n> patch.  Leaving the test failing on FC5 means I won't be able to\n> cut binary releases.  Checking early in the test script to see\n> if \"patch\" can grok a diff for a file with whitespaces, and\n> skipping the whitespace test if we have a bad \"patch\", seems to\n> the best workaround.\n>\n> BTW, the addition of TAB to the patch tail is done by \"git diff\"\n> automatically for files with whitespace in them in the proposed\n> updates version in \"pu\" (and it is done conditionally only for\n> files whose names have whitespace), so I think your fix will\n> become redundant when it graduates to \"master\".\n\nBut, then I can stop here, because that sounds like a much better solution. My\nhack is kind-of-kludgy anyway. I'll look in the pu branch. Still want the test \ncases, although not complete?\n\n-- robin\n"}]}