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.
>
Yes. We were asked to convert something to UTF-8, and cannot do this, so we
fail hard and let the caller figure it out. If people need the old "sweep the
error under the rug if the target string is ASCII" behaviour, we can make it
opt-in.
> 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
I'd be happy with just not calling into utf.c on log messages at all when
--source-prop-encoding isn't used.
Received on 2013-07-14 18:55:29 CEST