{"thread":{"id":"19324","subject":"[PATCH] git-svn: Handle multi-branch setups even if repo root cannot be accessed","startedAt":"2009-05-12T22:41:15Z","lastAt":"2009-05-12T22:41:15Z","messageCount":1,"participants":["Mattias Nissler"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"113698","messageId":"1242168075.4802.13.camel@kea","threadId":"19324","inReplyTo":null,"subject":"[PATCH] git-svn: Handle multi-branch setups even if repo root cannot be accessed","fromName":"Mattias Nissler","fromEmail":"mattias.nissler@gmx.de","sentAt":"2009-05-12T22:41:15Z","receivedAt":"2009-05-12T22:41:15Z","isPatch":true,"sender":{"key":"mattias.nissler@gmx.de","avatar":null},"body":"For the case of multiple projects sharing a single SVN repository, it is\ncommon practice to create the standard SVN directory layout within a\nsubdirectory for each project. In such setups, access control is often\nused to limit what projects a given user may access. This results in\ngit-svn not being able to minimize the URL of the repository to the\nrepository root URL, which triggered problems since path handling for\nmulti-branch setups was not aware of the path prefix for the project\ndirectory relative to the repository root. Fix this deficiency by\nconverting all paths received from get_log to a form relative to the URL\nthat is configured for the SVN remote. Also drop the hardcoded URL\nminimization flag for the multi-branch case since it is not required\nanymore and can be requested via a command line parameter if desired.\n\nSigned-off-by: Mattias Nissler <mattias.nissler@gmx.de>\n---\n\nHi,\n\nI've successfully tested this patch against our SVN repo at work, which\nis quite large in terms of number of files and disk usage. I hope I\ndidn't break anything, I tried to minimize the chance of doing so by\nisolating the important changes to the handling of the results of the\nget_log call. Still, I'd appreciate if someone could review the patch to\nsee whether I might have broken something I didn't think of.\n\nPlease CC me on replies, I'm not on the list.\n\nEnjoy!\n\nMattias\n\n\n git-svn.perl |   83 +++++++++++++++++++++++++--------------------------------\n 1 files changed, 36 insertions(+), 47 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex ef1d30d..ed95f05 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -798,10 +798,6 @@ sub cmd_multi_init {\n \t\tusage(1);\n \t}\n \n-\t# there are currently some bugs that prevent multi-init/multi-fetch\n-\t# setups from working well without this.\n-\t$Git::SVN::_minimize_url = 1;\n-\n \t$_prefix = '' unless defined $_prefix;\n \tif (defined $url) {\n \t\t$url =~ s#/+$##;\n@@ -1084,7 +1080,7 @@ sub complete_url_ls_init {\n \t\t    \"wanted to set to: $gs->{url}\\n\";\n \t}\n \tcommand_oneline('config', $k, $gs->{url}) unless $orig_url;\n-\tmy $remote_path = \"$ra->{svn_path}/$repo_path\";\n+\tmy $remote_path = \"$gs->{path}/$repo_path\";\n \t$remote_path =~ s#/+#/#g;\n \t$remote_path =~ s#^/##g;\n \t$remote_path .= \"/*\" if $remote_path !~ /\\*/;\n@@ -2066,16 +2062,6 @@ sub ra {\n \t$ra;\n }\n \n-sub rel_path {\n-\tmy ($self) = @_;\n-\tmy $repos_root = $self->ra->{repos_root};\n-\treturn $self->{path} if ($self->{url} eq $repos_root);\n-\tmy $url = $self->{url} .\n-\t          (length $self->{path} ? \"/$self->{path}\" : $self->{path});\n-\t$url =~ s!^\\Q$repos_root\\E(?:/+|$)!!g;\n-\t$url;\n-}\n-\n # prop_walk(PATH, REV, SUB)\n # -------------------------\n # Recursively traverse PATH at revision REV and invoke SUB for each\n@@ -2401,10 +2387,7 @@ sub match_paths {\n \tif (my $path = $paths->{\"/$self->{path}\"}) {\n \t\treturn ($path->{action} eq 'D') ? 0 : 1;\n \t}\n-\tmy $repos_root = $self->ra->{repos_root};\n-\tmy $extended_path = $self->{url} . '/' . $self->{path};\n-\t$extended_path =~ s#^\\Q$repos_root\\E(/|$)##;\n-\t$self->{path_regex} ||= qr/^\\/\\Q$extended_path\\E\\//;\n+\t$self->{path_regex} ||= qr/^\\/\\Q$self->{path}\\E\\//;\n \tif (grep /$self->{path_regex}/, keys %$paths) {\n \t\treturn 1;\n \t}\n@@ -2427,15 +2410,14 @@ sub find_parent_branch {\n \tunless (defined $paths) {\n \t\tmy $err_handler = $SVN::Error::handler;\n \t\t$SVN::Error::handler = \\&Git::SVN::Ra::skip_unknown_revs;\n-\t\t$self->ra->get_log([$self->{path}], $rev, $rev, 0, 1, 1, sub {\n-\t\t                   $paths =\n-\t\t\t\t      Git::SVN::Ra::dup_changed_paths($_[0]) });\n+\t\t$self->ra->get_log([$self->{path}], $rev, $rev, 0, 1, 1,\n+\t\t\tsub { $paths = $_[0] });\n \t\t$SVN::Error::handler = $err_handler;\n \t}\n \treturn undef unless defined $paths;\n \n \t# look for a parent from another branch:\n-\tmy @b_path_components = split m#/#, $self->rel_path;\n+\tmy @b_path_components = split m#/#, $self->{path};\n \tmy @a_path_components;\n \tmy $i;\n \twhile (@b_path_components) {\n@@ -2453,11 +2435,11 @@ sub find_parent_branch {\n \tmy $r = $i->{copyfrom_rev};\n \tmy $repos_root = $self->ra->{repos_root};\n \tmy $url = $self->ra->{url};\n-\tmy $new_url = $repos_root . $branch_from;\n+\tmy $new_url = $url . $branch_from;\n \tprint STDERR  \"Found possible branch point: \",\n \t              \"$new_url => \", $self->full_url, \", $r\\n\";\n \t$branch_from =~ s#^/##;\n-\tmy $gs = $self->other_gs($new_url, $url, $repos_root,\n+\tmy $gs = $self->other_gs($new_url, $url,\n \t\t                 $branch_from, $r, $self->{ref_id});\n \tmy ($r0, $parent) = $gs->find_rev_before($r, 1);\n \t{\n@@ -2642,9 +2624,9 @@ sub parse_svn_date {\n }\n \n sub other_gs {\n-\tmy ($self, $new_url, $url, $repos_root,\n+\tmy ($self, $new_url, $url,\n \t    $branch_from, $r, $old_ref_id) = @_;\n-\tmy $gs = Git::SVN->find_by_url($new_url, $repos_root, $branch_from);\n+\tmy $gs = Git::SVN->find_by_url($new_url, $url, $branch_from);\n \tunless ($gs) {\n \t\tmy $ref_id = $old_ref_id;\n \t\t$ref_id =~ s/\\@\\d+$//;\n@@ -4280,6 +4262,31 @@ sub get_log {\n \tmy ($self, @args) = @_;\n \tmy $pool = SVN::Pool->new;\n \n+\t# svn_log_changed_path_t objects passed to get_log are likely to be\n+\t# overwritten even if only the refs are copied to an external variable,\n+\t# so we should dup the structures in their entirety.  Using an\n+\t# externally passed pool (instead of our temporary and quickly cleared\n+\t# pool in Git::SVN::Ra) does not help matters at all...\n+\tmy $receiver = pop @args;\n+\tmy $prefix = \"/\".$self->{svn_path};\n+\t$prefix =~ s#/+($)##;\n+\tmy $prefix_regex = qr#^\\Q$prefix\\E#;\n+\tpush(@args, sub {\n+\t\tmy ($paths) = $_[0];\n+\t\treturn &$receiver(@_) unless $paths;\n+\t\t$_[0] = ();\n+\t\tforeach my $p (keys %$paths) {\n+\t\t\tmy $i = $paths->{$p};\n+\t\t\t# Make path relative to our url, not repos_root\n+\t\t\t$p =~ s/$prefix_regex//;\n+\t\t\tmy %s = map { $_ => $i->$_; }\n+\t\t\t\tqw/copyfrom_path copyfrom_rev action/;\n+\t\t\t$s{'copyfrom_path'} =~ s/$prefix_regex// if $s{'copyfrom_path'};\n+\t\t\t$_[0]{$p} = \\%s;\n+\t\t}\n+\t\t&$receiver(@_);\n+\t});\n+\n \t# the limit parameter was not supported in SVN 1.1.x, so we\n \t# drop it.  Therefore, the receiver callback passed to it\n \t# is made aware of this limitation by being wrapped if\n@@ -4291,6 +4298,7 @@ sub get_log {\n \t\t\tpush(@args, sub { &$receiver(@_) if (--$limit >= 0) });\n \t\t}\n \t}\n+\n \tmy $ret = $self->SUPER::get_log(@args, $pool);\n \t$pool->clear;\n \t$ret;\n@@ -4448,8 +4456,7 @@ sub gs_fetch_loop_common {\n \t\t};\n \t\tsub _cb {\n \t\t\tmy ($paths, $r, $author, $date, $log) = @_;\n-\t\t\t[ dup_changed_paths($paths),\n-\t\t\t  { author => $author, date => $date, log => $log } ];\n+\t\t\t[ $paths, { author => $author, date => $date, log => $log } ];\n \t\t}\n \t\t$self->get_log([$longest_path], $min, $max, 0, 1, 1,\n \t\t               sub { $revs{$_[1]} = _cb(@_) });\n@@ -4668,24 +4675,6 @@ sub skip_unknown_revs {\n \tdie \"Error from SVN, ($errno): \", $err->expanded_message,\"\\n\";\n }\n \n-# svn_log_changed_path_t objects passed to get_log are likely to be\n-# overwritten even if only the refs are copied to an external variable,\n-# so we should dup the structures in their entirety.  Using an externally\n-# passed pool (instead of our temporary and quickly cleared pool in\n-# Git::SVN::Ra) does not help matters at all...\n-sub dup_changed_paths {\n-\tmy ($paths) = @_;\n-\treturn undef unless $paths;\n-\tmy %ret;\n-\tforeach my $p (keys %$paths) {\n-\t\tmy $i = $paths->{$p};\n-\t\tmy %s = map { $_ => $i->$_ }\n-\t\t              qw/copyfrom_path copyfrom_rev action/;\n-\t\t$ret{$p} = \\%s;\n-\t}\n-\t\\%ret;\n-}\n-\n package Git::SVN::Log;\n use strict;\n use warnings;\n-- \n1.6.2.2\n"}]}