git-svn.perl: fix a false-positive in the "already exists" test

open_or_add_dir checks to see if the directory already exists or not.
If it already exists and is not a directory, then we fail.  However,
open_or_add_dir did not previously account for the possibility that the
path did exist as a file, but is deleted in the current commit.

In order to prevent this legitimate case from failing, open_or_add_dir
needs to know what files are deleted in the current commit.
Unfortunately that information has to be plumbed through a couple of
layers.

Signed-off-by: Steven Walter <stevenrwalter@gmail.com>
Acked-by: Eric Wong <normalperson@yhbt.net>

diff --git a/git-svn.perl b/git-svn.perl
index 028f1180374020c1eb6e30fd95b430e9324dc03c..4334b95f70fab5c3cb9e446a9d6c418748b95bca 100755 (executable)
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5137,7 +5137,7 @@ sub rmdirs {
 }
 
 sub open_or_add_dir {
 }
 
 sub open_or_add_dir {

-       my ($self, $full_path, $baton) = @_;
+       my ($self, $full_path, $baton, $deletions) = @_;

        my $t = $self->{types}->{$full_path};
        if (!defined $t) {
                die "$full_path not known in r$self->{r} or we have a bug!\n";
        my $t = $self->{types}->{$full_path};
        if (!defined $t) {
                die "$full_path not known in r$self->{r} or we have a bug!\n";


@@ -5146,7 +5146,7 @@ sub open_or_add_dir {
                no warnings 'once';
                # SVN::Node::none and SVN::Node::file are used only once,
                # so we're shutting up Perl's warnings about them.
                no warnings 'once';
                # SVN::Node::none and SVN::Node::file are used only once,
                # so we're shutting up Perl's warnings about them.

-               if ($t == $SVN::Node::none) {
+               if ($t == $SVN::Node::none || defined($deletions->{$full_path})) {

                        return $self->add_directory($full_path, $baton,
                            undef, -1, $self->{pool});
                } elsif ($t == $SVN::Node::dir) {
                        return $self->add_directory($full_path, $baton,
                            undef, -1, $self->{pool});
                } elsif ($t == $SVN::Node::dir) {


@@ -5161,17 +5161,18 @@ sub open_or_add_dir {
 }
 
 sub ensure_path {
 }
 
 sub ensure_path {

-       my ($self, $path) = @_;
+       my ($self, $path, $deletions) = @_;

        my $bat = $self->{bat};
        my $repo_path = $self->repo_path($path);
        return $bat->{''} unless (length $repo_path);
        my $bat = $self->{bat};
        my $repo_path = $self->repo_path($path);
        return $bat->{''} unless (length $repo_path);

+

        my @p = split m#/+#, $repo_path;
        my $c = shift @p;
        my @p = split m#/+#, $repo_path;
        my $c = shift @p;

-       $bat->{$c} ||= $self->open_or_add_dir($c, $bat->{''});
+       $bat->{$c} ||= $self->open_or_add_dir($c, $bat->{''}, $deletions);

        while (@p) {
                my $c0 = $c;
                $c .= '/' . shift @p;
        while (@p) {
                my $c0 = $c;
                $c .= '/' . shift @p;

-               $bat->{$c} ||= $self->open_or_add_dir($c, $bat->{$c0});
+               $bat->{$c} ||= $self->open_or_add_dir($c, $bat->{$c0}, $deletions);

        }
        return $bat->{$c};
 }
        }
        return $bat->{$c};
 }


@@ -5228,9 +5229,9 @@ sub apply_autoprops {
 }
 
 sub A {
 }
 
 sub A {

-       my ($self, $m) = @_;
+       my ($self, $m, $deletions) = @_;

        my ($dir, $file) = split_path($m->{file_b});
        my ($dir, $file) = split_path($m->{file_b});

-       my $pbat = $self->ensure_path($dir);
+       my $pbat = $self->ensure_path($dir, $deletions);

        my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
                                        undef, -1);
        print "\tA\t$m->{file_b}\n" unless $::_q;
        my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
                                        undef, -1);
        print "\tA\t$m->{file_b}\n" unless $::_q;


@@ -5240,9 +5241,9 @@ sub A {
 }
 
 sub C {
 }
 
 sub C {

-       my ($self, $m) = @_;
+       my ($self, $m, $deletions) = @_;

        my ($dir, $file) = split_path($m->{file_b});
        my ($dir, $file) = split_path($m->{file_b});

-       my $pbat = $self->ensure_path($dir);
+       my $pbat = $self->ensure_path($dir, $deletions);

        my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
                                $self->url_path($m->{file_a}), $self->{r});
        print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
        my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
                                $self->url_path($m->{file_a}), $self->{r});
        print "\tC\t$m->{file_a} => $m->{file_b}\n" unless $::_q;


@@ -5259,9 +5260,9 @@ sub delete_entry {
 }
 
 sub R {
 }
 
 sub R {

-       my ($self, $m) = @_;
+       my ($self, $m, $deletions) = @_;

        my ($dir, $file) = split_path($m->{file_b});
        my ($dir, $file) = split_path($m->{file_b});

-       my $pbat = $self->ensure_path($dir);
+       my $pbat = $self->ensure_path($dir, $deletions);

        my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
                                $self->url_path($m->{file_a}), $self->{r});
        print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q;
        my $fbat = $self->add_file($self->repo_path($m->{file_b}), $pbat,
                                $self->url_path($m->{file_a}), $self->{r});
        print "\tR\t$m->{file_a} => $m->{file_b}\n" unless $::_q;


@@ -5270,14 +5271,14 @@ sub R {
        $self->close_file($fbat,undef,$self->{pool});
 
        ($dir, $file) = split_path($m->{file_a});
        $self->close_file($fbat,undef,$self->{pool});
 
        ($dir, $file) = split_path($m->{file_a});

-       $pbat = $self->ensure_path($dir);
+       $pbat = $self->ensure_path($dir, $deletions);

        $self->delete_entry($m->{file_a}, $pbat);
 }
 
 sub M {
        $self->delete_entry($m->{file_a}, $pbat);
 }
 
 sub M {

-       my ($self, $m) = @_;
+       my ($self, $m, $deletions) = @_;

        my ($dir, $file) = split_path($m->{file_b});
        my ($dir, $file) = split_path($m->{file_b});

-       my $pbat = $self->ensure_path($dir);
+       my $pbat = $self->ensure_path($dir, $deletions);

        my $fbat = $self->open_file($self->repo_path($m->{file_b}),
                                $pbat,$self->{r},$self->{pool});
        print "\t$m->{chg}\t$m->{file_b}\n" unless $::_q;
        my $fbat = $self->open_file($self->repo_path($m->{file_b}),
                                $pbat,$self->{r},$self->{pool});
        print "\t$m->{chg}\t$m->{file_b}\n" unless $::_q;


@@ -5347,9 +5348,9 @@ sub chg_file {
 }
 
 sub D {
 }
 
 sub D {

-       my ($self, $m) = @_;
+       my ($self, $m, $deletions) = @_;

        my ($dir, $file) = split_path($m->{file_b});
        my ($dir, $file) = split_path($m->{file_b});

-       my $pbat = $self->ensure_path($dir);
+       my $pbat = $self->ensure_path($dir, $deletions);

        print "\tD\t$m->{file_b}\n" unless $::_q;
        $self->delete_entry($m->{file_b}, $pbat);
 }
        print "\tD\t$m->{file_b}\n" unless $::_q;
        $self->delete_entry($m->{file_b}, $pbat);
 }


@@ -5382,10 +5383,18 @@ sub apply_diff {
        my ($self) = @_;
        my $mods = $self->{mods};
        my %o = ( D => 0, C => 1, R => 2, A => 3, M => 4, T => 5 );
        my ($self) = @_;
        my $mods = $self->{mods};
        my %o = ( D => 0, C => 1, R => 2, A => 3, M => 4, T => 5 );

+       my %deletions;
+
+       foreach my $m (@$mods) {
+               if ($m->{chg} eq "D") {
+                       $deletions{$m->{file_b}} = 1;
+               }
+       }
+

        foreach my $m (sort { $o{$a->{chg}} <=> $o{$b->{chg}} } @$mods) {
                my $f = $m->{chg};
                if (defined $o{$f}) {
        foreach my $m (sort { $o{$a->{chg}} <=> $o{$b->{chg}} } @$mods) {
                my $f = $m->{chg};
                if (defined $o{$f}) {

-                       $self->$f($m);
+                       $self->$f($m, \%deletions);

                } else {
                        fatal("Invalid change type: $f");
                }
                } else {
                        fatal("Invalid change type: $f");
                }

diff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh
index b041516a1d6316dd36657b582c15100c0a7359d0..4029f844c4e2a34e6408468fd4f0f8d1da110b97 100755 (executable)
--- a/t/t9100-git-svn-basic.sh
+++ b/t/t9100-git-svn-basic.sh
@@ -92,9 +92,11 @@ test_expect_success "$name" '
        echo yyy > bar/zzz/yyy &&
        git update-index --add bar/zzz/yyy &&
        git commit -m "$name" &&
        echo yyy > bar/zzz/yyy &&
        git update-index --add bar/zzz/yyy &&
        git commit -m "$name" &&

-       test_must_fail git svn set-tree --find-copies-harder --rmdir \
-               ${remotes_git_svn}..mybranch3' || true
-
+       git svn set-tree --find-copies-harder --rmdir \
+               ${remotes_git_svn}..mybranch3 &&
+       svn_cmd up "$SVN_TREE" &&
+       test -d "$SVN_TREE"/bar/zzz &&
+       test -e "$SVN_TREE"/bar/zzz/yyy ' || true

 
 name='detect node change from directory to file #2'
 test_expect_success "$name" '
 
 name='detect node change from directory to file #2'
 test_expect_success "$name" '


@@ -134,10 +136,10 @@ test_expect_success "$name" '
        test -x "$SVN_TREE"/exec.sh'
 
 
        test -x "$SVN_TREE"/exec.sh'
 
 

-name='executable file becomes a symlink to bar/zzz (file)'
+name='executable file becomes a symlink to file'

 test_expect_success "$name" '
        rm exec.sh &&
 test_expect_success "$name" '
        rm exec.sh &&

-       ln -s bar/zzz exec.sh &&
+       ln -s file exec.sh &&

        git update-index exec.sh &&
        git commit -m "$name" &&
        git svn set-tree --find-copies-harder --rmdir \
        git update-index exec.sh &&
        git commit -m "$name" &&
        git svn set-tree --find-copies-harder --rmdir \


@@ -148,14 +150,14 @@ test_expect_success "$name" '
 name='new symlink is added to a file that was also just made executable'
 
 test_expect_success "$name" '
 name='new symlink is added to a file that was also just made executable'
 
 test_expect_success "$name" '

-       chmod +x bar/zzz &&
-       ln -s bar/zzz exec-2.sh &&
-       git update-index --add bar/zzz exec-2.sh &&
+       chmod +x file &&
+       ln -s file exec-2.sh &&
+       git update-index --add file exec-2.sh &&

        git commit -m "$name" &&
        git svn set-tree --find-copies-harder --rmdir \
                ${remotes_git_svn}..mybranch5 &&
        svn_cmd up "$SVN_TREE" &&
        git commit -m "$name" &&
        git svn set-tree --find-copies-harder --rmdir \
                ${remotes_git_svn}..mybranch5 &&
        svn_cmd up "$SVN_TREE" &&

-       test -x "$SVN_TREE"/bar/zzz &&
+       test -x "$SVN_TREE"/file &&

        test -h "$SVN_TREE"/exec-2.sh'
 
 name='modify a symlink to become a file'
        test -h "$SVN_TREE"/exec-2.sh'
 
 name='modify a symlink to become a file'


@@ -195,14 +197,15 @@ name='check imported tree checksums expected tree checksums'
 rm -f expected
 if test_have_prereq UTF8
 then
 rm -f expected
 if test_have_prereq UTF8
 then

-       echo tree bf522353586b1b883488f2bc73dab0d9f774b9a9 > expected
+       echo tree dc68b14b733e4ec85b04ab6f712340edc5dc936e > expected

 fi
 cat >> expected <<\EOF
 fi
 cat >> expected <<\EOF

-tree 83654bb36f019ae4fe77a0171f81075972087624
-tree 031b8d557afc6fea52894eaebb45bec52f1ba6d1
-tree 0b094cbff17168f24c302e297f55bfac65eb8bd3
-tree d667270a1f7b109f5eb3aaea21ede14b56bfdd6e
-tree 56a30b966619b863674f5978696f4a3594f2fca9
+tree c3322890dcf74901f32d216f05c5044f670ce632
+tree d3ccd5035feafd17b030c5732e7808cc49122853
+tree d03e1630363d4881e68929d532746b20b0986b83
+tree 149d63cd5878155c846e8c55d7d8487de283f89e
+tree 312b76e4f64ce14893aeac8591eb3960b065e247
+tree 149d63cd5878155c846e8c55d7d8487de283f89e

 tree d667270a1f7b109f5eb3aaea21ede14b56bfdd6e
 tree 8f51f74cf0163afc9ad68a4b1537288c4558b5a4
 EOF
 tree d667270a1f7b109f5eb3aaea21ede14b56bfdd6e
 tree 8f51f74cf0163afc9ad68a4b1537288c4558b5a4
 EOF