* [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
@ 2006-12-09 23:29 Robin Rosenberg
2006-12-10 1:18 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Robin Rosenberg @ 2006-12-09 23:29 UTC (permalink / raw)
To: git
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 14078 bytes --]
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(-)
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
+ )'
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
2006-12-09 23:29 [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters Robin Rosenberg
@ 2006-12-10 1:18 ` Junio C Hamano
2006-12-10 15:39 ` Robin Rosenberg
2006-12-10 23:30 ` Robin Rosenberg
0 siblings, 2 replies; 11+ messages in thread
From: Junio C Hamano @ 2006-12-10 1:18 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
Robin Rosenberg <robin.rosenberg@dewire.com> 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.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
2006-12-10 1:18 ` Junio C Hamano
@ 2006-12-10 15:39 ` Robin Rosenberg
2006-12-10 15:45 ` Robin Rosenberg
2006-12-10 21:27 ` Junio C Hamano
2006-12-10 23:30 ` Robin Rosenberg
1 sibling, 2 replies; 11+ messages in thread
From: Robin Rosenberg @ 2006-12-10 15:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
söndag 10 december 2006 02:18 skrev Junio C Hamano:
> Robin Rosenberg <robin.rosenberg@dewire.com> 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 ...
<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.
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
2006-12-10 15:39 ` Robin Rosenberg
@ 2006-12-10 15:45 ` Robin Rosenberg
2006-12-10 21:27 ` Junio C Hamano
1 sibling, 0 replies; 11+ messages in thread
From: Robin Rosenberg @ 2006-12-10 15:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 13412 bytes --]
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>
---
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
+ )'
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
2006-12-10 15:39 ` Robin Rosenberg
2006-12-10 15:45 ` Robin Rosenberg
@ 2006-12-10 21:27 ` Junio C Hamano
2006-12-10 23:48 ` Robin Rosenberg
1 sibling, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2006-12-10 21:27 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
Robin Rosenberg <robin.rosenberg@dewire.com> writes:
>> 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?
Maybe I am missing something. It tells you about added or
removed files (either binary or non-binary).
I'd prepare a pair of practice patch files. "forward" has
creation, "reverse" has addition.
$ git diff-tree -p --binary fe142b3a >forward.patch
$ git diff-tree -R -p --binary fe142b3a >reverse.patch
$ git apply --summary --numstat forward.patch
50 20 git-cvsexportcommit.perl
145 0 t/t9200-git-cvsexportcommit.sh
- - t/test9200a.png
- - t/test9200b.png
create mode 100755 t/t9200-git-cvsexportcommit.sh
create mode 100644 t/test9200a.png
create mode 100644 t/test9200b.png
$ git apply --summary --numstat reverse.patch
20 50 git-cvsexportcommit.perl
0 145 t/t9200-git-cvsexportcommit.sh
- - t/test9200a.png
- - t/test9200b.png
delete mode 100755 t/t9200-git-cvsexportcommit.sh
delete mode 100644 t/test9200a.png
delete mode 100644 t/test9200b.png
> This fuzz gives some, but not much slack. Reverting the option could be a
> good idea.
>
> Update follows.
Ok.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
2006-12-10 21:27 ` Junio C Hamano
@ 2006-12-10 23:48 ` Robin Rosenberg
2006-12-12 1:15 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Robin Rosenberg @ 2006-12-10 23:48 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
söndag 10 december 2006 22:27 skrev Junio C Hamano:
> Maybe I am missing something. It tells you about added or
> removed files (either binary or non-binary).
no, I was missing --summary. I thought --numstat did that (man page
says "similar to stat".
> I'd prepare a pair of practice patch files. "forward" has
> creation, "reverse" has addition.
t9200 serves that purpose.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
2006-12-10 23:48 ` Robin Rosenberg
@ 2006-12-12 1:15 ` Junio C Hamano
0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2006-12-12 1:15 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> söndag 10 december 2006 22:27 skrev Junio C Hamano:
>> Maybe I am missing something. It tells you about added or
>> removed files (either binary or non-binary).
>
> no, I was missing --summary. I thought --numstat did that (man page
> says "similar to stat".
Ah, figures.
Your patch removes more lines than it adds, which is quite nice.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
2006-12-10 1:18 ` Junio C Hamano
2006-12-10 15:39 ` Robin Rosenberg
@ 2006-12-10 23:30 ` Robin Rosenberg
1 sibling, 0 replies; 11+ messages in thread
From: Robin Rosenberg @ 2006-12-10 23:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=utf-8, Size: 15050 bytes --]
From: Robin Rosenberg <robin.rosenberg@dewire.com>
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 <robin.rosenberg@dewire.com>
---
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
+ )'
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
@ 2006-11-15 0:55 Robin Rosenberg
2006-11-15 6:08 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Robin Rosenberg @ 2006-11-15 0:55 UTC (permalink / raw)
To: git
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 testd (it did not work before).
Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
---
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
+ )'
+
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
2006-11-15 0:55 Robin Rosenberg
@ 2006-11-15 6:08 ` Junio C Hamano
2006-11-15 6:27 ` Junio C Hamano
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2006-11-15 6:08 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
Robin Rosenberg <robin.rosenberg@dewire.com> writes:
> 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 testd (it did not work before).
>
> Signed-off-by: Robin Rosenberg <robin.rosenberg@dewire.com>
Thanks. Allow me to put this in freezer for a few days while
releasing v1.4.4.
Some comments below.
> @@ -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";
> +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;
my @fields = ($f =~ m/^.../);
> 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;
> +
Logically these map {} should be done only on the c-quoted
names, but it is Ok because the names that have backslash with
octal are quoted. However, what's inside the map is not correct
c dequoting (see how Jakub does it in gitweb -- you need to
worry about \\, \" and such).
I think it is perhaps safer to parse "diff --git" and remember
the filenames of the "current patch" and then notice only
/^Binary files / part of the message.
But all of the above shows deficiency in the current set of
tools -- they are not helping Porcelain writers enough. I think
we should enhance 'apply --numstat' to let it show binary diffs
differently:
git diff-tree -p $parent $commit >.tmpfile
git apply --numstat -z <.tmpfile
would currently say "0 0" for binary files (the primary benefit
of using "--numstat -z" here is that it would give Perl scripts
pathnames parsable without C dequoting). We should somehow have
a way to show it differently from text files without any
added/deleted lines (e.g. only the mode change), and that would
make the life of Porcelain writers who needs to write something
like the above code much more pleasant. Perhaps show "- -"
instead of "0 0", since there is no notion of lines in "binary
files differ" case?
> - `cvs add $d`;
> - if ($?) {
> - $dirty = 1;
> + print "Adding directory $d";
> + if (system('cvs','add',$d)) {
> + $dirtypatch = 1;
Good.
> ## apply non-binary changes
> my $fuzz = $opt_p ? 0 : 2;
>
> -print "Patching non-binary files\n";
> +print "Patching\n";
Leftover comment that does not apply anymore.
> if (scalar(@afiles)+scalar(@dfiles)+scalar(@mfiles) != scalar(@bfiles)) {
> + `git-diff-tree --binary -z -p $parent -p $commit >.cvsexportcommit.diff`;
Haven't you run this diff before to grep for "Binary files..."
Maybe doing a temporary file upfront once and using it would
make sense.
Also why multiple -p? I do not think -z is wanted here, as -z
affects only output side of git apply and not input side (see
the above comment on --numstat -z).
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters.
2006-11-15 6:08 ` Junio C Hamano
@ 2006-11-15 6:27 ` Junio C Hamano
0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2006-11-15 6:27 UTC (permalink / raw)
To: Robin Rosenberg; +Cc: git
Junio C Hamano <junkio@cox.net> writes:
> But all of the above shows deficiency in the current set of
> tools -- they are not helping Porcelain writers enough. I think
> we should enhance 'apply --numstat' to let it show binary diffs
> differently:
>
> git diff-tree -p $parent $commit >.tmpfile
> git apply --numstat -z <.tmpfile
>
> would currently say "0 0" for binary files (the primary benefit
> of using "--numstat -z" here is that it would give Perl scripts
> pathnames parsable without C dequoting). We should somehow have
> a way to show it differently from text files without any
> added/deleted lines (e.g. only the mode change), and that would
> make the life of Porcelain writers who needs to write something
> like the above code much more pleasant. Perhaps show "- -"
> instead of "0 0", since there is no notion of lines in "binary
> files differ" case?
That is, something like this...
-- >8 --
[PATCH] apply --numstat: mark binary diffstat with - -, not 0 0
We do not even know number of lines so showing it as 0 0 is
lying. This would also help Porcelains like cvsexportcommit.
Signed-off-by: Junio C Hamano <junkio@cox.net>
---
builtin-apply.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index aad5526..b80ad2c 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2116,7 +2116,11 @@ static void numstat_patch_list(struct pa
for ( ; patch; patch = patch->next) {
const char *name;
name = patch->new_name ? patch->new_name : patch->old_name;
- printf("%d\t%d\t", patch->lines_added, patch->lines_deleted);
+ if (patch->is_binary)
+ printf("-\t-\t");
+ else
+ printf("%d\t%d\t",
+ patch->lines_added, patch->lines_deleted);
if (line_termination && quote_c_style(name, NULL, NULL, 0))
quote_c_style(name, NULL, stdout, 0);
else
--
1.4.4.rc2.g2a54
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2006-12-12 1:16 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-12-09 23:29 [PATCH] Make cvsexportcommit work with filenames with spaces and non-ascii characters Robin Rosenberg
2006-12-10 1:18 ` Junio C Hamano
2006-12-10 15:39 ` Robin Rosenberg
2006-12-10 15:45 ` Robin Rosenberg
2006-12-10 21:27 ` Junio C Hamano
2006-12-10 23:48 ` Robin Rosenberg
2006-12-12 1:15 ` Junio C Hamano
2006-12-10 23:30 ` Robin Rosenberg
-- strict thread matches above, loose matches on Subject: below --
2006-11-15 0:55 Robin Rosenberg
2006-11-15 6:08 ` Junio C Hamano
2006-11-15 6:27 ` Junio C Hamano
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).