Robin Rosenberg[PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
From: Robin Rosenberg <robin.rosenberg@dewire.com>
This patch uses git-apply to do the patching which simplifies the code a lot.
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 <robin.rosenberg@dewire.com> ---
This is a cleaned up version of a previously submitted patch with same name. I'm using non-ascii characters in the
git-cvsexportcommit.perl | 166 +++++++++++++--------------------------- t/t9200-git-cvsexportcommit.sh | 108 +++++++++++++++++++------- 2 files changed, 133 insertions(+), 141 deletions(-)
Show changes to 2 files +133 −141
git-cvsexportcommit.perl, t/t9200-git-cvsexportcommit.sh
diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl index c9d1d88..f1a9c1b 100755 --- a/git-cvsexportcommit.perl +++ b/git-cvsexportcommit.perl @@ -2,9 +2,8 @@ #!/usr/bin/perl -w # 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; @@ -116,16 +115,13 @@ if ($opt_a) { close MSG; my (@afiles, @dfiles, @mfiles, @dirs); -my %amodes; -my @files = safe_pipe_capture('git-diff-tree', '-r', $parent, $commit); -#print @files; +my $files = safe_pipe_capture_blob('git-diff-tree', '-z', '-r', $parent, $commit); $? && die "Error in git-diff-tree"; -foreach my $f (@files) { - chomp $f; - my @fields = split(m!\s+!, $f); +while ($files =~ m/(.*?\000.*?\000)/g) { + my $f=$1; + my @fields = ( $f =~ m/^(\S+) (\S+) (\S+) (\S+) (\S+)\000(.*)\000/ ); if ($fields[4] eq 'A') { my $path = $fields[5]; - $amodes{$path} = $fields[1]; push @afiles, $path; # add any needed parent directories $path = dirname $path; @@ -141,20 +137,27 @@ foreach my $f (@files) { 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; +my (@diff,@binfiles, @abfiles, @dbfiles, @bfiles, @mbfiles); +@diff = safe_pipe_capture('git-diff-tree','--binary','-p',$parent,$commit); +# the --binary format is harder to grok for names of binary files so we execute a new diff +# if it looks like binary files exists to find out +if (grep /^GIT binary patch$/, @diff) { + @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 @abfiles, grep s/^Binary files \/dev\/null and "b\/(.*)" differ$/$1/, @binfiles; + push @dbfiles, grep s/^Binary files "a\/(.*)" and \/dev\/null differ$/$1/, @binfiles; + push @mbfiles, grep s/^Binary files "a\/(.*)" and \"b\/(.*)\" differ$/$1/, @binfiles; + push @bfiles, @abfiles; + push @bfiles, @dbfiles; + push @bfiles, @mbfiles; + map { s/\\([\d]{3})/sprintf('%c',oct $1)/eg } @bfiles; +} $opt_v && print "The commit affects:\n "; $opt_v && print join ("\n ", @afiles,@mfiles,@dfiles) . "\n\n"; -undef @files; # don't need it anymore # check that the files are clean and up to date according to cvs my $dirty; @@ -197,87 +200,32 @@ if ($dirty) { } } -### -### NOTE: if you are planning to die() past this point -### you MUST call cleanupcvs(@files) before die() -### - - -print "Creating new directories\n"; -foreach my $d (@dirs) { - unless (mkdir $d) { - warn "Could not mkdir $d: $!"; - $dirty = 1; - } - `cvs add $d`; - if ($?) { - $dirty = 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"; +print "Patching\n"; -if (scalar(@afiles)+scalar(@dfiles)+scalar(@mfiles) != scalar(@bfiles)) { - print `(git-diff-tree -p $parent -p $commit | patch -p1 -F $fuzz ) 2>&1`; -} +open APPLY, "|GIT_DIR= git-apply -C$fuzz" || die "cannot patch"; +(print APPLY @diff) || die "cannot patch"; +close APPLY || die "cannot patch"; +print "Patch applied successfully. Adding new files and directories to CVS\n"; 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 $d (@dirs) { + if (system('cvs','add',$d)) { + $dirtypatch = 1; + warn "Failed to cvs add directory $d -- you may need to do it manually"; + } } 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,29 +233,29 @@ foreach my $f (@afiles) { 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 { $_; }; } (@afiles, @mfiles, @dfiles); +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); if ($?) { - cleanupcvs(@files); die "Exiting: The commit did not succeed"; } print "Committed successfully to CVS\n"; @@ -321,17 +269,6 @@ END exit(1); } -# ensure cvs is clean before we die -sub cleanupcvs { - my @files = @_; - foreach my $f (@files) { - system('cvs', '-q', 'update', '-C', $f); - if ($?) { - warn "Warning! Failed to cleanup state of $f\n"; - } - } -} - # An alternative to `command` that allows input to be passed as an array # to work around shell problems with weird characters in arguments # if the exec returns non-zero we die @@ -346,12 +283,15 @@ sub safe_pipe_capture { return wantarray ? @output : join('',@output); } -# For any file we want to add to cvs, we must first set its permissions -# properly, *before* the "cvs add ..." command. Otherwise, it is impossible -# to change the permission of the file in the CVS repository using only cvs -# commands. This should be fixed in cvs-1.12.14. -sub set_new_file_permissions { - my ($file, $perm) = @_; - chmod oct($perm), $file - or die "failed to set permissions of \"$file\": $!\n"; +sub safe_pipe_capture_blob { + my $output; + if (my $pid = open my $child, '-|') { + local $/; + undef $/; + $output = (<$child>); + 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 c102479..63eafc8 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 @@ # This test is here because a patch for # 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 � && + echo Foo >�/g�rdets�g�rdet.txt && + git add �/g�rdets�g�rdet.txt && + cp ../test9200a.png �/g�rdets�g�rdet.png && + git add �/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 �/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 + )'
