* [PATCH] Make cvsexportcommit work with filenames containing spaces. @ 2006-09-22 22:35 Robin Rosenberg 2006-09-23 4:17 ` Junio C Hamano 0 siblings, 1 reply; 7+ messages in thread From: Robin Rosenberg @ 2006-09-22 22:35 UTC (permalink / raw) To: git From: Robin Rosenberg <robin.rosenberg@dewire.com> Binary files are except to this so far. --- git-cvsexportcommit.perl | 24 +++++++++++++++-- t/t9200-git-cvsexportcommit.sh | 55 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 76 insertions(+), 3 deletions(-) diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl index 99b3dc3..d78100c 100755 --- a/git-cvsexportcommit.perl +++ b/git-cvsexportcommit.perl @@ -121,7 +121,14 @@ #print @files; $? && die "Error in git-diff-tree"; foreach my $f (@files) { chomp $f; - my @fields = split(m!\s+!, $f); + $f =~ m/^(\S+) (\S+) (\S+) (\S+) (\S+) (.*)/; + my @fields = (); + $fields[++$#fields] = $1; + $fields[++$#fields] = $2; + $fields[++$#fields] = $3; + $fields[++$#fields] = $4; + $fields[++$#fields] = $5; + $fields[++$#fields] = $6; if ($fields[4] eq 'A') { my $path = $fields[5]; push @afiles, $path; @@ -217,7 +224,7 @@ foreach my $f (@bfiles) { } # replace with the new file - `git-cat-file blob $blob > $f`; + `git-cat-file blob $blob > "$f"`; # TODO: something smart with file modes @@ -231,7 +238,18 @@ ## apply non-binary changes my $fuzz = $opt_p ? 0 : 2; print "Patching non-binary files\n"; -print `(git-diff-tree -p $parent -p $commit | patch -p1 -F $fuzz ) 2>&1`; + +my $saveslash = $/; +undef $/; + +open DIFF, "git-diff-tree -p $parent -p $commit|" || die "Cannot diff"; +open PATCH, "|patch -p1 -F $fuzz" || die "Cannot patch"; +my $delta = <DIFF>; +close DIFF || die "Could not diff"; +$delta =~ s/\n(index [^\n]*)\n(--- [^\n]*)\n(\+\+\+ [^\n]*)\n(@@[^\n]*@@)\n/$1\n$2\t\n$3\t\n$4\n/sg; +print PATCH $delta; +close PATCH || die "Could not patch"; +$/ = $saveslash; my $dirtypatch = 0; if (($? >> 8) == 2) { diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh new file mode 100755 index 0000000..1041bb6 --- /dev/null +++ b/t/t9200-git-cvsexportcommit.sh @@ -0,0 +1,55 @@ +#!/bin/bash + +test_description='CVS export comit. + +These tests are ad-hoc ones made to test +some changes, not a complete test.' + +. ./test-lib.sh + +export CVSROOT=$(dirname $(pwd))/cvsroot +export CVSWORK=$(dirname $(pwd))/cvswork +rm -rf $CVSROOT $CVSWORK +mkdir $CVSROOT +cvs init +cvs -Q co -d $CVSWORK . +export GIT_DIR=$(pwd)/.git + +echo >empty && +git add empty && +git commit -a -m "Initial" + +test_expect_success \ + 'New file' \ + 'echo hello >newfile.txt && + git add newfile.txt && + git commit -a -m "Hello" && + id=$(git rev-list --max-count=1 HEAD) && + (cd $CVSWORK && + git cvsexportcommit -c $id && + test $(cat CVS/Entries|wc -l) = 2 + )' + +test_expect_success \ + 'New file with spaces in file name' \ + 'echo ok then >"with spaces.txt" && + git add "with spaces.txt" && \ + git commit -a -m "With spaces" && + id=$(git rev-list --max-count=1 HEAD) && + (cd $CVSWORK && + git-cvsexportcommit.perl -c $id && + test $(cat CVS/Entries|wc -l) = 3 + )' + +test_expect_success \ + 'Update file with spaces in file name' \ + 'echo Ok then >>"with spaces.txt" && + git add "with spaces.txt" && + git commit -a -m "Update with spaces" && + id=$(git rev-list --max-count=1 HEAD) && + (cd $CVSWORK && + git-cvsexportcommit.perl -c $id && + test $(cat CVS/Entries|wc -l) = 3 + )' + +test_done ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Make cvsexportcommit work with filenames containing spaces. 2006-09-22 22:35 [PATCH] Make cvsexportcommit work with filenames containing spaces Robin Rosenberg @ 2006-09-23 4:17 ` Junio C Hamano 2006-09-23 12:27 ` Robin Rosenberg 2006-09-28 23:28 ` Robin Rosenberg 0 siblings, 2 replies; 7+ messages in thread From: Junio C Hamano @ 2006-09-23 4:17 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Robin Rosenberg <robin.rosenberg@dewire.com> writes: > Binary files are except to this so far. Funny. This works on my home machine (CVS 1.12.13) while it does not pass test #2 "with spaces" on another machine that has CVS 1.11.22. > +. ./test-lib.sh > + > +export CVSROOT=$(dirname $(pwd))/cvsroot > +export CVSWORK=$(dirname $(pwd))/cvswork You are creating t/{cvsroot,cvswork} directories. Do not contaminate outside the test/trash directory your tests is started in. > +rm -rf $CVSROOT $CVSWORK People's $(pwd) can contain shell $IFS characters, especially on Cygwin. Be careful and quote them when in doubt. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Make cvsexportcommit work with filenames containing spaces. 2006-09-23 4:17 ` Junio C Hamano @ 2006-09-23 12:27 ` Robin Rosenberg 2006-09-23 19:03 ` Junio C Hamano 2006-09-28 23:28 ` Robin Rosenberg 1 sibling, 1 reply; 7+ messages in thread From: Robin Rosenberg @ 2006-09-23 12:27 UTC (permalink / raw) To: Junio C Hamano; +Cc: git lördag 23 september 2006 06:17 skrev Junio C Hamano: > Robin Rosenberg <robin.rosenberg@dewire.com> writes: > > Binary files are except to this so far. > > Funny. This works on my home machine (CVS 1.12.13) while it > does not pass test #2 "with spaces" on another machine that has > CVS 1.11.22. That must be something else. I've tried with various cvs versions here (including plain 1.11.22) on Mandriva and SLES and it works on all with the lastest git on master (and 1.4.2.1). Could you give me some more info, like the output from -v on the failing machine and your perl and patch version numbers and locale? I'm just guessing what could matter here. Why patch? Well this patch works around (i.e. not perfect) a mismatch between what patch eats and git submits. They are not totally, compatible, and I'm not sure who to blame yet. git emits diff's without timestamps, and what matters to patch, without a TAB before the file timestamp. When patch sees a header like "+++ filename with spaces.txt" it patches "filename". When it sees "+++ filename with spaces.txt<TAB>" if patches "filename with spaces.txt". The real fix would ofcourse be in git diff or patch sometime in the future. > > +. ./test-lib.sh > > + > > +export CVSROOT=$(dirname $(pwd))/cvsroot > > +export CVSWORK=$(dirname $(pwd))/cvswork acknowledged > You are creating t/{cvsroot,cvswork} directories. Do not > contaminate outside the test/trash directory your tests is > started in. > > > +rm -rf $CVSROOT $CVSWORK > > People's $(pwd) can contain shell $IFS characters, especially on > Cygwin. Be careful and quote them when in doubt. Considering this is a patch that should improve on whitespace handlung... ok. I'll resubmit later. -- robin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Make cvsexportcommit work with filenames containing spaces. 2006-09-23 12:27 ` Robin Rosenberg @ 2006-09-23 19:03 ` Junio C Hamano 0 siblings, 0 replies; 7+ messages in thread From: Junio C Hamano @ 2006-09-23 19:03 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Robin Rosenberg <robin.rosenberg@dewire.com> writes: > Why patch? Well this patch works around (i.e. not perfect) a mismatch between > what patch eats and git submits. They are not totally, compatible, and I'm > not sure who to blame yet. git emits diff's without timestamps, and what > matters to patch, without a TAB before the file timestamp. When patch sees a > header like "+++ filename with spaces.txt" it patches "filename". When it > sees "+++ filename with spaces.txt<TAB>" if patches "filename with > spaces.txt". The real fix would ofcourse be in git diff or patch sometime in > the future. Ah, it might be because our diff output do not have trailing TAB (and timestamp) and if CVS uses GNU patch that would be confused and not detect the file being patched. Interestingly, I did a two-patch series to address that issue separately. But that is not even in "next" yet. More interestingly, the version of "GNU patch" that cannot grok our patch (without trailing TAB) for a file that has SP in its name is on my home machine where your patched cvsexportcommit works. I do not know if GNU patch on the other machine does. Will collect necessary info and follow up. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Make cvsexportcommit work with filenames containing spaces. 2006-09-23 4:17 ` Junio C Hamano 2006-09-23 12:27 ` Robin Rosenberg @ 2006-09-28 23:28 ` Robin Rosenberg 2006-09-29 6:37 ` Junio C Hamano 1 sibling, 1 reply; 7+ messages in thread From: Robin Rosenberg @ 2006-09-28 23:28 UTC (permalink / raw) To: Junio C Hamano; +Cc: git [-- Attachment #1: Type: text/plain, Size: 359 bytes --] Jounio. Thanks for the feedback. It is your fedora patch 2.5.4 that doesn't work. It doesn't seem to handle spaces in filenames in any way. Not much I can do about that. Btw, 2.5.4 is nine years old, but strangely that's the one that's around everywhere on source mirrors, but most (=the few ones I checked) Linux distros have 2.5.9 (except RH). -- robin [-- Attachment #2: cvsexportcommit --] [-- Type: text/x-diff, Size: 3929 bytes --] Make cvsexportcommit work with filenames containing spaces. From: Robin Rosenberg <robin.rosenberg@dewire.com> Binary files are except to this so far. Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com> --- git-cvsexportcommit.perl | 27 +++++++++++++++++--- t/t9200-git-cvsexportcommit.sh | 55 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl index 99b3dc3..ecded35 100755 --- a/git-cvsexportcommit.perl +++ b/git-cvsexportcommit.perl @@ -5,6 +5,7 @@ # - cannot add or remove binary files # - does not propagate permissions # - tells "ready for commit" even when things could not be completed # (eg addition of a binary file) +# - Fedora/RHEL uses patch 2.5.4 which doesn't handles spaces in file names use strict; use Getopt::Std; @@ -121,7 +122,14 @@ #print @files; $? && die "Error in git-diff-tree"; foreach my $f (@files) { chomp $f; - my @fields = split(m!\s+!, $f); + $f =~ m/^(\S+) (\S+) (\S+) (\S+) (\S+) (.*)/; + my @fields = (); + $fields[++$#fields] = $1; + $fields[++$#fields] = $2; + $fields[++$#fields] = $3; + $fields[++$#fields] = $4; + $fields[++$#fields] = $5; + $fields[++$#fields] = $6; if ($fields[4] eq 'A') { my $path = $fields[5]; push @afiles, $path; @@ -217,7 +225,7 @@ foreach my $f (@bfiles) { } # replace with the new file - `git-cat-file blob $blob > $f`; + `git-cat-file blob $blob > "$f"`; # TODO: something smart with file modes @@ -231,7 +239,20 @@ ## apply non-binary changes my $fuzz = $opt_p ? 0 : 2; print "Patching non-binary files\n"; -print `(git-diff-tree -p $parent -p $commit | patch -p1 -F $fuzz ) 2>&1`; + +my $saveslash = $/; +undef $/; + +open DIFF, "git-diff-tree -p $parent -p $commit|" || die "Cannot diff"; +open PATCH, "|patch -p1 -F $fuzz" || die "Cannot patch"; +my $delta = <DIFF>; +close DIFF || die "Could not diff"; +unless (defined $ENV{'GIT_CVSEXPORTCOMMIT_NO_SPACES'}) { + $delta =~ s/\n(index [^\n]*)\n(--- [^\n]*)\n(\+\+\+ [^\n]*)\n(@@[^\n]*@@)\n/$1\n$2\t\n$3\t\n$4\n/sg +} +print PATCH $delta; +close PATCH || die "Could not patch"; +$/ = $saveslash; my $dirtypatch = 0; if (($? >> 8) == 2) { diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh new file mode 100755 index 0000000..548e329 --- /dev/null +++ b/t/t9200-git-cvsexportcommit.sh @@ -0,0 +1,55 @@ +#!/bin/bash + +test_description='CVS export comit. + +These tests are ad-hoc ones made to test +some changes, not a complete test.' + +. ./test-lib.sh + +export CVSROOT=$(pwd)/cvsroot +export CVSWORK=$(pwd)/cvswork +rm -rf "$CVSROOT" "$CVSWORK" +mkdir "$CVSROOT" && +cvs init && +cvs -Q co -d "$CVSWORK" . && +export GIT_DIR=$(pwd)/.git && +echo >empty && +git add empty && +git commit -a -m "Initial" 2>/dev/null || +exit 1 + +test_expect_success \ + 'New file' \ + 'echo hello >newfile.txt && + git add newfile.txt && + git commit -a -m "Hello" && + id=$(git rev-list --max-count=1 HEAD) && + (cd "$CVSWORK" && + git cvsexportcommit -c $id && + test $(cat CVS/Entries|wc -l) = 2 + )' + +test_expect_success \ + 'New file with spaces in file name' \ + 'echo ok then >"with spaces.txt" && + git add "with spaces.txt" && \ + git commit -a -m "With spaces" && + id=$(git rev-list --max-count=1 HEAD) && + (cd "$CVSWORK" && + git-cvsexportcommit -c $id && + test $(cat CVS/Entries|wc -l) = 3 + )' + +test_expect_success \ + 'Update file with spaces in file name' \ + 'echo Ok then >>"with spaces.txt" && + git add "with spaces.txt" && + git commit -a -m "Update with spaces" && + id=$(git rev-list --max-count=1 HEAD) && + (cd "$CVSWORK" && + git-cvsexportcommit -c $id && + test $(cat CVS/Entries|wc -l) = 3 + )' + +test_done [-- Attachment #3: cvsexportcommitrm --] [-- Type: text/x-diff, Size: 1352 bytes --] Make cvsexportcommit remove files. From: Robin Rosenberg <robin.rosenberg@dewire.com> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com> --- git-cvsexportcommit.perl | 2 +- t/t9200-git-cvsexportcommit.sh | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletions(-) diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl index ecded35..4fb55a6 100755 --- a/git-cvsexportcommit.perl +++ b/git-cvsexportcommit.perl @@ -143,7 +143,7 @@ foreach my $f (@files) { if ($fields[4] eq 'M') { push @mfiles, $fields[5]; } - if ($fields[4] eq 'R') { + if ($fields[4] eq 'D') { push @dfiles, $fields[5]; } } diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index 548e329..6f28da6 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -52,4 +52,16 @@ test_expect_success \ test $(cat CVS/Entries|wc -l) = 3 )' +test_expect_success \ + 'Remove file with spaces in file name' \ + 'echo Ok then >"with spaces.txt" && + rm -v "with spaces.txt" && \ + git rm "with spaces.txt" && \ + git commit -a -m "Remove file" && + id=$(git rev-list --max-count=1 HEAD) && + (cd "$CVSWORK" && + git-cvsexportcommit -v -c $id && + test $(cat CVS/Entries|wc -l) = 2 + )' + test_done ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Make cvsexportcommit work with filenames containing spaces. 2006-09-28 23:28 ` Robin Rosenberg @ 2006-09-29 6:37 ` Junio C Hamano 2006-09-29 7:25 ` Robin Rosenberg 0 siblings, 1 reply; 7+ messages in thread From: Junio C Hamano @ 2006-09-29 6:37 UTC (permalink / raw) To: Robin Rosenberg; +Cc: git Please do not do more than one patch per e-mail; I'll have to save the attachment in separate files and manually make commits, which is more work. The build procedure for the release to build rpms runs testsuite as part of it, so I need to think a bit how to proceed with this patch. Leaving the test failing on FC5 means I won't be able to cut binary releases. Checking early in the test script to see if "patch" can grok a diff for a file with whitespaces, and skipping the whitespace test if we have a bad "patch", seems to the best workaround. BTW, the addition of TAB to the patch tail is done by "git diff" automatically for files with whitespace in them in the proposed updates version in "pu" (and it is done conditionally only for files whose names have whitespace), so I think your fix will become redundant when it graduates to "master". ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Make cvsexportcommit work with filenames containing spaces. 2006-09-29 6:37 ` Junio C Hamano @ 2006-09-29 7:25 ` Robin Rosenberg 0 siblings, 0 replies; 7+ messages in thread From: Robin Rosenberg @ 2006-09-29 7:25 UTC (permalink / raw) To: Junio C Hamano; +Cc: git fredag 29 september 2006 08:37 skrev Junio C Hamano: > Please do not do more than one patch per e-mail; I'll have to > save the attachment in separate files and manually make commits, > which is more work. > > The build procedure for the release to build rpms runs testsuite > as part of it, so I need to think a bit how to proceed with this > patch. Leaving the test failing on FC5 means I won't be able to > cut binary releases. Checking early in the test script to see > if "patch" can grok a diff for a file with whitespaces, and > skipping the whitespace test if we have a bad "patch", seems to > the best workaround. > > BTW, the addition of TAB to the patch tail is done by "git diff" > automatically for files with whitespace in them in the proposed > updates version in "pu" (and it is done conditionally only for > files whose names have whitespace), so I think your fix will > become redundant when it graduates to "master". But, then I can stop here, because that sounds like a much better solution. My hack is kind-of-kludgy anyway. I'll look in the pu branch. Still want the test cases, although not complete? -- robin ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2006-09-29 7:25 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-09-22 22:35 [PATCH] Make cvsexportcommit work with filenames containing spaces Robin Rosenberg 2006-09-23 4:17 ` Junio C Hamano 2006-09-23 12:27 ` Robin Rosenberg 2006-09-23 19:03 ` Junio C Hamano 2006-09-28 23:28 ` Robin Rosenberg 2006-09-29 6:37 ` Junio C Hamano 2006-09-29 7:25 ` Robin Rosenberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).