I just spent a very non-pleasant hour looking into why svn.apache.org's
sync fails with UTF-8 errors. Turns out that apr_xlate_open() was
returning APR_ENOTIMPL and libsvn_subr was kindly ignoring that error
without even a log message.
The error handling logic in question was added in r15379 <->
http://svn.apache.org/r855453. The log message of that revision
indicates the intent was to fix a memory leak; whether excluding EINVAL
and ENOTIMPL from the error handling block that follows was intentional
So, any objections to the following?
Follow-up to r15379(r855453): don't discard apr_xlate_open() failures.
The incumbent code just leads to "Unable to translate to/from UTF-8"
errors later on, which renders svnsync unable to sync log messages
containing non-ASCII (even though they can be committed) and fails our
test suite in three places:
FAIL: prop_tests.py 22: test prop* handle invalid property names
FAIL: svnsync_tests.py 28: copy and reencode non-UTF-8 svn:* props
FAIL: svnsync_tests.py 29: copy UTF-8 svn:* props identically
(xlate_alloc_handle): Have EINVAL and ENOTIMPL return values from
apr_xlate_open() fall through to the normal error handling.
Rejigger that handling so that apr_strerror(EINVAL) or
apr_strerror(ENOTIMPL) are included in the error chain.
(The incumbent code would report error code E70023, which is
APR_ENOTIMPL, but mere mortals aren't supposed to know that.)
--- subversion/libsvn_subr/utf.c (revision 1501080)
+++ subversion/libsvn_subr/utf.c (working copy)
@@ -230,9 +230,12 @@ xlate_alloc_handle(xlate_handle_node_t **ret,
if (APR_STATUS_IS_EINVAL(apr_err) || APR_STATUS_IS_ENOTIMPL(apr_err))
handle = NULL;
- else if (apr_err != APR_SUCCESS)
+ if (apr_err != APR_SUCCESS)
const char *errstr;
+ svn_error_t *err;
+ char apr_strerr;
/* Can't use svn_error_wrap_apr here because it calls functions in
this file, leading to infinite recursion. */
if (frompage == SVN_APR_LOCALE_CHARSET)
@@ -248,7 +251,13 @@ xlate_alloc_handle(xlate_handle_node_t **ret,
_("Can't create a character converter from "
"'%s' to '%s'"), frompage, topage);
- return svn_error_create(apr_err, NULL, errstr);
+ /* Just put the error on the stack, since svn_error_create duplicates it
+ later. APR_STRERR will be in the local encoding, not in UTF-8, though.
+ svn_strerror(apr_err, apr_strerr, sizeof(apr_strerr));
+ return svn_error_create(apr_err,
+ svn_error_create(apr_err, NULL, apr_strerr),
/* Allocate and initialize the node. */
The error upon ENOTIMPL looks like this:
svn: E070023: Can't create a character converter from native encoding to 'UTF-8'
svn: E070023: This function has not been implemented on this platform
It appears as soon as svn_utf_cstring_to_utf8() is called --- which is
normally during argv parsing. The error happens even if the argv
arguments are all ASCII, which effectively adds a new dependency on
apr_xlate_* even for people who use only ASCII. I assume this new
failure mode for ASCII-only users means we cannot backport this change.
Received on 2013-07-14 02:39:19 CEST