On 6/14/13 5:20 AM, Ben Reser wrote:
> Actually now that I think about it a bit more I'm not sure that helps.
> It'll probably just shift it to a link time error because the defines
> will be there so it'll build, SWIG will generate wrappers for them
> that depend on the functions in our libraries, which won't be
> available in the libraries they built because we exclude them on their
> platform.
>
> We can work around that by adding logic to SWIG to specifically
> exclude these functions based on the same conditions as we have in our
> headers and build systems. But then we've added yet another place to
> maintain this.
>
> So it turns into hacks all the way down.
>
> The alternative is to fix it so the error happens at runtime, which
> shifts that to the library user. In these specific cases, simply
> providing a noop provider, hides the entire problem from even the
> library user.
So fixing this doesn't require this level of effort. Daniel's original
suggestion actually would have worked fine for a couple of reasons.
1) The problem is actually caused by r1241554 and the various follow ups trying
to fix it. This change added support for the callback for retrieving the
gnome-keyring passphrase from the user to unlock the keyring to the bindings.
Having the typedef even when the gnome keyring isn't built is no problem since
even if a client registers the callback it'll never be called and there's
really nothing specific to gnome-keyring about it. So this typedef and the
supporting constants shouldn't have been excluded based on platform.
2) We already exclude the platform specific provider functions from the
bindings. So the problem I was concerned about at link time doesn't happen.
They can still use platform specific providers they just need to use
svn_auth_get_platform_specific_provider() or
svn_auth_get_platform_specific_client_providers().
So the fix I'm about to commit simply moves the #if directive down so that the
typedef and macros are available regardless of platform.
We still could go to the effort of making all of the auth apis available
regardless of platform and providing noop implementations of the functions on
platforms without the support. But we'd have to build the
libsvn_auth_gnome_keyring and libsvn_auth_kwallet libraries on all platforms
(though not linked against gnome or kwallet unless they were found/enabled).
We'd have to do this because we can't move the symbols between libraries
without breaking the ABI. Anyone uses the
svn_auth_get_{gnome_keyring,kwallet)_*() functions has to be loading the
dynamic library and then doing a symbol search.
To that end I produced a patch (which I've attached for posterity's sake). But
I don't see a reason to actually apply it. The only benefit would be allowing
cross platform clients to use these apis without caring about the platform or
to allow libraries on clients to be replaced without making sure to match the
platform specific auth support that their original library had if they're using
it. So until such a time that someone shows up with such a problem I'm
inclined to punt on this issue.
For that matter I intend to mark these get apis as deprecated by the
svn_auth_get_platform_specific_provider() or
svn_get_get_platform_specific_client_providers() apis.
Received on 2013-11-20 22:15:32 CET