From: Robin Rosenberg <robin.rosenberg@xxxxxxxxxx>
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 testd (it did not work before).
Signed-off-by: Robin Rosenberg <robin.rosenberg@xxxxxxxxxx>
---
git-cvsexportcommit.perl | 141 ++++++++++++++++------------------------
t/t9200-git-cvsexportcommit.sh | 79 +++++++++++++++++++---
2 files changed, 124 insertions(+), 96 deletions(-)
diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index 7bac16e..feff681 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,12 +115,18 @@ if ($opt_a) {
close MSG;
my (@afiles, @dfiles, @mfiles, @dirs);
-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;
+ $f =~ m/^(\S+) (\S+) (\S+) (\S+) (\S+)\000(.*)\000/;
+ 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;
@@ -139,12 +144,20 @@ 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 @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;
+map { s/\\([\d]{3})/sprintf('%c',oct $1)/eg; } @abfiles;
+map { s/\\([\d]{3})/sprintf('%c',oct $1)/eg; } @dbfiles;
+map { s/\\([\d]{3})/sprintf('%c',oct $1)/eg; } @mbfiles;
+
push @bfiles, @abfiles;
push @bfiles, @dbfiles;
push @bfiles, @mbfiles;
@@ -152,7 +165,6 @@ push @mfiles, @mbfiles;
$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;
@@ -195,76 +207,36 @@ if ($dirty) {
}
}
-###
-### NOTE: if you are planning to die() past this point
-### you MUST call cleanupcvs(@files) before die()
-###
-
-
+my $dirtypatch = 0;
print "Creating new directories\n";
foreach my $d (@dirs) {
unless (mkdir $d) {
warn "Could not mkdir $d: $!";
- $dirty = 1;
+ $dirtypatch = 1;
}
- `cvs add $d`;
- if ($?) {
- $dirty = 1;
+ print "Adding directory $d";
+ 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";
+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`;
-}
-
-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;
+ `git-diff-tree --binary -z -p $parent -p $commit >.cvsexportcommit.diff`;
+ if ($?) {
+ die("cannot diff");
+ }
+ `GIT_DIR= git-apply -z -C$fuzz .cvsexportcommit.diff`;
+ if ($?) {
+ $dirtypatch = 1;
+ } else {
+ $opt_v || unlink(".cvsexportcommit.diff");
+ }
}
foreach my $f (@afiles) {
@@ -274,7 +246,7 @@ foreach my $f (@afiles) {
system('cvs', 'add', $f);
}
if ($?) {
- $dirty = 1;
+ $dirtypatch = 1;
warn "Failed to cvs add $f -- you may need to do it manually";
}
}
@@ -282,19 +254,21 @@ 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: $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);
}
@@ -304,7 +278,6 @@ 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";
@@ -318,17 +291,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
@@ -342,3 +304,16 @@ sub safe_pipe_capture {
}
return wantarray ? @output : join('',@output);
}
+
+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 6e566d4..75b9f38 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,4 +141,58 @@ test_expect_success \
diff F/newfile6.png ../F/newfile6.png
)'
+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_done
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html