Re: Rework of collation code, extensibility

On 22.12.22 06:40, Jeff Davis wrote:
> On Sat, 2022-12-17 at 19:14 -0800, Jeff Davis wrote:
>> Attached is a new patch series. I think there are enough changes that
>> this has become more of a "rework" of the collation code rather than
>> just a refactoring. This is a continuation of some prior work[1][2]
>> in
>> a new thread given its new scope.
>
> Here's version 5. There are a number of fixes, and better tests, and
> it's passing in CI.
>
> The libc hook support is still experimental, but what's working is
> passing in CI, even on windows. The challenges with libc hook support
> are:
>
> * It obviously doesn't replace all of libc, so the separation is not
> as clean and there are a number of callers throughout the code that
> don't necessarily care about specific collations.
>
> * libc relies on setlocale() / uselocale(), which is global state and
> not as easy to track.
>
> * More platform issues (obviously) and harder to test.

I'm confused by this patch set.

It combines some refactoring that was previously posted with partial
support for multiple ICU libraries with partial support for some new
hooks. Shouldn't those be three separate threads? I think the multiple
ICU libraries already does have a separate thread; how does this relate
to that work? I don't know what the hooks are supposed to be for? What
other locale libraries are you thinking about using this way? How can
we asses whether these interfaces are sufficient for that? The
refactoring patches don't look convincing just by looking at the numbers:

3 files changed, 406 insertions(+), 247 deletions(-)
6 files changed, 481 insertions(+), 150 deletions(-)
12 files changed, 400 insertions(+), 323 deletions(-)

My sense is this is trying to do too many things at once, and those
things are each not fully developed yet.