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.
Index: configure.ac
===================================================================
--- configure.ac (revision 1543908)
+++ configure.ac (working copy)
@@ -859,10 +859,10 @@ AS_HELP_STRING([--disable-plaintext-password-stora
dnl Build and install rules -------------------
-INSTALL_STATIC_RULES="install-bin install-docs"
-INSTALL_RULES="install-fsmod-lib install-ramod-lib install-lib install-include
install-static"
+INSTALL_STATIC_RULES="install-bin install-docs install-kwallet-lib
install-gnome-keyring-lib"
+INSTALL_RULES="install-fsmod-lib install-ramod-lib install-lib
install-kwallet-lib install-gnome-keyring-lib install-include install-static"
INSTALL_RULES="$INSTALL_RULES $INSTALL_APACHE_RULE"
-BUILD_RULES="fsmod-lib ramod-lib lib bin test sub-test $BUILD_APACHE_RULE
tools"
+BUILD_RULES="fsmod-lib ramod-lib lib bin test sub-test $BUILD_APACHE_RULE
tools kwallet-lib gnome-keyring-lib"
if test "$svn_lib_berkeley_db" = "yes"; then
BUILD_RULES="$BUILD_RULES bdb-lib bdb-test"
@@ -878,18 +878,6 @@ if test "$svn_lib_serf" = "yes"; then
INSTALL_STATIC_RULES="$INSTALL_STATIC_RULES install-serf-lib"
fi
-if test "$svn_lib_kwallet" = "yes"; then
- BUILD_RULES="$BUILD_RULES kwallet-lib"
- INSTALL_RULES="`echo $INSTALL_RULES | $SED 's/install-lib/install-lib
install-kwallet-lib/'`"
- INSTALL_STATIC_RULES="$INSTALL_STATIC_RULES install-kwallet-lib"
-fi
-
-if test "$found_gnome_keyring" = "yes"; then
- BUILD_RULES="$BUILD_RULES gnome-keyring-lib"
- INSTALL_RULES="`echo $INSTALL_RULES | $SED 's/install-lib/install-lib
install-gnome-keyring-lib/'`"
- INSTALL_STATIC_RULES="$INSTALL_STATIC_RULES install-gnome-keyring-lib"
-fi
-
if test "$USE_NLS" = "yes"; then
BUILD_RULES="$BUILD_RULES locale"
INSTALL_RULES="$INSTALL_RULES install-locale"
Index: subversion/include/private/svn_auth_private.h
===================================================================
--- subversion/include/private/svn_auth_private.h (revision 1543908)
+++ subversion/include/private/svn_auth_private.h (working copy)
@@ -249,7 +249,44 @@ svn_auth__get_windows_ssl_server_authority_provide
apr_pool_t *pool);
#endif
+/**
+ * Set @a *provider to an authentication provider that implements a
+ * no operation simple provider.
+ *
+ * This function exists to allow get functions for platform specific
+ * providers to still exist in our API even when the provider is not
+ * available.
+ */
+void
+svn_auth__get_noop_simple_provider(svn_auth_provider_object_t **provider,
+ apr_pool_t *pool);
+/**
+ * Set @a *provider to an authentication provider that implements a
+ * no operation SSL client cert password provider
+ *
+ * This function exists to allow get functions for platform specific
+ * providers to still exist in our API even when the provider is not
+ * available.
+ */
+void
+svn_auth__get_noop_ssl_client_cert_pw_provider
+ (svn_auth_provider_object_t **provider,
+ apr_pool_t *pool);
+
+/**
+ * Set @a *provider to an authentication provider that implements a
+ * no operation SSL server trust provider
+ *
+ * This function exists to allow get functions for platform specific
+ * providers to still exist in our API even when the provider is not
+ * available.
+ */
+void
+svn_auth__get_noop_ssl_server_trust_provider
+ (svn_auth_provider_object_t **provider,
+ apr_pool_t *pool);
+
#ifdef __cplusplus
}
#endif /* __cplusplus */
Index: subversion/include/svn_auth.h
===================================================================
--- subversion/include/svn_auth.h (revision 1543908)
+++ subversion/include/svn_auth.h (working copy)
@@ -865,7 +865,6 @@ svn_auth_get_platform_specific_client_providers(
svn_config_t *config,
apr_pool_t *pool);
-#if (defined(WIN32) && !defined(__MINGW32__)) || defined(DOXYGEN)
/**
* Set @a *provider to an authentication provider of type @c
* svn_auth_cred_simple_t that gets/sets information from the user's
@@ -929,9 +928,6 @@ svn_auth_get_windows_ssl_server_trust_provider(
svn_auth_provider_object_t **provider,
apr_pool_t *pool);
-#endif /* WIN32 && !__MINGW32__ || DOXYGEN */
-
-#if defined(DARWIN) || defined(DOXYGEN)
/**
* Set @a *provider to an authentication provider of type @c
* svn_auth_cred_simple_t that gets/sets information from the user's
@@ -964,9 +960,7 @@ void
svn_auth_get_keychain_ssl_client_cert_pw_provider(
svn_auth_provider_object_t **provider,
apr_pool_t *pool);
-#endif /* DARWIN || DOXYGEN */
-#if (!defined(DARWIN) && !defined(WIN32)) || defined(DOXYGEN)
/** A type of callback function for obtaining the GNOME Keyring password.
*
* In this callback, the client should ask the user for default keyring
@@ -1104,9 +1098,7 @@ void
svn_auth_get_kwallet_ssl_client_cert_pw_provider(
svn_auth_provider_object_t **provider,
apr_pool_t *pool);
-#endif /* (!DARWIN && !WIN32) || DOXYGEN */
-#if !defined(WIN32) || defined(DOXYGEN)
/**
* Set @a *provider to an authentication provider of type @c
* svn_auth_cred_simple_t that gets/sets information from the user's
@@ -1126,9 +1118,7 @@ void
svn_auth_get_gpg_agent_simple_provider
(svn_auth_provider_object_t **provider,
apr_pool_t *pool);
-#endif /* !defined(WIN32) || defined(DOXYGEN) */
-
/** Set @a *provider to an authentication provider of type @c
* svn_auth_cred_username_t that gets/sets information from a user's
* ~/.subversion configuration directory. Allocate @a *provider in
Index: subversion/libsvn_auth_gnome_keyring/gnome_keyring.c
===================================================================
--- subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (revision
1543908)
+++ subversion/libsvn_auth_gnome_keyring/gnome_keyring.c (working copy)
@@ -29,8 +29,10 @@
#include <apr_pools.h>
#include <apr_strings.h>
+#ifdef SVN_HAVE_GNOME_KEYRING
#include <glib.h>
#include <gnome-keyring.h>
+#endif
#include "svn_private_config.h"
#include "svn_auth.h"
@@ -47,7 +49,7 @@
/* GNOME Keyring simple provider, puts passwords in GNOME Keyring */
/*-----------------------------------------------------------------------*/
-
+#ifdef SVN_HAVE_GNOME_KEYRING
struct gnome_keyring_baton
{
const char *keyring_name;
@@ -442,6 +444,8 @@ static const svn_auth_provider_t gnome_keyring_sim
simple_gnome_keyring_save_creds
};
+#endif /* SVN_HAVE_GNOME_KEYRING */
+
/* Public API */
void
svn_auth_get_gnome_keyring_simple_provider
@@ -448,6 +452,7 @@ svn_auth_get_gnome_keyring_simple_provider
(svn_auth_provider_object_t **provider,
apr_pool_t *pool)
{
+#ifdef SVN_HAVE_GNOME_KEYRING
svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
po->vtable = &gnome_keyring_simple_provider;
@@ -454,6 +459,9 @@ svn_auth_get_gnome_keyring_simple_provider
*provider = po;
init_gnome_keyring();
+#else
+ return svn_auth__get_noop_simple_provider(provider, pool);
+#endif
}
@@ -462,6 +470,7 @@ svn_auth_get_gnome_keyring_simple_provider
/* puts passphrases in GNOME Keyring */
/*-----------------------------------------------------------------------*/
+#ifdef SVN_HAVE_GNOME_KEYRING
/* Get cached encrypted credentials from the ssl client cert password
provider's cache. */
static svn_error_t *
@@ -500,6 +509,7 @@ static const svn_auth_provider_t gnome_keyring_ssl
NULL,
ssl_client_cert_pw_gnome_keyring_save_creds
};
+#endif /* SVN_HAVE_GNOME_KEYRING */
/* Public API */
void
@@ -507,6 +517,7 @@ svn_auth_get_gnome_keyring_ssl_client_cert_pw_prov
(svn_auth_provider_object_t **provider,
apr_pool_t *pool)
{
+#ifdef SVN_HAVE_GNOME_KEYRING
svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
po->vtable = &gnome_keyring_ssl_client_cert_pw_provider;
@@ -513,4 +524,7 @@ svn_auth_get_gnome_keyring_ssl_client_cert_pw_prov
*provider = po;
init_gnome_keyring();
+#else
+ return svn_auth__get_noop_ssl_client_cert_pw_provider(provider, pool);
+#endif
}
Index: subversion/libsvn_auth_kwallet/kwallet.cpp
===================================================================
--- subversion/libsvn_auth_kwallet/kwallet.cpp (revision 1543908)
+++ subversion/libsvn_auth_kwallet/kwallet.cpp (working copy)
@@ -34,6 +34,7 @@
#include <apr_pools.h>
#include <apr_strings.h>
+#ifdef SVN_HAVE_KWALLET
#include <dbus/dbus.h>
#include <QtCore/QCoreApplication>
#include <QtCore/QString>
@@ -43,6 +44,7 @@
#include <kcomponentdata.h>
#include <klocalizedstring.h>
#include <kwallet.h>
+#endif
#include "svn_auth.h"
#include "svn_config.h"
@@ -61,6 +63,7 @@
/* KWallet simple provider, puts passwords in KWallet */
/*-----------------------------------------------------------------------*/
+#ifdef SVN_HAVE_KWALLET
static int q_argc = 1;
static char q_argv0[] = "svn"; // Build non-const char * from string constant
static char *q_argv[] = { q_argv0 };
@@ -378,6 +381,7 @@ static const svn_auth_provider_t kwallet_simple_pr
NULL,
kwallet_simple_save_creds
};
+#endif /* SVN_HAVE_KWALLET */
/* Public API */
extern "C" {
@@ -385,11 +389,15 @@ void
svn_auth_get_kwallet_simple_provider(svn_auth_provider_object_t **provider,
apr_pool_t *pool)
{
+#ifdef SVN_HAVE_KWALLET
svn_auth_provider_object_t *po =
static_cast<svn_auth_provider_object_t *> (apr_pcalloc(pool, sizeof(*po)));
po->vtable = &kwallet_simple_provider;
*provider = po;
+#else
+ return svn_auth__get_noop_simple_provider(provider, pool);
+#endif
}
}
@@ -399,6 +407,7 @@ svn_auth_get_kwallet_simple_provider(svn_auth_prov
/* puts passphrases in KWallet */
/*-----------------------------------------------------------------------*/
+#ifdef SVN_HAVE_KWALLET
/* Get cached encrypted credentials from the ssl client cert password
provider's cache. */
static svn_error_t *
@@ -441,6 +450,7 @@ static const svn_auth_provider_t kwallet_ssl_clien
NULL,
kwallet_ssl_client_cert_pw_save_creds
};
+#endif /* SVN_HAVE_KWALLET */
/* Public API */
extern "C" {
@@ -449,10 +459,14 @@ svn_auth_get_kwallet_ssl_client_cert_pw_provider
(svn_auth_provider_object_t **provider,
apr_pool_t *pool)
{
+#ifdef SVN_HAVE_KWALLET
svn_auth_provider_object_t *po =
static_cast<svn_auth_provider_object_t *> (apr_pcalloc(pool, sizeof(*po)));
po->vtable = &kwallet_ssl_client_cert_pw_provider;
*provider = po;
+#else
+ return svn_auth__get_noop_ssl_client_cert_pw_provider(provider, pool);
+#endif
}
}
Index: subversion/libsvn_subr/auth.c
===================================================================
--- subversion/libsvn_subr/auth.c (revision 1543908)
+++ subversion/libsvn_subr/auth.c (working copy)
@@ -677,3 +677,71 @@ svn_auth_get_platform_specific_client_providers(ap
return SVN_NO_ERROR;
}
+
+/* No Operation Auth Providers */
+static svn_error_t *
+noop_provider_first_creds(void **credentials,
+ void **iter_baton,
+ void *provider_baton,
+ apr_hash_t *parameters,
+ const char *realmstring,
+ apr_pool_t *pool)
+{
+ *credentials = NULL;
+ *iter_baton = NULL;
+ return SVN_NO_ERROR;
+}
+
+static const svn_auth_provider_t noop_simple_provider = {
+ SVN_AUTH_CRED_SIMPLE,
+ noop_provider_first_creds,
+ NULL,
+ NULL
+};
+
+void
+svn_auth__get_noop_simple_provider(svn_auth_provider_object_t **provider,
+ apr_pool_t *pool)
+{
+ svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
+
+ po->vtable = &noop_simple_provider;
+ *provider = po;
+}
+
+static const svn_auth_provider_t noop_ssl_client_cert_pw_provider = {
+ SVN_AUTH_CRED_SSL_CLIENT_CERT_PW,
+ noop_provider_first_creds,
+ NULL,
+ NULL
+};
+
+void
+svn_auth__get_noop_ssl_client_cert_pw_provider
+ (svn_auth_provider_object_t **provider,
+ apr_pool_t *pool)
+{
+ svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
+
+ po->vtable = &noop_ssl_client_cert_pw_provider;
+ *provider = po;
+}
+
+static const svn_auth_provider_t noop_ssl_server_trust_provider = {
+ SVN_AUTH_CRED_SSL_SERVER_TRUST,
+ noop_provider_first_creds,
+ NULL,
+ NULL
+};
+
+void
+svn_auth__get_noop_ssl_server_trust_provider
+ (svn_auth_provider_object_t **provider,
+ apr_pool_t *pool)
+{
+ svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
+
+ po->vtable = &noop_ssl_server_trust_provider;
+ *provider = po;
+}
+
Index: subversion/libsvn_subr/gpg_agent.c
===================================================================
--- subversion/libsvn_subr/gpg_agent.c (revision 1543908)
+++ subversion/libsvn_subr/gpg_agent.c (working copy)
@@ -64,6 +64,8 @@
#include <sys/socket.h>
#include <sys/un.h>
+#endif /* !WIN32 */
+
#include <apr_pools.h>
#include "svn_auth.h"
#include "svn_config.h"
@@ -77,7 +79,7 @@
#include "svn_private_config.h"
-#ifdef SVN_HAVE_GPG_AGENT
+#if defined(SVN_HAVE_GPG_AGENT) && !defined(WIN32)
#define BUFFER_SIZE 1024
@@ -472,17 +474,21 @@ static const svn_auth_provider_t gpg_agent_simple_
simple_gpg_agent_save_creds
};
+#endif /* defined(SVN_HAVE_GPG_AGENT) && !defined(WIN32) */
+
/* Public API */
void
svn_auth_get_gpg_agent_simple_provider(svn_auth_provider_object_t **provider,
apr_pool_t *pool)
{
+#if defined(SVN_HAVE_GPG_AGENT) && !defined(WIN32)
svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
po->vtable = &gpg_agent_simple_provider;
*provider = po;
+#else
+ return svn_auth__get_noop_simple_provider(provider, pool);
+#endif
}
-#endif /* SVN_HAVE_GPG_AGENT */
-#endif /* !WIN32 */
Index: subversion/libsvn_subr/macos_keychain.c
===================================================================
--- subversion/libsvn_subr/macos_keychain.c (revision 1543908)
+++ subversion/libsvn_subr/macos_keychain.c (working copy)
@@ -237,6 +237,7 @@ static const svn_auth_provider_t keychain_ssl_clie
NULL,
keychain_ssl_client_cert_pw_save_creds
};
+#endif /* SVN_HAVE_KEYCHAIN_SERVICES */
/* Public API */
@@ -244,10 +245,14 @@ void
svn_auth_get_keychain_simple_provider(svn_auth_provider_object_t **provider,
apr_pool_t *pool)
{
+#ifdef SVN_HAVE_KEYCHAIN_SERVICES
svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
po->vtable = &keychain_simple_provider;
*provider = po;
+#else
+ return svn_auth__get_noop_simple_provider(provider, pool);
+#endif
}
void
@@ -255,9 +260,12 @@ svn_auth_get_keychain_ssl_client_cert_pw_provider
(svn_auth_provider_object_t **provider,
apr_pool_t *pool)
{
+#ifdef SVN_HAVE_KEYCHAIN_SERVICES
svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
po->vtable = &keychain_ssl_client_cert_pw_provider;
*provider = po;
+#else
+ return svn_auth__get_noop_ssl_client_cert_pw_provider(provider, pool);
+#endif
}
-#endif /* SVN_HAVE_KEYCHAIN_SERVICES */
Index: subversion/libsvn_subr/win32_crypto.c
===================================================================
--- subversion/libsvn_subr/win32_crypto.c (revision 1543908)
+++ subversion/libsvn_subr/win32_crypto.c (working copy)
@@ -21,12 +21,8 @@
* ====================================================================
*/
-/* prevent "empty compilation unit" warning on e.g. UNIX */
-typedef int win32_crypto__dummy;
-
/* ==================================================================== */
-#if defined(WIN32) && !defined(__MINGW32__)
/*** Includes. ***/
@@ -44,6 +40,8 @@
#include "private/svn_auth_private.h"
+#if defined(WIN32) && !defined(__MINGW32__)
+
#include <wincrypt.h>
@@ -210,6 +208,7 @@ static const svn_auth_provider_t windows_simple_pr
windows_simple_save_creds
};
+#endif /* WIN32 */
/* Public API */
void
@@ -216,12 +215,19 @@ void
svn_auth_get_windows_simple_provider(svn_auth_provider_object_t **provider,
apr_pool_t *pool)
{
+#if defined(WIN32) && !defined(__MINGW32__)
svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
po->vtable = &windows_simple_provider;
*provider = po;
+#else
+ return svn_auth__get_noop_simple_provider(provider, pool);
+#endif
+
}
+#if defined(WIN32) && !defined(__MINGW32__)
+
/*-----------------------------------------------------------------------*/
/* Windows SSL server trust provider, validates ssl certificate using */
@@ -328,6 +334,7 @@ static const svn_auth_provider_t windows_ssl_clien
windows_ssl_client_cert_pw_save_creds
};
+#endif /* WIN32 */
/* Public API */
void
@@ -335,12 +342,18 @@ svn_auth_get_windows_ssl_client_cert_pw_provider
(svn_auth_provider_object_t **provider,
apr_pool_t *pool)
{
+#if defined(WIN32) && !defined(__MINGW32__)
svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
po->vtable = &windows_ssl_client_cert_pw_provider;
*provider = po;
+#else
+ return svn_auth__get_noop_ssl_client_cert_pw_provider(provider, pool);
+#endif
}
+#if defined(WIN32) && !defined(__MINGW32__)
+
/*-----------------------------------------------------------------------*/
/* Windows SSL server trust provider, validates ssl certificate using */
@@ -480,17 +493,25 @@ static const svn_auth_provider_t windows_server_tr
NULL,
};
+#endif /* WIN32 */
+
/* Public API */
void
svn_auth_get_windows_ssl_server_trust_provider
(svn_auth_provider_object_t **provider, apr_pool_t *pool)
{
+#if defined(WIN32) && !defined(__MINGW32__)
svn_auth_provider_object_t *po = apr_pcalloc(pool, sizeof(*po));
po->vtable = &windows_server_trust_provider;
*provider = po;
+#else
+ return svn_auth__get_noop_ssl_server_trust_provider(provider, pool);
+#endif
}
+#if defined(WIN32) && !defined(__MINGW32__)
+
static const svn_auth_provider_t windows_server_authority_provider = {
SVN_AUTH_CRED_SSL_SERVER_AUTHORITY,
windows_ssl_server_trust_first_credentials,
