# problem with git worktree and git svn 10 messages from 2016-10-12 to 2016-10-26. Participants: Mathieu Arnold, Stefan Beller, Eric Wong, Duy Nguyen, Junio C Hamano. Thread: https://gitlist.dev/t/44290 ## Mathieu Arnold, 2016-10-12 14:45 Subject: problem with git worktree and git svn Message-ID: <6c83c905-b10a-7f54-873f-54186faacfc8@FreeBSD.org> URL: https://gitlist.dev/e/6c83c905-b10a-7f54-873f-54186faacfc8%40FreeBSD.org ``` 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, 2016-10-12 21:05 Subject: Re: problem with git worktree and git svn Message-ID: URL: https://gitlist.dev/e/CAGZ79kZo5W1r0s26G3foB7caP6%2Bu66mdzqzyneqXBX_B7A0RKg%40mail.gmail.com In-Reply-To: <6c83c905-b10a-7f54-873f-54186faacfc8@FreeBSD.org> ``` +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 wrote: > 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". > > 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, 2016-10-13 01:52 Subject: Re: problem with git worktree and git svn Message-ID: <20161013015233.GA18001@whir> URL: https://gitlist.dev/e/20161013015233.GA18001%40whir In-Reply-To: ``` Stefan Beller wrote: > On Wed, Oct 12, 2016 at 7:45 AM, Mathieu Arnold 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 > > 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, 2016-10-13 10:29 Subject: Re: problem with git worktree and git svn Message-ID: URL: https://gitlist.dev/e/CACsJy8ADmHLW5ba4bVTGN9ccqpMDNFgkqE_5JvnqZ0BxWcaCvw%40mail.gmail.com In-Reply-To: <20161013015233.GA18001@whir> ``` On Thu, Oct 13, 2016 at 8:52 AM, Eric Wong 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 } 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, 2016-10-13 20:55 Subject: Re: problem with git worktree and git svn Message-ID: <20161013205555.GA599@dcvr> URL: https://gitlist.dev/e/20161013205555.GA599%40dcvr In-Reply-To: ``` Duy Nguyen wrote: > On Thu, Oct 13, 2016 at 8:52 AM, Eric Wong 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, 2016-10-14 01:46 Subject: [PATCH 0/2] git-svn: implement "git worktree" awareness Message-ID: <20161014014623.15223-1-e@80x24.org> URL: https://gitlist.dev/e/20161014014623.15223-1-e%4080x24.org In-Reply-To: <20161013205555.GA599@dcvr> ``` +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, 2016-10-14 01:46 Subject: [PATCH 2/2] git-svn: "git worktree" awareness Message-ID: <20161014014623.15223-3-e@80x24.org> URL: https://gitlist.dev/e/20161014014623.15223-3-e%4080x24.org In-Reply-To: <20161014014623.15223-1-e@80x24.org> ``` 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 Reported-by: Mathieu Arnold Signed-off-by: Eric Wong --- 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 < URL: https://gitlist.dev/e/20161014014623.15223-2-e%4080x24.org In-Reply-To: <20161014014623.15223-1-e@80x24.org> ``` 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 --- 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, 2016-10-26 20:02 Subject: Re: [PATCH 0/2] git-svn: implement "git worktree" awareness Message-ID: <20161026200248.GA28105@starla> URL: https://gitlist.dev/e/20161026200248.GA28105%40starla In-Reply-To: <20161014014623.15223-1-e@80x24.org> ``` Eric Wong 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. > 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, 2016-10-26 21:17 Subject: Re: [PATCH 0/2] git-svn: implement "git worktree" awareness Message-ID: URL: https://gitlist.dev/e/xmqq8ttabyjt.fsf%40gitster.mtv.corp.google.com In-Reply-To: <20161026200248.GA28105@starla> ``` Eric Wong writes: > Eric Wong 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). >> 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. ```