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: > 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 { >
Attachment:
signature.asc
Description: Digital signature