Skip to contentSkip to conversation

git-svn: make test for SVK mirror path import

PATCH In review Sam Vilain 3 participants 11 messages Aug 11, 2016

lore
Share & tools
lore

Sam Vilain

author
#0 lore raw

PATCH 1/5 git-svn: make test for SVK mirror path import

From: <[email protected]>

A manual test that sets up a repository that looks like an SVK depot, and then imports it to check that it looks like we mirrored the 'original' source.

There is also a minor modification to the git-svn test library shell file which sets a variable for the subversion repository's filesystem path.
---

 t/lib-git-svn.sh                   |    3 -
 t/t9107-git-svn-svk-mirrorpaths.sh |   92 ++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 2 deletions(-)
t/lib-git-svn.sh +1 -2
index 63c6703..dffd1fb 100644
@@ -45,6 +45,5 @@ else
 	svnadmin create "$svnrepo"
 fi
 
+rawsvnrepo="$svnrepo"
 svnrepo="file://$svnrepo"
-
-
t/t9107-git-svn-svk-mirrorpaths.sh new file +92 -0

92 lines not on this page · raw message

#1 lore raw

Sam Vilain wrote:
^^^^^^^^^^

Quoted 1 line

From: <[email protected]>

[...]

Review

Quoted 1 line
(quoted diff) +0 -0

b/t/t9107-git-svn-svk-mirrorpaths.sh

Quoted 4 lines

new file mode 100755
index 0000000..130e786

t/t9107-git-svn-svk-mirrorpaths.sh new file +0 -0

[...]

Quoted 1 line

+# Copyright (c) 2006 Eric Wong

                        ^^^^^^^^^

???

Signature · 3 lines
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
author
#2 lore raw

PATCH 2/5 git-svn: let libsvn_ls_fullurl return properties too

In reply to Sam Vilain

From: Sam Vilain <[email protected]>

Allow an extra parameter to be passed to the libsvn_ls_fullurl function to collect and return the properties of the URL being listed.
---

 git-svn.perl |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
git-svn.perl +4 -3
index 3891122..93cfcc4 100755
@@ -3321,18 +3321,19 @@ sub libsvn_commit_cb {
 
 sub libsvn_ls_fullurl {
 	my $fullurl = shift;
+	my $want_props = shift;
 	my $ra = libsvn_connect($fullurl);
-	my @ret;
+	my (@ret, @props);
 	my $pool = SVN::Pool->new;
 	my $r = defined $_revision ? $_revision : $ra->get_latest_revnum;
-	my ($dirent, undef, undef) = $ra->get_dir('', $r, $pool);
+	my ($dirent, undef, $props) = $ra->get_dir('', $r, $pool);
 	foreach my $d (keys %$dirent) {
 		if ($dirent->{$d}->kind == $SVN::Node::dir) {
 			push @ret, "$d/"; # add '/' for compat with cli svn
 		}
 	}
 	$pool->clear;
-	return @ret;
+	return ($want_props ? (\@ret, $props) : @ret);
 }
 
 
author
#3 lore raw

PATCH 3/5 git-svn: collect SVK source URL on mirror paths

In reply to Sam Vilain

From: Sam Vilain <[email protected]>

If you use git-svn to import a mirror path within an SVK depot directly (eg, file:///home/you/.svk/local/mirror/foo), then the URLs and revisions in the generated commits will be of the wrong URL.

When we set up with git-svn multi-init, check whether the base URL is (the root of) a mirror path, and store it for later. Set up a couple of globals and helper functions for later use.
---

 git-svn.perl |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)
git-svn.perl +21 -0
index 93cfcc4..800c579 100755
@@ -5,6 +5,7 @@ use warnings;
 use strict;
 use vars qw/	$AUTHOR $VERSION
 		$SVN_URL $SVN_INFO $SVN_WC $SVN_UUID
+		$SVM_URL $SVM_UUID
 		$GIT_SVN_INDEX $GIT_SVN
 		$GIT_DIR $GIT_SVN_DIR $REVDB/;
 $AUTHOR = 'Eric Wong <[email protected]>';
@@ -733,6 +734,21 @@ sub multi_init {
 		init($_trunk);
 		sys('git-repo-config', 'svn.trunk', $_trunk);
 	}
+	if ( $url ) {
+		# check for the case of SVK mirror path
+		my ($ents, $props) = libsvn_ls_fullurl($url, "1");
+		if ( my $src = $props->{'svm:source'} ) {
+			$src =~ s{!$}{};  # don't know wtf a ! is there for
+			$src =~ s{(^[a-z\+]*://)[^/@]*@}{$1}; # username of no interest
+
+			# store the source as a repo-config item
+			sys('git-repo-config', 'svn.svkmirrorpath', $src);
+			my $uuid = $props->{'svm:uuid'};
+			$uuid =~ m{^[0-9a-f\-]{36,}}
+				or croak "doesn't look right - svm:uuid is '$uuid'";
+			sys('git-repo-config', 'svn.svkuuid', $uuid);
+		}
+	}
 	complete_url_ls_init($url, $_branches, '--branches/-b', '');
 	complete_url_ls_init($url, $_tags, '--tags/-t', 'tags/');
 }
@@ -2084,6 +2100,11 @@ sub check_repack {
 	}
 }
 
+sub get_svm_url {
+	chomp($SVM_URL = `git-repo-config --get svn.svkmirrorpath`);
+	chomp($SVM_UUID = `git-repo-config --get svn.svkuuid`);
+}
+
 sub set_commit_env {
 	my ($log_msg) = @_;
 	my $author = $log_msg->{author};
#4 lore raw
In reply to Sam Vilain
Quoted 33 lines · Sam Vilain

From: <[email protected]>

A manual test that sets up a repository that looks like an SVK depot, and then imports it to check that it looks like we mirrored the 'original' source.

There is also a minor modification to the git-svn test library shell file which sets a variable for the subversion repository's filesystem path.
---

 t/lib-git-svn.sh                   |    3 -
 t/t9107-git-svn-svk-mirrorpaths.sh |   92 ++++++++++++++++++++++++++++++++++++
 2 files changed, 93 insertions(+), 2 deletions(-)
t/lib-git-svn.sh +1 -2
index 63c6703..dffd1fb 100644
@@ -45,6 +45,5 @@ else
 	svnadmin create "$svnrepo"
 fi
 
+rawsvnrepo="$svnrepo"
 svnrepo="file://$svnrepo"
-
-
t/t9107-git-svn-svk-mirrorpaths.sh new file +0 -0
new file mode 100755
index 0000000..130e786

t/t9107-git-svn-svk-mirrorpaths.sh line 3

Quoted 3 lines
t/t9107-git-svn-svk-mirrorpaths.sh new file +3 -0
@@ -0,0 +1,92 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Eric Wong

Huh? I didn't write this test.

Quoted 10 lines
t/t9107-git-svn-svk-mirrorpaths.sh new file +10 -0
+test_description='git-svn on SVK mirror paths'
+. ./lib-git-svn.sh
+
+if test -n "$GIT_SVN_NO_LIB" && test "$GIT_SVN_NO_LIB" -ne 0
+then
+	echo 'Skipping: only implemented with SVN libraries'
+	test_done
+	exit 0
+fi
+

t/t9107-git-svn-svk-mirrorpaths.sh line 23

Quoted 10 lines
t/t9107-git-svn-svk-mirrorpaths.sh new file +10 -0
+# ok, people who don't have SVK installed probably don't care about
+# this test.
+
+# we set up the repository manually, because even if SVK is installed
+# it is difficult to use it in a way that is idempotent.
+
+# we are not yet testing merge tickets..
+
+uuid=b00bface-b1ff-c0ff-f0ff-b0bafe775e1e 
+url=https://really.slow.server.com/foobar 

I had to use --whitespace=strip with git-am above.

t/t9107-git-svn-svk-mirrorpaths.sh line 24

Quoted 1 line
t/t9107-git-svn-svk-mirrorpaths.sh new file +1 -0
+test_expect_success 'multi-fetch an SVK mirror path' "git-svn multi-fetch"

I had to use the following patch to get the multi-fetch test to pass with GIT_SVN_DELTA_FETCH=1. (I discovered it while running make -C t full-svn-test)

Is it safe to assume that svk-mirrored URLs will _always_ be file://? If so, then the delta fetching code should never be needed.

git-svn.perl +1 -2
@@ -3017,8 +3017,7 @@ sub libsvn_fetch_delta {
 	my $ed = SVN::Git::Fetcher->new({ c => $last_commit, q => $_q });
 	my $reporter = $SVN->do_update($rev, '', 1, $ed, $pool);
 	my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();
-	my (undef, $last_rev, undef) = cmt_metadata($last_commit);
-	$reporter->set_path('', $last_rev, 0, @lock, $pool);
+	$reporter->set_path('', ($rev - 1), 0, @lock, $pool);
 	$reporter->finish_report($pool);
 	$pool->clear;
 	unless ($ed->{git_commit_ok}) {

Also, unconditionally setting ($rev - 1) in the above patch disables the ability to do squashed history[1] imports (not sure if anybody cares about them, though).

[1] - like shallow clone, but shallow in the middle of history

t/t9107-git-svn-svk-mirrorpaths.sh line 25

Quoted 1 line
t/t9107-git-svn-svk-mirrorpaths.sh new file +1 -0
+test_expect_success 'got tag history OK' "test \`git-log --pretty=oneline remotes/tags/blah-1.0 | wc -l \` = 3"

But the above test breaks this one...

author
#5 lore raw

PATCH 4/5 git-svn: collect revision properties when fetching

In reply to Sam Vilain

From: Sam Vilain <[email protected]>

Perhaps there is information in the "revision properties" (unversioned metadata associated with commits) that will affect the way that we save the revision. Collect them.
---
Sorry for the long lines.

 git-svn.perl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
git-svn.perl +4 -0
index 800c579..70c34b0 100755
@@ -441,12 +441,16 @@ sub fetch_lib {
 					if ($last_commit) {
 						$log_msg = libsvn_fetch(
 							$last_commit, @_);
+						$log_msg->{revprops}
+							= $SVN->rev_proplist($log_msg->{revision});
 						$last_commit = git_commit(
 							$log_msg,
 							$last_commit,
 							@parents);
 					} else {
 						$log_msg = libsvn_new_tree(@_);
+						$log_msg->{revprops}
+							= $SVN->rev_proplist($log_msg->{revision});
 						$last_commit = git_commit(
 							$log_msg, @parents);
 					}
author
#6 lore raw

PATCH 5/5 git-svn: re-map repository URLs and UUIDs on SVK mirror paths

In reply to Sam Vilain

From: Sam Vilain <[email protected]>

If an SVN revision has a property, "svm:headrev", it is likely that the revision was created by "svk sync". The property contains a repository UUID and a revision. We want to make it look like we are mirroring the original URL, so introduce a helper function that returns the original identity trio, and use it when generating commit messages and dummy e-mail domains.
---

 git-svn.perl |   31 +++++++++++++++++++++++++++----
 1 files changed, 27 insertions(+), 4 deletions(-)
git-svn.perl +27 -4
index 70c34b0..13a1f24 100755
@@ -2076,8 +2076,9 @@ sub git_commit {
 								or croak $!;
 	print $msg_fh $log_msg->{msg} or croak $!;
 	unless ($_no_metadata) {
-		print $msg_fh "\ngit-svn-id: $SVN_URL\@$log_msg->{revision}",
-					" $SVN_UUID\n" or croak $!;
+		my ($url, $uuid, $rev) = svn_commit_id($log_msg);
+		print $msg_fh "\ngit-svn-id: $url\@$rev $uuid\n"
+			or croak $!;
 	}
 	$msg_fh->flush == 0 or croak $!;
 	close $msg_fh or croak $!;
@@ -2109,14 +2110,36 @@ sub get_svm_url {
 	chomp($SVM_UUID = `git-repo-config --get svn.svkuuid`);
 }
 
+sub svn_commit_id {
+	my $log_msg = shift;
+	my ($url, $uuid, $rev) = ($SVN_URL, $SVN_UUID, $log_msg->{revision});
+	my $svm_headrev = $log_msg->{revprops}{'svm:headrev'};
+	if ( $svm_headrev ) {
+		my ( $_uuid, $_rev) = split /:/, $svm_headrev;
+		chomp($_rev);
+		if ( !$SVM_URL ) {
+			get_svm_url();
+		}
+		if ( $_uuid ne $SVM_UUID ) {
+			warn "$uuid:$rev claims to be $_uuid:$_rev, but that's unknown";
+		} else {
+			($url, $uuid, $rev) = ($SVM_URL, $SVM_UUID, $_rev);
+		}
+	}
+	($url, $uuid, $rev);
+}
+
 sub set_commit_env {
 	my ($log_msg) = @_;
 	my $author = $log_msg->{author};
 	if (!defined $author || length $author == 0) {
 		$author = '(no author)';
 	}
-	my ($name,$email) = defined $users{$author} ?  @{$users{$author}}
-				: ($author,"$author\@$SVN_UUID");
+	my ($name,$email) = defined $users{$author} ? @{$users{$author}}
+		: do {
+			my (undef, $uuid, undef) = svn_commit_id($log_msg);
+			($author,"$author\@$uuid")
+		};
 	$ENV{GIT_AUTHOR_NAME} = $ENV{GIT_COMMITTER_NAME} = $name;
 	$ENV{GIT_AUTHOR_EMAIL} = $ENV{GIT_COMMITTER_EMAIL} = $email;
 	$ENV{GIT_AUTHOR_DATE} = $ENV{GIT_COMMITTER_DATE} = $log_msg->{date};
#7 lore raw

Re: [PATCH 5/5] git-svn: re-map repository URLs and UUIDs on SVK mirror paths

Quoted 8 lines · Sam Vilain

From: Sam Vilain <[email protected]>

If an SVN revision has a property, "svm:headrev", it is likely that the revision was created by "svk sync". The property contains a repository UUID and a revision. We want to make it look like we are mirroring the original URL, so introduce a helper function that returns the original identity trio, and use it when generating commit messages and dummy e-mail domains.

Upon further review, this would make 'git svn rebuild' behave unexpectedly (it would make the git-svn metadata, including .rev_db entries point to the original repo and not the SVK one). This may not necessarily be a big deal, however.

Also, incremental fetches (or fetching more than 1k sequential revisions) would probably fail. To fix this, read the offset of last entry in .rev_db instead of git-svn-id: from the last commit to get the last revision. But since rebuild won't work as expected; losing the .rev_db file means you wouldn't be able to fetch from the SVK repo anymore (but the original upstream one will be fine).

One last thing: feature should be made optional. I actually work day-to-day on a repository that was created with svm/SVN::Mirror, the original repository no longer exists; but the mirrored one still has these properties (I suppose I could remove the props server-side, but some people may not have the permissions).

author
#8 lore raw

Re: [PATCH 5/5] git-svn: re-map repository URLs and UUIDs on SVK mirror paths

Quoted 4 lines · Eric Wong

Upon further review, this would make 'git svn rebuild' behave unexpectedly (it would make the git-svn metadata, including .rev_db entries point to the original repo and not the SVK one). This may not necessarily be a big deal, however.

Yes, that's the idea; a 'rebuild' should set it up to pull from the original SVN repository directly. That probably needs some documentation...

Quoted 7 lines

Also, incremental fetches (or fetching more than 1k sequential revisions) would probably fail. To fix this, read the offset of last entry in .rev_db instead of git-svn-id: from the last commit to get the last revision. But since rebuild won't work as expected; losing the .rev_db file means you wouldn't be able to fetch from the SVK repo anymore (but the original upstream one will be fine).

Quoted 5 lines

One last thing: feature should be made optional. I actually work day-to-day on a repository that was created with svm/SVN::Mirror, the original repository no longer exists; but the mirrored one still has these properties (I suppose I could remove the props server-side, but some people may not have the permissions).

ok, I'll work on that and the other issues you highlighted... possibly the overhead of fetching the revprops during mirroring might hurt a little for people not doing this, too. Thanks for reviewing the patch!

Sam.

#9 lore raw

Re: [PATCH 5/5] git-svn: re-map repository URLs and UUIDs on SVK mirror paths

Quoted 14 lines · Sam Vilain
Quoted 7 lines · Eric Wong

Also, incremental fetches (or fetching more than 1k sequential revisions) would probably fail. To fix this, read the offset of last entry in .rev_db instead of git-svn-id: from the last commit to get the last revision. But since rebuild won't work as expected; losing the .rev_db file means you wouldn't be able to fetch from the SVK repo anymore (but the original upstream one will be fine).

Quoted 5 lines

One last thing: feature should be made optional. I actually work day-to-day on a repository that was created with svm/SVN::Mirror, the original repository no longer exists; but the mirrored one still has these properties (I suppose I could remove the props server-side, but some people may not have the permissions).

Quoted 3 lines

ok, I'll work on that and the other issues you highlighted... possibly the overhead of fetching the revprops during mirroring might hurt a little for people not doing this, too. Thanks for reviewing the patch!

For the git-svn in master using the delta fetcher; there's no additional overhead to fetch properties. I want to ditch the old non-delta fetching code (it's only a mild performance benefit when using local repositories) if I could get do_switch() working correctly.

#10 lore raw

PATCH 1/1 git-svn: use do_switch for --follow-parent if the SVN library supports it

do_switch works with the SVN Perl bindings after r22312 in the Subversion trunk. Since no released version of SVN currently supports it; we'll just autodetect it and enable its usage when a user has a recent-enough version of SVN.

---

Quoted 4 lines · Eric Wong

For the git-svn in master using the delta fetcher; there's no additional overhead to fetch properties. I want to ditch the old non-delta fetching code (it's only a mild performance benefit when using local repositories) if I could get do_switch() working correctly.

I don't think I can ditch the old code anytime soon. I was tempted to try using do_diff, but it appears SVN downloads the entire files (using get_file) and generates the diffs locally, negating any bandwidth saving it would have over the libsvn_fetch_full() path.

 git-svn.perl |   46 +++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 39 insertions(+), 7 deletions(-)
git-svn.perl +39 -7
index 747daf0..c907eb9 100755
@@ -72,7 +72,7 @@ my ($_revision,$_stdin,$_no_ignore_ext,$_no_stop_copy,$_help,$_rmdir,$_edit,
 	$_username, $_config_dir, $_no_auth_cache, $_xfer_delta,
 	$_pager, $_color);
 my (@_branch_from, %tree_map, %users, %rusers, %equiv);
-my ($_svn_co_url_revs, $_svn_pg_peg_revs);
+my ($_svn_co_url_revs, $_svn_pg_peg_revs, $_svn_can_do_switch);
 my @repo_path_split_cache;
 
 my %fc_opts = ( 'no-ignore-externals' => \$_no_ignore_ext,
@@ -2877,6 +2877,24 @@ sub libsvn_connect {
 	return $ra;
 }
 
+sub libsvn_can_do_switch {
+	unless (defined $_svn_can_do_switch) {
+		my $pool = SVN::Pool->new;
+		my $rep = eval {
+			$SVN->do_switch(1, '', 0, $SVN->{url},
+			                SVN::Delta::Editor->new, $pool);
+		};
+		if ($@) {
+			$_svn_can_do_switch = 0;
+		} else {
+			$rep->abort_report($pool);
+			$_svn_can_do_switch = 1;
+		}
+		$pool->clear;
+	}
+	$_svn_can_do_switch;
+}
+
 sub libsvn_dup_ra {
 	my ($ra) = @_;
 	SVN::Ra->new(map { $_ => $ra->{$_} } qw/config url
@@ -3198,12 +3216,26 @@ sub libsvn_find_parent_branch {
 		unlink $GIT_SVN_INDEX;
 		print STDERR "Found branch parent: ($GIT_SVN) $parent\n";
 		sys(qw/git-read-tree/, $parent);
-		# I can't seem to get do_switch() to work correctly with
-		# the SWIG interface (TypeError when passing switch_url...),
-		# so we'll unconditionally bypass the delta interface here
-		# for now
-		return libsvn_fetch_full($parent, $paths, $rev,
-					$author, $date, $msg);
+		unless (libsvn_can_do_switch()) {
+			return libsvn_fetch_full($parent, $paths, $rev,
+						$author, $date, $msg);
+		}
+		# do_switch works with svn/trunk >= r22312, but that is not
+		# included with SVN 1.4.2 (the latest version at the moment),
+		# so we can't rely on it.
+		my $ra = libsvn_connect("$url/$branch_from");
+		my $ed = SVN::Git::Fetcher->new({c => $parent, q => $_q});
+		my $pool = SVN::Pool->new;
+		my $reporter = $ra->do_switch($rev, '', 1, $SVN->{url},
+		                              $ed, $pool);
+		my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();
+		$reporter->set_path('', $r0, 0, @lock, $pool);
+		$reporter->finish_report($pool);
+		$pool->clear;
+		unless ($ed->{git_commit_ok}) {
+			die "SVN connection failed somewhere...\n";
+		}
+		return libsvn_log_entry($rev, $author, $date, $msg, [$parent]);
 	}
 	print STDERR "Nope, branch point not imported or unknown\n";
 	return undef;