subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko ─îibej <br...@wandisco.com>
Subject Re: svn commit: r1697654 - /subversion/branches/1.9.x/STATUS
Date Tue, 25 Aug 2015 11:55:31 GMT
On 25.08.2015 13:49, brane@apache.org wrote:
> Author: brane
> Date: Tue Aug 25 11:49:09 2015
> New Revision: 1697654
>
> URL: http://svn.apache.org/r1697654
> Log:
> * branches/1.9.x/STATUS:
>    - Approve r1693886.
>    - Temporarily veto r1694481; the change looks broken.

[...]

> @@ -98,5 +84,22 @@ Candidate changes:
>  Veto-blocked changes:
>  =====================
>  
> + * r1694481
> +   Fix Unix build on systems without GPG agent.
> +   Justification:
> +     This is a user-reported issue.
> +   Votes:
> +     +1: stefan2, philip
> +     -1: brane (You can't just remove a public API implementation,
> +                even if it is deprecated. And the prototyps is still
> +                right there in svn_auth.h)
> +
>  Approved changes:
>  =================

r1694481 (conditionally) removes the implementation of a public API,
whilst leaving the prototype in svn_auth.h untouched. This is a
violation of our ABI compatibility rules, and also a linking error
waiting to happen.

I suggest the correct fix is for the function to just do nothing if GPG
support is not available. Ideally it would return an error, but the
prototype doesn't let us do that.

For example:

--- ../trunk/subversion/libsvn_subr/deprecated.c	(revision 1697651)
+++ ../trunk/subversion/libsvn_subr/deprecated.c	(working copy)
@@ -1497,14 +1497,14 @@
 #endif /* DARWIN */
 
 #if !defined(WIN32)
-#ifdef SVN_HAVE_GPG_AGENT
 void
 svn_auth_get_gpg_agent_simple_provider(svn_auth_provider_object_t **provider,
                                        apr_pool_t *pool)
 {
+#ifdef SVN_HAVE_GPG_AGENT
   svn_auth__get_gpg_agent_simple_provider(provider, pool);
-}
 #endif /* SVN_HAVE_GPG_AGENT */
+}
 #endif /* !WIN32 */
 
 svn_error_t *


-- Brane


Mime
View raw message