[PATCH 1/5] git-svn: fix occasional Failed to strip path error on fetch next commit, try #3
When --stdlayout and --preserve-empty-dirs flags are used and a directory becomes empty, two things happen: Sometimes find_empty_directories() returns empty list and no empty dir placeholder file created. This happens, because find_empty_directories() marks all directories as non-empty, if at least one updated directory is non-empty. Sometimes git-svn dies with Failed to strip path error. This happens, because find_empty_directories() returns git paths and add_placeholder_file() expects svn paths --- perl/Git/SVN/Fetcher.pm| 12 t/t9160-git-svn-preserve-empty-dirs.sh | 18 +- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm index 046a7a2..4f96076 100644 --- a/perl/Git/SVN/Fetcher.pm +++ b/perl/Git/SVN/Fetcher.pm @@ -129,6 +129,7 @@ sub is_path_ignored { sub set_path_strip { my ($self, $path) = @_; + $self-{pathprefix_strip} = length $path ? ($path . /) : ; $self-{path_strip} = qr/^\Q$path\E(\/|$)/ if length $path; } @@ -458,9 +459,12 @@ sub find_empty_directories { my $skip_added = 0; foreach my $t (qw/dir_prop file_prop/) { foreach my $path (keys %{ $self-{$t} }) { - if (exists $self-{$t}-{dirname($path)}) { - $skip_added = 1; - last; + if (length $self-git_path($path)) { + $path = dirname($path); + if ($dir eq $self-git_path($path) exists $self-{$t}-{$path}) { + $skip_added = 1; + last; + } } } last if $skip_added; @@ -477,7 +481,7 @@ sub find_empty_directories { delete $files{$_} foreach (@deleted_gpath); # Report the directory if there are no filenames left. - push @empty_dirs, $dir unless (scalar %files); + push @empty_dirs, ($self-{pathprefix_strip} . $dir) unless (scalar %files); } @empty_dirs; } diff --git a/t/t9160-git-svn-preserve-empty-dirs.sh b/t/t9160-git-svn-preserve-empty-dirs.sh index b4a4434..1b5a286 100755 --- a/t/t9160-git-svn-preserve-empty-dirs.sh +++ b/t/t9160-git-svn-preserve-empty-dirs.sh @@ -51,13 +51,21 @@ test_expect_success 'initialize source svn repo containing empty dirs' ' echo Conflict file 5/.placeholder mkdir 6/.placeholder svn_cmd add 5/.placeholder 6/.placeholder - svn_cmd commit -m Placeholder Namespace conflict + svn_cmd commit -m Placeholder Namespace conflict + + echo x fil.txt + svn_cmd add fil.txt + svn_cmd commit -m this commit should not kill git-svn ) rm -rf $SVN_TREE ' -test_expect_success 'clone svn repo with --preserve-empty-dirs' ' - git svn clone $svnrepo/trunk --preserve-empty-dirs $GIT_REPO +test_expect_success 'clone svn repo with --preserve-empty-dirs --stdlayout' ' + git svn clone $svnrepo --preserve-empty-dirs --stdlayout $GIT_REPO || ( + cd $GIT_REPO + git svn fetch # fetch the rest can succeed even if clone failed + false # this test still failed + ) ' # $GIT_REPO/1 should only contain the placeholder file. @@ -81,11 +89,11 @@ test_expect_success 'add entry to previously empty directory' ' test -f $GIT_REPO/4/a/b/c/foo ' -# The HEAD~2 commit should not have introduced .gitignore placeholder files. +# The HEAD~3 commit should not have introduced .gitignore placeholder files. test_expect_success 'remove non-last entry from directory' ' ( cd $GIT_REPO - git checkout HEAD~2 + git checkout HEAD~3 ) test_must_fail test -f $GIT_REPO/2/.gitignore test_must_fail test -f $GIT_REPO/3/.gitignore -- 1.8.1.5 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/5] git-svn: fix occasional Failed to strip path error on fetch next commit, try #3
Ilya Basin basini...@gmail.com writes: [PATCH 1/5] git-svn: fix occasional Failed to strip path error on fetch next commit, try #3 Please make it like this. [PATCH v3 1/5] git-svn: fix occasional Failed to strip path error on fetch next commit When --stdlayout and --preserve-empty-dirs flags are used and a directory becomes empty, two things happen: Sometimes find_empty_directories() returns empty list and no empty dir placeholder file created. This happens, because find_empty_directories() marks all directories as non-empty, if at least one updated directory is non-empty. Sometimes git-svn dies with Failed to strip path error. This happens, because find_empty_directories() returns git paths and add_placeholder_file() expects svn paths Enumeration is easier to read if you did ... two things happen: * Thing one. * Thing two. The above is a good description of the problem and your diagnosis, and readers may be able to guess a few approaches to fix them. Here, after the description of the problem and before the three-dash line, is the place to summarize the approach you took to fix it, followed by an empty line followed by your Signed-off-by: line. --- Here is a place to summarize what changed since the earlier iterations of the patch you sent (a single liner e.g. with better log messages, corrected an off-by-one error in function X in the previous round, is often sufficient). perl/Git/SVN/Fetcher.pm| 12 t/t9160-git-svn-preserve-empty-dirs.sh | 18 +- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm index 046a7a2..4f96076 100644 --- a/perl/Git/SVN/Fetcher.pm +++ b/perl/Git/SVN/Fetcher.pm @@ -129,6 +129,7 @@ sub is_path_ignored { sub set_path_strip { my ($self, $path) = @_; + $self-{pathprefix_strip} = length $path ? ($path . /) : ; $self-{path_strip} = qr/^\Q$path\E(\/|$)/ if length $path; The name of the field (should I call it an instance variable?) feels somewhat strange. This is later used to be _added_ as a prefix to the files in the directory denoted by the $path. The only thing this is related to strip is because you have to prefix it because these files have their prefix stripped earlier, no? } @@ -458,9 +459,12 @@ sub find_empty_directories { my $skip_added = 0; foreach my $t (qw/dir_prop file_prop/) { foreach my $path (keys %{ $self-{$t} }) { - if (exists $self-{$t}-{dirname($path)}) { - $skip_added = 1; - last; + if (length $self-git_path($path)) { + $path = dirname($path); + if ($dir eq $self-git_path($path) exists $self-{$t}-{$path}) { + $skip_added = 1; + last; + } I am reading that this is a solution for your second issue (use git_path() to convert $path). An empty $path would be a top-level and skipping it corresponds to the next if $dir eq '.' at the beginning of the loop, I guess. When $dir ne $self-git_path(dirname($path)), what should happen? } } last if $skip_added; @@ -477,7 +481,7 @@ sub find_empty_directories { delete $files{$_} foreach (@deleted_gpath); # Report the directory if there are no filenames left. - push @empty_dirs, $dir unless (scalar %files); + push @empty_dirs, ($self-{pathprefix_strip} . $dir) unless (scalar %files); This makes me think path_prefix would be a better name. } @empty_dirs; } diff --git a/t/t9160-git-svn-preserve-empty-dirs.sh b/t/t9160-git-svn-preserve-empty-dirs.sh index b4a4434..1b5a286 100755 --- a/t/t9160-git-svn-preserve-empty-dirs.sh +++ b/t/t9160-git-svn-preserve-empty-dirs.sh @@ -51,13 +51,21 @@ test_expect_success 'initialize source svn repo containing empty dirs' ' echo Conflict file 5/.placeholder mkdir 6/.placeholder svn_cmd add 5/.placeholder 6/.placeholder - svn_cmd commit -m Placeholder Namespace conflict + svn_cmd commit -m Placeholder Namespace conflict + + echo x fil.txt Not a new problem but we prefer to write this as echo x fil.txt That is, a SP before a redirection operator, but no SP before the redirection target. + svn_cmd add fil.txt + svn_cmd commit -m this commit should not kill git-svn ) rm -rf $SVN_TREE ' -test_expect_success 'clone svn repo with --preserve
Re[2]: [PATCH 1/5] git-svn: fix occasional Failed to strip path error on fetch next commit, try #3
} @@ -458,9 +459,12 @@ sub find_empty_directories { my $skip_added = 0; foreach my $t (qw/dir_prop file_prop/) { foreach my $path (keys %{ $self-{$t} }) { - if (exists $self-{$t}-{dirname($path)}) { - $skip_added = 1; - last; + if (length $self-git_path($path)) { + $path = dirname($path); + if ($dir eq $self-git_path($path) exists $self-{$t}-{$path}) { + $skip_added = 1; + last; + } JCH I am reading that this is a solution for your second issue (use JCH git_path() to convert $path). An empty $path would be a top-level JCH and skipping it corresponds to the next if $dir eq '.' at the JCH beginning of the loop, I guess. JCH When $dir ne $self-git_path(dirname($path)), what should happen? 'ls-tree' will be executed. I guess, the original idea was to save processes, although I don't know why the dir is in @deleted_gpath, if it has children. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html