Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.

4 messages, 2 authors, 2016-08-11 · open the first message on its own page

Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.

From: Junio C Hamano <hidden>
Date: 2016-08-11 19:26:39

Robin Rosenberg [off-list ref] writes:
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.
Interesting.

I think you should be able to generate the patchfile once, and
use git-apply to figure out additions, deletions and binaryness,
and then use the same patchfile to apply the changes.  Currently
checking for binaryness is not easy with git-apply, so we would
want to fix git-apply first, instead of forcing you to have a
change like this:

   # 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);

which is way too ugly.

	... goes to look and comes back, with a big grin ...

Well, have you tried this?

	git diff-tree -p --binary fe142b3a | git apply --summary --numstat

The numstat part would let you see the binaryness, so we do not
have to "fix" git-apply.

Another thing that _might_ be interesting is to use rename
detection when preparing the patch, and make the matching rename
on the CVS side, but I do not recall the details of how one
would make CVS pretend to support renamed paths ;-).  I think it
involved copying the ,v file to a new name, and marking the
older revisions in that new ,v file as nonexistent or something
like that, but I did it only in my distant past and forgot the
details.

By the way, I am not sure if giving fuzz by default is such a
good idea, though.

Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.

From: Robin Rosenberg <hidden>
Date: 2016-08-11 19:33:56

söndag 10 december 2006 02:18 skrev Junio C Hamano:
Robin Rosenberg [off-list ref] writes:
quoted
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.
Interesting.

I think you should be able to generate the patchfile once, and
use git-apply to figure out additions, deletions and binaryness,
and then use the same patchfile to apply the changes.  Currently
checking for binaryness is not easy with git-apply, so we would
want to fix git-apply first, instead of forcing you to have a
change like this:

   # 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);

which is way too ugly.

	... goes to look and comes back, with a big grin ...
<grin> apparently
Well, have you tried this?

	git diff-tree -p --binary fe142b3a | git apply --summary --numstat
of course not. I didn't understand it. Why can't it tell me about removed
binary files, so I could remove the git-diff-tree invocation to find out about 
added/removed files?
The numstat part would let you see the binaryness, so we do not
have to "fix" git-apply.

Another thing that _might_ be interesting is to use rename
detection when preparing the patch, and make the matching rename
on the CVS side, but I do not recall the details of how one
would make CVS pretend to support renamed paths ;-).  I think it
involved copying the ,v file to a new name, and marking the
older revisions in that new ,v file as nonexistent or something
like that, but I did it only in my distant past and forgot the
details.
In server mode, which is the normal way of using CVS you cannot
do this with the CVS most of us are used with. CvsNT does support 
a rename command, I think, but I don't use it, partly due to rumors of it 
being somewhat unstable. If there's any truth in that, I don't know.

Anyway, I don't practice the rename trick in CVS myself. I'm not  sure how 
that would work with the roundtripping via CVS that I do.  cvsimport 
detects "renames" with the current approach so I'm happy as is, not that I 
rename files much.  I also think there than a copy is needed to play nice, 
like removing old tags from the copy, how about rename back and forth etc. 
There's a reason people want to migrate from CVS, as well as there are 
reasons not to hurry. CVS doesn't support rename, it's that simple.
By the way, I am not sure if giving fuzz by default is such a
good idea, though.
It was in the original. I don't know why. Maybe the original author can tell 
us why it was important. It may be problematic to stay fully in sync with a 
CVS repo because you have to git-cvsimport it first and that takes some time. 
This fuzz gives some, but not much slack. Reverting the option could be a 
good idea.

Update follows.

[PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.

From: Robin Rosenberg <hidden>
Date: 2016-08-11 20:27:23

From: Robin Rosenberg <redacted>

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 <redacted>
---

Yet another version (maybe I'm learning perl now....). This one uses one pass
to git-diff, but compensates that with two to git-apply.

-- robin

 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 c9d1d88..8f37123 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 @@ 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;
-$? && 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];
-    }
+`git-diff-tree --binary -p $parent $commit >.cvsexportcommit.diff`;# || die "Cannot diff";
+
+## 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=<APPLY>;
+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 \d+ (.*)$/;
+	push (@dfiles,$1) if m/^ +delete mode \d{6} (.*)$/;
 }
-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;
-
-$opt_v && print "The commit affects:\n ";
-$opt_v && print join ("\n ", @afiles,@mfiles,@dfiles) . "\n\n";
-undef @files; # don't need it anymore
+map { s/^"(.*)"$/$1/g } @bfiles,@files;
+map { s/\\([\d]{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 (@afiles) {
     }
 }
 
-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 @@ if ($dirty) {
     }
 }
 
-###
-### 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 (@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 { $_; }; } (@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 <<END;
 Usage: GIT_DIR=/path/to/.git ${\basename $0} [-h] [-p] [-v] [-c] [-f] [-m msgprefix] [ parent ] commit
@@ -321,17 +256,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 +270,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..ca0513b 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
+      )'
 

[PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.

From: Robin Rosenberg <hidden>
Date: 2016-08-11 20:45:58

From: Robin Rosenberg <redacted>

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 <redacted>
---

Updated patch using --numstat to figure which files are binary.

 git-cvsexportcommit.perl       |  158 +++++++++++-----------------------------
 t/t9200-git-cvsexportcommit.sh |  108 ++++++++++++++++++++-------
 2 files changed, 125 insertions(+), 141 deletions(-)
diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index c9d1d88..159af66 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;
@@ -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,10 @@ 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;
 
+`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
 
 # check that the files are clean and up to date according to cvs
 my $dirty;
@@ -197,87 +183,36 @@ 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`;
-}
+my @stat;
+open APPLY, "GIT_DIR= git-apply -C$fuzz --binary --numstat --apply <.cvsexportcommit.diff|" || die "cannot patch";
+@stat=<APPLY>;
+close APPLY || die "Cannot patch";
+my @bfiles=grep { chomp && s/^\-\t\-\t(.*)$/$1/; } @stat;
+map { s/^"(.*)"$/$1/g } @bfiles;
+map { s/\\([\d]{3})/sprintf('%c',oct $1)/eg } @bfiles;
 
+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,35 +220,40 @@ 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";
 } else {
     print "Ready for you to commit, just run:\n\n   $cmd\n";
 }
+
+# clean up
+unlink(".cvsexportcommit.diff");
+unlink(".msg");
+
 sub usage {
 	print STDERR <<END;
 Usage: GIT_DIR=/path/to/.git ${\basename $0} [-h] [-p] [-v] [-c] [-f] [-m msgprefix] [ parent ] commit
@@ -321,17 +261,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 +275,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 @@ 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  &&
+      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
+      )'
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help