On Tue, Jun 10, 2025 at 10:07 AM Timofei Zhakov <t...@chemodax.net> wrote:
> On Mon, Jun 9, 2025 at 7:26 PM Branko Čibej <br...@apache.org> wrote: > >> On 9. 6. 25 18:31, rin...@apache.org wrote: >> >> Author: rinrab >> Date: Mon Jun 9 16:31:15 2025 >> New Revision: 1926293 >> >> URL: http://svn.apache.org/viewvc?rev=1926293&view=rev >> Log: >> Do not include apr_xlate.h into svn_utf.h, but manually wrap the >> SVN_APR_*_CHARSET constants into APR_*_CHARSET. >> >> >> >> -1. You can't presume that this will never change in APR. >> >> Fix the cmake build to handle include paths correctly. This is not that >> fix. Please revert. >> >> -- Brane >> > > I want to make sure that you entirely understand this change. > > SVN_APR_*_CHARSET family has nothing similar with APR_*_CHARSET since they > are being converted into APR constants through the get_apr_xlate_charset() > function. > > This also means we can add our own shortcuts to SVN_APR_*_CHARSET, which I'm > planning to do for UTF-8. > As long as consumers of our library are not passing APR_*_CHARSET to our utf > routines, we are all fine. > > It doesn't make us dependent on apr's constants. Please withdraw your veto. > > Is there indeed a build system problem related to including apr_xlate.h? As I'm reading the diff, it seems the purpose is to decouple from APR, to lay groundwork for later changes that will also add new SVN_*_CHARSET constants that aren't provided by APR? If so, then perhaps removing "APR" from the names of the constants could help avoid confusion? #define SVN_LOCALE_CHARSET ... #define SVN_DEFAULT_CHARSET ... Nathan
