From e86ad71fe5f53ae4434566bd09ea4256090e5a3a Mon Sep 17 00:00:00 2001 From: Robin Rosenberg Date: Mon, 11 Dec 2006 00:30:06 +0100 Subject: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters. This patch uses git-apply to do the patching which simplifies the code a lot and also uses one pass to git-diff. git-apply gives information on added, removed files as well as which files are binary. Removed the test for checking for matching binary files when deleting them since git-apply happily deletes the file. This is matter of taste since we allow some fuzz for text patches also. Error handling was cleaned up, but not much tested. Signed-off-by: Robin Rosenberg Signed-off-by: Junio C Hamano --- git-cvsexportcommit.perl | 203 +++++++++++---------------------- t/t9200-git-cvsexportcommit.sh | 108 +++++++++++++----- 2 files changed, 145 insertions(+), 166 deletions(-) diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl index c9d1d88f2e..4863c91fe3 100755 --- a/git-cvsexportcommit.perl +++ b/git-cvsexportcommit.perl @@ -2,9 +2,8 @@ # Known limitations: # - does not propagate permissions -# - tells "ready for commit" even when things could not be completed -# (not sure this is true anymore, more testing is needed) -# - does not handle whitespace in pathnames at all. +# - error handling has not been extensively tested +# use strict; use Getopt::Std; @@ -115,49 +114,40 @@ } close MSG; -my (@afiles, @dfiles, @mfiles, @dirs); -my %amodes; -my @files = safe_pipe_capture('git-diff-tree', '-r', $parent, $commit); -#print @files; -$? && die "Error in git-diff-tree"; -foreach my $f (@files) { - chomp $f; - my @fields = split(m!\s+!, $f); - if ($fields[4] eq 'A') { - my $path = $fields[5]; - $amodes{$path} = $fields[1]; - push @afiles, $path; - # add any needed parent directories - $path = dirname $path; - while (!-d $path and ! grep { $_ eq $path } @dirs) { - unshift @dirs, $path; - $path = dirname $path; - } - } - if ($fields[4] eq 'M') { - push @mfiles, $fields[5]; - } - if ($fields[4] eq 'D') { - push @dfiles, $fields[5]; - } -} -my (@binfiles, @abfiles, @dbfiles, @bfiles, @mbfiles); -@binfiles = grep m/^Binary files/, safe_pipe_capture('git-diff-tree', '-p', $parent, $commit); -map { chomp } @binfiles; -@abfiles = grep s/^Binary files \/dev\/null and b\/(.*) differ$/$1/, @binfiles; -@dbfiles = grep s/^Binary files a\/(.*) and \/dev\/null differ$/$1/, @binfiles; -@mbfiles = grep s/^Binary files a\/(.*) and b\/(.*) differ$/$1/, @binfiles; -push @bfiles, @abfiles; -push @bfiles, @dbfiles; -push @bfiles, @mbfiles; -push @mfiles, @mbfiles; +`git-diff-tree --binary -p $parent $commit >.cvsexportcommit.diff`;# || die "Cannot diff"; -$opt_v && print "The commit affects:\n "; -$opt_v && print join ("\n ", @afiles,@mfiles,@dfiles) . "\n\n"; -undef @files; # don't need it anymore +## apply non-binary changes +my $fuzz = $opt_p ? 0 : 2; + +print "Checking if patch will apply\n"; + +my @stat; +open APPLY, "GIT_DIR= git-apply -C$fuzz --binary --summary --numstat<.cvsexportcommit.diff|" || die "cannot patch"; +@stat=; +close APPLY || die "Cannot patch"; +my (@bfiles,@files,@afiles,@dfiles); +chomp @stat; +foreach (@stat) { + push (@bfiles,$1) if m/^-\t-\t(.*)$/; + push (@files, $1) if m/^-\t-\t(.*)$/; + push (@files, $1) if m/^\d+\t\d+\t(.*)$/; + push (@afiles,$1) if m/^ create mode [0-7]+ (.*)$/; + push (@dfiles,$1) if m/^ delete mode [0-7]+ (.*)$/; +} +map { s/^"(.*)"$/$1/g } @bfiles,@files; +map { s/\\([0-7]{3})/sprintf('%c',oct $1)/eg } @bfiles,@files; # check that the files are clean and up to date according to cvs my $dirty; +my @dirs; +foreach my $p (@afiles) { + my $path = dirname $p; + while (!-d $path and ! grep { $_ eq $path } @dirs) { + unshift @dirs, $path; + $path = dirname $path; + } +} + foreach my $d (@dirs) { if (-e $d) { $dirty = 1; @@ -180,7 +170,8 @@ } } -foreach my $f (@mfiles, @dfiles) { +foreach my $f (@files) { + next if grep { $_ eq $f } @afiles; # TODO:we need to handle removed in cvs my @status = grep(m/^File/, safe_pipe_capture('cvs', '-q', 'status' ,$f)); if (@status > 1) { warn 'Strange! cvs status returned more than one line?'}; @@ -197,87 +188,26 @@ } } -### -### NOTE: if you are planning to die() past this point -### you MUST call cleanupcvs(@files) before die() -### +print "Applying\n"; +`GIT_DIR= git-apply -C$fuzz --binary --summary --numstat --apply <.cvsexportcommit.diff` || die "cannot patch"; - -print "Creating new directories\n"; +print "Patch applied successfully. Adding new files and directories to CVS\n"; +my $dirtypatch = 0; foreach my $d (@dirs) { - unless (mkdir $d) { - warn "Could not mkdir $d: $!"; - $dirty = 1; - } - `cvs add $d`; - if ($?) { - $dirty = 1; + if (system('cvs','add',$d)) { + $dirtypatch = 1; warn "Failed to cvs add directory $d -- you may need to do it manually"; } } -print "'Patching' binary files\n"; - -foreach my $f (@bfiles) { - # check that the file in cvs matches the "old" file - # extract the file to $tmpdir and compare with cmp - if (not(grep { $_ eq $f } @afiles)) { - my $tree = safe_pipe_capture('git-rev-parse', "$parent^{tree}"); - chomp $tree; - my $blob = `git-ls-tree $tree "$f" | cut -f 1 | cut -d ' ' -f 3`; - chomp $blob; - `git-cat-file blob $blob > $tmpdir/blob`; - if (system('cmp', '-s', $f, "$tmpdir/blob")) { - warn "Binary file $f in CVS does not match parent.\n"; - if (not $opt_f) { - $dirty = 1; - next; - } - } - } - if (not(grep { $_ eq $f } @dfiles)) { - my $tree = safe_pipe_capture('git-rev-parse', "$commit^{tree}"); - chomp $tree; - my $blob = `git-ls-tree $tree "$f" | cut -f 1 | cut -d ' ' -f 3`; - chomp $blob; - # replace with the new file - `git-cat-file blob $blob > $f`; - } - - # TODO: something smart with file modes - -} -if ($dirty) { - cleanupcvs(@files); - die "Exiting: Binary files in CVS do not match parent"; -} - -## apply non-binary changes -my $fuzz = $opt_p ? 0 : 2; - -print "Patching non-binary files\n"; - -if (scalar(@afiles)+scalar(@dfiles)+scalar(@mfiles) != scalar(@bfiles)) { - print `(git-diff-tree -p $parent -p $commit | patch -p1 -F $fuzz ) 2>&1`; -} - -my $dirtypatch = 0; -if (($? >> 8) == 2) { - cleanupcvs(@files); - die "Exiting: Patch reported serious trouble -- you will have to apply this patch manually"; -} elsif (($? >> 8) == 1) { # some hunks failed to apply - $dirtypatch = 1; -} - foreach my $f (@afiles) { - set_new_file_permissions($f, $amodes{$f}); if (grep { $_ eq $f } @bfiles) { system('cvs', 'add','-kb',$f); } else { system('cvs', 'add', $f); } if ($?) { - $dirty = 1; + $dirtypatch = 1; warn "Failed to cvs add $f -- you may need to do it manually"; } } @@ -285,35 +215,40 @@ foreach my $f (@dfiles) { system('cvs', 'rm', '-f', $f); if ($?) { - $dirty = 1; + $dirtypatch = 1; warn "Failed to cvs rm -f $f -- you may need to do it manually"; } } print "Commit to CVS\n"; -print "Patch: $title\n"; -my $commitfiles = join(' ', @afiles, @mfiles, @dfiles); -my $cmd = "cvs commit -F .msg $commitfiles"; +print "Patch title (first comment line): $title\n"; +my @commitfiles = map { unless (m/\s/) { '\''.$_.'\''; } else { $_; }; } (@files); +my $cmd = "cvs commit -F .msg @commitfiles"; if ($dirtypatch) { print "NOTE: One or more hunks failed to apply cleanly.\n"; - print "Resolve the conflicts and then commit using:\n"; + print "You'll need to apply the patch in .cvsexportcommit.diff manually\n"; + print "using a patch program. After applying the patch and resolving the\n"; + print "problems you may commit using:"; print "\n $cmd\n\n"; exit(1); } - if ($opt_c) { print "Autocommit\n $cmd\n"; - print safe_pipe_capture('cvs', 'commit', '-F', '.msg', @afiles, @mfiles, @dfiles); + print safe_pipe_capture('cvs', 'commit', '-F', '.msg', @files); if ($?) { - cleanupcvs(@files); die "Exiting: The commit did not succeed"; } print "Committed successfully to CVS\n"; } else { print "Ready for you to commit, just run:\n\n $cmd\n"; } + +# clean up +unlink(".cvsexportcommit.diff"); +unlink(".msg"); + sub usage { print STDERR <); + close $child or die join(' ',@_).": $! $?"; + } else { + exec(@_) or die "$! $?"; # exec() can fail the executable can't be found + } + return $output; } diff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh index c1024790e4..ca0513b162 100755 --- a/t/t9200-git-cvsexportcommit.sh +++ b/t/t9200-git-cvsexportcommit.sh @@ -89,18 +89,17 @@ test_expect_success \ ! git cvsexportcommit -c $id )' -# Should fail, but only on the git-cvsexportcommit stage -test_expect_success \ - 'Fail to remove binary file more than one generation old' \ - 'git reset --hard HEAD^ && - cat F/newfile6.png >>D/newfile4.png && - git commit -a -m "generation 2 (again)" && - rm -f D/newfile4.png && - git commit -a -m "generation 3" && - id=$(git rev-list --max-count=1 HEAD) && - (cd "$CVSWORK" && - ! git cvsexportcommit -c $id - )' +#test_expect_success \ +# 'Fail to remove binary file more than one generation old' \ +# 'git reset --hard HEAD^ && +# cat F/newfile6.png >>D/newfile4.png && +# git commit -a -m "generation 2 (again)" && +# rm -f D/newfile4.png && +# git commit -a -m "generation 3" && +# id=$(git rev-list --max-count=1 HEAD) && +# (cd "$CVSWORK" && +# ! git cvsexportcommit -c $id +# )' # We reuse the state from two tests back here @@ -108,7 +107,7 @@ test_expect_success \ # fail with gnu patch, so cvsexportcommit must handle that. test_expect_success \ 'Remove only binary files' \ - 'git reset --hard HEAD^^^ && + 'git reset --hard HEAD^^ && rm -f D/newfile4.png && git commit -a -m "test: remove only a binary file" && id=$(git rev-list --max-count=1 HEAD) && @@ -142,20 +141,73 @@ test_expect_success \ diff F/newfile6.png ../F/newfile6.png )' -test_expect_success 'Retain execute bit' ' - mkdir G && - echo executeon >G/on && - chmod +x G/on && - echo executeoff >G/off && - git add G/on && - git add G/off && - git commit -a -m "Execute test" && - ( - cd "$CVSWORK" && - git-cvsexportcommit -c HEAD - test -x G/on && - ! test -x G/off - ) -' +test_expect_success \ + 'New file with spaces in file name' \ + 'mkdir "G g" && + echo ok then >"G g/with spaces.txt" && + git add "G g/with spaces.txt" && \ + cp ../test9200a.png "G g/with spaces.png" && \ + git add "G g/with spaces.png" && + git commit -a -m "With spaces" && + id=$(git rev-list --max-count=1 HEAD) && + (cd "$CVSWORK" && + git-cvsexportcommit -c $id && + test "$(echo $(sort "G g/CVS/Entries"|cut -d/ -f2,3,5))" = "with spaces.png/1.1/-kb with spaces.txt/1.1/" + )' + +test_expect_success \ + 'Update file with spaces in file name' \ + 'echo Ok then >>"G g/with spaces.txt" && + cat ../test9200a.png >>"G g/with spaces.png" && \ + git add "G g/with spaces.png" && + git commit -a -m "Update with spaces" && + id=$(git rev-list --max-count=1 HEAD) && + (cd "$CVSWORK" && + git-cvsexportcommit -c $id + test "$(echo $(sort "G g/CVS/Entries"|cut -d/ -f2,3,5))" = "with spaces.png/1.2/-kb with spaces.txt/1.2/" + )' + +# This test contains ISO-8859-1 characters +test_expect_success \ + 'File with non-ascii file name' \ + 'mkdir -p Å/goo/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z/å/ä/ö && + echo Foo >Å/goo/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z/å/ä/ö/gårdetsågårdet.txt && + git add Å/goo/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z/å/ä/ö/gårdetsågårdet.txt && + cp ../test9200a.png Å/goo/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z/å/ä/ö/gårdetsågårdet.png && + git add Å/goo/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z/å/ä/ö/gårdetsågårdet.png && + git commit -a -m "Går det så går det" && \ + id=$(git rev-list --max-count=1 HEAD) && + (cd "$CVSWORK" && + git-cvsexportcommit -v -c $id && + test "$(echo $(sort Å/goo/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z/å/ä/ö/CVS/Entries|cut -d/ -f2,3,5))" = "gårdetsågårdet.png/1.1/-kb gårdetsågårdet.txt/1.1/" + )' + +test_expect_success \ + 'Mismatching patch should fail' \ + 'date >>"E/newfile5.txt" && + git add "E/newfile5.txt" && + git commit -a -m "Update one" && + date >>"E/newfile5.txt" && + git add "E/newfile5.txt" && + git commit -a -m "Update two" && + id=$(git rev-list --max-count=1 HEAD) && + (cd "$CVSWORK" && + ! git-cvsexportcommit -c $id + )' + +test_expect_success \ + 'Retain execute bit' \ + 'mkdir G && + echo executeon >G/on && + chmod +x G/on && + echo executeoff >G/off && + git add G/on && + git add G/off && + git commit -a -m "Execute test" && + (cd "$CVSWORK" && + git-cvsexportcommit -c HEAD + test -x G/on && + ! test -x G/off + )' test_done