Re: [PATCH] svn: Add && to t9107-git-svn-migrarte.sh

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

 



Adam Brewster <adambrewster@xxxxxxxxx> wrote:
> Eric,
> 
> Any thoughts on the other patch?
> 
> Am I close or is there a better way to go about this?
> 
> I didn't really know what to do with .git/svn/*.  The easy answer is
> 
>   mkdir -p $GIT_DIR/svn/refs/remotes &&
>   mv $GIT_DIR/svn/* $GIT_DIR/svn/refs/remotes

That should be acceptable.  I don't think it's worth the trouble to
force the majority of existing repos to move over to the new layout
(in case the user wants to switch between multiple versions of git).

> From the comments in the Migration module, it seems like that's frowned
> upon, so I came up with looking for .rev_map (or .rev_db) in both
> locations (.git/svn/git-svn and .git/svn/refs/remotes/svn) and letting
> it stay in whichever location it already exists.  (The next email has a
> slightly improved version of the patch.)
> 
> This solution is particularly inelegant in it's handling of
> unhandled.log, but as far as I know that file is unused.

I'd still rather avoid creating unhandled.log in two places and
having an unused set of directories.

I think the following change on top of yours would make most
sense:

diff --git a/git-svn.perl b/git-svn.perl
index cafd7fe..1117f28 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -3325,8 +3325,18 @@ sub _new {
 	}
 	$_[1] = $repo_id;
 	my $dir = "$ENV{GIT_DIR}/svn/$ref_id";
+
+	# Older repos imported by us used $GIT_DIR/svn/foo instead of
+	# $GIT_DIR/svn/refs/remotes/foo when tracking refs/remotes/foo
+	if ($ref_id =~ m{^refs/remotes/(.*)}) {
+		my $old_dir = "$ENV{GIT_DIR}/svn/$1";
+		if (-d $old_dir && ! -d $dir) {
+			$dir = $old_dir;
+		}
+	}
+
 	$_[3] = $path = '' unless (defined $path);
-	mkpath(["$ENV{GIT_DIR}/svn/$ref_id"]);
+	mkpath([$dir]);
 	bless {
 		ref_id => $ref_id, dir => $dir, index => "$dir/index",
 	        path => $path, config => "$ENV{GIT_DIR}/svn/config",
@@ -3355,16 +3365,7 @@ sub rev_db_path {
 sub map_path {
 	my ($self, $uuid) = @_;
 	$uuid ||= $self->ra_uuid;
-	my $map_path = "$self->{map_root}.$uuid";
-	return $map_path if ( -f $map_path );
-
-	my $db_path = $map_path;
-	$db_path =~ s/\.rev_map/.rev_db/;
-	return $map_path if ( -f $db_path );
-
-	return $1 if ($map_path =~ m(refs/remotes/(.*)) && -f $1);
-	return $1 if ($db_path =~ m(refs/remotes/(.*)) && -f $1);
-	return $map_path;
+	"$self->{map_root}.$uuid";
 }
 
 sub uri_encode {
----

The map_path() changes you originally made didn't work, either, since
the -f $1 never took GIT_DIR or GIT_DIR/svn into account.

> There's also the problem of what to do if someone has a ref called
> ref/remotes/refs/remotes/..., but that seems unlikely enough to not
> cause concern.

I concur.  There's always cases where things will be ambiguous
and cases where a user is just asking for trouble :)

I think the below is a good enough test case to for compatibility
against existing repos.  Let me know what you think, thanks!

diff --git a/t/t9144-git-svn-old-rev_map.sh b/t/t9144-git-svn-old-rev_map.sh
new file mode 100755
index 0000000..7600a35
--- /dev/null
+++ b/t/t9144-git-svn-old-rev_map.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+#
+# Copyright (c) 2009 Eric Wong
+
+test_description='git svn old rev_map preservd'
+. ./lib-git-svn.sh
+
+test_expect_success 'setup test repository with old layout' '
+	mkdir i &&
+	(cd i && > a) &&
+	svn_cmd import -m- i "$svnrepo" &&
+	git svn init "$svnrepo" &&
+	git svn fetch &&
+	test -d .git/svn/refs/remotes/git-svn/ &&
+	! test -e .git/svn/git-svn/ &&
+	mv .git/svn/refs/remotes/git-svn .git/svn/ &&
+	rm -r .git/svn/refs
+'
+
+test_expect_success 'old layout continues to work' '
+	svn_cmd import -m- i "$svnrepo/b" &&
+	git svn rebase &&
+	echo a >> b/a &&
+	git add b/a &&
+	git commit -m- -a &&
+	git svn dcommit &&
+	! test -d .git/svn/refs/ &&
+	test -e .git/svn/git-svn/
+'
+
+test_done
-- 
Eric Wong
--
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]