git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* git-svn won't remember pem password
@ 2012-02-18  0:36 Igor
  2012-02-18 11:30 ` Jakub Narebski
  2012-02-19  1:30 ` Eric Wong
  0 siblings, 2 replies; 35+ messages in thread
From: Igor @ 2012-02-18  0:36 UTC (permalink / raw)
  To: git; +Cc: Eric Wong

I'm running into an issue where I have to enter my pem certificate password every time I git-svn fetch or git-svn dcommit. Vanilla svn uses OS X KeyChain and remembers my password just fine. Is there a known solution for this? Other users have ran into same issue as described here: http://stackoverflow.com/questions/605519/does-git-svn-store-svn-passwords. However, that solution of removing .subversion folder did not work for me.

Thanks,
Igor

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: git-svn won't remember pem password
  2012-02-18  0:36 git-svn won't remember pem password Igor
@ 2012-02-18 11:30 ` Jakub Narebski
  2012-02-19  4:03   ` Nikolaus Demmel
  2012-02-20  0:57   ` Jeff King
  2012-02-19  1:30 ` Eric Wong
  1 sibling, 2 replies; 35+ messages in thread
From: Jakub Narebski @ 2012-02-18 11:30 UTC (permalink / raw)
  To: Igor; +Cc: git, Eric Wong

Igor <mrigor83@gmail.com> writes:

> I'm running into an issue where I have to enter my pem certificate
> password every time I git-svn fetch or git-svn dcommit. Vanilla svn
> uses OS X KeyChain and remembers my password just fine. Is there a
> known solution for this? Other users have ran into same issue as
> described here:
>
>   http://stackoverflow.com/questions/605519/does-git-svn-store-svn-passwords

> However, that solution of removing .subversion folder did not work
> for me.

I don't know if it is svn that has to remember password, or git that
has to remember password.  Git 1.7.9 learned "credentials API" that
allows integration with platform native keychain mechanisms, and I
think OS X Keychain is one of examples / supported platforms (but it
might not made it into core git)... though I am not sure if it affects
git-svn, or only HTTP(S) transport.

-- 
Jakub Narebski

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: git-svn won't remember pem password
  2012-02-18  0:36 git-svn won't remember pem password Igor
  2012-02-18 11:30 ` Jakub Narebski
@ 2012-02-19  1:30 ` Eric Wong
  2012-04-26 18:00   ` Matthijs Kooijman
  1 sibling, 1 reply; 35+ messages in thread
From: Eric Wong @ 2012-02-19  1:30 UTC (permalink / raw)
  To: Igor; +Cc: Matthijs Kooijman, Gustav Munkby, Edward Rudd, Carsten Bormann,
	git

Igor <mrigor83@gmail.com> wrote:
> I'm running into an issue where I have to enter my pem certificate
> password every time I git-svn fetch or git-svn dcommit. Vanilla svn
> uses OS X KeyChain and remembers my password just fine. Is there a
> known solution for this? Other users have ran into same issue as
> described here:
> http://stackoverflow.com/questions/605519/does-git-svn-store-svn-passwords.
> However, that solution of removing .subversion folder did not work for
> me.

Hi Igor, this issue seems related to the platform specific auth
providers patches.   There have been bugs in the SVN bindings in
previous releases and uncertainty about how everything works.

I haven't been interested enough to follow along closely[1], but maybe
some other folks Cc:-ed can finally push this through.

http://mid.gmane.org/20120103204403.GI17548@login.drsnuggles.stderr.nl

Basically I'm waiting for a patch that we can be certain won't break the
majority of existing use cases (especially no triggering of segfaults
and other nastiness in released versions of SVN bindings).

[1] - I barely use git-svn anymore, and wouldn't touch GNOME or OSX
      with a 10-foot pole...

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: git-svn won't remember pem password
  2012-02-18 11:30 ` Jakub Narebski
@ 2012-02-19  4:03   ` Nikolaus Demmel
  2012-02-20  0:57   ` Jeff King
  1 sibling, 0 replies; 35+ messages in thread
From: Nikolaus Demmel @ 2012-02-19  4:03 UTC (permalink / raw)
  To: git


Jakub Narebski wrote
> 
> Igor &lt;mrigor83@&gt; writes:
> 
>> I'm running into an issue where I have to enter my pem certificate
>> password every time I git-svn fetch or git-svn dcommit. Vanilla svn
>> uses OS X KeyChain and remembers my password just fine. Is there a
>> known solution for this?
> 
> I don't know if it is svn that has to remember password, or git that
> has to remember password.  Git 1.7.9 learned "credentials API" that
> allows integration with platform native keychain mechanisms, and I
> think OS X Keychain is one of examples / supported platforms (but it
> might not made it into core git)... though I am not sure if it affects
> git-svn, or only HTTP(S) transport.
> 

Wow, I just signed up to the mainling list to post about this, but it turns
out the latest message is exactly what I wanted to ask.

Like Eric wrote, I'm pretty sure it is svn that is meant to store the
password here and the perl bindings or the git-svn part fails to deal with
the os x keychain right. With pure svn the keychain authentication works
just fine. If I set up plaintext password storage in the svn configs, then
git svn is also able to store passwords.

There is also this macports ticket [1] that has been around for a while. But
that can only be fixed by an upstream fix here.

I would love to help to get this working, but I'm not sure how I can.

Cheers,
Nikolaus


[1] https://trac.macports.org/ticket/28329


--
View this message in context: http://git.661346.n2.nabble.com/git-svn-won-t-remember-pem-password-tp7295962p7298035.html
Sent from the git mailing list archive at Nabble.com.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: git-svn won't remember pem password
  2012-02-18 11:30 ` Jakub Narebski
  2012-02-19  4:03   ` Nikolaus Demmel
@ 2012-02-20  0:57   ` Jeff King
  2012-02-20  3:08     ` Nikolaus Demmel
  1 sibling, 1 reply; 35+ messages in thread
From: Jeff King @ 2012-02-20  0:57 UTC (permalink / raw)
  To: Jakub Narebski; +Cc: Igor, git, Eric Wong

On Sat, Feb 18, 2012 at 03:30:00AM -0800, Jakub Narebski wrote:

> Igor <mrigor83@gmail.com> writes:
> 
> > I'm running into an issue where I have to enter my pem certificate
> > password every time I git-svn fetch or git-svn dcommit. Vanilla svn
> > uses OS X KeyChain and remembers my password just fine. Is there a
> > known solution for this? Other users have ran into same issue as
> > described here:
> >
> >   http://stackoverflow.com/questions/605519/does-git-svn-store-svn-passwords
> 
> > However, that solution of removing .subversion folder did not work
> > for me.
> 
> I don't know if it is svn that has to remember password, or git that
> has to remember password.  Git 1.7.9 learned "credentials API" that
> allows integration with platform native keychain mechanisms, and I
> think OS X Keychain is one of examples / supported platforms (but it
> might not made it into core git)... though I am not sure if it affects
> git-svn, or only HTTP(S) transport.

It does not affect git-svn currently.

I have some thoughts on providing access to the credentials API for
scripts like git-svn (right now, it is accessible only by C git
programs). However, there is an important question: should password
prompting in git-svn behave like git, or behave like svn?

So far, it has been the latter, and I think that is reasonable. The
resource that requires the credentials is an svn repo, not a git repo,
so you are more likely to want to share credentials for it with real
svn, and not other git commands.

As to the lack of keychain support, it looks like libsvn should be
handling this for us. We simply give it a callback function that gets
called if the user needs prompted, but I would think things like the
keychain handling would happen before it gets to our prompt (and if I
understand it, for _some_ credential storage formats, it does). So
either there is a bug in libsvn, or we are somehow invoking it
incorrectly.

-Peff

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: git-svn won't remember pem password
  2012-02-20  0:57   ` Jeff King
@ 2012-02-20  3:08     ` Nikolaus Demmel
  0 siblings, 0 replies; 35+ messages in thread
From: Nikolaus Demmel @ 2012-02-20  3:08 UTC (permalink / raw)
  To: Jeff King; +Cc: Jakub Narebski, Igor, git, Eric Wong


Am 20.02.2012 um 01:57 schrieb Jeff King:

> On Sat, Feb 18, 2012 at 03:30:00AM -0800, Jakub Narebski wrote:
> 
>> Igor <mrigor83@gmail.com> writes:
>> 
>>> I'm running into an issue where I have to enter my pem certificate
>>> password every time I git-svn fetch or git-svn dcommit. Vanilla svn
>>> uses OS X KeyChain and remembers my password just fine. Is there a
>>> known solution for this? Other users have ran into same issue as
>>> described here:
>>> 
>>>  http://stackoverflow.com/questions/605519/does-git-svn-store-svn-passwords
>> 
>>> However, that solution of removing .subversion folder did not work
>>> for me.
>> 
>> I don't know if it is svn that has to remember password, or git that
>> has to remember password.  Git 1.7.9 learned "credentials API" that
>> allows integration with platform native keychain mechanisms, and I
>> think OS X Keychain is one of examples / supported platforms (but it
>> might not made it into core git)... though I am not sure if it affects
>> git-svn, or only HTTP(S) transport.
> 
> It does not affect git-svn currently.
> 
> I have some thoughts on providing access to the credentials API for
> scripts like git-svn (right now, it is accessible only by C git
> programs). However, there is an important question: should password
> prompting in git-svn behave like git, or behave like svn?
> 
> So far, it has been the latter, and I think that is reasonable. The
> resource that requires the credentials is an svn repo, not a git repo,
> so you are more likely to want to share credentials for it with real
> svn, and not other git commands.
> 

IMHO a normal user would expect git-svn to store credentials in the same way as normal svn. I think this is the way it should be.

Best regards,
Nikolaus

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: git-svn won't remember pem password
  2012-02-19  1:30 ` Eric Wong
@ 2012-04-26 18:00   ` Matthijs Kooijman
       [not found]     ` <41A093CB-CA4A-4FEF-9F5C-A9B626D10AFB@gmail.com>
  0 siblings, 1 reply; 35+ messages in thread
From: Matthijs Kooijman @ 2012-04-26 18:00 UTC (permalink / raw)
  To: Igor; +Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git, Eric Wong

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

Hi Igor,

> > I'm running into an issue where I have to enter my pem certificate
> > password every time I git-svn fetch or git-svn dcommit. Vanilla svn
> > uses OS X KeyChain and remembers my password just fine. Is there a
> > known solution for this? Other users have ran into same issue as
> > described here:
> > http://stackoverflow.com/questions/605519/does-git-svn-store-svn-passwords.
> > However, that solution of removing .subversion folder did not work for
> > me.

I suspect this would be solved by including what svn calls "platform
specific authentication providers". This works on Linux, but I don't
have access to OSX for testing.

Could you test the patch I pasted below to see if this fixes your
problem? The patch is made against the git source tree (git master
version), but you should probably be able to apply it to your installed
git-svn file as well (it's just a perl script). I'm not sure where that
file lives on OSX though, it's /usr/lib/git-core/git-svn on my Linux
(Debian) system.

Eric, I think I got this feature sorted out finally (there is a second
patch for entering the gnome-keyring unlock password which required some
changes in the subversion bindings which got applied a while ago). Let's
see what the patch does for Igor and then I'll send over both patches
afterwards.

Gr.

Matthijs

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: git-svn won't remember pem password
       [not found]       ` <20120426181327.GZ4023@login.drsnuggles.stderr.nl>
@ 2012-04-26 18:31         ` Igor
  2012-04-26 18:36           ` Matthijs Kooijman
  0 siblings, 1 reply; 35+ messages in thread
From: Igor @ 2012-04-26 18:31 UTC (permalink / raw)
  To: Matthijs Kooijman
  Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git, Eric Wong

Nice, that seems to work. I got prompted to allow access to my KeyChain.

Thanks!

On Apr 26, 2012, at 11:13 AM, Matthijs Kooijman wrote:

>> Did you forget to attach the patch?
> No, but I wrote it in invisible ink! But here's the patch in regular
> ink, just in case.... *cough*
> 
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -5436,7 +5436,7 @@ BEGIN {
> }
> 
> sub _auth_providers () {
> -       [
> +       my @rv = (
>          SVN::Client::get_simple_provider(),
>          SVN::Client::get_ssl_server_trust_file_provider(),
>          SVN::Client::get_simple_prompt_provider(
> @@ -5452,7 +5452,23 @@ sub _auth_providers () {
>            \&Git::SVN::Prompt::ssl_server_trust),
>          SVN::Client::get_username_prompt_provider(
>            \&Git::SVN::Prompt::username, 2)
> -       ]
> +       );
> +
> +       # earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
> +       # this function
> +       if ($SVN::Core::VERSION gt '1.6.12') {
> +               my $config = SVN::Core::config_get_config($config_dir);
> +               my ($p, @a);
> +               # config_get_config returns all config files from
> +               # ~/.subversion, auth_get_platform_specific_client_providers
> +               # just wants the config "file".
> +               @a = ($config->{'config'}, undef);
> +               $p = SVN::Core::auth_get_platform_specific_client_providers(@a);
> +               # Insert the return value from
> +               # auth_get_platform_specific_providers
> +               unshift @rv, @$p;
> +       }
> +       \@rv;
> }
> 
> sub escape_uri_only {

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: git-svn won't remember pem password
  2012-04-26 18:31         ` Igor
@ 2012-04-26 18:36           ` Matthijs Kooijman
  2012-04-26 19:34             ` [PATCH 1/2] git-svn: use platform specific auth providers Matthijs Kooijman
  0 siblings, 1 reply; 35+ messages in thread
From: Matthijs Kooijman @ 2012-04-26 18:36 UTC (permalink / raw)
  To: Igor; +Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git, Eric Wong

[-- Attachment #1: Type: text/plain, Size: 172 bytes --]

Hi Igor,

> Nice, that seems to work. I got prompted to allow access to my KeyChain.
Thanks for the quick testing!

Eric, I'll send the patches in a minute.

Gr.

Matthijs

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-26 18:36           ` Matthijs Kooijman
@ 2012-04-26 19:34             ` Matthijs Kooijman
  2012-04-26 19:34               ` [PATCH 2/2] git-svn: Configure a prompt callback for gnome_keyring Matthijs Kooijman
  2012-04-27  8:21               ` [PATCH 1/2] git-svn: use platform specific auth providers Eric Wong
  0 siblings, 2 replies; 35+ messages in thread
From: Matthijs Kooijman @ 2012-04-26 19:34 UTC (permalink / raw)
  To: Eric Wong
  Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git,
	Matthijs Kooijman

On Linux, this makes authentication using passwords from gnome-keyring
and kwallet work (only the former was tested). On Mac OS X, this allows
using the OS X Keychain.
---
 git-svn.perl |   20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/git-svn.perl b/git-svn.perl
index 4334b95..1790d10 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5436,7 +5436,7 @@ BEGIN {
 }
 
 sub _auth_providers () {
-	[
+	my @rv = (
 	  SVN::Client::get_simple_provider(),
 	  SVN::Client::get_ssl_server_trust_file_provider(),
 	  SVN::Client::get_simple_prompt_provider(
@@ -5452,7 +5452,23 @@ sub _auth_providers () {
 	    \&Git::SVN::Prompt::ssl_server_trust),
 	  SVN::Client::get_username_prompt_provider(
 	    \&Git::SVN::Prompt::username, 2)
-	]
+	);
+
+	# earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
+	# this function
+	if ($SVN::Core::VERSION gt '1.6.12') {
+		my $config = SVN::Core::config_get_config($config_dir);
+		my ($p, @a);
+		# config_get_config returns all config files from
+		# ~/.subversion, auth_get_platform_specific_client_providers
+		# just wants the config "file".
+		@a = ($config->{'config'}, undef);
+		$p = SVN::Core::auth_get_platform_specific_client_providers(@a);
+		# Insert the return value from
+		# auth_get_platform_specific_providers
+		unshift @rv, @$p;
+	}
+	\@rv;
 }
 
 sub escape_uri_only {
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 2/2] git-svn: Configure a prompt callback for gnome_keyring.
  2012-04-26 19:34             ` [PATCH 1/2] git-svn: use platform specific auth providers Matthijs Kooijman
@ 2012-04-26 19:34               ` Matthijs Kooijman
  2012-04-27  8:28                 ` Eric Wong
  2012-04-27  8:29                 ` [PATCH 2/2] " Matthijs Kooijman
  2012-04-27  8:21               ` [PATCH 1/2] git-svn: use platform specific auth providers Eric Wong
  1 sibling, 2 replies; 35+ messages in thread
From: Matthijs Kooijman @ 2012-04-26 19:34 UTC (permalink / raw)
  To: Eric Wong
  Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git,
	Matthijs Kooijman

This allows git-svn to prompt for a keyring unlock password, when a
the needed gnome keyring is locked.

This requires changes in the subversion perl bindings which have been
committed to trunk (1241554 and some followup commits) and should be
available with the (as of yet unreleased) 1.8.0 release.
---
 git-svn.perl |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/git-svn.perl b/git-svn.perl
index 1790d10..6565f4a 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -4420,6 +4420,11 @@ sub username {
 	$SVN::_Core::SVN_NO_ERROR;
 }
 
+sub gnome_keyring_unlock {
+	my ($keyring, $pool) = @_;
+	_read_password("Password for '$keyring' GNOME keyring: ", undef);
+}
+
 sub _read_password {
 	my ($prompt, $realm) = @_;
 	my $password = '';
@@ -5524,6 +5529,21 @@ sub new {
 			$Git::SVN::Prompt::_no_auth_cache = 1;
 		}
 	} # no warnings 'once'
+
+
+	# Allow git-svn to show a prompt for opening up a gnome-keyring, if needed.
+	if (defined(&SVN::Core::auth_set_gnome_keyring_unlock_prompt_func)) {
+		my $keyring_callback = SVN::Core::auth_set_gnome_keyring_unlock_prompt_func(
+			$baton,
+			\&Git::SVN::Prompt::gnome_keyring_unlock
+		);
+		# Keep a reference to this callback, to prevent the function
+		# (reference) from being garbage collected.  We just add it to
+		# the callbacks value, which are also used only to prevent the
+		# garbage collector from eating stuff.
+		$callbacks = [$callbacks, $keyring_callback]
+	}
+
 	my $self = SVN::Ra->new(url => escape_url($url), auth => $baton,
 	                      config => $config,
 			      pool => SVN::Pool->new,
-- 
1.7.10

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-26 19:34             ` [PATCH 1/2] git-svn: use platform specific auth providers Matthijs Kooijman
  2012-04-26 19:34               ` [PATCH 2/2] git-svn: Configure a prompt callback for gnome_keyring Matthijs Kooijman
@ 2012-04-27  8:21               ` Eric Wong
  2012-04-27  8:25                 ` Matthijs Kooijman
  1 sibling, 1 reply; 35+ messages in thread
From: Eric Wong @ 2012-04-27  8:21 UTC (permalink / raw)
  To: Matthijs Kooijman; +Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git

Matthijs Kooijman <matthijs@stdin.nl> wrote:
> On Linux, this makes authentication using passwords from gnome-keyring
> and kwallet work (only the former was tested). On Mac OS X, this allows
> using the OS X Keychain.

Thanks, this looks good.  Can you add a Signed-off-by? (you can just
reply here and I'll add it to the commit message when pushing.

Otherwise, consider this Acked-by: Eric Wong <normalperson@yhbt.net>

> +	# earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
> +	# this function
> +	if ($SVN::Core::VERSION gt '1.6.12') {

Thank you for documenting this segfault, btw.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-27  8:21               ` [PATCH 1/2] git-svn: use platform specific auth providers Eric Wong
@ 2012-04-27  8:25                 ` Matthijs Kooijman
  2012-04-29  8:23                   ` Eric Wong
  0 siblings, 1 reply; 35+ messages in thread
From: Matthijs Kooijman @ 2012-04-27  8:25 UTC (permalink / raw)
  To: Eric Wong; +Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git

[-- Attachment #1: Type: text/plain, Size: 233 bytes --]

> > On Linux, this makes authentication using passwords from gnome-keyring
> > and kwallet work (only the former was tested). On Mac OS X, this allows
> > using the OS X Keychain.
Signed-off-by: Matthijs Kooijman <matthijs@stdin.nl>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/2] git-svn: Configure a prompt callback for gnome_keyring.
  2012-04-26 19:34               ` [PATCH 2/2] git-svn: Configure a prompt callback for gnome_keyring Matthijs Kooijman
@ 2012-04-27  8:28                 ` Eric Wong
  2012-04-27  9:36                   ` Matthijs Kooijman
  2013-06-18 16:36                   ` Matthijs Kooijman
  2012-04-27  8:29                 ` [PATCH 2/2] " Matthijs Kooijman
  1 sibling, 2 replies; 35+ messages in thread
From: Eric Wong @ 2012-04-27  8:28 UTC (permalink / raw)
  To: Matthijs Kooijman; +Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git

Matthijs Kooijman <matthijs@stdin.nl> wrote:
> This allows git-svn to prompt for a keyring unlock password, when a
> the needed gnome keyring is locked.
> 
> This requires changes in the subversion perl bindings which have been
> committed to trunk (1241554 and some followup commits) and should be
> available with the (as of yet unreleased) 1.8.0 release.

I'm a hesitant to use/depend on unreleased functionality in SVN.

Is there a chance the API could change before the release.  Also,
what kind of tests do the SVN guys do on the Perl bindings + GNOME?
I'm especially concerned since we just worked around segfault
bugs in the other patch.

Can we put this on hold until somebody can test the 1.8.0 release?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/2] git-svn: Configure a prompt callback for gnome_keyring.
  2012-04-26 19:34               ` [PATCH 2/2] git-svn: Configure a prompt callback for gnome_keyring Matthijs Kooijman
  2012-04-27  8:28                 ` Eric Wong
@ 2012-04-27  8:29                 ` Matthijs Kooijman
  1 sibling, 0 replies; 35+ messages in thread
From: Matthijs Kooijman @ 2012-04-27  8:29 UTC (permalink / raw)
  To: Eric Wong; +Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git

[-- Attachment #1: Type: text/plain, Size: 354 bytes --]

Hi Eric,

just noticed a minor typo in this commit message:

> committed to trunk (1241554 and some followup commits) and should be
I forgot to add an "r" in front of this revision number, so the line
should be:
> committed to trunk (r1241554 and some followup commits) and should be

Not a big deal of course, but slightly more readable.

Gr.

Matthijs

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/2] git-svn: Configure a prompt callback for gnome_keyring.
  2012-04-27  8:28                 ` Eric Wong
@ 2012-04-27  9:36                   ` Matthijs Kooijman
  2013-06-18 16:36                   ` Matthijs Kooijman
  1 sibling, 0 replies; 35+ messages in thread
From: Matthijs Kooijman @ 2012-04-27  9:36 UTC (permalink / raw)
  To: Eric Wong; +Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git

[-- Attachment #1: Type: text/plain, Size: 1512 bytes --]

Hi Eric,

> > This requires changes in the subversion perl bindings which have been
> > committed to trunk (1241554 and some followup commits) and should be
> > available with the (as of yet unreleased) 1.8.0 release.
> I'm a hesitant to use/depend on unreleased functionality in SVN.
> 
> Is there a chance the API could change before the release.  Also, what
> kind of tests do the SVN guys do on the Perl bindings + GNOME?  I'm
> especially concerned since we just worked around segfault bugs in the
> other patch.
There is a testcase for this particular function (though it only tests
calling the function, not the full workings because no gnome-keyring is
available in the testsuite).

As for the other segfault: the reason for that one was that apparently
swig autogenerates bindings even when some typemaps are missing and then
just puts an abort or failing assert or something like that in there.
Unlikely to happen here, but I understand your restraint.

> Can we put this on hold until somebody can test the 1.8.0 release?
Sure, the change won't be useful without svn 1.8.0 anyway (and since
it's not a critical feature, we can live without it in the window
between 1.8.0 and the next git release as well).

I'll try to keep any eye on the 1.8.0 release. According to the svn
roadmap [1] it is planned for july (1.7.0 + 9 months) but looking at the
list of features "not started", I'd say it might be later...

http://subversion.apache.org/roadmap.html

Gr.

Matthijs

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-27  8:25                 ` Matthijs Kooijman
@ 2012-04-29  8:23                   ` Eric Wong
  2012-04-30  0:03                     ` Junio C Hamano
  0 siblings, 1 reply; 35+ messages in thread
From: Eric Wong @ 2012-04-29  8:23 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git,
	Matthijs Kooijman

Matthijs Kooijman <matthijs@stdin.nl> wrote:
> > > On Linux, this makes authentication using passwords from gnome-keyring
> > > and kwallet work (only the former was tested). On Mac OS X, this allows
> > > using the OS X Keychain.
> Signed-off-by: Matthijs Kooijman <matthijs@stdin.nl>

Thanks Matthijs, pushed to master of git://bogomips.org/git-svn for
Junio.

(actually pushed the other night, but I got distracted before
 sending this email :x)

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-29  8:23                   ` Eric Wong
@ 2012-04-30  0:03                     ` Junio C Hamano
  2012-04-30  1:21                       ` Junio C Hamano
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2012-04-30  0:03 UTC (permalink / raw)
  To: Eric Wong
  Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git,
	Matthijs Kooijman

Eric Wong <normalperson@yhbt.net> writes:

> Matthijs Kooijman <matthijs@stdin.nl> wrote:
>> > > On Linux, this makes authentication using passwords from gnome-keyring
>> > > and kwallet work (only the former was tested). On Mac OS X, this allows
>> > > using the OS X Keychain.
>> Signed-off-by: Matthijs Kooijman <matthijs@stdin.nl>
>
> Thanks Matthijs, pushed to master of git://bogomips.org/git-svn for
> Junio.
>
> (actually pushed the other night, but I got distracted before
>  sending this email :x)

Thanks. Pulled.

Eric, if it is not too much trouble, I'd appreciate an update to RelNotes
in pull requests.  I'll add the following to the "Foreign Interface"
section.

 Documentation/RelNotes/1.7.11.txt |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/RelNotes/1.7.11.txt b/Documentation/RelNotes/1.7.11.txt
index af73659..3609042 100644
--- a/Documentation/RelNotes/1.7.11.txt
+++ b/Documentation/RelNotes/1.7.11.txt
@@ -47,6 +47,9 @@ Foreign Interface
  * "git svn" used to die with unwanted SIGPIPE when talking with HTTP
    server that uses keep-alive.
 
+ * "git svn" learned to use platform specific authentication
+   providers, e.g. gnome-keyring, kwallet, etc.
+
  * "git p4" has been moved out of contrib/ area.
 
 Performance

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-30  0:03                     ` Junio C Hamano
@ 2012-04-30  1:21                       ` Junio C Hamano
  2012-04-30  8:19                         ` Eric Wong
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2012-04-30  1:21 UTC (permalink / raw)
  To: Eric Wong, git
  Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, Matthijs Kooijman

Junio C Hamano <gitster@pobox.com> writes:

> Eric Wong <normalperson@yhbt.net> writes:
>
>> Matthijs Kooijman <matthijs@stdin.nl> wrote:
>>> > > On Linux, this makes authentication using passwords from gnome-keyring
>>> > > and kwallet work (only the former was tested). On Mac OS X, this allows
>>> > > using the OS X Keychain.
>>> Signed-off-by: Matthijs Kooijman <matthijs@stdin.nl>
>>
>> Thanks Matthijs, pushed to master of git://bogomips.org/git-svn for
>> Junio.
>>
>> (actually pushed the other night, but I got distracted before
>>  sending this email :x)
>
> Thanks. Pulled.

As I am already too deep into today's integration round, I will not be
rewinding this pull anymore, but on one of my Ubuntu 10.04 boxes, I am
seeing this:

    Initialized empty Git repository in
    /srv/git/t/trash directory.t9118-git-svn-funky-branch-names/project/.git/
    ValueError svn_auth_get_platform_specific_client_providers is not implemented yet

So the tip of 'master' may be broken for some "git svn" users.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-30  1:21                       ` Junio C Hamano
@ 2012-04-30  8:19                         ` Eric Wong
  2012-04-30 16:04                           ` Junio C Hamano
  0 siblings, 1 reply; 35+ messages in thread
From: Eric Wong @ 2012-04-30  8:19 UTC (permalink / raw)
  To: Junio C Hamano, Matthijs Kooijman
  Cc: git, Gustav Munkby, Edward Rudd, Carsten Bormann

Junio C Hamano <gitster@pobox.com> wrote:
> As I am already too deep into today's integration round, I will not be
> rewinding this pull anymore, but on one of my Ubuntu 10.04 boxes, I am
> seeing this:
> 
>     Initialized empty Git repository in
>     /srv/git/t/trash directory.t9118-git-svn-funky-branch-names/project/.git/
>     ValueError svn_auth_get_platform_specific_client_providers is not implemented yet

Which version of SVN is that?  (git svn --version)
http://packages.ubuntu.com/lucid/libsvn-perl says 1.6.6
(+distro patches), which shouldn't be affected by this patch...

Matthijs: any suggestions/ideas on what could be wrong?

> So the tip of 'master' may be broken for some "git svn" users.

-- 
Eric Wong

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-30  8:19                         ` Eric Wong
@ 2012-04-30 16:04                           ` Junio C Hamano
  2012-04-30 16:53                             ` Matthijs Kooijman
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2012-04-30 16:04 UTC (permalink / raw)
  To: Eric Wong
  Cc: Matthijs Kooijman, git, Gustav Munkby, Edward Rudd,
	Carsten Bormann

Eric Wong <normalperson@yhbt.net> writes:

> Which version of SVN is that?  (git svn --version)
> http://packages.ubuntu.com/lucid/libsvn-perl says 1.6.6
> (+distro patches), which shouldn't be affected by this patch...

: a00 git.git/master; git version
git version 1.7.10.405.g10d433
: a00 git.git/master; git svn --version
git-svn version 1.7.10.405.g10d433 (svn 1.6.6)
: a00 git.git/master; dpkg -l libsvn-perl
Desired=Unknown/Install/Remove/Purge/Hold
|
Status=Not/Inst/Cfg-files/Unpacked/Failed-cfg/Half-inst/trig-aWait/Trig-pend
|/ Err?=(none)/Reinst-required (Status,Err: uppercase=bad)
||/ Name              Version           Description
+++-=================-=================-==================================================
ii  libsvn-perl       1.6.6dfsg-2ubuntu Perl bindings for Subversion
: a00 git.git/master; cd t
: a00 t/master; sh t9100-git-svn-basic.sh -i -v
Initialized empty Git repository in /scratch/buildfarm/git.git/t/trash
directory.t9100-git-svn-basic/.git/
define NO_SVN_TESTS to skip git svn tests
expecting success:
        mkdir import &&
        (
                cd import &&
                echo foo >foo &&
                ln -s foo foo.link
                mkdir -p dir/a/b/c/d/e &&
                echo "deep dir" >dir/a/b/c/d/e/file &&
                mkdir bar &&
                echo "zzz" >bar/zzz &&
                echo "#!/bin/sh" >exec.sh &&
                chmod +x exec.sh &&
                svn_cmd import -m "import for git svn" . "$svnrepo"
                >/dev/null
        ) &&
        rm -rf import &&
        git svn init "$svnrepo"
ok 1 - initialize git svn

expecting success: git svn fetch
ValueError svn_auth_get_platform_specific_client_providers is not
implemented yet

not ok - 2 import an SVN revision into git
#       git svn fetch

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-30 16:04                           ` Junio C Hamano
@ 2012-04-30 16:53                             ` Matthijs Kooijman
  2012-04-30 19:02                               ` Eric Wong
  0 siblings, 1 reply; 35+ messages in thread
From: Matthijs Kooijman @ 2012-04-30 16:53 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eric Wong, git, Gustav Munkby, Edward Rudd, Carsten Bormann

[-- Attachment #1: Type: text/plain, Size: 1076 bytes --]

Hi folks,

I could reproduce this problem (after installing half a dozen Ubuntue perl
packages on my Debian system ;-p). It seems the problem is because the
version comparison is wrong:

        if ($SVN::Core::VERSION gt '1.6.12') {

This does textual comparison, so 1.6.6 > 1.6.12. To do proper version
comparison, I think the version numbers should be split into
major/minor/revision and each be compared numerically.

This is not the only place where this comparison happens in this way,
there are 6 more comparisons in this way, which would have to be fixed
as well.

We could introduce a helper function for comparing version numbers by
splitting and comparing the parts separately, but that might be hard to
get right (especially when non-numeric version parts are involved).

An alternative would be to use the Sort::Versions perl module [1] for
this, but that would add an external dependency.

I'd be happy to code and test both approaches, just let me know which
would be preferred.

[1]: http://search.cpan.org/~edavis/Sort-Versions-1.5/Versions.pm

Gr.

Matthijs

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-30 16:53                             ` Matthijs Kooijman
@ 2012-04-30 19:02                               ` Eric Wong
  2012-04-30 19:20                                 ` Matthijs Kooijman
  0 siblings, 1 reply; 35+ messages in thread
From: Eric Wong @ 2012-04-30 19:02 UTC (permalink / raw)
  To: Matthijs Kooijman
  Cc: Junio C Hamano, git, Gustav Munkby, Edward Rudd, Carsten Bormann

Matthijs Kooijman <matthijs@stdin.nl> wrote:
> Hi folks,
> 
> I could reproduce this problem (after installing half a dozen Ubuntue perl
> packages on my Debian system ;-p). It seems the problem is because the
> version comparison is wrong:
> 
>         if ($SVN::Core::VERSION gt '1.6.12') {
> 
> This does textual comparison, so 1.6.6 > 1.6.12. To do proper version
> comparison, I think the version numbers should be split into
> major/minor/revision and each be compared numerically.
> 
> This is not the only place where this comparison happens in this way,
> there are 6 more comparisons in this way, which would have to be fixed
> as well.

Ah, thanks for the analysis, we were lucky in the past that all version
components only had a single character.

> We could introduce a helper function for comparing version numbers by
> splitting and comparing the parts separately, but that might be hard to
> get right (especially when non-numeric version parts are involved).
> 
> An alternative would be to use the Sort::Versions perl module [1] for
> this, but that would add an external dependency.
> 
> I'd be happy to code and test both approaches, just let me know which
> would be preferred.

I think the former is preferable for git.  Sort::Versions isn't used
anywhere else in git and I don't think it's widely installed.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-30 19:02                               ` Eric Wong
@ 2012-04-30 19:20                                 ` Matthijs Kooijman
  2012-05-01  1:08                                   ` Junio C Hamano
  0 siblings, 1 reply; 35+ messages in thread
From: Matthijs Kooijman @ 2012-04-30 19:20 UTC (permalink / raw)
  To: Eric Wong
  Cc: Junio C Hamano, git, Gustav Munkby, Edward Rudd, Carsten Bormann

[-- Attachment #1: Type: text/plain, Size: 923 bytes --]

Hi Eric,

> > This does textual comparison, so 1.6.6 > 1.6.12. To do proper version
> > comparison, I think the version numbers should be split into
> > major/minor/revision and each be compared numerically.
> 
> Ah, thanks for the analysis, we were lucky in the past that all version
> components only had a single character.
Indeed. Note that this includes the released subversion versions. For
example, the code contains this check:

    $SVN::Core::VERSION le '1.5.4'

and 1.5.10 < 1.5.4. Fortunately, 1.5.9 was the last release in the 1.5
series, and no other checks compare against 1.6.x.

If subversion would ever reach the 1.10.x version number, things would
also start breaking.

> I think the former is preferable for git.  Sort::Versions isn't used
> anywhere else in git and I don't think it's widely installed.
I guessed as much. I'll have a look at providing a patch.

Gr.

Matthijs

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-04-30 19:20                                 ` Matthijs Kooijman
@ 2012-05-01  1:08                                   ` Junio C Hamano
  2012-06-03 10:49                                     ` Charles Bailey
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2012-05-01  1:08 UTC (permalink / raw)
  To: Eric Wong; +Cc: git, Gustav Munkby, Edward Rudd, Carsten Bormann

Matthijs Kooijman <matthijs@stdin.nl> writes:

> Hi Eric,
>
>> > This does textual comparison, so 1.6.6 > 1.6.12. To do proper version
>> > comparison, I think the version numbers should be split into
>> > major/minor/revision and each be compared numerically.
>> 
>> Ah, thanks for the analysis, we were lucky in the past that all version
>> components only had a single character.
> Indeed. Note that this includes the released subversion versions. For
> example, the code contains this check:
>
>     $SVN::Core::VERSION le '1.5.4'
>
> and 1.5.10 < 1.5.4. Fortunately, 1.5.9 was the last release in the 1.5
> series, and no other checks compare against 1.6.x.
>
> If subversion would ever reach the 1.10.x version number, things would
> also start breaking.
>
>> I think the former is preferable for git.  Sort::Versions isn't used
>> anywhere else in git and I don't think it's widely installed.

Ok, something along the lines of this.  Perhaps instead of "compare_",
we may want to call it "require_", so that negative return maps naturally
to a failure.

diff --git a/git-svn.perl b/git-svn.perl
index 427da9e..4a2ec43 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -41,11 +41,28 @@ sub fatal (@) { print STDERR "@_\n"; exit 1 }
 # repository decides to close the connection which we expect to be kept alive.
 $SIG{PIPE} = 'IGNORE';
 
+# Given a dot separated version number, "subtract" it from
+# the SVN::Core::VERSION; non-negaitive return means the SVN::Core
+# is at least at the version the caller asked for.
+sub compare_svn_version {
+	my (@ours) = split(/\./, $SVN::Core::VERSION);
+	my (@theirs) = split(/\./, $_[0]);
+	my ($i, $diff);
+
+	for ($i = 0; $i < @ours && $i < @theirs; $i++) {
+		$diff = $ours[$i] - $theirs[$i];
+		return $diff if ($diff);
+	}
+	return 1 if ($i < @ours);
+	return -1 if ($i < @theirs);
+	return 0;
+}
+
 sub _req_svn {
 	require SVN::Core; # use()-ing this causes segfaults for me... *shrug*
 	require SVN::Ra;
 	require SVN::Delta;
-	if ($SVN::Core::VERSION lt '1.1.0') {
+	if (compare_svn_version('1.1.0') < 0) {
 		fatal "Need SVN::Core 1.1.0 or better (got $SVN::Core::VERSION)";
 	}
 }
@@ -1474,7 +1491,7 @@ sub cmd_info {
 	}
 	::_req_svn();
 	$result .= "Repository UUID: $uuid\n" unless $diff_status eq "A" &&
-		($SVN::Core::VERSION le '1.5.4' || $file_type ne "dir");
+		(compare_svn_version('1.5.4') <= 0 || $file_type ne "dir");
 	$result .= "Revision: " . ($diff_status eq "A" ? 0 : $rev) . "\n";
 
 	$result .= "Node Kind: " .
@@ -5464,7 +5481,7 @@ sub _auth_providers () {
 
 	# earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
 	# this function
-	if ($SVN::Core::VERSION gt '1.6.12') {
+	if (compare_svn_version('1.6.12') > 0) {
 		my $config = SVN::Core::config_get_config($config_dir);
 		my ($p, @a);
 		# config_get_config returns all config files from
@@ -5623,7 +5640,7 @@ sub get_log {
 	# drop it.  Therefore, the receiver callback passed to it
 	# is made aware of this limitation by being wrapped if
 	# the limit passed to is being wrapped.
-	if ($SVN::Core::VERSION le '1.2.0') {
+	if (compare_svn_version('1.2.0') <= 0) {
 		my $limit = splice(@args, 3, 1);
 		if ($limit > 0) {
 			my $receiver = pop @args;
@@ -5655,7 +5672,8 @@ sub trees_match {
 
 sub get_commit_editor {
 	my ($self, $log, $cb, $pool) = @_;
-	my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef, 0) : ();
+	
+	my @lock = (compare_svn_version('1.2.0') >= 0) ? (undef, 0) : ();
 	$self->SUPER::get_commit_editor($log, $cb, @lock, $pool);
 }
 
@@ -5673,7 +5691,7 @@ sub gs_do_update {
 	my (@pc) = split m#/#, $path;
 	my $reporter = $self->do_update($rev_b, (@pc ? shift @pc : ''),
 	                                1, $editor, $pool);
-	my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();
+	my @lock = (compare_svn_version('1.2.0') >= 0) ? (undef) : ();
 
 	# Since we can't rely on svn_ra_reparent being available, we'll
 	# just have to do some magic with set_path to make it so
@@ -5723,7 +5741,7 @@ sub gs_do_switch {
 	$ra ||= $self;
 	$url_b = escape_url($url_b);
 	my $reporter = $ra->do_switch($rev_b, '', 1, $url_b, $editor, $pool);
-	my @lock = $SVN::Core::VERSION ge '1.2.0' ? (undef) : ();
+	my @lock = (compare_svn_version('1.2.0') >= 0) ? (undef) : ();
 	$reporter->set_path('', $rev_a, 0, @lock, $pool);
 	$reporter->finish_report($pool);
 

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-05-01  1:08                                   ` Junio C Hamano
@ 2012-06-03 10:49                                     ` Charles Bailey
  2012-06-03 21:44                                       ` Junio C Hamano
  0 siblings, 1 reply; 35+ messages in thread
From: Charles Bailey @ 2012-06-03 10:49 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Eric Wong, git, Gustav Munkby, Edward Rudd, Carsten Bormann

On Mon, Apr 30, 2012 at 06:08:29PM -0700, Junio C Hamano wrote:
> 
> Ok, something along the lines of this.  Perhaps instead of "compare_",
> we may want to call it "require_", so that negative return maps naturally
> to a failure.
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 427da9e..4a2ec43 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl

snip

> @@ -5464,7 +5481,7 @@ sub _auth_providers () {
>  
>  	# earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
>  	# this function
> -	if ($SVN::Core::VERSION gt '1.6.12') {
> +	if (compare_svn_version('1.6.12') > 0) {
>  		my $config = SVN::Core::config_get_config($config_dir);
>  		my ($p, @a);
>  		# config_get_config returns all config files from

I presume this patch turned into this commit:

commit f760c903b8525878cd3b426fc61a7a2cf8742609
Author: Junio C Hamano <gitster@pobox.com>
Date:   Wed May 2 19:53:50 2012 +0000

    git-svn: introduce SVN version comparison function

Although it was advertised as fixing the breakage introduced by
082afee621aeb2d3746c8ae290af98823f981f34 it didn't fix things for me.
Although I haven't investigate why, trial and error proved that I
could fix it if I changed the comparison to:

       if (::compare_svn_version('1.6.13') > 0) {

For me:
$ svn --version --quiet
1.6.13

Is this the correct fix or do I have a bad svn install on my linux
box?

Charles.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-06-03 10:49                                     ` Charles Bailey
@ 2012-06-03 21:44                                       ` Junio C Hamano
  2012-06-04  9:00                                         ` Matthijs Kooijman
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2012-06-03 21:44 UTC (permalink / raw)
  To: Charles Bailey
  Cc: Eric Wong, git, Gustav Munkby, Edward Rudd, Carsten Bormann,
	Matthijs Kooijman

Charles Bailey <charles@hashpling.org> writes:

>> @@ -5464,7 +5481,7 @@ sub _auth_providers () {
>>  
>>  	# earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
>>  	# this function
>> -	if ($SVN::Core::VERSION gt '1.6.12') {
>> +	if (compare_svn_version('1.6.12') > 0) {
>>  		my $config = SVN::Core::config_get_config($config_dir);
>>  		my ($p, @a);
>>  		# config_get_config returns all config files from
>
> I presume this patch turned into this commit:
>
> commit f760c903b8525878cd3b426fc61a7a2cf8742609
> Author: Junio C Hamano <gitster@pobox.com>
> Date:   Wed May 2 19:53:50 2012 +0000
>
>     git-svn: introduce SVN version comparison function
>
> Although it was advertised as fixing the breakage introduced by
> 082afee621aeb2d3746c8ae290af98823f981f34 it didn't fix things for me.
> Although I haven't investigate why, trial and error proved that I
> could fix it if I changed the comparison to:
>
>        if (::compare_svn_version('1.6.13') > 0) {
>
> For me:
> $ svn --version --quiet
> 1.6.13
>
> Is this the correct fix or do I have a bad svn install on my linux
> box?

The conversion from "gt '1.6.12'" to "compare() > 0" is correct, I
think, as the original wanted to make sure that you have 1.6.13 or
newer (in other words, '1.6.12' is not good enough, but '1.6.13' is).
The fix was about not doing string comparison between the version
string and '1.6.12' string, i.e.

	if ("1.6.9" gt '1.6.12') {
		use the logic for newer versions
	}

incorrectly used the logic for newer versions.  That was the only
thing f760c90 (git-svn: introduce SVN version comparison function,
2012-05-02) addressed.

It is possible that '1.6.12' is not the right cut-off point and the
logic may require a version newer than that but that is outside the
scope of f760c90.

The cut-off point comes from 082afee (git-svn: use platform specific
auth providers, 2012-04-26).

Matthijs?  Eric?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-06-03 21:44                                       ` Junio C Hamano
@ 2012-06-04  9:00                                         ` Matthijs Kooijman
  2012-06-04 19:26                                           ` Junio C Hamano
  0 siblings, 1 reply; 35+ messages in thread
From: Matthijs Kooijman @ 2012-06-04  9:00 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Charles Bailey, Eric Wong, git, Gustav Munkby, Edward Rudd,
	Carsten Bormann

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

Hey folks,

> It is possible that '1.6.12' is not the right cut-off point and the
> logic may require a version newer than that but that is outside the
> scope of f760c90.
It seems this is indeed the case. I can't remember what I based the
1.6.12 on, but looking at the svn changelog [1], the first working version
is 1.6.15:

    Version 1.6.15
    [...]
       * improve some swig parameter mapping (r984565, r1035745)

[1]: http://svn.apache.org/repos/asf/subversion/trunk/CHANGES

Gr.

Matthijs

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-06-04  9:00                                         ` Matthijs Kooijman
@ 2012-06-04 19:26                                           ` Junio C Hamano
  2012-06-04 19:36                                             ` Eric Wong
  0 siblings, 1 reply; 35+ messages in thread
From: Junio C Hamano @ 2012-06-04 19:26 UTC (permalink / raw)
  To: Matthijs Kooijman, Eric Wong
  Cc: Charles Bailey, git, Gustav Munkby, Edward Rudd, Carsten Bormann

Matthijs Kooijman <matthijs@stdin.nl> writes:

> It seems this is indeed the case. I can't remember what I based the
> 1.6.12 on, but looking at the svn changelog [1], the first working version
> is 1.6.15:
>
>     Version 1.6.15
>     [...]
>        * improve some swig parameter mapping (r984565, r1035745)
>
> [1]: http://svn.apache.org/repos/asf/subversion/trunk/CHANGES
>
> Gr.

Thanks.  I find that "improve some mapping" is a bit too subtle way
to say "we are fixing a bug that can lead to a segfault" to my
taste, though ;-)

Eric, I can directly queue this as a regression fix in my tree, you
can eyeball and give your blessing (or "No, that is wrong--here is
the right version" is even better), or you can queue it and tell me
to pull from you.  How do we want to proceed?

-- >8 --
Subject: [PATCH] git-svn: platform auth providers are working only on 1.6.15
 or newer

Matthijs Kooijman reports that the cut-off point 082afee (git-svn:
use platform specific auth providers, 2012-04-26) set at 1.6.12 to
use this feature safely was incorrect, and it is 1.6.15 instead:

    http://svn.apache.org/repos/asf/subversion/trunk/CHANGES
    Version 1.6.15
       * improve some swig parameter mapping (r984565, r1035745)

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-svn.perl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/git-svn.perl b/git-svn.perl
index 1a17f94..abbd6b8 100755
--- a/git-svn.perl
+++ b/git-svn.perl
@@ -5481,7 +5481,7 @@ ()
 
 	# earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
 	# this function
-	if (::compare_svn_version('1.6.12') > 0) {
+	if (::compare_svn_version('1.6.15') >= 0) {
 		my $config = SVN::Core::config_get_config($config_dir);
 		my ($p, @a);
 		# config_get_config returns all config files from
-- 
1.7.11.rc1.2.g33fe195

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH 1/2] git-svn: use platform specific auth providers
  2012-06-04 19:26                                           ` Junio C Hamano
@ 2012-06-04 19:36                                             ` Eric Wong
  0 siblings, 0 replies; 35+ messages in thread
From: Eric Wong @ 2012-06-04 19:36 UTC (permalink / raw)
  To: Junio C Hamano
  Cc: Matthijs Kooijman, Charles Bailey, git, Gustav Munkby,
	Edward Rudd, Carsten Bormann

Junio C Hamano <gitster@pobox.com> wrote:
> Eric, I can directly queue this as a regression fix in my tree, you
> can eyeball and give your blessing (or "No, that is wrong--here is
> the right version" is even better), or you can queue it and tell me
> to pull from you.  How do we want to proceed?

Looks reasonable, you can queue directly
Acked-by: Eric Wong <normalperson@yhbt.net>

> -- >8 --
> Subject: [PATCH] git-svn: platform auth providers are working only on 1.6.15
>  or newer
> 
> Matthijs Kooijman reports that the cut-off point 082afee (git-svn:
> use platform specific auth providers, 2012-04-26) set at 1.6.12 to
> use this feature safely was incorrect, and it is 1.6.15 instead:
> 
>     http://svn.apache.org/repos/asf/subversion/trunk/CHANGES
>     Version 1.6.15
>        * improve some swig parameter mapping (r984565, r1035745)
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  git-svn.perl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/git-svn.perl b/git-svn.perl
> index 1a17f94..abbd6b8 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -5481,7 +5481,7 @@ ()
>  
>  	# earlier 1.6.x versions would segfault, and <= 1.5.x didn't have
>  	# this function
> -	if (::compare_svn_version('1.6.12') > 0) {
> +	if (::compare_svn_version('1.6.15') >= 0) {
>  		my $config = SVN::Core::config_get_config($config_dir);
>  		my ($p, @a);
>  		# config_get_config returns all config files from
> -- 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH 2/2] git-svn: Configure a prompt callback for gnome_keyring.
  2012-04-27  8:28                 ` Eric Wong
  2012-04-27  9:36                   ` Matthijs Kooijman
@ 2013-06-18 16:36                   ` Matthijs Kooijman
  2013-06-18 16:38                     ` [PATCH] " Matthijs Kooijman
  1 sibling, 1 reply; 35+ messages in thread
From: Matthijs Kooijman @ 2013-06-18 16:36 UTC (permalink / raw)
  To: Eric Wong; +Cc: Gustav Munkby, Edward Rudd, Carsten Bormann, git

Hi folks,

On Fri, Apr 27, 2012 at 08:28:40AM +0000, Eric Wong wrote:
> Matthijs Kooijman <matthijs@stdin.nl> wrote:
> > This allows git-svn to prompt for a keyring unlock password, when a
> > the needed gnome keyring is locked.
> > 
> > This requires changes in the subversion perl bindings which have been
> > committed to trunk (1241554 and some followup commits) and should be
> > available with the (as of yet unreleased) 1.8.0 release.
> 
> I'm a hesitant to use/depend on unreleased functionality in SVN.
> 
> Is there a chance the API could change before the release.  Also,
> what kind of tests do the SVN guys do on the Perl bindings + GNOME?
> I'm especially concerned since we just worked around segfault
> bugs in the other patch.
> 
> Can we put this on hold until somebody can test the 1.8.0 release?

After over a year, Subversion has finally started with 1.8.0 release
candidates. I've rebased this patch and succesfully tested it against
1.8.0-rc3.

I'll send the updated patch over as a reply to this mail.


As for testing, it took a bit of messing to get all the paths correct,
so I'll document what I did here.

I used a the 1.8.0-rc3 tarball and ran:

	subversion-1.8.0-rc3$ ./configure --prefix=/usr/local/svn
	subversion-1.8.0-rc3$ make all install
	subversion-1.8.0-rc3$ make swig-pl install-swig-pl PREFIX=/usr/local/svn

I took a git master checkout with the patch applied and ran:

	git$ make prefix=/usr/local/git all install

Then, inside some git-svn clone, I ran:

	$ PERL5LIB=/usr/local/svn/local/lib/perl/5.14.2/ LD_LIBRARY_PATH=/usr/local/svn/lib/  /usr/local/git/bin/git svn rebase
	Password for 'default' GNOME keyring:
	Current branch master is up to date.

When removing the PERL5LIB and LD_LIBRARY_PATH variables to run against
the system version of subversion (1.6.17 here), I get an authorization
failure as before:

	$ /usr/local/git/bin/git svn rebase
	Authorization failed: OPTIONS of 'http://svn.example.org': authorization failed: Could not authenticate to server: rejected Basic challenge (http://example.org) at /usr/local/git/share/perl/5.14.2/Git/SVN.pm line 717

Gr.

Matthijs

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH] git-svn: Configure a prompt callback for gnome_keyring.
  2013-06-18 16:36                   ` Matthijs Kooijman
@ 2013-06-18 16:38                     ` Matthijs Kooijman
  2013-08-29  9:42                       ` Matthijs Kooijman
  0 siblings, 1 reply; 35+ messages in thread
From: Matthijs Kooijman @ 2013-06-18 16:38 UTC (permalink / raw)
  To: git
  Cc: Eric Wong, Gustav Munkby, Edward Rudd, Carsten Bormann,
	Matthijs Kooijman

This allows git-svn to prompt for a keyring unlock password, when a
the needed gnome keyring is locked.

This requires changes in the subversion perl bindings which have been
committed to svn trunk (r1241554 and some followup commits) and are
first available in the 1.8.0 release.
---
 perl/Git/SVN/Prompt.pm |  5 +++++
 perl/Git/SVN/Ra.pm     | 13 +++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm
index e940b08..faeda01 100644
--- a/perl/Git/SVN/Prompt.pm
+++ b/perl/Git/SVN/Prompt.pm
@@ -23,6 +23,11 @@ sub simple {
 	$SVN::_Core::SVN_NO_ERROR;
 }
 
+sub gnome_keyring_unlock {
+	my ($keyring, $pool) = @_;
+	_read_password("Password for '$keyring' GNOME keyring: ", undef);
+}
+
 sub ssl_server_trust {
 	my ($cred, $realm, $failures, $cert_info, $may_save, $pool) = @_;
 	$may_save = undef if $_no_auth_cache;
diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
index 75ecc42..38ed0cb 100644
--- a/perl/Git/SVN/Ra.pm
+++ b/perl/Git/SVN/Ra.pm
@@ -104,6 +104,19 @@ sub new {
 		}
 	} # no warnings 'once'
 
+	# Allow git-svn to show a prompt for opening up a gnome-keyring, if needed.
+	if (defined(&SVN::Core::auth_set_gnome_keyring_unlock_prompt_func)) {
+		my $keyring_callback = SVN::Core::auth_set_gnome_keyring_unlock_prompt_func(
+			$baton,
+			\&Git::SVN::Prompt::gnome_keyring_unlock
+		);
+		# Keep a reference to this callback, to prevent the function
+		# (reference) from being garbage collected.  We just add it to
+		# the callbacks value, which are also used only to prevent the
+		# garbage collector from eating stuff.
+		$callbacks = [$callbacks, $keyring_callback]
+	}
+
 	my $self = SVN::Ra->new(url => $url, auth => $baton,
 	                      config => $config,
 			      pool => SVN::Pool->new,
-- 
1.8.3.rc1

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH] git-svn: Configure a prompt callback for gnome_keyring.
  2013-06-18 16:38                     ` [PATCH] " Matthijs Kooijman
@ 2013-08-29  9:42                       ` Matthijs Kooijman
  2013-08-29 17:38                         ` Eric Wong
  0 siblings, 1 reply; 35+ messages in thread
From: Matthijs Kooijman @ 2013-08-29  9:42 UTC (permalink / raw)
  To: git; +Cc: Eric Wong, Gustav Munkby, Edward Rudd, Carsten Bormann

Hi folks,

any chance this patch can be merged?

Gr.

Matthijs

On Tue, Jun 18, 2013 at 06:38:10PM +0200, Matthijs Kooijman wrote:
> This allows git-svn to prompt for a keyring unlock password, when a
> the needed gnome keyring is locked.
> 
> This requires changes in the subversion perl bindings which have been
> committed to svn trunk (r1241554 and some followup commits) and are
> first available in the 1.8.0 release.
> ---
>  perl/Git/SVN/Prompt.pm |  5 +++++
>  perl/Git/SVN/Ra.pm     | 13 +++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/perl/Git/SVN/Prompt.pm b/perl/Git/SVN/Prompt.pm
> index e940b08..faeda01 100644
> --- a/perl/Git/SVN/Prompt.pm
> +++ b/perl/Git/SVN/Prompt.pm
> @@ -23,6 +23,11 @@ sub simple {
>  	$SVN::_Core::SVN_NO_ERROR;
>  }
>  
> +sub gnome_keyring_unlock {
> +	my ($keyring, $pool) = @_;
> +	_read_password("Password for '$keyring' GNOME keyring: ", undef);
> +}
> +
>  sub ssl_server_trust {
>  	my ($cred, $realm, $failures, $cert_info, $may_save, $pool) = @_;
>  	$may_save = undef if $_no_auth_cache;
> diff --git a/perl/Git/SVN/Ra.pm b/perl/Git/SVN/Ra.pm
> index 75ecc42..38ed0cb 100644
> --- a/perl/Git/SVN/Ra.pm
> +++ b/perl/Git/SVN/Ra.pm
> @@ -104,6 +104,19 @@ sub new {
>  		}
>  	} # no warnings 'once'
>  
> +	# Allow git-svn to show a prompt for opening up a gnome-keyring, if needed.
> +	if (defined(&SVN::Core::auth_set_gnome_keyring_unlock_prompt_func)) {
> +		my $keyring_callback = SVN::Core::auth_set_gnome_keyring_unlock_prompt_func(
> +			$baton,
> +			\&Git::SVN::Prompt::gnome_keyring_unlock
> +		);
> +		# Keep a reference to this callback, to prevent the function
> +		# (reference) from being garbage collected.  We just add it to
> +		# the callbacks value, which are also used only to prevent the
> +		# garbage collector from eating stuff.
> +		$callbacks = [$callbacks, $keyring_callback]
> +	}
> +
>  	my $self = SVN::Ra->new(url => $url, auth => $baton,
>  	                      config => $config,
>  			      pool => SVN::Pool->new,
> -- 
> 1.8.3.rc1
> 

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] git-svn: Configure a prompt callback for gnome_keyring.
  2013-08-29  9:42                       ` Matthijs Kooijman
@ 2013-08-29 17:38                         ` Eric Wong
       [not found]                           ` <35AE7D09-F859-4277-AC74-729BA1188D10@outoforder.cc>
  0 siblings, 1 reply; 35+ messages in thread
From: Eric Wong @ 2013-08-29 17:38 UTC (permalink / raw)
  To: Matthijs Kooijman, git, Gustav Munkby, Edward Rudd,
	Carsten Bormann

Matthijs Kooijman <matthijs@stdin.nl> wrote:
> Hi folks,
> 
> any chance this patch can be merged?

It's probably fine.  Does anybody else have testing/feedback?  I haven't
used git-svn/SVN in years, and I don't use GNOME (nor much GUI).

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH] git-svn: Configure a prompt callback for gnome_keyring.
       [not found]                           ` <35AE7D09-F859-4277-AC74-729BA1188D10@outoforder.cc>
@ 2013-08-29 17:56                             ` Eric Wong
  0 siblings, 0 replies; 35+ messages in thread
From: Eric Wong @ 2013-08-29 17:56 UTC (permalink / raw)
  To: Edward Rudd; +Cc: Matthijs Kooijman, git, Gustav Munkby, Carsten Bormann

Edward Rudd <urkle@outoforder.cc> wrote:
> Where is a link to the latest patch? I can give it a quick once-over
> with one of my git-svn'ed game ports I'm working on. (due for
> launching on the 10th.. WOOHOO!)

http://mid.gmane.org/1371573490-21973-1-git-send-email-matthijs@stdin.nl
Thanks!

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2013-08-29 17:56 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-18  0:36 git-svn won't remember pem password Igor
2012-02-18 11:30 ` Jakub Narebski
2012-02-19  4:03   ` Nikolaus Demmel
2012-02-20  0:57   ` Jeff King
2012-02-20  3:08     ` Nikolaus Demmel
2012-02-19  1:30 ` Eric Wong
2012-04-26 18:00   ` Matthijs Kooijman
     [not found]     ` <41A093CB-CA4A-4FEF-9F5C-A9B626D10AFB@gmail.com>
     [not found]       ` <20120426181327.GZ4023@login.drsnuggles.stderr.nl>
2012-04-26 18:31         ` Igor
2012-04-26 18:36           ` Matthijs Kooijman
2012-04-26 19:34             ` [PATCH 1/2] git-svn: use platform specific auth providers Matthijs Kooijman
2012-04-26 19:34               ` [PATCH 2/2] git-svn: Configure a prompt callback for gnome_keyring Matthijs Kooijman
2012-04-27  8:28                 ` Eric Wong
2012-04-27  9:36                   ` Matthijs Kooijman
2013-06-18 16:36                   ` Matthijs Kooijman
2013-06-18 16:38                     ` [PATCH] " Matthijs Kooijman
2013-08-29  9:42                       ` Matthijs Kooijman
2013-08-29 17:38                         ` Eric Wong
     [not found]                           ` <35AE7D09-F859-4277-AC74-729BA1188D10@outoforder.cc>
2013-08-29 17:56                             ` Eric Wong
2012-04-27  8:29                 ` [PATCH 2/2] " Matthijs Kooijman
2012-04-27  8:21               ` [PATCH 1/2] git-svn: use platform specific auth providers Eric Wong
2012-04-27  8:25                 ` Matthijs Kooijman
2012-04-29  8:23                   ` Eric Wong
2012-04-30  0:03                     ` Junio C Hamano
2012-04-30  1:21                       ` Junio C Hamano
2012-04-30  8:19                         ` Eric Wong
2012-04-30 16:04                           ` Junio C Hamano
2012-04-30 16:53                             ` Matthijs Kooijman
2012-04-30 19:02                               ` Eric Wong
2012-04-30 19:20                                 ` Matthijs Kooijman
2012-05-01  1:08                                   ` Junio C Hamano
2012-06-03 10:49                                     ` Charles Bailey
2012-06-03 21:44                                       ` Junio C Hamano
2012-06-04  9:00                                         ` Matthijs Kooijman
2012-06-04 19:26                                           ` Junio C Hamano
2012-06-04 19:36                                             ` Eric Wong

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).