Robin Rosenberg [off-list ref] 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 <redacted>
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);
måndag 13 november 2006 05:39 skrev Junio C Hamano:
Robin Rosenberg [off-list ref] writes:
quoted
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 <redacted>
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.
I hardly remember now. I wrote test cases and modified the code until it
worked. I'll skip that remark.
You seem to like:
my $p = "m,^$pathname\$,";
if (grep $p,@array) { do this... }
I can't say I'm fond of any perl syntax, really. Perl is a very usable, but
ugly tool.
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.
Thanks.
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.
I sent a patch earlier, which was not applied, due to imperfections that I
cannot solve fully. One issue was that patch 2.5.9 was required and hacking
it to handle binary diffs with spaces would require and even worse kludge,
I can, however, send the previous patch adapted onto this one.
It appears that this program was never used in western
hemisphere, by the way ;-).
Now, Sweden is definitely in the northern hemisphere, even during the cold
war, though a number of computer games colored it red (or was it only
Finland) during the cold war.
The tests work with my locale (swedish ISO-8859-15), even though they like
all other tests run in the C "hemishpere" by default. AFAIK, git isn't very
user friendly with non-ascii filenames as it is today and cvsexportcommit
didn't work on such files before, and it doesn't now. No change there.
Another patch, another day, maybe.
-- robin
Robin Rosenberg [off-list ref] writes:
quoted
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.
I sent a patch earlier, which was not applied, due to imperfections that I
cannot solve fully. One issue was that patch 2.5.9 was required and hacking
it to handle binary diffs with spaces would require and even worse kludge,
I suspect that we should not be using patch, but instead be
using git-apply perhaps with -C option if people want fuzz.
quoted
It appears that this program was never used in western
hemisphere, by the way ;-).
Now, Sweden is definitely in the northern hemisphere, even during the cold
war, though a number of computer games colored it red (or was it only
Finland) during the cold war.
I know this script originally came from southern hemisphere, but
I was talking about _western_ hemisphere. The comment refers to
the part of the code the attached patch fixes, which I will
apply along with your updated patch.
The tests work with my locale (swedish ISO-8859-15), even though they like
all other tests run in the C "hemishpere" by default. AFAIK, git isn't very
user friendly with non-ascii filenames as it is today and cvsexportcommit
didn't work on such files before, and it doesn't now. No change there.
That is a separate issue. I think scripts that work with git
plumbing should read from -z output when they need to and are
capable of, as we have done for git-cvsserver recently, and
things written in Perl certainly are capable of doing so.
diff --git a/git-cvsexportcommit.perl b/git-cvsexportcommit.perl
index facb466..b1cc014 100755
--- a/git-cvsexportcommit.perl
+++ b/git-cvsexportcommit.perl
@@ -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 {tisdag 14 november 2006 01:47 skrev Junio C Hamano:
Robin Rosenberg wrote:
quoted
I sent a patch earlier, which was not applied, due to imperfections that I
cannot solve fully. One issue was that patch 2.5.9 was required and hacking
it to handle binary diffs with spaces would require and even worse kludge,
I suspect that we should not be using patch, but instead be
using git-apply perhaps with -C option if people want fuzz.
Does git-apply work without a git repo? We're applying patches onto a CVS
repo.
[...]
I know this script originally came from southern hemisphere, but
I was talking about _western_ hemisphere. The comment refers to
the part of the code the attached patch fixes, which I will
apply along with your updated patch.
Western/nothern, both apply to where I live.
[...]
I think scripts that work with git
plumbing should read from -z output when they need to and are
capable of, as we have done for git-cvsserver recently, and
things written in Perl certainly are capable of doing so.
I'll investgate that.