[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Remove platform visibility from Subversion auth functions (Was: Python bindings build seems broken)

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Sat, 11 Oct 2008 12:51:01 +0200 (Jerusalem Standard Time)

Jeremy Whitlock wrote on Sat, 11 Oct 2008 at 04:20 -0600:
> > When the platform-specific providers aren't available, Subversion will
> > fall back to the basic provider (store the user/pass in plaintext in
> > ~/.subversion). The fallback will be silent (since the tests disable
> > all interactive prompts, including the 'plaintext passwords' prompt).
> >
> > Therefore, I suspect your patch may be passing the tests but doesn't use
> > keychain at all. Partially because your patch #if's away the
> > declaration of svn_auth_get_keychain_simple_provider()...
> >
> > More below.
>
> I tested the Keychain stuff and it worked as expected. Whenever I
> used the trunk build, with patch, to access something with cached
> credentials stored in the Keychain, the Keychain was accessed. (On
> OSX, I get a prompt asking if it's okay to use the new version of
> Subversion to access the keychain since the cached credentials were
> stored by an older version of Subversion.) Seems to be working fine
> on OSX.

Good to hear that. (I still don't understand how it could have worked,
though.)

> > Also, when I said on IRC that 'individual providers can promise more' -- we
> > could make the individual providers we declare promise to return non-NULL on
> > the appropriate platforms.
> >
> > For example,
> >
> > @@ svn_auth.h @@
> > - * @note This function is only available on Mac OS 10.2 and higher.
> > + * @note This function returns non-NULL iff the platform is Mac OS ™10.2.
> > */
> > void
> > svn_auth_get_keychain_simple_provider(svn_auth_provider_object_t **provider,
> >
> > and then we don't need to touch this code in cmdline.c. (But presumably
> > the bindings will be happier because the functions are defined on all
> > platforms.)
>
> Better documentation makes sense. Are you sure with the "promise" to
> return non-NULL is enough? It just seemed like a good idea although
> the only feasible way to get a NULL was on an unsupported platform.
>

I don't understand the question. If we make a promise in the API, then by
definition callers don't have to check for it (or, callers that *do* check
for it (for paranoia or debugging reasons) should use SVN_ERR_ASSERT or
SVN_ERR_MALFUNCTION.) Does that answer your question?

Thanks,

Daniel

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-10-11 12:51:19 CEST

This is an archived mail posted to the Subversion Dev mailing list.