{"thread":{"id":"15204","subject":"Feature request: git-svn dcommit should send deltas upstream","startedAt":"2008-08-26T10:54:36Z","lastAt":"2008-09-01T08:09:02Z","messageCount":8,"participants":["Florian Weimer","Eric Wong","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"88568","messageId":"87myj0f3mb.fsf@mid.deneb.enyo.de","threadId":"15204","inReplyTo":null,"subject":"Feature request: git-svn dcommit should send deltas upstream","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2008-08-26T10:54:36Z","receivedAt":"2008-08-26T10:54:36Z","isPatch":false,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"Looking at my network traffic and the Perl code, it seems to me that\ngit-svn fails to create a diff (delta) before sending data to the\nserver. As a result, a few changes in a multi-megabyte file lead to a\nlarge upload (similar to the situation with CVS). git-svn should be\nable to compute this diff in all cases because it has got an up-to-date\ncopy of the current revision in the Subversion repository.\n\nAs far as I can tell, this can't be fixed with a one-liner; some handles\nneed to be passed down to the code that actually handles the upload.\n"},{"id":"89101","messageId":"20080829082311.GA7128@yp-box.dyndns.org","threadId":"15204","inReplyTo":"87myj0f3mb.fsf@mid.deneb.enyo.de","subject":"Re: Feature request: git-svn dcommit should send deltas upstream","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2008-08-29T08:23:11Z","receivedAt":"2008-08-29T08:23:11Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Florian Weimer wrote:\n> Looking at my network traffic and the Perl code, it seems to me that\n> git-svn fails to create a diff (delta) before sending data to the\n> server. As a result, a few changes in a multi-megabyte file lead to a\n> large upload (similar to the situation with CVS). git-svn should be\n> able to compute this diff in all cases because it has got an up-to-date\n> copy of the current revision in the Subversion repository.\n> \n> As far as I can tell, this can't be fixed with a one-liner; some handles\n> need to be passed down to the code that actually handles the upload.\n\nOdd. Can you verify that svn(1) does not send full files in this case,\ntoo?\n\nIt's been too long since I've looked at the SVN TxDelta API, but I\nthought SVN::TxDelta::apply would take care of the delta computation for\nus...\n\n-- \nEric Wong\n"},{"id":"89115","messageId":"87tzd4p1lt.fsf@mid.deneb.enyo.de","threadId":"15204","inReplyTo":"20080829082311.GA7128@yp-box.dyndns.org","subject":"Re: Feature request: git-svn dcommit should send deltas upstream","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2008-08-29T10:17:02Z","receivedAt":"2008-08-29T10:17:02Z","isPatch":false,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Eric Wong:\n\n> Florian Weimer wrote:\n>> Looking at my network traffic and the Perl code, it seems to me that\n>> git-svn fails to create a diff (delta) before sending data to the\n>> server. As a result, a few changes in a multi-megabyte file lead to a\n>> large upload (similar to the situation with CVS). git-svn should be\n>> able to compute this diff in all cases because it has got an up-to-date\n>> copy of the current revision in the Subversion repository.\n>> \n>> As far as I can tell, this can't be fixed with a one-liner; some handles\n>> need to be passed down to the code that actually handles the upload.\n>\n> Odd. Can you verify that svn(1) does not send full files in this case,\n> too?\n\nThese two are pcap files of single-line edits to the same file:\n\n-rw-r--r-- 1 root root 1.3M 2008-08-29 11:54 /tmp/git\n-rw-r--r-- 1 root root 40K 2008-08-29 11:53 /tmp/svn\n\n> It's been too long since I've looked at the SVN TxDelta API, but I\n> thought SVN::TxDelta::apply would take care of the delta computation for\n> us...\n\nSVN::Git::Editor::M does not seem to make use of the base text.\n"},{"id":"89320","messageId":"87y72dkvq4.fsf@mid.deneb.enyo.de","threadId":"15204","inReplyTo":"87tzd4p1lt.fsf@mid.deneb.enyo.de","subject":"Re: Feature request: git-svn dcommit should send deltas upstream","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2008-08-31T16:14:27Z","receivedAt":"2008-08-31T16:14:27Z","isPatch":false,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Florian Weimer:\n\n>> It's been too long since I've looked at the SVN TxDelta API, but I\n>> thought SVN::TxDelta::apply would take care of the delta computation for\n>> us...\n>\n> SVN::Git::Editor::M does not seem to make use of the base text.\n\nHere's an attempt at delta generation. I don't know if it is entirely\ncorrect, but it does work in the sense that it passes \"make test\" and\nthe test cases I reported eariler.\n\nSorry for the format. Has anybody got some Emacs code to submit diffs\nusing Gnus?\n\ncommit c94c8b36b41bac20ebf952244675507d2fb927cb\nAuthor: Florian Weimer \nDate: Sun Aug 31 17:05:09 2008 +0200\n\n git-svn: extract base blob in generate_diff\n \n We need the base blob to compute a delta to be sent to the server.\n \n Signed-off-by: Florian Weimer \n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 7a1d26d..0479f41 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -3380,11 +3380,12 @@ sub generate_diff {\n \twhile (<$diff_fh>) {\n \t\tchomp $_; # this gets rid of the trailing \"\\0\"\n \t\tif ($state eq 'meta' && /^:(\\d{6})\\s(\\d{6})\\s\n-\t\t\t\t\t$::sha1\\s($::sha1)\\s\n+\t\t\t\t\t($::sha1)\\s($::sha1)\\s\n \t\t\t\t\t([MTCRAD])\\d*$/xo) {\n \t\t\tpush @mods, {\tmode_a => $1, mode_b => $2,\n-\t\t\t\t\tsha1_b => $3, chg => $4 };\n-\t\t\tif ($4 =~ /^(?:C|R)$/) {\n+\t\t\t\t\tsha1_a => $3, sha1_b => $4,\n+\t\t\t\t\tchg => $5 };\n+\t\t\tif ($5 =~ /^(?:C|R)$/) {\n \t\t\t\t$state = 'file_a';\n \t\t\t} else {\n \t\t\t\t$state = 'file_b';\n\ncommit 020f941b8b606ed3dc2d323194ab11e019380b03\nAuthor: Florian Weimer \nDate: Sun Aug 31 17:45:04 2008 +0200\n\n git-svn: Introduce SVN::Git::Editor::_chg_file_get_blob\n \n Signed-off-by: Florian Weimer \n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 0479f41..2c3e13f 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -3663,28 +3663,35 @@ sub change_file_prop {\n \t$self->SUPER::change_file_prop($fbat, $pname, $pval, $self->{pool});\n }\n \n-sub chg_file {\n-\tmy ($self, $fbat, $m) = @_;\n-\tif ($m->{mode_b} =~ /755$/ && $m->{mode_a} !~ /755$/) {\n-\t\t$self->change_file_prop($fbat,'svn:executable','*');\n-\t} elsif ($m->{mode_b} !~ /755$/ && $m->{mode_a} =~ /755$/) {\n-\t\t$self->change_file_prop($fbat,'svn:executable',undef);\n-\t}\n-\tmy $fh = Git::temp_acquire('git_blob');\n-\tif ($m->{mode_b} =~ /^120/) {\n+sub _chg_file_get_blob ($$$$) {\n+\tmy ($self, $fbat, $m, $which) = @_;\n+\tmy $fh = Git::temp_acquire(\"git_blob_$which\");\n+\tif ($m->{\"mode_$which\"} =~ /^120/) {\n \t\tprint $fh 'link ' or croak $!;\n \t\t$self->change_file_prop($fbat,'svn:special','*');\n-\t} elsif ($m->{mode_a} =~ /^120/ && $m->{mode_b} !~ /^120/) {\n+\t} elsif ($m->{mode_a} =~ /^120/ && $m->{\"mode_$which\"} !~ /^120/) {\n \t\t$self->change_file_prop($fbat,'svn:special',undef);\n \t}\n-\tmy $size = $::_repository->cat_blob($m->{sha1_b}, $fh);\n-\tcroak \"Failed to read object $m->{sha1_b}\" if ($size < 0);\n+\tmy $blob = $m->{\"sha1_$which\"};\n+\treturn ($fh,) if ($blob =~ /^0{40}$/);\n+\tmy $size = $::_repository->cat_blob($blob, $fh);\n+\tcroak \"Failed to read object $blob\" if ($size < 0);\n \t$fh->flush == 0 or croak $!;\n \tseek $fh, 0, 0 or croak $!;\n \n \tmy $exp = ::md5sum($fh);\n \tseek $fh, 0, 0 or croak $!;\n+\treturn ($fh, $exp);\n+}\n \n+sub chg_file {\n+\tmy ($self, $fbat, $m) = @_;\n+\tif ($m->{mode_b} =~ /755$/ && $m->{mode_a} !~ /755$/) {\n+\t\t$self->change_file_prop($fbat,'svn:executable','*');\n+\t} elsif ($m->{mode_b} !~ /755$/ && $m->{mode_a} =~ /755$/) {\n+\t\t$self->change_file_prop($fbat,'svn:executable',undef);\n+\t}\n+\tmy ($fh, $exp) = _chg_file_get_blob $self, $fbat, $m, 'b';\n \tmy $pool = SVN::Pool->new;\n \tmy $atd = $self->apply_textdelta($fbat, undef, $pool);\n \tmy $got = SVN::TxDelta::send_stream($fh, @$atd, $pool);\n\ncommit 846d32cbe8283ffdf4ad5ddb2905dfbc30eace27\nAuthor: Florian Weimer \nDate: Sun Aug 31 17:47:09 2008 +0200\n\n git-svn: Send deltas during commits\n \n Signed-off-by: Florian Weimer \n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 2c3e13f..fdf4e4a 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -3691,12 +3691,20 @@ sub chg_file {\n \t} elsif ($m->{mode_b} !~ /755$/ && $m->{mode_a} =~ /755$/) {\n \t\t$self->change_file_prop($fbat,'svn:executable',undef);\n \t}\n-\tmy ($fh, $exp) = _chg_file_get_blob $self, $fbat, $m, 'b';\n+\tmy ($fh_a, $exp_a) = _chg_file_get_blob $self, $fbat, $m, 'a';\n+\tmy ($fh_b, $exp_b) = _chg_file_get_blob $self, $fbat, $m, 'b';\n \tmy $pool = SVN::Pool->new;\n-\tmy $atd = $self->apply_textdelta($fbat, undef, $pool);\n-\tmy $got = SVN::TxDelta::send_stream($fh, @$atd, $pool);\n-\tdie \"Checksum mismatch\\nexpected: $exp\\ngot: $got\\n\" if ($got ne $exp);\n-\tGit::temp_release($fh, 1);\n+\tmy $atd = $self->apply_textdelta($fbat, $exp_a, $pool);\n+\tif (-s $fh_a) {\n+\t\tmy $txstream = SVN::TxDelta::new ($fh_a, $fh_b, $pool);\n+\t\tSVN::TxDelta::send_txstream($txstream, @$atd, $pool);\n+\t} else {\n+\t\tmy $got = SVN::TxDelta::send_stream($fh_b, @$atd, $pool);\n+\t\tdie \"Checksum mismatch\\nexpected: $exp_b\\ngot: $got\\n\"\n+\t\t if ($got ne $exp_b);\n+\t}\n+\tGit::temp_release($fh_b, 1);\n+\tGit::temp_release($fh_a, 1);\n \t$pool->clear;\n }\n \n"},{"id":"89327","messageId":"7vsksl2k1y.fsf@gitster.siamese.dyndns.org","threadId":"15204","inReplyTo":"87y72dkvq4.fsf@mid.deneb.enyo.de","subject":"Re: Feature request: git-svn dcommit should send deltas upstream","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-31T17:03:53Z","receivedAt":"2008-08-31T17:03:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Florian Weimer writes:\n\n> * Florian Weimer:\n>\n>>> It's been too long since I've looked at the SVN TxDelta API, but I\n>>> thought SVN::TxDelta::apply would take care of the delta computation for\n>>> us...\n>>\n>> SVN::Git::Editor::M does not seem to make use of the base text.\n>\n> Here's an attempt at delta generation. I don't know if it is entirely\n> correct, but it does work in the sense that it passes \"make test\" and\n> the test cases I reported eariler.\n>\n> Sorry for the format. Has anybody got some Emacs code to submit diffs\n> using Gnus?\n\nWhat's wrong with using \"C-x C-i\" in the message mode to read format-patch\noutput in (alternatively \"C-u M-! git format-patch --stdout -1 $commit\"\nin the message mode), move \"Subject: \" up and remove the remainder of the\nfake headers?\n"},{"id":"89328","messageId":"871w05kt0f.fsf@mid.deneb.enyo.de","threadId":"15204","inReplyTo":"7vsksl2k1y.fsf@gitster.siamese.dyndns.org","subject":"Re: Feature request: git-svn dcommit should send deltas upstream","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2008-08-31T17:13:04Z","receivedAt":"2008-08-31T17:13:04Z","isPatch":false,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Junio C. Hamano:\n\n>> Sorry for the format. Has anybody got some Emacs code to submit diffs\n>> using Gnus?\n>\n> What's wrong with using \"C-x C-i\" in the message mode to read format-patch\n> output in (alternatively \"C-u M-! git format-patch --stdout -1 $commit\"\n> in the message mode), move \"Subject: \" up and remove the remainder of the\n> fake headers?\n\nFor single patches, it's okay. Using this approach, proper threading of\na series of patches is rather error-prone, though.\n"},{"id":"89358","messageId":"20080901034127.GA22120@untitled","threadId":"15204","inReplyTo":"87y72dkvq4.fsf@mid.deneb.enyo.de","subject":"Re: Feature request: git-svn dcommit should send deltas upstream","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2008-09-01T03:41:47Z","receivedAt":"2008-09-01T03:41:47Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Florian Weimer wrote:\n> * Florian Weimer:\n> \n> >> It's been too long since I've looked at the SVN TxDelta API, but I\n> >> thought SVN::TxDelta::apply would take care of the delta computation for\n> >> us...\n> >\n> > SVN::Git::Editor::M does not seem to make use of the base text.\n> \n> Here's an attempt at delta generation. I don't know if it is entirely\n> correct, but it does work in the sense that it passes \"make test\" and\n> the test cases I reported eariler.\n\nThanks Florian.\n\nWorks for me, here. I've also tested this editing an audio file and\ntransmitting it over the wire without any trouble. I've added one more\npatch on top of your series to check the (undef) return value of\nsend_txstream.\n\nAcked and pushed out to git://git.bogomips.org/git-svn.git along\nwith Thomas's other patch.\n\n-- \nEric Wong\n"},{"id":"89373","messageId":"87k5dw1e5d.fsf@mid.deneb.enyo.de","threadId":"15204","inReplyTo":"20080901034127.GA22120@untitled","subject":"Re: Feature request: git-svn dcommit should send deltas upstream","fromName":"Florian Weimer","fromEmail":"fw@deneb.enyo.de","sentAt":"2008-09-01T08:09:02Z","receivedAt":"2008-09-01T08:09:02Z","isPatch":false,"sender":{"key":"fw@deneb.enyo.de","avatar":null},"body":"* Eric Wong:\n\n> Works for me, here. I've also tested this editing an audio file and\n> transmitting it over the wire without any trouble. I've added one more\n> patch on top of your series to check the (undef) return value of\n> send_txstream.\n\nI assumed that send_txstream croaks when it encounters an error.\nLooking at the SWIG-generated sources, this seems to be the case.\n"}]}