threads / discuss / 44290

problem with git worktree and git svn

Subject: problem with git worktree and git svn

## tl;dr

10 messages between Oct 12, 2016 and Oct 26, 2016.

replies: 9people: 5as markdown or json

Mathieu Arnold· Oct 12, 2016, 14:45 UTC · lore
Hi,

I'm a FreeBSD ports developer, and while our main repository is in Subversion, some of us use git-svn a lot for development.

I discovered git worktree earlier this week, and I found it a great asset to be able to have more than one branch of my worktree accessible at the same time...

Anyway, back to my problem, the way git-svn works, is that it looks for a directory named "svn" in its gitdir and if it is not present, decide the repository is using git-svn version 1 (whatever that is) and goes to parse all the revisions to recreate the svn directory. So I can only use git svn commands in my main worktree, the one with the real gitdir. To fix that, all I had to do is to add a symlink named svn in each worktree's gitdir and pointing to ../../svn.

I think all that needs to happen is that when adding a new worktree, if the main git directory has a "svn" directory, add a symlink to it in the worktree's gitdir.

-- 
Mathieu Arnold
Stefan Beller· Oct 12, 2016, 21:05 UTC · re: Mathieu Arnold · lore

Re: problem with git worktree and git svn

+cc Duy who knows about worktree, +cc Eric Wong, who knows about git-svn

On Wed, Oct 12, 2016 at 7:45 AM, Mathieu Arnold <mat@freebsd.org> wrote:
Show 17 quoted lines
> Hi,
>
> I'm a FreeBSD ports developer, and while our main repository is in
> Subversion, some of us use git-svn a lot for development.
>
> I discovered git worktree earlier this week, and I found it a great
> asset to be able to have more than one branch of my worktree accessible
> at the same time...
>
> Anyway, back to my problem, the way git-svn works, is that it looks for
> a directory named "svn" in its gitdir and if it is not present, decide
> the repository is using git-svn version 1 (whatever that is) and goes to
> parse all the revisions to recreate the svn directory.
> So I can only use git svn commands in my main worktree, the one with the
> real gitdir.
> To fix that, all I had to do is to add a symlink named svn in each
> worktree's gitdir and pointing to ../../svn.

For some definition of fix. ;) Sure it fixes your local setup now, but would we want to use that as well here? My gut reaction:

* not all platforms know symlinks
* IIRC there is some worktree magic that tells you the "main" dir,
  so if that was used in git-svn instead it should "just work".
Show 9 quoted lines
>
> I think all that needs to happen is that when adding a new worktree, if
> the main git directory has a "svn" directory, add a symlink to it in the
> worktree's gitdir.
>
> --
> Mathieu Arnold
>
>
Eric Wong· Oct 13, 2016, 01:52 UTC · re: Stefan Beller · lore

Re: problem with git worktree and git svn

Stefan Beller <sbeller@google.com> wrote:
Show 12 quoted lines
> On Wed, Oct 12, 2016 at 7:45 AM, Mathieu Arnold <mat@freebsd.org> wrote:
> 
> > I discovered git worktree earlier this week, and I found it a great
> > asset to be able to have more than one branch of my worktree accessible
> > at the same time...
> >
> > Anyway, back to my problem, the way git-svn works, is that it looks for
> > a directory named "svn" in its gitdir and if it is not present, decide
> > the repository is using git-svn version 1 (whatever that is) and goes to
> > parse all the revisions to recreate the svn directory.
> > So I can only use git svn commands in my main worktree, the one with the
> > real gitdir.

Right, I haven't updated git-svn to be worktree-aware, yet, but a work-in-progress is below

Show 15 quoted lines
> > To fix that, all I had to do is to add a symlink named svn in each
> > worktree's gitdir and pointing to ../../svn.
> 
> For some definition of fix. ;)
> Sure it fixes your local setup now, but would we want to use that as well here?
> My gut reaction:
> 
> * not all platforms know symlinks
> * IIRC there is some worktree magic that tells you the "main" dir,
>   so if that was used in git-svn instead it should "just work".
> 
> >
> > I think all that needs to happen is that when adding a new worktree, if
> > the main git directory has a "svn" directory, add a symlink to it in the
> > worktree's gitdir.

I'm fairly sure the worktree C code should not care about how git-svn works, but rather git-svn should be made aware of worktree bits...

I haven't studied worktrees much nor do I use git-svn much, but the following seems to work for fetch/rebase/dcommit:

------------8<-----------
Subject: [PATCH] git-svn: WIP worktree awareness
---
 perl/Git/SVN.pm           | 29 ++++++++++++++++++++---------
 perl/Git/SVN/Migration.pm | 23 ++++++++++++-----------
 2 files changed, 32 insertions(+), 20 deletions(-)
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 018beb8..267cc09 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -807,10 +807,20 @@ sub get_fetch_range {
 	(++$min, $max);
 }
 
+sub svn_dir {
+	my $git_dir = scalar @_ ? $_[0] : $ENV{GIT_DIR};
+	my $common = $ENV{GIT_COMMON_DIR} || "$git_dir/commondir";
+	$git_dir .= '/'.::file_to_s($common) if -e $common;
+	my $svn_dir = $git_dir . '/svn';
+	$svn_dir =~ tr!/!/!s;
+	$svn_dir;
+}
+
 sub tmp_config {
 	my (@args) = @_;
-	my $old_def_config = "$ENV{GIT_DIR}/svn/config";
-	my $config = "$ENV{GIT_DIR}/svn/.metadata";
+	my $svn_dir = svn_dir();
+	my $old_def_config = "$svn_dir/config";
+	my $config = "$svn_dir/.metadata";
 	if (! -f $config && -f $old_def_config) {
 		rename $old_def_config, $config or
 		       die "Failed rename $old_def_config => $config: $!\n";
@@ -1671,7 +1681,7 @@ sub tie_for_persistent_memoization {
 		return if $memoized;
 		$memoized = 1;
 
-		my $cache_path = "$ENV{GIT_DIR}/svn/.caches/";
+		my $cache_path = svn_dir() . '/.caches/';
 		mkpath([$cache_path]) unless -d $cache_path;
 
 		my %lookup_svn_merge_cache;
@@ -1712,7 +1722,7 @@ sub tie_for_persistent_memoization {
 	sub clear_memoized_mergeinfo_caches {
 		die "Only call this method in non-memoized context" if ($memoized);
 
-		my $cache_path = "$ENV{GIT_DIR}/svn/.caches/";
+		my $cache_path = svn_dir() . '/.caches/';
 		return unless -d $cache_path;
 
 		for my $cache_file (("$cache_path/lookup_svn_merge",
@@ -2446,12 +2456,13 @@ sub _new {
 		             "refs/remotes/$prefix$default_ref_id";
 	}
 	$_[1] = $repo_id;
-	my $dir = "$ENV{GIT_DIR}/svn/$ref_id";
+	my $svn_dir = svn_dir();
+	my $dir = "$svn_dir/$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
+	# Older repos imported by us used $svn_dir/foo instead of
+	# $svn_dir/refs/remotes/foo when tracking refs/remotes/foo
 	if ($ref_id =~ m{^refs/remotes/(.+)}) {
-		my $old_dir = "$ENV{GIT_DIR}/svn/$1";
+		my $old_dir = "$svn_dir/$1";
 		if (-d $old_dir && ! -d $dir) {
 			$dir = $old_dir;
 		}
@@ -2461,7 +2472,7 @@ sub _new {
 	mkpath([$dir]);
 	my $obj = bless {
 		ref_id => $ref_id, dir => $dir, index => "$dir/index",
-	        config => "$ENV{GIT_DIR}/svn/config",
+	        config => "$svn_dir/config",
 	        map_root => "$dir/.rev_map", repo_id => $repo_id }, $class;
 
 	# Ensure it gets canonicalized
diff --git a/perl/Git/SVN/Migration.pm b/perl/Git/SVN/Migration.pm
index cf6ffa7..887fd93 100644
--- a/perl/Git/SVN/Migration.pm
+++ b/perl/Git/SVN/Migration.pm
@@ -45,6 +45,7 @@ use Git qw(
 	command_output_pipe
 	command_close_pipe
 );
+use Git::SVN;
 
 sub migrate_from_v0 {
 	my $git_dir = $ENV{GIT_DIR};
@@ -82,7 +83,7 @@ sub migrate_from_v1 {
 	my $git_dir = $ENV{GIT_DIR};
 	my $migrated = 0;
 	return $migrated unless -d $git_dir;
-	my $svn_dir = "$git_dir/svn";
+	my $svn_dir = Git::SVN::svn_dir($git_dir);
 
 	# just in case somebody used 'svn' as their $id at some point...
 	return $migrated if -d $svn_dir && ! -f "$svn_dir/info/url";
@@ -100,24 +101,23 @@ sub migrate_from_v1 {
 		next unless -f "$git_dir/$x/info/url";
 		my $u = eval { ::file_to_s("$git_dir/$x/info/url") };
 		next unless $u;
-		my $dn = dirname("$git_dir/svn/$x");
+		my $dn = dirname("$svn_dir/$x");
 		mkpath([$dn]) unless -d $dn;
 		if ($x eq 'svn') { # they used 'svn' as GIT_SVN_ID:
-			mkpath(["$git_dir/svn/svn"]);
+			mkpath(["$svn_dir/svn"]);
 			print STDERR " - $git_dir/$x/info => ",
-			                "$git_dir/svn/$x/info\n";
-			rename "$git_dir/$x/info", "$git_dir/svn/$x/info" or
+			                "$svn_dir/$x/info\n";
+			rename "$git_dir/$x/info", "$svn_dir/$x/info" or
 			       croak "$!: $x";
 			# don't worry too much about these, they probably
 			# don't exist with repos this old (save for index,
 			# and we can easily regenerate that)
 			foreach my $f (qw/unhandled.log index .rev_db/) {
-				rename "$git_dir/$x/$f", "$git_dir/svn/$x/$f";
+				rename "$git_dir/$x/$f", "$svn_dir/$x/$f";
 			}
 		} else {
-			print STDERR " - $git_dir/$x => $git_dir/svn/$x\n";
-			rename "$git_dir/$x", "$git_dir/svn/$x" or
-			       croak "$!: $x";
+			print STDERR " - $git_dir/$x => $svn_dir/$x\n";
+			rename "$git_dir/$x", "$svn_dir/$x" or croak "$!: $x";
 		}
 		$migrated++;
 	}
@@ -139,9 +139,10 @@ sub read_old_urls {
 			push @dir, $_;
 		}
 	}
+	my $svn_dir = Git::SVN::svn_dir();
 	foreach (@dir) {
 		my $x = $_;
-		$x =~ s!^\Q$ENV{GIT_DIR}\E/svn/!!o;
+		$x =~ s!^\Q$svn_dir\E/!!o;
 		read_old_urls($l_map, $x, $_);
 	}
 }
@@ -150,7 +151,7 @@ sub migrate_from_v2 {
 	my @cfg = command(qw/config -l/);
 	return if grep /^svn-remote\..+\.url=/, @cfg;
 	my %l_map;
-	read_old_urls(\%l_map, '', "$ENV{GIT_DIR}/svn");
+	read_old_urls(\%l_map, '', Git::SVN::svn_dir());
 	my $migrated = 0;
 
 	require Git::SVN;
-- 
EW
Duy Nguyen· Oct 13, 2016, 10:29 UTC · re: Eric Wong · lore

Re: problem with git worktree and git svn

On Thu, Oct 13, 2016 at 8:52 AM, Eric Wong <e@80x24.org> wrote:
Show 8 quoted lines
> +sub svn_dir {
> +       my $git_dir = scalar @_ ? $_[0] : $ENV{GIT_DIR};
> +       my $common = $ENV{GIT_COMMON_DIR} || "$git_dir/commondir";
> +       $git_dir .= '/'.::file_to_s($common) if -e $common;
> +       my $svn_dir = $git_dir . '/svn';
> +       $svn_dir =~ tr!/!/!s;
> +       $svn_dir;
> +}
If this is shell script, this function could be just
svn_dir() {
    git rev-parse --git-path svn
}

which should give you correct path in either single or multi-worktree context and you don't need to bother with details like $GIT_COMMON_DIR. But I don't know how Perl bindings are implemented, I don't know if we have something similar (or easy to add it, like Git::git_path()).

I don't know much about git-svn, but from the look of it I agree replacing $ENV{GIT_DIR}/svn with svn_dir() should fix it, assuming that you don't hard code $ENV{GIT_DIR}/blahblah somewhere else. I don't see any other potential problems (from multi-worktree perspective).

-- 
Duy
Eric Wong· Oct 13, 2016, 20:55 UTC · re: Duy Nguyen · lore

Re: problem with git worktree and git svn

Duy Nguyen <pclouds@gmail.com> wrote:
Show 16 quoted lines
> On Thu, Oct 13, 2016 at 8:52 AM, Eric Wong <e@80x24.org> wrote:
> > +sub svn_dir {
> > +       my $git_dir = scalar @_ ? $_[0] : $ENV{GIT_DIR};
> > +       my $common = $ENV{GIT_COMMON_DIR} || "$git_dir/commondir";
> > +       $git_dir .= '/'.::file_to_s($common) if -e $common;
> > +       my $svn_dir = $git_dir . '/svn';
> > +       $svn_dir =~ tr!/!/!s;
> > +       $svn_dir;
> > +}
> 
> 
> If this is shell script, this function could be just
> 
> svn_dir() {
>     git rev-parse --git-path svn
> }
Ah, thanks; I missed --git-path.  I will do this in Git/SVN.pm:
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -808,12 +808,7 @@ sub get_fetch_range {
 }
 
 sub svn_dir {
-	my $git_dir = scalar @_ ? $_[0] : $ENV{GIT_DIR};
-	my $common = $ENV{GIT_COMMON_DIR} || "$git_dir/commondir";
-	$git_dir .= '/'.::file_to_s($common) if -e $common;
-	my $svn_dir = $git_dir . '/svn';
-	$svn_dir =~ tr!/!/!s;
-	$svn_dir;
+	command_oneline(qw(rev-parse --git-path svn));
 }
 
 sub tmp_config {

> which should give you correct path in either single or multi-worktree
> context and you don't need to bother with details like
> $GIT_COMMON_DIR. But I don't know how Perl bindings are implemented, I
> don't know if we have something similar (or easy to add it, like
> Git::git_path()).

I'm not sure it's necessary given the convenience of command_oneline,
and I'd rather avoid the overhead of documenting+supporting a new API
for Git.pm

> I don't know much about git-svn, but from the look of it I agree
> replacing $ENV{GIT_DIR}/svn with svn_dir() should fix it, assuming
> that you don't hard code $ENV{GIT_DIR}/blahblah somewhere else. I
> don't see any other potential problems (from multi-worktree
> perspective).

I think there was a place where $GIT_DIR/config was used, but
only for documentation purposes.
Eric Wong· Oct 14, 2016, 01:46 UTC · re: Eric Wong · lore

[PATCH 0/2] git-svn: implement "git worktree" awareness

+Cc Jakub since gitweb could probably take advantage of get_record from the first patch, too. I'm not completely sure about the API for this, though.

The following changes since commit 3cdd5d19178a54d2e51b5098d43b57571241d0ab:
  Sync with maint (2016-10-11 14:55:48 -0700)
are available in the git repository at:
  git://bogomips.org/git-svn.git svn-wt
for you to fetch changes up to 112423eb905cf28c9445781a7647ba590d597ab3:
  git-svn: "git worktree" awareness (2016-10-14 01:36:12 +0000)
----------------------------------------------------------------
Eric Wong (2):
      git-svn: reduce scope of input record separator change
      git-svn: "git worktree" awareness
 git-svn.perl              | 13 +++++++------
 perl/Git.pm               | 16 +++++++++++++++-
 perl/Git/SVN.pm           | 24 +++++++++++++++---------
 perl/Git/SVN/Editor.pm    | 12 +++++-------
 perl/Git/SVN/Fetcher.pm   | 15 +++++----------
 perl/Git/SVN/Migration.pm | 37 ++++++++++++++++++++++---------------
 6 files changed, 69 insertions(+), 48 deletions(-)
-- 
EW
Eric Wong· Oct 14, 2016, 01:46 UTC · re: Eric Wong · lore

[PATCH 2/2] git-svn: "git worktree" awareness

git-svn internals were previously not aware of repository layout differences for users of the "git worktree" command. Introduce this awareness by using "git rev-parse --git-path" instead of relying on outdated uses of GIT_DIR and friends.

Thanks-to: Duy Nguyen <pclouds@gmail.com>
Reported-by: Mathieu Arnold <mat@freebsd.org>
Signed-off-by: Eric Wong <e@80x24.org>
---
 git-svn.perl              |  9 +++++----
 perl/Git/SVN.pm           | 24 +++++++++++++++---------
 perl/Git/SVN/Migration.pm | 37 ++++++++++++++++++++++---------------
 3 files changed, 42 insertions(+), 28 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 6d1a142..fa42364 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -1700,7 +1700,7 @@ sub cmd_gc {
 		     "files will not be compressed.\n";
 	}
 	File::Find::find({ wanted => \&gc_directory, no_chdir => 1},
-			 "$ENV{GIT_DIR}/svn");
+			 Git::SVN::svn_dir());
 }
 
 ########################### utility functions #########################
@@ -1734,7 +1734,7 @@ sub post_fetch_checkout {
 	return unless verify_ref('HEAD^0');
 
 	return if $ENV{GIT_DIR} !~ m#^(?:.*/)?\.git$#;
-	my $index = $ENV{GIT_INDEX_FILE} || "$ENV{GIT_DIR}/index";
+	my $index = command_oneline(qw(rev-parse --git-path index));
 	return if -f $index;
 
 	return if command_oneline(qw/rev-parse --is-inside-work-tree/) eq 'false';
@@ -1836,8 +1836,9 @@ sub get_tree_from_treeish {
 sub get_commit_entry {
 	my ($treeish) = shift;
 	my %log_entry = ( log => '', tree => get_tree_from_treeish($treeish) );
-	my $commit_editmsg = "$ENV{GIT_DIR}/COMMIT_EDITMSG";
-	my $commit_msg = "$ENV{GIT_DIR}/COMMIT_MSG";
+	my @git_path = qw(rev-parse --git-path);
+	my $commit_editmsg = command_oneline(@git_path, 'COMMIT_EDITMSG');
+	my $commit_msg = command_oneline(@git_path, 'COMMIT_MSG');
 	open my $log_fh, '>', $commit_editmsg or croak $!;
 
 	my $type = command_oneline(qw/cat-file -t/, $treeish);
diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm
index 018beb8..499e84b 100644
--- a/perl/Git/SVN.pm
+++ b/perl/Git/SVN.pm
@@ -807,10 +807,15 @@ sub get_fetch_range {
 	(++$min, $max);
 }
 
+sub svn_dir {
+	command_oneline(qw(rev-parse --git-path svn));
+}
+
 sub tmp_config {
 	my (@args) = @_;
-	my $old_def_config = "$ENV{GIT_DIR}/svn/config";
-	my $config = "$ENV{GIT_DIR}/svn/.metadata";
+	my $svn_dir = svn_dir();
+	my $old_def_config = "$svn_dir/config";
+	my $config = "$svn_dir/.metadata";
 	if (! -f $config && -f $old_def_config) {
 		rename $old_def_config, $config or
 		       die "Failed rename $old_def_config => $config: $!\n";
@@ -1671,7 +1676,7 @@ sub tie_for_persistent_memoization {
 		return if $memoized;
 		$memoized = 1;
 
-		my $cache_path = "$ENV{GIT_DIR}/svn/.caches/";
+		my $cache_path = svn_dir() . '/.caches/';
 		mkpath([$cache_path]) unless -d $cache_path;
 
 		my %lookup_svn_merge_cache;
@@ -1712,7 +1717,7 @@ sub tie_for_persistent_memoization {
 	sub clear_memoized_mergeinfo_caches {
 		die "Only call this method in non-memoized context" if ($memoized);
 
-		my $cache_path = "$ENV{GIT_DIR}/svn/.caches/";
+		my $cache_path = svn_dir() . '/.caches/';
 		return unless -d $cache_path;
 
 		for my $cache_file (("$cache_path/lookup_svn_merge",
@@ -2446,12 +2451,13 @@ sub _new {
 		             "refs/remotes/$prefix$default_ref_id";
 	}
 	$_[1] = $repo_id;
-	my $dir = "$ENV{GIT_DIR}/svn/$ref_id";
+	my $svn_dir = svn_dir();
+	my $dir = "$svn_dir/$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
+	# Older repos imported by us used $svn_dir/foo instead of
+	# $svn_dir/refs/remotes/foo when tracking refs/remotes/foo
 	if ($ref_id =~ m{^refs/remotes/(.+)}) {
-		my $old_dir = "$ENV{GIT_DIR}/svn/$1";
+		my $old_dir = "$svn_dir/$1";
 		if (-d $old_dir && ! -d $dir) {
 			$dir = $old_dir;
 		}
@@ -2461,7 +2467,7 @@ sub _new {
 	mkpath([$dir]);
 	my $obj = bless {
 		ref_id => $ref_id, dir => $dir, index => "$dir/index",
-	        config => "$ENV{GIT_DIR}/svn/config",
+	        config => "$svn_dir/config",
 	        map_root => "$dir/.rev_map", repo_id => $repo_id }, $class;
 
 	# Ensure it gets canonicalized
diff --git a/perl/Git/SVN/Migration.pm b/perl/Git/SVN/Migration.pm
index cf6ffa7..dc90f6a 100644
--- a/perl/Git/SVN/Migration.pm
+++ b/perl/Git/SVN/Migration.pm
@@ -44,7 +44,9 @@ use Git qw(
 	command_noisy
 	command_output_pipe
 	command_close_pipe
+	command_oneline
 );
+use Git::SVN;
 
 sub migrate_from_v0 {
 	my $git_dir = $ENV{GIT_DIR};
@@ -55,7 +57,9 @@ sub migrate_from_v0 {
 		chomp;
 		my ($id, $orig_ref) = ($_, $_);
 		next unless $id =~ s#^refs/heads/(.+)-HEAD$#$1#;
-		next unless -f "$git_dir/$id/info/url";
+		my $info_url = command_oneline(qw(rev-parse --git-path),
+						"$id/info/url");
+		next unless -f $info_url;
 		my $new_ref = "refs/remotes/$id";
 		if (::verify_ref("$new_ref^0")) {
 			print STDERR "W: $orig_ref is probably an old ",
@@ -82,7 +86,7 @@ sub migrate_from_v1 {
 	my $git_dir = $ENV{GIT_DIR};
 	my $migrated = 0;
 	return $migrated unless -d $git_dir;
-	my $svn_dir = "$git_dir/svn";
+	my $svn_dir = Git::SVN::svn_dir();
 
 	# just in case somebody used 'svn' as their $id at some point...
 	return $migrated if -d $svn_dir && ! -f "$svn_dir/info/url";
@@ -97,27 +101,28 @@ sub migrate_from_v1 {
 		my $x = $_;
 		next unless $x =~ s#^refs/remotes/##;
 		chomp $x;
-		next unless -f "$git_dir/$x/info/url";
-		my $u = eval { ::file_to_s("$git_dir/$x/info/url") };
+		my $info_url = command_oneline(qw(rev-parse --git-path),
+						"$x/info/url");
+		next unless -f $info_url;
+		my $u = eval { ::file_to_s($info_url) };
 		next unless $u;
-		my $dn = dirname("$git_dir/svn/$x");
+		my $dn = dirname("$svn_dir/$x");
 		mkpath([$dn]) unless -d $dn;
 		if ($x eq 'svn') { # they used 'svn' as GIT_SVN_ID:
-			mkpath(["$git_dir/svn/svn"]);
+			mkpath(["$svn_dir/svn"]);
 			print STDERR " - $git_dir/$x/info => ",
-			                "$git_dir/svn/$x/info\n";
-			rename "$git_dir/$x/info", "$git_dir/svn/$x/info" or
+			                "$svn_dir/$x/info\n";
+			rename "$git_dir/$x/info", "$svn_dir/$x/info" or
 			       croak "$!: $x";
 			# don't worry too much about these, they probably
 			# don't exist with repos this old (save for index,
 			# and we can easily regenerate that)
 			foreach my $f (qw/unhandled.log index .rev_db/) {
-				rename "$git_dir/$x/$f", "$git_dir/svn/$x/$f";
+				rename "$git_dir/$x/$f", "$svn_dir/$x/$f";
 			}
 		} else {
-			print STDERR " - $git_dir/$x => $git_dir/svn/$x\n";
-			rename "$git_dir/$x", "$git_dir/svn/$x" or
-			       croak "$!: $x";
+			print STDERR " - $git_dir/$x => $svn_dir/$x\n";
+			rename "$git_dir/$x", "$svn_dir/$x" or croak "$!: $x";
 		}
 		$migrated++;
 	}
@@ -139,9 +144,10 @@ sub read_old_urls {
 			push @dir, $_;
 		}
 	}
+	my $svn_dir = Git::SVN::svn_dir();
 	foreach (@dir) {
 		my $x = $_;
-		$x =~ s!^\Q$ENV{GIT_DIR}\E/svn/!!o;
+		$x =~ s!^\Q$svn_dir\E/!!o;
 		read_old_urls($l_map, $x, $_);
 	}
 }
@@ -150,7 +156,7 @@ sub migrate_from_v2 {
 	my @cfg = command(qw/config -l/);
 	return if grep /^svn-remote\..+\.url=/, @cfg;
 	my %l_map;
-	read_old_urls(\%l_map, '', "$ENV{GIT_DIR}/svn");
+	read_old_urls(\%l_map, '', Git::SVN::svn_dir());
 	my $migrated = 0;
 
 	require Git::SVN;
@@ -239,7 +245,8 @@ sub minimize_connections {
 		}
 	}
 	if (@emptied) {
-		my $file = $ENV{GIT_CONFIG} || "$ENV{GIT_DIR}/config";
+		my $file = $ENV{GIT_CONFIG} ||
+			command_oneline(qw(rev-parse --git-path config));
 		print STDERR <<EOF;
 The following [svn-remote] sections in your config file ($file) are empty
 and can be safely removed:
-- 
EW
Eric Wong· Oct 14, 2016, 01:46 UTC · re: Eric Wong · lore

[PATCH 1/2] git-svn: reduce scope of input record separator change

Reducing the scope of where we change the record separator ($/) avoids bugs in calls which rely on the input record separator further down, such as the 'chomp' usage in command_oneline.

This is necessary for a future change to git-svn, but exists in Git.pm since it seems useful for gitweb and our other Perl scripts, too.

Signed-off-by: Eric Wong <e@80x24.org>
---
 git-svn.perl            |  4 ++--
 perl/Git.pm             | 16 +++++++++++++++-
 perl/Git/SVN/Editor.pm  | 12 +++++-------
 perl/Git/SVN/Fetcher.pm | 15 +++++----------
 4 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/git-svn.perl b/git-svn.perl
index 4d41d22..6d1a142 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -44,6 +44,7 @@ use Git qw(
 	command_close_pipe
 	command_bidi_pipe
 	command_close_bidi_pipe
+	get_record
 );
 
 BEGIN {
@@ -1880,10 +1881,9 @@ sub get_commit_entry {
 	{
 		require Encode;
 		# SVN requires messages to be UTF-8 when entering the repo
-		local $/;
 		open $log_fh, '<', $commit_msg or croak $!;
 		binmode $log_fh;
-		chomp($log_entry{log} = <$log_fh>);
+		chomp($log_entry{log} = get_record($log_fh, undef));
 
 		my $enc = Git::config('i18n.commitencoding') || 'UTF-8';
 		my $msg = $log_entry{log};
diff --git a/perl/Git.pm b/perl/Git.pm
index ce7e4e8..d2c5a8d 100644
--- a/perl/Git.pm
+++ b/perl/Git.pm
@@ -59,7 +59,7 @@ require Exporter;
                 command_bidi_pipe command_close_bidi_pipe
                 version exec_path html_path hash_object git_cmd_try
                 remote_refs prompt
-                get_tz_offset
+                get_tz_offset get_record
                 credential credential_read credential_write
                 temp_acquire temp_is_locked temp_release temp_reset temp_path);
 
@@ -538,6 +538,20 @@ sub get_tz_offset {
 	return sprintf("%s%02d%02d", $sign, (gmtime(abs($t - $gm)))[2,1]);
 }
 
+=item get_record ( FILEHANDLE, INPUT_RECORD_SEPARATOR )
+
+Read one record from FILEHANDLE delimited by INPUT_RECORD_SEPARATOR,
+removing any trailing INPUT_RECORD_SEPARATOR.
+
+=cut
+
+sub get_record {
+	my ($fh, $rs) = @_;
+	local $/ = $rs;
+	my $rec = <$fh>;
+	chomp $rec if defined $rs;
+	$rec;
+}
 
 =item prompt ( PROMPT , ISPASSWORD  )
 
diff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm
index 4c4199a..0df16ed 100644
--- a/perl/Git/SVN/Editor.pm
+++ b/perl/Git/SVN/Editor.pm
@@ -7,7 +7,9 @@ use SVN::Delta;
 use Carp qw/croak/;
 use Git qw/command command_oneline command_noisy command_output_pipe
            command_input_pipe command_close_pipe
-           command_bidi_pipe command_close_bidi_pipe/;
+           command_bidi_pipe command_close_bidi_pipe
+           get_record/;
+
 BEGIN {
 	@ISA = qw(SVN::Delta::Editor);
 }
@@ -57,11 +59,9 @@ sub generate_diff {
 	push @diff_tree, "-l$_rename_limit" if defined $_rename_limit;
 	push @diff_tree, $tree_a, $tree_b;
 	my ($diff_fh, $ctx) = command_output_pipe(@diff_tree);
-	local $/ = "\0";
 	my $state = 'meta';
 	my @mods;
-	while (<$diff_fh>) {
-		chomp $_; # this gets rid of the trailing "\0"
+	while (defined($_ = get_record($diff_fh, "\0"))) {
 		if ($state eq 'meta' && /^:(\d{6})\s(\d{6})\s
 					($::sha1)\s($::sha1)\s
 					([MTCRAD])\d*$/xo) {
@@ -173,9 +173,7 @@ sub rmdirs {
 
 	my ($fh, $ctx) = command_output_pipe(qw/ls-tree --name-only -r -z/,
 	                                     $self->{tree_b});
-	local $/ = "\0";
-	while (<$fh>) {
-		chomp;
+	while (defined($_ = get_record($fh, "\0"))) {
 		my @dn = split m#/#, $_;
 		while (pop @dn) {
 			delete $rm->{join '/', @dn};
diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm
index d8c21ad..64e900a 100644
--- a/perl/Git/SVN/Fetcher.pm
+++ b/perl/Git/SVN/Fetcher.pm
@@ -9,7 +9,8 @@ use Carp qw/croak/;
 use File::Basename qw/dirname/;
 use Git qw/command command_oneline command_noisy command_output_pipe
            command_input_pipe command_close_pipe
-           command_bidi_pipe command_close_bidi_pipe/;
+           command_bidi_pipe command_close_bidi_pipe
+           get_record/;
 BEGIN {
 	@ISA = qw(SVN::Delta::Editor);
 }
@@ -86,11 +87,9 @@ sub _mark_empty_symlinks {
 	my $printed_warning;
 	chomp(my $empty_blob = `git hash-object -t blob --stdin < /dev/null`);
 	my ($ls, $ctx) = command_output_pipe(qw/ls-tree -r -z/, $cmt);
-	local $/ = "\0";
 	my $pfx = defined($switch_path) ? $switch_path : $git_svn->path;
 	$pfx .= '/' if length($pfx);
-	while (<$ls>) {
-		chomp;
+	while (defined($_ = get_record($ls, "\0"))) {
 		s/\A100644 blob $empty_blob\t//o or next;
 		unless ($printed_warning) {
 			print STDERR "Scanning for empty symlinks, ",
@@ -179,9 +178,7 @@ sub delete_entry {
 		my ($ls, $ctx) = command_output_pipe(qw/ls-tree
 		                                     -r --name-only -z/,
 				                     $tree);
-		local $/ = "\0";
-		while (<$ls>) {
-			chomp;
+		while (defined($_ = get_record($ls, "\0"))) {
 			my $rmpath = "$gpath/$_";
 			$self->{gii}->remove($rmpath);
 			print "\tD\t$rmpath\n" unless $::_q;
@@ -247,9 +244,7 @@ sub add_directory {
 		my ($ls, $ctx) = command_output_pipe(qw/ls-tree
 		                                     -r --name-only -z/,
 				                     $self->{c});
-		local $/ = "\0";
-		while (<$ls>) {
-			chomp;
+		while (defined($_ = get_record($ls, "\0"))) {
 			$self->{gii}->remove($_);
 			print "\tD\t$_\n" unless $::_q;
 			push @deleted_gpath, $gpath;
-- 
EW
Eric Wong· Oct 26, 2016, 20:02 UTC · re: Eric Wong · lore

Re: [PATCH 0/2] git-svn: implement "git worktree" awareness

Eric Wong <e@80x24.org> wrote:
> +Cc Jakub since gitweb could probably take advantage of get_record
> from the first patch, too.  I'm not completely sure about the API
> for this, though.
Jakub: ping?
+Cc: Junio, too.  I'm hoping to have this in 2.11.
Show 24 quoted lines
> The following changes since commit 3cdd5d19178a54d2e51b5098d43b57571241d0ab:
> 
>   Sync with maint (2016-10-11 14:55:48 -0700)
> 
> are available in the git repository at:
> 
>   git://bogomips.org/git-svn.git svn-wt
> 
> for you to fetch changes up to 112423eb905cf28c9445781a7647ba590d597ab3:
> 
>   git-svn: "git worktree" awareness (2016-10-14 01:36:12 +0000)
> 
> ----------------------------------------------------------------
> Eric Wong (2):
>       git-svn: reduce scope of input record separator change
>       git-svn: "git worktree" awareness
> 
>  git-svn.perl              | 13 +++++++------
>  perl/Git.pm               | 16 +++++++++++++++-
>  perl/Git/SVN.pm           | 24 +++++++++++++++---------
>  perl/Git/SVN/Editor.pm    | 12 +++++-------
>  perl/Git/SVN/Fetcher.pm   | 15 +++++----------
>  perl/Git/SVN/Migration.pm | 37 ++++++++++++++++++++++---------------
>  6 files changed, 69 insertions(+), 48 deletions(-)
Junio C Hamano· Oct 26, 2016, 21:17 UTC · re: Eric Wong · lore

Re: [PATCH 0/2] git-svn: implement "git worktree" awareness

Eric Wong <e@80x24.org> writes:
Show 8 quoted lines
> Eric Wong <e@80x24.org> wrote:
>> +Cc Jakub since gitweb could probably take advantage of get_record
>> from the first patch, too.  I'm not completely sure about the API
>> for this, though.
>
> Jakub: ping?
>
> +Cc: Junio, too.  I'm hoping to have this in 2.11.

I somehow was hoping that I can pull this as part of git-svn updates for the upcoming release without having to even think about it (I did read the patch when they were posted and did not find anything wrong with them, fwiw).

Show 11 quoted lines
>> The following changes since commit 3cdd5d19178a54d2e51b5098d43b57571241d0ab:
>> 
>>   Sync with maint (2016-10-11 14:55:48 -0700)
>> 
>> are available in the git repository at:
>> 
>>   git://bogomips.org/git-svn.git svn-wt
>> 
>> for you to fetch changes up to 112423eb905cf28c9445781a7647ba590d597ab3:
>> 
>>   git-svn: "git worktree" awareness (2016-10-14 01:36:12 +0000)
Thanks.

← back to recent threads