Re: [PATCH] Fix merge parent checking with svn.pushmergeinfo.
On Fri, Sep 15, 2017 at 5:53 PM, Jonathan Nieder <jrnie...@gmail.com> wrote: > Jason Merrill wrote: >> On Fri, Sep 15, 2017 at 1:52 PM, Jonathan Nieder <jrnie...@gmail.com> wrote: >> > Jason Merrill wrote: > >>>> Subject: Fix merge parent checking with svn.pushmergeinfo. >>>> >>>> Without this fix, svn dcommit of a merge with svn.pushmergeinfo set would >>>> get error messages like "merge parent for is on branch >>>> svn+ssh://gcc.gnu.org/svn/gcc/trunk, which is not under the git-svn root >>>> svn+ssh://ja...@gcc.gnu.org/svn/gcc!" >>>> >>>> * git-svn.perl: Remove username from rooturl before comparing to branchurl. >>>> >>>> Signed-off-by: Jason Merrill <ja...@redhat.com> >>> >>> Interesting. Thanks for writing it. >> >> Thanks for the review. >> >>> Could there be a test for this to make sure this doesn't regress in >>> the future? See t/t9151-svn-mergeinfo.sh for some examples. >> >> Hmm, I'm afraid figuring out how to write such a test would take >> longer than I can really spare for this issue. There don't seem to be >> any svn+ssh tests currently. > > Well, could you give manual commands to allow me to reproduce the > problem? > > Then I'll translate them into a test. :) Something like this: git svn clone -s svn+ssh://user@host/repo git config svn.pushmergeinfo yes git checkout -b branch origin/branch git merge origin/trunk git svn dcommit Thanks! > FWIW remove_username seems to be able to cope fine with an http:// > URL. t/lib-httpd.sh starts an http server with Subversion enabled, > as long as the envvar GIT_SVN_TEST_HTTPD is set to true. Its address > is $svnrepo, which is an http URL (but I don't see a username in the > URL). Does that help? I think the http transport handles the username separately, not in the URL. I would expect that a dummy ssh wrapper like some of the tests use would be sufficient, no need for an actual network connection. > Alternatively, does using rewrite-root as in t9151-svn-mergeinfo.sh > help? Hmm, I'm not sure how rewriteRoot would interact with this issue, whether it would be useful as a workaround or another problematic case. Jason
[PATCH v2] git-svn: Fix svn.pushmergeinfo handling of svn+ssh usernames.
Previously, svn dcommit of a merge with svn.pushmergeinfo set would get error messages like "merge parent for is on branch svn+ssh://gcc.gnu.org/svn/gcc/trunk, which is not under the git-svn root svn+ssh://ja...@gcc.gnu.org/svn/gcc!" So, let's call remove_username (as we do for svn info) before comparing rooturl to branchurl. Signed-off-by: Jason Merrill <ja...@redhat.com> Reviewed-by: Jonathan Nieder <jrnie...@gmail.com> --- git-svn.perl | 1 + 1 file changed, 1 insertion(+) diff --git a/git-svn.perl b/git-svn.perl index fa42364785..3b95d67bde 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -931,6 +931,7 @@ sub cmd_dcommit { # information from different SVN repos, and paths # which are not underneath this repository root. my $rooturl = $gs->repos_root; + Git::SVN::remove_username($rooturl); foreach my $d (@$linear_refs) { my %parentshash; read_commit_parents(\%parentshash, $d); -- 2.13.5
Re: [PATCH] Fix merge parent checking with svn.pushmergeinfo.
On Fri, Sep 15, 2017 at 1:52 PM, Jonathan Nieder <jrnie...@gmail.com> wrote: > Hi, > > Jason Merrill wrote: > >> Subject: Fix merge parent checking with svn.pushmergeinfo. >> >> Without this fix, svn dcommit of a merge with svn.pushmergeinfo set would >> get error messages like "merge parent for is on branch >> svn+ssh://gcc.gnu.org/svn/gcc/trunk, which is not under the git-svn root >> svn+ssh://ja...@gcc.gnu.org/svn/gcc!" >> >> * git-svn.perl: Remove username from rooturl before comparing to branchurl. >> >> Signed-off-by: Jason Merrill <ja...@redhat.com> > > Interesting. Thanks for writing it. Thanks for the review. > Could there be a test for this to make sure this doesn't regress in > the future? See t/t9151-svn-mergeinfo.sh for some examples. Hmm, I'm afraid figuring out how to write such a test would take longer than I can really spare for this issue. There don't seem to be any svn+ssh tests currently. > Nit: git doesn't use GNU-style changelogs, preferring to let the code > speak for itself. Maybe it would work better as the subject line? > E.g. something like > > git-svn: remove username from root before comparing to branch URL > > Without this fix, ... > > Signed-off-by: ... How about this? git-svn: Fix svn.pushmergeinfo handling of svn+ssh usernames. Previously, svn dcommit of a merge with svn.pushmergeinfo set would get error messages like "merge parent for is on branch svn+ssh://gcc.gnu.org/svn/gcc/trunk, which is not under the git-svn root svn+ssh://ja...@gcc.gnu.org/svn/gcc!" So, let's call remove_username (as we do for svn info) before comparing rooturl to branchurl. >> --- >> git-svn.perl | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/git-svn.perl b/git-svn.perl >> index fa42364785..1663612b1c 100755 >> --- a/git-svn.perl >> +++ b/git-svn.perl >> @@ -931,6 +931,7 @@ sub cmd_dcommit { >> # information from different SVN repos, and paths >> # which are not underneath this repository root. >> my $rooturl = $gs->repos_root; >> + Git::SVN::remove_username ($rooturl); > > style nit: Git doesn't include a space between function names and > their argument list. Fixed. > I wonder if it would make sense to rename the $rooturl variable > since now it is not the unmodified root. E.g. how about > > my $expect_url = $gs->repos_root; > Git::SVN::remove_username($expect_url); > ... > >> foreach my $d (@$linear_refs) { >> my %parentshash; >> read_commit_parents(\%parentshash, $d); It isn't the unmodified root, but it is the effective root that is printed by svn info and used in branch URLs in git-svn-id, so it seems to me that the name $rooturl is still appropriate. > The rest looks good. > > Thanks and hope that helps, > Jonathan
[PATCH] Fix merge parent checking with svn.pushmergeinfo.
Without this fix, svn dcommit of a merge with svn.pushmergeinfo set would get error messages like "merge parent for is on branch svn+ssh://gcc.gnu.org/svn/gcc/trunk, which is not under the git-svn root svn+ssh://ja...@gcc.gnu.org/svn/gcc!" * git-svn.perl: Remove username from rooturl before comparing to branchurl. Signed-off-by: Jason Merrill <ja...@redhat.com> --- git-svn.perl | 1 + 1 file changed, 1 insertion(+) diff --git a/git-svn.perl b/git-svn.perl index fa42364785..1663612b1c 100755 --- a/git-svn.perl +++ b/git-svn.perl @@ -931,6 +931,7 @@ sub cmd_dcommit { # information from different SVN repos, and paths # which are not underneath this repository root. my $rooturl = $gs->repos_root; + Git::SVN::remove_username ($rooturl); foreach my $d (@$linear_refs) { my %parentshash; read_commit_parents(\%parentshash, $d); -- 2.13.5