{"thread":{"id":"10667","subject":"[PATCH 1/2] git-svn: fix dcommit clobbering when committing a series of diffs","startedAt":"2007-11-05T11:21:47Z","lastAt":"2007-11-05T16:40:12Z","messageCount":4,"participants":["Eric Wong","Peter Baumann"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"58360","messageId":"1194261708-32256-1-git-send-email-normalperson@yhbt.net","threadId":"10667","inReplyTo":null,"subject":"[PATCH 1/2] git-svn: fix dcommit clobbering when committing a series of diffs","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-11-05T11:21:47Z","receivedAt":"2007-11-05T11:21:47Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Our revision number sent to SVN is set to the last revision we\ncommitted if we've made any previous commits in a dcommit\ninvocation.\n\nAlthough our SVN Editor code uses the delta of two (old) trees\nto generate information to send upstream, it'll still send\ncomplete resultant files upstream; even if the tree they're\nbased against is out-of-date.\n\nThe combination of sending a file that does not include the\nlatest changes, but set with a revision number of a commit we\njust made will cause SVN to accept the resultant file even if it\nwas generated against an old tree.\n\nMore trouble was caused when fixing this because we were\nrebasing uncessarily at times.  We used git-diff-tree to check\nthe imported SVN revision against our HEAD, not the last tree we\ncommitted to SVN.  The unnecessary rebasing caused merge commits\nupstream to SVN to fail.\n\nSigned-off-by: Eric Wong <normalperson@yhbt.net>\n---\n\n Junio: please apply this to maint, thanks.\n\n git-svn.perl                              |   47 ++++++++++++++++++++++--\n t/t9106-git-svn-dcommit-clobber-series.sh |   56 +++++++++++++++++++++++++++++\n 2 files changed, 99 insertions(+), 4 deletions(-)\n create mode 100755 t/t9106-git-svn-dcommit-clobber-series.sh\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4900f57..62a4a69 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -406,7 +406,8 @@ sub cmd_dcommit {\n \t\t     \"If these changes depend on each other, re-running \",\n \t\t     \"without --no-rebase will be required.\"\n \t}\n-\tforeach my $d (@$linear_refs) {\n+\twhile (1) {\n+\t\tmy $d = shift @$linear_refs or last;\n \t\tunless (defined $last_rev) {\n \t\t\t(undef, $last_rev, undef) = cmt_metadata(\"$d~1\");\n \t\t\tunless (defined $last_rev) {\n@@ -439,14 +440,14 @@ sub cmd_dcommit {\n \n \t\t\t# we always want to rebase against the current HEAD,\n \t\t\t# not any head that was passed to us\n-\t\t\tmy @diff = command('diff-tree', 'HEAD',\n+\t\t\tmy @diff = command('diff-tree', $d,\n \t\t\t                   $gs->refname, '--');\n \t\t\tmy @finish;\n \t\t\tif (@diff) {\n \t\t\t\t@finish = rebase_cmd();\n-\t\t\t\tprint STDERR \"W: HEAD and \", $gs->refname,\n+\t\t\t\tprint STDERR \"W: $d and \", $gs->refname,\n \t\t\t\t             \" differ, using @finish:\\n\",\n-\t\t\t\t             \"@diff\";\n+\t\t\t\t             join(\"\\n\", @diff), \"\\n\";\n \t\t\t} else {\n \t\t\t\tprint \"No changes between current HEAD and \",\n \t\t\t\t      $gs->refname,\n@@ -455,6 +456,45 @@ sub cmd_dcommit {\n \t\t\t\t@finish = qw/reset --mixed/;\n \t\t\t}\n \t\t\tcommand_noisy(@finish, $gs->refname);\n+\t\t\tif (@diff) {\n+\t\t\t\t@refs = ();\n+\t\t\t\tmy ($url_, $rev_, $uuid_, $gs_) =\n+\t\t\t\t              working_head_info($head, \\@refs);\n+\t\t\t\tmy ($linear_refs_, $parents_) =\n+\t\t\t\t              linearize_history($gs_, \\@refs);\n+\t\t\t\tif (scalar(@$linear_refs) !=\n+\t\t\t\t    scalar(@$linear_refs_)) {\n+\t\t\t\t\tfatal \"# of revisions changed \",\n+\t\t\t\t\t  \"\\nbefore:\\n\",\n+\t\t\t\t\t  join(\"\\n\", @$linear_refs),\n+\t\t\t\t\t  \"\\n\\nafter:\\n\",\n+\t\t\t\t\t  join(\"\\n\", @$linear_refs_), \"\\n\",\n+\t\t\t\t\t  'If you are attempting to commit ',\n+\t\t\t\t\t  \"merges, try running:\\n\\t\",\n+\t\t\t\t\t  'git rebase --interactive',\n+\t\t\t\t\t  '--preserve-merges ',\n+\t\t\t\t\t  $gs->refname,\n+\t\t\t\t\t  \"\\nBefore dcommitting\";\n+\t\t\t\t}\n+\t\t\t\tif ($url_ ne $url) {\n+\t\t\t\t\tfatal \"URL mismatch after rebase: \",\n+\t\t\t\t\t      \"$url_ != $url\";\n+\t\t\t\t}\n+\t\t\t\tif ($uuid_ ne $uuid) {\n+\t\t\t\t\tfatal \"uuid mismatch after rebase: \",\n+\t\t\t\t\t      \"$uuid_ != $uuid\";\n+\t\t\t\t}\n+\t\t\t\t# remap parents\n+\t\t\t\tmy (%p, @l, $i);\n+\t\t\t\tfor ($i = 0; $i < scalar @$linear_refs; $i++) {\n+\t\t\t\t\tmy $new = $linear_refs_->[$i] or next;\n+\t\t\t\t\t$p{$new} =\n+\t\t\t\t\t\t$parents->{$linear_refs->[$i]};\n+\t\t\t\t\tpush @l, $new;\n+\t\t\t\t}\n+\t\t\t\t$parents = \\%p;\n+\t\t\t\t$linear_refs = \\@l;\n+\t\t\t}\n \t\t\t$last_rev = $cmt_rev;\n \t\t}\n \t}\ndiff --git a/t/t9106-git-svn-dcommit-clobber-series.sh b/t/t9106-git-svn-dcommit-clobber-series.sh\nnew file mode 100755\nindex 0000000..4216a88\n--- /dev/null\n+++ b/t/t9106-git-svn-dcommit-clobber-series.sh\n@@ -0,0 +1,56 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2007 Eric Wong\n+test_description='git-svn dcommit clobber series'\n+. ./lib-git-svn.sh\n+\n+test_expect_success 'initialize repo' \"\n+\tmkdir import &&\n+\tcd import &&\n+\tawk 'BEGIN { for (i = 1; i < 64; i++) { print i } }' > file\n+\tsvn import -m 'initial' . $svnrepo &&\n+\tcd .. &&\n+\tgit svn init $svnrepo &&\n+\tgit svn fetch &&\n+\ttest -e file\n+\t\"\n+\n+test_expect_success '(supposedly) non-conflicting change from SVN' \"\n+\ttest x\\\"\\`sed -n -e 58p < file\\`\\\" = x58 &&\n+\ttest x\\\"\\`sed -n -e 61p < file\\`\\\" = x61 &&\n+\tsvn co $svnrepo tmp &&\n+\tcd tmp &&\n+\t\tperl -i -p -e 's/^58\\$/5588/' file &&\n+\t\tperl -i -p -e 's/^61\\$/6611/' file &&\n+\t\ttest x\\\"\\`sed -n -e 58p < file\\`\\\" = x5588 &&\n+\t\ttest x\\\"\\`sed -n -e 61p < file\\`\\\" = x6611 &&\n+\t\tsvn commit -m '58 => 5588, 61 => 6611' &&\n+\t\tcd ..\n+\t\"\n+\n+test_expect_success 'unrelated some unrelated changes to git' \"\n+\techo hi > life &&\n+\tgit update-index --add life &&\n+\tgit commit -m hi-life &&\n+\techo bye >> life &&\n+\tgit commit -m bye-life life\n+\t\"\n+\n+test_expect_success 'change file but in unrelated area' \"\n+\ttest x\\\"\\`sed -n -e 4p < file\\`\\\" = x4 &&\n+\ttest x\\\"\\`sed -n -e 7p < file\\`\\\" = x7 &&\n+\tperl -i -p -e 's/^4\\$/4444/' file &&\n+\tperl -i -p -e 's/^7\\$/7777/' file &&\n+\ttest x\\\"\\`sed -n -e 4p < file\\`\\\" = x4444 &&\n+\ttest x\\\"\\`sed -n -e 7p < file\\`\\\" = x7777 &&\n+\tgit commit -m '4 => 4444, 7 => 7777' file &&\n+\tgit svn dcommit &&\n+\tsvn up tmp &&\n+\tcd tmp &&\n+\t\ttest x\\\"\\`sed -n -e 4p < file\\`\\\" = x4444 &&\n+\t\ttest x\\\"\\`sed -n -e 7p < file\\`\\\" = x7777 &&\n+\t\ttest x\\\"\\`sed -n -e 58p < file\\`\\\" = x5588 &&\n+\t\ttest x\\\"\\`sed -n -e 61p < file\\`\\\" = x6611\n+\t\"\n+\n+test_done\n-- \n1.5.3.5.566.gc207\n"},{"id":"58361","messageId":"1194261708-32256-2-git-send-email-normalperson@yhbt.net","threadId":"10667","inReplyTo":"1194261708-32256-1-git-send-email-normalperson@yhbt.net","subject":"[PATCH 2/2] git-svn: t9114: verify merge commit message in test","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-11-05T11:21:48Z","receivedAt":"2007-11-05T11:21:48Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"It's possible that we end up with an incorrect commit message\nin this test after making changes to fix the clobber bug\nin dcommit.\n\nSigned-off-by: Eric Wong <normalperson@yhbt.net>\n---\n t/t9114-git-svn-dcommit-merge.sh |    5 +++++\n 1 files changed, 5 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh\nindex d6ca955..225060b 100755\n--- a/t/t9114-git-svn-dcommit-merge.sh\n+++ b/t/t9114-git-svn-dcommit-merge.sh\n@@ -86,4 +86,9 @@ test_expect_success 'verify post-merge ancestry' \"\n \tgit cat-file commit refs/heads/svn^ | grep '^friend$'\n \t\"\n \n+test_expect_success 'verify merge commit message' \"\n+\tgit rev-list --pretty=raw -1 refs/heads/svn | \\\n+\t  grep \\\"    Merge branch 'merge' into svn\\\"\n+\t\"\n+\n test_done\n-- \n1.5.3.5.566.gc207\n"},{"id":"58384","messageId":"20071105142254.GA20277@xp.machine.xx","threadId":"10667","inReplyTo":"1194261708-32256-1-git-send-email-normalperson@yhbt.net","subject":"Re: [PATCH 1/2] git-svn: fix dcommit clobbering when committing a series of diffs","fromName":"Peter Baumann","fromEmail":"waste.manager@gmx.de","sentAt":"2007-11-05T14:22:54Z","receivedAt":"2007-11-05T14:22:54Z","isPatch":true,"sender":{"key":"waste.manager@gmx.de","avatar":null},"body":"On Mon, Nov 05, 2007 at 03:21:47AM -0800, Eric Wong wrote:\n> Our revision number sent to SVN is set to the last revision we\n> committed if we've made any previous commits in a dcommit\n> invocation.\n> \n> Although our SVN Editor code uses the delta of two (old) trees\n> to generate information to send upstream, it'll still send\n> complete resultant files upstream; even if the tree they're\n> based against is out-of-date.\n> \n> The combination of sending a file that does not include the\n> latest changes, but set with a revision number of a commit we\n> just made will cause SVN to accept the resultant file even if it\n> was generated against an old tree.\n> \n> More trouble was caused when fixing this because we were\n> rebasing uncessarily at times.  We used git-diff-tree to check\n> the imported SVN revision against our HEAD, not the last tree we\n> committed to SVN.  The unnecessary rebasing caused merge commits\n> upstream to SVN to fail.\n> \n> Signed-off-by: Eric Wong <normalperson@yhbt.net>\n> ---\n> \n\n[...]\n\n> diff --git a/t/t9106-git-svn-dcommit-clobber-series.sh b/t/t9106-git-svn-dcommit-clobber-series.sh\n> new file mode 100755\n> index 0000000..4216a88\n> --- /dev/null\n> +++ b/t/t9106-git-svn-dcommit-clobber-series.sh\n> @@ -0,0 +1,56 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (c) 2007 Eric Wong\n> +test_description='git-svn dcommit clobber series'\n> +. ./lib-git-svn.sh\n> +\n> +test_expect_success 'initialize repo' \"\n> +\tmkdir import &&\n> +\tcd import &&\n> +\tawk 'BEGIN { for (i = 1; i < 64; i++) { print i } }' > file\n> +\tsvn import -m 'initial' . $svnrepo &&\n> +\tcd .. &&\n> +\tgit svn init $svnrepo &&\n> +\tgit svn fetch &&\n> +\ttest -e file\n> +\t\"\n> +\n> +test_expect_success '(supposedly) non-conflicting change from SVN' \"\n> +\ttest x\\\"\\`sed -n -e 58p < file\\`\\\" = x58 &&\n> +\ttest x\\\"\\`sed -n -e 61p < file\\`\\\" = x61 &&\n> +\tsvn co $svnrepo tmp &&\n> +\tcd tmp &&\n> +\t\tperl -i -p -e 's/^58\\$/5588/' file &&\n> +\t\tperl -i -p -e 's/^61\\$/6611/' file &&\n> +\t\ttest x\\\"\\`sed -n -e 58p < file\\`\\\" = x5588 &&\n> +\t\ttest x\\\"\\`sed -n -e 61p < file\\`\\\" = x6611 &&\n> +\t\tsvn commit -m '58 => 5588, 61 => 6611' &&\n> +\t\tcd ..\n> +\t\"\n> +\n> +test_expect_success 'unrelated some unrelated changes to git' \"\n\nThe first unrelated seems odd here.\n\n-Peter\n\n> +\techo hi > life &&\n> +\tgit update-index --add life &&\n> +\tgit commit -m hi-life &&\n> +\techo bye >> life &&\n> +\tgit commit -m bye-life life\n> +\t\"\n> +\n[...] \n"},{"id":"58389","messageId":"20071105164012.GA29921@hand.yhbt.net","threadId":"10667","inReplyTo":"20071105142254.GA20277@xp.machine.xx","subject":"Re: [PATCH 1/2] git-svn: fix dcommit clobbering when committing a series of diffs","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-11-05T16:40:12Z","receivedAt":"2007-11-05T16:40:12Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Peter Baumann <waste.manager@gmx.de> wrote:\n> On Mon, Nov 05, 2007 at 03:21:47AM -0800, Eric Wong wrote:\n> > +\n> > +test_expect_success 'unrelated some unrelated changes to git' \"\n> \n> The first unrelated seems odd here.\n\nOops, must have been a brain glitch.  Good catch, thanks.\n\n-- \nEric Wong\n"}]}