{"thread":{"id":"36430","subject":"[PATCH 1/2] git-svn: only look at the new parts of svn:mergeinfo","startedAt":"2014-04-17T06:54:05Z","lastAt":"2014-04-27T19:00:02Z","messageCount":5,"participants":["Jakob Stoklund Olesen","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"239008","messageId":"1397717646-54248-1-git-send-email-stoklund@2pi.dk","threadId":"36430","inReplyTo":null,"subject":"[PATCH 1/2] git-svn: only look at the new parts of svn:mergeinfo","fromName":"Jakob Stoklund Olesen","fromEmail":"stoklund@2pi.dk","sentAt":"2014-04-17T06:54:05Z","receivedAt":"2014-04-17T06:54:05Z","isPatch":true,"sender":{"key":"stoklund@2pi.dk","avatar":"https://avatars.githubusercontent.com/u/12660495?v=4"},"body":"In a Subversion repository where many feature branches are merged into a\ntrunk, the svn:mergeinfo property can grow very large. This severely\nslows down git-svn's make_log_entry() because it is checking all\nmergeinfo entries every time the property changes.\n\nIn most cases, the additions to svn:mergeinfo since the last commit are\npretty small, and there is nothing to gain by checking merges that were\nalready checked for the last commit in the branch.\n\nAdd a mergeinfo_changes() function which computes the set of interesting\nchanges to svn:mergeinfo since the last commit. Filter out merged\nbranches whose ranges haven't changed, and remove a common prefix of\nranges from other merged branches.\n\nThis speeds up \"git svn fetch\" by several orders of magnitude on a large\nrepository where thousands of feature branches have been merged.\n\nSigned-off-by: Jakob Stoklund Olesen <stoklund@2pi.dk>\n---\n perl/Git/SVN.pm | 84 ++++++++++++++++++++++++++++++++++++++++++++++++---------\n 1 file changed, 72 insertions(+), 12 deletions(-)\n\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex a59564f..d3785ab 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -1178,7 +1178,7 @@ sub find_parent_branch {\n \t\t\t  or die \"SVN connection failed somewhere...\\n\";\n \t\t}\n \t\tprint STDERR \"Successfully followed parent\\n\" unless $::_q > 1;\n-\t\treturn $self->make_log_entry($rev, [$parent], $ed);\n+\t\treturn $self->make_log_entry($rev, [$parent], $ed, $r0, $branch_from);\n \t}\n \treturn undef;\n }\n@@ -1210,7 +1210,7 @@ sub do_fetch {\n \tunless ($self->ra->gs_do_update($last_rev, $rev, $self, $ed)) {\n \t\tdie \"SVN connection failed somewhere...\\n\";\n \t}\n-\t$self->make_log_entry($rev, \\@parents, $ed);\n+\t$self->make_log_entry($rev, \\@parents, $ed, $last_rev);\n }\n \n sub mkemptydirs {\n@@ -1478,9 +1478,9 @@ sub find_extra_svk_parents {\n sub lookup_svn_merge {\n \tmy $uuid = shift;\n \tmy $url = shift;\n-\tmy $merge = shift;\n+\tmy $source = shift;\n+\tmy $revs = shift;\n \n-\tmy ($source, $revs) = split \":\", $merge;\n \tmy $path = $source;\n \t$path =~ s{^/}{};\n \tmy $gs = Git::SVN->find_by_url($url.$source, $url, $path);\n@@ -1702,6 +1702,62 @@ sub parents_exclude {\n \treturn @excluded;\n }\n \n+# Compute what's new in svn:mergeinfo.\n+sub mergeinfo_changes {\n+\tmy ($self, $old_path, $old_rev, $path, $rev, $mergeinfo_prop) = @_;\n+\tmy %minfo = map {split \":\", $_ } split \"\\n\", $mergeinfo_prop;\n+\tmy $old_minfo = {};\n+\n+\t# Initialize cache on the first call.\n+\tunless (defined $self->{cached_mergeinfo_rev}) {\n+\t\t$self->{cached_mergeinfo_rev} = {};\n+\t\t$self->{cached_mergeinfo} = {};\n+\t}\n+\n+\tmy $cached_rev = $self->{cached_mergeinfo_rev}{$old_path};\n+\tif (defined $cached_rev && $cached_rev == $old_rev) {\n+\t\t$old_minfo = $self->{cached_mergeinfo}{$old_path};\n+\t} else {\n+\t\tmy $ra = $self->ra;\n+\t\t# Give up if $old_path isn't in the repo.\n+\t\t# This is probably a merge on a subtree.\n+\t\tif ($ra->check_path($old_path, $old_rev) != $SVN::Node::dir) {\n+\t\t\twarn \"W: ignoring svn:mergeinfo on $old_path, \",\n+\t\t\t\t\"directory didn't exist in r$old_rev\\n\";\n+\t\t\treturn {};\n+\t\t}\n+\t\tmy (undef, undef, $props) =\n+\t\t\t$self->ra->get_dir($old_path, $old_rev);\n+\t\tif (defined $props->{\"svn:mergeinfo\"}) {\n+\t\t\tmy %omi = map {split \":\", $_ } split \"\\n\",\n+\t\t\t\t$props->{\"svn:mergeinfo\"};\n+\t\t\t$old_minfo = \\%omi;\n+\t\t}\n+\t\t$self->{cached_mergeinfo}{$old_path} = $old_minfo;\n+\t\t$self->{cached_mergeinfo_rev}{$old_path} = $old_rev;\n+\t}\n+\n+\t# Cache the new mergeinfo.\n+\t$self->{cached_mergeinfo}{$path} = \\%minfo;\n+\t$self->{cached_mergeinfo_rev}{$path} = $rev;\n+\n+\tmy %changes = ();\n+\tforeach my $p (keys %minfo) {\n+\t\tmy $a = $old_minfo->{$p} || \"\";\n+\t\tmy $b = $minfo{$p};\n+\t\t# Omit merged branches whose ranges lists are unchanged.\n+\t\tnext if $a eq $b;\n+\t\t# Remove any common range list prefix.\n+\t\t($a ^ $b) =~ /^[\\0]*/;\n+\t\tmy $common_prefix = rindex $b, \",\", $+[0] - 1;\n+\t\t$changes{$p} = substr $b, $common_prefix + 1;\n+\t}\n+\tprint STDERR \"Checking svn:mergeinfo changes since r$old_rev: \",\n+\t\tscalar(keys %minfo), \" sources, \",\n+\t\tscalar(keys %changes), \" changed\\n\";\n+\n+\treturn \\%changes;\n+}\n \n # note: this function should only be called if the various dirprops\n # have actually changed\n@@ -1715,14 +1771,15 @@ sub find_extra_svn_parents {\n \t# history.  Then, we figure out which git revisions are in\n \t# that tip, but not this revision.  If all of those revisions\n \t# are now marked as merge, we can add the tip as a parent.\n-\tmy @merges = split \"\\n\", $mergeinfo;\n+\tmy @merges = sort keys %$mergeinfo;\n \tmy @merge_tips;\n \tmy $url = $self->url;\n \tmy $uuid = $self->ra_uuid;\n \tmy @all_ranges;\n \tfor my $merge ( @merges ) {\n \t\tmy ($tip_commit, @ranges) =\n-\t\t\tlookup_svn_merge( $uuid, $url, $merge );\n+\t\t\tlookup_svn_merge( $uuid, $url,\n+\t\t\t\t\t  $merge, $mergeinfo->{$merge} );\n \t\tunless (!$tip_commit or\n \t\t\t\tgrep { $_ eq $tip_commit } @$parents ) {\n \t\t\tpush @merge_tips, $tip_commit;\n@@ -1738,8 +1795,9 @@ sub find_extra_svn_parents {\n \t# check merge tips for new parents\n \tmy @new_parents;\n \tfor my $merge_tip ( @merge_tips ) {\n-\t\tmy $spec = shift @merges;\n+\t\tmy $merge = shift @merges;\n \t\tnext unless $merge_tip and $excluded{$merge_tip};\n+\t\tmy $spec = \"$merge:$mergeinfo->{$merge}\";\n \n \t\t# check out 'new' tips\n \t\tmy $merge_base;\n@@ -1770,7 +1828,7 @@ sub find_extra_svn_parents {\n \t\t\t\t.@incomplete.\" commit(s) (eg $incomplete[0])\\n\";\n \t\t} else {\n \t\t\twarn\n-\t\t\t\t\"Found merge parent (svn:mergeinfo prop): \",\n+\t\t\t\t\"Found merge parent ($spec): \",\n \t\t\t\t\t$merge_tip, \"\\n\";\n \t\t\tpush @new_parents, $merge_tip;\n \t\t}\n@@ -1797,7 +1855,7 @@ sub find_extra_svn_parents {\n }\n \n sub make_log_entry {\n-\tmy ($self, $rev, $parents, $ed) = @_;\n+\tmy ($self, $rev, $parents, $ed, $parent_rev, $parent_path) = @_;\n \tmy $untracked = $self->get_untracked($ed);\n \n \tmy @parents = @$parents;\n@@ -1809,10 +1867,12 @@ sub make_log_entry {\n \t\t\t\t($ed, $props->{\"svk:merge\"}, \\@parents);\n \t\t}\n \t\tif ( $props->{\"svn:mergeinfo\"} ) {\n+\t\t\tmy $mi_changes = $self->mergeinfo_changes\n+\t\t\t\t($parent_path || $path, $parent_rev,\n+\t\t\t\t $path, $rev,\n+\t\t\t\t $props->{\"svn:mergeinfo\"});\n \t\t\t$self->find_extra_svn_parents\n-\t\t\t\t($ed,\n-\t\t\t\t $props->{\"svn:mergeinfo\"},\n-\t\t\t\t \\@parents);\n+\t\t\t\t($ed, $mi_changes, \\@parents);\n \t\t}\n \t}\n \n-- \n1.8.5.2 (Apple Git-48)\n"},{"id":"239009","messageId":"1397717646-54248-2-git-send-email-stoklund@2pi.dk","threadId":"36430","inReplyTo":"1397717646-54248-1-git-send-email-stoklund@2pi.dk","subject":"[PATCH 2/2] git-svn: only look at the root path for svn:mergeinfo","fromName":"Jakob Stoklund Olesen","fromEmail":"stoklund@2pi.dk","sentAt":"2014-04-17T06:54:06Z","receivedAt":"2014-04-17T06:54:06Z","isPatch":true,"sender":{"key":"stoklund@2pi.dk","avatar":"https://avatars.githubusercontent.com/u/12660495?v=4"},"body":"Subversion can put mergeinfo on any sub-directory to track cherry-picks.\nSince cherry-picks are not represented explicitly in git, git-svn should\njust ignore it.\n\nSigned-off-by: Jakob Stoklund Olesen <stoklund@2pi.dk>\n---\n perl/Git/SVN.pm | 29 +++++++++++++----------------\n 1 file changed, 13 insertions(+), 16 deletions(-)\n\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex d3785ab..0aa4dd3 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -1210,7 +1210,7 @@ sub do_fetch {\n \tunless ($self->ra->gs_do_update($last_rev, $rev, $self, $ed)) {\n \t\tdie \"SVN connection failed somewhere...\\n\";\n \t}\n-\t$self->make_log_entry($rev, \\@parents, $ed, $last_rev);\n+\t$self->make_log_entry($rev, \\@parents, $ed, $last_rev, $self->path);\n }\n \n sub mkemptydirs {\n@@ -1859,21 +1859,18 @@ sub make_log_entry {\n \tmy $untracked = $self->get_untracked($ed);\n \n \tmy @parents = @$parents;\n-\tmy $ps = $ed->{path_strip} || \"\";\n-\tfor my $path ( grep { m/$ps/ } %{$ed->{dir_prop}} ) {\n-\t\tmy $props = $ed->{dir_prop}{$path};\n-\t\tif ( $props->{\"svk:merge\"} ) {\n-\t\t\t$self->find_extra_svk_parents\n-\t\t\t\t($ed, $props->{\"svk:merge\"}, \\@parents);\n-\t\t}\n-\t\tif ( $props->{\"svn:mergeinfo\"} ) {\n-\t\t\tmy $mi_changes = $self->mergeinfo_changes\n-\t\t\t\t($parent_path || $path, $parent_rev,\n-\t\t\t\t $path, $rev,\n-\t\t\t\t $props->{\"svn:mergeinfo\"});\n-\t\t\t$self->find_extra_svn_parents\n-\t\t\t\t($ed, $mi_changes, \\@parents);\n-\t\t}\n+\tmy $props = $ed->{dir_prop}{$self->path};\n+\tif ( $props->{\"svk:merge\"} ) {\n+\t\t$self->find_extra_svk_parents\n+\t\t\t($ed, $props->{\"svk:merge\"}, \\@parents);\n+\t}\n+\tif ( $props->{\"svn:mergeinfo\"} ) {\n+\t\tmy $mi_changes = $self->mergeinfo_changes\n+\t\t\t($parent_path, $parent_rev,\n+\t\t\t $self->path, $rev,\n+\t\t\t $props->{\"svn:mergeinfo\"});\n+\t\t$self->find_extra_svn_parents\n+\t\t\t($ed, $mi_changes, \\@parents);\n \t}\n \n \topen my $un, '>>', \"$self->{dir}/unhandled.log\" or croak $!;\n-- \n1.8.5.2 (Apple Git-48)\n"},{"id":"239364","messageId":"20140422184717.GA16766@dcvr.yhbt.net","threadId":"36430","inReplyTo":"1397717646-54248-1-git-send-email-stoklund@2pi.dk","subject":"Re: [PATCH 1/2] git-svn: only look at the new parts of svn:mergeinfo","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2014-04-22T18:47:17Z","receivedAt":"2014-04-22T18:47:17Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Thanks!  I still haven't gotten around to looking at svn:mergeinfo\nthings, but this passes tests so I'm inclined to merge this unless\nsomebody disagrees.\n"},{"id":"239365","messageId":"20140422185459.GA17248@dcvr.yhbt.net","threadId":"36430","inReplyTo":"1397717646-54248-2-git-send-email-stoklund@2pi.dk","subject":"Re: [PATCH 2/2] git-svn: only look at the root path for svn:mergeinfo","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2014-04-22T18:54:59Z","receivedAt":"2014-04-22T18:54:59Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jakob Stoklund Olesen <stoklund@2pi.dk> wrote:\n> Subversion can put mergeinfo on any sub-directory to track cherry-picks.\n> Since cherry-picks are not represented explicitly in git, git-svn should\n> just ignore it.\n\nHi, was git-svn trying to track cherry-picks as merge before?\n\nThis changes behavior a bit, so two independent users of git-svn\nmay not have identical histories as a result, correct?\n\nCan you add a test to ensure this behavior is preserved?\nThanks.\n\nSorry, I've never looked at mergeinfo myself, mainly relying on\nSam + tests for this.\n\n[1] - Historically, git-svn (using defaults) has always tried to\n      preserve identical histories for independent users across\n      different git-svn versions.  However, mergeinfo may be\n      enough of a corner-case where we can make an exception.\n"},{"id":"239817","messageId":"7C3E8DB5-4E0D-48B4-B5B6-3EE268AE639F@2pi.dk","threadId":"36430","inReplyTo":"20140422185459.GA17248@dcvr.yhbt.net","subject":"Re: [PATCH 2/2] git-svn: only look at the root path for svn:mergeinfo","fromName":"Jakob Stoklund Olesen","fromEmail":"stoklund@2pi.dk","sentAt":"2014-04-27T19:00:02Z","receivedAt":"2014-04-27T19:00:02Z","isPatch":true,"sender":{"key":"stoklund@2pi.dk","avatar":"https://avatars.githubusercontent.com/u/12660495?v=4"},"body":"\nOn Apr 22, 2014, at 11:54 AM, Eric Wong <normalperson@yhbt.net> wrote:\n\n> Jakob Stoklund Olesen <stoklund@2pi.dk> wrote:\n>> Subversion can put mergeinfo on any sub-directory to track cherry-picks.\n>> Since cherry-picks are not represented explicitly in git, git-svn should\n>> just ignore it.\n> \n> Hi, was git-svn trying to track cherry-picks as merge before?\n\nIt would try and fail. I didn't explain that properly in the commit message.\n\nSuppose I have a standard svn layout with $url/trunk and $url/branches/topic1. My topic1 branch has a change in subdir1 that I want to cherry-pick into trunk:\n\n% svn switch $url/trunk\n% cd subdir1\n% svn merge $url/branches/topic1/subdir1\n% cd ..\n% svn commit\n\nThis operation will set svn:mergeinfo on $url/trunk/subdir1 where a normal full merge would set it on $url/trunk:\n\n% svn pg svn:mergeinfo subdir1 \n/branches/topic1/subdir1:3-4\n\nWhen git-svn fetches these changes, it currently does examine the svn:mergeinfo change on the subdirectory as if it were a full merge. It then fails to find a revmap for /branches/topic1/subdir1:\n\nCouldn't find revmap for file:///tmp/sdb/branches/topic1/subdir1\nr5 = 5ce1f687c30495deca40730fb7be3baa0e145479 (refs/remotes/trunk)\n\nIt is looking for refs/remotes/topic1/subdir1, but we only have the refs/remotes/topic1 branch in git.\n\nThis patch makes git-svn stop trying to reconstruct those subdirectory merges that we know will fail anyway.\n\n> This changes behavior a bit, so two independent users of git-svn\n> may not have identical histories as a result, correct?\n\nFor normal subdirectory cherry-picks as described above, the behavior doesn't change. This is just a performance optimization.\n\nFor weirder cases where a whole branch has been merged onto a subdirectory of trunk, behavior does change. Currently, git-svn will mark that as a full merge in git. With this change it won't.\n\n> Can you add a test to ensure this behavior is preserved?\n> Thanks.\n\nI'll add a test for the subdirectory merge described above.\n\n> Sorry, I've never looked at mergeinfo myself, mainly relying on\n> Sam + tests for this.\n> \n> [1] - Historically, git-svn (using defaults) has always tried to\n>      preserve identical histories for independent users across\n>      different git-svn versions.  However, mergeinfo may be\n>      enough of a corner-case where we can make an exception.\n\n\nI agree. It doesn't seem worthwhile to try to preserve git-svn's historical behavior in weird corner cases.\n\nBTW, this performance optimization matters not because of sporadic manual cherry-picks, but because certain older svn releases would replicate svn:mergeinfo on every subdirectory in a standard merge. With hundreds of subdirectories and thousands of merged branches, git-svn gets completely stuck processing all those mergeinfo lines.\n\nThanks,\n/jakob\n"}]}