Re: [PATCH] git-svn: enable platform-specific authentication
- From
Matthijs Kooijman <matthijs@stdin.nl>
- Date
- Jan 3, 2012, 20:44 UTC
- Message-ID
- <20120103204403.GI17548@login.drsnuggles.stderr.nl>
- In-Reply-To
- <20110809210638.GK6418@login.drsnuggles.stderr.nl>
Hey folks,
I sent the below patch a few months ago, and not having it applied in git-svn bit me again just now. Did any of you get a chance to have a look at it?
I'm still not 100% sure if this patch is correct for all the corner cases, but it works like a charm in the regular case.
Perhaps it should just be included as is?
Gr.
Matthijs
On Tue, Aug 09, 2011 at 11:06:38PM +0200, Matthijs Kooijman wrote:
Show 96 quoted lines
> Hey folks,
>
> > > Use the platform-specific authentication providers that are
> > > exposed to subversion bindings starting with subversion 1.6.
> >
> > This came up several months ago, I understand there were some issues
> > with the SVN Perl bindings. Cc-ing interested parties.
> I missed the CC, sorry for that.
>
> > > sub _auth_providers () {
> > > [
> > > + $SVN::Core::VERSION lt '1.6' ? () :
> > > + @{SVN::Core::auth_get_platform_specific_client_providers(
> > > + undef,undef)},
> >
> > I think it needs to take into account the config from
> > SVN::Core::config_get_config, otherwise people with non-standard SVN
> > configurations could get locked out. I seem to recall this was the
> > broken part in the SVN Perl bindings, but one of the Cc-ed parties would
> > know for sure.
>
> Indeed, but a proposed patch by Eric for this did not work. I solved the
> problem quite some time ago, but apparently I never sent out the
> solution (I think I got distracted by trying to get a passphrase prompt
> to unlock locked keychains). I couldn't find my fixes anymore either,
> but I think I've managed to reproduce them just now.
>
> Some basic testing shows below patch works, but I think it might need
> some more testing and work. At least the below patch allows for example
> to disable the gnome-keyring provider from a different svn config
> directory by passing --config-dir /some/path to git-svn (which is not
> possible using above patch passing undef, which will only read from
> ~/.subversion).
>
> Using strace, I did notice that git-svn still reads stuff
> from ~/.subversion/auth/svn.ssl.server/ and
> .subversion/auth/svn.simple/, but I couldn't exactly find why this is
> right away. In any case, it also happens without this patch applied, so
> I guess it's a completely separate issue.
>
> As for the actual patch, notice that config_get_config returns a hash
> that consists again of a "config" and "servers" patch. Previous attempts
> at this patch passed the entire hash to
> auth_get_platform_specific_client_providers, but it only wants the
> "client" part. It's a bit confusing until you realize that the
> config_get_config return value represents your ~/.subversion directory,
> which again contains a "config" and "servers" file.
>
> I'm not 100% sure this patch is correct as it is now. I hope to get
> another look at my "automatically unlock keychain" work tomorrow,
> in case there are some hints about flaws in this patch there. In the
> meanwhile, feedback on this patch is welcome.
>
> Gr.
>
> Matthijs
>
> diff --git a/git-svn.perl b/git-svn.perl
> index da3fea8..6dc5196 100755
> --- a/git-svn.perl
> +++ b/git-svn.perl
> @@ -4916,7 +4916,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(
> @@ -4932,7 +4932,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 {
>