It's very broken when committing. It's off by default, and somehow my tests didn't catch it. I'll fix it ASAP tonight.
Sorry if anybody else hit it before I did.
-- Eric Wong
threads / discuss / 4572
Subject: git-svn: don't use the --rmdir feature with SVN libs
It's very broken when committing. It's off by default, and somehow my tests didn't catch it. I'll fix it ASAP tonight.
Sorry if anybody else hit it before I did.
-- Eric Wong
Re: git-svn: don't use the --rmdir feature with SVN libs
Fortunately, the fix is pretty simple. But yes, this is the scariest bug in git-svn that's ever happened :x
[PATCH] git-svn: fix --rmdir when using SVN:: libraries
When tracking directories with nearly all of its files at the most nested levels, --rmdir would accidentally go too far when deleting.
Of course, we'll add a test for this condition, too.
Makefile: automatically run new tests as they appear in t/
Signed-off-by: Eric Wong <normalperson@yhbt.net> --- contrib/git-svn/git-svn.perl | 15 +++++++++++---- contrib/git-svn/t/t0002-deep-rmdir.sh | 29 +++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-)
diff --git a/contrib/git-svn/git-svn.perl b/contrib/git-svn/git-svn.perl index da0ff9a..7e7f2f0 100755 --- a/contrib/git-svn/git-svn.perl +++ b/contrib/git-svn/git-svn.perl @@ -2841,13 +2841,20 @@ sub rmdirs { exec qw/git-ls-tree --name-only -r -z/, $self->{c} or croak $!; } local $/ = "\0"; + my @svn_path = split m#/#, $self->{svn_path}; while (<$fh>) { chomp; - $_ = $self->{svn_path} . '/' . $_; - my ($dn) = ($_ =~ m#^(.*?)/?(?:[^/]+)$#); - delete $rm->{$dn}; - last unless %$rm; + my @dn = (@svn_path, (split m#/#, $_)); + while (pop @dn) { + delete $rm->{join '/', @dn}; + } + unless (%$rm) { + close $fh; + return; + } } + close $fh; + my ($r, $p, $bat) = ($self->{r}, $self->{pool}, $self->{bat}); foreach my $d (sort { $b =~ tr#/#/# <=> $a =~ tr#/#/# } keys %$rm) { $self->close_directory($bat->{$d}, $p); diff --git a/contrib/git-svn/t/t0002-deep-rmdir.sh b/contrib/git-svn/t/t0002-deep-rmdir.sh new file mode 100755 index 0000000..d693d18 --- /dev/null +++ b/contrib/git-svn/t/t0002-deep-rmdir.sh @@ -0,0 +1,29 @@ +test_description='git-svn rmdir' +. ./lib-git-svn.sh + +test_expect_success 'initialize repo' " + mkdir import && + cd import && + mkdir -p deeply/nested/directory/number/1 && + mkdir -p deeply/nested/directory/number/2 && + echo foo > deeply/nested/directory/number/1/file && + echo foo > deeply/nested/directory/number/2/another && + svn import -m 'import for git-svn' . $svnrepo && + cd .. + " + +test_expect_success 'mirror via git-svn' " + git-svn init $svnrepo && + git-svn fetch && + git checkout -f -b test-rmdir remotes/git-svn + " + +test_expect_success 'Try a commit on rmdir' " + git rm -f deeply/nested/directory/number/2/another && + git commit -a -m 'remove another' && + git-svn commit --rmdir HEAD && + svn ls -R $svnrepo | grep ^deeply/nested/directory/number/1 + " + + +test_done
-- 1.4.GIT
[PATCH (fixed)] git-svn: fix --rmdir when using SVN:: libraries
When tracking directories with nearly all of its files at the most nested levels, --rmdir would accidentally go too far when deleting.
Of course, we'll add a test for this condition, too.
Makefile: automatically run new tests as they appear in t/
Signed-off-by: Eric Wong <normalperson@yhbt.net> ---
Oops, I left out the Makefile change in the other patch, had the new test file as +x
contrib/git-svn/Makefile | 3 +-- contrib/git-svn/git-svn.perl | 15 +++++++++++---- contrib/git-svn/t/t0002-deep-rmdir.sh | 29 +++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 6 deletions(-)
diff --git a/contrib/git-svn/Makefile b/contrib/git-svn/Makefile index 6aedb10..7c20946 100644 --- a/contrib/git-svn/Makefile +++ b/contrib/git-svn/Makefile @@ -29,8 +29,7 @@ git-svn.html : git-svn.txt asciidoc -b xhtml11 -d manpage \ -f ../../Documentation/asciidoc.conf $< test: git-svn - cd t && $(SHELL) ./t0000-contrib-git-svn.sh $(TEST_FLAGS) - cd t && $(SHELL) ./t0001-contrib-git-svn-props.sh $(TEST_FLAGS) + cd t && for i in t????-*.sh; do $(SHELL) ./$$i $(TEST_FLAGS); done # we can test NO_OPTIMIZE_COMMITS independently of LC_ALL full-test: diff --git a/contrib/git-svn/git-svn.perl b/contrib/git-svn/git-svn.perl index da0ff9a..7e7f2f0 100755 --- a/contrib/git-svn/git-svn.perl +++ b/contrib/git-svn/git-svn.perl @@ -2841,13 +2841,20 @@ sub rmdirs { exec qw/git-ls-tree --name-only -r -z/, $self->{c} or croak $!; } local $/ = "\0"; + my @svn_path = split m#/#, $self->{svn_path}; while (<$fh>) { chomp; - $_ = $self->{svn_path} . '/' . $_; - my ($dn) = ($_ =~ m#^(.*?)/?(?:[^/]+)$#); - delete $rm->{$dn}; - last unless %$rm; + my @dn = (@svn_path, (split m#/#, $_)); + while (pop @dn) { + delete $rm->{join '/', @dn}; + } + unless (%$rm) { + close $fh; + return; + } } + close $fh; + my ($r, $p, $bat) = ($self->{r}, $self->{pool}, $self->{bat}); foreach my $d (sort { $b =~ tr#/#/# <=> $a =~ tr#/#/# } keys %$rm) { $self->close_directory($bat->{$d}, $p); diff --git a/contrib/git-svn/t/t0002-deep-rmdir.sh b/contrib/git-svn/t/t0002-deep-rmdir.sh new file mode 100644 index 0000000..d693d18 --- /dev/null +++ b/contrib/git-svn/t/t0002-deep-rmdir.sh @@ -0,0 +1,29 @@ +test_description='git-svn rmdir' +. ./lib-git-svn.sh + +test_expect_success 'initialize repo' " + mkdir import && + cd import && + mkdir -p deeply/nested/directory/number/1 && + mkdir -p deeply/nested/directory/number/2 && + echo foo > deeply/nested/directory/number/1/file && + echo foo > deeply/nested/directory/number/2/another && + svn import -m 'import for git-svn' . $svnrepo && + cd .. + " + +test_expect_success 'mirror via git-svn' " + git-svn init $svnrepo && + git-svn fetch && + git checkout -f -b test-rmdir remotes/git-svn + " + +test_expect_success 'Try a commit on rmdir' " + git rm -f deeply/nested/directory/number/2/another && + git commit -a -m 'remove another' && + git-svn commit --rmdir HEAD && + svn ls -R $svnrepo | grep ^deeply/nested/directory/number/1 + " + + +test_done
-- 1.4.GIT
Re: [PATCH (fixed)] git-svn: fix --rmdir when using SVN:: libraries
Eric Wong <normalperson@yhbt.net> writes:
> Makefile: automatically run new tests as they appear in t/ > > Signed-off-by: Eric Wong <normalperson@yhbt.net> > --- > > Oops, I left out the Makefile change in the other patch,...
Yeah, I was wondering about that.
> ... had the new > test file as +x
Shouldn't they be executable? It is not a big deal, because you run them via $(SHELL), though...
Re: [PATCH (fixed)] git-svn: fix --rmdir when using SVN:: libraries
Junio C Hamano <junkio@cox.net> wrote:
> Eric Wong <normalperson@yhbt.net> writes: > > > Makefile: automatically run new tests as they appear in t/ > > > > Signed-off-by: Eric Wong <normalperson@yhbt.net> > > --- > > > > Oops, I left out the Makefile change in the other patch,... > > Yeah, I was wondering about that. > > > ... had the new > > test file as +x > > Shouldn't they be executable? It is not a big deal, because you > run them via $(SHELL), though...
Yes, I prefer to use $(SHELL) instead. Makes it easier to test with different shells without mucking /bin/sh
-- Eric Wong