сб, 19 нояб. 2022 г. в 15:51, Peter Eisentraut
<peter.eisentr...@enterprisedb.com>:
>
> On 19.11.22 13:12, Марина Полякова wrote:
> >> I'm not in favor of changing this. The existing code intentionally
> >> tries to centralize the "ICU is not supported in this build" knowledge
> >> in few places. Your patch tries to make this check early, but in the
> >> process adds more places where ICU support needs to be checked
> >> explicitly. This increases the code size and also creates a future
> >> burden to maintain that level of checking. I think building without ICU
> >> should be considered a marginal configuration at this point, so we don't
> >> need to go out of our way to create a perfect user experience for this
> >> configuration, as long as we check somewhere in the end.
> > Maybe this should be written in the documentation [1] or --with-icu
> > should be used by default? As a developer I usually check something
> > with the simplest configure run to make sure other options do not
> > affect the checked behaviour. And some other developers in our company
> > also use simple configure runs, without --with-icu etc.
>
> Well, this isn't a hard rule, just my opinion and where I see the world
> moving. It's similar to --with-openssl and --with-lz4 etc.
Here is another set of proposed patches:
v2-0001-Fix-encoding-check-in-initdb-when-the-option-icu-.patch
Target: PG 15+
Fix encoding check in initdb when the option --icu-locale is not used:
$ initdb --encoding sql-ascii --locale-provider icu hoge
...
initdb: error: encoding mismatch
initdb: detail: The encoding you selected (SQL_ASCII) is not supported
with the ICU provider.
initdb: hint: Rerun initdb and either do not specify an encoding
explicitly, or choose a matching combination.
As with the previous version of this fix a side effect is that if ICU
locale is missed (or ICU is not supported in this build), the
provider, locales and encoding are reported before the error message:
$ initdb --locale-provider icu hoge
The files belonging to this database system will be owned by user "marina".
This user must also own the server process.
The database cluster will be initialized with this locale configuration:
provider: icu
LC_COLLATE: en_US.UTF-8
LC_CTYPE: en_US.UTF-8
LC_MESSAGES: en_US.UTF-8
LC_MONETARY: ru_RU.UTF-8
LC_NUMERIC: ru_RU.UTF-8
LC_TIME: ru_RU.UTF-8
The default database encoding has been set to "UTF8".
initdb: error: ICU locale must be specified
$ initdb --locale-provider icu --icu-locale en hoge
The files belonging to this database system will be owned by user "marina".
This user must also own the server process.
The database cluster will be initialized with this locale configuration:
provider: icu
ICU locale: en
LC_COLLATE: en_US.UTF-8
LC_CTYPE: en_US.UTF-8
LC_MESSAGES: en_US.UTF-8
LC_MONETARY: ru_RU.UTF-8
LC_NUMERIC: ru_RU.UTF-8
LC_TIME: ru_RU.UTF-8
The default database encoding has been set to "UTF8".
initdb: error: ICU is not supported in this build
v2-0002-doc-building-without-ICU-is-not-recommended.patch
Target: PG 15+
Fix the documentation that --without-icu is a marginal configuration.
v2-0003-Build-with-ICU-by-default.patch
Target: PG 16+
Build with ICU by default as already done for readline and zlib libraries.
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From b4dca9097633c585da232bc640f2517b3c443f09 Mon Sep 17 00:00:00 2001
From: Marina Polyakova <m.polyak...@postgrespro.ru>
Date: Sat, 19 Nov 2022 17:24:55 +0300
Subject: [PATCH v2 1/3] Fix encoding check in initdb when the option
--icu-locale is not used
First check that the selected encoding is supported by ICU (the encoding can be
set from lc_ctype which can be set from an environment variable). Only then
check the option for the ICU locale.
---
src/bin/initdb/initdb.c | 35 +++++++++++++------------------
src/bin/initdb/t/001_initdb.pl | 17 ++++++++++-----
src/bin/scripts/t/020_createdb.pl | 26 ++++++++++++++++++-----
3 files changed, 48 insertions(+), 30 deletions(-)
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index f61a043055..444c8a505a 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -2047,7 +2047,7 @@ check_locale_encoding(const char *locale, int user_enc)
*
* this should match the similar check in the backend createdb() function
*/
-static bool
+static void
check_icu_locale_encoding(int user_enc)
{
if (!(is_encoding_supported_by_icu(user_enc)))
@@ -2058,9 +2058,19 @@ check_icu_locale_encoding(int user_enc)
pg_log_error_hint("Rerun %s and either do not specify an encoding explicitly, "
"or choose a matching combination.",
progname);
- return false;
+ exit(1);
}
- return true;
+
+ if (!icu_locale)
+ pg_fatal("ICU locale must be specified");
+
+ /*
+ * In supported builds, the ICU locale ID will be checked by the backend
+ * during post-bootstrap initialization.
+ */
+#ifndef USE_ICU
+ pg_fatal("ICU is not supported in this build");
+#endif
}
/*
@@ -2113,20 +2123,6 @@ setlocales(void)
check_locale_name(LC_CTYPE, lc_messages, &canonname);
lc_messages = canonname;
#endif
-
- if (locale_provider == COLLPROVIDER_ICU)
- {
- if (!icu_locale)
- pg_fatal("ICU locale must be specified");
-
- /*
- * In supported builds, the ICU locale ID will be checked by the
- * backend during post-bootstrap initialization.
- */
-#ifndef USE_ICU
- pg_fatal("ICU is not supported in this build");
-#endif
- }
}
/*
@@ -2388,9 +2384,8 @@ setup_locale_encoding(void)
!check_locale_encoding(lc_collate, encodingid))
exit(1); /* check_locale_encoding printed the error */
- if (locale_provider == COLLPROVIDER_ICU &&
- !check_icu_locale_encoding(encodingid))
- exit(1);
+ if (locale_provider == COLLPROVIDER_ICU)
+ check_icu_locale_encoding(encodingid);
}
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 164fc11cbf..20fe29851c 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -123,28 +123,35 @@ if ($ENV{with_icu} eq 'yes')
[
'initdb', '--no-sync',
'--locale-provider=icu', '--encoding=SQL_ASCII',
- '--icu-locale=en', "$tempdir/dataX"
+ "$tempdir/dataX"
],
qr/error: encoding mismatch/,
'fails for encoding not supported by ICU');
}
else
{
- command_fails(
- [ 'initdb', '--no-sync', '--locale-provider=icu', "$tempdir/data2" ],
+ command_fails_like(
+ [
+ 'initdb', '--no-sync',
+ '--locale-provider=icu', '--icu-locale=en',
+ "$tempdir/data2"
+ ],
+ qr/error: ICU is not supported in this build/,
'locale provider ICU fails since no ICU support');
}
-command_fails(
+command_fails_like(
[ 'initdb', '--no-sync', '--locale-provider=xyz', "$tempdir/dataX" ],
+ qr/error: unrecognized locale provider: xyz/,
'fails for invalid locale provider');
-command_fails(
+command_fails_like(
[
'initdb', '--no-sync',
'--locale-provider=libc', '--icu-locale=en',
"$tempdir/dataX"
],
+ qr/error: --icu-locale cannot be specified unless locale provider "icu" is chosen/,
'fails for invalid option combination');
done_testing();
diff --git a/src/bin/scripts/t/020_createdb.pl b/src/bin/scripts/t/020_createdb.pl
index a74bf3b0d8..3030ea7f9d 100644
--- a/src/bin/scripts/t/020_createdb.pl
+++ b/src/bin/scripts/t/020_createdb.pl
@@ -30,11 +30,12 @@ if ($ENV{with_icu} eq 'yes')
# This fails because template0 uses libc provider and has no ICU
# locale set. It would succeed if template0 used the icu
# provider. XXX Maybe split into multiple tests?
- $node->command_fails(
+ $node->command_fails_like(
[
'createdb', '-T', 'template0', '-E', 'UTF8',
'--locale-provider=icu', 'foobar4'
],
+ qr/ERROR: ICU locale must be specified/,
'create database with ICU fails without ICU locale specified');
$node->issues_sql_like(
@@ -46,12 +47,13 @@ if ($ENV{with_icu} eq 'yes')
qr/statement: CREATE DATABASE foobar5 .* LOCALE_PROVIDER icu ICU_LOCALE 'en'/,
'create database with ICU locale specified');
- $node->command_fails(
+ $node->command_fails_like(
[
'createdb', '-T', 'template0', '-E', 'UTF8',
'--locale-provider=icu',
'--icu-locale=@colNumeric=lower', 'foobarX'
],
+ qr/ERROR: could not open collator for locale/,
'fails for invalid ICU locale');
$node->command_fails_like(
@@ -78,18 +80,32 @@ if ($ENV{with_icu} eq 'yes')
}
else
{
- $node->command_fails(
- [ 'createdb', '-T', 'template0', '--locale-provider=icu', 'foobar4' ],
+ $node->command_fails_like(
+ [
+ 'createdb', '-T', 'template0', '-E', 'UTF8',
+ '--locale-provider=icu', '--icu-locale=en', 'foobar4'
+ ],
+ qr/ERROR: ICU is not supported in this build/,
'create database with ICU fails since no ICU support');
}
$node->command_fails([ 'createdb', 'foobar1' ],
'fails if database already exists');
-$node->command_fails(
+$node->command_fails_like(
[ 'createdb', '-T', 'template0', '--locale-provider=xyz', 'foobarX' ],
+ qr/ERROR: unrecognized locale provider: xyz/,
'fails for invalid locale provider');
+$node->command_fails_like(
+ [
+ 'createdb', '-T',
+ 'template0', '--locale-provider=libc',
+ '--icu-locale=en', 'foobarX'
+ ],
+ qr/ERROR: ICU locale cannot be specified unless locale provider is ICU/,
+ 'fails for invalid option combination');
+
# Check use of templates with shared dependencies copied from the template.
my ($ret, $stdout, $stderr) = $node->psql(
'foobar2',
--
2.25.1
From fe405bda51528493beda3461255cbe3eac361c6b Mon Sep 17 00:00:00 2001
From: Marina Polyakova <m.polyak...@postgrespro.ru>
Date: Sat, 19 Nov 2022 18:57:12 +0300
Subject: [PATCH v2 3/3] Build with ICU by default
Add the option --without-icu to turn it off.
---
configure | 35 ++++++++-------
configure.ac | 18 ++++++--
doc/src/sgml/installation.sgml | 82 ++++++++++++++++------------------
meson.build | 2 +
4 files changed, 74 insertions(+), 63 deletions(-)
diff --git a/configure b/configure
index 3966368b8d..1b3ecf9d11 100755
--- a/configure
+++ b/configure
@@ -1558,7 +1558,7 @@ Optional Packages:
set WAL block size in kB [8]
--with-CC=CMD set compiler (deprecated)
--with-llvm build with LLVM based JIT support
- --with-icu build with ICU support
+ --without-icu build without ICU support
--with-tcl build Tcl modules (PL/Tcl)
--with-tclconfig=DIR tclConfig.sh is in DIR
--with-perl build Perl modules (PL/Perl)
@@ -8320,7 +8320,9 @@ $as_echo "#define USE_ICU 1" >>confdefs.h
esac
else
- with_icu=no
+ with_icu=yes
+
+$as_echo "#define USE_ICU 1" >>confdefs.h
fi
@@ -8389,31 +8391,27 @@ fi
# Put the nasty error message in config.log where it belongs
echo "$ICU_PKG_ERRORS" >&5
- as_fn_error $? "Package requirements (icu-uc icu-i18n) were not met:
-
-$ICU_PKG_ERRORS
-
-Consider adjusting the PKG_CONFIG_PATH environment variable if you
-installed software in a non-standard prefix.
+ as_fn_error $? "ICU library not found
+If you have ICU already installed, see config.log for details on the
+failure. It is possible pkg-config isn't looking in the proper directory.
Alternatively, you may set the environment variables ICU_CFLAGS
and ICU_LIBS to avoid the need to call pkg-config.
-See the pkg-config man page for more details." "$LINENO" 5
+See the pkg-config man page for more details.
+
+Use --without-icu to disable ICU support." "$LINENO" 5
elif test $pkg_failed = untried; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
$as_echo "no" >&6; }
- { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
-$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
-as_fn_error $? "The pkg-config script could not be found or is too old. Make sure it
-is in your PATH or set the PKG_CONFIG environment variable to the full
-path to pkg-config.
+ as_fn_error $? "ICU library not found
+If you have ICU already installed, see config.log for details on the
+failure. It is possible pkg-config isn't looking in the proper directory.
Alternatively, you may set the environment variables ICU_CFLAGS
and ICU_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.
-To get pkg-config, see <http://pkg-config.freedesktop.org/>.
-See \`config.log' for more details" "$LINENO" 5; }
+Use --without-icu to disable ICU support." "$LINENO" 5
else
ICU_CFLAGS=$pkg_cv_ICU_CFLAGS
ICU_LIBS=$pkg_cv_ICU_LIBS
@@ -16715,7 +16713,10 @@ if test "$with_icu" = yes; then
if test "x$ac_cv_header_unicode_ucol_h" = xyes; then :
else
- as_fn_error $? "header file <unicode/ucol.h> is required for ICU" "$LINENO" 5
+ as_fn_error $? "ICU header file <unicode/ucol.h> not found
+If you have ICU already installed, see config.log for details on the
+failure. It is possible the compiler isn't looking in the proper directory.
+Use --without-icu to disable ICU support." "$LINENO" 5
fi
diff --git a/configure.ac b/configure.ac
index f76b7ee31f..bf396f8d83 100644
--- a/configure.ac
+++ b/configure.ac
@@ -831,13 +831,22 @@ AC_SUBST(enable_thread_safety)
# ICU
#
AC_MSG_CHECKING([whether to build with ICU support])
-PGAC_ARG_BOOL(with, icu, no, [build with ICU support],
+PGAC_ARG_BOOL(with, icu, yes, [build without ICU support],
[AC_DEFINE([USE_ICU], 1, [Define to build with ICU support. (--with-icu)])])
AC_MSG_RESULT([$with_icu])
AC_SUBST(with_icu)
if test "$with_icu" = yes; then
- PKG_CHECK_MODULES(ICU, icu-uc icu-i18n)
+ PKG_CHECK_MODULES(ICU, icu-uc icu-i18n, [],
+ [AC_MSG_ERROR([ICU library not found
+If you have ICU already installed, see config.log for details on the
+failure. It is possible pkg-config isn't looking in the proper directory.
+
+Alternatively, you may set the environment variables ICU_CFLAGS
+and ICU_LIBS to avoid the need to call pkg-config.
+See the pkg-config man page for more details.
+
+Use --without-icu to disable ICU support.])])
fi
#
@@ -1936,7 +1945,10 @@ if test "$with_icu" = yes; then
# Verify we have ICU's header files
AC_CHECK_HEADER(unicode/ucol.h, [],
- [AC_MSG_ERROR([header file <unicode/ucol.h> is required for ICU])])
+ [AC_MSG_ERROR([ICU header file <unicode/ucol.h> not found
+If you have ICU already installed, see config.log for details on the
+failure. It is possible the compiler isn't looking in the proper directory.
+Use --without-icu to disable ICU support.])])
CPPFLAGS=$ac_save_CPPFLAGS
fi
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index df62573db1..e15f6f8643 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -913,49 +913,6 @@ build-postgresql:
</listitem>
</varlistentry>
- <varlistentry>
- <term><option>--with-icu</option></term>
- <listitem>
- <para>
- Build with support for
- the <productname>ICU</productname><indexterm><primary>ICU</primary></indexterm>
- library, enabling use of ICU collation
- features<phrase condition="standalone-ignore"> (see
- <xref linkend="collation"/>)</phrase>.
- This requires the <productname>ICU4C</productname> package
- to be installed. The minimum required version
- of <productname>ICU4C</productname> is currently 4.2.
- </para>
-
- <para>
- By default,
- <productname>pkg-config</productname><indexterm><primary>pkg-config</primary></indexterm>
- will be used to find the required compilation options. This is
- supported for <productname>ICU4C</productname> version 4.6 and later.
- For older versions, or if <productname>pkg-config</productname> is
- not available, the variables <envar>ICU_CFLAGS</envar>
- and <envar>ICU_LIBS</envar> can be specified
- to <filename>configure</filename>, like in this example:
-<programlisting>
-./configure ... --with-icu ICU_CFLAGS='-I/some/where/include' ICU_LIBS='-L/some/where/lib -licui18n -licuuc -licudata'
-</programlisting>
- (If <productname>ICU4C</productname> is in the default search path
- for the compiler, then you still need to specify nonempty strings in
- order to avoid use of <productname>pkg-config</productname>, for
- example, <literal>ICU_CFLAGS=' '</literal>.)
- </para>
-
- <note>
- <para>
- Building without
- <productname>ICU</productname><indexterm><primary>ICU</primary></indexterm>
- library is considered a marginal configuration and is not recommended
- unless really necessary.
- </para>
- </note>
- </listitem>
- </varlistentry>
-
<varlistentry id="configure-with-llvm">
<term><option>--with-llvm</option></term>
<listitem>
@@ -1266,6 +1223,45 @@ build-postgresql:
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><option>--without-icu</option></term>
+ <listitem>
+ <para>
+ Prevents use of the
+ <productname>ICU</productname><indexterm><primary>ICU</primary></indexterm>
+ library. This disables support of ICU collation
+ features<phrase condition="standalone-ignore"> (see
+ <xref linkend="collation"/>)</phrase>.
+ </para>
+ <note>
+ <para>
+ Building with support for
+ the <productname>ICU</productname><indexterm><primary>ICU</primary></indexterm>
+ library requires the <productname>ICU4C</productname> package
+ to be installed. The minimum required version
+ of <productname>ICU4C</productname> is currently 4.2.
+ </para>
+ <para>
+ By default,
+ <productname>pkg-config</productname><indexterm><primary>pkg-config</primary></indexterm>
+ will be used to find the required compilation options. This is
+ supported for <productname>ICU4C</productname> version 4.6 and later.
+ For older versions, or if <productname>pkg-config</productname> is
+ not available, the variables <envar>ICU_CFLAGS</envar>
+ and <envar>ICU_LIBS</envar> can be specified
+ to <filename>configure</filename>, like in this example:
+ <programlisting>
+ ./configure ... ICU_CFLAGS='-I/some/where/include' ICU_LIBS='-L/some/where/lib -licui18n -licuuc -licudata'
+ </programlisting>
+ (If <productname>ICU4C</productname> is in the default search path
+ for the compiler, then you still need to specify nonempty strings in
+ order to avoid use of <productname>pkg-config</productname>, for
+ example, <literal>ICU_CFLAGS=' '</literal>.)
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><option>--disable-spinlocks</option></term>
<listitem>
diff --git a/meson.build b/meson.build
index 058382046e..5e3cd93a69 100644
--- a/meson.build
+++ b/meson.build
@@ -709,6 +709,8 @@ if not icuopt.disabled()
if icu.found()
cdata.set('USE_ICU', 1)
+ else
+ warning('did not find icu')
endif
else
--
2.25.1
From 5a994c9d039987c2739ccd788450205b06e1782c Mon Sep 17 00:00:00 2001
From: Marina Polyakova <m.polyak...@postgrespro.ru>
Date: Sat, 19 Nov 2022 17:39:42 +0300
Subject: [PATCH v2 2/3] doc: building without ICU is not recommended
---
doc/src/sgml/installation.sgml | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index 319c7e6966..df62573db1 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -944,6 +944,15 @@ build-postgresql:
order to avoid use of <productname>pkg-config</productname>, for
example, <literal>ICU_CFLAGS=' '</literal>.)
</para>
+
+ <note>
+ <para>
+ Building without
+ <productname>ICU</productname><indexterm><primary>ICU</primary></indexterm>
+ library is considered a marginal configuration and is not recommended
+ unless really necessary.
+ </para>
+ </note>
</listitem>
</varlistentry>
--
2.25.1
