{"thread":{"id":"28287","subject":"[PATCH] git-svn: teach git-svn to populate svn:mergeinfo","startedAt":"2011-09-02T18:07:02Z","lastAt":"2011-09-07T14:14:34Z","messageCount":11,"participants":["Bryan Jacobs","Sam Vilain","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"174761","messageId":"20110902140702.066a4668@robyn.woti.com","threadId":"28287","inReplyTo":null,"subject":"[PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Bryan Jacobs","fromEmail":"bjacobs@woti.com","sentAt":"2011-09-02T18:07:02Z","receivedAt":"2011-09-02T18:07:02Z","isPatch":true,"sender":{"key":"bjacobs@woti.com","avatar":null},"body":">From a74814d4cd91627098e5be0209da30a2bc23904e Mon Sep 17 00:00:00 2001\nFrom: Bryan Jacobs <bjacobs@woti.com>\nDate: Thu, 1 Sep 2011 16:53:17 -0400\nSubject: [PATCH] git-svn: teach git-svn to populate svn:mergeinfo\n\nAllow git-svn to populate the svn:mergeinfo property automatically in\na narrow range of circumstances. Specifically, when dcommitting a\nrevision with multiple parents, each of which have been already\ncommitted to SVN.\n\nIn this case, the merge info is the union of that given by each of the\nparents, plus all changes introduced to the first parent by the other\nparents.\n\nIn all other cases where a revision has multiple parents, cause \"git\nsvn dcommit\" to raise an error rather than completing the commit and\npotentially losing history information in the upstream SVN repository.\n\nThis behavior is disabled by default, and can be enabled by setting\nthe svn.pushmergeinfo config option.\n\nSigned-off-by: Bryan Jacobs <bjacobs@woti.com>\n---\n\nPer previous discussion re: svn:mergeinfo handling, I am submitting for your consideration a patch which allows git-svn to write mergeinfo as well as read it.\n\nThis has several limitations. Specifically, it does not handle any cases concerning fast-forward merges, nor any with cherry-picks. It exclusively deals with --no-ff merges where both parents are already committed to SVN.\n\nFor this particular case, it works well: svn:mergeinfo is populated in such a way that the local merge history is recreated when another git-svn user pulls down the repository. This patch thus allows to git users to exchange branching and merging development through a central SVN server without loss of fidelity and without explicitly manipulating the mergeinfo property by hand.\n\nIf the user has made commits to two local branches, merges them with --no-ff, and attempts to dcommit the result, the tool will error out with a message stating that all parents must reside in the SVN repository before their merge can be committed.\n\nI was unable to discover a way to clean way to restore the repository state in the case where this happens before reaching the HEAD commit. For example:\n\nr1 --- B --- C -- F\n \\               /\n  r2 --- E ------\n\nIf r1 and r2 already reside in SVN, git-svn will dcommit B and C, then error when it tries to dcommit F (since the SVN revision number for E has not yet been set; the user should dcommit E before dcommitting F). The repository SHOULD be set to the following state after this happens:\n\nr1 -- r3 -- r4 -- F\n \\               /\n  r2 --- E ------\n\n... but git-svn seems to put all the changes from all objects that it will be dcommitting into the working copy, which means that cherry-picking F atop the state the WC is in fails (due to conflicts with \"untracked\" objects already added). So in this patch if you try the above, you actually end up in this state:\n\nr1 --- r3 -- r4\n \\\n  r2 --- E\n\nF is lost and cannot be cherry-picked back onto the WC, as any files created in E are already present but untracked locally.\n\nI would appreciate any help anyone can give with what the proper way to \"partially reset\" a git-svn commit which gets halfway through is. My attempt is the commented-out code in merge_commit_fail below. So far as I can tell, if *any* multi-revision dcommit of a merge is aborted partway through, it may end with an unclean WC and make reflog recovery necessary (even before my changes).\n\nThe other corner case I am unsure about is when two branches are merged, then the merge result is itself merged into a third. Like so:\n\nbranch1: r1 ----- r4\n                 /  \\\nbranch2:   r2 ---    \\\n                      \\\nbranch3: r3 --------- r5\n\nThis has the effect of branch3's mergeinfo containing r2 as part of both branch1 and branch2 (branch2:2 from the merge history set in r4, and branch1:2 from the changes being introduced by the second parent of r5). I wasn't sure whether the mergeinfo should be \"deduplicated\" and only contain r2 on branch2. Not sure what the stock SVN client does here.\n\nFinally, this makes NO EFFORT to handle svn:mergeinfo set on subdirectories of the branch root folder. The SVN red book advises against using mergeinfo on anything other than a branch top-level folder, so I think this is acceptable.\n\nAll new behavior is disabled by default, and all non-error-handling behavior is tested/demonstrated.\n\n Documentation/git-svn.txt         |    8 +\n git-svn.perl                      |  230 ++++++++++++++++++++++-\n t/t9160-git-svn-mergeinfo-push.sh |   97 ++++++++++\n t/t9160/branches.dump             |  374 +++++++++++++++++++++++++++++++++++++\n 4 files changed, 707 insertions(+), 2 deletions(-)\n create mode 100755 t/t9160-git-svn-mergeinfo-push.sh\n create mode 100644 t/t9160/branches.dump\n\ndiff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt\nindex ed5eca1..2bf5703 100644\n--- a/Documentation/git-svn.txt\n+++ b/Documentation/git-svn.txt\n@@ -213,6 +213,14 @@ discouraged.\n \tstore this information (as a property), and svn clients starting from\n \tversion 1.5 can make use of it. 'git svn' currently does not use it\n \tand does not set it automatically.\n++\n+[verse]\n+config key: svn.pushmergeinfo\n++\n+This option will cause git-svn to attempt to automatically populate the\n+svn:mergeinfo property in the SVN repository when possible. Currently, this can\n+only be done when dcommitting non-fast-forward merges where all parents have\n+already been pushed into SVN.\n \n 'branch'::\n \tCreate a branch in the SVN repository.\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 89f83fd..80df8a0 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -497,6 +497,216 @@ sub cmd_set_tree {\n \tunlink $gs->{index};\n }\n \n+sub split_merge_info_range {\n+\tmy ($range) = @_;\n+\tif ($range =~ /(\\d+)-(\\d+)/o) {\n+\t\treturn (int($1), int($2));\n+\t} else {\n+\t\treturn (int($range), int($range));\n+\t}\n+}\n+\n+sub combine_ranges {\n+\tmy ($in) = @_;\n+\n+\tmy @fnums = ();\n+\tmy @arr = split(/,/o, $in);\n+\tfor my $element (@arr) {\n+\t\tmy ($start, $end) = split_merge_info_range($element);\n+\t\tpush @fnums, $start;\n+\t}\n+\n+\tmy @sorted = @arr [ sort {\n+\t\t$fnums[$a] <=> $fnums[$b]\n+\t} 0..$#arr ];\n+\n+\tmy @return = ();\n+\tmy $last = -1;\n+\tmy $first = -1;\n+\tfor my $element (@sorted) {\n+\t\tmy ($start, $end) = split_merge_info_range($element);\n+\n+\t\tif ($last == -1) {\n+\t\t\t$first = $start;\n+\t\t\t$last = $end;\n+\t\t\tnext;\n+\t\t}\n+\t\tif ($start <= $last+1) {\n+\t\t\tif ($end > $last) {\n+\t\t\t\t$last = $end;\n+\t\t\t}\n+\t\t\tnext;\n+\t\t}\n+\t\tif ($first == $last) {\n+\t\t\tpush @return, \"$first\";\n+\t\t} else {\n+\t\t\tpush @return, \"$first-$last\";\n+\t\t}\n+\t\t$first = $start;\n+\t\t$last = $end;\n+\t}\n+\n+\tif ($first != -1) {\n+\t\tif ($first == $last) {\n+\t\t\tpush @return, \"$first\";\n+\t\t} else {\n+\t\t\tpush @return, \"$first-$last\";\n+\t\t}\n+\t}\n+\n+\treturn join(',', @return);\n+}\n+\n+sub merge_revs_into_hash {\n+\tmy ($hash, $minfo) = @_;\n+\tmy @lines = split(' ', $minfo);\n+\n+\tfor my $line (@lines) {\n+\t\tmy ($branchpath, $revs) = split(/:/o, $line);\n+\n+\t\tif (exists($hash->{$branchpath})) {\n+\t\t\t# Merge the two revision sets\n+\t\t\tmy $combined = \"$hash->{$branchpath},$revs\";\n+\t\t\t$hash->{$branchpath} = combine_ranges($combined);\n+\t\t} else {\n+\t\t\t# Just do range combining for consolidation\n+\t\t\t$hash->{$branchpath} = combine_ranges($revs);\n+\t\t}\n+\t}\n+}\n+\n+sub merge_merge_info {\n+\tmy ($mergeinfo_one, $mergeinfo_two) = @_;\n+\tmy %result_hash = ();\n+\n+\tmerge_revs_into_hash(\\%result_hash, $mergeinfo_one);\n+\tmerge_revs_into_hash(\\%result_hash, $mergeinfo_two);\n+\n+\tmy $result = '';\n+\t# Sort below is for consistency's sake\n+\tfor my $branchname (sort keys(%result_hash)) {\n+\t\tmy $revlist = $result_hash{$branchname};\n+\t\t$result .= \"$branchname:$revlist\\n\"\n+\t}\n+\treturn $result;\n+}\n+\n+sub merge_commit_fail {\n+\tmy ($gs, $linear_refs, $d) = @_;\n+\t#while (1) {\n+\t#\tmy $cs = shift @$linear_refs or last;\n+\t#\tcommand_noisy(qw/cherry-pick/, $cs);\n+\t#}\n+\t#command_noisy(qw/cherry-pick -m/, '1', $d);\n+\tfatal \"Aborted after failed dcommit of merge revision\";\n+}\n+\n+sub populate_merge_info {\n+\tmy ($d, $gs, $uuid, $linear_refs) = @_;\n+\n+\tmy %parentshash;\n+\tread_commit_parents(\\%parentshash, $d);\n+\tmy @parents = @{$parentshash{$d}};\n+\tif ($#parents > 0) {\n+\t\t# Merge commit\n+\t\tmy $all_parents_ok = 1;\n+\t\tmy $aggregate_mergeinfo = '';\n+\t\tmy $rooturl = $gs->repos_root;\n+\t\tforeach my $parent (@parents) {\n+\t\t\tmy ($branchurl, $svnrev, $paruuid) =\n+\t\t\t\tcmt_metadata($parent);\n+\n+\t\t\tunless (defined $paruuid) {\n+\t\t\t\t# A parent is missing SVN annotations...\n+\t\t\t\t# abort the whole operation.\n+\t\t\t\tprint \"$parent is merged into revision $d, \"\n+\t\t\t\t\t .\"but does not have git-svn metadata. \"\n+\t\t\t\t\t .\"Either dcommit the branch or use a \"\n+\t\t\t\t\t .\"local cherry-pick, FF merge, or rebase \"\n+\t\t\t\t\t .\"instead of an explicit merge commit.\\n\";\n+\t\t\t\tmerge_commit_fail($gs, $linear_refs, $d);\n+\t\t\t}\n+\n+\t\t\tunless ($paruuid eq $uuid) {\n+\t\t\t\t# Parent has SVN metadata from different repository\n+\t\t\t\tprint \"merge parent $parent for change $d has \"\n+\t\t\t\t\t .\"git-svn uuid $paruuid, while current change \"\n+\t\t\t\t\t .\"has uuid $uuid!\\n\";\n+\t\t\t\tmerge_commit_fail($gs, $linear_refs, $d);\n+\t\t\t}\n+\n+\t\t\tunless ($branchurl =~ /^$rooturl(.*)/) {\n+\t\t\t\t# This branch is very strange indeed.\n+\t\t\t\tprint \"merge parent $parent for $d is on branch \"\n+\t\t\t\t\t .\"$branchurl, which is not under the \"\n+\t\t\t\t\t .\"git-svn root $rooturl!\\n\";\n+\t\t\t\tmerge_commit_fail($gs, $linear_refs, $d);\n+\t\t\t}\n+\t\t\tmy $branchpath = $1;\n+\n+\t\t\tmy $ra = Git::SVN::Ra->new($branchurl);\n+\t\t\tmy (undef, undef, $props) =\n+\t\t\t\t$ra->get_dir(canonicalize_path(\".\"), $svnrev);\n+\t\t\tmy $par_mergeinfo = $props->{'svn:mergeinfo'};\n+\t\t\tunless (defined $par_mergeinfo) {\n+\t\t\t\t$par_mergeinfo = '';\n+\t\t\t}\n+\t\t\t# Merge previous mergeinfo values\n+\t\t\t$aggregate_mergeinfo =\n+\t\t\t\tmerge_merge_info($aggregate_mergeinfo,\n+\t\t\t\t\t\t\t\t $par_mergeinfo, 0);\n+\n+\t\t\tnext if $parent eq $parents[0]; # Skip first parent\n+\t\t\t# Add new changes being placed in tree by merge\n+\t\t\tmy @cmd = (qw/rev-list --reverse/,\n+\t\t\t\t\t   $parent, qw/--not/);\n+\t\t\tforeach my $par (@parents) {\n+\t\t\t\tunless ($par eq $parent) {\n+\t\t\t\t\tpush @cmd, $par;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\tmy @revsin = ();\n+\t\t\tmy ($revlist, $ctx) = command_output_pipe(@cmd);\n+\t\t\twhile (<$revlist>) {\n+\t\t\t\tmy $irev = $_;\n+\t\t\t\tchomp $irev;\n+\t\t\t\tmy (undef, $csvnrev, undef) =\n+\t\t\t\t\tcmt_metadata($irev);\n+\t\t\t\tunless (defined $csvnrev) {\n+\t\t\t\t\t# A child is missing SVN annotations...\n+\t\t\t\t\t# this might be OK, or might not be.\n+\t\t\t\t\twarn \"W:child $irev is merged into revision \"\n+\t\t\t\t\t\t .\"$d but does not have git-svn metadata. \"\n+\t\t\t\t\t\t .\"This means git-svn cannot determine the \"\n+\t\t\t\t\t\t .\"svn revision numbers to place into the \"\n+\t\t\t\t\t\t .\"svn:mergeinfo property. You must ensure \"\n+\t\t\t\t\t\t .\"a branch is entirely committed to \"\n+\t\t\t\t\t\t .\"SVN before merging it in order for \"\n+\t\t\t\t\t\t .\"svn:mergeinfo population to function \"\n+\t\t\t\t\t\t .\"properly\";\n+\t\t\t\t}\n+\t\t\t\tpush @revsin, $csvnrev;\n+\t\t\t}\n+\t\t\tcommand_close_pipe($revlist, $ctx);\n+\n+\t\t\tlast unless $all_parents_ok;\n+\n+\t\t\t# We now have a list of all SVN revnos which are\n+\t\t\t# merged by this particular parent. Integrate them.\n+\t\t\tnext if $#revsin == -1;\n+\t\t\tmy $newmergeinfo = \"$branchpath:\" . join(',', @revsin);\n+\t\t\t$aggregate_mergeinfo =\n+\t\t\t\tmerge_merge_info($aggregate_mergeinfo,\n+\t\t\t\t\t\t\t\t $newmergeinfo, 1);\n+\t\t}\n+\t\tif ($all_parents_ok and $aggregate_mergeinfo) {\n+\t\t\treturn $aggregate_mergeinfo;\n+\t\t}\n+\t}\n+\n+\treturn undef;\n+}\n+\n sub cmd_dcommit {\n \tmy $head = shift;\n \tcommand_noisy(qw/update-index --refresh/);\n@@ -547,6 +757,14 @@ sub cmd_dcommit {\n \t\t     \"without --no-rebase may be required.\"\n \t}\n \tmy $expect_url = $url;\n+\n+\tmy $push_merge_info = eval {\n+\t\tcommand_oneline(qw/config --get svn.pushmergeinfo/) };\n+\tif ($push_merge_info eq \"false\" or $push_merge_info eq \"no\"\n+\t\t\tor $push_merge_info eq \"never\") {\n+\t\t$push_merge_info = 0;\n+\t}\n+\n \tGit::SVN::remove_username($expect_url);\n \twhile (1) {\n \t\tmy $d = shift @$linear_refs or last;\n@@ -561,6 +779,12 @@ sub cmd_dcommit {\n \t\t\tprint \"diff-tree $d~1 $d\\n\";\n \t\t} else {\n \t\t\tmy $cmt_rev;\n+\n+\n+\t\t\tunless (defined $_merge_info or not $push_merge_info) {\n+\t\t\t\t$_merge_info = populate_merge_info($d, $gs, $uuid, $linear_refs);\n+\t\t\t}\n+\n \t\t\tmy %ed_opts = ( r => $last_rev,\n \t\t\t                log => get_commit_entry($d)->{log},\n \t\t\t                ra => Git::SVN::Ra->new($url),\n@@ -3341,8 +3565,10 @@ sub find_extra_svn_parents {\n \t\t       );\n \n \t\tif ( @incomplete ) {\n-\t\t\twarn \"W:svn cherry-pick ignored ($spec) - missing \"\n-\t\t\t\t.@incomplete.\" commit(s) (eg $incomplete[0])\\n\";\n+\t\t\twarn \"W:svn mergeinfo ignored ($spec) - \"\n+\t\t\t\t.@incomplete.\" commit(s) have not been merged into \"\n+\t\t\t\t.\"$merge_base (eg $incomplete[0]). This change \"\n+\t\t\t\t.\"represents a cherry-pick.\\n\";\n \t\t} else {\n \t\t\twarn\n \t\t\t\t\"Found merge parent (svn:mergeinfo prop): \",\ndiff --git a/t/t9160-git-svn-mergeinfo-push.sh b/t/t9160-git-svn-mergeinfo-push.sh\nnew file mode 100755\nindex 0000000..740adef\n--- /dev/null\n+++ b/t/t9160-git-svn-mergeinfo-push.sh\n@@ -0,0 +1,97 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007, 2009 Sam Vilain\n+#\n+\n+test_description='git-svn svn mergeinfo propagation'\n+\n+. ./lib-git-svn.sh\n+\n+test_expect_success 'load svn dump' \"\n+\tsvnadmin load -q '$rawsvnrepo' \\\n+\t  < '$TEST_DIRECTORY/t9160/branches.dump' &&\n+\tgit svn init --minimize-url -R svnmerge \\\n+\t  -T trunk -b branches '$svnrepo' &&\n+\tgit svn fetch --all\n+\t\"\n+\n+test_expect_success 'propagate merge information' '\n+\tgit config svn.pushmergeinfo yes &&\n+\tgit checkout svnb1 &&\n+\tgit merge --no-ff svnb2 &&\n+\tgit svn dcommit\n+\t'\n+\n+test_expect_success 'check svn:mergeinfo' '\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1)\n+\techo \"$mergeinfo\"\n+\ttest \"$mergeinfo\" = \"/branches/svnb2:3,8\"\n+\t'\n+\n+test_expect_success 'merge another branch' '\n+\tgit merge --no-ff svnb3 &&\n+\tgit svn dcommit\n+\t'\n+\n+test_expect_success 'check primary parent mergeinfo respected' '\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1)\n+\ttest \"$mergeinfo\" = \"/branches/svnb2:3,8\n+/branches/svnb3:4,9\"\n+\t'\n+\n+test_expect_success 'merge existing merge' '\n+\tgit merge --no-ff svnb4 &&\n+\tgit svn dcommit\n+\t'\n+\n+test_expect_success \"check both parents' mergeinfo respected\" '\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1)\n+\ttest \"$mergeinfo\" = \"/branches/svnb2:3,8\n+/branches/svnb3:4,9\n+/branches/svnb4:5-6,10-12\n+/branches/svnb5:6,11\"\n+\t'\n+\n+test_expect_success 'make further commits to branch' '\n+\tgit checkout svnb2 &&\n+\ttouch newb2file &&\n+\tgit add newb2file &&\n+\tgit commit -m \"later b2 commit\" &&\n+\ttouch newb2file-2 &&\n+\tgit add newb2file-2 &&\n+\tgit commit -m \"later b2 commit 2\" &&\n+\tgit svn dcommit\n+\t'\n+\n+test_expect_success 'second forward merge' '\n+\tgit checkout svnb1 &&\n+\tgit merge --no-ff svnb2 &&\n+\tgit svn dcommit\n+\t'\n+\n+test_expect_success \"check new mergeinfo added\" '\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1)\n+\techo \"$mergeinfo\"\n+\ttest \"$mergeinfo\" = \"/branches/svnb2:3,8,16-17\n+/branches/svnb3:4,9\n+/branches/svnb4:5-6,10-12\n+/branches/svnb5:6,11\"\n+\t'\n+\n+test_expect_success 'reintegration merge' '\n+\tgit checkout svnb4 &&\n+\tgit merge --no-ff svnb1 &&\n+\tgit svn dcommit\n+\t'\n+\n+test_expect_success \"check reintegration mergeinfo\" '\n+\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb4)\n+\techo \"$mergeinfo\"\n+\ttest \"$mergeinfo\" = \"/branches/svnb1:2-4,7-9,13-18\n+/branches/svnb2:3,8,16-17\n+/branches/svnb3:4,9\n+/branches/svnb4:5-6,10-12\n+/branches/svnb5:6,11\"\n+\t'\n+\n+test_done\ndiff --git a/t/t9160/branches.dump b/t/t9160/branches.dump\nnew file mode 100644\nindex 0000000..e61c3e7\n--- /dev/null\n+++ b/t/t9160/branches.dump\n@@ -0,0 +1,374 @@\n+SVN-fs-dump-format-version: 2\n+\n+UUID: 1ef08553-f2d1-45df-b38c-19af6b7c926d\n+\n+Revision-number: 0\n+Prop-content-length: 56\n+Content-length: 56\n+\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:08:02.941384Z\n+PROPS-END\n+\n+Revision-number: 1\n+Prop-content-length: 114\n+Content-length: 114\n+\n+K 7\n+svn:log\n+V 12\n+Base commit\n+\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:08:27.205062Z\n+PROPS-END\n+\n+Node-path: branches\n+Node-kind: dir\n+Node-action: add\n+Prop-content-length: 10\n+Content-length: 10\n+\n+PROPS-END\n+\n+\n+Node-path: trunk\n+Node-kind: dir\n+Node-action: add\n+Prop-content-length: 10\n+Content-length: 10\n+\n+PROPS-END\n+\n+\n+Revision-number: 2\n+Prop-content-length: 121\n+Content-length: 121\n+\n+K 7\n+svn:log\n+V 19\n+Create branch svnb1\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:09:43.628137Z\n+PROPS-END\n+\n+Node-path: branches/svnb1\n+Node-kind: dir\n+Node-action: add\n+Node-copyfrom-rev: 1\n+Node-copyfrom-path: trunk\n+\n+\n+Revision-number: 3\n+Prop-content-length: 121\n+Content-length: 121\n+\n+K 7\n+svn:log\n+V 19\n+Create branch svnb2\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:09:46.339930Z\n+PROPS-END\n+\n+Node-path: branches/svnb2\n+Node-kind: dir\n+Node-action: add\n+Node-copyfrom-rev: 1\n+Node-copyfrom-path: trunk\n+\n+\n+Revision-number: 4\n+Prop-content-length: 121\n+Content-length: 121\n+\n+K 7\n+svn:log\n+V 19\n+Create branch svnb3\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:09:49.394515Z\n+PROPS-END\n+\n+Node-path: branches/svnb3\n+Node-kind: dir\n+Node-action: add\n+Node-copyfrom-rev: 1\n+Node-copyfrom-path: trunk\n+\n+\n+Revision-number: 5\n+Prop-content-length: 121\n+Content-length: 121\n+\n+K 7\n+svn:log\n+V 19\n+Create branch svnb4\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:09:54.114607Z\n+PROPS-END\n+\n+Node-path: branches/svnb4\n+Node-kind: dir\n+Node-action: add\n+Node-copyfrom-rev: 1\n+Node-copyfrom-path: trunk\n+\n+\n+Revision-number: 6\n+Prop-content-length: 121\n+Content-length: 121\n+\n+K 7\n+svn:log\n+V 19\n+Create branch svnb5\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:09:58.602623Z\n+PROPS-END\n+\n+Node-path: branches/svnb5\n+Node-kind: dir\n+Node-action: add\n+Node-copyfrom-rev: 1\n+Node-copyfrom-path: trunk\n+\n+\n+Revision-number: 7\n+Prop-content-length: 110\n+Content-length: 110\n+\n+K 7\n+svn:log\n+V 9\n+b1 commit\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:10:20.292369Z\n+PROPS-END\n+\n+Node-path: branches/svnb1/b1file\n+Node-kind: file\n+Node-action: add\n+Prop-content-length: 10\n+Text-content-length: 0\n+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e\n+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709\n+Content-length: 10\n+\n+PROPS-END\n+\n+\n+Revision-number: 8\n+Prop-content-length: 110\n+Content-length: 110\n+\n+K 7\n+svn:log\n+V 9\n+b2 commit\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:10:38.429199Z\n+PROPS-END\n+\n+Node-path: branches/svnb2/b2file\n+Node-kind: file\n+Node-action: add\n+Prop-content-length: 10\n+Text-content-length: 0\n+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e\n+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709\n+Content-length: 10\n+\n+PROPS-END\n+\n+\n+Revision-number: 9\n+Prop-content-length: 110\n+Content-length: 110\n+\n+K 7\n+svn:log\n+V 9\n+b3 commit\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:10:52.843023Z\n+PROPS-END\n+\n+Node-path: branches/svnb3/b3file\n+Node-kind: file\n+Node-action: add\n+Prop-content-length: 10\n+Text-content-length: 0\n+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e\n+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709\n+Content-length: 10\n+\n+PROPS-END\n+\n+\n+Revision-number: 10\n+Prop-content-length: 110\n+Content-length: 110\n+\n+K 7\n+svn:log\n+V 9\n+b4 commit\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:11:17.489870Z\n+PROPS-END\n+\n+Node-path: branches/svnb4/b4file\n+Node-kind: file\n+Node-action: add\n+Prop-content-length: 10\n+Text-content-length: 0\n+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e\n+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709\n+Content-length: 10\n+\n+PROPS-END\n+\n+\n+Revision-number: 11\n+Prop-content-length: 110\n+Content-length: 110\n+\n+K 7\n+svn:log\n+V 9\n+b5 commit\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:11:32.277404Z\n+PROPS-END\n+\n+Node-path: branches/svnb5/b5file\n+Node-kind: file\n+Node-action: add\n+Prop-content-length: 10\n+Text-content-length: 0\n+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e\n+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709\n+Content-length: 10\n+\n+PROPS-END\n+\n+\n+Revision-number: 12\n+Prop-content-length: 192\n+Content-length: 192\n+\n+K 7\n+svn:log\n+V 90\n+Merge remote-tracking branch 'svnb5' into HEAD\n+\n+* svnb5:\n+  b5 commit\n+  Create branch svnb5\n+K 10\n+svn:author\n+V 7\n+bjacobs\n+K 8\n+svn:date\n+V 27\n+2011-09-02T16:11:54.274722Z\n+PROPS-END\n+\n+Node-path: branches/svnb4\n+Node-kind: dir\n+Node-action: change\n+Prop-content-length: 56\n+Content-length: 56\n+\n+K 13\n+svn:mergeinfo\n+V 21\n+/branches/svnb5:6,11\n+\n+PROPS-END\n+\n+\n+Node-path: branches/svnb4/b5file\n+Node-kind: file\n+Node-action: add\n+Prop-content-length: 10\n+Text-content-length: 0\n+Text-content-md5: d41d8cd98f00b204e9800998ecf8427e\n+Text-content-sha1: da39a3ee5e6b4b0d3255bfef95601890afd80709\n+Content-length: 10\n+\n+PROPS-END\n+\n+\n-- \n1.7.7.rc0.73.gc42335.dirty\n"},{"id":"174764","messageId":"4E612319.7030006@vilain.net","threadId":"28287","inReplyTo":"20110902140702.066a4668@robyn.woti.com","subject":"Re: [spf:guess,iffy] [PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2011-09-02T18:40:25Z","receivedAt":"2011-09-02T18:40:25Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On 9/2/11 11:07 AM, Bryan Jacobs wrote:\n> For this particular case, it works well: svn:mergeinfo is populated in such a way that the local merge history is recreated when another git-svn user pulls down the repository. This patch thus allows to git users to exchange branching and merging development through a central SVN server without loss of fidelity and without explicitly manipulating the mergeinfo property by hand.\n\nWhee!  That's what I was intending when I wrote the original change.  I \nmight have written it myself back in 2008 or whenever it was, but I \nfound I didn't actually have any SVN projects I was sending commits to, \nlet alone merges.  git-svn is a project with a continually atrophying \nuserbase :-).  Thanks for picking it up.\n\n> If the user has made commits to two local branches, merges them with --no-ff, and attempts to dcommit the result, the tool will error out with a message stating that all parents must reside in the SVN repository before their merge can be committed.\n>\n> I was unable to discover a way to clean way to restore the repository state in the case where this happens before reaching the HEAD commit. For example:\n>\n> r1 --- B --- C -- F\n>   \\               /\n>    r2 --- E ------\n>\n> If r1 and r2 already reside in SVN, git-svn will dcommit B and C, then error when it tries to dcommit F (since the SVN revision number for E has not yet been set; the user should dcommit E before dcommitting F). The repository SHOULD be set to the following state after this happens:\n>\n> r1 -- r3 -- r4 -- F\n>   \\               /\n>    r2 --- E ------\n>\n> ... but git-svn seems to put all the changes from all objects that it will be dcommitting into the working copy, which means that cherry-picking F atop the state the WC is in fails (due to conflicts with \"untracked\" objects already added). So in this patch if you try the above, you actually end up in this state:\n>\n> r1 --- r3 -- r4\n>   \\\n>    r2 --- E\n>\n> F is lost and cannot be cherry-picked back onto the WC, as any files created in E are already present but untracked locally.\n\nAre r1 and r2 supposed to be on the same SVN branch?\n\nOverall, I could believe that.  Perhaps it is simpler to detect those \nsituations in advance and insist the user dcommits them independently, \nalthough it appears to me that it would apply to any dcommit which \nfailed for any reason part way through.  So perhaps there is a wider \njustification for fixing that.\n\nSam\n"},{"id":"174766","messageId":"20110902144922.383ed0f1@robyn.woti.com","threadId":"28287","inReplyTo":"4E612319.7030006@vilain.net","subject":"Re: [spf:guess,iffy] [PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Bryan Jacobs","fromEmail":"bjacobs@woti.com","sentAt":"2011-09-02T18:49:22Z","receivedAt":"2011-09-02T18:49:22Z","isPatch":true,"sender":{"key":"bjacobs@woti.com","avatar":null},"body":"On Fri, 02 Sep 2011 11:40:25 -0700\nSam Vilain <sam@vilain.net> wrote:\n\n> On 9/2/11 11:07 AM, Bryan Jacobs wrote:\n> > For this particular case, it works well: svn:mergeinfo is populated\n> > in such a way that the local merge history is recreated when\n> > another git-svn user pulls down the repository. This patch thus\n> > allows to git users to exchange branching and merging development\n> > through a central SVN server without loss of fidelity and without\n> > explicitly manipulating the mergeinfo property by hand.\n> \n> Whee!  That's what I was intending when I wrote the original change.\n> I might have written it myself back in 2008 or whenever it was, but I \n> found I didn't actually have any SVN projects I was sending commits\n> to, let alone merges.  git-svn is a project with a continually\n> atrophying userbase :-).  Thanks for picking it up.\n> \n\nGlad to hear it. I think there's still work to be done, mostly because\nI'm not very familiar with the git codebase and the \"right\" way to do\nthings, but I want this to work.\n\n> > r1 --- r3 -- r4\n> >   \\\n> >    r2 --- E\n> >\n> > F is lost and cannot be cherry-picked back onto the WC, as any\n> > files created in E are already present but untracked locally.\n> \n> Are r1 and r2 supposed to be on the same SVN branch?\n\nNo, different SVN branches.\n\n> \n> Overall, I could believe that.  Perhaps it is simpler to detect those \n> situations in advance and insist the user dcommits them\n> independently, although it appears to me that it would apply to any\n> dcommit which failed for any reason part way through.  So perhaps\n> there is a wider justification for fixing that.\n\nI could do a pass through all the commits which are about to be sent\nout to SVN to check if this is going to happen, yes. But I think a\nbetter solution would be to change how the changes are replayed by\ngit-svn dcommit: right now, all changes are applied to the WC, then it\nsequentially does an add+dcommit for each patch? Right? I think it might\nbe better to reset --hard to the parent, then pick each change into the\nWC+index before committing. That way if you abort early, cleaning up\njust consists of rebasing the stack onto the last change you sent\nupstream.\n\nIf I get around to making git-svn put its stuff into notes, this would\nbe a lot easier since you could just reset --hard back to the original\nHEAD, since none of the earlier commits would have been mangled. But of\ncourse everyone who already imported a repo would be SOL if the new\nversion relied on that Hippocratic behavior...\n\n> Sam\n"},{"id":"174767","messageId":"4E6127F5.5070009@vilain.net","threadId":"28287","inReplyTo":"20110902144922.383ed0f1@robyn.woti.com","subject":"Re: [spf:guess,iffy] Re: [spf:guess,iffy] [PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2011-09-02T19:01:09Z","receivedAt":"2011-09-02T19:01:09Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On 9/2/11 11:49 AM, Bryan Jacobs wrote:\n> I could do a pass through all the commits which are about to be sent\n> out to SVN to check if this is going to happen, yes. But I think a\n> better solution would be to change how the changes are replayed by\n> git-svn dcommit: right now, all changes are applied to the WC, then it\n> sequentially does an add+dcommit for each patch? Right? I think it might\n> be better to reset --hard to the parent, then pick each change into the\n> WC+index before committing. That way if you abort early, cleaning up\n> just consists of rebasing the stack onto the last change you sent\n> upstream.\n\nThat's one way to do it; in fact, if the trees match you don't need to \ndo anything complicated like cherry-pick.\n\nie, say you're committing\n\n    r1---A---B---C---D\n\nand it blows up at\n\n    r1--r2--r3--C---D\n\nSo long as the tree from the fetched r3 == the tree from B, then you can \njust go ahead and write out new commits for C and D without doing any \nmerging (ie cherry-pick or rebase).  You could also put merge commits \nback the way they were, too.\n\nIf they don't match, then something went wrong with the push really, or \nthere is something weird going on.  I'd try to avoid using cherry pick \nautomatically in situations like this.  There are too many error modes, \nand if it only happens when you don't know what's going on, it's not a \ngood idea to try to fix that.  If it /is/ a sufficiently unlikely error \n(ie, the trees not matching as above), then it would be better to simply \nbomb out and provide two commands:\n\n* a 'git reset' command to restore to previous state (ie, before the \ndcommit)\n* a 'git rebase' command to attempt to put the new history on top of the \nnew upstream.  Rebase doesn't work with merges of course but it still \nshould help the user figure out what to do.\n\nAnother benefit of this approach is that you don't need to muck with the \nWC + index at all, no matter what happens.\n\nSam\n"},{"id":"174769","messageId":"20110902154206.331b80e9@robyn.woti.com","threadId":"28287","inReplyTo":"4E6127F5.5070009@vilain.net","subject":"Re: [spf:guess,iffy] Re: [spf:guess,iffy] [PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Bryan Jacobs","fromEmail":"bjacobs@woti.com","sentAt":"2011-09-02T19:42:06Z","receivedAt":"2011-09-02T19:42:06Z","isPatch":true,"sender":{"key":"bjacobs@woti.com","avatar":null},"body":"On Fri, 02 Sep 2011 12:01:09 -0700\nSam Vilain <sam@vilain.net> wrote:\n\n> That's one way to do it; in fact, if the trees match you don't need\n> to do anything complicated like cherry-pick.\n> \n> ie, say you're committing\n> \n>     r1---A---B---C---D\n> \n> and it blows up at\n> \n>     r1--r2--r3--C---D\n> \n> So long as the tree from the fetched r3 == the tree from B, then you\n> can just go ahead and write out new commits for C and D without doing\n> any merging (ie cherry-pick or rebase).  You could also put merge\n> commits back the way they were, too.\n\nWhen you say \"write out new commits\" you mean create a commit object\nwith the same contents, but a different parent? Does git-svn do this\nsomewhere already?\n\n> If they don't match, then something went wrong with the push really,\n> or there is something weird going on.  I'd try to avoid using cherry\n> pick automatically in situations like this.  There are too many error\n> modes, and if it only happens when you don't know what's going on,\n> it's not a good idea to try to fix that.  If it /is/ a sufficiently\n> unlikely error (ie, the trees not matching as above), then it would\n> be better to simply bomb out and provide two commands:\n> \n> * a 'git reset' command to restore to previous state (ie, before the \n> dcommit)\n> * a 'git rebase' command to attempt to put the new history on top of\n> the new upstream.  Rebase doesn't work with merges of course but it\n> still should help the user figure out what to do.\n> \n> Another benefit of this approach is that you don't need to muck with\n> the WC + index at all, no matter what happens.\n\nAll of the above sounds good to me. I haven't taken the time to\nunderstand how git-svn sends changesets upstream (I only know it mucks\nwith the WC from empirical experience) so I don't know how easy it would\nbe to change the methodology, though.\n\nWould this also mean we could dcommit from a dirty checkout? Having to\nstash/unstash is a nuisance.\n\n> Sam\n> \n"},{"id":"174773","messageId":"4E614AE7.7090706@vilain.net","threadId":"28287","inReplyTo":"20110902154206.331b80e9@robyn.woti.com","subject":"Re: [PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Sam Vilain","fromEmail":"sam@vilain.net","sentAt":"2011-09-02T21:30:15Z","receivedAt":"2011-09-02T21:30:15Z","isPatch":true,"sender":{"key":"sam@vilain.net","avatar":"https://gravatar.com/avatar/8fc840ca854dbf6f7065b4335e3b934951c1dca3b11db688e95e471901f8f4a8?d=mp&s=160"},"body":"On 9/2/11 12:42 PM, Bryan Jacobs wrote:\n> On Fri, 02 Sep 2011 12:01:09 -0700\n> Sam Vilain<sam@vilain.net>  wrote:\n>\n>> That's one way to do it; in fact, if the trees match you don't need\n>> to do anything complicated like cherry-pick.\n>>\n>> ie, say you're committing\n>>\n>>      r1---A---B---C---D\n>>\n>> and it blows up at\n>>\n>>      r1--r2--r3--C---D\n>>\n>> So long as the tree from the fetched r3 == the tree from B, then you\n>> can just go ahead and write out new commits for C and D without doing\n>> any merging (ie cherry-pick or rebase).  You could also put merge\n>> commits back the way they were, too.\n> When you say \"write out new commits\" you mean create a commit object\n> with the same contents, but a different parent? Does git-svn do this\n> somewhere already?\n\nI guess it doesn't, but if it did it would certainly make this easier.  \nI'm not sure why it would need to modify the WC at all.  Eric, is this \njust historical or is there a better reason for that?\n\nSam\n"},{"id":"174781","messageId":"20110903084947.GA16711@dcvr.yhbt.net","threadId":"28287","inReplyTo":"4E614AE7.7090706@vilain.net","subject":"Re: [PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2011-09-03T08:49:47Z","receivedAt":"2011-09-03T08:49:47Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Sam Vilain <sam@vilain.net> wrote:\n> On 9/2/11 12:42 PM, Bryan Jacobs wrote:\n> >On Fri, 02 Sep 2011 12:01:09 -0700\n> >Sam Vilain<sam@vilain.net>  wrote:\n> >\n> >>That's one way to do it; in fact, if the trees match you don't need\n> >>to do anything complicated like cherry-pick.\n> >>\n> >>ie, say you're committing\n> >>\n> >>     r1---A---B---C---D\n> >>\n> >>and it blows up at\n> >>\n> >>     r1--r2--r3--C---D\n> >>\n> >>So long as the tree from the fetched r3 == the tree from B, then you\n> >>can just go ahead and write out new commits for C and D without doing\n> >>any merging (ie cherry-pick or rebase).  You could also put merge\n> >>commits back the way they were, too.\n> >When you say \"write out new commits\" you mean create a commit object\n> >with the same contents, but a different parent? Does git-svn do this\n> >somewhere already?\n> \n> I guess it doesn't, but if it did it would certainly make this\n> easier.  I'm not sure why it would need to modify the WC at all.\n> Eric, is this just historical or is there a better reason for that?\n\ndcommit needs to continually rebase because it's possible somebody else\nmay make a commit to the SVN repo while a git-svn user is dcommiting\nand cause a conflict the user would need to resolve in the working tree.\n\nAt least I think that was the reason...  There is also the \"commit-diff\"\ncommand in git-svn.  It was the precursor to dcommit which requires no\nchanges to the working tree.\n\n-- \nEric Wong\n"},{"id":"174926","messageId":"20110906100003.4c87daba@robyn.woti.com","threadId":"28287","inReplyTo":"20110903084947.GA16711@dcvr.yhbt.net","subject":"Re: [PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Bryan Jacobs","fromEmail":"bjacobs@woti.com","sentAt":"2011-09-06T14:00:03Z","receivedAt":"2011-09-06T14:00:03Z","isPatch":true,"sender":{"key":"bjacobs@woti.com","avatar":null},"body":"On Sat, 3 Sep 2011 08:49:47 +0000\nEric Wong <normalperson@yhbt.net> wrote:\n\n> dcommit needs to continually rebase because it's possible somebody\n> else may make a commit to the SVN repo while a git-svn user is\n> dcommiting and cause a conflict the user would need to resolve in the\n> working tree.\n> \n> At least I think that was the reason...  There is also the\n> \"commit-diff\" command in git-svn.  It was the precursor to dcommit\n> which requires no changes to the working tree.\n> \n\nLet me see if I've got this right.\n\nThe goal here is to commit each x~..x for each x in A..B, aborting if\nthe SVN tree is not in state \"x~\" when the diff arrives.\n\n\"commit-diff\" appears to be doing exactly what \"dcommit\" is doing, but\niteratively for each change in linearized A..B, rebasing after each\nstep. This sounds correct to me, assuming that the \"apply_diff\" method\nwill correctly abort if a commit races into the upstream SVN before it\nis called. So why am I seeing files added in changes on alternate\nbranches ending up in the working copy when I abort before apply_diff\nis called for the commit which merges them into the present branch?\n\nYou can check for this yourself with my patch using the example setup I\ngave earlier. You'll see files in the present/untracked state - these\ninterfere with rebasing the user-created-but-not-SVN-dcommited merge\nonto the partially-sent-to-SVN tree.\n\nBryan Jacobs\n"},{"id":"174959","messageId":"20110906204558.GA12574@dcvr.yhbt.net","threadId":"28287","inReplyTo":"20110906100003.4c87daba@robyn.woti.com","subject":"Re: [PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2011-09-06T20:45:58Z","receivedAt":"2011-09-06T20:45:58Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Bryan Jacobs <bjacobs@woti.com> wrote:\n> On Sat, 3 Sep 2011 08:49:47 +0000\n> Eric Wong <normalperson@yhbt.net> wrote:\n> > dcommit needs to continually rebase because it's possible somebody\n> > else may make a commit to the SVN repo while a git-svn user is\n> > dcommiting and cause a conflict the user would need to resolve in the\n> > working tree.\n> > \n> > At least I think that was the reason...  There is also the\n> > \"commit-diff\" command in git-svn.  It was the precursor to dcommit\n> > which requires no changes to the working tree.\n> \n> Let me see if I've got this right.\n> \n> The goal here is to commit each x~..x for each x in A..B, aborting if\n> the SVN tree is not in state \"x~\" when the diff arrives.\n\nYes.\n\n> So why am I seeing files added in changes on alternate\n> branches ending up in the working copy when I abort before apply_diff\n> is called for the commit which merges them into the present branch?\n\nI don't know.\n\nIn my past use of git-svn, I've _always_ stuck with linear changes and\navoided anything non-linear.  SVN mergeinfo didn't exist when/where I\nused SVN and my only current uses of git-svn is read-only.\n\n\nAnyhow, I'm willing to accept your change since it doesn't appear to\nbreak anything for existing users and Sam seems to approve.\n\n-- \nEric Wong\n"},{"id":"174960","messageId":"20110906205750.GB12574@dcvr.yhbt.net","threadId":"28287","inReplyTo":"20110902140702.066a4668@robyn.woti.com","subject":"Re: [PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2011-09-06T20:57:50Z","receivedAt":"2011-09-06T20:57:50Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Bryan Jacobs <bjacobs@woti.com> wrote:\n> +sub split_merge_info_range {\n> +\tmy ($range) = @_;\n> +\tif ($range =~ /(\\d+)-(\\d+)/o) {\n\nNo need for \"/o\" in regexps unless you have a (constant) variable\nexpansion in there.\n\n> +sub merge_commit_fail {\n> +\tmy ($gs, $linear_refs, $d) = @_;\n> +\t#while (1) {\n> +\t#\tmy $cs = shift @$linear_refs or last;\n> +\t#\tcommand_noisy(qw/cherry-pick/, $cs);\n> +\t#}\n> +\t#command_noisy(qw/cherry-pick -m/, '1', $d);\n\nHuh?  If there's commented-out code, it must be explained or removed.\n\n> +\tfatal \"Aborted after failed dcommit of merge revision\";\n> +}\n\n> +++ b/t/t9160-git-svn-mergeinfo-push.sh\n> @@ -0,0 +1,97 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (c) 2007, 2009 Sam Vilain\n\nThat should be: \"Copyright (c) 2011 Brian Jacobs\", correct?\n\n> +test_expect_success 'check svn:mergeinfo' '\n> +\tmergeinfo=$(svn_cmd propget svn:mergeinfo \"$svnrepo\"/branches/svnb1)\n> +\techo \"$mergeinfo\"\n\nNo need to echo unless you're debugging a test, right?\n\n-- \nEric Wong\n"},{"id":"174992","messageId":"20110907101434.281d037f@robyn.woti.com","threadId":"28287","inReplyTo":"20110906205750.GB12574@dcvr.yhbt.net","subject":"Re: [PATCH] git-svn: teach git-svn to populate svn:mergeinfo","fromName":"Bryan Jacobs","fromEmail":"bjacobs@woti.com","sentAt":"2011-09-07T14:14:34Z","receivedAt":"2011-09-07T14:14:34Z","isPatch":true,"sender":{"key":"bjacobs@woti.com","avatar":null},"body":"On Tue, 6 Sep 2011 13:57:50 -0700\nEric Wong <normalperson@yhbt.net> wrote:\n\n> Bryan Jacobs <bjacobs@woti.com> wrote:\n> > +sub split_merge_info_range {\n> > +\tmy ($range) = @_;\n> > +\tif ($range =~ /(\\d+)-(\\d+)/o) {\n> \n> No need for \"/o\" in regexps unless you have a (constant) variable\n> expansion in there.\n\nOkay, I'll take that out. I got into the habit of putting \"optimize\" on\nall regexes without an explicitly dynamic variable on some earlier Perl\nversion.\n\n> > +sub merge_commit_fail {\n> > +\tmy ($gs, $linear_refs, $d) = @_;\n> > +\t#while (1) {\n> > +\t#\tmy $cs = shift @$linear_refs or last;\n> > +\t#\tcommand_noisy(qw/cherry-pick/, $cs);\n> > +\t#}\n> > +\t#command_noisy(qw/cherry-pick -m/, '1', $d);\n> \n> Huh?  If there's commented-out code, it must be explained or removed.\n\nI think I did explain that in my earlier comments. I'm still not happy\nwith the recovery-from-aborted-commit-series handling. That commented\nbit was my attempt.\n\nThe best suggestion so far is to prescan the commits to fail-fast. I\nwill do that in the next revision of the patch, just give me some time\nto put it together.\n\n> > +\tfatal \"Aborted after failed dcommit of merge revision\";\n> > +}\n> \n> > +++ b/t/t9160-git-svn-mergeinfo-push.sh\n> > @@ -0,0 +1,97 @@\n> > +#!/bin/sh\n> > +#\n> > +# Copyright (c) 2007, 2009 Sam Vilain\n> \n> That should be: \"Copyright (c) 2011 Brian Jacobs\", correct?\n\nWell, the file was copied from one bearing the Vilain copyright bit.\nI'm not sure I entirely understand why it matters who holds the\nindividual copyrights if you have a collective license which is going to\nbe changed, but I can't just stick my own name on derived work - as the\nsetup code for that unit is.\n\n> > +test_expect_success 'check svn:mergeinfo' '\n> > +\tmergeinfo=$(svn_cmd propget svn:mergeinfo\n> > \"$svnrepo\"/branches/svnb1)\n> > +\techo \"$mergeinfo\"\n> \n> No need to echo unless you're debugging a test, right?\n> \n\nCorrect, leftover test-debugging cruft, will remove.\n\nI will submit another revision shortly.\n\nBryan Jacobs\n"}]}