Re: [PATCH] Fix merge parent checking with svn.pushmergeinfo.

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sep 15, 2017, 21:53 UTC
Message-ID: <20170915215303.GV27425@aiede.mtv.corp.google.com>
In-Reply-To: <CADzB+2mxyXcROYx72tac8cUxBMxi=ZxUNQYxpUR1CZ43e-j9gA@mail.gmail.com>

Jason Merrill wrote:

> On Fri, Sep 15, 2017 at 1:52 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> > Jason Merrill wrote:

Show 21 quoted lines

>>> 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 <X> for <Y> is on branch
>>> svn+ssh://gcc.gnu.org/svn/gcc/trunk, which is not under the git-svn root
>>> svn+ssh://jason@gcc.gnu.org/svn/gcc!"
>>>
>>> * git-svn.perl: Remove username from rooturl before comparing to branchurl.
>>>
>>> Signed-off-by: Jason Merrill <jason@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. :)

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?

Alternatively, does using rewrite-root as in t9151-svn-mergeinfo.sh help?

[...]

Show 11 quoted lines

> 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 <X> for <Y> is on branch
>     svn+ssh://gcc.gnu.org/svn/gcc/trunk, which is not under the git-svn root
>     svn+ssh://jason@gcc.gnu.org/svn/gcc!"
>
>     So, let's call remove_username (as we do for svn info) before comparing
>     rooturl to branchurl.

Looks good.

Thanks.

Jonathan