{"thread":{"id":"43268","subject":"[PATCH] git-svn: fix dcommit losing changes when out-of-date from svn","startedAt":"2006-11-09T00:34:46Z","lastAt":"2006-11-09T22:37:03Z","messageCount":9,"participants":["Eric Wong","Steven Grimm","Seth Falcon","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"297040","messageId":"455277A6.2000404@midwinter.com","threadId":"43268","inReplyTo":null,"subject":"git-svn can lose changes silently","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2006-11-09T00:34:46Z","receivedAt":"2006-11-09T00:34:46Z","isPatch":false,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"git-svn is happy to overwrite changes in the svn repository with no \nwarnings. Didn't seem to be known behavior when I mentioned it in #git, \nso here's an example, starting completely from scratch to make it easier \nto reproduce. I'm using git-svn 1.4.3 and svn 1.2.3 on OS X.\n\nFirst I create a new svn repository and stick a test file there:\n\n% mkdir /tmp/svntest\n% cd /tmp/svntest\n% svnadmin create svn-repo\n% svn co file:///tmp/svntest/svn-repo /tmp/svntest/svn-client\nChecked out revision 0.\n% cd /tmp/svntest/svn-client\n% mkdir -p project/trunk\n% echo \"initial contents\" > project/trunk/testfile\n% svn add project/trunk/testfile\nsvn: 'project/trunk' is not a working copy\n% svn add project\nA         project\nA         project/trunk\nA         project/trunk/testfile\n% svn commit -m \"initial commit\"\nAdding         project\nAdding         project/trunk\nAdding         project/trunk/testfile\nTransmitting file data .\nCommitted revision 1.\n\nNow I clone that svn repository using git-svn and pull in the contents \nof the svn repo:\n\n% cd /tmp/svntest\n% git-svn init file:///tmp/svntest/svn-repo/project/trunk git-repo \n% cd git-repo\n% git-svn fetch\n        A       project/trunk/testfile\nCommitting initial tree 9ca0a5a8cb5ee41744aaf17f859e945f2ebaa7d4\nr1 = 63c70a5e17ffb095a31e96b1a56612f1f8423202\n\nNow I create a development branch and commit a change to the test file:\n\n% git-checkout -b devel\n% echo \"a second line from the git side\" >> testfile\n% git-commit -a -m \"git-side commit\"\n\nNow I go make a change on the svn side and commit it.\n\n% cd /tmp/svntest/svn-client/project/trunk\n% echo \"a second line from the svn side\" >> testfile\n% svn commit -m \"a second svn commit\"\nSending        trunk/testfile\nTransmitting file data .\nCommitted revision 2.\n\nAt this point the svn repository has the testfile with two lines: \n\"initial contents\" and \"a second line from the svn side\". Now, back on \nthe git side, I commit my git-side change to svn (here's where the bug \nhappens):\n\n% cd /tmp/svntest/git-repo\n% git-svn dcommit                     \ndiff-tree 679c0db253781216b9b72b51f2dfffec5711f1a3~1 \n679c0db253781216b9b72b51f2dfffec5711f1a3\n        M       testfile\nCommitted 3\n        M       project/trunk/testfile\nr2 = 7d7923588ffb41eb756959d71623581df9318603\n        M       project/trunk/testfile\nr3 = 25a5fefe01389260274bb2617bc36a2cce18f15d\nNo changes between current HEAD and refs/remotes/git-svn\nHard resetting to the latest refs/remotes/git-svn\n\nFinally, I go back to the svn side and update from the repo:\n\n% cd /tmp/svntest/svn-client\n% svn up\nU    project/trunk/testfile\nUpdated to revision 3.\n% cat project/trunk/testfile\ninitial contents\na second line from the git side\n\nThe change I checked in from the svn side has vanished without a trace, \nno warning messages or anything.\n\nIt is probably not a feature that you can lose changes without knowing \nabout it! Even if I'd run git-svn fetch before that commit, it still \nwouldn't help if the svn version of the file changed between the time I \nran fetch and the time I ran dcommit, totally possible with a busy svn \nrepository.\n\nOpinions? Suggestions on fixing it? Do other people agree this is a bug?\n\n"},{"id":"297109","messageId":"m2u019ysym.fsf@ziti.local","threadId":"43268","inReplyTo":"455277A6.2000404@midwinter.com","subject":"Re: git-svn can lose changes silently","fromName":"Seth Falcon","fromEmail":"sethfalcon@gmail.com","sentAt":"2006-11-09T07:33:53Z","receivedAt":"2006-11-09T07:33:53Z","isPatch":false,"sender":{"key":"sethfalcon@gmail.com","avatar":"https://gravatar.com/avatar/fd62fe73d3013b12fce71d5269ec52eaca3c4cee479efc6eea3603d6f1d8bf8d?d=mp&s=160"},"body":"Steven Grimm <koreth@midwinter.com> writes:\n\n> git-svn is happy to overwrite changes in the svn repository with no\n> warnings. Didn't seem to be known behavior when I mentioned it in\n> #git, so here's an example, starting completely from scratch to make\n> it easier to reproduce. I'm using git-svn 1.4.3 and svn 1.2.3 on OS\n> X.\n\nI can reproduce your example and also see the same thing when\naccessing an svn repository via https [I've never used the local\nfile:// mode and was leary of it].\n\n> It is probably not a feature that you can lose changes without knowing\n> about it! Even if I'd run git-svn fetch before that commit, it still\n> wouldn't help if the svn version of the file changed between the time\n> I ran fetch and the time I ran dcommit, totally possible with a busy\n> svn repository.\n>\n> Opinions? Suggestions on fixing it? Do other people agree this is a\n> bug?\n\nI think this is a pretty big deal -- at least in terms of user\nexpectation.  The man page for 'git-svn commit' warns that upstream\nchanges will be lost.  But in suggesting to use 'git-svn dcommit'\ninstead, one might expect (hope?) this overwritting upstream changes\nto not occur.\n\nI'm embarassed to admit that I've been using git-svn for a few weeks\nnow and didn't test out these details.  \n\nHere is a patch that I think improves matters a bit:\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4a56f18..daa1764 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -830,6 +830,17 @@ sub commit_diff {\n        ($repo, $SVN_PATH) = repo_path_split($SVN_URL);\n        $SVN_LOG ||= libsvn_connect($repo);\n        $SVN ||= libsvn_connect($repo);\n+\n+        my ($r_last, $cmt_last) = svn_grab_base_rev();\n+        my $fetched = fetch();\n+       if ($r_last != $fetched->{revision}) {\n+               print STDERR \"There are new revisions that were fetched \",\n+                               \"and need to be merged (or acknowledged) \",\n+                               \"before committing.\\n\",\n+                               \"last rev: $r_last\\n\",\n+                               \" current: $fetched->{revision}\\n\";\n+               exit 1;\n+       }\n        my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef, 0) : ();\n        my $ed = SVN::Git::Editor->new({        r => $SVN->get_latest_revnum,\n                                                ra => $SVN_LOG, c => $tb,\n\nBut for a possibly busy svn repository, the only real solution is one\nthat uses the svn libs in a way that behaves like the svn client in\nterms of atomicity.\n\n"},{"id":"294187","messageId":"20061109091937.GA22853@localdomain","threadId":"43268","inReplyTo":"455277A6.2000404@midwinter.com","subject":"[PATCH] git-svn: fix dcommit losing changes when out-of-date from svn","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2006-11-09T09:19:37Z","receivedAt":"2006-11-09T09:19:37Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"There was a bug in dcommit (and commit-diff) which caused deltas\nto be generated against the latest version of the changed file\nin a repository, and not the revision we are diffing (the tree)\nagainst locally.\n\nThis bug can cause recent changes to the svn repository to be\nsilently clobbered by git-svn if our repository is out-of-date.\n\nThanks to Steven Grimm for noticing the bug.\n\nThe (few) people using the commit-diff command are now required\nto use the -r/--revision argument.  dcommit usage is unchanged.\n\nSigned-off-by: Eric Wong <normalperson@yhbt.net>\n---\n Documentation/git-svn.txt              |    1 +\n git-svn.perl                           |   29 +++++++++++-\n t/t9105-git-svn-commit-diff.sh         |    2 +-\n t/t9106-git-svn-commit-diff-clobber.sh |   74 ++++++++++++++++++++++++++++++++\n 4 files changed, 102 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-svn.txt b/Documentation/git-svn.txt\nindex 450ff1f..a764d1f 100644\n--- a/Documentation/git-svn.txt\n+++ b/Documentation/git-svn.txt\n@@ -120,6 +120,7 @@ manually joining branches on commit.\n \tURL of the target Subversion repository.  The final argument\n \t(URL) may be omitted if you are working from a git-svn-aware\n \trepository (that has been init-ed with git-svn).\n+\tThe -r<revision> option is required for this.\n \n 'graft-branches'::\n \tThis command attempts to detect merges/branches from already\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 4a56f18..80b7b87 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -134,6 +134,7 @@ my %cmd = (\n \t'commit-diff' => [ \\&commit_diff, 'Commit a diff between two trees',\n \t\t\t{ 'message|m=s' => \\$_message,\n \t\t\t  'file|F=s' => \\$_file,\n+\t\t\t  'revision|r=s' => \\$_revision,\n \t\t\t%cmt_opts } ],\n \tdcommit => [ \\&dcommit, 'Commit several diffs to merge with upstream',\n \t\t\t{ 'merge|m|M' => \\$_merge,\n@@ -586,11 +587,21 @@ sub commit_lib {\n sub dcommit {\n \tmy $gs = \"refs/remotes/$GIT_SVN\";\n \tchomp(my @refs = safe_qx(qw/git-rev-list --no-merges/, \"$gs..HEAD\"));\n+\tmy $last_rev;\n \tforeach my $d (reverse @refs) {\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+\t\t\t\tdie \"Unable to extract revision information \",\n+\t\t\t\t    \"from commit $d~1\\n\";\n+\t\t\t}\n+\t\t}\n \t\tif ($_dry_run) {\n \t\t\tprint \"diff-tree $d~1 $d\\n\";\n \t\t} else {\n-\t\t\tcommit_diff(\"$d~1\", $d);\n+\t\t\tif (my $r = commit_diff(\"$d~1\", $d, undef, $last_rev)) {\n+\t\t\t\t$last_rev = $r;\n+\t\t\t} # else: no changes, same $last_rev\n \t\t}\n \t}\n \treturn if $_dry_run;\n@@ -814,6 +825,8 @@ sub commit_diff {\n \t\tprint STDERR \"Needed URL or usable git-svn id command-line\\n\";\n \t\tcommit_diff_usage();\n \t}\n+\tmy $r = shift || $_revision;\n+\tdie \"-r|--revision is a required argument\\n\" unless (defined $r);\n \tif (defined $_message && defined $_file) {\n \t\tprint STDERR \"Both --message/-m and --file/-F specified \",\n \t\t\t\t\"for the commit message.\\n\",\n@@ -830,13 +843,22 @@ sub commit_diff {\n \t($repo, $SVN_PATH) = repo_path_split($SVN_URL);\n \t$SVN_LOG ||= libsvn_connect($repo);\n \t$SVN ||= libsvn_connect($repo);\n+\tif ($r eq 'HEAD') {\n+\t\t$r = $SVN->get_latest_revnum;\n+\t} elsif ($r !~ /^\\d+$/) {\n+\t\tdie \"revision argument: $r not understood by git-svn\\n\";\n+\t}\n \tmy @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef, 0) : ();\n-\tmy $ed = SVN::Git::Editor->new({\tr => $SVN->get_latest_revnum,\n+\tmy $rev_committed;\n+\tmy $ed = SVN::Git::Editor->new({\tr => $r,\n \t\t\t\t\t\tra => $SVN_LOG, c => $tb,\n \t\t\t\t\t\tsvn_path => $SVN_PATH\n \t\t\t\t\t},\n \t\t\t\t$SVN->get_commit_editor($_message,\n-\t\t\t\t\tsub {print \"Committed $_[0]\\n\"},@lock)\n+\t\t\t\t\tsub {\n+\t\t\t\t\t\t$rev_committed = $_[0];\n+\t\t\t\t\t\tprint \"Committed $_[0]\\n\";\n+\t\t\t\t\t}, @lock)\n \t\t\t\t);\n \tmy $mods = libsvn_checkout_tree($ta, $tb, $ed);\n \tif (@$mods == 0) {\n@@ -846,6 +868,7 @@ sub commit_diff {\n \t\t$ed->close_edit;\n \t}\n \t$_message = $_file = undef;\n+\treturn $rev_committed;\n }\n \n ########################### utility functions #########################\ndiff --git a/t/t9105-git-svn-commit-diff.sh b/t/t9105-git-svn-commit-diff.sh\nindex f994b72..746c827 100755\n--- a/t/t9105-git-svn-commit-diff.sh\n+++ b/t/t9105-git-svn-commit-diff.sh\n@@ -33,7 +33,7 @@ prev=`git rev-parse --verify HEAD^1`\n \n test_expect_success 'test the commit-diff command' \"\n \ttest -n '$prev' && test -n '$head' &&\n-\tgit-svn commit-diff '$prev' '$head' '$svnrepo' &&\n+\tgit-svn commit-diff -r1 '$prev' '$head' '$svnrepo' &&\n \tsvn co $svnrepo wc &&\n \tcmp readme wc/readme\n \t\"\ndiff --git a/t/t9106-git-svn-commit-diff-clobber.sh b/t/t9106-git-svn-commit-diff-clobber.sh\nnew file mode 100755\nindex 0000000..58698b3\n--- /dev/null\n+++ b/t/t9106-git-svn-commit-diff-clobber.sh\n@@ -0,0 +1,74 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2006 Eric Wong\n+test_description='git-svn commit-diff clobber'\n+. ./lib-git-svn.sh\n+\n+if test -n \"$GIT_SVN_NO_LIB\" && test \"$GIT_SVN_NO_LIB\" -ne 0\n+then\n+\techo 'Skipping: commit-diff clobber needs SVN libraries'\n+\ttest_done\n+\texit 0\n+fi\n+\n+test_expect_success 'initialize repo' \"\n+\tmkdir import &&\n+\tcd import &&\n+\techo initial > file &&\n+\tsvn import -m 'initial' . $svnrepo &&\n+\tcd .. &&\n+\techo initial > file &&\n+\tgit update-index --add file &&\n+\tgit commit -a -m 'initial'\n+\t\"\n+test_expect_success 'commit change from svn side' \"\n+\tsvn co $svnrepo t.svn &&\n+\tcd t.svn &&\n+\techo second line from svn >> file &&\n+\tsvn commit -m 'second line from svn' &&\n+\tcd .. &&\n+\trm -rf t.svn\n+\t\"\n+\n+test_expect_failure 'commit conflicting change from git' \"\n+\techo second line from git >> file &&\n+\tgit commit -a -m 'second line from git' &&\n+\tgit-svn commit-diff -r1 HEAD~1 HEAD $svnrepo\n+\t\" || true\n+\n+test_expect_success 'commit complementing change from git' \"\n+\tgit reset --hard HEAD~1 &&\n+\techo second line from svn >> file &&\n+\tgit commit -a -m 'second line from svn' &&\n+\techo third line from git >> file &&\n+\tgit commit -a -m 'third line from git' &&\n+\tgit-svn commit-diff -r2 HEAD~1 HEAD $svnrepo\n+\t\"\n+\n+test_expect_failure 'dcommit fails to commit because of conflict' \"\n+\tgit-svn init $svnrepo &&\n+\tgit-svn fetch &&\n+\tgit reset --hard refs/remotes/git-svn &&\n+\tsvn co $svnrepo t.svn &&\n+\tcd t.svn &&\n+\techo fourth line from svn >> file &&\n+\tsvn commit -m 'fourth line from svn' &&\n+\tcd .. &&\n+\trm -rf t.svn &&\n+\techo 'fourth line from git' >> file &&\n+\tgit commit -a -m 'fourth line from git' &&\n+\tgit-svn dcommit\n+\t\" || true\n+\n+test_expect_success 'dcommit does the svn equivalent of an index merge' \"\n+\tgit reset --hard refs/remotes/git-svn &&\n+\techo 'index merge' > file2 &&\n+\tgit update-index --add file2 &&\n+\tgit commit -a -m 'index merge' &&\n+\techo 'more changes' >> file2 &&\n+\tgit update-index file2 &&\n+\tgit commit -a -m 'more changes' &&\n+\tgit-svn dcommit\n+\t\"\n+\n+test_done\n-- \n1.4.4.rc1.g40e94\n\n-- \n"},{"id":"298520","messageId":"7vfyctkki5.fsf@assigned-by-dhcp.cox.net","threadId":"43268","inReplyTo":"20061109091937.GA22853@localdomain","subject":"Re: [PATCH] git-svn: fix dcommit losing changes when out-of-date from svn","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-09T10:00:18Z","receivedAt":"2006-11-09T10:00:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong <normalperson@yhbt.net> writes:\n\n> There was a bug in dcommit (and commit-diff) which caused deltas\n> to be generated against the latest version of the changed file\n> in a repository, and not the revision we are diffing (the tree)\n> against locally.\n>\n> This bug can cause recent changes to the svn repository to be\n> silently clobbered by git-svn if our repository is out-of-date.\n>\n> Thanks to Steven Grimm for noticing the bug.\n>\n> The (few) people using the commit-diff command are now required\n> to use the -r/--revision argument.  dcommit usage is unchanged.\n>\n> Signed-off-by: Eric Wong <normalperson@yhbt.net>\n\nThanks both for your clear problem report and quick resolution\nof the issue.\n\nSteven, I do not interact with real svn repository myself so I\ncan only judge from the test in this patch and Steven's test\ncase, so it would be more assuring for me if you can confirm it\nfixes the issue for you.\n"},{"id":"298312","messageId":"m2r6wcy0t0.fsf@ziti.local","threadId":"43268","inReplyTo":"7vfyctkki5.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-svn: fix dcommit losing changes when out-of-date from svn","fromName":"Seth Falcon","fromEmail":"sethfalcon@gmail.com","sentAt":"2006-11-09T17:42:03Z","receivedAt":"2006-11-09T17:42:03Z","isPatch":true,"sender":{"key":"sethfalcon@gmail.com","avatar":"https://gravatar.com/avatar/fd62fe73d3013b12fce71d5269ec52eaca3c4cee479efc6eea3603d6f1d8bf8d?d=mp&s=160"},"body":"Junio C Hamano <junkio@cox.net> writes:\n> Eric Wong <normalperson@yhbt.net> writes:\n>\n>> There was a bug in dcommit (and commit-diff) which caused deltas\n>> to be generated against the latest version of the changed file\n>> in a repository, and not the revision we are diffing (the tree)\n>> against locally.\n>>\n>> This bug can cause recent changes to the svn repository to be\n>> silently clobbered by git-svn if our repository is out-of-date.\n\nEric, with this patch, is a dcommit operation as safe as a regular svn\ncommit from an svn working copy?  That is, the commit will abort if\nthe svn repository has changes that your git-svn/git repo hasn't yet\nseen?  I'm pretty sure the answer is yes, but I'd like to be sure :-)\n\n> Steven, I do not interact with real svn repository myself so I\n> can only judge from the test in this patch and Steven's test\n> case, so it would be more assuring for me if you can confirm it\n> fixes the issue for you.\n\nI'm not Steven, but I was reproducing the bug and with Eric's patch, I\nget a nice error/abort when I try to dcommit when the svn repos has\nrelevant changes that I have not fetched yet.\n\nBest,\n\n"},{"id":"298734","messageId":"20061109192200.GA3604@localdomain","threadId":"43268","inReplyTo":"m2r6wcy0t0.fsf@ziti.local","subject":"Re: [PATCH] git-svn: fix dcommit losing changes when out-of-date from svn","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2006-11-09T19:22:00Z","receivedAt":"2006-11-09T19:22:00Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Seth Falcon <sethfalcon@gmail.com> wrote:\n> Junio C Hamano <junkio@cox.net> writes:\n> > Eric Wong <normalperson@yhbt.net> writes:\n> >\n> >> There was a bug in dcommit (and commit-diff) which caused deltas\n> >> to be generated against the latest version of the changed file\n> >> in a repository, and not the revision we are diffing (the tree)\n> >> against locally.\n> >>\n> >> This bug can cause recent changes to the svn repository to be\n> >> silently clobbered by git-svn if our repository is out-of-date.\n> \n> Eric, with this patch, is a dcommit operation as safe as a regular svn\n> commit from an svn working copy?  That is, the commit will abort if\n> the svn repository has changes that your git-svn/git repo hasn't yet\n> seen?  I'm pretty sure the answer is yes, but I'd like to be sure :-)\n\nYes, this is as safe as a regular svn commit from a working copy.\nTransactions will abort if there are conflicts in the files being\ncommitted.\n\n-- \n"},{"id":"298253","messageId":"455381C7.8080207@midwinter.com","threadId":"43268","inReplyTo":"7vfyctkki5.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-svn: fix dcommit losing changes when out-of-date from svn","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2006-11-09T19:30:15Z","receivedAt":"2006-11-09T19:30:15Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Steven, I do not interact with real svn repository myself so I\n> can only judge from the test in this patch and Steven's test\n> case, so it would be more assuring for me if you can confirm it\n> fixes the issue for you.\n>   \n\nIt seems to; I can't make the problem happen any more. I am slightly \nconcerned -- but I don't know libsvn well enough to say for sure -- that \nthis doesn't actually *eliminate* the problem, but rather tightens the \nwindow of opportunity down to some very small amount of time. Which is \ncertainly an improvement, of course!\n\nMaybe only Eric can answer this, but from a cursory inspection, it \ndoesn't look like it actually locks the modified files before generating \nthe patch to apply. Is there still a possibility of losing a change that \nhits the svn repository in the middle of git-svn dcommit running? Or \ndoes locking happen implicitly somewhere I'm not seeing? (Again, I \nhaven't combed the code deeply, so it's entirely possible I've missed \nsomething.)\n\n-Steve\n"},{"id":"297422","messageId":"20061109204759.GA8560@localdomain","threadId":"43268","inReplyTo":"455381C7.8080207@midwinter.com","subject":"Re: [PATCH] git-svn: fix dcommit losing changes when out-of-date from svn","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2006-11-09T20:47:59Z","receivedAt":"2006-11-09T20:47:59Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Steven Grimm <koreth@midwinter.com> wrote:\n> Junio C Hamano wrote:\n> >Steven, I do not interact with real svn repository myself so I\n> >can only judge from the test in this patch and Steven's test\n> >case, so it would be more assuring for me if you can confirm it\n> >fixes the issue for you.\n> >  \n> \n> It seems to; I can't make the problem happen any more. I am slightly \n> concerned -- but I don't know libsvn well enough to say for sure -- that \n> this doesn't actually *eliminate* the problem, but rather tightens the \n> window of opportunity down to some very small amount of time. Which is \n> certainly an improvement, of course!\n> \n> Maybe only Eric can answer this, but from a cursory inspection, it \n> doesn't look like it actually locks the modified files before generating \n> the patch to apply. Is there still a possibility of losing a change that \n> hits the svn repository in the middle of git-svn dcommit running? Or \n> does locking happen implicitly somewhere I'm not seeing? (Again, I \n> haven't combed the code deeply, so it's entirely possible I've missed \n> something.)\n\nThe commit runs as a transaction on the server-side, where all the real\nlocking occurs.  SVN supports user-side locking (svn lock), but it is\nonly advisory and can be taken/unlocked by other users\n(svn lock/unlock --force).\n\nWhen SVN::Git::Editor is instantiated in commit_diff() (line 853), the\n'r' parameter passed to it is the revision we'll generate our diffs\nagainst.  Before, we were generating diffs against the latest revision.\n\nWe generate diffs against 'r' in SVN::Git::Editor::M() (line 3339) and\nSVN::Git::Editor::chg_file (line 3383) passing the $fbat baton object\n(which represents the file at revision 'r') around.\n\n-- \n"},{"id":"297796","messageId":"4553AD8F.2050603@midwinter.com","threadId":"43268","inReplyTo":"20061109204759.GA8560@localdomain","subject":"Re: [PATCH] git-svn: fix dcommit losing changes when out-of-date from svn","fromName":"Steven Grimm","fromEmail":"koreth@midwinter.com","sentAt":"2006-11-09T22:37:03Z","receivedAt":"2006-11-09T22:37:03Z","isPatch":true,"sender":{"key":"koreth@midwinter.com","avatar":"https://gravatar.com/avatar/71b4d2e8b62f168bdc9e9205341159e3567003b4f9e2127c617c5fa0a1f5bad2?d=mp&s=160"},"body":"Eric Wong wrote:\n> The commit runs as a transaction on the server-side, where all the real\n> locking occurs.  SVN supports user-side locking (svn lock), but it is\n> only advisory and can be taken/unlocked by other users\n> (svn lock/unlock --force).\n>   \n\nGreat, that sounds convincing to me. Thanks for such a quick turnaround \non this fix!\n\n-Steve\n"}]}