{"thread":{"id":"29581","subject":"[PATCH 2/2] git-svn.perl: fix a false-positive in the \"already exists\" test","startedAt":"2012-02-09T18:55:24Z","lastAt":"2012-02-23T23:17:33Z","messageCount":17,"participants":["Steven Walter","Junio C Hamano","Thomas Rast","Eric Wong"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"184249","messageId":"1328813725-16638-1-git-send-email-stevenrwalter@gmail.com","threadId":"29581","inReplyTo":null,"subject":"[PATCH 1/2] git-svn.perl: perform deletions before anything else","fromName":"Steven Walter","fromEmail":"stevenrwalter@gmail.com","sentAt":"2012-02-09T18:55:24Z","receivedAt":"2012-02-09T18:55:24Z","isPatch":true,"sender":{"key":"stevenrwalter@gmail.com","avatar":"https://avatars.githubusercontent.com/u/79127?v=4"},"body":"If we delete a file and recreate it as a directory in a single commit,\nwe have to tell the server about the deletion first or else we'll get\n\"RA layer request failed: Server sent unexpected return value (405\nMethod Not Allowed) in response to MKCOL request\"\n---\n git-svn.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 570d83d..520b02b 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -5391,7 +5391,7 @@ sub DESTROY {\n sub apply_diff {\n \tmy ($self) = @_;\n \tmy $mods = $self->{mods};\n-\tmy %o = ( D => 1, R => 0, C => -1, A => 3, M => 3, T => 3 );\n+\tmy %o = ( D => -2, R => 0, C => -1, A => 3, M => 3, T => 3 );\n \tforeach my $m (sort { $o{$a->{chg}} <=> $o{$b->{chg}} } @$mods) {\n \t\tmy $f = $m->{chg};\n \t\tif (defined $o{$f}) {\n-- \n1.7.9.4.ge7a0d\n"},{"id":"184248","messageId":"1328813725-16638-2-git-send-email-stevenrwalter@gmail.com","threadId":"29581","inReplyTo":"1328813725-16638-1-git-send-email-stevenrwalter@gmail.com","subject":"[PATCH 2/2] git-svn.perl: fix a false-positive in the \"already exists\" test","fromName":"Steven Walter","fromEmail":"stevenrwalter@gmail.com","sentAt":"2012-02-09T18:55:25Z","receivedAt":"2012-02-09T18:55:25Z","isPatch":true,"sender":{"key":"stevenrwalter@gmail.com","avatar":"https://avatars.githubusercontent.com/u/79127?v=4"},"body":"open_or_add_dir checks to see if the directory already exists or not.\nIf it already exists and is not a directory, then we fail.  However,\nopen_or_add_dir did not previously account for the possibility that the\npath did exist as a file, but is deleted in the current commit.\n\nIn order to prevent this legitimate case from failing, open_or_add_dir\nneeds to know what files are deleted in the current commit.\nUnfortunately that information has to be plumbed through a couple of\nlayers.\n---\n git-svn.perl |   43 ++++++++++++++++++++++++++-----------------\n 1 files changed, 26 insertions(+), 17 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 520b02b..351e9e3 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -5147,7 +5147,7 @@ sub rmdirs {\n }\n \n sub open_or_add_dir {\n-\tmy ($self, $full_path, $baton) = @_;\n+\tmy ($self, $full_path, $baton, $deletions) = @_;\n \tmy $t = $self->{types}->{$full_path};\n \tif (!defined $t) {\n \t\tdie \"$full_path not known in r$self->{r} or we have a bug!\\n\";\n@@ -5156,7 +5156,7 @@ sub open_or_add_dir {\n \t\tno warnings 'once';\n \t\t# SVN::Node::none and SVN::Node::file are used only once,\n \t\t# so we're shutting up Perl's warnings about them.\n-\t\tif ($t == $SVN::Node::none) {\n+\t\tif ($t == $SVN::Node::none || defined($deletions->{$full_path})) {\n \t\t\treturn $self->add_directory($full_path, $baton,\n \t\t\t    undef, -1, $self->{pool});\n \t\t} elsif ($t == $SVN::Node::dir) {\n@@ -5171,17 +5171,18 @@ sub open_or_add_dir {\n }\n \n sub ensure_path {\n-\tmy ($self, $path) = @_;\n+\tmy ($self, $path, $deletions) = @_;\n \tmy $bat = $self->{bat};\n \tmy $repo_path = $self->repo_path($path);\n \treturn $bat->{''} unless (length $repo_path);\n+\n \tmy @p = split m#/+#, $repo_path;\n \tmy $c = shift @p;\n-\t$bat->{$c} ||= $self->open_or_add_dir($c, $bat->{''});\n+\t$bat->{$c} ||= $self->open_or_add_dir($c, $bat->{''}, $deletions);\n \twhile (@p) {\n \t\tmy $c0 = $c;\n \t\t$c .= '/' . shift @p;\n-\t\t$bat->{$c} ||= $self->open_or_add_dir($c, $bat->{$c0});\n+\t\t$bat->{$c} ||= $self->open_or_add_dir($c, $bat->{$c0}, $deletions);\n \t}\n \treturn $bat->{$c};\n }\n@@ -5238,9 +5239,9 @@ sub apply_autoprops {\n }\n \n sub A {\n-\tmy ($self, $m) = @_;\n+\tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n-\tmy $pbat = $self->ensure_path($dir);\n+\tmy $pbat = $self->ensure_path($dir, $deletions);\n \tmy $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,\n \t\t\t\t\tundef, -1);\n \tprint \"\\tA\\t$m->{file_b}\\n\" unless $::_q;\n@@ -5250,9 +5251,9 @@ sub A {\n }\n \n sub C {\n-\tmy ($self, $m) = @_;\n+\tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n-\tmy $pbat = $self->ensure_path($dir);\n+\tmy $pbat = $self->ensure_path($dir, $deletions);\n \tmy $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,\n \t\t\t\t$self->url_path($m->{file_a}), $self->{r});\n \tprint \"\\tC\\t$m->{file_a} => $m->{file_b}\\n\" unless $::_q;\n@@ -5269,9 +5270,9 @@ sub delete_entry {\n }\n \n sub R {\n-\tmy ($self, $m) = @_;\n+\tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n-\tmy $pbat = $self->ensure_path($dir);\n+\tmy $pbat = $self->ensure_path($dir, $deletions);\n \tmy $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,\n \t\t\t\t$self->url_path($m->{file_a}), $self->{r});\n \tprint \"\\tR\\t$m->{file_a} => $m->{file_b}\\n\" unless $::_q;\n@@ -5280,14 +5281,14 @@ sub R {\n \t$self->close_file($fbat,undef,$self->{pool});\n \n \t($dir, $file) = split_path($m->{file_a});\n-\t$pbat = $self->ensure_path($dir);\n+\t$pbat = $self->ensure_path($dir, $deletions);\n \t$self->delete_entry($m->{file_a}, $pbat);\n }\n \n sub M {\n-\tmy ($self, $m) = @_;\n+\tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n-\tmy $pbat = $self->ensure_path($dir);\n+\tmy $pbat = $self->ensure_path($dir, $deletions);\n \tmy $fbat = $self->open_file($self->repo_path($m->{file_b}),\n \t\t\t\t$pbat,$self->{r},$self->{pool});\n \tprint \"\\t$m->{chg}\\t$m->{file_b}\\n\" unless $::_q;\n@@ -5357,9 +5358,9 @@ sub chg_file {\n }\n \n sub D {\n-\tmy ($self, $m) = @_;\n+\tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n-\tmy $pbat = $self->ensure_path($dir);\n+\tmy $pbat = $self->ensure_path($dir, $deletions);\n \tprint \"\\tD\\t$m->{file_b}\\n\" unless $::_q;\n \t$self->delete_entry($m->{file_b}, $pbat);\n }\n@@ -5392,10 +5393,18 @@ sub apply_diff {\n \tmy ($self) = @_;\n \tmy $mods = $self->{mods};\n \tmy %o = ( D => -2, R => 0, C => -1, A => 3, M => 3, T => 3 );\n+\tmy %deletions;\n+\n+\tforeach my $m (@$mods) {\n+\t\tif ($m->{chg} eq \"D\") {\n+\t\t\t$deletions{$m->{file_b}} = 1;\n+\t\t}\n+\t}\n+\n \tforeach my $m (sort { $o{$a->{chg}} <=> $o{$b->{chg}} } @$mods) {\n \t\tmy $f = $m->{chg};\n \t\tif (defined $o{$f}) {\n-\t\t\t$self->$f($m);\n+\t\t\t$self->$f($m, \\%deletions);\n \t\t} else {\n \t\t\tfatal(\"Invalid change type: $f\");\n \t\t}\n-- \n1.7.9.4.ge7a0d\n"},{"id":"184260","messageId":"7vzkcrvkfa.fsf@alter.siamese.dyndns.org","threadId":"29581","inReplyTo":"1328813725-16638-1-git-send-email-stevenrwalter@gmail.com","subject":"Re: [PATCH 1/2] git-svn.perl: perform deletions before anything else","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-09T20:08:57Z","receivedAt":"2012-02-09T20:08:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Walter <stevenrwalter@gmail.com> writes:\n\n> -\tmy %o = ( D => 1, R => 0, C => -1, A => 3, M => 3, T => 3 );\n> +\tmy %o = ( D => -2, R => 0, C => -1, A => 3, M => 3, T => 3 );\n\nI know this code arrangement dates back to cf52b8f (git-svn: fix several\ncorner-case and rare bugs with 'commit', 2006-02-20), but somehow I find\nit extremely hard to follow.  The absolute values do not matter (this is\nonly used to sort the classes of operations), and the fact that A/M/T\nshares the same value does not help a stable sort result (as it is used as\na key to sort {} that is not given any key other than $o{$ab->{chg}} to\ntie-break).  I suspect that writing it this way\n\n\tmy %o = (D => 0, C => 1, R => 2, A => 3, M => 4, T => 5)\n\nor even\n\n\tmy $ord = 0;\n\tmy %o = map { $_ => $ord++ } qw(D C R A M T);\n\nwould make it much easier to follow.\n"},{"id":"184264","messageId":"1328820742-4795-1-git-send-email-stevenrwalter@gmail.com","threadId":"29581","inReplyTo":"7vzkcrvkfa.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] git-svn.perl: perform deletions before anything else","fromName":"Steven Walter","fromEmail":"stevenrwalter@gmail.com","sentAt":"2012-02-09T20:52:21Z","receivedAt":"2012-02-09T20:52:21Z","isPatch":true,"sender":{"key":"stevenrwalter@gmail.com","avatar":"https://avatars.githubusercontent.com/u/79127?v=4"},"body":"> I suspect that writing it this way [...] would make it much easier to\n> follow.\n\nAgreed.  New patch to follow, this time with sign-off.\n"},{"id":"184263","messageId":"1328820742-4795-2-git-send-email-stevenrwalter@gmail.com","threadId":"29581","inReplyTo":"1328820742-4795-1-git-send-email-stevenrwalter@gmail.com","subject":"[PATCH 1/2] git-svn.perl: perform deletions before anything else","fromName":"Steven Walter","fromEmail":"stevenrwalter@gmail.com","sentAt":"2012-02-09T20:52:22Z","receivedAt":"2012-02-09T20:52:22Z","isPatch":true,"sender":{"key":"stevenrwalter@gmail.com","avatar":"https://avatars.githubusercontent.com/u/79127?v=4"},"body":"From: Steven Walter <swalter@lexmark.com>\n\nIf we delete a file and recreate it as a directory in a single commit,\nwe have to tell the server about the deletion first or else we'll get\n\"RA layer request failed: Server sent unexpected return value (405\nMethod Not Allowed) in response to MKCOL request\"\n\nSigned-off-by: Steven Walter <stevenrwalter@gmail.com>\n---\n git-svn.perl |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex eeb83d3..06c9322 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -5374,7 +5374,7 @@ sub DESTROY {\n sub apply_diff {\n \tmy ($self) = @_;\n \tmy $mods = $self->{mods};\n-\tmy %o = ( D => 1, R => 0, C => -1, A => 3, M => 3, T => 3 );\n+\tmy %o = ( D => 0, C => 1, R => 2, A => 3, M => 4, T => 5 );\n \tforeach my $m (sort { $o{$a->{chg}} <=> $o{$b->{chg}} } @$mods) {\n \t\tmy $f = $m->{chg};\n \t\tif (defined $o{$f}) {\n-- \n1.7.5.4\n"},{"id":"184265","messageId":"87bop7rajx.fsf@thomas.inf.ethz.ch","threadId":"29581","inReplyTo":"CAK8d-aJ3wi0e_NPunow-aBnhs1=o5K25r3e-Ha0m1U0ujTv7OA@mail.gmail.com","subject":"Re: [PATCH 1/2] git-svn.perl: perform deletions before anything else","fromName":"Thomas Rast","fromEmail":"trast@inf.ethz.ch","sentAt":"2012-02-09T20:55:46Z","receivedAt":"2012-02-09T20:55:46Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Oops, as Steven noticed I accidentally hit the wrong reply button.  So\nhere's my earlier reply and his answer.\n\nSteven Walter <stevenrwalter@gmail.com> writes:\n\n> On Thu, Feb 9, 2012 at 2:16 PM, Thomas Rast <trast@inf.ethz.ch> wrote:\n>> Steven Walter <stevenrwalter@gmail.com> writes:\n>>\n>>> If we delete a file and recreate it as a directory in a single commit,\n>>> we have to tell the server about the deletion first or else we'll get\n>>> \"RA layer request failed: Server sent unexpected return value (405\n>>> Method Not Allowed) in response to MKCOL request\"\n>> [...]\n>>> -     my %o = ( D => 1, R => 0, C => -1, A => 3, M => 3, T => 3 );\n>>> +     my %o = ( D => -2, R => 0, C => -1, A => 3, M => 3, T => 3 );\n>>\n>> You are making it delete first, but the original code seems to quite\n>> deliberately put deletion after R (rename?).  Are you sure you're not\n>> breaking anything else?\n>\n> No, I'm not 100% sure of that.\n>\n> In fact, looking at cf52b8f063 where this code seems to have started,\n> it lists my case explicitly as one that subversion does not support:\n>\n> \"a file is removed and a directory of the same name of the removed\n> file is created.\"\n>\n> One thing that might make a difference is that the \"file\" that removed\n> was actually a symlink.  So either svn treats symlinks as a special\n> case to that rule, or else the limitation the commit was meant to\n> address is not present on recent versions of svn.  I can run some\n> checks to see if that is the case.\n\n-- \nThomas Rast\ntrast@{inf,student}.ethz.ch\n"},{"id":"184501","messageId":"20120212070353.GA30477@dcvr.yhbt.net","threadId":"29581","inReplyTo":"1328820742-4795-2-git-send-email-stevenrwalter@gmail.com","subject":"Re: [PATCH 1/2] git-svn.perl: perform deletions before anything else","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-02-12T07:03:53Z","receivedAt":"2012-02-12T07:03:53Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Steven Walter <stevenrwalter@gmail.com> wrote:\n> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>\n\nThanks, shall I fixup 2/2 and assume you meant to Sign-off on that, too?\n"},{"id":"184515","messageId":"CAK8d-aKJCBq2xpsz65hA4g8oa_szKaofLpkYB3v3_2dd=BAgiQ@mail.gmail.com","threadId":"29581","inReplyTo":"20120212070353.GA30477@dcvr.yhbt.net","subject":"Re: [PATCH 1/2] git-svn.perl: perform deletions before anything else","fromName":"Steven Walter","fromEmail":"stevenrwalter@gmail.com","sentAt":"2012-02-12T15:35:43Z","receivedAt":"2012-02-12T15:35:43Z","isPatch":true,"sender":{"key":"stevenrwalter@gmail.com","avatar":"https://avatars.githubusercontent.com/u/79127?v=4"},"body":"On Sun, Feb 12, 2012 at 2:03 AM, Eric Wong <normalperson@yhbt.net> wrote:\n> Steven Walter <stevenrwalter@gmail.com> wrote:\n>> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>\n>\n> Thanks, shall I fixup 2/2 and assume you meant to Sign-off on that, too?\n\nYes, thanks\n-- \n-Steven Walter <stevenrwalter@gmail.com>\n"},{"id":"184534","messageId":"20120212234928.GA4513@dcvr.yhbt.net","threadId":"29581","inReplyTo":"CAK8d-aKJCBq2xpsz65hA4g8oa_szKaofLpkYB3v3_2dd=BAgiQ@mail.gmail.com","subject":"Re: [PATCH 1/2] git-svn.perl: perform deletions before anything else","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-02-12T23:49:28Z","receivedAt":"2012-02-12T23:49:28Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Steven Walter <stevenrwalter@gmail.com> wrote:\n> On Sun, Feb 12, 2012 at 2:03 AM, Eric Wong <normalperson@yhbt.net> wrote:\n> > Steven Walter <stevenrwalter@gmail.com> wrote:\n> >> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>\n> >\n> > Thanks, shall I fixup 2/2 and assume you meant to Sign-off on that, too?\n> \n> Yes, thanks\n\nUgh, I got a bunch of test failures on t9100-git-svn-basic.sh with your\nupdated 1/2 and a trivially merged 2/2:\n\nnot ok - 7 detect node change from file to directory #2\nnot ok - 12 new symlink is added to a file that was also just made executable\nnot ok - 13 modify a symlink to become a file\nnot ok - 14 commit with UTF-8 message: locale: en_US.UTF-8\nnot ok - 16 check imported tree checksums expected tree checksums\n\n1/2 alone seems to pass all existing tests.\n\nI would very much appreciate new test cases that can show exactly what's\nfixed by your patches  (esp given the only times I run/use git-svn is\nwhen reviewing patches).  Thanks!.\n"},{"id":"184773","messageId":"CAK8d-a+tdK=Jn6D+X=bJmKTzbESPqd8+S2nJr9_sfdb7MhLN1A@mail.gmail.com","threadId":"29581","inReplyTo":"20120212234928.GA4513@dcvr.yhbt.net","subject":"Re: [PATCH 1/2] git-svn.perl: perform deletions before anything else","fromName":"Steven Walter","fromEmail":"stevenrwalter@gmail.com","sentAt":"2012-02-15T17:47:06Z","receivedAt":"2012-02-15T17:47:06Z","isPatch":true,"sender":{"key":"stevenrwalter@gmail.com","avatar":"https://avatars.githubusercontent.com/u/79127?v=4"},"body":"On Sun, Feb 12, 2012 at 6:49 PM, Eric Wong <normalperson@yhbt.net> wrote:\n> Steven Walter <stevenrwalter@gmail.com> wrote:\n>> On Sun, Feb 12, 2012 at 2:03 AM, Eric Wong <normalperson@yhbt.net> wrote:\n>> > Steven Walter <stevenrwalter@gmail.com> wrote:\n>> >> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>\n>> >\n>> > Thanks, shall I fixup 2/2 and assume you meant to Sign-off on that, too?\n>>\n>> Yes, thanks\n>\n> Ugh, I got a bunch of test failures on t9100-git-svn-basic.sh with your\n> updated 1/2 and a trivially merged 2/2:\n>\n> not ok - 7 detect node change from file to directory #2\n\nI believe that \"test_must_fail\" is incorrect for this case.  \"git svn\nset-tree\" is succeeding, and the git commit is being faithfully\nrecorded into the svn repository.  If svn will allow us to do it, then\nI don't think git-svn should artificially fail in the case.  This is\nusing svn 1.6.17\n\nWhat's the oldest version of svn supported by git-svn?  Perhaps if I\nretry with that version of svn, I would see a failure.  However, if\nlibsvn-perl reports the failure correctly, isn't that good enough\nbehavior?  No need to fail in git-svn before even trying, IMHO.\n\n> not ok - 12 new symlink is added to a file that was also just made executable\n> not ok - 13 modify a symlink to become a file\n> not ok - 14 commit with UTF-8 message: locale: en_US.UTF-8\n> not ok - 16 check imported tree checksums expected tree checksums\n\nThe rest of these problems seem to have been cascading failures\nresulting from the unexpected success of \"git svn set-tree\" in test 7.\n This left the git and svn repositories in a different state.  To get\nthese to pass, I changed later references to \"bar/zzz\" (which is now a\ndirectory) to use \"file\" instead.  I also had to update the expected\nchecksum values for test 16.  Is there a way to validate what the\nchecksums should be, other than to look at it and say, \"yup, the trees\nlook okay?\"\n\n> I would very much appreciate new test cases that can show exactly what's\n> fixed by your patches  (esp given the only times I run/use git-svn is\n> when reviewing patches).  Thanks!.\n\nIn fact test 7 is exactly what I was trying to make work.  The fact\nthat \"git svn set-tree\" now succeeds in that case is proof that my\nchange had the desired effect.  I modified test 7 to verify that\nset-tree succeeds and that bar/zzz and bar/zzz/yyy get created in\n$SVN_TREE.\n\nAssuming you agree with the above analysis, should I squash the test\nchanges into my 2/2, or would you prefer a separate patch?\n-- \n-Steven Walter <stevenrwalter@gmail.com>\n"},{"id":"184946","messageId":"20120219105442.GA11889@dcvr.yhbt.net","threadId":"29581","inReplyTo":"CAK8d-a+tdK=Jn6D+X=bJmKTzbESPqd8+S2nJr9_sfdb7MhLN1A@mail.gmail.com","subject":"Re: [PATCH 1/2] git-svn.perl: perform deletions before anything else","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-02-19T10:54:42Z","receivedAt":"2012-02-19T10:54:42Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Steven Walter <stevenrwalter@gmail.com> wrote:\n> I don't think git-svn should artificially fail in the case.  This is\n> using svn 1.6.17\n\n> What's the oldest version of svn supported by git-svn?  Perhaps if I\n> retry with that version of svn, I would see a failure.  However, if\n> libsvn-perl reports the failure correctly, isn't that good enough\n> behavior?  No need to fail in git-svn before even trying, IMHO.\n\nOriginally (back in 2006/2007), the goal was to support SVN 1.1.x+.\nI'm not sure if I we ever lost support for such old versions.\n\nI use Debian stable for testing patches, and SVN is 1.6.12 there.\nOtherwise, whatever people are willing to support and send\npatches/bugreports for is good.\n\n> Is there a way to validate what the checksums should be, other than to\n> look at it and say, \"yup, the trees look okay?\"\n\nAs far as I remember, that's how I originally wrote the tests.\n\n> Assuming you agree with the above analysis, should I squash the test\n> changes into my 2/2, or would you prefer a separate patch?\n\nYour analysis seems correct.  I always prefer test changes to be\ncombined with corresponding commits to avoid breakage during bisect.\nThanks!\n"},{"id":"184994","messageId":"1329747474-17976-1-git-send-email-stevenrwalter@gmail.com","threadId":"29581","inReplyTo":"20120219105442.GA11889@dcvr.yhbt.net","subject":"[PATCH] git-svn.perl: fix a false-positive in the \"already exists\" test","fromName":"Steven Walter","fromEmail":"stevenrwalter@gmail.com","sentAt":"2012-02-20T14:17:54Z","receivedAt":"2012-02-20T14:17:54Z","isPatch":true,"sender":{"key":"stevenrwalter@gmail.com","avatar":"https://avatars.githubusercontent.com/u/79127?v=4"},"body":"open_or_add_dir checks to see if the directory already exists or not.\nIf it already exists and is not a directory, then we fail.  However,\nopen_or_add_dir did not previously account for the possibility that the\npath did exist as a file, but is deleted in the current commit.\n\nIn order to prevent this legitimate case from failing, open_or_add_dir\nneeds to know what files are deleted in the current commit.\nUnfortunately that information has to be plumbed through a couple of\nlayers.\n\nSigned-off-by: Steven Walter <stevenrwalter@gmail.com>\n---\n git-svn.perl             |   43 ++++++++++++++++++++++++++-----------------\n t/t9100-git-svn-basic.sh |   33 ++++++++++++++++++---------------\n 2 files changed, 44 insertions(+), 32 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 06c9322..c7a961d 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -5130,7 +5130,7 @@ sub rmdirs {\n }\n \n sub open_or_add_dir {\n-\tmy ($self, $full_path, $baton) = @_;\n+\tmy ($self, $full_path, $baton, $deletions) = @_;\n \tmy $t = $self->{types}->{$full_path};\n \tif (!defined $t) {\n \t\tdie \"$full_path not known in r$self->{r} or we have a bug!\\n\";\n@@ -5139,7 +5139,7 @@ sub open_or_add_dir {\n \t\tno warnings 'once';\n \t\t# SVN::Node::none and SVN::Node::file are used only once,\n \t\t# so we're shutting up Perl's warnings about them.\n-\t\tif ($t == $SVN::Node::none) {\n+\t\tif ($t == $SVN::Node::none || defined($deletions->{$full_path})) {\n \t\t\treturn $self->add_directory($full_path, $baton,\n \t\t\t    undef, -1, $self->{pool});\n \t\t} elsif ($t == $SVN::Node::dir) {\n@@ -5154,17 +5154,18 @@ sub open_or_add_dir {\n }\n \n sub ensure_path {\n-\tmy ($self, $path) = @_;\n+\tmy ($self, $path, $deletions) = @_;\n \tmy $bat = $self->{bat};\n \tmy $repo_path = $self->repo_path($path);\n \treturn $bat->{''} unless (length $repo_path);\n+\n \tmy @p = split m#/+#, $repo_path;\n \tmy $c = shift @p;\n-\t$bat->{$c} ||= $self->open_or_add_dir($c, $bat->{''});\n+\t$bat->{$c} ||= $self->open_or_add_dir($c, $bat->{''}, $deletions);\n \twhile (@p) {\n \t\tmy $c0 = $c;\n \t\t$c .= '/' . shift @p;\n-\t\t$bat->{$c} ||= $self->open_or_add_dir($c, $bat->{$c0});\n+\t\t$bat->{$c} ||= $self->open_or_add_dir($c, $bat->{$c0}, $deletions);\n \t}\n \treturn $bat->{$c};\n }\n@@ -5221,9 +5222,9 @@ sub apply_autoprops {\n }\n \n sub A {\n-\tmy ($self, $m) = @_;\n+\tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n-\tmy $pbat = $self->ensure_path($dir);\n+\tmy $pbat = $self->ensure_path($dir, $deletions);\n \tmy $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,\n \t\t\t\t\tundef, -1);\n \tprint \"\\tA\\t$m->{file_b}\\n\" unless $::_q;\n@@ -5233,9 +5234,9 @@ sub A {\n }\n \n sub C {\n-\tmy ($self, $m) = @_;\n+\tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n-\tmy $pbat = $self->ensure_path($dir);\n+\tmy $pbat = $self->ensure_path($dir, $deletions);\n \tmy $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,\n \t\t\t\t$self->url_path($m->{file_a}), $self->{r});\n \tprint \"\\tC\\t$m->{file_a} => $m->{file_b}\\n\" unless $::_q;\n@@ -5252,9 +5253,9 @@ sub delete_entry {\n }\n \n sub R {\n-\tmy ($self, $m) = @_;\n+\tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n-\tmy $pbat = $self->ensure_path($dir);\n+\tmy $pbat = $self->ensure_path($dir, $deletions);\n \tmy $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,\n \t\t\t\t$self->url_path($m->{file_a}), $self->{r});\n \tprint \"\\tR\\t$m->{file_a} => $m->{file_b}\\n\" unless $::_q;\n@@ -5263,14 +5264,14 @@ sub R {\n \t$self->close_file($fbat,undef,$self->{pool});\n \n \t($dir, $file) = split_path($m->{file_a});\n-\t$pbat = $self->ensure_path($dir);\n+\t$pbat = $self->ensure_path($dir, $deletions);\n \t$self->delete_entry($m->{file_a}, $pbat);\n }\n \n sub M {\n-\tmy ($self, $m) = @_;\n+\tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n-\tmy $pbat = $self->ensure_path($dir);\n+\tmy $pbat = $self->ensure_path($dir, $deletions);\n \tmy $fbat = $self->open_file($self->repo_path($m->{file_b}),\n \t\t\t\t$pbat,$self->{r},$self->{pool});\n \tprint \"\\t$m->{chg}\\t$m->{file_b}\\n\" unless $::_q;\n@@ -5340,9 +5341,9 @@ sub chg_file {\n }\n \n sub D {\n-\tmy ($self, $m) = @_;\n+\tmy ($self, $m, $deletions) = @_;\n \tmy ($dir, $file) = split_path($m->{file_b});\n-\tmy $pbat = $self->ensure_path($dir);\n+\tmy $pbat = $self->ensure_path($dir, $deletions);\n \tprint \"\\tD\\t$m->{file_b}\\n\" unless $::_q;\n \t$self->delete_entry($m->{file_b}, $pbat);\n }\n@@ -5375,10 +5376,18 @@ sub apply_diff {\n \tmy ($self) = @_;\n \tmy $mods = $self->{mods};\n \tmy %o = ( D => 0, C => 1, R => 2, A => 3, M => 4, T => 5 );\n+\tmy %deletions;\n+\n+\tforeach my $m (@$mods) {\n+\t\tif ($m->{chg} eq \"D\") {\n+\t\t\t$deletions{$m->{file_b}} = 1;\n+\t\t}\n+\t}\n+\n \tforeach my $m (sort { $o{$a->{chg}} <=> $o{$b->{chg}} } @$mods) {\n \t\tmy $f = $m->{chg};\n \t\tif (defined $o{$f}) {\n-\t\t\t$self->$f($m);\n+\t\t\t$self->$f($m, \\%deletions);\n \t\t} else {\n \t\t\tfatal(\"Invalid change type: $f\");\n \t\t}\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex b041516..4029f84 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -92,9 +92,11 @@ test_expect_success \"$name\" '\n \techo yyy > bar/zzz/yyy &&\n \tgit update-index --add bar/zzz/yyy &&\n \tgit commit -m \"$name\" &&\n-\ttest_must_fail git svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch3' || true\n-\n+\tgit svn set-tree --find-copies-harder --rmdir \\\n+\t\t${remotes_git_svn}..mybranch3 &&\n+\tsvn_cmd up \"$SVN_TREE\" &&\n+\ttest -d \"$SVN_TREE\"/bar/zzz &&\n+\ttest -e \"$SVN_TREE\"/bar/zzz/yyy ' || true\n \n name='detect node change from directory to file #2'\n test_expect_success \"$name\" '\n@@ -134,10 +136,10 @@ test_expect_success \"$name\" '\n \ttest -x \"$SVN_TREE\"/exec.sh'\n \n \n-name='executable file becomes a symlink to bar/zzz (file)'\n+name='executable file becomes a symlink to file'\n test_expect_success \"$name\" '\n \trm exec.sh &&\n-\tln -s bar/zzz exec.sh &&\n+\tln -s file exec.sh &&\n \tgit update-index exec.sh &&\n \tgit commit -m \"$name\" &&\n \tgit svn set-tree --find-copies-harder --rmdir \\\n@@ -148,14 +150,14 @@ test_expect_success \"$name\" '\n name='new symlink is added to a file that was also just made executable'\n \n test_expect_success \"$name\" '\n-\tchmod +x bar/zzz &&\n-\tln -s bar/zzz exec-2.sh &&\n-\tgit update-index --add bar/zzz exec-2.sh &&\n+\tchmod +x file &&\n+\tln -s file exec-2.sh &&\n+\tgit update-index --add file exec-2.sh &&\n \tgit commit -m \"$name\" &&\n \tgit svn set-tree --find-copies-harder --rmdir \\\n \t\t${remotes_git_svn}..mybranch5 &&\n \tsvn_cmd up \"$SVN_TREE\" &&\n-\ttest -x \"$SVN_TREE\"/bar/zzz &&\n+\ttest -x \"$SVN_TREE\"/file &&\n \ttest -h \"$SVN_TREE\"/exec-2.sh'\n \n name='modify a symlink to become a file'\n@@ -195,14 +197,15 @@ name='check imported tree checksums expected tree checksums'\n rm -f expected\n if test_have_prereq UTF8\n then\n-\techo tree bf522353586b1b883488f2bc73dab0d9f774b9a9 > expected\n+\techo tree dc68b14b733e4ec85b04ab6f712340edc5dc936e > expected\n fi\n cat >> expected <<\\EOF\n-tree 83654bb36f019ae4fe77a0171f81075972087624\n-tree 031b8d557afc6fea52894eaebb45bec52f1ba6d1\n-tree 0b094cbff17168f24c302e297f55bfac65eb8bd3\n-tree d667270a1f7b109f5eb3aaea21ede14b56bfdd6e\n-tree 56a30b966619b863674f5978696f4a3594f2fca9\n+tree c3322890dcf74901f32d216f05c5044f670ce632\n+tree d3ccd5035feafd17b030c5732e7808cc49122853\n+tree d03e1630363d4881e68929d532746b20b0986b83\n+tree 149d63cd5878155c846e8c55d7d8487de283f89e\n+tree 312b76e4f64ce14893aeac8591eb3960b065e247\n+tree 149d63cd5878155c846e8c55d7d8487de283f89e\n tree d667270a1f7b109f5eb3aaea21ede14b56bfdd6e\n tree 8f51f74cf0163afc9ad68a4b1537288c4558b5a4\n EOF\n-- \n1.7.3.4\n"},{"id":"185122","messageId":"20120222003317.GA1069@dcvr.yhbt.net","threadId":"29581","inReplyTo":"1329747474-17976-1-git-send-email-stevenrwalter@gmail.com","subject":"Re: [PATCH] git-svn.perl: fix a false-positive in the \"already exists\" test","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-02-22T00:33:17Z","receivedAt":"2012-02-22T00:33:17Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Steven Walter <stevenrwalter@gmail.com> wrote:\n> open_or_add_dir checks to see if the directory already exists or not.\n> If it already exists and is not a directory, then we fail.  However,\n> open_or_add_dir did not previously account for the possibility that the\n> path did exist as a file, but is deleted in the current commit.\n> \n> In order to prevent this legitimate case from failing, open_or_add_dir\n> needs to know what files are deleted in the current commit.\n> Unfortunately that information has to be plumbed through a couple of\n> layers.\n> \n> Signed-off-by: Steven Walter <stevenrwalter@gmail.com>\n\nThanks, will push.\nAcked-by: Eric Wong <normalperson@yhbt.net>\n"},{"id":"185139","messageId":"7vk43feho8.fsf@alter.siamese.dyndns.org","threadId":"29581","inReplyTo":"1329747474-17976-1-git-send-email-stevenrwalter@gmail.com","subject":"Re: [PATCH] git-svn.perl: fix a false-positive in the \"already exists\" test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-22T02:16:39Z","receivedAt":"2012-02-22T02:16:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Walter <stevenrwalter@gmail.com> writes:\n\n> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\n> index b041516..4029f84 100755\n> --- a/t/t9100-git-svn-basic.sh\n> +++ b/t/t9100-git-svn-basic.sh\n> @@ -92,9 +92,11 @@ test_expect_success \"$name\" '\n>  \techo yyy > bar/zzz/yyy &&\n>  \tgit update-index --add bar/zzz/yyy &&\n>  \tgit commit -m \"$name\" &&\n> +\tgit svn set-tree --find-copies-harder --rmdir \\\n> +\t\t${remotes_git_svn}..mybranch3 &&\n> +\tsvn_cmd up \"$SVN_TREE\" &&\n> +\ttest -d \"$SVN_TREE\"/bar/zzz &&\n> +\ttest -e \"$SVN_TREE\"/bar/zzz/yyy ' || true\n\nCare to explain what this \" || true\" is doing here, please?\n"},{"id":"185142","messageId":"CAK8d-aLXs0yMzYMXm7fKytOGDXesUEx7a8PN_Mg9gw6+Q6OTBA@mail.gmail.com","threadId":"29581","inReplyTo":"7vk43feho8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-svn.perl: fix a false-positive in the \"already exists\" test","fromName":"Steven Walter","fromEmail":"stevenrwalter@gmail.com","sentAt":"2012-02-22T02:32:29Z","receivedAt":"2012-02-22T02:32:29Z","isPatch":true,"sender":{"key":"stevenrwalter@gmail.com","avatar":"https://avatars.githubusercontent.com/u/79127?v=4"},"body":"On Tue, Feb 21, 2012 at 9:16 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Steven Walter <stevenrwalter@gmail.com> writes:\n>\n>> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\n>> index b041516..4029f84 100755\n>> --- a/t/t9100-git-svn-basic.sh\n>> +++ b/t/t9100-git-svn-basic.sh\n>> @@ -92,9 +92,11 @@ test_expect_success \"$name\" '\n>>       echo yyy > bar/zzz/yyy &&\n>>       git update-index --add bar/zzz/yyy &&\n>>       git commit -m \"$name\" &&\n>> +     git svn set-tree --find-copies-harder --rmdir \\\n>> +             ${remotes_git_svn}..mybranch3 &&\n>> +     svn_cmd up \"$SVN_TREE\" &&\n>> +     test -d \"$SVN_TREE\"/bar/zzz &&\n>> +     test -e \"$SVN_TREE\"/bar/zzz/yyy ' || true\n>\n> Care to explain what this \" || true\" is doing here, please?\n\nAhh, good catch.  I think the answer is that it shouldn't be there.\nIt was originally there because of the \"test_must_fail\" line, I think\n(at least the other tests that use test_must_fail also have \"||\ntrue\").  The tests all still pass with that \"|| true\" removed.  Do you\nwant to just fix that up, or a new version of the original patch, or a\nfix on top of the original patches?\n-- \n-Steven Walter <stevenrwalter@gmail.com>\n"},{"id":"185147","messageId":"7vmx8bcv4u.fsf@alter.siamese.dyndns.org","threadId":"29581","inReplyTo":"CAK8d-aLXs0yMzYMXm7fKytOGDXesUEx7a8PN_Mg9gw6+Q6OTBA@mail.gmail.com","subject":"Re: [PATCH] git-svn.perl: fix a false-positive in the \"already exists\" test","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-02-22T05:08:49Z","receivedAt":"2012-02-22T05:08:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Steven Walter <stevenrwalter@gmail.com> writes:\n\n>>> +     test -e \"$SVN_TREE\"/bar/zzz/yyy ' || true\n>>\n>> Care to explain what this \" || true\" is doing here, please?\n>\n> Ahh, good catch.  I think the answer is that it shouldn't be there.\n> It was originally there because of the \"test_must_fail\" line, I think\n> (at least the other tests that use test_must_fail also have \"||\n> true\").\n\nOk, that may explain the copy&paste error.\n\nBut I do not think test_must_fail followed by || true makes much sense,\neither.  The purpose of \"test_must_fail\" is to make sure the tested git\ncommand exits with non-zero status in a controlled way (i.e. not crash)\nso if the tested command that is expected to exit with non-zero status\nexited with zero status, the test has detected an *error*.  E.g. if you\nknow that the index and the working tree are different at one point in the\ntest sequence, you would say:\n\n\t... other setup steps ... &&\n\ttest_must_fail git diff --exit-code &&\n        ... and other tests ...\n\nso that failure by \"git diff --exit-code\" to exit with non-zero status\n(i.e. it did not find any difference when it should have) breaks the &&\ncascade.\n\nI just took a quick look at t9100 but I think all \" || true\" can be safely\nremoved.  None of them is associated with test_must_fail in any way.  For\nwhatever reason, these test seem to do\n\n\ttest_expect_success 'label of the test' '\n        \tbody of the test\n\t' || true\n\nfor no good reason.\n\n> Do you want to just fix that up, or a new version of the original patch,\n> or a fix on top of the original patches?\n\nEric queued the patch and then had me pull it as part of his history\nalready, so it is doubly too late to replace it.\n\nCan you apply this patch and re-test?\n\n\n t/t9100-git-svn-basic.sh |   14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 4029f84..749b75e 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -65,7 +65,8 @@ test_expect_success \"$name\" \"\n \tgit update-index --add dir/file/file &&\n \tgit commit -m '$name' &&\n \ttest_must_fail git svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch\" || true\n+\t\t${remotes_git_svn}..mybranch\n+\"\n \n \n name='detect node change from directory to file #1'\n@@ -79,7 +80,8 @@ test_expect_success \"$name\" '\n \tgit update-index --add -- bar &&\n \tgit commit -m \"$name\" &&\n \ttest_must_fail git svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch2' || true\n+\t\t${remotes_git_svn}..mybranch2\n+'\n \n \n name='detect node change from file to directory #2'\n@@ -96,7 +98,8 @@ test_expect_success \"$name\" '\n \t\t${remotes_git_svn}..mybranch3 &&\n \tsvn_cmd up \"$SVN_TREE\" &&\n \ttest -d \"$SVN_TREE\"/bar/zzz &&\n-\ttest -e \"$SVN_TREE\"/bar/zzz/yyy ' || true\n+\ttest -e \"$SVN_TREE\"/bar/zzz/yyy\n+'\n \n name='detect node change from directory to file #2'\n test_expect_success \"$name\" '\n@@ -109,7 +112,8 @@ test_expect_success \"$name\" '\n \tgit update-index --add -- dir &&\n \tgit commit -m \"$name\" &&\n \ttest_must_fail git svn set-tree --find-copies-harder --rmdir \\\n-\t\t${remotes_git_svn}..mybranch4' || true\n+\t\t${remotes_git_svn}..mybranch4\n+'\n \n \n name='remove executable bit from a file'\n@@ -162,7 +166,7 @@ test_expect_success \"$name\" '\n \n name='modify a symlink to become a file'\n test_expect_success \"$name\" '\n-\techo git help > help || true &&\n+\techo git help >help &&\n \trm exec-2.sh &&\n \tcp help exec-2.sh &&\n \tgit update-index exec-2.sh &&\n"},{"id":"185306","messageId":"CAK8d-aJufwFobREQ6R3Oxr=J7hbVtoZ7wvhurb=LQGUFO9tTsw@mail.gmail.com","threadId":"29581","inReplyTo":"7vmx8bcv4u.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] git-svn.perl: fix a false-positive in the \"already exists\" test","fromName":"Steven Walter","fromEmail":"stevenrwalter@gmail.com","sentAt":"2012-02-23T23:17:33Z","receivedAt":"2012-02-23T23:17:33Z","isPatch":true,"sender":{"key":"stevenrwalter@gmail.com","avatar":"https://avatars.githubusercontent.com/u/79127?v=4"},"body":"Signed-Off-By: Steven Walter <stevenrwalter@gmail.com>\n\nOn Wed, Feb 22, 2012 at 12:08 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Steven Walter <stevenrwalter@gmail.com> writes:\n>\n>>>> +     test -e \"$SVN_TREE\"/bar/zzz/yyy ' || true\n>>>\n>>> Care to explain what this \" || true\" is doing here, please?\n>>\n>> Ahh, good catch.  I think the answer is that it shouldn't be there.\n>> It was originally there because of the \"test_must_fail\" line, I think\n>> (at least the other tests that use test_must_fail also have \"||\n>> true\").\n>\n> Ok, that may explain the copy&paste error.\n>\n> But I do not think test_must_fail followed by || true makes much sense,\n> either.  The purpose of \"test_must_fail\" is to make sure the tested git\n> command exits with non-zero status in a controlled way (i.e. not crash)\n> so if the tested command that is expected to exit with non-zero status\n> exited with zero status, the test has detected an *error*.  E.g. if you\n> know that the index and the working tree are different at one point in the\n> test sequence, you would say:\n>\n>        ... other setup steps ... &&\n>        test_must_fail git diff --exit-code &&\n>        ... and other tests ...\n>\n> so that failure by \"git diff --exit-code\" to exit with non-zero status\n> (i.e. it did not find any difference when it should have) breaks the &&\n> cascade.\n>\n> I just took a quick look at t9100 but I think all \" || true\" can be safely\n> removed.  None of them is associated with test_must_fail in any way.  For\n> whatever reason, these test seem to do\n>\n>        test_expect_success 'label of the test' '\n>                body of the test\n>        ' || true\n>\n> for no good reason.\n>\n>> Do you want to just fix that up, or a new version of the original patch,\n>> or a fix on top of the original patches?\n>\n> Eric queued the patch and then had me pull it as part of his history\n> already, so it is doubly too late to replace it.\n>\n> Can you apply this patch and re-test?\n>\n>\n>  t/t9100-git-svn-basic.sh |   14 +++++++++-----\n>  1 file changed, 9 insertions(+), 5 deletions(-)\n>\n> diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\n> index 4029f84..749b75e 100755\n> --- a/t/t9100-git-svn-basic.sh\n> +++ b/t/t9100-git-svn-basic.sh\n> @@ -65,7 +65,8 @@ test_expect_success \"$name\" \"\n>        git update-index --add dir/file/file &&\n>        git commit -m '$name' &&\n>        test_must_fail git svn set-tree --find-copies-harder --rmdir \\\n> -               ${remotes_git_svn}..mybranch\" || true\n> +               ${remotes_git_svn}..mybranch\n> +\"\n>\n>\n>  name='detect node change from directory to file #1'\n> @@ -79,7 +80,8 @@ test_expect_success \"$name\" '\n>        git update-index --add -- bar &&\n>        git commit -m \"$name\" &&\n>        test_must_fail git svn set-tree --find-copies-harder --rmdir \\\n> -               ${remotes_git_svn}..mybranch2' || true\n> +               ${remotes_git_svn}..mybranch2\n> +'\n>\n>\n>  name='detect node change from file to directory #2'\n> @@ -96,7 +98,8 @@ test_expect_success \"$name\" '\n>                ${remotes_git_svn}..mybranch3 &&\n>        svn_cmd up \"$SVN_TREE\" &&\n>        test -d \"$SVN_TREE\"/bar/zzz &&\n> -       test -e \"$SVN_TREE\"/bar/zzz/yyy ' || true\n> +       test -e \"$SVN_TREE\"/bar/zzz/yyy\n> +'\n>\n>  name='detect node change from directory to file #2'\n>  test_expect_success \"$name\" '\n> @@ -109,7 +112,8 @@ test_expect_success \"$name\" '\n>        git update-index --add -- dir &&\n>        git commit -m \"$name\" &&\n>        test_must_fail git svn set-tree --find-copies-harder --rmdir \\\n> -               ${remotes_git_svn}..mybranch4' || true\n> +               ${remotes_git_svn}..mybranch4\n> +'\n>\n>\n>  name='remove executable bit from a file'\n> @@ -162,7 +166,7 @@ test_expect_success \"$name\" '\n>\n>  name='modify a symlink to become a file'\n>  test_expect_success \"$name\" '\n> -       echo git help > help || true &&\n> +       echo git help >help &&\n>        rm exec-2.sh &&\n>        cp help exec-2.sh &&\n>        git update-index exec-2.sh &&\n\n\n\n-- \n-Steven Walter <stevenrwalter@gmail.com>\n"}]}