threads / patch / 4459

patch, 5 partsgit-svn: eol_cp corner-case fixes

Subject: [PATCH 3/5] git-svn: eol_cp corner-case fixes

## tl;dr

6 messages between Jun 11, 2006 and Jun 11, 2006. Diffs are folded; open one to read it.

replies: 5people: 1as markdown or json

Eric Wong· Jun 11, 2006, 07:03 UTC · lore

[PATCH] git-svn: bug fixes (some resends)

[PATCH 1/5] git-svn: t0000: add -f flag to checkout
[PATCH 2/5] git-svn: fix handling of filenames with embedded '@'
	These two are resends, patch 2 only affects 1.1.0-pre.

[PATCH 3/5] git-svn: eol_cp corner-case fixes
	Kinda urgent (but only affects 1.1.0-pre)

[PATCH 4/5] git-svn: restore original LC_ALL setting (or unset) for commit
	For people that want to commit UTF-8 commit messages.

[PATCH 5/5] git-svn: don't allow commit if svn tree is not current
	Extra sanity check, just in case.

Eric Wong· Jun 11, 2006, 07:03 UTC · re: Eric Wong · lore

[PATCH 1/5] git-svn: t0000: add -f flag to checkout

Some changes to the latest git.git made this test croak. So we'll always just force everything when using a new branch.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 contrib/git-svn/t/t0000-contrib-git-svn.sh |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

Show changes to contrib/git-svn/t/t0000-contrib-git-svn.sh +5 −5

diff --git a/contrib/git-svn/t/t0000-contrib-git-svn.sh b/contrib/git-svn/t/t0000-contrib-git-svn.sh
index 8b3a0d9..a07fbad 100644
--- a/contrib/git-svn/t/t0000-contrib-git-svn.sh
+++ b/contrib/git-svn/t/t0000-contrib-git-svn.sh
@@ -32,7 +32,7 @@ test_expect_success \
 
 
 name='try a deep --rmdir with a commit'
-git checkout -b mybranch remotes/git-svn
+git checkout -f -b mybranch remotes/git-svn
 mv dir/a/b/c/d/e/file dir/file
 cp dir/file file
 git update-index --add --remove dir/a/b/c/d/e/file dir/file file
@@ -58,7 +58,7 @@ test_expect_code 1 "$name" \
 
 name='detect node change from directory to file #1'
 rm -rf dir $GIT_DIR/index
-git checkout -b mybranch2 remotes/git-svn
+git checkout -f -b mybranch2 remotes/git-svn
 mv bar/zzz zzz
 rm -rf bar
 mv zzz bar
@@ -73,7 +73,7 @@ test_expect_code 1 "$name" \
 
 name='detect node change from file to directory #2'
 rm -f $GIT_DIR/index
-git checkout -b mybranch3 remotes/git-svn
+git checkout -f -b mybranch3 remotes/git-svn
 rm bar/zzz
 git-update-index --remove bar/zzz
 mkdir bar/zzz
@@ -88,7 +88,7 @@ test_expect_code 1 "$name" \
 
 name='detect node change from directory to file #2'
 rm -f $GIT_DIR/index
-git checkout -b mybranch4 remotes/git-svn
+git checkout -f -b mybranch4 remotes/git-svn
 rm -rf dir
 git update-index --remove -- dir/file
 touch dir
@@ -103,7 +103,7 @@ test_expect_code 1 "$name" \
 
 name='remove executable bit from a file'
 rm -f $GIT_DIR/index
-git checkout -b mybranch5 remotes/git-svn
+git checkout -f -b mybranch5 remotes/git-svn
 chmod -x exec.sh
 git update-index exec.sh
 git commit -m "$name"

-- 
1.3.3.g2dc7b-dirty

Eric Wong· Jun 11, 2006, 07:03 UTC · re: Eric Wong · lore

[PATCH 2/5] git-svn: fix handling of filenames with embedded '@'

svn has trouble parsing files with embedded '@' characters. For example,

  svn propget svn:keywords foo@bar.c
  svn: Syntax error parsing revision 'bar.c'

I asked about this on #svn and the workaround suggested was to append an explicit revision specifier:

  svn propget svn:keywords foo@bar.c@BASE

This patch appends '@BASE' to the filename in all calls to 'svn propget'.

Patch originally by Seth Falcon <sethfalcon@gmail.com>
Seth: signoff?

[ew: Made to work with older svn that don't support peg revisions]

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 contrib/git-svn/git-svn.perl |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

Show changes to contrib/git-svn/git-svn.perl +13 −4

diff --git a/contrib/git-svn/git-svn.perl b/contrib/git-svn/git-svn.perl
index aac8779..7ed11ef 100755
--- a/contrib/git-svn/git-svn.perl
+++ b/contrib/git-svn/git-svn.perl
@@ -34,7 +34,7 @@ my $sha1_short = qr/[a-f\d]{4,40}/;
 my ($_revision,$_stdin,$_no_ignore_ext,$_no_stop_copy,$_help,$_rmdir,$_edit,
 	$_find_copies_harder, $_l, $_version, $_upgrade, $_authors);
 my (@_branch_from, %tree_map, %users);
-my $_svn_co_url_revs;
+my ($_svn_co_url_revs, $_svn_pg_peg_revs);
 
 my %fc_opts = ( 'no-ignore-externals' => \$_no_ignore_ext,
 		'branch|b=s' => \@_branch_from,
@@ -336,7 +336,7 @@ sub show_ignore {
 	my %ign;
 	File::Find::find({wanted=>sub{if(lstat $_ && -d _ && -d "$_/.svn"){
 		s#^\./##;
-		@{$ign{$_}} = safe_qx(qw(svn propget svn:ignore),$_);
+		@{$ign{$_}} = svn_propget_base('svn:ignore', $_);
 		}}, no_chdir=>1},'.');
 
 	print "\n# /\n";
@@ -859,7 +859,7 @@ sub sys { system(@_) == 0 or croak $? }
 
 sub eol_cp {
 	my ($from, $to) = @_;
-	my $es = safe_qx(qw/svn propget svn:eol-style/, $to);
+	my $es = svn_propget_base('svn:eol-style', $to);
 	open my $rfd, '<', $from or croak $!;
 	binmode $rfd or croak $!;
 	open my $wfd, '>', $to or croak $!;
@@ -897,7 +897,7 @@ sub do_update_index {
 	while (my $x = <$p>) {
 		chomp $x;
 		if (!$no_text_base && lstat $x && ! -l _ &&
-				safe_qx(qw/svn propget svn:keywords/,$x)) {
+				svn_propget_base('svn:keywords', $x)) {
 			my $mode = -x _ ? 0755 : 0644;
 			my ($v,$d,$f) = File::Spec->splitpath($x);
 			my $tb = File::Spec->catfile($d, '.svn', 'tmp',
@@ -1135,6 +1135,9 @@ sub svn_compat_check {
 	if (grep /usage: checkout URL\[\@REV\]/,@co_help) {
 		$_svn_co_url_revs = 1;
 	}
+	if (grep /\[TARGET\[\@REV\]\.\.\.\]/, `svn propget -h`) {
+		$_svn_pg_peg_revs = 1;
+	}
 
 	# I really, really hope nobody hits this...
 	unless (grep /stop-on-copy/, (safe_qx(qw(svn log -h)))) {
@@ -1214,6 +1217,12 @@ sub load_authors {
 	close $authors or croak $!;
 }
 
+sub svn_propget_base {
+	my ($p, $f) = @_;
+	$f .= '@BASE' if $_svn_pg_peg_revs;
+	return safe_qx(qw/svn propget/, $p, $f);
+}
+
 __END__
 
 Data structures:

-- 
1.3.3.g2dc7b-dirty

Eric Wong· Jun 11, 2006, 07:03 UTC · re: Eric Wong · lore

If we read the maximum size of our buffer into $buf, and the last character is '\015', there's a chance that the character is '\012', which means our regex won't work correctly. At the worst case, this could introduce an extra newline into the code. We'll now read an extra character if we see '\015' is the last character in $buf.

We also forgot to recalculate the length of $buf after doing the newline substitution, causing some files to appeare truncated. We'll do that now and force byte semantics in length() for good measure.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 contrib/git-svn/git-svn.perl |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

Show changes to contrib/git-svn/git-svn.perl +11 −4

diff --git a/contrib/git-svn/git-svn.perl b/contrib/git-svn/git-svn.perl
index 7ed11ef..8d2e7f7 100755
--- a/contrib/git-svn/git-svn.perl
+++ b/contrib/git-svn/git-svn.perl
@@ -866,19 +866,26 @@ sub eol_cp {
 	binmode $wfd or croak $!;
 
 	my $eol = $EOL{$es} or undef;
-	if ($eol) {
-		print  "$eol: $from => $to\n";
-	}
 	my $buf;
+	use bytes;
 	while (1) {
 		my ($r, $w, $t);
 		defined($r = sysread($rfd, $buf, 4096)) or croak $!;
 		return unless $r;
-		$buf =~ s/(?:\015|\012|\015\012)/$eol/gs if $eol;
+		if ($eol) {
+			if ($buf =~ /\015$/) {
+				my $c;
+				defined($r = sysread($rfd,$c,1)) or croak $!;
+				$buf .= $c if $r > 0;
+			}
+			$buf =~ s/(?:\015\012|\015|\012)/$eol/gs;
+			$r = length($buf);
+		}
 		for ($w = 0; $w < $r; $w += $t) {
 			$t = syswrite($wfd, $buf, $r - $w, $w) or croak $!;
 		}
 	}
+	no bytes;
 }
 
 sub do_update_index {

-- 
1.3.3.g2dc7b-dirty

Eric Wong· Jun 11, 2006, 07:03 UTC · re: Eric Wong · lore

[PATCH 4/5] git-svn: restore original LC_ALL setting (or unset) for commit

svn forces UTF-8 for commit messages, and with LC_ALL set to 'C' it is unable to determine encoding of the git commit message.

Now we'll just assume the user has set LC_* correctly for the commit message they're using.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 contrib/git-svn/git-svn.perl |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)

Show changes to contrib/git-svn/git-svn.perl +23 −11

diff --git a/contrib/git-svn/git-svn.perl b/contrib/git-svn/git-svn.perl
index 8d2e7f7..8bc3d69 100755
--- a/contrib/git-svn/git-svn.perl
+++ b/contrib/git-svn/git-svn.perl
@@ -14,6 +14,7 @@ use Cwd qw/abs_path/;
 $GIT_DIR = abs_path($ENV{GIT_DIR} || '.git');
 $ENV{GIT_DIR} = $GIT_DIR;
 
+my $LC_ALL = $ENV{LC_ALL};
 # make sure the svn binary gives consistent output between locales and TZs:
 $ENV{TZ} = 'UTC';
 $ENV{LC_ALL} = 'C';
@@ -704,23 +705,34 @@ sub svn_commit_tree {
 	my ($oneline) = ($log_msg{msg} =~ /([^\n\r]+)/);
 	print "Committing $commit: $oneline\n";
 
+	if (defined $LC_ALL) {
+		$ENV{LC_ALL} = $LC_ALL;
+	} else {
+		delete $ENV{LC_ALL};
+	}
 	my @ci_output = safe_qx(qw(svn commit -F),$commit_msg);
-	my ($committed) = grep(/^Committed revision \d+\./,@ci_output);
+	$ENV{LC_ALL} = 'C';
 	unlink $commit_msg;
-	defined $committed or croak
+	my ($committed) = ($ci_output[$#ci_output] =~ /(\d+)/);
+	if (!defined $committed) {
+		my $out = join("\n",@ci_output);
+		print STDERR "W: Trouble parsing \`svn commit' output:\n\n",
+				$out, "\n\nAssuming English locale...";
+		($committed) = ($out =~ /^Committed revision \d+\./sm);
+		defined $committed or die " FAILED!\n",
 			"Commit output failed to parse committed revision!\n",
-			join("\n",@ci_output),"\n";
-	my ($rev_committed) = ($committed =~ /^Committed revision (\d+)\./);
+		print STDERR " OK\n";
+	}
 
 	my @svn_up = qw(svn up);
 	push @svn_up, '--ignore-externals' unless $_no_ignore_ext;
-	if ($rev_committed == ($svn_rev + 1)) {
-		push @svn_up, "-r$rev_committed";
+	if ($committed == ($svn_rev + 1)) {
+		push @svn_up, "-r$committed";
 		sys(@svn_up);
 		my $info = svn_info('.');
 		my $date = $info->{'Last Changed Date'} or die "Missing date\n";
-		if ($info->{'Last Changed Rev'} != $rev_committed) {
-			croak "$info->{'Last Changed Rev'} != $rev_committed\n"
+		if ($info->{'Last Changed Rev'} != $committed) {
+			croak "$info->{'Last Changed Rev'} != $committed\n"
 		}
 		my ($Y,$m,$d,$H,$M,$S,$tz) = ($date =~
 					/(\d{4})\-(\d\d)\-(\d\d)\s
@@ -728,16 +740,16 @@ sub svn_commit_tree {
 					 or croak "Failed to parse date: $date\n";
 		$log_msg{date} = "$tz $Y-$m-$d $H:$M:$S";
 		$log_msg{author} = $info->{'Last Changed Author'};
-		$log_msg{revision} = $rev_committed;
+		$log_msg{revision} = $committed;
 		$log_msg{msg} .= "\n";
 		my $parent = file_to_s("$REV_DIR/$svn_rev");
 		git_commit(\%log_msg, $parent, $commit);
-		return $rev_committed;
+		return $committed;
 	}
 	# resync immediately
 	push @svn_up, "-r$svn_rev";
 	sys(@svn_up);
-	return fetch("$rev_committed=$commit")->{revision};
+	return fetch("$committed=$commit")->{revision};
 }
 
 # read the entire log into a temporary file (which is removed ASAP)

-- 
1.3.3.g2dc7b-dirty

Eric Wong· Jun 11, 2006, 07:03 UTC · re: Eric Wong · lore

[PATCH 5/5] git-svn: don't allow commit if svn tree is not current

If new revisions are fetched, that implies we haven't merged, acked, or nacked them yet, and attempting to write the tree we're committing means we'd silently clobber the newly fetched changes.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 contrib/git-svn/git-svn.perl |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

Show changes to contrib/git-svn/git-svn.perl +9 −2

diff --git a/contrib/git-svn/git-svn.perl b/contrib/git-svn/git-svn.perl
index 8bc3d69..72129de 100755
--- a/contrib/git-svn/git-svn.perl
+++ b/contrib/git-svn/git-svn.perl
@@ -309,9 +309,16 @@ sub commit {
 	}
 	chomp @revs;
 
-	fetch();
-	chdir $SVN_WC or croak $!;
+	chdir $SVN_WC or croak "Unable to chdir $SVN_WC: $!\n";
 	my $info = svn_info('.');
+	my $fetched = fetch();
+	if ($info->{Revision} != $fetched->{revision}) {
+		print STDERR "There are new revisions that were fetched ",
+				"and need to be merged (or acknowledged) ",
+				"before committing.\n";
+		exit 1;
+	}
+	$info = svn_info('.');
 	read_uuid($info);
 	my $svn_current_rev =  $info->{'Last Changed Rev'};
 	foreach my $c (@revs) {

-- 
1.3.3.g2dc7b-dirty