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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <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@xxxxxxxxxxx/svn/gcc!"
>
> * git-svn.perl: Remove username from rooturl before comparing to branchurl.
>
> Signed-off-by: Jason Merrill <jason@xxxxxxxxxx>

Interesting.  Thanks for writing it.

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.

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: ...

> ---
>  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.

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);

The rest looks good.

Thanks and hope that helps,
Jonathan



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux