On Sun, Jul 14, 2013 at 02:06:02AM -0400, Justin Erenkrantz wrote:
> On Sat, Jul 13, 2013 at 8:39 PM, Daniel Shahaf <danielsh_at_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.
Received on 2013-07-14 11:40:20 CEST