{"thread":{"id":"27392","subject":"[PATCH] git-svn: enable platform-specific authentication","startedAt":"2011-05-18T08:45:20Z","lastAt":"2012-02-19T04:06:11Z","messageCount":5,"participants":["Gustav Munkby","Eric Wong","Matthijs Kooijman","Nikolaus Demmel"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"168119","messageId":"1305708320-8614-1-git-send-email-grddev@gmail.com","threadId":"27392","inReplyTo":null,"subject":"[PATCH] git-svn: enable platform-specific authentication","fromName":"Gustav Munkby","fromEmail":"grddev@gmail.com","sentAt":"2011-05-18T08:45:20Z","receivedAt":"2011-05-18T08:45:20Z","isPatch":true,"sender":{"key":"grddev@gmail.com","avatar":null},"body":"Use the platform-specific authentication providers that are\nexposed to subversion bindings starting with subversion 1.6.\n\nSigned-off-by: Gustav Munkby <grddev@gmail.com>\n---\n git-svn.perl |    3 +++\n 1 files changed, 3 insertions(+), 0 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 0fd2fd2..3f7c3c8 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4930,6 +4930,9 @@ BEGIN {\n \n sub _auth_providers () {\n \t[\n+\t  $SVN::Core::VERSION lt '1.6' ? () :\n+\t    @{SVN::Core::auth_get_platform_specific_client_providers(\n+\t      undef,undef)},\n \t  SVN::Client::get_simple_provider(),\n \t  SVN::Client::get_ssl_server_trust_file_provider(),\n \t  SVN::Client::get_simple_prompt_provider(\n-- \n1.7.5.1\n"},{"id":"168155","messageId":"20110518195710.GA10697@dcvr.yhbt.net","threadId":"27392","inReplyTo":"1305708320-8614-1-git-send-email-grddev@gmail.com","subject":"Re: [PATCH] git-svn: enable platform-specific authentication","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2011-05-18T19:57:10Z","receivedAt":"2011-05-18T19:57:10Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Gustav Munkby <grddev@gmail.com> wrote:\n> Use the platform-specific authentication providers that are\n> exposed to subversion bindings starting with subversion 1.6.\n\nThis came up several months ago, I understand there were some issues\nwith the SVN Perl bindings.  Cc-ing interested parties.\n\n>  sub _auth_providers () {\n>  \t[\n> +\t  $SVN::Core::VERSION lt '1.6' ? () :\n> +\t    @{SVN::Core::auth_get_platform_specific_client_providers(\n> +\t      undef,undef)},\n\nI think it needs to take into account the config from\nSVN::Core::config_get_config, otherwise people with non-standard SVN\nconfigurations could get locked out.  I seem to recall this was the\nbroken part in the SVN Perl bindings, but one of the Cc-ed parties would\nknow for sure.\n\n-- \nEric Wong\n"},{"id":"173224","messageId":"20110809210638.GK6418@login.drsnuggles.stderr.nl","threadId":"27392","inReplyTo":"20110518195710.GA10697@dcvr.yhbt.net","subject":"Re: [PATCH] git-svn: enable platform-specific authentication","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2011-08-09T21:06:39Z","receivedAt":"2011-08-09T21:06:39Z","isPatch":true,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"Hey folks,\n\n> > Use the platform-specific authentication providers that are\n> > exposed to subversion bindings starting with subversion 1.6.\n> \n> This came up several months ago, I understand there were some issues\n> with the SVN Perl bindings.  Cc-ing interested parties.\nI missed the CC, sorry for that.\n\n> >  sub _auth_providers () {\n> >  \t[\n> > +\t  $SVN::Core::VERSION lt '1.6' ? () :\n> > +\t    @{SVN::Core::auth_get_platform_specific_client_providers(\n> > +\t      undef,undef)},\n> \n> I think it needs to take into account the config from\n> SVN::Core::config_get_config, otherwise people with non-standard SVN\n> configurations could get locked out.  I seem to recall this was the\n> broken part in the SVN Perl bindings, but one of the Cc-ed parties would\n> know for sure.\n\nIndeed, but a proposed patch by Eric for this did not work. I solved the\nproblem quite some time ago, but apparently I never sent out the\nsolution (I think I got distracted by trying to get a passphrase prompt\nto unlock locked keychains). I couldn't find my fixes anymore either,\nbut I think I've managed to reproduce them just now.\n\nSome basic testing shows below patch works, but I think it might need\nsome more testing and work. At least the below patch allows for example\nto disable the gnome-keyring provider from a different svn config\ndirectory by passing --config-dir /some/path to git-svn (which is not\npossible using above patch passing undef, which will only read from\n~/.subversion).\n\nUsing strace, I did notice that git-svn still reads stuff\nfrom ~/.subversion/auth/svn.ssl.server/ and\n.subversion/auth/svn.simple/, but I couldn't exactly find why this is\nright away. In any case, it also happens without this patch applied, so\nI guess it's a completely separate issue.\n\nAs for the actual patch, notice that config_get_config returns a hash\nthat consists again of a \"config\" and \"servers\" patch. Previous attempts\nat this patch passed the entire hash to\nauth_get_platform_specific_client_providers, but it only wants the\n\"client\" part. It's a bit confusing until you realize that the\nconfig_get_config return value represents your ~/.subversion directory,\nwhich again contains a \"config\" and \"servers\" file.\n\nI'm not 100% sure this patch is correct as it is now. I hope to get\nanother look at my \"automatically unlock keychain\" work tomorrow,\nin case there are some hints about flaws in this patch there. In the\nmeanwhile, feedback on this patch is welcome.\n\nGr.\n\nMatthijs\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex da3fea8..6dc5196 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -4916,7 +4916,7 @@ BEGIN {\n }\n \n sub _auth_providers () {\n-       [\n+       my @rv = (\n          SVN::Client::get_simple_provider(),\n          SVN::Client::get_ssl_server_trust_file_provider(),\n          SVN::Client::get_simple_prompt_provider(\n@@ -4932,7 +4932,23 @@ sub _auth_providers () {\n            \\&Git::SVN::Prompt::ssl_server_trust),\n          SVN::Client::get_username_prompt_provider(\n            \\&Git::SVN::Prompt::username, 2)\n-       ]\n+       );\n+\n+       # earlier 1.6.x versions would segfault, and <= 1.5.x didn't have\n+       # this function\n+       if ($SVN::Core::VERSION gt '1.6.12') {\n+               my $config = SVN::Core::config_get_config($config_dir);\n+               my ($p, @a);\n+              # config_get_config returns all config files from\n+              # ~/.subversion, auth_get_platform_specific_client_providers\n+              # just wants the config \"file\".\n+               @a = ($config->{'config'}, undef);\n+               $p = SVN::Core::auth_get_platform_specific_client_providers(@a);\n+              # Insert the return value from\n+              # auth_get_platform_specific_providers\n+               unshift @rv, @$p;\n+       }\n+       \\@rv;\n }\n \n sub escape_uri_only {\n\n"},{"id":"181895","messageId":"20120103204403.GI17548@login.drsnuggles.stderr.nl","threadId":"27392","inReplyTo":"20110809210638.GK6418@login.drsnuggles.stderr.nl","subject":"Re: [PATCH] git-svn: enable platform-specific authentication","fromName":"Matthijs Kooijman","fromEmail":"matthijs@stdin.nl","sentAt":"2012-01-03T20:44:04Z","receivedAt":"2012-01-03T20:44:04Z","isPatch":true,"sender":{"key":"matthijs@stdin.nl","avatar":"https://avatars.githubusercontent.com/u/194491?v=4"},"body":"Hey folks,\n\nI sent the below patch a few months ago, and not having it applied in\ngit-svn bit me again just now. Did any of you get a chance to have a\nlook at it?\n\nI'm still not 100% sure if this patch is correct for all the corner\ncases, but it works like a charm in the regular case.\n\nPerhaps it should just be included as is?\n\nGr.\n\nMatthijs\n\nOn Tue, Aug 09, 2011 at 11:06:38PM +0200, Matthijs Kooijman wrote:\n> Hey folks,\n> \n> > > Use the platform-specific authentication providers that are\n> > > exposed to subversion bindings starting with subversion 1.6.\n> > \n> > This came up several months ago, I understand there were some issues\n> > with the SVN Perl bindings.  Cc-ing interested parties.\n> I missed the CC, sorry for that.\n> \n> > >  sub _auth_providers () {\n> > >  \t[\n> > > +\t  $SVN::Core::VERSION lt '1.6' ? () :\n> > > +\t    @{SVN::Core::auth_get_platform_specific_client_providers(\n> > > +\t      undef,undef)},\n> > \n> > I think it needs to take into account the config from\n> > SVN::Core::config_get_config, otherwise people with non-standard SVN\n> > configurations could get locked out.  I seem to recall this was the\n> > broken part in the SVN Perl bindings, but one of the Cc-ed parties would\n> > know for sure.\n> \n> Indeed, but a proposed patch by Eric for this did not work. I solved the\n> problem quite some time ago, but apparently I never sent out the\n> solution (I think I got distracted by trying to get a passphrase prompt\n> to unlock locked keychains). I couldn't find my fixes anymore either,\n> but I think I've managed to reproduce them just now.\n> \n> Some basic testing shows below patch works, but I think it might need\n> some more testing and work. At least the below patch allows for example\n> to disable the gnome-keyring provider from a different svn config\n> directory by passing --config-dir /some/path to git-svn (which is not\n> possible using above patch passing undef, which will only read from\n> ~/.subversion).\n> \n> Using strace, I did notice that git-svn still reads stuff\n> from ~/.subversion/auth/svn.ssl.server/ and\n> .subversion/auth/svn.simple/, but I couldn't exactly find why this is\n> right away. In any case, it also happens without this patch applied, so\n> I guess it's a completely separate issue.\n> \n> As for the actual patch, notice that config_get_config returns a hash\n> that consists again of a \"config\" and \"servers\" patch. Previous attempts\n> at this patch passed the entire hash to\n> auth_get_platform_specific_client_providers, but it only wants the\n> \"client\" part. It's a bit confusing until you realize that the\n> config_get_config return value represents your ~/.subversion directory,\n> which again contains a \"config\" and \"servers\" file.\n> \n> I'm not 100% sure this patch is correct as it is now. I hope to get\n> another look at my \"automatically unlock keychain\" work tomorrow,\n> in case there are some hints about flaws in this patch there. In the\n> meanwhile, feedback on this patch is welcome.\n> \n> Gr.\n> \n> Matthijs\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index da3fea8..6dc5196 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -4916,7 +4916,7 @@ BEGIN {\n>  }\n>  \n>  sub _auth_providers () {\n> -       [\n> +       my @rv = (\n>           SVN::Client::get_simple_provider(),\n>           SVN::Client::get_ssl_server_trust_file_provider(),\n>           SVN::Client::get_simple_prompt_provider(\n> @@ -4932,7 +4932,23 @@ sub _auth_providers () {\n>             \\&Git::SVN::Prompt::ssl_server_trust),\n>           SVN::Client::get_username_prompt_provider(\n>             \\&Git::SVN::Prompt::username, 2)\n> -       ]\n> +       );\n> +\n> +       # earlier 1.6.x versions would segfault, and <= 1.5.x didn't have\n> +       # this function\n> +       if ($SVN::Core::VERSION gt '1.6.12') {\n> +               my $config = SVN::Core::config_get_config($config_dir);\n> +               my ($p, @a);\n> +              # config_get_config returns all config files from\n> +              # ~/.subversion, auth_get_platform_specific_client_providers\n> +              # just wants the config \"file\".\n> +               @a = ($config->{'config'}, undef);\n> +               $p = SVN::Core::auth_get_platform_specific_client_providers(@a);\n> +              # Insert the return value from\n> +              # auth_get_platform_specific_providers\n> +               unshift @rv, @$p;\n> +       }\n> +       \\@rv;\n>  }\n>  \n>  sub escape_uri_only {\n> \n\n\n"},{"id":"184939","messageId":"1329624371869-7298038.post@n2.nabble.com","threadId":"27392","inReplyTo":"20120103204403.GI17548@login.drsnuggles.stderr.nl","subject":"Re: [PATCH] git-svn: enable platform-specific authentication","fromName":"Nikolaus Demmel","fromEmail":"nikolaus@nikolaus-demmel.de","sentAt":"2012-02-19T04:06:11Z","receivedAt":"2012-02-19T04:06:11Z","isPatch":true,"sender":{"key":"nikolaus@nikolaus-demmel.de","avatar":"https://gravatar.com/avatar/2c9741b87d0f456387a06f577a904933d4614758202292a48ca1c739ebb48916?d=mp&s=160"},"body":"\nMatthijs Kooijman wrote\n> \n> I sent the below patch a few months ago, and not having it applied in\n> git-svn bit me again just now. Did any of you get a chance to have a\n> look at it?\n> \n> I'm still not 100% sure if this patch is correct for all the corner\n> cases, but it works like a charm in the regular case.\n> \n> Perhaps it should just be included as is?\n> \n\nHi,\n\nis this patch also meant to deal with / fix the handling the keychain as an\nauthentication handler on OS X?\n\nIs there anything I could do to help getting this moving forward? I could\ntry test it on OS X, if noone else can.\n\nCheers,\nNikolaus\n\n\n--\nView this message in context: http://git.661346.n2.nabble.com/PATCH-git-svn-enable-platform-specific-authentication-tp6376961p7298038.html\nSent from the git mailing list archive at Nabble.com.\n"}]}