{"thread":{"id":"33685","subject":"[PATCH 5/5] git-svn: fix empty dir tracking in branches","startedAt":"2013-04-30T17:38:14Z","lastAt":"2013-05-01T03:25:02Z","messageCount":2,"participants":["Ilya Basin","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"216024","messageId":"51800471.6905700a.65c8.00b9@mx.google.com","threadId":"33685","inReplyTo":null,"subject":"[PATCH 5/5] git-svn: fix empty dir tracking in branches","fromName":"Ilya Basin","fromEmail":"basinilya@gmail.com","sentAt":"2013-04-30T17:38:14Z","receivedAt":"2013-04-30T17:38:14Z","isPatch":true,"sender":{"key":"basinilya@gmail.com","avatar":null},"body":"  - When creating a tag or branch from a subdir, a disjoint branch is\n    created. Then git-svn re-imports the commits using this dir as strip\n    path.\n\n    During this re-import the variable %added_placeholder is not up to\n    date. Because the branch is disjoint, this variable should be empty\n    in the beginning, but it's not. Because of that git-svn tries to\n    delete non-existent .gitignore files and dies.\n\n  - When creating a tag or branch from a subdir, the strip path is e.g.\n    \"trunk/module\", but change_dir_prop() can be called with just\n    \"trunk\". This breaks tracking of placeholder files, because it\n    relise on the hash {dir_prop}, filled in change_dir_prop().\n\n  - When creating a normal tag or branch, git-svn creates a normal\n    branch without reimport, but the placeholder files in the new\n    branch are not added to %added_placeholder.\n\nThis patch does 3 things:\n\n  - It makes git-svn store paths in %added_placeholder already\n    translated from \"trunk/subdir/\" to \"tags/subdir_1.0/\" during\n    reimport.\n\n  - When strip path is \"trunk/subdir\", don't add \"trunk\" to {dir_prop}\n    in change_dir_prop().\n\n  - When a normal branch is created, it takes entries in\n    %added_placeholder belonging to the source branch, translates them\n    to target branch and adds them to %added_placeholder.\n---\n perl/Git/SVN.pm                        |  2 +\n perl/Git/SVN/Fetcher.pm                | 72 ++++++++++++++++++++++++++++++----\n t/t9160-git-svn-preserve-empty-dirs.sh | 51 ++++++++++++++++++++++--\n 3 files changed, 114 insertions(+), 11 deletions(-)\n\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex 5273ee8..660921d 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -1143,6 +1143,7 @@ sub find_parent_branch {\n \t\t($r0, $parent) = $gs->find_rev_before($r, 1);\n \t}\n \tif (defined $r0 && defined $parent) {\n+\t\tGit::SVN::Fetcher::_end_reimport($self, $branch_from, $self->path);\n \t\tprint STDERR \"Found branch parent: ($self->{ref_id}) $parent\\n\"\n \t\t             unless $::_q > 1;\n \t\tmy $ed;\n@@ -1395,6 +1396,7 @@ sub other_gs {\n \t\t\tlast if ($url eq $gs->metadata_url);\n \t\t\t$ref_id .= '-';\n \t\t}\n+\t\tGit::SVN::Fetcher::_begin_reimport($self->path);\n \t\tprint STDERR \"Initializing parent: $ref_id\\n\" unless $::_q > 1;\n \t}\n \t$gs\ndiff --git a/perl/Git/SVN/Fetcher.pm b/perl/Git/SVN/Fetcher.pm\nindex a5ad4cd..aaf5d9a 100644\n--- a/perl/Git/SVN/Fetcher.pm\n+++ b/perl/Git/SVN/Fetcher.pm\n@@ -1,6 +1,7 @@\n package Git::SVN::Fetcher;\n use vars qw/@ISA $_ignore_regex $_preserve_empty_dirs $_placeholder_filename\n             $_package_inited\n+            $_reimportpath\n             @deleted_gpath %added_placeholder $repo_id/;\n use strict;\n use warnings;\n@@ -162,13 +163,59 @@ sub git_path {\n \t\trequire Encode;\n \t\tEncode::from_to($path, 'UTF-8', $enc);\n \t}\n-\tif ($self->{path_strip}) {\n-\t\t$path =~ s!$self->{path_strip}!! or\n-\t\t  die \"Failed to strip path '$path' ($self->{path_strip})\\n\";\n+\t_strip_path($path, $self->{path_strip})\n+}\n+\n+sub _strip_path {\n+\tmy ($path, $re_strip) = @_;\n+\tif ($re_strip) {\n+\t\t$path =~ s!$re_strip!! or\n+\t\t  die \"Failed to strip path '$path' ($re_strip)\\n\";\n \t}\n \t$path;\n }\n \n+sub _begin_reimport {\n+\t( $_reimportpath ) = @_;\n+\tundef\n+}\n+\n+sub _end_reimport {\n+\tmy ( $git_svn, $branch_from, $branch_to ) = @_;\n+\t_try_init_package($git_svn);\n+\tif (defined $_reimportpath) {\n+\t\t$_reimportpath = undef;\n+\t} else {\n+\t\tmy $re_strip = qr/^\\Q$branch_from\\E(\\/|$)/ if length $branch_from;\n+\t\tforeach (values %added_placeholder) {\n+\t\t\tmy $path = $_;\n+\t\t\tif ( (!length $branch_from) || $path =~ s!$re_strip!! ) {\n+\t\t\t\t$path = $branch_to . (length $branch_to && length $path ? \"/\" : \"\") . $path;\n+\t\t\t\t$added_placeholder{ dirname($path) } = $path;\n+\t\t\t}\n+\t\t}\n+\t}\n+\tundef\n+}\n+\n+sub svn2ph_path {\n+\tmy ($self, $path) = @_;\n+\tif (defined $_reimportpath && defined $path) {\n+\t\t$path = _strip_path($path, $self->{path_strip});\n+\t\t$path = $_reimportpath . (length $_reimportpath && length $path ? \"/\" : \"\") . $path;\n+\t}\n+\t$path\n+}\n+\n+sub ph2svn_path {\n+\tmy ($self, $path) = @_;\n+\tif (defined $_reimportpath && defined $path) {\n+\t\t$path = _strip_path($path, qr/^\\Q$_reimportpath\\E(\\/|$)/ ) if length $_reimportpath;\n+\t\t$path = $self->{pathprefix_strip} . $path; # if not empty, pathprefix_strip already ends with slash\n+\t}\n+\t$path\n+}\n+\n sub delete_entry {\n \tmy ($self, $path, $rev, $pb) = @_;\n \treturn undef if $self->is_path_ignored($path);\n@@ -197,7 +244,8 @@ sub delete_entry {\n \t\tprint \"\\tD\\t$gpath\\n\" unless $::_q;\n \t}\n \t# Don't add to @deleted_gpath if we're deleting a placeholder file.\n-\tpush @deleted_gpath, $gpath unless $added_placeholder{dirname($path)};\n+\tmy $phkey = $self->svn2ph_path(dirname($path));\n+\tpush @deleted_gpath, $gpath unless $added_placeholder{$phkey};\n \t$self->{empty}->{$path} = 0;\n \tundef;\n }\n@@ -231,10 +279,12 @@ sub add_file {\n \t\tdelete $self->{empty}->{$dir};\n \t\t$mode = '100644';\n \n+\t\t$dir = $self->svn2ph_path($dir);\n \t\tif ($added_placeholder{$dir}) {\n \t\t\t# Remove our placeholder file, if we created one.\n-\t\t\tdelete_entry($self, $added_placeholder{$dir})\n-\t\t\t\tunless $path eq $added_placeholder{$dir};\n+\t\t\tmy $svnph = $self->ph2svn_path($added_placeholder{$dir});\n+\t\t\tdelete_entry($self, $svnph)\n+\t\t\t\tunless $path eq $svnph;\n \t\t\tdelete $added_placeholder{$dir}\n \t\t}\n \t}\n@@ -265,9 +315,11 @@ sub add_directory {\n \tdelete $self->{empty}->{$dir};\n \t$self->{empty}->{$path} = 1;\n \n+\t$dir = $self->svn2ph_path($dir);\n \tif ($added_placeholder{$dir}) {\n \t\t# Remove our placeholder file, if we created one.\n-\t\tdelete_entry($self, $added_placeholder{$dir});\n+\t\tmy $svnph = $self->ph2svn_path($added_placeholder{$dir});\n+\t\tdelete_entry($self, $svnph);\n \t\tdelete $added_placeholder{$dir}\n \t}\n \n@@ -278,6 +330,10 @@ out:\n sub change_dir_prop {\n \tmy ($self, $db, $prop, $value) = @_;\n \treturn undef if $self->is_path_ignored($db->{path});\n+\tif ($self->{path_strip}) {\n+\t\t$db->{path} =~ m!$self->{path_strip}! or\n+\t\t\treturn undef;\n+\t}\n \t$self->{dir_prop}->{$db->{path}} ||= {};\n \t$self->{dir_prop}->{$db->{path}}->{$prop} = $value;\n \tundef;\n@@ -514,6 +570,8 @@ sub add_placeholder_file {\n \tdelete $self->{empty}->{$dir} if exists $self->{empty}->{$dir};\n \n \t# Keep track of any placeholder files we create.\n+\t$dir = $self->svn2ph_path($dir);\n+\t$path = $self->svn2ph_path($path);\n \t$added_placeholder{$dir} = $path;\n }\n \ndiff --git a/t/t9160-git-svn-preserve-empty-dirs.sh b/t/t9160-git-svn-preserve-empty-dirs.sh\nindex ff06a86..4b0ba75 100755\n--- a/t/t9160-git-svn-preserve-empty-dirs.sh\n+++ b/t/t9160-git-svn-preserve-empty-dirs.sh\n@@ -15,7 +15,7 @@ say 'define NO_SVN_TESTS to skip git svn tests'\n GIT_REPO=git-svn-repo\n \n test_expect_success 'initialize source svn repo containing empty dirs' '\n-\tsvn_cmd mkdir -m x \"$svnrepo\"/trunk &&\n+\tsvn_cmd mkdir -m x \"$svnrepo\"/trunk \"$svnrepo\"/tags &&\n \tsvn_cmd co \"$svnrepo\"/trunk \"$SVN_TREE\" &&\n \t(\n \t\tcd \"$SVN_TREE\" &&\n@@ -23,8 +23,6 @@ test_expect_success 'initialize source svn repo containing empty dirs' '\n \t\techo x > module/foo/file.txt &&\n \t\tsvn_cmd add module &&\n \t\tsvn_cmd commit -mx &&\n-\t\tsvn_cmd mv module/foo/file.txt module/bar/file.txt &&\n-\t\tsvn_cmd commit -mx &&\n \t\tmkdir -p 1 2 3/a 3/b 4 5 6 &&\n \t\techo \"First non-empty file\"  > 2/file1.txt &&\n \t\techo \"Second non-empty file\" > 2/file2.txt &&\n@@ -50,12 +48,18 @@ test_expect_success 'initialize source svn repo containing empty dirs' '\n \t\tsvn_cmd del 3/b &&\n \t\tsvn_cmd commit -m \"delete non-last entry in directory\" &&\n \n-\t\tsvn_cmd rm -m\"x\" \"$svnrepo\"/trunk/module &&\n+\t\tsvn_cmd mv module/foo/file.txt module/bar/file.txt &&\n+\t\tsvn_cmd commit -mx &&\n+\t\tsvn_cmd cp \"$svnrepo\"/trunk \"$svnrepo\"/tags/v1.0 -m\"create standard tag\" &&\n+\t\tsvn_cmd cp \"$svnrepo\"/trunk/module \"$svnrepo\"/tags/module_v1.0 -m\"create non-standard tag\" &&\n+\t\tsvn_cmd rm -m\"removed dir should not be recreated\" \"$svnrepo\"/trunk/module &&\n \n \t\tsvn_cmd del 2/file1.txt &&\n \t\tsvn_cmd del 3/a &&\n \t\tsvn_cmd commit -m \"delete last entry in directory\" &&\n \n+\t\tsvn_cmd mkdir \"$svnrepo\"/tags/v1.0/module/foo/baz \"$svnrepo\"/tags/module_v1.0/foo/baz -m\"this commit should remove known .gitignore from tags\" &&\n+\n \t\techo \"Conflict file\" > 5/.placeholder &&\n \t\tmkdir 6/.placeholder &&\n \t\tsvn_cmd add 5/.placeholder 6/.placeholder &&\n@@ -104,6 +108,45 @@ test_expect_success 'remove non-last entry from directory' '\n \ttest_must_fail test -f \"$GIT_REPO\"/3/.gitignore\n '\n \n+branchtests() {\n+\tbranchname=$1\n+\tprefix=$2\n+\n+\ttest_expect_success \"$branchname: \"'existing placeholders are tracked when creating a branch' '\n+\t\t(\n+\t\t\tcd \"$GIT_REPO\" &&\n+\t\t\tgit checkout \"$branchname\"\n+\t\t) &&\n+\t\ttest -f \"$GIT_REPO\"/\"$prefix\"foo/baz/.gitignore &&\n+\t\ttest_must_fail test -f \"$GIT_REPO\"/\"$prefix\"foo/.gitignore &&\n+\t\ttest_must_fail test -f \"$GIT_REPO\"/\"$prefix\"bar/.gitignore\n+\t'\n+\n+\ttest_expect_success \"$branchname: \"'remove last entry from a directory' '\n+\t\t(\n+\t\t\tcd \"$GIT_REPO\" &&\n+\t\t\tgit checkout HEAD~1\n+\t\t) &&\n+\t\ttest -f \"$GIT_REPO\"/\"$prefix\"foo/.gitignore\n+\t'\n+\n+\ttest_expect_success \"$branchname: \"'add entry to previously empty directory' '\n+\t\ttest_must_fail test -f \"$GIT_REPO\"/\"$prefix\"bar/.gitignore\n+\t'\n+\n+\t# Skip 2 commits, one of them is empty commit of tag creation\n+\ttest_expect_success \"$branchname: \"'create empty directory' '\n+\t\t(\n+\t\t\tcd \"$GIT_REPO\" &&\n+\t\t\tgit checkout HEAD~2\n+\t\t) &&\n+\t\ttest -f \"$GIT_REPO\"/\"$prefix\"bar/.gitignore\n+\t'\n+}\n+\n+branchtests \"tags/v1.0\"        \"module/\"\n+branchtests \"tags/module_v1.0\" \"\"\n+\n # After re-cloning the repository with --placeholder-file specified, there\n # should be 5 files named \".placeholder\" in the local Git repo.\n test_expect_success 'clone svn repo with --placeholder-file specified' '\n-- \n1.8.1.5\n"},{"id":"216121","messageId":"CAPig+cT4657Amv2E5yj3K3cv-FYXSSm=-=CE_apXomp4QvSiEw@mail.gmail.com","threadId":"33685","inReplyTo":"51800471.6905700a.65c8.00b9@mx.google.com","subject":"Re: [PATCH 5/5] git-svn: fix empty dir tracking in branches","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-05-01T03:25:02Z","receivedAt":"2013-05-01T03:25:02Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Apr 30, 2013 at 1:38 PM, Ilya Basin <basinilya@gmail.com> wrote:\n>   - When creating a tag or branch from a subdir, a disjoint branch is\n>     created. Then git-svn re-imports the commits using this dir as strip\n>     path.\n>\n>     During this re-import the variable %added_placeholder is not up to\n>     date. Because the branch is disjoint, this variable should be empty\n>     in the beginning, but it's not. Because of that git-svn tries to\n>     delete non-existent .gitignore files and dies.\n>\n>   - When creating a tag or branch from a subdir, the strip path is e.g.\n>     \"trunk/module\", but change_dir_prop() can be called with just\n>     \"trunk\". This breaks tracking of placeholder files, because it\n>     relise on the hash {dir_prop}, filled in change_dir_prop().\n\ns/relise/relies/\n\n>\n>   - When creating a normal tag or branch, git-svn creates a normal\n>     branch without reimport, but the placeholder files in the new\n>     branch are not added to %added_placeholder.\n>\n> This patch does 3 things:\n>\n>   - It makes git-svn store paths in %added_placeholder already\n>     translated from \"trunk/subdir/\" to \"tags/subdir_1.0/\" during\n>     reimport.\n>\n>   - When strip path is \"trunk/subdir\", don't add \"trunk\" to {dir_prop}\n>     in change_dir_prop().\n>\n>   - When a normal branch is created, it takes entries in\n>     %added_placeholder belonging to the source branch, translates them\n>     to target branch and adds them to %added_placeholder.\n"}]}