a couple of git-svn patches

a couple of git-svn patches

[Alejandro R. Sedeño]

Here are two independent git-svn patches that my co-worker handed off
to me back in January.

The first changes the behavior of commit_url to be like that of url so
that you can commit to multiple branches.

The second uses Memoize to cache the results of shelling out to
git-config, which has good performance benefits for git-svn fetch.

[PATCH 1/2] git-svn: Fix the commit-url config to be the base url, just like the url config

[Alejandro R. Sedeño]

From: James Knight <jknight@itasoftware.com>

This is necessary if you want to be able to commit to multiple branches.

Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
---
 git-svn.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index a5857c1..aa41896 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -523,12 +523,14 @@ sub cmd_dcommit {
 	}
 
 	if (defined $_commit_url) {
-		$url = $_commit_url;
+		$url = $_commit_url . (length $gs->{path} ? '/' . $gs->{path} : '');
 	} else {
 		$url = eval { command_oneline('config', '--get',
 			      "svn-remote.$gs->{repo_id}.commiturl") };
 		if (!$url) {
 			$url = $gs->full_url
+		} else {
+			$url = $url . (length $gs->{path} ? '/' . $gs->{path} : '');
 		}
 	}
 
-- 
1.7.4.2.1.gd6f1f

[PATCH 2/2] git-svn: Cache results of running the executable "git config"

[Alejandro R. Sedeño]

From: James Y Knight <jknight@itasoftware.com>

Running programs is not cheap!

Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
---
 git-svn.perl |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index aa41896..e47e04c 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -59,6 +59,7 @@ use File::Find;
 use Getopt::Long qw/:config gnu_getopt no_ignore_case auto_abbrev/;
 use IPC::Open3;
 use Git;
+use Memoize;  # core since 5.8.0, Jul 2002
 
 BEGIN {
 	# import functions from Git into our packages, en masse
@@ -72,6 +73,8 @@ BEGIN {
 			*{"${package}::$_"} = \&{"Git::$_"};
 		}
 	}
+	Memoize::memoize 'Git::config';
+	Memoize::memoize 'Git::config_bool';
 }
 
 my ($SVN);
@@ -3199,6 +3202,8 @@ sub has_no_changes {
 		Memoize::unmemoize 'check_cherry_pick';
 		Memoize::unmemoize 'has_no_changes';
 	}
+
+	Memoize::memoize 'Git::SVN::repos_root';
 }
 
 END {
-- 
1.7.4.2.1.gd6f1f

[PATCHv2 0/2] a couple of git-svn patches

[Alejandro R. Sedeño]

(The patches themselves haven't changed since I sent them to the list
on Friday, but this does clean up attribution and sign-offs in the
commit messages.)

Here are two independent git-svn patches that my co-worker handed off
to me back in January.

The first changes the behavior of commit_url to be like that of url so
that you can commit to multiple branches.

The second uses Memoize to cache the results of shelling out to
git-config, which has good performance benefits for git-svn fetch.

[PATCH 1/2] git-svn: Fix the commit-url config to be the base url, just like the url config

[Alejandro R. Sedeño]

From: James Y Knight <jknight@itasoftware.com>

This is necessary if you want to be able to commit to multiple branches.

Signed-off-by: James Y Knight <jknight@itasoftware.com>
Signed-off-by: Alejandro R. Sedeño <asedeno@mit.edu>
---
 git-svn.perl |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index a5857c1..aa41896 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -523,12 +523,14 @@ sub cmd_dcommit {
 	}
 
 	if (defined $_commit_url) {
-		$url = $_commit_url;
+		$url = $_commit_url . (length $gs->{path} ? '/' . $gs->{path} : '');
 	} else {
 		$url = eval { command_oneline('config', '--get',
 			      "svn-remote.$gs->{repo_id}.commiturl") };
 		if (!$url) {
 			$url = $gs->full_url
+		} else {
+			$url = $url . (length $gs->{path} ? '/' . $gs->{path} : '');
 		}
 	}
 
-- 
1.7.4.2.1.gd6f1f

Re: [PATCH 1/2] git-svn: Fix the commit-url config to be the base url, just like the url config

[Eric Wong]

"Alejandro R. Sedeño" <asedeno@mit.edu> wrote:
> From: James Y Knight <jknight@itasoftware.com>
> 
> This is necessary if you want to be able to commit to multiple branches.

I'm not sure I understand the need for this patch, see below.

> @@ -523,12 +523,14 @@ sub cmd_dcommit {
>  	}
>  
>  	if (defined $_commit_url) {
> -		$url = $_commit_url;
> +		$url = $_commit_url . (length $gs->{path} ? '/' . $gs->{path} : '');

$_commit_url is a user-specified parameter from the --commit-url switch.
If they want to override it it's the user's perogative.  It's not the
default and not commonly used.

>  	} else {
>  		$url = eval { command_oneline('config', '--get',
>  			      "svn-remote.$gs->{repo_id}.commiturl") };
>  		if (!$url) {
>  			$url = $gs->full_url

If unspecified, we always fall back to the gs->full_url case anyways
which just duplicates the logic you've added.  Again, commitUrl usually
does not need to be specified since it is autodetected.

> +		} else {
> +			$url = $url . (length $gs->{path} ? '/' . $gs->{path} : '');
>  		}
>  	}

-- 
Eric Wong

Re: [PATCH 1/2] git-svn: Fix the commit-url config to be the base url, just like the url config

[James Y Knight]

On Apr 4, 2011, at 5:52 PM, Eric Wong wrote:
> $_commit_url is a user-specified parameter from the --commit-url switch.
> If they want to override it it's the user's perogative.  It's not the
> default and not commonly used.

In .git/config:

[svn-remote "svn"]
        url = http://hostname/svnrepo/
        commiturl = svn+ssh://hostname/svnrepo
        branches = branches/*:refs/remotes/origin/*
        fetch = trunk:refs/remotes/origin/master

The commiturl configuration is necessary so that the canonical URL is the http:// url (which can be used for read-only access), but if you want to commit, you have to use svn+ssh. This is a fairly common way of setting up access to an svn repository, so I'm surprised to be the first one to hit this issue.

Without the patch, you need:
        commiturl = svn+ssh://hostname/svnrepo/trunk
which of course then doesn't allow you to commit to other branches.

James

Re: [PATCH 1/2] git-svn: Fix the commit-url config to be the base url, just like the url config

[Eric Wong]

James Y Knight <jknight@itasoftware.com> wrote:
> On Apr 4, 2011, at 5:52 PM, Eric Wong wrote:
> > $_commit_url is a user-specified parameter from the --commit-url switch.
> > If they want to override it it's the user's perogative.  It's not the
> > default and not commonly used.
> 
> In .git/config:
> 
> [svn-remote "svn"]
>         url = http://hostname/svnrepo/
>         commiturl = svn+ssh://hostname/svnrepo
>         branches = branches/*:refs/remotes/origin/*
>         fetch = trunk:refs/remotes/origin/master
> 
> The commiturl configuration is necessary so that the canonical URL is the http:// url (which can be used for read-only access), but if you want to commit, you have to use svn+ssh. This is a fairly common way of setting up access to an svn repository, so I'm surprised to be the first one to hit this issue.
> 
> Without the patch, you need:
>         commiturl = svn+ssh://hostname/svnrepo/trunk
> which of course then doesn't allow you to commit to other branches.

Originally --commit-url was only intended to be a command-line option
and for overriding specific cases and also for dealing with permission
mismatches (limited commit access to a branch, unlimited read access
to the repo).

Your patch breaks existing use cases, I think.

I think Junio's suggestion for a pushurl config which does what you
think commitUrl does would be what you're after...

Thanks


-- 
Eric Wong

Re: [PATCH 1/2] git-svn: Fix the commit-url config to be the base url, just like the url config

["Alejandro R. Sedeño"]

On 04/04/2011 06:54 PM, Eric Wong wrote:
> James Y Knight <jknight@itasoftware.com> wrote:
>> > On Apr 4, 2011, at 5:52 PM, Eric Wong wrote:
>>> > > $_commit_url is a user-specified parameter from the --commit-url switch.
>>> > > If they want to override it it's the user's perogative.  It's not the
>>> > > default and not commonly used.
>> > 
>> > In .git/config:
>> > 
>> > [svn-remote "svn"]
>> >         url = http://hostname/svnrepo/
>> >         commiturl = svn+ssh://hostname/svnrepo
>> >         branches = branches/*:refs/remotes/origin/*
>> >         fetch = trunk:refs/remotes/origin/master
>> > 
>> > The commiturl configuration is necessary so that the canonical URL is the http:// url (which can be used for read-only access), but if you want to commit, you have to use svn+ssh. This is a fairly common way of setting up access to an svn repository, so I'm surprised to be the first one to hit this issue.
>> > 
>> > Without the patch, you need:
>> >         commiturl = svn+ssh://hostname/svnrepo/trunk
>> > which of course then doesn't allow you to commit to other branches.
> Originally --commit-url was only intended to be a command-line option
> and for overriding specific cases and also for dealing with permission
> mismatches (limited commit access to a branch, unlimited read access
> to the repo).
> 
> Your patch breaks existing use cases, I think.
> 
> I think Junio's suggestion for a pushurl config which does what you
> think commitUrl does would be what you're after...

That sounds good to me. I'll submit an updated patch later today.

-Alejandro