* Make git-svn Use accessors for paths and urls
@ 2012-07-27 20:00 Michael G. Schwern
2012-07-27 20:00 ` [PATCH 1/5] Make Git::SVN use accessors internally for path Michael G. Schwern
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Michael G. Schwern @ 2012-07-27 20:00 UTC (permalink / raw)
To: git, gitster; +Cc: robbat2, bwalton, normalperson, jrnieder, schwern
This patch series gives Git::SVN and Git::SVN::Ra accessors for
path and url and then makes the rest of the code use them, rather
than grab at $obj->{path} and $obj->{url}. This then will give
us the control necessary to canonicalize them as early as
possible (done in the next patch series).
There are plenty of other places in the code which will benefit
from accessors and functions, but those will come later. path
and url were the most obvious.
This is a refactoring and has no functional change. All git-svn
tests pass with SVN 1.6 for each patch.
This goes on top of my previous patch series to extract other
classes. That hasn't been reviewed yet, but both that and this are
a simple patch series and I figure we can review a bit ahead.
This is the last refactoring patch series. After this bugs, start
getting fixed.
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/5] Make Git::SVN use accessors internally for path. 2012-07-27 20:00 Make git-svn Use accessors for paths and urls Michael G. Schwern @ 2012-07-27 20:00 ` Michael G. Schwern 2012-09-17 9:04 ` Jonathan Nieder 2012-07-27 20:00 ` [PATCH 2/5] Make Git::SVN use an accessor for URLs internally Michael G. Schwern ` (4 subsequent siblings) 5 siblings, 1 reply; 18+ messages in thread From: Michael G. Schwern @ 2012-07-27 20:00 UTC (permalink / raw) To: git, gitster; +Cc: robbat2, bwalton, normalperson, jrnieder, schwern From: "Michael G. Schwern" <schwern@pobox.com> Then later it can be canonicalized automatically rather than everywhere its used. Later patch will make other things use it. --- perl/Git/SVN.pm | 87 +++++++++++++++++++++++++++++++++++++-------------------- 1 file changed, 56 insertions(+), 31 deletions(-) diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index b8b3474..0aff9d0 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -314,12 +314,12 @@ sub init_remote_config { print STDERR "Using higher level of URL: ", "$url => $min_url\n"; } - my $old_path = $self->{path}; - $self->{path} = $url; - $self->{path} =~ s!^\Q$min_url\E(/|$)!!; + my $old_path = $self->path; + $url =~ s!^\Q$min_url\E(/|$)!!; if (length $old_path) { - $self->{path} .= "/$old_path"; + $url .= "/$old_path"; } + $self->path($url); $url = $min_url; } } @@ -343,11 +343,13 @@ sub init_remote_config { unless ($no_write) { command_noisy('config', "svn-remote.$self->{repo_id}.url", $url); - $self->{path} =~ s{^/}{}; - $self->{path} =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg; + my $path = $self->path; + $path =~ s{^/}{}; + $path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg; + $self->path($path); command_noisy('config', '--add', "svn-remote.$self->{repo_id}.fetch", - "$self->{path}:".$self->refname); + $self->path.":".$self->refname); } $self->{url} = $url; } @@ -435,17 +437,22 @@ sub new { } } my $self = _new($class, $repo_id, $ref_id, $path); - if (!defined $self->{path} || !length $self->{path}) { + if (!defined $self->path || !length $self->path) { my $fetch = command_oneline('config', '--get', "svn-remote.$repo_id.fetch", ":$ref_id\$") or die "Failed to read \"svn-remote.$repo_id.fetch\" ", "\":$ref_id\$\" in config\n"; - ($self->{path}, undef) = split(/\s*:\s*/, $fetch); + my($path) = split(/\s*:\s*/, $fetch); + $self->path($path); + } + { + my $path = $self->path; + $path =~ s{/+}{/}g; + $path =~ s{\A/}{}; + $path =~ s{/\z}{}; + $self->path($path); } - $self->{path} =~ s{/+}{/}g; - $self->{path} =~ s{\A/}{}; - $self->{path} =~ s{/\z}{}; $self->{url} = command_oneline('config', '--get', "svn-remote.$repo_id.url") or die "Failed to read \"svn-remote.$repo_id.url\" in config\n"; @@ -567,7 +574,7 @@ sub _set_svm_vars { } my $r = $ra->get_latest_revnum; - my $path = $self->{path}; + my $path = $self->path; my %tried; while (length $path) { unless ($tried{"$self->{url}/$path"}) { @@ -728,7 +735,7 @@ sub prop_walk { $path =~ s#^/*#/#g; my $p = $path; # Strip the irrelevant part of the path. - $p =~ s#^/+\Q$self->{path}\E(/|$)#/#; + $p =~ s#^/+\Q@{[$self->path]}\E(/|$)#/#; # Ensure the path is terminated by a `/'. $p =~ s#/*$#/#; @@ -749,7 +756,7 @@ sub prop_walk { foreach (sort keys %$dirent) { next if $dirent->{$_}->{kind} != $SVN::Node::dir; - $self->prop_walk($self->{path} . $p . $_, $rev, $sub); + $self->prop_walk($self->path . $p . $_, $rev, $sub); } } @@ -920,19 +927,19 @@ sub rewrite_uuid { sub metadata_url { my ($self) = @_; ($self->rewrite_root || $self->{url}) . - (length $self->{path} ? '/' . $self->{path} : ''); + (length $self->path ? '/' . $self->path : ''); } sub full_url { my ($self) = @_; - $self->{url} . (length $self->{path} ? '/' . $self->{path} : ''); + $self->{url} . (length $self->path ? '/' . $self->path : ''); } sub full_pushurl { my ($self) = @_; if ($self->{pushurl}) { - return $self->{pushurl} . (length $self->{path} ? '/' . - $self->{path} : ''); + return $self->{pushurl} . (length $self->path ? '/' . + $self->path : ''); } else { return $self->full_url; } @@ -1048,20 +1055,20 @@ sub do_git_commit { sub match_paths { my ($self, $paths, $r) = @_; - return 1 if $self->{path} eq ''; - if (my $path = $paths->{"/$self->{path}"}) { + return 1 if $self->path eq ''; + if (my $path = $paths->{"/".$self->path}) { return ($path->{action} eq 'D') ? 0 : 1; } - $self->{path_regex} ||= qr/^\/\Q$self->{path}\E\//; + $self->{path_regex} ||= qr{^/\Q@{[$self->path]}\E/}; if (grep /$self->{path_regex}/, keys %$paths) { return 1; } my $c = ''; - foreach (split m#/#, $self->{path}) { + foreach (split m#/#, $self->path) { $c .= "/$_"; next unless ($paths->{$c} && ($paths->{$c}->{action} =~ /^[AR]$/)); - if ($self->ra->check_path($self->{path}, $r) == + if ($self->ra->check_path($self->path, $r) == $SVN::Node::dir) { return 1; } @@ -1075,14 +1082,14 @@ sub find_parent_branch { unless (defined $paths) { my $err_handler = $SVN::Error::handler; $SVN::Error::handler = \&Git::SVN::Ra::skip_unknown_revs; - $self->ra->get_log([$self->{path}], $rev, $rev, 0, 1, 1, + $self->ra->get_log([$self->path], $rev, $rev, 0, 1, 1, sub { $paths = $_[0] }); $SVN::Error::handler = $err_handler; } return undef unless defined $paths; # look for a parent from another branch: - my @b_path_components = split m#/#, $self->{path}; + my @b_path_components = split m#/#, $self->path; my @a_path_components; my $i; while (@b_path_components) { @@ -1235,7 +1242,7 @@ sub mkemptydirs { close $fh; } - my $strip = qr/\A\Q$self->{path}\E(?:\/|$)/; + my $strip = qr/\A\Q@{[$self->path]}\E(?:\/|$)/; foreach my $d (sort keys %empty_dirs) { $d = uri_decode($d); $d =~ s/$strip//; @@ -1858,7 +1865,7 @@ sub make_log_entry { $commit_email ||= "$author\@$uuid"; } elsif ($self->use_svnsync_props) { my $full_url = $self->svnsync->{url}; - $full_url .= "/$self->{path}" if length $self->{path}; + $full_url .= "/".$self->path if length $self->path; remove_username($full_url); my $uuid = $self->svnsync->{uuid}; $log_entry{metadata} = "$full_url\@$rev $uuid"; @@ -1905,7 +1912,7 @@ sub set_tree { tree_b => $tree, editor_cb => sub { $self->set_tree_cb($log_entry, $tree, @_) }, - svn_path => $self->{path} ); + svn_path => $self->path ); if (!Git::SVN::Editor->new(\%ed_opts)->apply_diff) { print "No changes\nr$self->{last_rev} = $tree\n"; } @@ -2276,10 +2283,28 @@ sub _new { $_[3] = $path = '' unless (defined $path); mkpath([$dir]); - bless { + my $obj = bless { ref_id => $ref_id, dir => $dir, index => "$dir/index", - path => $path, config => "$ENV{GIT_DIR}/svn/config", + config => "$ENV{GIT_DIR}/svn/config", map_root => "$dir/.rev_map", repo_id => $repo_id }, $class; + + # Ensure it gets canonicalized + $obj->path($path); + + return $obj; +} + + +sub path { + my $self = shift; + + if( @_ ) { + my $path = shift; + $self->{path} = $path; + return; + } + + return $self->{path}; } # for read-only access of old .rev_db formats -- 1.7.11.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] Make Git::SVN use accessors internally for path. 2012-07-27 20:00 ` [PATCH 1/5] Make Git::SVN use accessors internally for path Michael G. Schwern @ 2012-09-17 9:04 ` Jonathan Nieder 2012-09-17 9:08 ` [FYI/PATCH 1/5] Git::SVN: introduce path accessor Jonathan Nieder ` (4 more replies) 0 siblings, 5 replies; 18+ messages in thread From: Jonathan Nieder @ 2012-09-17 9:04 UTC (permalink / raw) To: Eric Wong; +Cc: git, gitster, robbat2, bwalton, Michael G. Schwern Hi Eric et al, Michael G. Schwern wrote: > Then later it can be canonicalized automatically rather than everywhere > its used. > > Later patch will make other things use it. Wow am I slow. I've finally got around to starting to parse these patches to apply to a 1.7.10.y tree so they can (hopefully) be part of Debian 7.0 when it comes out. Do I understand correctly that this patch splits logically into the following steps? The result is only cosmetically different from the original patch --- interdiff below the shortlog. The completeness of the conversion to accessors is checked by renaming the underlying variable in patch 5. Jonathan Nieder (1): Git::SVN: rename private path field Michael G. Schwern (4): Git::SVN: introduce path accessor Git::SVN: use accessor to read path Git::SVN: use accessor to write path Git::SVN::_new: use accessor to write path field git-svn.perl | 12 +++---- perl/Git/SVN.pm | 84 ++++++++++++++++++++++++++++------------------- perl/Git/SVN/Fetcher.pm | 2 +- perl/Git/SVN/Ra.pm | 8 ++--- 4 files changed, 62 insertions(+), 44 deletions(-) --- diff --git c/git-svn.perl w/git-svn.perl index 5711c571..af7d5308 100755 --- c/git-svn.perl +++ w/git-svn.perl @@ -1195,7 +1195,7 @@ sub cmd_show_ignore { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; print STDOUT "\n# $path\n"; my $s = $props->{'svn:ignore'} or return; @@ -1211,7 +1211,7 @@ sub cmd_show_externals { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; print STDOUT "\n# $path\n"; my $s = $props->{'svn:externals'} or return; @@ -1226,7 +1226,7 @@ sub cmd_create_ignore { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; # $path is of the form /path/to/dir/ $path = '.' . $path; @@ -1294,7 +1294,7 @@ sub get_svnprops { $path = $cmd_dir_prefix . $path; fatal("No such file or directory: $path") unless -e $path; my $is_dir = -d $path ? 1 : 0; - $path = $gs->{path} . '/' . $path; + $path = $gs->path . '/' . $path; # canonicalize the path (otherwise libsvn will abort or fail to # find the file) @@ -1396,7 +1396,7 @@ sub cmd_commit_diff { "the command-line\n", $usage); } $url = $gs->{url}; - $svn_path = $gs->{path}; + $svn_path = $gs->path; } unless (defined $_revision) { fatal("-r|--revision is a required argument\n", $usage); @@ -1670,7 +1670,7 @@ sub complete_url_ls_init { "wanted to set to: $gs->{url}\n"; } command_oneline('config', $k, $gs->{url}) unless $orig_url; - my $remote_path = "$gs->{path}/$repo_path"; + my $remote_path = $gs->path . "/$repo_path"; $remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg; $remote_path =~ s#/+#/#g; $remote_path =~ s#^/##g; diff --git c/perl/Git/SVN.pm w/perl/Git/SVN.pm index a93ac61b..3aa20109 100644 --- c/perl/Git/SVN.pm +++ w/perl/Git/SVN.pm @@ -437,22 +437,19 @@ sub new { } } my $self = _new($class, $repo_id, $ref_id, $path); - if (!defined $self->path || !length $self->path) { + $path = $self->path; + if (!defined $path || !length $path) { my $fetch = command_oneline('config', '--get', "svn-remote.$repo_id.fetch", ":$ref_id\$") or die "Failed to read \"svn-remote.$repo_id.fetch\" ", "\":$ref_id\$\" in config\n"; - my($path) = split(/\s*:\s*/, $fetch); - $self->path($path); - } - { - my $path = $self->path; - $path =~ s{/+}{/}g; - $path =~ s{\A/}{}; - $path =~ s{/\z}{}; - $self->path($path); + ($path, undef) = split(/\s*:\s*/, $fetch); } + $path =~ s{/+}{/}g; + $path =~ s{\A/}{}; + $path =~ s{/\z}{}; + $self->path($path); $self->{url} = command_oneline('config', '--get', "svn-remote.$repo_id.url") or die "Failed to read \"svn-remote.$repo_id.url\" in config\n"; @@ -1059,7 +1056,7 @@ sub match_paths { if (my $path = $paths->{"/".$self->path}) { return ($path->{action} eq 'D') ? 0 : 1; } - $self->{path_regex} ||= qr{^/\Q@{[$self->path]}\E/}; + $self->{path_regex} ||= qr/^\/\Q@{[$self->path]}\E\//; if (grep /$self->{path_regex}/, keys %$paths) { return 1; } @@ -1121,7 +1118,7 @@ sub find_parent_branch { ($base, $head) = parse_revision_argument(0, $r); } else { if ($r0 < $r) { - $gs->ra->get_log([$gs->{path}], $r0 + 1, $r, 1, + $gs->ra->get_log([$gs->path], $r0 + 1, $r, 1, 0, 1, sub { $base = $_[1] - 1 }); } } @@ -1143,7 +1140,7 @@ sub find_parent_branch { # at the moment), so we can't rely on it $self->{last_rev} = $r0; $self->{last_commit} = $parent; - $ed = Git::SVN::Fetcher->new($self, $gs->{path}); + $ed = Git::SVN::Fetcher->new($self, $gs->path); $gs->ra->gs_do_switch($r0, $rev, $gs, $self->full_url, $ed) or die "SVN connection failed somewhere...\n"; @@ -2287,10 +2284,7 @@ sub _new { ref_id => $ref_id, dir => $dir, index => "$dir/index", config => "$ENV{GIT_DIR}/svn/config", map_root => "$dir/.rev_map", repo_id => $repo_id }, $class; - - # Ensure it gets canonicalized $obj->path($path); - return $obj; } diff --git c/perl/Git/SVN/Fetcher.pm w/perl/Git/SVN/Fetcher.pm index 76fae9bc..046a7a2f 100644 --- c/perl/Git/SVN/Fetcher.pm +++ w/perl/Git/SVN/Fetcher.pm @@ -83,7 +83,7 @@ sub _mark_empty_symlinks { 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}; + my $pfx = defined($switch_path) ? $switch_path : $git_svn->path; $pfx .= '/' if length($pfx); while (<$ls>) { chomp; diff --git c/perl/Git/SVN/Ra.pm w/perl/Git/SVN/Ra.pm index 23ff43e8..64d00672 100644 --- c/perl/Git/SVN/Ra.pm +++ w/perl/Git/SVN/Ra.pm @@ -246,7 +246,7 @@ sub get_commit_editor { sub gs_do_update { my ($self, $rev_a, $rev_b, $gs, $editor) = @_; my $new = ($rev_a == $rev_b); - my $path = $gs->{path}; + my $path = $gs->path; if ($new && -e $gs->{index}) { unlink $gs->{index} or die @@ -282,7 +282,7 @@ sub gs_do_update { # svn_ra_reparent didn't work before 1.4) sub gs_do_switch { my ($self, $rev_a, $rev_b, $gs, $url_b, $editor) = @_; - my $path = $gs->{path}; + my $path = $gs->path; my $pool = SVN::Pool->new; my $full_url = $self->{url}; @@ -326,7 +326,7 @@ sub longest_common_path { my $common_max = scalar @$gsv; foreach my $gs (@$gsv) { - my @tmp = split m#/#, $gs->{path}; + my @tmp = split m#/#, $gs->path; my $p = ''; foreach (@tmp) { $p .= length($p) ? "/$_" : $_; @@ -407,7 +407,7 @@ sub gs_fetch_loop_common { } $SVN::Error::handler = $err_handler; - my %exists = map { $_->{path} => $_ } @$gsv; + my %exists = map { $_->path => $_ } @$gsv; foreach my $r (sort {$a <=> $b} keys %revs) { my ($paths, $logged) = @{$revs{$r}}; --- diff --git c/git-svn.perl w/git-svn.perl index 5711c571..af7d5308 100755 --- c/git-svn.perl +++ w/git-svn.perl @@ -1195,7 +1195,7 @@ sub cmd_show_ignore { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; print STDOUT "\n# $path\n"; my $s = $props->{'svn:ignore'} or return; @@ -1211,7 +1211,7 @@ sub cmd_show_externals { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; print STDOUT "\n# $path\n"; my $s = $props->{'svn:externals'} or return; @@ -1226,7 +1226,7 @@ sub cmd_create_ignore { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; # $path is of the form /path/to/dir/ $path = '.' . $path; @@ -1294,7 +1294,7 @@ sub get_svnprops { $path = $cmd_dir_prefix . $path; fatal("No such file or directory: $path") unless -e $path; my $is_dir = -d $path ? 1 : 0; - $path = $gs->{path} . '/' . $path; + $path = $gs->path . '/' . $path; # canonicalize the path (otherwise libsvn will abort or fail to # find the file) @@ -1396,7 +1396,7 @@ sub cmd_commit_diff { "the command-line\n", $usage); } $url = $gs->{url}; - $svn_path = $gs->{path}; + $svn_path = $gs->path; } unless (defined $_revision) { fatal("-r|--revision is a required argument\n", $usage); @@ -1670,7 +1670,7 @@ sub complete_url_ls_init { "wanted to set to: $gs->{url}\n"; } command_oneline('config', $k, $gs->{url}) unless $orig_url; - my $remote_path = "$gs->{path}/$repo_path"; + my $remote_path = $gs->path . "/$repo_path"; $remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg; $remote_path =~ s#/+#/#g; $remote_path =~ s#^/##g; diff --git c/perl/Git/SVN.pm w/perl/Git/SVN.pm index a93ac61b..3aa20109 100644 --- c/perl/Git/SVN.pm +++ w/perl/Git/SVN.pm @@ -437,22 +437,19 @@ sub new { } } my $self = _new($class, $repo_id, $ref_id, $path); - if (!defined $self->path || !length $self->path) { + $path = $self->path; + if (!defined $path || !length $path) { my $fetch = command_oneline('config', '--get', "svn-remote.$repo_id.fetch", ":$ref_id\$") or die "Failed to read \"svn-remote.$repo_id.fetch\" ", "\":$ref_id\$\" in config\n"; - my($path) = split(/\s*:\s*/, $fetch); - $self->path($path); - } - { - my $path = $self->path; - $path =~ s{/+}{/}g; - $path =~ s{\A/}{}; - $path =~ s{/\z}{}; - $self->path($path); + ($path, undef) = split(/\s*:\s*/, $fetch); } + $path =~ s{/+}{/}g; + $path =~ s{\A/}{}; + $path =~ s{/\z}{}; + $self->path($path); $self->{url} = command_oneline('config', '--get', "svn-remote.$repo_id.url") or die "Failed to read \"svn-remote.$repo_id.url\" in config\n"; @@ -1059,7 +1056,7 @@ sub match_paths { if (my $path = $paths->{"/".$self->path}) { return ($path->{action} eq 'D') ? 0 : 1; } - $self->{path_regex} ||= qr{^/\Q@{[$self->path]}\E/}; + $self->{path_regex} ||= qr/^\/\Q@{[$self->path]}\E\//; if (grep /$self->{path_regex}/, keys %$paths) { return 1; } @@ -1121,7 +1118,7 @@ sub find_parent_branch { ($base, $head) = parse_revision_argument(0, $r); } else { if ($r0 < $r) { - $gs->ra->get_log([$gs->{path}], $r0 + 1, $r, 1, + $gs->ra->get_log([$gs->path], $r0 + 1, $r, 1, 0, 1, sub { $base = $_[1] - 1 }); } } @@ -1143,7 +1140,7 @@ sub find_parent_branch { # at the moment), so we can't rely on it $self->{last_rev} = $r0; $self->{last_commit} = $parent; - $ed = Git::SVN::Fetcher->new($self, $gs->{path}); + $ed = Git::SVN::Fetcher->new($self, $gs->path); $gs->ra->gs_do_switch($r0, $rev, $gs, $self->full_url, $ed) or die "SVN connection failed somewhere...\n"; @@ -2287,10 +2284,7 @@ sub _new { ref_id => $ref_id, dir => $dir, index => "$dir/index", config => "$ENV{GIT_DIR}/svn/config", map_root => "$dir/.rev_map", repo_id => $repo_id }, $class; - - # Ensure it gets canonicalized $obj->path($path); - return $obj; } diff --git c/perl/Git/SVN/Fetcher.pm w/perl/Git/SVN/Fetcher.pm index 76fae9bc..046a7a2f 100644 --- c/perl/Git/SVN/Fetcher.pm +++ w/perl/Git/SVN/Fetcher.pm @@ -83,7 +83,7 @@ sub _mark_empty_symlinks { 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}; + my $pfx = defined($switch_path) ? $switch_path : $git_svn->path; $pfx .= '/' if length($pfx); while (<$ls>) { chomp; diff --git c/perl/Git/SVN/Ra.pm w/perl/Git/SVN/Ra.pm index 23ff43e8..64d00672 100644 --- c/perl/Git/SVN/Ra.pm +++ w/perl/Git/SVN/Ra.pm @@ -246,7 +246,7 @@ sub get_commit_editor { sub gs_do_update { my ($self, $rev_a, $rev_b, $gs, $editor) = @_; my $new = ($rev_a == $rev_b); - my $path = $gs->{path}; + my $path = $gs->path; if ($new && -e $gs->{index}) { unlink $gs->{index} or die @@ -282,7 +282,7 @@ sub gs_do_update { # svn_ra_reparent didn't work before 1.4) sub gs_do_switch { my ($self, $rev_a, $rev_b, $gs, $url_b, $editor) = @_; - my $path = $gs->{path}; + my $path = $gs->path; my $pool = SVN::Pool->new; my $full_url = $self->{url}; @@ -326,7 +326,7 @@ sub longest_common_path { my $common_max = scalar @$gsv; foreach my $gs (@$gsv) { - my @tmp = split m#/#, $gs->{path}; + my @tmp = split m#/#, $gs->path; my $p = ''; foreach (@tmp) { $p .= length($p) ? "/$_" : $_; @@ -407,7 +407,7 @@ sub gs_fetch_loop_common { } $SVN::Error::handler = $err_handler; - my %exists = map { $_->{path} => $_ } @$gsv; + my %exists = map { $_->path => $_ } @$gsv; foreach my $r (sort {$a <=> $b} keys %revs) { my ($paths, $logged) = @{$revs{$r}}; ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [FYI/PATCH 1/5] Git::SVN: introduce path accessor 2012-09-17 9:04 ` Jonathan Nieder @ 2012-09-17 9:08 ` Jonathan Nieder 2012-09-17 9:09 ` [FYI/PATCH 2/5] Git::SVN: use accessor to read path Jonathan Nieder ` (3 subsequent siblings) 4 siblings, 0 replies; 18+ messages in thread From: Jonathan Nieder @ 2012-09-17 9:08 UTC (permalink / raw) To: Eric Wong; +Cc: git, gitster, robbat2, bwalton, Michael G. Schwern From: Michael G. Schwern <schwern@pobox.com> Date: Fri, 27 Jul 2012 13:00:48 -0700 Each Git::SVN handle has a (base) URL and a (relative) path pointing to the top-level directory of the branch it handles. Introduce a getter and setter for the path as preparation for automatically canonicalizing it when reading or writing. For example, instead of $oldpath = $gs->{path}; $gs->{path} = $url; $gs->{path} =~ s!^\Q$min_url\E(/|$)!!; now you can write $oldpath = $gs->path; $url =~ s!^\Q$min_url\E(/|$)!!; $gs->path($url); [jn: split from a larger patch] Signed-off-by: Eric Wong <normalperson@yhbt.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- perl/Git/SVN.pm | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index b8b34744..268e0e84 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -2282,6 +2282,18 @@ sub _new { map_root => "$dir/.rev_map", repo_id => $repo_id }, $class; } +sub path { + my $self = shift; + + if (@_) { + my $path = shift; + $self->{path} = $path; + return; + } + + return $self->{path}; +} + # for read-only access of old .rev_db formats sub unlink_rev_db_symlink { my ($self) = @_; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [FYI/PATCH 2/5] Git::SVN: use accessor to read path 2012-09-17 9:04 ` Jonathan Nieder 2012-09-17 9:08 ` [FYI/PATCH 1/5] Git::SVN: introduce path accessor Jonathan Nieder @ 2012-09-17 9:09 ` Jonathan Nieder 2012-09-17 9:10 ` [FYI/PATCH 3/5] Git::SVN: use accessor to write path Jonathan Nieder ` (2 subsequent siblings) 4 siblings, 0 replies; 18+ messages in thread From: Jonathan Nieder @ 2012-09-17 9:09 UTC (permalink / raw) To: Eric Wong; +Cc: git, gitster, robbat2, bwalton, Michael G. Schwern From: Michael G. Schwern <schwern@pobox.com> Date: Fri, 27 Jul 2012 13:00:48 -0700 This patch only touches the simplest cases that simply read the Git::SVN field rather than assigning to or applying a substitution to it. Code to change found by searching for the term {path}. [jn: extracted from a larger patch] Signed-off-by: Eric Wong <normalperson@yhbt.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- git-svn.perl | 12 ++++++------ perl/Git/SVN.pm | 44 ++++++++++++++++++++++---------------------- perl/Git/SVN/Fetcher.pm | 2 +- perl/Git/SVN/Ra.pm | 8 ++++---- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/git-svn.perl b/git-svn.perl index 5711c571..af7d5308 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -1195,7 +1195,7 @@ sub cmd_show_ignore { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; print STDOUT "\n# $path\n"; my $s = $props->{'svn:ignore'} or return; @@ -1211,7 +1211,7 @@ sub cmd_show_externals { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; print STDOUT "\n# $path\n"; my $s = $props->{'svn:externals'} or return; @@ -1226,7 +1226,7 @@ sub cmd_create_ignore { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; # $path is of the form /path/to/dir/ $path = '.' . $path; @@ -1294,7 +1294,7 @@ sub get_svnprops { $path = $cmd_dir_prefix . $path; fatal("No such file or directory: $path") unless -e $path; my $is_dir = -d $path ? 1 : 0; - $path = $gs->{path} . '/' . $path; + $path = $gs->path . '/' . $path; # canonicalize the path (otherwise libsvn will abort or fail to # find the file) @@ -1396,7 +1396,7 @@ sub cmd_commit_diff { "the command-line\n", $usage); } $url = $gs->{url}; - $svn_path = $gs->{path}; + $svn_path = $gs->path; } unless (defined $_revision) { fatal("-r|--revision is a required argument\n", $usage); @@ -1670,7 +1670,7 @@ sub complete_url_ls_init { "wanted to set to: $gs->{url}\n"; } command_oneline('config', $k, $gs->{url}) unless $orig_url; - my $remote_path = "$gs->{path}/$repo_path"; + my $remote_path = $gs->path . "/$repo_path"; $remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg; $remote_path =~ s#/+#/#g; $remote_path =~ s#^/##g; diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index 268e0e84..02d5abc0 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -314,7 +314,7 @@ sub init_remote_config { print STDERR "Using higher level of URL: ", "$url => $min_url\n"; } - my $old_path = $self->{path}; + my $old_path = $self->path; $self->{path} = $url; $self->{path} =~ s!^\Q$min_url\E(/|$)!!; if (length $old_path) { @@ -347,7 +347,7 @@ sub init_remote_config { $self->{path} =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg; command_noisy('config', '--add', "svn-remote.$self->{repo_id}.fetch", - "$self->{path}:".$self->refname); + $self->path.":".$self->refname); } $self->{url} = $url; } @@ -435,7 +435,7 @@ sub new { } } my $self = _new($class, $repo_id, $ref_id, $path); - if (!defined $self->{path} || !length $self->{path}) { + if (!defined $self->path || !length $self->path) { my $fetch = command_oneline('config', '--get', "svn-remote.$repo_id.fetch", ":$ref_id\$") or @@ -567,7 +567,7 @@ sub _set_svm_vars { } my $r = $ra->get_latest_revnum; - my $path = $self->{path}; + my $path = $self->path; my %tried; while (length $path) { unless ($tried{"$self->{url}/$path"}) { @@ -728,7 +728,7 @@ sub prop_walk { $path =~ s#^/*#/#g; my $p = $path; # Strip the irrelevant part of the path. - $p =~ s#^/+\Q$self->{path}\E(/|$)#/#; + $p =~ s#^/+\Q@{[$self->path]}\E(/|$)#/#; # Ensure the path is terminated by a `/'. $p =~ s#/*$#/#; @@ -749,7 +749,7 @@ sub prop_walk { foreach (sort keys %$dirent) { next if $dirent->{$_}->{kind} != $SVN::Node::dir; - $self->prop_walk($self->{path} . $p . $_, $rev, $sub); + $self->prop_walk($self->path . $p . $_, $rev, $sub); } } @@ -920,19 +920,19 @@ sub rewrite_uuid { sub metadata_url { my ($self) = @_; ($self->rewrite_root || $self->{url}) . - (length $self->{path} ? '/' . $self->{path} : ''); + (length $self->path ? '/' . $self->path : ''); } sub full_url { my ($self) = @_; - $self->{url} . (length $self->{path} ? '/' . $self->{path} : ''); + $self->{url} . (length $self->path ? '/' . $self->path : ''); } sub full_pushurl { my ($self) = @_; if ($self->{pushurl}) { - return $self->{pushurl} . (length $self->{path} ? '/' . - $self->{path} : ''); + return $self->{pushurl} . (length $self->path ? '/' . + $self->path : ''); } else { return $self->full_url; } @@ -1048,20 +1048,20 @@ sub do_git_commit { sub match_paths { my ($self, $paths, $r) = @_; - return 1 if $self->{path} eq ''; - if (my $path = $paths->{"/$self->{path}"}) { + return 1 if $self->path eq ''; + if (my $path = $paths->{"/".$self->path}) { return ($path->{action} eq 'D') ? 0 : 1; } - $self->{path_regex} ||= qr/^\/\Q$self->{path}\E\//; + $self->{path_regex} ||= qr/^\/\Q@{[$self->path]}\E\//; if (grep /$self->{path_regex}/, keys %$paths) { return 1; } my $c = ''; - foreach (split m#/#, $self->{path}) { + foreach (split m#/#, $self->path) { $c .= "/$_"; next unless ($paths->{$c} && ($paths->{$c}->{action} =~ /^[AR]$/)); - if ($self->ra->check_path($self->{path}, $r) == + if ($self->ra->check_path($self->path, $r) == $SVN::Node::dir) { return 1; } @@ -1075,14 +1075,14 @@ sub find_parent_branch { unless (defined $paths) { my $err_handler = $SVN::Error::handler; $SVN::Error::handler = \&Git::SVN::Ra::skip_unknown_revs; - $self->ra->get_log([$self->{path}], $rev, $rev, 0, 1, 1, + $self->ra->get_log([$self->path], $rev, $rev, 0, 1, 1, sub { $paths = $_[0] }); $SVN::Error::handler = $err_handler; } return undef unless defined $paths; # look for a parent from another branch: - my @b_path_components = split m#/#, $self->{path}; + my @b_path_components = split m#/#, $self->path; my @a_path_components; my $i; while (@b_path_components) { @@ -1114,7 +1114,7 @@ sub find_parent_branch { ($base, $head) = parse_revision_argument(0, $r); } else { if ($r0 < $r) { - $gs->ra->get_log([$gs->{path}], $r0 + 1, $r, 1, + $gs->ra->get_log([$gs->path], $r0 + 1, $r, 1, 0, 1, sub { $base = $_[1] - 1 }); } } @@ -1136,7 +1136,7 @@ sub find_parent_branch { # at the moment), so we can't rely on it $self->{last_rev} = $r0; $self->{last_commit} = $parent; - $ed = Git::SVN::Fetcher->new($self, $gs->{path}); + $ed = Git::SVN::Fetcher->new($self, $gs->path); $gs->ra->gs_do_switch($r0, $rev, $gs, $self->full_url, $ed) or die "SVN connection failed somewhere...\n"; @@ -1235,7 +1235,7 @@ sub mkemptydirs { close $fh; } - my $strip = qr/\A\Q$self->{path}\E(?:\/|$)/; + my $strip = qr/\A\Q@{[$self->path]}\E(?:\/|$)/; foreach my $d (sort keys %empty_dirs) { $d = uri_decode($d); $d =~ s/$strip//; @@ -1858,7 +1858,7 @@ sub make_log_entry { $commit_email ||= "$author\@$uuid"; } elsif ($self->use_svnsync_props) { my $full_url = $self->svnsync->{url}; - $full_url .= "/$self->{path}" if length $self->{path}; + $full_url .= "/".$self->path if length $self->path; remove_username($full_url); my $uuid = $self->svnsync->{uuid}; $log_entry{metadata} = "$full_url\@$rev $uuid"; @@ -1905,7 +1905,7 @@ sub set_tree { tree_b => $tree, editor_cb => sub { $self->set_tree_cb($log_entry, $tree, @_) }, - svn_path => $self->{path} ); + svn_path => $self->path ); if (!Git::SVN::Editor->new(\%ed_opts)->apply_diff) { print "No changes\nr$self->{last_rev} = $tree\n"; } diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm index 76fae9bc..046a7a2f 100644 --- a/perl/Git/SVN/Fetcher.pm +++ b/perl/Git/SVN/Fetcher.pm @@ -83,7 +83,7 @@ sub _mark_empty_symlinks { 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}; + my $pfx = defined($switch_path) ? $switch_path : $git_svn->path; $pfx .= '/' if length($pfx); while (<$ls>) { chomp; diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm index 23ff43e8..64d00672 100644 --- a/perl/Git/SVN/Ra.pm +++ b/perl/Git/SVN/Ra.pm @@ -246,7 +246,7 @@ sub get_commit_editor { sub gs_do_update { my ($self, $rev_a, $rev_b, $gs, $editor) = @_; my $new = ($rev_a == $rev_b); - my $path = $gs->{path}; + my $path = $gs->path; if ($new && -e $gs->{index}) { unlink $gs->{index} or die @@ -282,7 +282,7 @@ sub gs_do_update { # svn_ra_reparent didn't work before 1.4) sub gs_do_switch { my ($self, $rev_a, $rev_b, $gs, $url_b, $editor) = @_; - my $path = $gs->{path}; + my $path = $gs->path; my $pool = SVN::Pool->new; my $full_url = $self->{url}; @@ -326,7 +326,7 @@ sub longest_common_path { my $common_max = scalar @$gsv; foreach my $gs (@$gsv) { - my @tmp = split m#/#, $gs->{path}; + my @tmp = split m#/#, $gs->path; my $p = ''; foreach (@tmp) { $p .= length($p) ? "/$_" : $_; @@ -407,7 +407,7 @@ sub gs_fetch_loop_common { } $SVN::Error::handler = $err_handler; - my %exists = map { $_->{path} => $_ } @$gsv; + my %exists = map { $_->path => $_ } @$gsv; foreach my $r (sort {$a <=> $b} keys %revs) { my ($paths, $logged) = @{$revs{$r}}; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [FYI/PATCH 3/5] Git::SVN: use accessor to write path 2012-09-17 9:04 ` Jonathan Nieder 2012-09-17 9:08 ` [FYI/PATCH 1/5] Git::SVN: introduce path accessor Jonathan Nieder 2012-09-17 9:09 ` [FYI/PATCH 2/5] Git::SVN: use accessor to read path Jonathan Nieder @ 2012-09-17 9:10 ` Jonathan Nieder 2012-09-17 9:12 ` [FYI/PATCH 4/5] Git::SVN::_new: use accessor to write path field Jonathan Nieder 2012-09-17 9:13 ` [PATCH/RFC 5/5] Git::SVN: rename private " Jonathan Nieder 4 siblings, 0 replies; 18+ messages in thread From: Jonathan Nieder @ 2012-09-17 9:10 UTC (permalink / raw) To: Eric Wong; +Cc: git, gitster, robbat2, bwalton, Michael G. Schwern From: Michael G. Schwern <schwern@pobox.com> Date: Fri, 27 Jul 2012 13:00:48 -0700 This patch only touches cases where the path field is written to using $gs->{path}. Cases where the path is set directly in a hash literal will be addressed separately. [jn: split from a larger patch] Signed-off-by: Eric Wong <normalperson@yhbt.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- perl/Git/SVN.pm | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index 02d5abc0..826a7fa6 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -315,11 +315,11 @@ sub init_remote_config { "$url => $min_url\n"; } my $old_path = $self->path; - $self->{path} = $url; - $self->{path} =~ s!^\Q$min_url\E(/|$)!!; + $url =~ s!^\Q$min_url\E(/|$)!!; if (length $old_path) { - $self->{path} .= "/$old_path"; + $url .= "/$old_path"; } + $self->path($url); $url = $min_url; } } @@ -343,8 +343,10 @@ sub init_remote_config { unless ($no_write) { command_noisy('config', "svn-remote.$self->{repo_id}.url", $url); - $self->{path} =~ s{^/}{}; - $self->{path} =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg; + my $path = $self->path; + $path =~ s{^/}{}; + $path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg; + $self->path($path); command_noisy('config', '--add', "svn-remote.$self->{repo_id}.fetch", $self->path.":".$self->refname); @@ -435,17 +437,19 @@ sub new { } } my $self = _new($class, $repo_id, $ref_id, $path); - if (!defined $self->path || !length $self->path) { + $path = $self->path; + if (!defined $path || !length $path) { my $fetch = command_oneline('config', '--get', "svn-remote.$repo_id.fetch", ":$ref_id\$") or die "Failed to read \"svn-remote.$repo_id.fetch\" ", "\":$ref_id\$\" in config\n"; - ($self->{path}, undef) = split(/\s*:\s*/, $fetch); + ($path, undef) = split(/\s*:\s*/, $fetch); } - $self->{path} =~ s{/+}{/}g; - $self->{path} =~ s{\A/}{}; - $self->{path} =~ s{/\z}{}; + $path =~ s{/+}{/}g; + $path =~ s{\A/}{}; + $path =~ s{/\z}{}; + $self->path($path); $self->{url} = command_oneline('config', '--get', "svn-remote.$repo_id.url") or die "Failed to read \"svn-remote.$repo_id.url\" in config\n"; -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [FYI/PATCH 4/5] Git::SVN::_new: use accessor to write path field 2012-09-17 9:04 ` Jonathan Nieder ` (2 preceding siblings ...) 2012-09-17 9:10 ` [FYI/PATCH 3/5] Git::SVN: use accessor to write path Jonathan Nieder @ 2012-09-17 9:12 ` Jonathan Nieder 2012-09-17 9:13 ` [PATCH/RFC 5/5] Git::SVN: rename private " Jonathan Nieder 4 siblings, 0 replies; 18+ messages in thread From: Jonathan Nieder @ 2012-09-17 9:12 UTC (permalink / raw) To: Eric Wong; +Cc: git, gitster, robbat2, bwalton, Michael G. Schwern From: Michael G. Schwern <schwern@pobox.com> Date: Fri, 27 Jul 2012 13:00:48 -0700 If some day the setter is taught to canonicalize paths, make sure the path gets canonicalized at construction time, too. [jn: split from a larger patch] Signed-off-by: Eric Wong <normalperson@yhbt.net> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- perl/Git/SVN.pm | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index 826a7fa6..3aa20109 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -2280,10 +2280,12 @@ sub _new { $_[3] = $path = '' unless (defined $path); mkpath([$dir]); - bless { + my $obj = bless { ref_id => $ref_id, dir => $dir, index => "$dir/index", - path => $path, config => "$ENV{GIT_DIR}/svn/config", + config => "$ENV{GIT_DIR}/svn/config", map_root => "$dir/.rev_map", repo_id => $repo_id }, $class; + $obj->path($path); + return $obj; } sub path { -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH/RFC 5/5] Git::SVN: rename private path field 2012-09-17 9:04 ` Jonathan Nieder ` (3 preceding siblings ...) 2012-09-17 9:12 ` [FYI/PATCH 4/5] Git::SVN::_new: use accessor to write path field Jonathan Nieder @ 2012-09-17 9:13 ` Jonathan Nieder 2012-09-18 0:00 ` Junio C Hamano 2012-09-18 0:07 ` Eric Wong 4 siblings, 2 replies; 18+ messages in thread From: Jonathan Nieder @ 2012-09-17 9:13 UTC (permalink / raw) To: Eric Wong; +Cc: git, gitster, robbat2, bwalton, Michael G. Schwern All users of $gs->{path} should have been converted to use the accessor by now. Check our work by renaming the underlying variable to break callers that try to use it directly. Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> --- perl/Git/SVN.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index 3aa20109..33f15682 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -2293,11 +2293,11 @@ sub path { if (@_) { my $path = shift; - $self->{path} = $path; + $self->{_path} = $path; return; } - return $self->{path}; + return $self->{_path}; } # for read-only access of old .rev_db formats -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC 5/5] Git::SVN: rename private path field 2012-09-17 9:13 ` [PATCH/RFC 5/5] Git::SVN: rename private " Jonathan Nieder @ 2012-09-18 0:00 ` Junio C Hamano 2012-09-18 0:07 ` Eric Wong 1 sibling, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2012-09-18 0:00 UTC (permalink / raw) To: Jonathan Nieder; +Cc: Eric Wong, git, robbat2, bwalton, Michael G. Schwern Jonathan Nieder <jrnieder@gmail.com> writes: > All users of $gs->{path} should have been converted to use the > accessor by now. Check our work by renaming the underlying variable > to break callers that try to use it directly. > > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> > --- I like this ;-) If we know we have good coverage, this would be a sensible way to catch remaining code that hasn't been converted. > perl/Git/SVN.pm | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm > index 3aa20109..33f15682 100644 > --- a/perl/Git/SVN.pm > +++ b/perl/Git/SVN.pm > @@ -2293,11 +2293,11 @@ sub path { > > if (@_) { > my $path = shift; > - $self->{path} = $path; > + $self->{_path} = $path; > return; > } > > - return $self->{path}; > + return $self->{_path}; > } > > # for read-only access of old .rev_db formats ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH/RFC 5/5] Git::SVN: rename private path field 2012-09-17 9:13 ` [PATCH/RFC 5/5] Git::SVN: rename private " Jonathan Nieder 2012-09-18 0:00 ` Junio C Hamano @ 2012-09-18 0:07 ` Eric Wong 1 sibling, 0 replies; 18+ messages in thread From: Eric Wong @ 2012-09-18 0:07 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git, gitster, robbat2, bwalton, Michael G. Schwern Jonathan Nieder <jrnieder@gmail.com> wrote: > All users of $gs->{path} should have been converted to use the > accessor by now. Check our work by renaming the underlying variable > to break callers that try to use it directly. > > Signed-off-by: Jonathan Nieder <jrnieder@gmail.com> I think this is a good patch for master, too. Thanks. Signed-off-by: Eric Wong <normalperson@yhbt.net> > --- > perl/Git/SVN.pm | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > -- I'll apply the following (on top of a patch which fixes some {path} usages): diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index 88b9164..59215fa 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -2331,11 +2331,11 @@ sub path { if (@_) { my $path = shift; - $self->{path} = canonicalize_path($path); + $self->{_path} = canonicalize_path($path); return; } - return $self->{path}; + return $self->{_path}; } sub url { ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/5] Make Git::SVN use an accessor for URLs internally. 2012-07-27 20:00 Make git-svn Use accessors for paths and urls Michael G. Schwern 2012-07-27 20:00 ` [PATCH 1/5] Make Git::SVN use accessors internally for path Michael G. Schwern @ 2012-07-27 20:00 ` Michael G. Schwern 2012-07-27 20:00 ` [PATCH 3/5] Make Git::SVN::Ra use an accessor for URLs Michael G. Schwern ` (3 subsequent siblings) 5 siblings, 0 replies; 18+ messages in thread From: Michael G. Schwern @ 2012-07-27 20:00 UTC (permalink / raw) To: git, gitster; +Cc: robbat2, bwalton, normalperson, jrnieder, schwern From: "Michael G. Schwern" <schwern@pobox.com> So later it can do automatic canonicalization. A later patch will make other things use the accessor. No functional change here. --- perl/Git/SVN.pm | 44 ++++++++++++++++++++++++++++++-------------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index 0aff9d0..59bca51 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -351,7 +351,7 @@ sub init_remote_config { "svn-remote.$self->{repo_id}.fetch", $self->path.":".$self->refname); } - $self->{url} = $url; + $self->url($url); } sub find_by_url { # repos_root and, path are optional @@ -453,9 +453,10 @@ sub new { $path =~ s{/\z}{}; $self->path($path); } - $self->{url} = command_oneline('config', '--get', - "svn-remote.$repo_id.url") or + my $url = command_oneline('config', '--get', + "svn-remote.$repo_id.url") or die "Failed to read \"svn-remote.$repo_id.url\" in config\n"; + $self->url($url); $self->{pushurl} = eval { command_oneline('config', '--get', "svn-remote.$repo_id.pushurl") }; $self->rebuild; @@ -577,17 +578,18 @@ sub _set_svm_vars { my $path = $self->path; my %tried; while (length $path) { - unless ($tried{"$self->{url}/$path"}) { + my $try = $self->url . "/$path"; + unless ($tried{$try}) { return $ra if $self->read_svm_props($ra, $path, $r); - $tried{"$self->{url}/$path"} = 1; + $tried{$try} = 1; } $path =~ s#/?[^/]+$##; } die "Path: '$path' should be ''\n" if $path ne ''; return $ra if $self->read_svm_props($ra, $path, $r); - $tried{"$self->{url}/$path"} = 1; + $tried{$self->url."/$path"} = 1; - if ($ra->{repos_root} eq $self->{url}) { + if ($ra->{repos_root} eq $self->url) { die @err, (map { " $_\n" } keys %tried), "\n"; } @@ -610,7 +612,7 @@ sub _set_svm_vars { if (!$ok) { die @err, (map { " $_\n" } keys %tried), "\n"; } - Git::SVN::Ra->new($self->{url}); + Git::SVN::Ra->new($self->url); } sub svnsync { @@ -677,7 +679,7 @@ sub ra_uuid { if (!$@ && $uuid && $uuid =~ /^([a-f\d\-]{30,})$/i) { $self->{ra_uuid} = $uuid; } else { - die "ra_uuid called without URL\n" unless $self->{url}; + die "ra_uuid called without URL\n" unless $self->url; $self->{ra_uuid} = $self->ra->get_uuid; tmp_config('--add', $key, $self->{ra_uuid}); } @@ -701,7 +703,7 @@ sub repos_root { sub ra { my ($self) = shift; - my $ra = Git::SVN::Ra->new($self->{url}); + my $ra = Git::SVN::Ra->new($self->url); $self->_set_repos_root($ra->{repos_root}); if ($self->use_svm_props && !$self->{svm}) { if ($self->no_metadata) { @@ -926,13 +928,13 @@ sub rewrite_uuid { sub metadata_url { my ($self) = @_; - ($self->rewrite_root || $self->{url}) . + ($self->rewrite_root || $self->url) . (length $self->path ? '/' . $self->path : ''); } sub full_url { my ($self) = @_; - $self->{url} . (length $self->path ? '/' . $self->path : ''); + $self->url . (length $self->path ? '/' . $self->path : ''); } sub full_pushurl { @@ -1436,7 +1438,7 @@ sub find_extra_svk_parents { for my $ticket ( @tickets ) { my ($uuid, $path, $rev) = split /:/, $ticket; if ( $uuid eq $self->ra_uuid ) { - my $url = $self->{url}; + my $url = $self->url; my $repos_root = $url; my $branch_from = $path; $branch_from =~ s{^/}{}; @@ -1682,7 +1684,7 @@ sub find_extra_svn_parents { # are now marked as merge, we can add the tip as a parent. my @merges = split "\n", $mergeinfo; my @merge_tips; - my $url = $self->{url}; + my $url = $self->url; my $uuid = $self->ra_uuid; my %ranges; for my $merge ( @merges ) { @@ -2307,6 +2309,20 @@ sub path { return $self->{path}; } + +sub url { + my $self = shift; + + if( @_ ) { + my $url = shift; + $self->{url} = $url; + return; + } + + return $self->{url}; +} + + # for read-only access of old .rev_db formats sub unlink_rev_db_symlink { my ($self) = @_; -- 1.7.11.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/5] Make Git::SVN::Ra use an accessor for URLs 2012-07-27 20:00 Make git-svn Use accessors for paths and urls Michael G. Schwern 2012-07-27 20:00 ` [PATCH 1/5] Make Git::SVN use accessors internally for path Michael G. Schwern 2012-07-27 20:00 ` [PATCH 2/5] Make Git::SVN use an accessor for URLs internally Michael G. Schwern @ 2012-07-27 20:00 ` Michael G. Schwern 2012-07-27 20:00 ` [PATCH 4/5] Change the rest of the code to use Git::SVN->path instead of the hash directly Michael G. Schwern ` (2 subsequent siblings) 5 siblings, 0 replies; 18+ messages in thread From: Michael G. Schwern @ 2012-07-27 20:00 UTC (permalink / raw) To: git, gitster; +Cc: robbat2, bwalton, normalperson, jrnieder, schwern From: "Michael G. Schwern" <schwern@pobox.com> Later it can canonicalize automatically. A later change will make other things use the accessor. No functional change. --- perl/Git/SVN/Ra.pm | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm index 23ff43e..329f855 100644 --- a/perl/Git/SVN/Ra.pm +++ b/perl/Git/SVN/Ra.pm @@ -84,7 +84,7 @@ sub escape_url { sub new { my ($class, $url) = @_; $url =~ s!/+$!!; - return $RA if ($RA && $RA->{url} eq $url); + return $RA if ($RA && $RA->url eq $url); ::_req_svn(); @@ -119,15 +119,33 @@ sub new { config => $config, pool => SVN::Pool->new, auth_provider_callbacks => $callbacks); - $self->{url} = $url; + $RA = bless $self, $class; + + # Make sure its canonicalized + $self->url($url); $self->{svn_path} = $url; $self->{repos_root} = $self->get_repos_root; $self->{svn_path} =~ s#^\Q$self->{repos_root}\E(/|$)##; $self->{cache} = { check_path => { r => 0, data => {} }, get_dir => { r => 0, data => {} } }; - $RA = bless $self, $class; + + return $RA; +} + + +sub url { + my $self = shift; + + if( @_ ) { + my $url = shift; + $self->{url} = $url; + return; + } + + return $self->{url}; } + sub check_path { my ($self, $path, $r) = @_; my $cache = $self->{cache}->{check_path}; @@ -285,7 +303,7 @@ sub gs_do_switch { my $path = $gs->{path}; my $pool = SVN::Pool->new; - my $full_url = $self->{url}; + my $full_url = $self->url; my $old_url = $full_url; $full_url .= '/' . $path if length $path; my ($ra, $reparented); @@ -300,7 +318,7 @@ sub gs_do_switch { $ra_invalid = 1; } elsif ($old_url ne $full_url) { SVN::_Ra::svn_ra_reparent($self->{session}, $full_url, $pool); - $self->{url} = $full_url; + $self->url($full_url); $reparented = 1; } @@ -313,7 +331,7 @@ sub gs_do_switch { if ($reparented) { SVN::_Ra::svn_ra_reparent($self->{session}, $old_url, $pool); - $self->{url} = $old_url; + $self->url($old_url); } $pool->clear; @@ -362,7 +380,7 @@ sub gs_fetch_loop_common { my $inc = $_log_window_size; my ($min, $max) = ($base, $head < $base + $inc ? $head : $base + $inc); my $longest_path = longest_common_path($gsv, $globs); - my $ra_url = $self->{url}; + my $ra_url = $self->url; my $find_trailing_edge; while (1) { my %revs; @@ -508,7 +526,7 @@ sub match_globs { ($self->check_path($p, $r) != $SVN::Node::dir)); next unless $p =~ /$g->{path}->{regex}/; - $exists->{$p} = Git::SVN->init($self->{url}, $p, undef, + $exists->{$p} = Git::SVN->init($self->url, $p, undef, $g->{ref}->full_path($de), 1); } } @@ -532,7 +550,7 @@ sub match_globs { next if ($self->check_path($pathname, $r) != $SVN::Node::dir); $exists->{$pathname} = Git::SVN->init( - $self->{url}, $pathname, undef, + $self->url, $pathname, undef, $g->{ref}->full_path($p), 1); } my $c = ''; @@ -548,7 +566,7 @@ sub match_globs { sub minimize_url { my ($self) = @_; - return $self->{url} if ($self->{url} eq $self->{repos_root}); + return $self->url if ($self->url eq $self->{repos_root}); my $url = $self->{repos_root}; my @components = split(m!/!, $self->{svn_path}); my $c = ''; @@ -568,7 +586,7 @@ sub can_do_switch { unless (defined $can_do_switch) { my $pool = SVN::Pool->new; my $rep = eval { - $self->do_switch(1, '', 0, $self->{url}, + $self->do_switch(1, '', 0, $self->url, SVN::Delta::Editor->new, $pool); }; if ($@) { -- 1.7.11.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/5] Change the rest of the code to use Git::SVN->path instead of the hash directly. 2012-07-27 20:00 Make git-svn Use accessors for paths and urls Michael G. Schwern ` (2 preceding siblings ...) 2012-07-27 20:00 ` [PATCH 3/5] Make Git::SVN::Ra use an accessor for URLs Michael G. Schwern @ 2012-07-27 20:00 ` Michael G. Schwern 2012-07-27 20:00 ` [PATCH 5/5] Change the rest of the code to use the Git::SVN and Git::SVN::Ra url accessors Michael G. Schwern 2012-07-28 3:10 ` Make git-svn Use accessors for paths and urls Jonathan Nieder 5 siblings, 0 replies; 18+ messages in thread From: Michael G. Schwern @ 2012-07-27 20:00 UTC (permalink / raw) To: git, gitster; +Cc: robbat2, bwalton, normalperson, jrnieder, schwern From: "Michael G. Schwern" <schwern@pobox.com> No functional change. --- git-svn.perl | 12 +++++++----- perl/Git/SVN.pm | 4 ++-- perl/Git/SVN/Fetcher.pm | 2 +- perl/Git/SVN/Ra.pm | 6 +++--- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/git-svn.perl b/git-svn.perl index 5711c57..039623e 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -1195,7 +1195,7 @@ sub cmd_show_ignore { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; print STDOUT "\n# $path\n"; my $s = $props->{'svn:ignore'} or return; @@ -1211,7 +1211,7 @@ sub cmd_show_externals { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; print STDOUT "\n# $path\n"; my $s = $props->{'svn:externals'} or return; @@ -1226,7 +1226,7 @@ sub cmd_create_ignore { my ($url, $rev, $uuid, $gs) = working_head_info('HEAD'); $gs ||= Git::SVN->new; my $r = (defined $_revision ? $_revision : $gs->ra->get_latest_revnum); - $gs->prop_walk($gs->{path}, $r, sub { + $gs->prop_walk($gs->path, $r, sub { my ($gs, $path, $props) = @_; # $path is of the form /path/to/dir/ $path = '.' . $path; @@ -1396,7 +1396,7 @@ sub cmd_commit_diff { "the command-line\n", $usage); } $url = $gs->{url}; - $svn_path = $gs->{path}; + $svn_path = $gs->path; } unless (defined $_revision) { fatal("-r|--revision is a required argument\n", $usage); @@ -1634,6 +1634,8 @@ sub post_fetch_checkout { sub complete_svn_url { my ($url, $path) = @_; $path =~ s#/+$##; + + # If the path is not a URL... if ($path !~ m#^[a-z\+]+://#) { if (!defined $url || $url !~ m#^[a-z\+]+://#) { fatal("E: '$path' is not a complete URL ", @@ -1670,7 +1672,7 @@ sub complete_url_ls_init { "wanted to set to: $gs->{url}\n"; } command_oneline('config', $k, $gs->{url}) unless $orig_url; - my $remote_path = "$gs->{path}/$repo_path"; + my $remote_path = $gs->path . "/$repo_path"; $remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg; $remote_path =~ s#/+#/#g; $remote_path =~ s#^/##g; diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index 59bca51..fc907a0 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -1123,7 +1123,7 @@ sub find_parent_branch { ($base, $head) = parse_revision_argument(0, $r); } else { if ($r0 < $r) { - $gs->ra->get_log([$gs->{path}], $r0 + 1, $r, 1, + $gs->ra->get_log([$gs->path], $r0 + 1, $r, 1, 0, 1, sub { $base = $_[1] - 1 }); } } @@ -1145,7 +1145,7 @@ sub find_parent_branch { # at the moment), so we can't rely on it $self->{last_rev} = $r0; $self->{last_commit} = $parent; - $ed = Git::SVN::Fetcher->new($self, $gs->{path}); + $ed = Git::SVN::Fetcher->new($self, $gs->path); $gs->ra->gs_do_switch($r0, $rev, $gs, $self->full_url, $ed) or die "SVN connection failed somewhere...\n"; diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm index 76fae9b..046a7a2 100644 --- a/perl/Git/SVN/Fetcher.pm +++ b/perl/Git/SVN/Fetcher.pm @@ -83,7 +83,7 @@ sub _mark_empty_symlinks { 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}; + my $pfx = defined($switch_path) ? $switch_path : $git_svn->path; $pfx .= '/' if length($pfx); while (<$ls>) { chomp; diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm index 329f855..27dcdd5 100644 --- a/perl/Git/SVN/Ra.pm +++ b/perl/Git/SVN/Ra.pm @@ -264,7 +264,7 @@ sub get_commit_editor { sub gs_do_update { my ($self, $rev_a, $rev_b, $gs, $editor) = @_; my $new = ($rev_a == $rev_b); - my $path = $gs->{path}; + my $path = $gs->path; if ($new && -e $gs->{index}) { unlink $gs->{index} or die @@ -300,7 +300,7 @@ sub gs_do_update { # svn_ra_reparent didn't work before 1.4) sub gs_do_switch { my ($self, $rev_a, $rev_b, $gs, $url_b, $editor) = @_; - my $path = $gs->{path}; + my $path = $gs->path; my $pool = SVN::Pool->new; my $full_url = $self->url; @@ -344,7 +344,7 @@ sub longest_common_path { my $common_max = scalar @$gsv; foreach my $gs (@$gsv) { - my @tmp = split m#/#, $gs->{path}; + my @tmp = split m#/#, $gs->path; my $p = ''; foreach (@tmp) { $p .= length($p) ? "/$_" : $_; -- 1.7.11.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/5] Change the rest of the code to use the Git::SVN and Git::SVN::Ra url accessors. 2012-07-27 20:00 Make git-svn Use accessors for paths and urls Michael G. Schwern ` (3 preceding siblings ...) 2012-07-27 20:00 ` [PATCH 4/5] Change the rest of the code to use Git::SVN->path instead of the hash directly Michael G. Schwern @ 2012-07-27 20:00 ` Michael G. Schwern 2012-07-28 2:59 ` Eric Wong 2012-07-28 3:10 ` Make git-svn Use accessors for paths and urls Jonathan Nieder 5 siblings, 1 reply; 18+ messages in thread From: Michael G. Schwern @ 2012-07-27 20:00 UTC (permalink / raw) To: git, gitster; +Cc: robbat2, bwalton, normalperson, jrnieder, schwern From: "Michael G. Schwern" <schwern@pobox.com> Note: The structure returned from Git::SVN->read_all_remotes() does not appear to contain objects, so I'm leaving them alone. That's everything converted over to the url and path accessors. No functional change. --- git-svn.perl | 11 ++++++----- perl/Git/SVN.pm | 11 ++++++----- perl/Git/SVN/Migration.pm | 6 +++--- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/git-svn.perl b/git-svn.perl index 039623e..de1ddd1 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -1395,7 +1395,7 @@ sub cmd_commit_diff { fatal("Needed URL or usable git-svn --id in ", "the command-line\n", $usage); } - $url = $gs->{url}; + $url = $gs->url; $svn_path = $gs->path; } unless (defined $_revision) { @@ -1663,15 +1663,16 @@ sub complete_url_ls_init { "and a separate URL is not specified"); } } - my $url = $ra->{url}; + my $url = $ra->url; my $gs = Git::SVN->init($url, undef, undef, undef, 1); my $k = "svn-remote.$gs->{repo_id}.url"; my $orig_url = eval { command_oneline(qw/config --get/, $k) }; - if ($orig_url && ($orig_url ne $gs->{url})) { + if ($orig_url && ($orig_url ne $gs->url)) { die "$k already set: $orig_url\n", - "wanted to set to: $gs->{url}\n"; + "wanted to set to: $gs->url\n"; } - command_oneline('config', $k, $gs->{url}) unless $orig_url; + command_oneline('config', $k, $gs->url) unless $orig_url; + my $remote_path = $gs->path . "/$repo_path"; $remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg; $remote_path =~ s#/+#/#g; diff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm index fc907a0..7913d8f 100644 --- a/perl/Git/SVN.pm +++ b/perl/Git/SVN.pm @@ -560,7 +560,7 @@ sub _set_svm_vars { # username is of no interest $src =~ s{(^[a-z\+]*://)[^/@]*@}{$1}; - my $replace = $ra->{url}; + my $replace = $ra->url; $replace .= "/$path" if length $path; my $section = "svn-remote.$self->{repo_id}"; @@ -599,16 +599,17 @@ sub _set_svm_vars { $path = $ra->{svn_path}; $ra = Git::SVN::Ra->new($ra->{repos_root}); while (length $path) { - unless ($tried{"$ra->{url}/$path"}) { + my $try = $ra->url ."/$path"; + unless ($tried{$try}) { $ok = $self->read_svm_props($ra, $path, $r); last if $ok; - $tried{"$ra->{url}/$path"} = 1; + $tried{$try} = 1; } $path =~ s#/?[^/]+$##; } die "Path: '$path' should be ''\n" if $path ne ''; $ok ||= $self->read_svm_props($ra, $path, $r); - $tried{"$ra->{url}/$path"} = 1; + $tried{$ra->url ."/$path"} = 1; if (!$ok) { die @err, (map { " $_\n" } keys %tried), "\n"; } @@ -1108,7 +1109,7 @@ sub find_parent_branch { } my $r = $i->{copyfrom_rev}; my $repos_root = $self->ra->{repos_root}; - my $url = $self->ra->{url}; + my $url = $self->ra->url; my $new_url = $url . $branch_from; print STDERR "Found possible branch point: ", "$new_url => ", $self->full_url, ", $r\n" diff --git a/perl/Git/SVN/Migration.pm b/perl/Git/SVN/Migration.pm index 75d7429..30daf35 100644 --- a/perl/Git/SVN/Migration.pm +++ b/perl/Git/SVN/Migration.pm @@ -177,14 +177,14 @@ sub minimize_connections { my $ra = Git::SVN::Ra->new($url); # skip existing cases where we already connect to the root - if (($ra->{url} eq $ra->{repos_root}) || + if (($ra->url eq $ra->{repos_root}) || ($ra->{repos_root} eq $repo_id)) { - $root_repos->{$ra->{url}} = $repo_id; + $root_repos->{$ra->url} = $repo_id; next; } my $root_ra = Git::SVN::Ra->new($ra->{repos_root}); - my $root_path = $ra->{url}; + my $root_path = $ra->url; $root_path =~ s#^\Q$ra->{repos_root}\E(/|$)##; foreach my $path (keys %$fetch) { my $ref_id = $fetch->{$path}; -- 1.7.11.3 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] Change the rest of the code to use the Git::SVN and Git::SVN::Ra url accessors. 2012-07-27 20:00 ` [PATCH 5/5] Change the rest of the code to use the Git::SVN and Git::SVN::Ra url accessors Michael G. Schwern @ 2012-07-28 2:59 ` Eric Wong 0 siblings, 0 replies; 18+ messages in thread From: Eric Wong @ 2012-07-28 2:59 UTC (permalink / raw) To: Michael G. Schwern; +Cc: git, gitster, robbat2, bwalton, jrnieder "Michael G. Schwern" <schwern@pobox.com> wrote: We try to keep Subject: lines short (~50 char soft limit) as documented in SubmittingPatches. How about: Subject: [PATCH 4/5] use Git::SVN->path accessor globally Subject: [PATCH 5/5] use Git::SVN{,::RA}->url accessor globally ? I can make the changes to the commit message on my side before I push for Junio. > Note: The structure returned from Git::SVN->read_all_remotes() does > not appear to contain objects, so I'm leaving them alone. Right, just hashrefs and strings there. > No functional change. This series looks good with minor edits. I made minor edits to reformat the new accessor subs (please do so on your end before sending in the future). ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Make git-svn Use accessors for paths and urls 2012-07-27 20:00 Make git-svn Use accessors for paths and urls Michael G. Schwern ` (4 preceding siblings ...) 2012-07-27 20:00 ` [PATCH 5/5] Change the rest of the code to use the Git::SVN and Git::SVN::Ra url accessors Michael G. Schwern @ 2012-07-28 3:10 ` Jonathan Nieder 2012-07-28 7:40 ` Michael G Schwern 5 siblings, 1 reply; 18+ messages in thread From: Jonathan Nieder @ 2012-07-28 3:10 UTC (permalink / raw) To: Michael G. Schwern; +Cc: git, gitster, robbat2, bwalton, normalperson Michael G. Schwern wrote: > This is the last refactoring patch series. After this bugs, start > getting fixed. I just wanted to say thanks for your thoughtful presentation of this code. I was worried before, but these have been pleasantly submitted. If you have a chance at some point to offer advice, I'd love to add the information to Documentation/SubmittingPatches that was missing. Proposed text is ideal, but outline form or a list of missing aspects and confusing existing coverage would be fine, too. Jonathan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Make git-svn Use accessors for paths and urls 2012-07-28 3:10 ` Make git-svn Use accessors for paths and urls Jonathan Nieder @ 2012-07-28 7:40 ` Michael G Schwern 2012-07-28 7:54 ` Jonathan Nieder 0 siblings, 1 reply; 18+ messages in thread From: Michael G Schwern @ 2012-07-28 7:40 UTC (permalink / raw) To: Jonathan Nieder; +Cc: git, gitster, robbat2, bwalton, normalperson On 2012.7.27 8:10 PM, Jonathan Nieder wrote: >> This is the last refactoring patch series. After this bugs, start >> getting fixed. > > I just wanted to say thanks for your thoughtful presentation of this > code. I was worried before, but these have been pleasantly submitted. You're welcome. I've gained at least three levels in rebasing in the process. > If you have a chance at some point to offer advice, I'd love to add > the information to Documentation/SubmittingPatches that was missing. > Proposed text is ideal, but outline form or a list of missing aspects > and confusing existing coverage would be fine, too. Remind me when I'm done with the 1.7 fix please? -- You are wicked and wrong to have broken inside and peeked at the implementation and then relied upon it. -- tchrist in <31832.969261130@chthon> ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: Make git-svn Use accessors for paths and urls 2012-07-28 7:40 ` Michael G Schwern @ 2012-07-28 7:54 ` Jonathan Nieder 0 siblings, 0 replies; 18+ messages in thread From: Jonathan Nieder @ 2012-07-28 7:54 UTC (permalink / raw) To: Michael G Schwern; +Cc: git, gitster, robbat2, bwalton, normalperson Michael G Schwern wrote: > On 2012.7.27 8:10 PM, Jonathan Nieder wrote: >> If you have a chance at some point to offer advice, I'd love to add >> the information to Documentation/SubmittingPatches that was missing. [...] > Remind me when I'm done with the 1.7 fix please? Sure, if I remember to. :) Thanks, Jonathan ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-09-18 0:07 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-07-27 20:00 Make git-svn Use accessors for paths and urls Michael G. Schwern 2012-07-27 20:00 ` [PATCH 1/5] Make Git::SVN use accessors internally for path Michael G. Schwern 2012-09-17 9:04 ` Jonathan Nieder 2012-09-17 9:08 ` [FYI/PATCH 1/5] Git::SVN: introduce path accessor Jonathan Nieder 2012-09-17 9:09 ` [FYI/PATCH 2/5] Git::SVN: use accessor to read path Jonathan Nieder 2012-09-17 9:10 ` [FYI/PATCH 3/5] Git::SVN: use accessor to write path Jonathan Nieder 2012-09-17 9:12 ` [FYI/PATCH 4/5] Git::SVN::_new: use accessor to write path field Jonathan Nieder 2012-09-17 9:13 ` [PATCH/RFC 5/5] Git::SVN: rename private " Jonathan Nieder 2012-09-18 0:00 ` Junio C Hamano 2012-09-18 0:07 ` Eric Wong 2012-07-27 20:00 ` [PATCH 2/5] Make Git::SVN use an accessor for URLs internally Michael G. Schwern 2012-07-27 20:00 ` [PATCH 3/5] Make Git::SVN::Ra use an accessor for URLs Michael G. Schwern 2012-07-27 20:00 ` [PATCH 4/5] Change the rest of the code to use Git::SVN->path instead of the hash directly Michael G. Schwern 2012-07-27 20:00 ` [PATCH 5/5] Change the rest of the code to use the Git::SVN and Git::SVN::Ra url accessors Michael G. Schwern 2012-07-28 2:59 ` Eric Wong 2012-07-28 3:10 ` Make git-svn Use accessors for paths and urls Jonathan Nieder 2012-07-28 7:40 ` Michael G Schwern 2012-07-28 7:54 ` Jonathan Nieder
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).