{"thread":{"id":"31120","subject":"Fix git-svn for SVN 1.7","startedAt":"2012-07-28T09:47:44Z","lastAt":"2012-10-10T22:33:14Z","messageCount":50,"participants":["Michael G. Schwern","Jonathan Nieder","Michael G Schwern","Eric Wong","Junio C Hamano","Robin H. Johnson","Michael J Gruber"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"196006","messageId":"1343468872-72133-1-git-send-email-schwern@pobox.com","threadId":"31120","inReplyTo":null,"subject":"Fix git-svn for SVN 1.7","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-28T09:47:44Z","receivedAt":"2012-07-28T09:47:44Z","isPatch":false,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"This patch series fixes git-svn for SVN 1.7 tested against SVN 1.7.5 and\n1.6.18. Patch 7/8 is where SVN 1.7 starts passing.\n\nThere is one exception. t9100-git-svn-basic.sh fails 11-13. This appears\nto be due to a bug in SVN to do with symlinks. Leave that for somebody\nelse, this is the final submission in the series.\n\nThe work was difficult because the code relies on simple string equalty\nwhen comparing URLs and paths. Turning on canonicalization in one part\nof the code would cause another part to fail if it also did not\ncanonicalize. There's likely still issues.\n\nA better solution would be to have path and URL objects which overload\nthe eq operator and automatically stringify canonicalized and escaped.\n\nThis patch series should be placed on top of the previous which\nmade accessors canonicalize. I'm getting a bit ahead of things\nby submitting it now, but I'd like to get the review process\nrolling.\n"},{"id":"196008","messageId":"1343468872-72133-2-git-send-email-schwern@pobox.com","threadId":"31120","inReplyTo":"1343468872-72133-1-git-send-email-schwern@pobox.com","subject":"[PATCH 1/8] SVN 1.7 will truncate \"not-a%40{0}\" to just \"not-a\".","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-28T09:47:45Z","receivedAt":"2012-07-28T09:47:45Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" \n\nRather than guess what SVN is going to do for each version, make the test use\nthe branch name that was actually created.\n---\n t/t9118-git-svn-funky-branch-names.sh | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t9118-git-svn-funky-branch-names.sh b/t/t9118-git-svn-funky-branch-names.sh\nindex 63fc982..193d3ca 100755\n--- a/t/t9118-git-svn-funky-branch-names.sh\n+++ b/t/t9118-git-svn-funky-branch-names.sh\n@@ -32,6 +32,11 @@ test_expect_success 'setup svnrepo' '\n \tstart_httpd\n \t'\n \n+# SVN 1.7 will truncate \"not-a%40{0]\" to just \"not-a\".\n+# Look at what SVN wound up naming the branch and use that.\n+# Be sure to escape the @ if it shows up.\n+non_reflog=`svn_cmd ls \"$svnrepo/pr ject/branches\" | grep not-a | sed 's/\\///' | sed 's/@/%40/'`\n+\n test_expect_success 'test clone with funky branch names' '\n \tgit svn clone -s \"$svnrepo/pr ject\" project &&\n \t(\n@@ -42,7 +47,7 @@ test_expect_success 'test clone with funky branch names' '\n \t\tgit rev-parse \"refs/remotes/%2Eleading_dot\" &&\n \t\tgit rev-parse \"refs/remotes/trailing_dot%2E\" &&\n \t\tgit rev-parse \"refs/remotes/trailing_dotlock%2Elock\" &&\n-\t\tgit rev-parse \"refs/remotes/not-a%40{0}reflog\"\n+\t\tgit rev-parse \"refs/remotes/$non_reflog\"\n \t)\n \t'\n \n-- \n1.7.11.3\n"},{"id":"196007","messageId":"1343468872-72133-3-git-send-email-schwern@pobox.com","threadId":"31120","inReplyTo":"1343468872-72133-1-git-send-email-schwern@pobox.com","subject":"[PATCH 2/8] Fix typo in test","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-28T09:47:46Z","receivedAt":"2012-07-28T09:47:46Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" \n\nTest to check that the migration got rid of the old style git-svn directory.\nIt wasn't failing, just throwing a message to STDERR.\n---\n t/t9107-git-svn-migrate.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh\nindex 289fc31..cfb4453 100755\n--- a/t/t9107-git-svn-migrate.sh\n+++ b/t/t9107-git-svn-migrate.sh\n@@ -32,7 +32,7 @@ test_expect_success 'initialize old-style (v0) git svn layout' '\n \techo \"$svnrepo\" > \"$GIT_DIR\"/git-svn/info/url &&\n \techo \"$svnrepo\" > \"$GIT_DIR\"/svn/info/url &&\n \tgit svn migrate &&\n-\t! test -d \"$GIT_DIR\"/git svn &&\n+\t! test -d \"$GIT_DIR\"/git-svn &&\n \tgit rev-parse --verify refs/${remotes_git_svn}^0 &&\n \tgit rev-parse --verify refs/remotes/svn^0 &&\n \ttest \"$(git config --get svn-remote.svn.url)\" = \"$svnrepo\" &&\n-- \n1.7.11.3\n"},{"id":"196014","messageId":"1343468872-72133-4-git-send-email-schwern@pobox.com","threadId":"31120","inReplyTo":"1343468872-72133-1-git-send-email-schwern@pobox.com","subject":"[PATCH 3/8] Improve our URL canonicalization to be more like SVN 1.7's.","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-28T09:47:47Z","receivedAt":"2012-07-28T09:47:47Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" \n\nPreviously, our URL canonicalization didn't do much of anything.\nNow it actually escapes and collapses slashes. This is mostly a cut & paste\nof escape_url from git-svn.\n\nThis is closer to how SVN 1.7's canonicalization behaves. Doing it with\n1.6 lets us chase down some problems caused by more effective canonicalization\nwithout having to deal with all the other 1.7 issues on top of that.\n\n* Remote URLs have to be canonicalized otherwise Git::SVN->find_existing_remote\n will think they're different.\n\n* The SVN remote is now written to the git config canonicalized. That\n should be ok. Adjust a test to account for that.\n---\n perl/Git/SVN.pm | 4 ++--\n perl/Git/SVN/Utils.pm | 19 +++++++++++++++++--\n t/Git-SVN/Utils/canonicalize_url.t | 26 ++++++++++++++++++++++++++\n t/t9107-git-svn-migrate.sh | 4 +++-\n 4 files changed, 48 insertions(+), 5 deletions(-)\n create mode 100644 t/Git-SVN/Utils/canonicalize_url.t\n\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex 798f6c4..cb6d83a 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -201,9 +201,9 @@ sub read_all_remotes {\n \t\t} elsif (m!^(.+)\\.usesvmprops=\\s*(.*)\\s*$!) {\n \t\t\t$r->{$1}->{svm} = {};\n \t\t} elsif (m!^(.+)\\.url=\\s*(.*)\\s*$!) {\n-\t\t\t$r->{$1}->{url} = $2;\n+\t\t\t$r->{$1}->{url} = canonicalize_url($2);\n \t\t} elsif (m!^(.+)\\.pushurl=\\s*(.*)\\s*$!) {\n-\t\t\t$r->{$1}->{pushurl} = $2;\n+\t\t\t$r->{$1}->{pushurl} = canonicalize_url($2);\n \t\t} elsif (m!^(.+)\\.ignore-refs=\\s*(.*)\\s*$!) {\n \t\t\t$r->{$1}->{ignore_refs_regex} = $2;\n \t\t} elsif (m!^(.+)\\.(branches|tags)=$svn_refspec$!) {\ndiff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm\nindex 7ae6fac..dab6e4d 100644\n--- a/perl/Git/SVN/Utils.pm\n+++ b/perl/Git/SVN/Utils.pm\n@@ -147,10 +147,25 @@ sub canonicalize_url {\n }\n \n \n+sub _canonicalize_url_path {\n+\tmy ($uri_path) = @_;\n+\n+\tmy @parts;\n+\tforeach my $part (split m{/+}, $uri_path) {\n+\t\t$part =~ s/([^~\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n+\t\tpush @parts, $part;\n+\t}\n+\n+\treturn join('/', @parts);\n+}\n+\n sub _canonicalize_url_ourselves {\n \tmy ($url) = @_;\n-\t$url =~ s#^([^:]+://[^/]*/)(.*)$#$1 . canonicalize_path($2)#e;\n-\treturn $url;\n+\tif ($url =~ m#^([^:]+)://([^/]*)(.*)$#) {\n+\t\tmy ($scheme, $domain, $uri) = ($1, $2, _canonicalize_url_path(canonicalize_path($3)));\n+\t\t$url = \"$scheme://$domain$uri\";\n+\t}\n+\t$url;\n }\n \n \ndiff --git a/t/Git-SVN/Utils/canonicalize_url.t b/t/Git-SVN/Utils/canonicalize_url.t\nnew file mode 100644\nindex 0000000..05795ab\n--- /dev/null\n+++ b/t/Git-SVN/Utils/canonicalize_url.t\n@@ -0,0 +1,26 @@\n+#!/usr/bin/env perl\n+\n+# Test our own home rolled URL canonicalizer. Test the private one\n+# directly because we can't predict what the SVN API is doing to do.\n+\n+use strict;\n+use warnings;\n+\n+use Test::More 'no_plan';\n+\n+use Git::SVN::Utils;\n+my $canonicalize_url = \\&Git::SVN::Utils::_canonicalize_url_ourselves;\n+\n+my %tests = (\n+\t\"http://x.com\"\t\t\t=> \"http://x.com\",\n+\t\"http://x.com/\"\t\t\t=> \"http://x.com\",\n+\t\"http://x.com/foo/bar\"\t\t=> \"http://x.com/foo/bar\",\n+\t\"http://x.com//foo//bar//\"\t=> \"http://x.com/foo/bar\",\n+\t\"http://x.com/ /%/\"\t\t=> \"http://x.com/%20%20/%25\",\n+);\n+\n+for my $arg (keys %tests) {\n+\tmy $want = $tests{$arg};\n+\n+\tis $canonicalize_url->($arg), $want, \"canonicalize_url('$arg') => $want\";\n+}\ndiff --git a/t/t9107-git-svn-migrate.sh b/t/t9107-git-svn-migrate.sh\nindex cfb4453..ee73013 100755\n--- a/t/t9107-git-svn-migrate.sh\n+++ b/t/t9107-git-svn-migrate.sh\n@@ -27,6 +27,8 @@ test_expect_success 'setup old-looking metadata' '\n head=`git rev-parse --verify refs/heads/git-svn-HEAD^0`\n test_expect_success 'git-svn-HEAD is a real HEAD' \"test -n '$head'\"\n \n+svnrepo_escaped=`echo $svnrepo | sed 's/ /%20/'`\n+\n test_expect_success 'initialize old-style (v0) git svn layout' '\n \tmkdir -p \"$GIT_DIR\"/git-svn/info \"$GIT_DIR\"/svn/info &&\n \techo \"$svnrepo\" > \"$GIT_DIR\"/git-svn/info/url &&\n@@ -35,7 +37,7 @@ test_expect_success 'initialize old-style (v0) git svn layout' '\n \t! test -d \"$GIT_DIR\"/git-svn &&\n \tgit rev-parse --verify refs/${remotes_git_svn}^0 &&\n \tgit rev-parse --verify refs/remotes/svn^0 &&\n-\ttest \"$(git config --get svn-remote.svn.url)\" = \"$svnrepo\" &&\n+\ttest \"$(git config --get svn-remote.svn.url)\" = \"$svnrepo_escaped\" &&\n \ttest `git config --get svn-remote.svn.fetch` = \\\n \":refs/${remotes_git_svn}\"\n \t'\n-- \n1.7.11.3\n"},{"id":"196013","messageId":"1343468872-72133-5-git-send-email-schwern@pobox.com","threadId":"31120","inReplyTo":"1343468872-72133-1-git-send-email-schwern@pobox.com","subject":"[PATCH 4/8] Replace hand rolled URL escapes with canonicalization","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-28T09:47:48Z","receivedAt":"2012-07-28T09:47:48Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" \n\nContinuing to move towards getting everything canonicalizing the same way.\n\n* Git::SVN->init_remote_config and Git::SVN::Ra->minimize_url both\n have to canonicalize the same way else init_remote_config\n will incorrectly think they're different URLs causing\n t9107-git-svn-migrate.sh to fail.\n---\n git-svn.perl | 24 +++---------------------\n perl/Git/SVN.pm | 2 +-\n perl/Git/SVN/Ra.pm | 27 +++++----------------------\n 3 files changed, 9 insertions(+), 44 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 6e3e240..6e97545 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -1412,24 +1412,6 @@ sub cmd_commit_diff {\n \t}\n }\n \n-sub escape_uri_only {\n-\tmy ($uri) = @_;\n-\tmy @tmp;\n-\tforeach (split m{/}, $uri) {\n-\t\ts/([^~\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n-\t\tpush @tmp, $_;\n-\t}\n-\tjoin('/', @tmp);\n-}\n-\n-sub escape_url {\n-\tmy ($url) = @_;\n-\tif ($url =~ m#^([^:]+)://([^/]*)(.*)$#) {\n-\t\tmy ($scheme, $domain, $uri) = ($1, $2, escape_uri_only($3));\n-\t\t$url = \"$scheme://$domain$uri\";\n-\t}\n-\t$url;\n-}\n \n sub cmd_info {\n \tmy $path = canonicalize_path(defined($_[0]) ? $_[0] : \".\");\n@@ -1457,18 +1439,18 @@ sub cmd_info {\n \tmy $full_url = $url . ($fullpath eq \"\" ? \"\" : \"/$fullpath\");\n \n \tif ($_url) {\n-\t\tprint escape_url($full_url), \"\\n\";\n+\t\tprint canonicalize_url($full_url), \"\\n\";\n \t\treturn;\n \t}\n \n \tmy $result = \"Path: $path\\n\";\n \t$result .= \"Name: \" . basename($path) . \"\\n\" if $file_type ne \"dir\";\n-\t$result .= \"URL: \" . escape_url($full_url) . \"\\n\";\n+\t$result .= \"URL: \" . canonicalize_url($full_url) . \"\\n\";\n \n \teval {\n \t\tmy $repos_root = $gs->repos_root;\n \t\tGit::SVN::remove_username($repos_root);\n-\t\t$result .= \"Repository Root: \" . escape_url($repos_root) . \"\\n\";\n+\t\t$result .= \"Repository Root: \" . canonicalize_url($repos_root) . \"\\n\";\n \t};\n \tif ($@) {\n \t\t$result .= \"Repository Root: (offline)\\n\";\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex cb6d83a..4219e5b 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -296,7 +296,7 @@ sub find_existing_remote {\n \n sub init_remote_config {\n \tmy ($self, $url, $no_write) = @_;\n-\t$url =~ s!/+$!!; # strip trailing slash\n+\t$url = canonicalize_url($url);\n \tmy $r = read_all_remotes();\n \tmy $existing = find_existing_remote($url, $r);\n \tif ($existing) {\ndiff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\nindex ef7b3dd..ed9dbe9 100644\n--- a/perl/Git/SVN/Ra.pm\n+++ b/perl/Git/SVN/Ra.pm\n@@ -66,24 +66,6 @@ sub _auth_providers () {\n \t\\@rv;\n }\n \n-sub escape_uri_only {\n-\tmy ($uri) = @_;\n-\tmy @tmp;\n-\tforeach (split m{/}, $uri) {\n-\t\ts/([^~\\w.%+-]|%(?![a-fA-F0-9]{2}))/sprintf(\"%%%02X\",ord($1))/eg;\n-\t\tpush @tmp, $_;\n-\t}\n-\tjoin('/', @tmp);\n-}\n-\n-sub escape_url {\n-\tmy ($url) = @_;\n-\tif ($url =~ m#^(https?)://([^/]+)(.*)$#) {\n-\t\tmy ($scheme, $domain, $uri) = ($1, $2, escape_uri_only($3));\n-\t\t$url = \"$scheme://$domain$uri\";\n-\t}\n-\t$url;\n-}\n \n sub new {\n \tmy ($class, $url) = @_;\n@@ -119,7 +101,7 @@ sub new {\n \t\t\t$Git::SVN::Prompt::_no_auth_cache = 1;\n \t\t}\n \t} # no warnings 'once'\n-\tmy $self = SVN::Ra->new(url => escape_url($url), auth => $baton,\n+\tmy $self = SVN::Ra->new(url => canonicalize_url($url), auth => $baton,\n \t config => $config,\n \t\t\t pool => SVN::Pool->new,\n \t auth_provider_callbacks => $callbacks);\n@@ -314,7 +296,7 @@ sub gs_do_switch {\n \n \tif ($old_url =~ m#^svn(\\+ssh)?://# ||\n \t ($full_url =~ m#^https?://# &&\n-\t escape_url($full_url) ne $full_url)) {\n+\t canonicalize_url($full_url) ne $full_url)) {\n \t\t$_[0] = undef;\n \t\t$self = undef;\n \t\t$RA = undef;\n@@ -327,7 +309,7 @@ sub gs_do_switch {\n \t}\n \n \t$ra ||= $self;\n-\t$url_b = escape_url($url_b);\n+\t$url_b = canonicalize_url($url_b);\n \tmy $reporter = $ra->do_switch($rev_b, '', 1, $url_b, $editor, $pool);\n \tmy @lock = (::compare_svn_version('1.2.0') >= 0) ? (undef) : ();\n \t$reporter->set_path('', $rev_a, 0, @lock, $pool);\n@@ -582,7 +564,8 @@ sub minimize_url {\n \t\t\t$ra->get_log(\"\", $latest, 0, 1, 0, 1, sub {});\n \t\t};\n \t} while ($@ && ($c = shift @components));\n-\t$url;\n+\n+\treturn canonicalize_url($url);\n }\n \n sub can_do_switch {\n-- \n1.7.11.3\n"},{"id":"196012","messageId":"1343468872-72133-6-git-send-email-schwern@pobox.com","threadId":"31120","inReplyTo":"1343468872-72133-1-git-send-email-schwern@pobox.com","subject":"[PATCH 5/8] Canonicalize earlier in a couple spots.","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-28T09:47:49Z","receivedAt":"2012-07-28T09:47:49Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" \n\nJust a few things I noticed. Its good to canonicalize as early as\npossible.\n---\n git-svn.perl | 6 +++---\n perl/Git/SVN/Ra.pm | 4 ++--\n 2 files changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 6e97545..6b90765 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -1436,16 +1436,16 @@ sub cmd_info {\n \t# canonicalize_path() will return \"\" to make libsvn 1.5.x happy,\n \t$path = \".\" if $path eq \"\";\n \n-\tmy $full_url = $url . ($fullpath eq \"\" ? \"\" : \"/$fullpath\");\n+\tmy $full_url = canonicalize_url( $url . ($fullpath eq \"\" ? \"\" : \"/$fullpath\") );\n \n \tif ($_url) {\n-\t\tprint canonicalize_url($full_url), \"\\n\";\n+\t\tprint \"$full_url\\n\";\n \t\treturn;\n \t}\n \n \tmy $result = \"Path: $path\\n\";\n \t$result .= \"Name: \" . basename($path) . \"\\n\" if $file_type ne \"dir\";\n-\t$result .= \"URL: \" . canonicalize_url($full_url) . \"\\n\";\n+\t$result .= \"URL: $full_url\\n\";\n \n \teval {\n \t\tmy $repos_root = $gs->repos_root;\ndiff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\nindex ed9dbe9..eee7c00 100644\n--- a/perl/Git/SVN/Ra.pm\n+++ b/perl/Git/SVN/Ra.pm\n@@ -69,7 +69,7 @@ sub _auth_providers () {\n \n sub new {\n \tmy ($class, $url) = @_;\n-\t$url =~ s!/+$!!;\n+\t$url = canonicalize_url($url);\n \treturn $RA if ($RA && $RA->url eq $url);\n \n \t::_req_svn();\n@@ -101,7 +101,7 @@ sub new {\n \t\t\t$Git::SVN::Prompt::_no_auth_cache = 1;\n \t\t}\n \t} # no warnings 'once'\n-\tmy $self = SVN::Ra->new(url => canonicalize_url($url), auth => $baton,\n+\tmy $self = SVN::Ra->new(url => $url, auth => $baton,\n \t config => $config,\n \t\t\t pool => SVN::Pool->new,\n \t auth_provider_callbacks => $callbacks);\n-- \n1.7.11.3\n"},{"id":"196009","messageId":"1343468872-72133-7-git-send-email-schwern@pobox.com","threadId":"31120","inReplyTo":"1343468872-72133-1-git-send-email-schwern@pobox.com","subject":"[PATCH 6/8] Add function to append a path to a URL.","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-28T09:47:50Z","receivedAt":"2012-07-28T09:47:50Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" \n\nRemove the ad-hoc versions.\n\nThis is mostly to normalize the process and ensure the URLs produced\ndon't have double slashes or anything.\n\nAlso provides a place to fix the corner case where a file path\ncontains a percent sign.\n---\n git-svn.perl | 3 ++-\n perl/Git/SVN.pm | 33 +++++++++++++++------------------\n perl/Git/SVN/Ra.pm | 8 ++++----\n perl/Git/SVN/Utils.pm | 27 +++++++++++++++++++++++++++\n t/Git-SVN/Utils/add_path_to_url.t | 27 +++++++++++++++++++++++++++\n 5 files changed, 75 insertions(+), 23 deletions(-)\n create mode 100644 t/Git-SVN/Utils/add_path_to_url.t\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 6b90765..3d120d5 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -35,6 +35,7 @@ use Git::SVN::Utils qw(\n \tcanonicalize_path\n \tcanonicalize_url\n \tjoin_paths\n+\tadd_path_to_url\n );\n \n use Git qw(\n@@ -1436,7 +1437,7 @@ sub cmd_info {\n \t# canonicalize_path() will return \"\" to make libsvn 1.5.x happy,\n \t$path = \".\" if $path eq \"\";\n \n-\tmy $full_url = canonicalize_url( $url . ($fullpath eq \"\" ? \"\" : \"/$fullpath\") );\n+\tmy $full_url = canonicalize_url( add_path_to_url( $url, $fullpath ) );\n \n \tif ($_url) {\n \t\tprint \"$full_url\\n\";\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex 4219e5b..22bf207 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -29,6 +29,7 @@ use Git::SVN::Utils qw(\n \tjoin_paths\n \tcanonicalize_path\n \tcanonicalize_url\n+\tadd_path_to_url\n );\n \n my $can_use_yaml;\n@@ -564,8 +565,7 @@ sub _set_svm_vars {\n \t\t# username is of no interest\n \t\t$src =~ s{(^[a-z\\+]*://)[^/@]*@}{$1};\n \n-\t\tmy $replace = $ra->url;\n-\t\t$replace .= \"/$path\" if length $path;\n+\t\tmy $replace = add_path_to_url($ra->url, $path);\n \n \t\tmy $section = \"svn-remote.$self->{repo_id}\";\n \t\ttmp_config(\"$section.svm-source\", $src);\n@@ -582,7 +582,7 @@ sub _set_svm_vars {\n \tmy $path = $self->path;\n \tmy %tried;\n \twhile (length $path) {\n-\t\tmy $try = $self->url . \"/$path\";\n+\t\tmy $try = add_path_to_url($self->url, $path);\n \t\tunless ($tried{$try}) {\n \t\t\treturn $ra if $self->read_svm_props($ra, $path, $r);\n \t\t\t$tried{$try} = 1;\n@@ -591,7 +591,7 @@ sub _set_svm_vars {\n \t}\n \tdie \"Path: '$path' should be ''\\n\" if $path ne '';\n \treturn $ra if $self->read_svm_props($ra, $path, $r);\n-\t$tried{$self->url.\"/$path\"} = 1;\n+\t$tried{ add_path_to_url($self->url, $path) } = 1;\n \n \tif ($ra->{repos_root} eq $self->url) {\n \t\tdie @err, (map { \" $_\\n\" } keys %tried), \"\\n\";\n@@ -603,7 +603,7 @@ sub _set_svm_vars {\n \t$path = $ra->{svn_path};\n \t$ra = Git::SVN::Ra->new($ra->{repos_root});\n \twhile (length $path) {\n-\t\tmy $try = $ra->url .\"/$path\";\n+\t\tmy $try = add_path_to_url($ra->url, $path);\n \t\tunless ($tried{$try}) {\n \t\t\t$ok = $self->read_svm_props($ra, $path, $r);\n \t\t\tlast if $ok;\n@@ -613,7 +613,7 @@ sub _set_svm_vars {\n \t}\n \tdie \"Path: '$path' should be ''\\n\" if $path ne '';\n \t$ok ||= $self->read_svm_props($ra, $path, $r);\n-\t$tried{$ra->url .\"/$path\"} = 1;\n+\t$tried{ add_path_to_url($ra->url, $path) } = 1;\n \tif (!$ok) {\n \t\tdie @err, (map { \" $_\\n\" } keys %tried), \"\\n\";\n \t}\n@@ -933,20 +933,19 @@ sub rewrite_uuid {\n \n sub metadata_url {\n \tmy ($self) = @_;\n-\t($self->rewrite_root || $self->url) .\n-\t (length $self->path ? '/' . $self->path : '');\n+\tmy $url = $self->rewrite_root || $self->url;\n+\treturn add_path_to_url( $url, $self->path );\n }\n \n sub full_url {\n \tmy ($self) = @_;\n-\t$self->url . (length $self->path ? '/' . $self->path : '');\n+\treturn add_path_to_url( $self->url, $self->path );\n }\n \n sub full_pushurl {\n \tmy ($self) = @_;\n \tif ($self->{pushurl}) {\n-\t\treturn $self->{pushurl} . (length $self->path ? '/' .\n-\t\t $self->path : '');\n+\t\treturn add_path_to_url( $self->{pushurl}, $self->path );\n \t} else {\n \t\treturn $self->full_url;\n \t}\n@@ -1114,7 +1113,7 @@ sub find_parent_branch {\n \tmy $r = $i->{copyfrom_rev};\n \tmy $repos_root = $self->ra->{repos_root};\n \tmy $url = $self->ra->url;\n-\tmy $new_url = $url . $branch_from;\n+\tmy $new_url = add_path_to_url( $url, $branch_from );\n \tprint STDERR \"Found possible branch point: \",\n \t \"$new_url => \", $self->full_url, \", $r\\n\"\n \t unless $::_q > 1;\n@@ -1443,12 +1442,11 @@ sub find_extra_svk_parents {\n \tfor my $ticket ( @tickets ) {\n \t\tmy ($uuid, $path, $rev) = split /:/, $ticket;\n \t\tif ( $uuid eq $self->ra_uuid ) {\n-\t\t\tmy $url = $self->url;\n-\t\t\tmy $repos_root = $url;\n+\t\t\tmy $repos_root = $self->url;\n \t\t\tmy $branch_from = $path;\n \t\t\t$branch_from =~ s{^/}{};\n-\t\t\tmy $gs = $self->other_gs($repos_root.\"/\".$branch_from,\n-\t\t\t $url,\n+\t\t\tmy $gs = $self->other_gs(add_path_to_url( $repos_root, $branch_from ),\n+\t\t\t $repos_root,\n \t\t\t $branch_from,\n \t\t\t $rev,\n \t\t\t $self->{ref_id});\n@@ -1871,8 +1869,7 @@ sub make_log_entry {\n \t\t$email ||= \"$author\\@$uuid\";\n \t\t$commit_email ||= \"$author\\@$uuid\";\n \t} elsif ($self->use_svnsync_props) {\n-\t\tmy $full_url = $self->svnsync->{url};\n-\t\t$full_url .= \"/\".$self->path if length $self->path;\n+\t\tmy $full_url = add_path_to_url( $self->svnsync->{url}, $self->path );\n \t\tremove_username($full_url);\n \t\tmy $uuid = $self->svnsync->{uuid};\n \t\t$log_entry{metadata} = \"$full_url\\@$rev $uuid\";\ndiff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\nindex eee7c00..788e642 100644\n--- a/perl/Git/SVN/Ra.pm\n+++ b/perl/Git/SVN/Ra.pm\n@@ -5,6 +5,7 @@ use warnings;\n use SVN::Client;\n use Git::SVN::Utils qw(\n \tcanonicalize_url\n+\tadd_path_to_url\n );\n \n use SVN::Ra;\n@@ -289,9 +290,8 @@ sub gs_do_switch {\n \tmy $path = $gs->path;\n \tmy $pool = SVN::Pool->new;\n \n-\tmy $full_url = $self->url;\n-\tmy $old_url = $full_url;\n-\t$full_url .= '/' . $path if length $path;\n+\tmy $old_url = $self->url;\n+\tmy $full_url = add_path_to_url( $self->url, $path );\n \tmy ($ra, $reparented);\n \n \tif ($old_url =~ m#^svn(\\+ssh)?://# ||\n@@ -557,7 +557,7 @@ sub minimize_url {\n \tmy @components = split(m!/!, $self->{svn_path});\n \tmy $c = '';\n \tdo {\n-\t\t$url .= \"/$c\" if length $c;\n+\t\t$url = add_path_to_url($url, $c);\n \t\teval {\n \t\t\tmy $ra = (ref $self)->new($url);\n \t\t\tmy $latest = $ra->get_latest_revnum;\ndiff --git a/perl/Git/SVN/Utils.pm b/perl/Git/SVN/Utils.pm\nindex dab6e4d..3aec207 100644\n--- a/perl/Git/SVN/Utils.pm\n+++ b/perl/Git/SVN/Utils.pm\n@@ -13,6 +13,7 @@ our @EXPORT_OK = qw(\n \tcanonicalize_path\n \tcanonicalize_url\n \tjoin_paths\n+\tadd_path_to_url\n );\n \n \n@@ -200,4 +201,30 @@ sub join_paths {\n \treturn $new_path .= \"/$last_path\";\n }\n \n+\n+=head3 add_path_to_url\n+\n+ my $new_url = add_path_to_url($url, $path);\n+\n+Appends $path onto the $url. If $path is empty, $url is returned unchanged.\n+\n+=cut\n+\n+sub add_path_to_url {\n+\tmy($url, $path) = @_;\n+\n+\treturn $url if !defined $path or !length $path;\n+\n+\t# Strip trailing and leading slashes so we don't\n+\t# wind up with http://x.com///path\n+\t$url =~ s{/+$}{};\n+\t$path =~ s{^/+}{};\n+\n+\t# If a path has a % in it, URI escape it so it's not\n+\t# mistaken for a URI escape later.\n+\t$path =~ s{%}{%25}g;\n+\n+\treturn join '/', $url, $path;\n+}\n+\n 1;\ndiff --git a/t/Git-SVN/Utils/add_path_to_url.t b/t/Git-SVN/Utils/add_path_to_url.t\nnew file mode 100644\nindex 0000000..bfbd878\n--- /dev/null\n+++ b/t/Git-SVN/Utils/add_path_to_url.t\n@@ -0,0 +1,27 @@\n+#!/usr/bin/env perl\n+\n+use strict;\n+use warnings;\n+\n+use Test::More 'no_plan';\n+\n+use Git::SVN::Utils qw(\n+\tadd_path_to_url\n+);\n+\n+# A reference cannot be a hash key, so we use an array.\n+my @tests = (\n+\t[\"http://x.com\", \"bar\"]\t\t\t=> 'http://x.com/bar',\n+\t[\"http://x.com\", \"\"]\t\t\t=> 'http://x.com',\n+\t[\"http://x.com/foo/\", undef]\t\t=> 'http://x.com/foo/',\n+\t[\"http://x.com/foo/\", \"/bar/baz/\"]\t=> 'http://x.com/foo/bar/baz/',\n+\t[\"http://x.com\", 'per%cent']\t\t=> 'http://x.com/per%25cent',\n+);\n+\n+while(@tests) {\n+\tmy($have, $want) = splice @tests, 0, 2;\n+\n+\tmy $args = join \", \", map { qq['$_'] } map { defined($_) ? $_ : 'undef' } @$have;\n+\tmy $name = \"add_path_to_url($args) eq $want\";\n+\tis add_path_to_url(@$have), $want, $name;\n+}\n-- \n1.7.11.3\n"},{"id":"196011","messageId":"1343468872-72133-8-git-send-email-schwern@pobox.com","threadId":"31120","inReplyTo":"1343468872-72133-1-git-send-email-schwern@pobox.com","subject":"[PATCH 7/8] Turn on canonicalization on newly minted URLs.","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-28T09:47:51Z","receivedAt":"2012-07-28T09:47:51Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" \n\nGo through all the spots that use the new add_path_to_url() to\nmake a new URL and canonicalize them.\n\n* copyfrom_path has to be canonicalized else find_parent_branch\n will get confused\n\n* due to the `canonicalize_url($full_url) ne $full_url)` line of\n logic in gs_do_switch(), $full_url is left alone until after.\n\nAt this point SVN 1.7 passes except for 3 tests in\nt9100-git-svn-basic.sh that look like an SVN bug to do with\nsymlinks.\n---\n perl/Git/SVN.pm | 19 ++++++++++++++-----\n perl/Git/SVN/Ra.pm | 9 ++++++++-\n 2 files changed, 22 insertions(+), 6 deletions(-)\n\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex 22bf207..e5f7acc 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -362,6 +362,8 @@ sub init_remote_config {\n sub find_by_url { # repos_root and, path are optional\n \tmy ($class, $full_url, $repos_root, $path) = @_;\n \n+\t$full_url = canonicalize_url($full_url);\n+\n \treturn undef unless defined $full_url;\n \tremove_username($full_url);\n \tremove_username($repos_root) if defined $repos_root;\n@@ -400,6 +402,11 @@ sub find_by_url { # repos_root and, path are optional\n \t\t\t}\n \t\t\t$p =~ s#^\\Q$z\\E(?:/|$)#$prefix# or next;\n \t\t}\n+\n+\t\t# remote fetch paths are not URI escaped. Decode ours\n+\t\t# so they match\n+\t\t$p = uri_decode($p);\n+\n \t\tforeach my $f (keys %$fetch) {\n \t\t\tnext if $f ne $p;\n \t\t\treturn Git::SVN->new($fetch->{$f}, $repo_id, $f);\n@@ -934,18 +941,18 @@ sub rewrite_uuid {\n sub metadata_url {\n \tmy ($self) = @_;\n \tmy $url = $self->rewrite_root || $self->url;\n-\treturn add_path_to_url( $url, $self->path );\n+\treturn canonicalize_url( add_path_to_url( $url, $self->path ) );\n }\n \n sub full_url {\n \tmy ($self) = @_;\n-\treturn add_path_to_url( $self->url, $self->path );\n+\treturn canonicalize_url( add_path_to_url( $self->url, $self->path ) );\n }\n \n sub full_pushurl {\n \tmy ($self) = @_;\n \tif ($self->{pushurl}) {\n-\t\treturn add_path_to_url( $self->{pushurl}, $self->path );\n+\t\treturn canonicalize_url( add_path_to_url( $self->{pushurl}, $self->path ) );\n \t} else {\n \t\treturn $self->full_url;\n \t}\n@@ -1113,7 +1120,7 @@ sub find_parent_branch {\n \tmy $r = $i->{copyfrom_rev};\n \tmy $repos_root = $self->ra->{repos_root};\n \tmy $url = $self->ra->url;\n-\tmy $new_url = add_path_to_url( $url, $branch_from );\n+\tmy $new_url = canonicalize_url( add_path_to_url( $url, $branch_from ) );\n \tprint STDERR \"Found possible branch point: \",\n \t \"$new_url => \", $self->full_url, \", $r\\n\"\n \t unless $::_q > 1;\n@@ -1869,7 +1876,9 @@ sub make_log_entry {\n \t\t$email ||= \"$author\\@$uuid\";\n \t\t$commit_email ||= \"$author\\@$uuid\";\n \t} elsif ($self->use_svnsync_props) {\n-\t\tmy $full_url = add_path_to_url( $self->svnsync->{url}, $self->path );\n+\t\tmy $full_url = canonicalize_url(\n+\t\t\tadd_path_to_url( $self->svnsync->{url}, $self->path )\n+\t\t);\n \t\tremove_username($full_url);\n \t\tmy $uuid = $self->svnsync->{uuid};\n \t\t$log_entry{metadata} = \"$full_url\\@$rev $uuid\";\ndiff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm\nindex 788e642..29b46e8 100644\n--- a/perl/Git/SVN/Ra.pm\n+++ b/perl/Git/SVN/Ra.pm\n@@ -5,6 +5,7 @@ use warnings;\n use SVN::Client;\n use Git::SVN::Utils qw(\n \tcanonicalize_url\n+\tcanonicalize_path\n \tadd_path_to_url\n );\n \n@@ -102,6 +103,7 @@ sub new {\n \t\t\t$Git::SVN::Prompt::_no_auth_cache = 1;\n \t\t}\n \t} # no warnings 'once'\n+\n \tmy $self = SVN::Ra->new(url => $url, auth => $baton,\n \t config => $config,\n \t\t\t pool => SVN::Pool->new,\n@@ -200,6 +202,7 @@ sub get_log {\n \t\t\t\tqw/copyfrom_path copyfrom_rev action/;\n \t\t\tif ($s{'copyfrom_path'}) {\n \t\t\t\t$s{'copyfrom_path'} =~ s/$prefix_regex//;\n+\t\t\t\t$s{'copyfrom_path'} = canonicalize_path($s{'copyfrom_path'});\n \t\t\t}\n \t\t\t$_[0]{$p} = \\%s;\n \t\t}\n@@ -303,7 +306,11 @@ sub gs_do_switch {\n \t\t$ra = Git::SVN::Ra->new($full_url);\n \t\t$ra_invalid = 1;\n \t} elsif ($old_url ne $full_url) {\n-\t\tSVN::_Ra::svn_ra_reparent($self->{session}, $full_url, $pool);\n+\t\tSVN::_Ra::svn_ra_reparent(\n+\t\t\t$self->{session},\n+\t\t\tcanonicalize_url($full_url),\n+\t\t\t$pool\n+\t\t);\n \t\t$self->url($full_url);\n \t\t$reparented = 1;\n \t}\n-- \n1.7.11.3\n"},{"id":"196010","messageId":"1343468872-72133-9-git-send-email-schwern@pobox.com","threadId":"31120","inReplyTo":"1343468872-72133-1-git-send-email-schwern@pobox.com","subject":"[PATCH 8/8] Remove some ad hoc canonicalizations.","fromName":"Michael G. Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-28T09:47:52Z","receivedAt":"2012-07-28T09:47:52Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"From: \"Michael G. Schwern\" \n\n---\n git-svn.perl | 8 ++++----\n perl/Git/SVN.pm | 1 -\n 2 files changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 3d120d5..56d1ba7 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -36,6 +36,7 @@ use Git::SVN::Utils qw(\n \tcanonicalize_url\n \tjoin_paths\n \tadd_path_to_url\n+\tjoin_paths\n );\n \n use Git qw(\n@@ -1598,7 +1599,7 @@ sub post_fetch_checkout {\n \n sub complete_svn_url {\n \tmy ($url, $path) = @_;\n-\t$path =~ s#/+$##;\n+\t$path = canonicalize_path($path);\n \n \t# If the path is not a URL...\n \tif ($path !~ m#^[a-z\\+]+://#) {\n@@ -1617,7 +1618,7 @@ sub complete_url_ls_init {\n \t\tprint STDERR \"W: $switch not specified\\n\";\n \t\treturn;\n \t}\n-\t$repo_path =~ s#/+$##;\n+\t$repo_path = canonicalize_path($repo_path);\n \tif ($repo_path =~ m#^[a-z\\+]+://#) {\n \t\t$ra = Git::SVN::Ra->new($repo_path);\n \t\t$repo_path = '';\n@@ -1638,9 +1639,8 @@ sub complete_url_ls_init {\n \t}\n \tcommand_oneline('config', $k, $gs->url) unless $orig_url;\n \n-\tmy $remote_path = $gs->path . \"/$repo_path\";\n+\tmy $remote_path = join_paths( $gs->path, $repo_path );\n \t$remote_path =~ s{%([0-9A-F]{2})}{chr hex($1)}ieg;\n-\t$remote_path =~ s#/+#/#g;\n \t$remote_path =~ s#^/##g;\n \t$remote_path .= \"/*\" if $remote_path !~ /\\*/;\n \tmy ($n) = ($switch =~ /^--(\\w+)/);\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex e5f7acc..3c68c09 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -460,7 +460,6 @@ sub new {\n \t}\n \t{\n \t\tmy $path = $self->path;\n-\t\t$path =~ s{/+}{/}g;\n \t\t$path =~ s{\\A/}{};\n \t\t$path =~ s{/\\z}{};\n \t\t$self->path($path);\n-- \n1.7.11.3\n"},{"id":"196025","messageId":"20120728141652.GA1603@burratino","threadId":"31120","inReplyTo":"1343468872-72133-2-git-send-email-schwern@pobox.com","subject":"Re: [PATCH 1/8] SVN 1.7 will truncate \"not-a%40{0}\" to just \"not-a\".","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-07-28T14:16:52Z","receivedAt":"2012-07-28T14:16:52Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael G. Schwern wrote:\n\n> Rather than guess what SVN is going to do for each version, make the test use\n> the branch name that was actually created.\n[...]\n> -\t\tgit rev-parse \"refs/remotes/not-a%40{0}reflog\"\n> +\t\tgit rev-parse \"refs/remotes/$non_reflog\"\n\nDoesn't this defeat the point of the testcase (checking that git-svn\nis able to avoid creating git refs containing @{, following the rules\nfrom git-check-ref-format(1))?\n\nDo you know when SVN truncates the directory name? Would historical\nSVN repositories or historical SVN servers be able to have a directory\nnamed with a %40 in it, or has this been disallowed completely,\nleaving problematic historical repositories to be dumped with old SVN,\ntweaked, and reloaded with new SVN?\n\nThanks,\nJonathan\n"},{"id":"196045","messageId":"50143E34.8090802@pobox.com","threadId":"31120","inReplyTo":"20120728141652.GA1603@burratino","subject":"Re: [PATCH 1/8] SVN 1.7 will truncate \"not-a%40{0}\" to just \"not-a\".","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-28T19:32:04Z","receivedAt":"2012-07-28T19:32:04Z","isPatch":true,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"On 2012.7.28 7:16 AM, Jonathan Nieder wrote:\n> Michael G. Schwern wrote:\n> \n>> Rather than guess what SVN is going to do for each version, make the test use\n>> the branch name that was actually created.\n> [...]\n>> -\t\tgit rev-parse \"refs/remotes/not-a%40{0}reflog\"\n>> +\t\tgit rev-parse \"refs/remotes/$non_reflog\"\n> \n> Doesn't this defeat the point of the testcase (checking that git-svn\n> is able to avoid creating git refs containing @{, following the rules\n> from git-check-ref-format(1))?\n\nUnless I messed up, entirely possible as I'm not a shell programmer, the test\nis still useful for testing SVN 1.6. Under SVN 1.6 $non_reflog should be\n'not-a%40{0}reflog' as before.\n\n\n> Do you know when SVN truncates the directory name?\n\nIIRC its silently does it during the \"svn cp\".\n\n\n> Would historical\n> SVN repositories or historical SVN servers be able to have a directory\n> named with a %40 in it, or has this been disallowed completely,\n> leaving problematic historical repositories to be dumped with old SVN,\n> tweaked, and reloaded with new SVN?\n\nDunno, lemme check...\n\n$ source ~/bin/svn16\n$ svnadmin --version\nsvnadmin, version 1.6.18 (r1303927)\n...\n$ svnadmin create svnrepo\n$ mkdir project project/trunk project/branches project/tags\n$ echo foo > project/trunk/foo\n$ svn import -m 'test import' project\nfile:///Users/schwern/tmp/test/svnrepo/project\nAdding project/tags\nAdding project/trunk\nAdding project/trunk/foo\nAdding project/branches\n\nCommitted revision 1.\n$ rm -rf project/\n$ svn cp -m 'reflog' file:///Users/schwern/tmp/test/svnrepo/project/trunk\n'file:///Users/schwern/tmp/test/svnrepo/project/branches/not-a%40{0}reflog'\n\nCommitted revision 2.\n$ svn ls file:///Users/schwern/tmp/test/svnrepo/project/branches\nnot-a@{0}reflog/\n$ source ~/bin/svn17\n$ svn --version\nsvn, version 1.7.5 (r1336830)\n...\n$ svn ls file:///Users/schwern/tmp/test/svnrepo/project/branches\nnot-a@{0}reflog/\n\nIf you make it with SVN 1.6 its still there with SVN 1.7. That's good, it\nmeans you can ship a prebuilt repository and check it against SVN 1.7.\n\nThe bad news is the new code segfaults on it. I don't know if that's the SVN\n1.7 API choking on its own stuff or because of my changes or both. If you set\nup the test I can try and fix it. Otherwise I'll just flounder in shell.\n\n\n-- \n\"I went to college, which is a lot like being in the Army, except when\n stupid people yell at me for stupid things, I can hit them.\"\n -- Jonathan Schwarz\n"},{"id":"196192","messageId":"20120730203844.GA23892@dcvr.yhbt.net","threadId":"31120","inReplyTo":"1343468872-72133-1-git-send-email-schwern@pobox.com","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-30T20:38:44Z","receivedAt":"2012-07-30T20:38:44Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"\"Michael G. Schwern\" wrote:\n> There is one exception. t9100-git-svn-basic.sh fails 11-13. This appears\n> to be due to a bug in SVN to do with symlinks. Leave that for somebody\n> else, this is the final submission in the series.\n\nThat's fine, a few failing tests is better than completely failing.\n\n> The work was difficult because the code relies on simple string equalty\n> when comparing URLs and paths. Turning on canonicalization in one part\n> of the code would cause another part to fail if it also did not\n> canonicalize. There's likely still issues.\n> \n> A better solution would be to have path and URL objects which overload\n> the eq operator and automatically stringify canonicalized and escaped.\n\nPerhaps we can depend on the URI.pm module? It seems to be\nwidely-available and not be a significant barrier to installation. On\nthe other hand, I don't know its history, either (especially since we're\nnow dealing with SVN changes...).\n\nAnyways, I don't like relying on operator overloading, it makes code\nharder to read and review.\n"},{"id":"196196","messageId":"5016F832.7030604@pobox.com","threadId":"31120","inReplyTo":"20120730203844.GA23892@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-30T21:10:10Z","receivedAt":"2012-07-30T21:10:10Z","isPatch":false,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"On 2012.7.30 1:38 PM, Eric Wong wrote:\n>> A better solution would be to have path and URL objects which overload\n>> the eq operator and automatically stringify canonicalized and escaped.\n> \n> Perhaps we can depend on the URI.pm module? It seems to be\n> widely-available and not be a significant barrier to installation. On\n> the other hand, I don't know its history, either (especially since we're\n> now dealing with SVN changes...).\n\nIf you want to go down the road of having CPAN dependencies, then it should\ndefinitely be used rather than rolling our own and generating our own bugs.\nIt's a very commonly needed Perl module.\n\nYou'd make a subclass and put any special work arounds for SVN in there.\n\n\n> Anyways, I don't like relying on operator overloading, it makes code\n> harder to read and review.\n\nRight now, canonicalization is a bug generator. Paths and URLs have to be in\nthe same form when they're compared. This requires meticulous care on the\npart of the coder and reviewer to check every comparison. It scatters the\nlogic for proper comparison all over the code. Redundant logic scattered\naround the code is a Bad Thing. It makes it more likely a coder will forget\nthe logic, or get it wrong, and a human reviewer must be far more vigilant.\n\nRight now I'm pretty sure there's still a ton of bugs.\n\nIt also slows things down. As strings, URLs and paths have to be\ncanonicalized every time they're used or compared. An object representing the\nURI or path can cache the canonicalization.\n\nWith string comparison overloaded, you'd no longer have to meticulously check\nthat URLs and paths are always in the same form when they're compared. It\njust does it. The logic is in one place. We don't even have to care if one\nof them is a string (or which one), it works even if only one half of the\ncomparison is an object. A new coder to the project doesn't need to know\nanything special about URIs and paths, they just treat them as strings.\nFinally, they can be slipped into existing code without having to rewrite\neverything.\n\nOverloaded comparison and stringification can even be used as a tool to find\nall the places in the code where URLs and paths are being used, where they're\nbeing turned into strings, and where URL and path manipulation is being done\nad-hoc. For example, if comparison sees one of its arguments as a string, or\nif concatenation is used.\n\nThe only downside is when chasing down a bug related to canonicalization one\nmight have to realize that eq is overloaded. But we'd have far less bugs due\nto canonicalization. So worth it.\n\n\n-- \nBeing faith-based doesn't trump reality.\n\t-- Bruce Sterling\n"},{"id":"196202","messageId":"20120730221548.GA388@dcvr.yhbt.net","threadId":"31120","inReplyTo":"5016F832.7030604@pobox.com","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-30T22:15:48Z","receivedAt":"2012-07-30T22:15:48Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Michael G Schwern wrote:\n> On 2012.7.30 1:38 PM, Eric Wong wrote:\n> > Anyways, I don't like relying on operator overloading, it makes code\n> > harder to read and review.\n> \n> Right now, canonicalization is a bug generator. Paths and URLs have to be in\n> the same form when they're compared. This requires meticulous care on the\n> part of the coder and reviewer to check every comparison. It scatters the\n> logic for proper comparison all over the code. Redundant logic scattered\n> around the code is a Bad Thing. It makes it more likely a coder will forget\n> the logic, or get it wrong, and a human reviewer must be far more vigilant.\n\n I agree completely with canonicalization.\n\n> The only downside is when chasing down a bug related to canonicalization one\n> might have to realize that eq is overloaded.\n\nHaving to realize eq is overloaded is a huge downside to me.\n"},{"id":"196203","messageId":"50172F10.2030402@pobox.com","threadId":"31120","inReplyTo":"20120730221548.GA388@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-31T01:04:16Z","receivedAt":"2012-07-31T01:04:16Z","isPatch":false,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"On 2012.7.30 3:15 PM, Eric Wong wrote:\n>> Right now, canonicalization is a bug generator. Paths and URLs have to be in\n>> the same form when they're compared. This requires meticulous care on the\n>> part of the coder and reviewer to check every comparison. It scatters the\n>> logic for proper comparison all over the code. Redundant logic scattered\n>> around the code is a Bad Thing. It makes it more likely a coder will forget\n>> the logic, or get it wrong, and a human reviewer must be far more vigilant.\n> \n> I agree completely with canonicalization.\n\nSorry, I'm not sure what you're agreeing with.\n\n\n>> The only downside is when chasing down a bug related to canonicalization one\n>> might have to realize that eq is overloaded.\n> \n> Having to realize eq is overloaded is a huge downside to me.\n\nPresumably you'd be reviewing the change which implements the overloaded\nobjects, so you'd know about it. And it would be documented.\n\nI've listed a bunch of concrete positives for using comparison overloaded\nURI/path objects vs how it's currently being done. How about you voice some\nof the downsides in concrete terms? Or an alternative that solves the current\nproblems?\n\n\n-- \nAhh email, my old friend. Do you know that revenge is a dish that is best\nserved cold? And it is very cold on the Internet!\n"},{"id":"196204","messageId":"20120731021816.GA12640@dcvr.yhbt.net","threadId":"31120","inReplyTo":"50172F10.2030402@pobox.com","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-31T02:18:18Z","receivedAt":"2012-07-31T02:18:18Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Michael G Schwern wrote:\n> On 2012.7.30 3:15 PM, Eric Wong wrote:\n> >> Right now, canonicalization is a bug generator. Paths and URLs have to be in\n> >> the same form when they're compared. This requires meticulous care on the\n> >> part of the coder and reviewer to check every comparison. It scatters the\n> >> logic for proper comparison all over the code. Redundant logic scattered\n> >> around the code is a Bad Thing. It makes it more likely a coder will forget\n> >> the logic, or get it wrong, and a human reviewer must be far more vigilant.\n> > \n> > I agree completely with canonicalization.\n> \n> Sorry, I'm not sure what you're agreeing with.\n\nThat's it's a bug generator and we shouldn't have redundant logic.\nHaving functions to compare objects themselves is a good thing.\n\n> >> The only downside is when chasing down a bug related to canonicalization one\n> >> might have to realize that eq is overloaded.\n> > \n> > Having to realize eq is overloaded is a huge downside to me.\n> \n> Presumably you'd be reviewing the change which implements the overloaded\n> objects, so you'd know about it. And it would be documented.\n\nThe change itself is easy to review. Picking up the code a few\nmonths/years down the line and having to know \"eq\" is overloaded\ntends to bite people.\n\n> I've listed a bunch of concrete positives for using comparison overloaded\n> URI/path objects vs how it's currently being done. How about you voice some\n> of the downsides in concrete terms? Or an alternative that solves the current\n> problems?\n\nAny custom comparison function would do the trick (e.g. URI::eq()).\n\nI _want_ URI/path objects. I do not want a bare \"eq\" operator to\nobscure the fact it's calling URI::eq() behind-the-scenes.\n\nThat said, I don't mind overloads when it's obvious an overload is being\nused (e.g. stringify). It's things like \"eq\" which obscure the fact\nfunction calls are happening in the background.\n"},{"id":"196209","messageId":"50175F82.7070606@pobox.com","threadId":"31120","inReplyTo":"20120731021816.GA12640@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-31T04:30:58Z","receivedAt":"2012-07-31T04:30:58Z","isPatch":false,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"On 2012.7.30 7:18 PM, Eric Wong wrote:\n> Michael G Schwern wrote:\n>> On 2012.7.30 3:15 PM, Eric Wong wrote:\n>>>> Right now, canonicalization is a bug generator. Paths and URLs have to be in\n>>>> the same form when they're compared. This requires meticulous care on the\n>>>> part of the coder and reviewer to check every comparison. It scatters the\n>>>> logic for proper comparison all over the code. Redundant logic scattered\n>>>> around the code is a Bad Thing. It makes it more likely a coder will forget\n>>>> the logic, or get it wrong, and a human reviewer must be far more vigilant.\n>>>\n>>> I agree completely with canonicalization.\n>>\n>> Sorry, I'm not sure what you're agreeing with.\n> \n> That's it's a bug generator and we shouldn't have redundant logic.\n> Having functions to compare objects themselves is a good thing.\n\nThat doesn't make it much better than what we have now. One still has to\nremember to pepper those special comparisons all over the code.\n\n\n>>>> The only downside is when chasing down a bug related to canonicalization one\n>>>> might have to realize that eq is overloaded.\n>>>\n>>> Having to realize eq is overloaded is a huge downside to me.\n>>\n>> Presumably you'd be reviewing the change which implements the overloaded\n>> objects, so you'd know about it. And it would be documented.\n> \n> The change itself is easy to review. Picking up the code a few\n> months/years down the line and having to know \"eq\" is overloaded\n> tends to bite people.\n\nWhy does a reviewer, or a reader of the code, have to know eq is overloaded?\n\nHow often would string comparing an overloaded uri/path object be the wrong\nthing to do? Just about never. Compare that to how often it would be\nincorrect to string compare a non-overloaded uri/path object. Most of the\ntime. Do you feel it would be otherwise?\n\nIf they're overloaded, somebody patching the code doesn't have to know to use\na special uri_eq() function. It'll just happen when they naturally string\ncompare. The coder doesn't have to know or do anything special. The reviewer\ndoesn't have to do any special work.\n\nIf they're not overloaded, coders must know about the special URI and path\nrequirements. Each string comparison is suspect and must be scrutinized by\nthe reviewer. They have to think \"is this actually a uri or path comparison?\n Should it be using the special comparison functions?\"\n\nWhich procedure offers more opportunities for mistakes?\n\n\n>> I've listed a bunch of concrete positives for using comparison overloaded\n>> URI/path objects vs how it's currently being done. How about you voice some\n>> of the downsides in concrete terms? Or an alternative that solves the current\n>> problems?\n> \n> Any custom comparison function would do the trick (e.g. URI::eq()).\n>\n> I _want_ URI/path objects. I do not want a bare \"eq\" operator to\n> obscure the fact it's calling URI::eq() behind-the-scenes.\n>\n> That said, I don't mind overloads when it's obvious an overload is being\n> used (e.g. stringify). It's things like \"eq\" which obscure the fact\n> function calls are happening in the background.\n\nIs that a problem? If so, why?\n\nIf the objects stringify, but comparing them as strings is generally the wrong\nthing to do (even if the object stringifies to the canonical form, you don't\nknow the other side of the operator is an object), isn't that asking for bugs?\n If the objects are going to act like strings, shouldn't they act like strings\ncompletely?\n\nObject overloading fails when the encapsulation is incomplete.\n\n\n-- \n151. The proper way to report to my Commander is \"Specialist Schwarz,\n reporting as ordered, Sir\" not \"You can't prove a thing!\"\n -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army\n http://skippyslist.com/list/\n"},{"id":"196211","messageId":"7v1ujsl8ut.fsf@alter.siamese.dyndns.org","threadId":"31120","inReplyTo":"20120730203844.GA23892@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-31T06:53:30Z","receivedAt":"2012-07-31T06:53:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Perhaps we can depend on the URI.pm module? It seems to be\n> widely-available and not be a significant barrier to installation. On\n> the other hand, I don't know its history, either (especially since we're\n> now dealing with SVN changes...).\n>\n> Anyways, I don't like relying on operator overloading, it makes code\n> harder to read and review.\n\nI think code that uses operator overloading, when printed in a\ntextbook, cast in stone and makes the reader aware that it is never\ngoing to change, is indeed \"easy\" to read through. But I suspect\nthat it may be merely giving a false illusion that it is easy to\nreaders.\n\nThe problem is that use of such obscure overloading tends to hurt\nmaintainability. If the initial version Michael produces converts\nall the external strings into instances of CanonicalizedPath class,\naccording to the \"convert as early as possible\" principle, you can\nbe assured that all \"eq\" you see are about the normalized strings\nthe svn library wants to see, and that may allow us sleep safely.\n\nBut the real problem begins six months down the road, when somebody\nwants to add a new codepath that reads a new string from an external\nsource (e.g. perhaps you add a new configuration variable that\nspecifies a path in the svn repository and does something special\nwhen that path is touched by a revision; the exact nature of the new\nfeature does not matter in this discussion). The new code can\nforget to follow the \"convert early\" principle, and pass a bare\nstring read from the configuration around.\n\nA comparison between such a new string and another variable that\nholds path that comes from the existing codepath (i.e. Michael's\ninitial code that perfectly follows the \"convert early\" principle)\nwill still use the overloaded eq in \"$new_str eq $old_path\", thanks\nto the language rule of Perl (namely, even though the new string is\na non object, the other side is still an instance of the class).\n\nWhen the code needs to compare two or more such \"new\" strings (e.g.\nperhaps it wants to remove duplicates from the set of paths it reads\nfrom the configuration), however, \"eq\" silently turns back to a\nsimple string comparison, as \"$new_1 eq $new_2\" will not magically\nturn into \"Canonicalize($new_1)->cmp(Canonicalize($new_2))\".\n\nThis kind of error is unnecessarily hard to catch mostly because the\nprevious \"$new_str eq $old_path\" does work; it masks the problem.\nOverloading of \"eq\" is making it harder to spot new bugs.\n\nIf the code never uses \"eq\" to compare canonicalized paths, and all\nthe surrounding code compare paths using explicit method call on\nobjects, it makes it crystal clear to the readers that paths held in\na bare string is unwelcome in the codepath. It makes it harder to\nadd new code that uses and passes around a bare string by mistake to\nsuch a codepath, I would think.\n"},{"id":"196223","messageId":"5017AB63.6080909@pobox.com","threadId":"31120","inReplyTo":"7v1ujsl8ut.fsf@alter.siamese.dyndns.org","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-31T09:54:43Z","receivedAt":"2012-07-31T09:54:43Z","isPatch":false,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"It just doesn't matter.\n\nWhy are we arguing over which solution will be 4% better two years from now,\nor if my commits are formatted perfectly, when tremendous amounts of basic\nwork to be done improving git-svn? The code is undocumented, lacking unit\ntests, difficult to understand and riddled with bugs.\n\nEither solution would be a vast improvement. The most important thing is that\none of them actually gets done. If both solutions offer a huge improvement,\ndo it the way the person actually writing the code wants to do it. It'll be\nmore enjoyable for them, they'll be more likely to complete the work, and more\nlikely to stick around and code some more.\n"},{"id":"196258","messageId":"20120731200108.GA14462@dcvr.yhbt.net","threadId":"31120","inReplyTo":"5017AB63.6080909@pobox.com","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-07-31T20:01:08Z","receivedAt":"2012-07-31T20:01:08Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Michael G Schwern wrote:\n> It just doesn't matter.\n> \n> Why are we arguing over which solution will be 4% better two years from now,\n> or if my commits are formatted perfectly, when tremendous amounts of basic\n> work to be done improving git-svn? The code is undocumented, lacking unit\n> tests, difficult to understand and riddled with bugs.\n\nYes it does matter.\n\ngit-svn has the problems it has because it traditionally had lower\nreview standards than the rest of git. So yes, we're being more careful\nnowadays about the long-term ramifications of changes.\n\n> Either solution would be a vast improvement. The most important thing is that\n> one of them actually gets done. If both solutions offer a huge improvement,\n> do it the way the person actually writing the code wants to do it. It'll be\n> more enjoyable for them, they'll be more likely to complete the work, and more\n> likely to stick around and code some more.\n\nThe self-obsoleting nature of git-svn makes it hard for anybody to stick\naround. Most of the original contributors (including myself) hardly see\nan SVN repo anymore, so users/contributors forget about it and new ones\ncome along...\n\nI want to make sure things stay consistent with the core parts of git,\nespecially if the Perl were to be replaced with a pure C version.\n"},{"id":"196273","messageId":"7vtxwnh6qq.fsf@alter.siamese.dyndns.org","threadId":"31120","inReplyTo":"20120731200108.GA14462@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-07-31T23:05:01Z","receivedAt":"2012-07-31T23:05:01Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Michael G Schwern wrote:\n>> It just doesn't matter.\n>> \n>> Why are we arguing over which solution will be 4% better two years from now,\n>> or if my commits are formatted perfectly, when tremendous amounts of basic\n>> work to be done improving git-svn? The code is undocumented, lacking unit\n>> tests, difficult to understand and riddled with bugs.\n>\n> Yes it does matter.\n>\n> git-svn has the problems it has because it traditionally had lower\n> review standards than the rest of git. So yes, we're being more careful\n> nowadays about the long-term ramifications of changes.\n\nThanks. I know it takes guts to publicly admit that over time your\nown creation has become less ideal than you wish it to be, but it\nneeded to be said.\n\nMichael, please realize that the only reason people comment on the\npatch series is because they care about what the series brings to\nus. In other words, your effort is appreciated. For a change that\nwe want to have in our codebase, the functionality of the code\nimmediately after the change is applied of course is important, but\nthe maintainability of the result also matters.\n\nWe want to make sure that anybody who wants to understand and\nimprove the system can read the code without distraction from\ninconsistent coding styles used in different sections of code. We\nwant \"git log\" (or \"git log git-svn.perl perl/\") output to tell a\ncoherent story about how the code evolved and why these changes are\nmade in a consistent voice to the readers. We want people to be\nable to \"git log | grep Signed-off-by:\" to count the contributors.\n\nA contributor has enough room to be creative in how his or her code\nis designed. Updating the code to follow the \"convert as early as\npossible\", and (during subsequent discussion with Eric) suggesting\nuse of class instances instead of bare strings to make it harder to\nmistakenly use bare unconverted strings are two examples you already\nshowed creativity in areas that matter.\n\nThere is no need to be creative in ChangeLog and coding styles; it\nonly hurts maintainability.\n\nRegarding the operator overloading of \"eq\" for comparing the\nconverted strings, I still think it will hurt maintainablity (we\nwant to make sure that it is harder, not easier, to make wrong\nchanges to the code in the future), but I may be mistaken and you\nmay have better ideas. If you can use overloading in such a way\nthat it won't harm maintainability and yet makes the resulting code\neasier to read, I don't have any objection.\n\nWhat I won't accept is \"maintainability does not matter\". It does.\n\nThanks.\n"},{"id":"196275","messageId":"5018691A.9050904@pobox.com","threadId":"31120","inReplyTo":"20120731200108.GA14462@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-31T23:24:10Z","receivedAt":"2012-07-31T23:24:10Z","isPatch":false,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"On 2012.7.31 1:01 PM, Eric Wong wrote:\n> Michael G Schwern wrote:\n>> It just doesn't matter.\n>>\n>> Why are we arguing over which solution will be 4% better two years from now,\n>> or if my commits are formatted perfectly, when tremendous amounts of basic\n>> work to be done improving git-svn? The code is undocumented, lacking unit\n>> tests, difficult to understand and riddled with bugs.\n> \n> Yes it does matter.\n> \n> git-svn has the problems it has because it traditionally had lower\n> review standards than the rest of git. So yes, we're being more careful\n> nowadays about the long-term ramifications of changes.\n\nYes, review does matter. And so far we've been arguing over whether reviewing\nobjects-with-overloading or objects-without-overloading would be better. And\nwe can argue about that forever.\n\nThat's the part that doesn't matter. People matter.\n\nI think we can all agree that either solution is a vast improvement along\nmultiple axes, including review. So what really matters is making sure one of\nthem gets done. Once either of them is done, we can see how it works out in\npractice instead of arguing theoretical futures. Once either of them is done,\nit's much easier to switch to the other.\n\nWhat I'm trying to say is I have much less interest in doing it without the\noverloading. It's not interesting to me. It's no fun. No fun means no\npatch. No patch means no improvement. No improvement is the worst of all\npossible options.\n\nI had a lot of enthusiasm for this project when I came in. I like refactoring\nPerl code. I like git. That's all but sunk at how painful and slow and\nnit-picking the process has been. We've barely talked about the content of\nthe patches I've submitted, it's all process. This is no fun.\n\nWe're all volunteers here and we're all getting something personal out of\nthis. Some form of personal enjoyment. I'm not getting that, so I'm unlikely\nto stick around.\n\n\n-- \nDefender of Lexical Encapsulation\n"},{"id":"196276","messageId":"50186A0B.9050707@pobox.com","threadId":"31120","inReplyTo":"7vtxwnh6qq.fsf@alter.siamese.dyndns.org","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Michael G Schwern","fromEmail":"schwern@pobox.com","sentAt":"2012-07-31T23:28:11Z","receivedAt":"2012-07-31T23:28:11Z","isPatch":false,"sender":{"key":"schwern@pobox.com","avatar":"https://avatars.githubusercontent.com/u/25888?v=4"},"body":"On 2012.7.31 4:05 PM, Junio C Hamano wrote:\n> What I won't accept is \"maintainability does not matter\". It does.\n\nI'm sorry, that's not what I intended to convey at all. My reply to Eric lays\nit out more clearly, I think.\n\n\n-- \nReality is that which, when you stop believing in it, doesn't go away.\n -- Phillip K. Dick\n"},{"id":"196305","messageId":"20120801213031.GA10847@dcvr.yhbt.net","threadId":"31120","inReplyTo":"5018691A.9050904@pobox.com","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-01T21:30:31Z","receivedAt":"2012-08-01T21:30:31Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Michael G Schwern wrote:\n> That's the part that doesn't matter. People matter.\n\n> What I'm trying to say is I have much less interest in doing it without the\n> overloading. It's not interesting to me. It's no fun. No fun means no\n> patch. No patch means no improvement. No improvement is the worst of all\n> possible options.\n\nWe want to ensure the code you contribute can be improved by others, not\njust you. I thank you for your changes so far, other developers should\nfind it easier to contribute to git-svn.\n\n> I had a lot of enthusiasm for this project when I came in. I like refactoring\n> Perl code. I like git. That's all but sunk at how painful and slow and\n> nit-picking the process has been. We've barely talked about the content of\n> the patches I've submitted, it's all process. This is no fun.\n\nI haven't found objections to the actual code you've contributed so far.\nI'll be applying your changes once I've had a chance to reread/test\nthem.\n\nYes, we are nitpicky about process, but I think it's important to\nmaintain that consistency given the number of contributors we attract.\n\nI'll also need to review/rewrite some of the Subject: lines so they make\nsense when read in --pretty=oneline/shortlog output. (unless you want to\nvolunteer to resubmit that).\n"},{"id":"196327","messageId":"20120802103122.GA24385@dcvr.yhbt.net","threadId":"31120","inReplyTo":"1343468872-72133-1-git-send-email-schwern@pobox.com","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-02T10:31:22Z","receivedAt":"2012-08-02T10:31:22Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"\"Michael G. Schwern\" wrote:\n> This patch series fixes git-svn for SVN 1.7 tested against SVN 1.7.5 and\n> 1.6.18. Patch 7/8 is where SVN 1.7 starts passing.\n\nThanks Michael. I've made minor editorial changes (mostly rewording\ncommit titles to fit the larger project).\n\nJunio:\n\nThe following changes since commit 05a20c87abd08441c98dfcca0606bc0f8432ab26:\n\n Merge git://github.com/git-l10n/git-po (2012-08-01 15:59:08 -0700)\n\nare available in the git repository at:\n\n\n git://bogomips.org/git-svn master\n\nfor you to fetch changes up to db7c5388b6d843f7cd248dc465af4507d1de7918:\n\n git-svn: remove ad-hoc canonicalizations (2012-08-02 09:42:25 +0000)\n\n----------------------------------------------------------------\nMichael G. Schwern (20):\n Git::SVN: use accessors internally for path\n Git::SVN: use accessor for URLs internally\n Git::SVN::Ra: use accessor for URLs\n use Git::SVN->path accessor globally\n use Git::SVN{,::RA}->url accessor globally\n git-svn: move canonicalization to Git::SVN::Utils\n git-svn: use SVN 1.7 to canonicalize when possible\n git-svn: factor out _collapse_dotdot function\n git-svn: add join_paths() to safely concatenate paths\n Git::SVN::Utils: remove irrelevant comment\n git-svn: path canonicalization uses SVN API\n Git::SVN{,::Ra}: canonicalize earlier\n t9118: workaround inconsistency between SVN versions\n t9107: fix typo\n git-svn: attempt to mimic SVN 1.7 URL canonicalization\n git-svn: replace URL escapes with canonicalization\n git-svn: canonicalize earlier\n git-svn: introduce add_path_to_url function\n git-svn: canonicalize newly-minted URLs\n git-svn: remove ad-hoc canonicalizations\n\n git-svn.perl | 92 ++++++------------\n perl/Git/SVN.pm | 174 ++++++++++++++++++++++------------\n perl/Git/SVN/Fetcher.pm | 2 +-\n perl/Git/SVN/Migration.pm | 6 +-\n perl/Git/SVN/Ra.pm | 92 ++++++++++--------\n perl/Git/SVN/Utils.pm | 173 ++++++++++++++++++++++++++++++++-\n t/Git-SVN/Utils/add_path_to_url.t | 27 ++++++\n t/Git-SVN/Utils/canonicalize_url.t | 26 +++++\n t/Git-SVN/Utils/collapse_dotdot.t | 23 +++++\n t/Git-SVN/Utils/join_paths.t | 32 +++++++\n t/t9107-git-svn-migrate.sh | 6 +-\n t/t9118-git-svn-funky-branch-names.sh | 7 +-\n 12 files changed, 486 insertions(+), 174 deletions(-)\n create mode 100644 t/Git-SVN/Utils/add_path_to_url.t\n create mode 100644 t/Git-SVN/Utils/canonicalize_url.t\n create mode 100644 t/Git-SVN/Utils/collapse_dotdot.t\n create mode 100644 t/Git-SVN/Utils/join_paths.t\n"},{"id":"196362","messageId":"20120802160753.GA17158@copier","threadId":"31120","inReplyTo":"20120802103122.GA24385@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-08-02T16:07:54Z","receivedAt":"2012-08-02T16:07:54Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nEric Wong wrote:\n> \"Michael G. Schwern\" wrote:\n\n>> This patch series fixes git-svn for SVN 1.7 tested against SVN 1.7.5 and\n>> 1.6.18. Patch 7/8 is where SVN 1.7 starts passing.\n>\n> Thanks Michael. I've made minor editorial changes (mostly rewording\n> commit titles to fit the larger project).\n\nThanks from me as well. I'm still worried about whether the increased\nuse of canonicalize_url will introduce regressions for the existing\nSVN 1.6 support, and I should have time to look it over this weekend.\n\nThe comment in canonicalize_url \"There wasn't a 1.6 way to do it\" is\nnot true. The relevant thread on the git list had a little\nconversation about keeping svn 1.4 support, but I'm not sure why\nthat's relevant, given that svn_canonicalize_path has worked largely\nthe same way starting with SVN 1.1 (and on the other hand had\nsignificant changes in SVN 1.7).\n\nHopefully you've looked this over carefully already and I'm worrying\nneedlessly.\n\nHope that helps,\nJonathan\n"},{"id":"196365","messageId":"7vy5lxce9r.fsf@alter.siamese.dyndns.org","threadId":"31120","inReplyTo":"20120802160753.GA17158@copier","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-02T18:58:08Z","receivedAt":"2012-08-02T18:58:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder writes:\n\n> Hi,\n>\n> Eric Wong wrote:\n>> \"Michael G. Schwern\" wrote:\n>\n>>> This patch series fixes git-svn for SVN 1.7 tested against SVN 1.7.5 and\n>>> 1.6.18. Patch 7/8 is where SVN 1.7 starts passing.\n>>\n>> Thanks Michael. I've made minor editorial changes (mostly rewording\n>> commit titles to fit the larger project).\n>\n> Thanks from me as well. I'm still worried about whether the increased\n> use of canonicalize_url will introduce regressions for the existing\n> SVN 1.6 support, and I should have time to look it over this weekend.\n\nLikewise. I'd prefer to see it cook during the feature freeze and\nnot merge to 'master' until post 1.7.12 cycle opens.\n"},{"id":"196366","messageId":"robbat2-20120802T194817-892501136Z@orbis-terrarum.net","threadId":"31120","inReplyTo":"7vy5lxce9r.fsf@alter.siamese.dyndns.org","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Robin H. Johnson","fromEmail":"robbat2@gentoo.org","sentAt":"2012-08-02T19:50:18Z","receivedAt":"2012-08-02T19:50:18Z","isPatch":false,"sender":{"key":"robbat2@gentoo.org","avatar":"https://avatars.githubusercontent.com/u/373898?v=4"},"body":"On Thu, Aug 02, 2012 at 11:58:08AM -0700, Junio C Hamano wrote:\n> > Thanks from me as well. I'm still worried about whether the increased\n> > use of canonicalize_url will introduce regressions for the existing\n> > SVN 1.6 support, and I should have time to look it over this weekend.\n> \n> Likewise. I'd prefer to see it cook during the feature freeze and\n> not merge to 'master' until post 1.7.12 cycle opens.\nI'm going to spin it and include in Gentoo's 1.7.12 packages, as we're\nin need of this explicitly, and this is why we funded Michael to do the\nwork.\n\n-- \nRobin Hugh Johnson\nGentoo Linux: Developer, Trustee & Infrastructure Lead\nE-Mail : robbat2@gentoo.org\nGnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85\n"},{"id":"196369","messageId":"20120802205123.GA14391@dcvr.yhbt.net","threadId":"31120","inReplyTo":"20120802160753.GA17158@copier","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-02T20:51:23Z","receivedAt":"2012-08-02T20:51:23Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jonathan Nieder wrote:\n> Thanks from me as well. I'm still worried about whether the increased\n> use of canonicalize_url will introduce regressions for the existing\n> SVN 1.6 support, and I should have time to look it over this weekend.\n> \n> The comment in canonicalize_url \"There wasn't a 1.6 way to do it\" is\n> not true. The relevant thread on the git list had a little\n> conversation about keeping svn 1.4 support, but I'm not sure why\n> that's relevant, given that svn_canonicalize_path has worked largely\n> the same way starting with SVN 1.1 (and on the other hand had\n> significant changes in SVN 1.7).\n> \n> Hopefully you've looked this over carefully already and I'm worrying\n> needlessly.\n\nThanks for reminding me, I went back to an old chroot 1.4.2 indeed\ndoes fail canonicalization.\n\nWill bisect and squash a fix in.\n"},{"id":"196374","messageId":"7vmx2dc7lw.fsf@alter.siamese.dyndns.org","threadId":"31120","inReplyTo":"20120802205123.GA14391@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-02T21:22:03Z","receivedAt":"2012-08-02T21:22:03Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Jonathan Nieder wrote:\n>> Thanks from me as well. I'm still worried about whether the increased\n>> use of canonicalize_url will introduce regressions for the existing\n>> SVN 1.6 support, and I should have time to look it over this weekend.\n>> \n>> The comment in canonicalize_url \"There wasn't a 1.6 way to do it\" is\n>> not true. The relevant thread on the git list had a little\n>> conversation about keeping svn 1.4 support, but I'm not sure why\n>> that's relevant, given that svn_canonicalize_path has worked largely\n>> the same way starting with SVN 1.1 (and on the other hand had\n>> significant changes in SVN 1.7).\n>> \n>> Hopefully you've looked this over carefully already and I'm worrying\n>> needlessly.\n>\n> Thanks for reminding me, I went back to an old chroot 1.4.2 indeed\n> does fail canonicalization.\n>\n> Will bisect and squash a fix in.\n\nOops; should I eject this out of next and wait for a reroll, then?\n"},{"id":"196376","messageId":"20120802214201.GB24385@dcvr.yhbt.net","threadId":"31120","inReplyTo":"7vmx2dc7lw.fsf@alter.siamese.dyndns.org","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-02T21:42:01Z","receivedAt":"2012-08-02T21:42:01Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Eric Wong writes:\n> > Thanks for reminding me, I went back to an old chroot 1.4.2 indeed\n> > does fail canonicalization.\n> >\n> > Will bisect and squash a fix in.\n> \n> Oops; should I eject this out of next and wait for a reroll, then?\n\nYour call, I doubt anybody on next uses SVN 1.4.2. Rerolling now.\n"},{"id":"196379","messageId":"20120802215544.GA28193@dcvr.yhbt.net","threadId":"31120","inReplyTo":"20120802214201.GB24385@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-02T21:55:44Z","receivedAt":"2012-08-02T21:55:44Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Eric Wong wrote:\n> Junio C Hamano wrote:\n> > Eric Wong writes:\n> > > Thanks for reminding me, I went back to an old chroot 1.4.2 indeed\n> > > does fail canonicalization.\n> > >\n> > > Will bisect and squash a fix in.\n> > \n> > Oops; should I eject this out of next and wait for a reroll, then?\n> \n> Your call, I doubt anybody on next uses SVN 1.4.2. Rerolling now.\n\nOK, rerolled with the patch in\nhttp://mid.gmane.org/20120802215141.GA5284@dcvr.yhbt.net squashed into\n[PATCH 11/20] git-svn: path canonicalization uses SVN API\n\nThe following changes since commit 05a20c87abd08441c98dfcca0606bc0f8432ab26:\n\n Merge git://github.com/git-l10n/git-po (2012-08-01 15:59:08 -0700)\n\nare available in the git repository at:\n\n\n git://bogomips.org/git-svn master\n\nfor you to fetch changes up to 5eaa1fd086e826b1ac8d9346a740527edbdb3c34:\n\n git-svn: remove ad-hoc canonicalizations (2012-08-02 21:46:06 +0000)\n\n----------------------------------------------------------------\nMichael G. Schwern (20):\n Git::SVN: use accessors internally for path\n Git::SVN: use accessor for URLs internally\n Git::SVN::Ra: use accessor for URLs\n use Git::SVN->path accessor globally\n use Git::SVN{,::RA}->url accessor globally\n git-svn: move canonicalization to Git::SVN::Utils\n git-svn: use SVN 1.7 to canonicalize when possible\n git-svn: factor out _collapse_dotdot function\n git-svn: add join_paths() to safely concatenate paths\n Git::SVN::Utils: remove irrelevant comment\n git-svn: path canonicalization uses SVN API\n Git::SVN{,::Ra}: canonicalize earlier\n t9118: workaround inconsistency between SVN versions\n t9107: fix typo\n git-svn: attempt to mimic SVN 1.7 URL canonicalization\n git-svn: replace URL escapes with canonicalization\n git-svn: canonicalize earlier\n git-svn: introduce add_path_to_url function\n git-svn: canonicalize newly-minted URLs\n git-svn: remove ad-hoc canonicalizations\n\n git-svn.perl | 92 ++++++------------\n perl/Git/SVN.pm | 174 +++++++++++++++++++++------------\n perl/Git/SVN/Fetcher.pm | 2 +-\n perl/Git/SVN/Migration.pm | 6 +-\n perl/Git/SVN/Ra.pm | 92 ++++++++++--------\n perl/Git/SVN/Utils.pm | 176 +++++++++++++++++++++++++++++++++-\n t/Git-SVN/Utils/add_path_to_url.t | 27 ++++++\n t/Git-SVN/Utils/canonicalize_url.t | 26 +++++\n t/Git-SVN/Utils/collapse_dotdot.t | 23 +++++\n t/Git-SVN/Utils/join_paths.t | 32 +++++++\n t/t9107-git-svn-migrate.sh | 6 +-\n t/t9118-git-svn-funky-branch-names.sh | 7 +-\n 12 files changed, 489 insertions(+), 174 deletions(-)\n create mode 100644 t/Git-SVN/Utils/add_path_to_url.t\n create mode 100644 t/Git-SVN/Utils/canonicalize_url.t\n create mode 100644 t/Git-SVN/Utils/collapse_dotdot.t\n create mode 100644 t/Git-SVN/Utils/join_paths.t\n"},{"id":"196381","messageId":"7va9ydc5lh.fsf@alter.siamese.dyndns.org","threadId":"31120","inReplyTo":"20120802215544.GA28193@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-02T22:05:30Z","receivedAt":"2012-08-02T22:05:30Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Eric Wong wrote:\n>> Junio C Hamano wrote:\n>> > Eric Wong writes:\n>> > > Thanks for reminding me, I went back to an old chroot 1.4.2 indeed\n>> > > does fail canonicalization.\n>> > >\n>> > > Will bisect and squash a fix in.\n>> > \n>> > Oops; should I eject this out of next and wait for a reroll, then?\n>> \n>> Your call, I doubt anybody on next uses SVN 1.4.2. Rerolling now.\n>\n> OK, rerolled with the patch in\n> http://mid.gmane.org/20120802215141.GA5284@dcvr.yhbt.net squashed into\n> [PATCH 11/20] git-svn: path canonicalization uses SVN API\n>\n> The following changes since commit 05a20c87abd08441c98dfcca0606bc0f8432ab26:\n>\n> Merge git://github.com/git-l10n/git-po (2012-08-01 15:59:08 -0700)\n>\n> are available in the git repository at:\n>\n>\n> git://bogomips.org/git-svn master\n>\n> for you to fetch changes up to 5eaa1fd086e826b1ac8d9346a740527edbdb3c34:\n>\n> git-svn: remove ad-hoc canonicalizations (2012-08-02 21:46:06 +0000)\n\nThanks.\n\nWill park in 'pu' for now, and depending how it goes, I may be\ntempted to merge it down to 'next', but seeing how quickly an issue\nwas found, it is not likely I'd feel it safe enough to merge it to\n'master' before the upcoming release.\n"},{"id":"196382","messageId":"20120802221035.GA9651@dcvr.yhbt.net","threadId":"31120","inReplyTo":"robbat2-20120802T194817-892501136Z@orbis-terrarum.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-02T22:10:35Z","receivedAt":"2012-08-02T22:10:35Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"\"Robin H. Johnson\" wrote:\n> On Thu, Aug 02, 2012 at 11:58:08AM -0700, Junio C Hamano wrote:\n> > > Thanks from me as well. I'm still worried about whether the increased\n> > > use of canonicalize_url will introduce regressions for the existing\n> > > SVN 1.6 support, and I should have time to look it over this weekend.\n> > \n> > Likewise. I'd prefer to see it cook during the feature freeze and\n> > not merge to 'master' until post 1.7.12 cycle opens.\n> \n> I'm going to spin it and include in Gentoo's 1.7.12 packages, as we're\n> in need of this explicitly, and this is why we funded Michael to do the\n> work.\n\nThanks. Btw, have you gotten the chance to report the new SVN 1.7 test\nfailures Michael mentioned to the SVN folks?\n"},{"id":"197491","messageId":"7vehn0gaam.fsf@alter.siamese.dyndns.org","threadId":"31120","inReplyTo":"7vy5lxce9r.fsf@alter.siamese.dyndns.org","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-21T04:04:49Z","receivedAt":"2012-08-21T04:04:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Jonathan Nieder writes:\n>\n>> Hi,\n>>\n>> Eric Wong wrote:\n>>> \"Michael G. Schwern\" wrote:\n>>\n>>>> This patch series fixes git-svn for SVN 1.7 tested against SVN 1.7.5 and\n>>>> 1.6.18. Patch 7/8 is where SVN 1.7 starts passing.\n>>>\n>>> Thanks Michael. I've made minor editorial changes (mostly rewording\n>>> commit titles to fit the larger project).\n>>\n>> Thanks from me as well. I'm still worried about whether the increased\n>> use of canonicalize_url will introduce regressions for the existing\n>> SVN 1.6 support, and I should have time to look it over this weekend.\n>\n> Likewise. I'd prefer to see it cook during the feature freeze and\n> not merge to 'master' until post 1.7.12 cycle opens.\n\nSo we had a chance to cook this late topic outside 'master' during\nthe feature freeze. As you already queued and signed it off, I am\ngoing to fast-track this down to 'master' as promised.\n\nUnless you found a reason not to in the meantime, that is. Is what\nI have on 'pu' still good, or do you (Eric and/or Michael) have any\nupdates you'd rather have me pull instead?\n\nThanks.\n"},{"id":"197554","messageId":"20120821210352.GA13200@dcvr.yhbt.net","threadId":"31120","inReplyTo":"7vehn0gaam.fsf@alter.siamese.dyndns.org","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-08-21T21:03:52Z","receivedAt":"2012-08-21T21:03:52Z","isPatch":false,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> Unless you found a reason not to in the meantime, that is. Is what\n> I have on 'pu' still good, or do you (Eric and/or Michael) have any\n> updates you'd rather have me pull instead?\n\nNo updates, everything is still good.\n"},{"id":"197556","messageId":"7vk3wsc4kf.fsf@alter.siamese.dyndns.org","threadId":"31120","inReplyTo":"20120821210352.GA13200@dcvr.yhbt.net","subject":"Re: Fix git-svn for SVN 1.7","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-08-21T21:34:24Z","receivedAt":"2012-08-21T21:34:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Junio C Hamano wrote:\n>> Unless you found a reason not to in the meantime, that is. Is what\n>> I have on 'pu' still good, or do you (Eric and/or Michael) have any\n>> updates you'd rather have me pull instead?\n>\n> No updates, everything is still good.\n\nThanks.\n"},{"id":"200690","messageId":"20121006192455.GA14969@elie.Belkin","threadId":"31120","inReplyTo":"1343468872-72133-8-git-send-email-schwern@pobox.com","subject":"[PATCH/RFC] test: work around SVN 1.7 mishandling of svn:special changes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-06T19:24:56Z","receivedAt":"2012-10-06T19:24:56Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Subversion represents symlinks as ordinary files with content\nstarting with \"link \" and the svn:special property set to \"*\". Thus a\nfile can switch between being a symlink and a non-symlink simply by\ntoggling its svn:special property, and new checkouts will\nautomatically write a file of the appropriate type. Likewise, in\nsubversion 1.6 and older, running \"svn update\" would notice changes\nin filetype and update the working copy appropriately.\n\nUnfortunately, starting in subversion 1.7 ,changes to the svn:special\nproperty trip an assertion instead:\n\n\t$ svn up svn-tree\n\tUpdating 'svn-tree':\n\tsvn: E235000: In file 'subversion/libsvn_wc/update_editor.c' \\\n\tline 1583: assertion failed (action == svn_wc_conflict_action_edit \\\n\t|| action == svn_wc_conflict_action_delete || action == \\\n\tsvn_wc_conflict_action_replace)\n\nThis is a known bug in \"svn update\" (Subversion issue 4091) and for\nthe sake of old repositories it will need to be fixed some day.\n\nRevisions prepared with ordinary svn commands (\"svn add\" and not \"svn\npropset\") don't trip this because they represent filetype changes\nusing a replace operation, which is approximately equivalent to\nremoval followed by adding a new file and works fine. Perhaps \"git\nsvn\" should mimic that, but for now let's teach the test suite to\nrecover from the bug by testing the content of HEAD with a new\ncheckout.\n\nAfter this change, tests t9100.11-13 pass again.\n\nSigned-off-by: Jonathan Nieder \n---\nHi Eric,\n\nMichael G. Schwern wrote:\n\n> At this point SVN 1.7 passes except for 3 tests in\n> t9100-git-svn-basic.sh that look like an SVN bug to do with\n> symlinks.\n\nHow about this patch?\n\nI didn't add a new xfail test for \"svn up\" working because I'm not yet\nsure what good git-svn behavior would be. Probably it would be better\nto track down that svn bug and get a fix backported to the 1.7.x\nbranch.\n\nReference: http://subversion.tigris.org/issues/show_bug.cgi?id=4160\n\n t/t9100-git-svn-basic.sh | 13 +++++++++++--\n 1 file changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t9100-git-svn-basic.sh b/t/t9100-git-svn-basic.sh\nindex 749b75e8..34d3485f 100755\n--- a/t/t9100-git-svn-basic.sh\n+++ b/t/t9100-git-svn-basic.sh\n@@ -19,6 +19,15 @@ case \"$GIT_SVN_LC_ALL\" in\n \t;;\n esac\n \n+svn_up_avoiding_issue4091 () {\n+\tif ! svn_cmd_up \"$SVN_TREE\"\n+\tthen\n+\t\t# work around Subversion issue 4091\n+\t\trm -r \"$SVN_TREE\" &&\n+\t\tsvn_cmd checkout \"$svnrepo\" \"$SVN_TREE\"\n+\tfi\n+}\n+\n test_expect_success \\\n 'initialize git svn' '\n \tmkdir import &&\n@@ -148,7 +157,7 @@ test_expect_success \"$name\" '\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+\tsvn_up_avoiding_issue4091 &&\n \ttest -h \"$SVN_TREE\"/exec.sh'\n \n name='new symlink is added to a file that was also just made executable'\n@@ -173,7 +182,7 @@ test_expect_success \"$name\" '\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+\tsvn_up_avoiding_issue4091 &&\n \ttest -f \"$SVN_TREE\"/exec-2.sh &&\n \ttest ! -h \"$SVN_TREE\"/exec-2.sh &&\n \ttest_cmp help \"$SVN_TREE\"/exec-2.sh'\n-- \n1.7.10.4\n"},{"id":"200824","messageId":"20121009084145.GA19784@elie.Belkin","threadId":"31120","inReplyTo":"50143E34.8090802@pobox.com","subject":"[PATCH/RFC] svn test: escape peg revision separator using empty peg rev","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-09T08:41:45Z","receivedAt":"2012-10-09T08:41:45Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This test script uses \"svn cp\" to create a branch with an @-sign in\nits name:\n\n\tsvn cp \"pr ject/trunk\" \"pr ject/branches/not-a@{0}reflog\"\n\nThat sets up for later tests that fetch the branch and check that git\nsvn mangles the refname appropriately.\n\nUnfortunately, modern svn versions interpret path arguments with an\n@-sign as an example of path@revision syntax (which pegs a path to a\nparticular revision) and truncate the path or error out with message\n\"svn: E205000: Syntax error parsing peg revision '{0}reflog'\".\n\nWhen using subversion 1.6.x, escaping the @ sign as %40 avoids trouble\n(see 08fd28bb, 2010-07-08). Newer versions are stricter:\n\n\t$ svn cp \"$repo/pr ject/trunk\" \"$repo/pr ject/branches/not-a%40{reflog}\"\n\tsvn: E205000: Syntax error parsing peg revision '%7B0%7Dreflog'\n\nThe recommended method for escaping a literal @ sign in a path passed\nto subversion is to add an empty peg revision at the end of the path\n(\"branches/not-a@{0}reflog@\"). Do that.\n\nPre-1.6.12 versions of Subversion probably treat the trailing @ as\nanother literal @-sign (svn issue 3651). Luckily ever since\nv1.8.0-rc0~155^2~7 (t9118: workaround inconsistency between SVN\nversions, 2012-07-28) the test can survive that.\n\nTested with Debian Subversion 1.6.12dfsg-6 and 1.7.5-1 and r1395837\nof Subversion trunk (1.8.x).\n\nSigned-off-by: Jonathan Nieder \n---\nMichael G Schwern wrote:\n> On 2012.7.28 7:16 AM, Jonathan Nieder wrote:\n>> Michael G. Schwern wrote:\n\n>>> -\t\tgit rev-parse \"refs/remotes/not-a%40{0}reflog\"\n>>> +\t\tgit rev-parse \"refs/remotes/$non_reflog\"\n>>\n>> Doesn't this defeat the point of the testcase (checking that git-svn\n>> is able to avoid creating git refs containing @{, following the rules\n>> from git-check-ref-format(1))?\n>\n> Unless I messed up, entirely possible as I'm not a shell programmer, the test\n> is still useful for testing SVN 1.6. Under SVN 1.6 $non_reflog should be\n> 'not-a%40{0}reflog' as before.\n\nHere's a patch to make the test useful again for SVN 1.7. Sensible?\n\n t/t9118-git-svn-funky-branch-names.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t9118-git-svn-funky-branch-names.sh b/t/t9118-git-svn-funky-branch-names.sh\nindex 193d3cab..15f93b4c 100755\n--- a/t/t9118-git-svn-funky-branch-names.sh\n+++ b/t/t9118-git-svn-funky-branch-names.sh\n@@ -28,7 +28,7 @@ test_expect_success 'setup svnrepo' '\n \tsvn_cmd cp -m \"trailing .lock\" \"$svnrepo/pr ject/trunk\" \\\n \t\t\t\"$svnrepo/pr ject/branches/trailing_dotlock.lock\" &&\n \tsvn_cmd cp -m \"reflog\" \"$svnrepo/pr ject/trunk\" \\\n-\t\t\t\"$svnrepo/pr ject/branches/not-a%40{0}reflog\" &&\n+\t\t\t\"$svnrepo/pr ject/branches/not-a@{0}reflog@\" &&\n \tstart_httpd\n \t'\n \n-- \n1.7.10.4\n"},{"id":"200829","messageId":"5073F2C0.6000504@drmicha.warpmail.net","threadId":"31120","inReplyTo":"20121009084145.GA19784@elie.Belkin","subject":"Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2012-10-09T09:47:44Z","receivedAt":"2012-10-09T09:47:44Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jonathan Nieder venit, vidit, dixit 09.10.2012 10:41:\n> This test script uses \"svn cp\" to create a branch with an @-sign in\n> its name:\n> \n> \tsvn cp \"pr ject/trunk\" \"pr ject/branches/not-a@{0}reflog\"\n> \n> That sets up for later tests that fetch the branch and check that git\n> svn mangles the refname appropriately.\n> \n> Unfortunately, modern svn versions interpret path arguments with an\n> @-sign as an example of path@revision syntax (which pegs a path to a\n> particular revision) and truncate the path or error out with message\n> \"svn: E205000: Syntax error parsing peg revision '{0}reflog'\".\n> \n> When using subversion 1.6.x, escaping the @ sign as %40 avoids trouble\n> (see 08fd28bb, 2010-07-08). Newer versions are stricter:\n> \n> \t$ svn cp \"$repo/pr ject/trunk\" \"$repo/pr ject/branches/not-a%40{reflog}\"\n> \tsvn: E205000: Syntax error parsing peg revision '%7B0%7Dreflog'\n> \n> The recommended method for escaping a literal @ sign in a path passed\n> to subversion is to add an empty peg revision at the end of the path\n> (\"branches/not-a@{0}reflog@\"). Do that.\n> \n> Pre-1.6.12 versions of Subversion probably treat the trailing @ as\n> another literal @-sign (svn issue 3651). Luckily ever since\n> v1.8.0-rc0~155^2~7 (t9118: workaround inconsistency between SVN\n> versions, 2012-07-28) the test can survive that.\n> \n> Tested with Debian Subversion 1.6.12dfsg-6 and 1.7.5-1 and r1395837\n> of Subversion trunk (1.8.x).\n> \n> Signed-off-by: Jonathan Nieder \n\nTested with Subversion 1.6.18.\n\n> ---\n> Michael G Schwern wrote:\n>> On 2012.7.28 7:16 AM, Jonathan Nieder wrote:\n>>> Michael G. Schwern wrote:\n> \n>>>> -\t\tgit rev-parse \"refs/remotes/not-a%40{0}reflog\"\n>>>> +\t\tgit rev-parse \"refs/remotes/$non_reflog\"\n>>>\n>>> Doesn't this defeat the point of the testcase (checking that git-svn\n>>> is able to avoid creating git refs containing @{, following the rules\n>>> from git-check-ref-format(1))?\n>>\n>> Unless I messed up, entirely possible as I'm not a shell programmer, the test\n>> is still useful for testing SVN 1.6. Under SVN 1.6 $non_reflog should be\n>> 'not-a%40{0}reflog' as before.\n> \n> Here's a patch to make the test useful again for SVN 1.7. Sensible?\n> \n> t/t9118-git-svn-funky-branch-names.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t9118-git-svn-funky-branch-names.sh b/t/t9118-git-svn-funky-branch-names.sh\n> index 193d3cab..15f93b4c 100755\n> --- a/t/t9118-git-svn-funky-branch-names.sh\n> +++ b/t/t9118-git-svn-funky-branch-names.sh\n> @@ -28,7 +28,7 @@ test_expect_success 'setup svnrepo' '\n> \tsvn_cmd cp -m \"trailing .lock\" \"$svnrepo/pr ject/trunk\" \\\n> \t\t\t\"$svnrepo/pr ject/branches/trailing_dotlock.lock\" &&\n> \tsvn_cmd cp -m \"reflog\" \"$svnrepo/pr ject/trunk\" \\\n> -\t\t\t\"$svnrepo/pr ject/branches/not-a%40{0}reflog\" &&\n> +\t\t\t\"$svnrepo/pr ject/branches/not-a@{0}reflog@\" &&\n> \tstart_httpd\n> \t'\n\nI haven't checked other svn versions but this approach looks perfectly\nsensible. It makes us test branch names which can't even be created\neasily with current svn. Does svn really deserve this much attention? ;)\n\nSeriously, our tests prepare us well for an svn remote helper...\n"},{"id":"200830","messageId":"20121009101239.GA28120@elie.Belkin","threadId":"31120","inReplyTo":"20121006192455.GA14969@elie.Belkin","subject":"[PATCH/RFC v2] git svn: work around SVN 1.7 mishandling of svn:special changes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-09T10:12:39Z","receivedAt":"2012-10-09T10:12:39Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Subversion represents symlinks as ordinary files with content starting\nwith \"link \" and the svn:special property set to \"*\". Thus a file can\nswitch between being a symlink and a non-symlink simply by toggling\nits svn:special property, and new checkouts will automatically write a\nfile of the appropriate type. Likewise, in subversion 1.6 and older,\nrunning \"svn update\" would notice changes in filetype and update the\nworking copy appropriately.\n\nStarting in subversion 1.7 (issue 4091), changes to the svn:special\nproperty trip an assertion instead:\n\n\t$ svn up svn-tree\n\tUpdating 'svn-tree':\n\tsvn: E235000: In file 'subversion/libsvn_wc/update_editor.c' \\\n\tline 1583: assertion failed (action == svn_wc_conflict_action_edit \\\n\t|| action == svn_wc_conflict_action_delete || action == \\\n\tsvn_wc_conflict_action_replace)\n\nRevisions prepared with ordinary svn commands (\"svn add\" and not \"svn\npropset\") don't trip this because they represent these filetype\nchanges using a replace operation, which is approximately equivalent\nto removal followed by adding a new file and works fine. Follow suit.\n\nNoticed using t9100. After this change, git-svn's file-to-symlink\nchanges are sent in a format that modern \"svn update\" can handle and\ntests t9100.11-13 pass again.\n\nSigned-off-by: Jonathan Nieder \n---\nJonathan Nieder wrote:\n\n> Revisions prepared with ordinary svn commands (\"svn add\" and not \"svn\n> propset\") don't trip this because they represent filetype changes\n> using a replace operation [...]\n> Perhaps \"git\n> svn\" should mimic that,\n\n... and here's what that looks like. I like this more than ignoring\nthe problem in tests, and I suppose something like this is necessary\nregardless of how quickly issue 4091 is fixed for compatibility with\nthe broken versions of svn.\n\nIt would be nice if the patch could be more concise, though. What do\nyou think?\n\n git-svn.perl | 25 ++++++++++++++++++++++++-\n 1 file changed, 24 insertions(+), 1 deletion(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 9d57aa0c..40ccdd50 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -5439,7 +5439,30 @@ sub M {\n \t$self->close_file($fbat,undef,$self->{pool});\n }\n \n-sub T { shift->M(@_) }\n+sub T {\n+\tmy ($self, $m, $deletions) = @_;\n+\n+\t# Work around subversion issue 4091: toggling the \"is a\n+\t# symlink\" property requires removing and re-adding a\n+\t# file or else \"svn up\" on affected clients trips an\n+\t# assertion and aborts.\n+\tif (($m->{mode_b} =~ /^120/ && $m->{mode_a} !~ /^120/) ||\n+\t ($m->{mode_b} !~ /^120/ && $m->{mode_a} =~ /^120/)) {\n+\t\t$self->D({\n+\t\t\tmode_a => $m->{mode_a}, mode_b => '000000',\n+\t\t\tsha1_a => $m->{sha1_a}, sha1_b => '0' x 40,\n+\t\t\tchg => 'D', file_b => $m->{file_b}\n+\t\t});\n+\t\t$self->A({\n+\t\t\tmode_a => '000000', mode_b => $m->{mode_b},\n+\t\t\tsha1_a => '0' x 40, sha1_b => $m->{sha1_b},\n+\t\t\tchg => 'A', file_b => $m->{file_b}\n+\t\t});\n+\t\treturn;\n+\t}\n+\n+\t$self->M($m, $deletions);\n+}\n \n sub change_file_prop {\n \tmy ($self, $fbat, $pname, $pval) = @_;\n-- \n1.7.10.4\n"},{"id":"200831","messageId":"20121009101953.GB28120@elie.Belkin","threadId":"31120","inReplyTo":"5073F2C0.6000504@drmicha.warpmail.net","subject":"Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-09T10:19:53Z","receivedAt":"2012-10-09T10:19:53Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Michael J Gruber wrote:\n> Jonathan Nieder venit, vidit, dixit 09.10.2012 10:41:\n\n>> Signed-off-by: Jonathan Nieder \n>\n> Tested with Subversion 1.6.18.\n[...]\n> I haven't checked other svn versions but this approach looks perfectly\n> sensible. It makes us test branch names which can't even be created\n> easily with current svn. Does svn really deserve this much attention? ;)\n\nThanks for the quick and thorough feedback. I'm glad to hear it seems\nsane. ;-)\n\n> Seriously, our tests prepare us well for an svn remote helper...\n\nThat might be a good reason to make a mock implementation of the\nexisting git-svn interface on top of git-remote-svn. Sounds fun but\nhard.\n\nCiao,\nJonathan\n"},{"id":"200949","messageId":"20121010201125.GA30952@dcvr.yhbt.net","threadId":"31120","inReplyTo":"20121009101239.GA28120@elie.Belkin","subject":"Re: [PATCH/RFC v2] git svn: work around SVN 1.7 mishandling of svn:special changes","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-10-10T20:11:25Z","receivedAt":"2012-10-10T20:11:25Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jonathan Nieder wrote:\n> Jonathan Nieder wrote:\n> \n> > Revisions prepared with ordinary svn commands (\"svn add\" and not \"svn\n> > propset\") don't trip this because they represent filetype changes\n> > using a replace operation [...]\n> > Perhaps \"git\n> > svn\" should mimic that,\n> \n> ... and here's what that looks like. I like this more than ignoring\n> the problem in tests, and I suppose something like this is necessary\n> regardless of how quickly issue 4091 is fixed for compatibility with\n> the broken versions of svn.\n\nI prefer this v2 more than ignoring the problem in tests, too.\n\n> It would be nice if the patch could be more concise, though. What do\n> you think?\n\nI think it's fine.\n\n> git-svn.perl | 25 ++++++++++++++++++++++++-\n\nI needed to filter the patch through:\n\n s,git-svn\\.perl,perl/Git/SVN/Editor.pm,g\n\nthough... Will push the edited version to my master on\ngit://bogomips.org/git-svn\n\n>From b8c78e2a9d6141589202e98b898f477861fcb111 Mon Sep 17 00:00:00 2001\nFrom: Jonathan Nieder \nDate: Tue, 9 Oct 2012 03:12:39 -0700\nSubject: [PATCH] git svn: work around SVN 1.7 mishandling of svn:special\n changes\n\nSubversion represents symlinks as ordinary files with content starting\nwith \"link \" and the svn:special property set to \"*\". Thus a file can\nswitch between being a symlink and a non-symlink simply by toggling\nits svn:special property, and new checkouts will automatically write a\nfile of the appropriate type. Likewise, in subversion 1.6 and older,\nrunning \"svn update\" would notice changes in filetype and update the\nworking copy appropriately.\n\nStarting in subversion 1.7 (issue 4091), changes to the svn:special\nproperty trip an assertion instead:\n\n\t$ svn up svn-tree\n\tUpdating 'svn-tree':\n\tsvn: E235000: In file 'subversion/libsvn_wc/update_editor.c' \\\n\tline 1583: assertion failed (action == svn_wc_conflict_action_edit \\\n\t|| action == svn_wc_conflict_action_delete || action == \\\n\tsvn_wc_conflict_action_replace)\n\nRevisions prepared with ordinary svn commands (\"svn add\" and not \"svn\npropset\") don't trip this because they represent these filetype\nchanges using a replace operation, which is approximately equivalent\nto removal followed by adding a new file and works fine. Follow suit.\n\nNoticed using t9100. After this change, git-svn's file-to-symlink\nchanges are sent in a format that modern \"svn update\" can handle and\ntests t9100.11-13 pass again.\n\n[ew: s,git-svn\\.perl,perl/Git/SVN/Editor.pm,g]\n\nSigned-off-by: Jonathan Nieder \nSigned-off-by: Eric Wong \n---\n perl/Git/SVN/Editor.pm | 25 ++++++++++++++++++++++++-\n 1 file changed, 24 insertions(+), 1 deletion(-)\n\ndiff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm\nindex 755092f..3bbc20a 100644\n--- a/perl/Git/SVN/Editor.pm\n+++ b/perl/Git/SVN/Editor.pm\n@@ -345,7 +345,30 @@ sub M {\n \t$self->close_file($fbat,undef,$self->{pool});\n }\n \n-sub T { shift->M(@_) }\n+sub T {\n+\tmy ($self, $m, $deletions) = @_;\n+\n+\t# Work around subversion issue 4091: toggling the \"is a\n+\t# symlink\" property requires removing and re-adding a\n+\t# file or else \"svn up\" on affected clients trips an\n+\t# assertion and aborts.\n+\tif (($m->{mode_b} =~ /^120/ && $m->{mode_a} !~ /^120/) ||\n+\t ($m->{mode_b} !~ /^120/ && $m->{mode_a} =~ /^120/)) {\n+\t\t$self->D({\n+\t\t\tmode_a => $m->{mode_a}, mode_b => '000000',\n+\t\t\tsha1_a => $m->{sha1_a}, sha1_b => '0' x 40,\n+\t\t\tchg => 'D', file_b => $m->{file_b}\n+\t\t});\n+\t\t$self->A({\n+\t\t\tmode_a => '000000', mode_b => $m->{mode_b},\n+\t\t\tsha1_a => '0' x 40, sha1_b => $m->{sha1_b},\n+\t\t\tchg => 'A', file_b => $m->{file_b}\n+\t\t});\n+\t\treturn;\n+\t}\n+\n+\t$self->M($m, $deletions);\n+}\n \n sub change_file_prop {\n \tmy ($self, $fbat, $pname, $pval) = @_;\n-- \n1.8.0.rc0.42.gb8c78e2\n"},{"id":"200950","messageId":"20121010203730.GA19115@dcvr.yhbt.net","threadId":"31120","inReplyTo":"20121009101953.GB28120@elie.Belkin","subject":"Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-10-10T20:37:30Z","receivedAt":"2012-10-10T20:37:30Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jonathan Nieder wrote:\n> Michael J Gruber wrote:\n> > Jonathan Nieder venit, vidit, dixit 09.10.2012 10:41:\n> \n> >> Signed-off-by: Jonathan Nieder \n> >\n> > Tested with Subversion 1.6.18.\n\nThanks both. Also pushed to \"master\" on git://bogomips.org/git-svn.git\n(commit 44bc5ac71fd99f195bf1a3bea63c11139d2d535f)\n\nJonathan Nieder (2):\n git svn: work around SVN 1.7 mishandling of svn:special changes\n svn test: escape peg revision separator using empty peg rev\n"},{"id":"200952","messageId":"20121010204733.GA4517@elie.Belkin","threadId":"31120","inReplyTo":"20121010201125.GA30952@dcvr.yhbt.net","subject":"[PATCH v3] git svn: work around SVN 1.7 mishandling of svn:special changes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-10T20:47:33Z","receivedAt":"2012-10-10T20:47:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Subversion represents symlinks as ordinary files with content starting\nwith \"link \" and the svn:special property set to \"*\". Thus a file can\nswitch between being a symlink and a non-symlink simply by toggling\nits svn:special property, and new checkouts will automatically write a\nfile of the appropriate type. Likewise, in subversion 1.6 and older,\nrunning \"svn update\" would notice changes in filetype and update the\nworking copy appropriately.\n\nStarting in subversion 1.7 (issue 4091), changes to the svn:special\nproperty trip an assertion instead:\n\n\t$ svn up svn-tree\n\tUpdating 'svn-tree':\n\tsvn: E235000: In file 'subversion/libsvn_wc/update_editor.c' \\\n\tline 1583: assertion failed (action == svn_wc_conflict_action_edit \\\n\t|| action == svn_wc_conflict_action_delete || action == \\\n\tsvn_wc_conflict_action_replace)\n\nRevisions prepared with ordinary svn commands (\"svn add\" and not \"svn\npropset\") don't trip this because they represent these filetype\nchanges using a replace operation, which is approximately equivalent\nto removal followed by adding a new file and works fine. Follow suit.\n\nNoticed using t9100. After this change, git-svn's file-to-symlink\nchanges are sent in a format that modern \"svn update\" can handle and\ntests t9100.11-13 pass again.\n\nSigned-off-by: Jonathan Nieder \n---\nEric Wong wrote:\n\n> I needed to filter the patch through:\n>\n> s,git-svn\\.perl,perl/Git/SVN/Editor.pm,g\n>\n> though...\n\nYeah, good catch. Here's a v3 tested against \"master\". Unlike in v2,\nit remembers to pass the $deletions parameter to D() and A() --- which\nshouldn't make a difference because we are not adding a directory, but\nit's nice to be consistent to make reading smoother.\n\n perl/Git/SVN/Editor.pm | 25 ++++++++++++++++++++++++-\n 1 file changed, 24 insertions(+), 1 deletion(-)\n\ndiff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm\nindex 755092fd..178920c8 100644\n--- a/perl/Git/SVN/Editor.pm\n+++ b/perl/Git/SVN/Editor.pm\n@@ -345,7 +345,30 @@ sub M {\n \t$self->close_file($fbat,undef,$self->{pool});\n }\n \n-sub T { shift->M(@_) }\n+sub T {\n+\tmy ($self, $m, $deletions) = @_;\n+\n+\t# Work around subversion issue 4091: toggling the \"is a\n+\t# symlink\" property requires removing and re-adding a\n+\t# file or else \"svn up\" on affected clients trips an\n+\t# assertion and aborts.\n+\tif (($m->{mode_b} =~ /^120/ && $m->{mode_a} !~ /^120/) ||\n+\t ($m->{mode_b} !~ /^120/ && $m->{mode_a} =~ /^120/)) {\n+\t\t$self->D({\n+\t\t\tmode_a => $m->{mode_a}, mode_b => '000000',\n+\t\t\tsha1_a => $m->{sha1_a}, sha1_b => '0' x 40,\n+\t\t\tchg => 'D', file_b => $m->{file_b}\n+\t\t}, $deletions);\n+\t\t$self->A({\n+\t\t\tmode_a => '000000', mode_b => $m->{mode_b},\n+\t\t\tsha1_a => '0' x 40, sha1_b => $m->{sha1_b},\n+\t\t\tchg => 'A', file_b => $m->{file_b}\n+\t\t}, $deletions);\n+\t\treturn;\n+\t}\n+\n+\t$self->M($m, $deletions);\n+}\n \n sub change_file_prop {\n \tmy ($self, $fbat, $pname, $pval) = @_;\n-- \n1.7.10.4\n"},{"id":"200953","messageId":"20121010210218.GB4517@elie.Belkin","threadId":"31120","inReplyTo":"20121010203730.GA19115@dcvr.yhbt.net","subject":"Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-10T21:02:18Z","receivedAt":"2012-10-10T21:02:18Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Eric Wong wrote:\n\n> Thanks both. Also pushed to \"master\" on git://bogomips.org/git-svn.git\n> (commit 44bc5ac71fd99f195bf1a3bea63c11139d2d535f)\n>\n> Jonathan Nieder (2):\n> git svn: work around SVN 1.7 mishandling of svn:special changes\n> svn test: escape peg revision separator using empty peg rev\n\nThanks. Here's the $deletions nit as a patch on top.\n\n-- >8 --\nSubject: Git::SVN::Editor::T: pass $deletions to ->A and ->D\n\nThis shouldn't make a difference because the $deletions hash is\nonly used when adding a directory (see 379862ec, 2012-02-20) but\nit's nice to be consistent to make reading smoother anyway. No\nfunctional change intended.\n\nSigned-off-by: Jonathan Nieder \n---\n perl/Git/SVN/Editor.pm | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/perl/Git/SVN/Editor.pm b/perl/Git/SVN/Editor.pm\nindex 3bbc20a0..178920c8 100644\n--- a/perl/Git/SVN/Editor.pm\n+++ b/perl/Git/SVN/Editor.pm\n@@ -358,12 +358,12 @@ sub T {\n \t\t\tmode_a => $m->{mode_a}, mode_b => '000000',\n \t\t\tsha1_a => $m->{sha1_a}, sha1_b => '0' x 40,\n \t\t\tchg => 'D', file_b => $m->{file_b}\n-\t\t});\n+\t\t}, $deletions);\n \t\t$self->A({\n \t\t\tmode_a => '000000', mode_b => $m->{mode_b},\n \t\t\tsha1_a => '0' x 40, sha1_b => $m->{sha1_b},\n \t\t\tchg => 'A', file_b => $m->{file_b}\n-\t\t});\n+\t\t}, $deletions);\n \t\treturn;\n \t}\n \n-- \n1.7.10.4\n"},{"id":"200955","messageId":"20121010213120.GA12935@dcvr.yhbt.net","threadId":"31120","inReplyTo":"20121010210218.GB4517@elie.Belkin","subject":"Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-10-10T21:31:20Z","receivedAt":"2012-10-10T21:31:20Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jonathan Nieder wrote:\n> Eric Wong wrote:\n> \n> > Thanks both. Also pushed to \"master\" on git://bogomips.org/git-svn.git\n> > (commit 44bc5ac71fd99f195bf1a3bea63c11139d2d535f)\n> >\n> > Jonathan Nieder (2):\n> > git svn: work around SVN 1.7 mishandling of svn:special changes\n> > svn test: escape peg revision separator using empty peg rev\n> \n> Thanks. Here's the $deletions nit as a patch on top.\n> \n> -- >8 --\n> Subject: Git::SVN::Editor::T: pass $deletions to ->A and ->D\n\nFor future reference, it'd be slightly easier for me to apply if you\nincluded the From: (and Date:) headers so I don't have to yank+paste\nthem myself :>\n\n> This shouldn't make a difference because the $deletions hash is\n> only used when adding a directory (see 379862ec, 2012-02-20) but\n> it's nice to be consistent to make reading smoother anyway. No\n> functional change intended.\n> \n> Signed-off-by: Jonathan Nieder \n\nSigned-off-by: Eric Wong \n\nAnd pushed to master on git://bogomips.org/git-svn.git\n(commit a9608896587718549e82c5bae11740f2c0eac4c6)\n\nJonathan Nieder (3):\n git svn: work around SVN 1.7 mishandling of svn:special changes\n svn test: escape peg revision separator using empty peg rev\n Git::SVN::Editor::T: pass $deletions to ->A and ->D\n"},{"id":"200957","messageId":"20121010214205.GD4517@elie.Belkin","threadId":"31120","inReplyTo":"20121010213120.GA12935@dcvr.yhbt.net","subject":"Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2012-10-10T21:42:05Z","receivedAt":"2012-10-10T21:42:05Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Eric Wong wrote:\n> Jonathan Nieder wrote:\n\n>> -- >8 --\n>> Subject: Git::SVN::Editor::T: pass $deletions to ->A and ->D\n>\n> For future reference, it'd be slightly easier for me to apply if you\n> included the From: (and Date:) headers so I don't have to yank+paste\n> them myself :>\n\nAh, I assumed you were using \"git am --scissors\". Will do next time.\n\nJonathan\n"},{"id":"200960","messageId":"20121010221613.GA14466@dcvr.yhbt.net","threadId":"31120","inReplyTo":"20121010214205.GD4517@elie.Belkin","subject":"Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2012-10-10T22:16:13Z","receivedAt":"2012-10-10T22:16:13Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Jonathan Nieder wrote:\n> Eric Wong wrote:\n> > Jonathan Nieder wrote:\n> \n> >> -- >8 --\n> >> Subject: Git::SVN::Editor::T: pass $deletions to ->A and ->D\n> >\n> > For future reference, it'd be slightly easier for me to apply if you\n> > included the From: (and Date:) headers so I don't have to yank+paste\n> > them myself :>\n> \n> Ah, I assumed you were using \"git am --scissors\". Will do next time.\n\nI missed the addition of --scissors. Will use it in the future :>\n"},{"id":"200962","messageId":"7vpq4q9cut.fsf@alter.siamese.dyndns.org","threadId":"31120","inReplyTo":"20121010203730.GA19115@dcvr.yhbt.net","subject":"Re: [PATCH/RFC] svn test: escape peg revision separator using empty peg rev","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-10T22:33:14Z","receivedAt":"2012-10-10T22:33:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> Jonathan Nieder wrote:\n>> Michael J Gruber wrote:\n>> > Jonathan Nieder venit, vidit, dixit 09.10.2012 10:41:\n>> \n>> >> Signed-off-by: Jonathan Nieder \n>> >\n>> > Tested with Subversion 1.6.18.\n>\n> Thanks both. Also pushed to \"master\" on git://bogomips.org/git-svn.git\n> (commit 44bc5ac71fd99f195bf1a3bea63c11139d2d535f)\n>\n> Jonathan Nieder (2):\n> git svn: work around SVN 1.7 mishandling of svn:special changes\n> svn test: escape peg revision separator using empty peg rev\n\nThanks; pulled.\n"}]}