On Sun, Jul 14, 2013 at 02:06:02AM -0400, Justin Erenkrantz wrote: > On Sat, Jul 13, 2013 at 8:39 PM, Daniel Shahaf <danie...@apache.org> wrote: > > > 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. > > > > This now means iconv or apr-iconv are a hard global dependency where they > haven't been before. I'm not sure that I like that. (If you're going to > do this patch, remove the if check for EINVAL/ENOTIMPL - it's unnecessary.) > > IIRC, the reason is that this code gets called all the time - we had to > silence the ENOTIMPL and EINVAL errors as it really shouldn't be treated as > a fatal error. So, in this case, we're back to treating it as an > irrecoverable error. > > It's probably better to just check APR_HAS_XLATE and return an error in > svnsync if that's 0...and let the string pass through unmodified - which is > probably fine for svnsync case...or perhaps go a step further and just > refuse to build svnsync if iconv isn't supported as we might not be able to > guarantee the integrity of the sync'd logs. I'm not sure how paranoid we > need to be here. -- justin
Requiring iconv for svnsync is a good idea, we need it for the --source-prop-encoding option of 'svnsync sync'. But I don't think that Daniel's patch is about build-time dependencies. The problem Daniel is trying to fix is where iconv is detected at build time but fails to load at runtime, e.g. due to problems with dlopen() -- shared libraries can fail to load for many reasons.
