Re: [PATCH] Rework cvsexportcommit to handle binary files for all cases.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Robin Rosenberg <robin.rosenberg@xxxxxxxxxx> writes:

> Now adding, removing and changing binary files works. I added
> test cases to make sure qit works and can be verified by
> others. Som other corner cases were resolved too.
>
> Signed-off-by: Robin Rosenberg <robin.rosenberg@xxxxxxxxxx>

Thanks.

It is necessary to clarify what these other corner cases are in
the commit log message.  Three months down the road you yourself
would not remember what they were.

You seem to like:

	my $p = "m,^$pathname\$,";
        if (grep $p,@array) { do this... }

but this is a bad habit.  $pathname can contain regexp
metacharacters or a comma.  You should just say:

	if (grep { $_ eq $pathname } @arrray) { do this... }

instead.  I'd fix them up with the attached patch.

It would also be nice to rework this program so that it can work
with whitespaces in pathnames.  I do not think it currently
works with them at all.

It appears that this program was never used in western
hemisphere, by the way ;-).

---

diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index 78c847e..b1cc014 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -1,10 +1,10 @@
 #!/usr/bin/perl -w
 
 # Known limitations:
-# - cannot add or remove binary files
 # - does not propagate permissions
 # - tells "ready for commit" even when things could not be completed
 #   (eg addition of a binary file)
+# - does not handle whitespace in pathnames at all.
 
 use strict;
 use Getopt::Std;
@@ -68,9 +68,9 @@ foreach my $line (@commit) {
     if ($stage eq 'headers') {
 	if ($line =~ m/^parent (\w{40})$/) { # found a parent
 	    push @parents, $1;
-	} elsif ($line =~ m/^author (.+) \d+ \+\d+$/) {
+	} elsif ($line =~ m/^author (.+) \d+ [-+]\d+$/) {
 	    $author = $1;
-	} elsif ($line =~ m/^committer (.+) \d+ \+\d+$/) {
+	} elsif ($line =~ m/^committer (.+) \d+ [-+]\d+$/) {
 	    $committer = $1;
 	}
     } else {
@@ -165,8 +165,8 @@ foreach my $d (@dirs) {
 foreach my $f (@afiles) {
     # This should return only one value
     if ($f =~ m,(.*)/[^/]*$,) {
-        my $p="m,^".$1."\$,";
-	next if grep $p,@dirs;
+	my $p = $1;
+	next if (grep { $_ eq $p } @dirs);
     }
     my @status = grep(m/^File/,  safe_pipe_capture('cvs', '-q', 'status' ,$f));
     if (@status > 1) { warn 'Strange! cvs status returned more than one line?'};
@@ -219,8 +219,7 @@ 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
-    my $p="m/^$f$/";
-    if (not(grep $p,@afiles)) {
+    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`;
@@ -234,7 +233,7 @@ foreach my $f (@bfiles) {
 	    }
         }
     }
-    if (not(grep m/^$f$/,@dfiles)) {
+    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`;
@@ -256,7 +255,7 @@ my $fuzz = $opt_p ? 0 : 2;
 
 print "Patching non-binary files\n";
 
-if (scalar(@afiles)+scalar(@dfiles)+scalar(@mfiles) ne scalar(@bfiles)) {
+if (scalar(@afiles)+scalar(@dfiles)+scalar(@mfiles) != scalar(@bfiles)) {
     print `(git-diff-tree -p $parent -p $commit | patch -p1 -F $fuzz ) 2>&1`;
 }
 
@@ -269,7 +268,7 @@ if (($? >> 8) == 2) {
 }
 
 foreach my $f (@afiles) {
-    if (grep /^$f$/,@bfiles) {
+    if (grep { $_ eq $f } @bfiles) {
       system('cvs', 'add','-kb',$f);
     } else {
       system('cvs', 'add', $f);



-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]