Thread: [PATCH] Use PKG_CHECK_MODULES to detect the libxml2 library
Dear developers, Debian (and Ubuntu) are beginning to remove foo-config legacy scripts. Already, xml2-config has been flagged for removal, with packages being asked to switch to pkg-config. This patch uses pkg-config's PKG_CHECK_MODULES macro to detect libxml2 or, if pkg-config is not available, falls back to xml2-confg. The patch was created against the master branch of git. I have built PostgreSQL and run `make check`. All 196 tests passed. Hugh
Attachment
> On 10 Mar 2020, at 11:53, Hugh McMaster <hugh.mcmaster@outlook.com> wrote: > Debian (and Ubuntu) are beginning to remove foo-config legacy scripts. > Already, xml2-config has been flagged for removal, with packages being > asked to switch to pkg-config. > > This patch uses pkg-config's PKG_CHECK_MODULES macro to detect libxml2 > or, if pkg-config is not available, falls back to xml2-confg. This was previously discussed in 20200120204715.GA73984@msg.df7cb.de which ended without a real conclusion on what could/should be done (except that nothing *had* to be done). What is the situation on non-Debian/Ubuntu systems (BSD's, macOS etc etc)? Is it worth adding pkg-config support if we still need a fallback to xml2-config? cheers ./daniel
On 2020-03-10 12:41, Daniel Gustafsson wrote: >> On 10 Mar 2020, at 11:53, Hugh McMaster <hugh.mcmaster@outlook.com> wrote: > >> Debian (and Ubuntu) are beginning to remove foo-config legacy scripts. >> Already, xml2-config has been flagged for removal, with packages being >> asked to switch to pkg-config. >> >> This patch uses pkg-config's PKG_CHECK_MODULES macro to detect libxml2 >> or, if pkg-config is not available, falls back to xml2-confg. > > This was previously discussed in 20200120204715.GA73984@msg.df7cb.de which > ended without a real conclusion on what could/should be done (except that > nothing *had* to be done). > > What is the situation on non-Debian/Ubuntu systems (BSD's, macOS etc etc)? Is > it worth adding pkg-config support if we still need a fallback to xml2-config? Btw., here is an older thread for the same issue <https://www.postgresql.org/message-id/flat/1358164265.29612.7.camel%40vanquo.pezone.net>. Might be worth reflecting on the issues discussed there. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
> On 10 Mar 2020, at 18:38, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > > On 2020-03-10 12:41, Daniel Gustafsson wrote: >>> On 10 Mar 2020, at 11:53, Hugh McMaster <hugh.mcmaster@outlook.com> wrote: >>> Debian (and Ubuntu) are beginning to remove foo-config legacy scripts. >>> Already, xml2-config has been flagged for removal, with packages being >>> asked to switch to pkg-config. >>> >>> This patch uses pkg-config's PKG_CHECK_MODULES macro to detect libxml2 >>> or, if pkg-config is not available, falls back to xml2-confg. >> This was previously discussed in 20200120204715.GA73984@msg.df7cb.de which >> ended without a real conclusion on what could/should be done (except that >> nothing *had* to be done). >> What is the situation on non-Debian/Ubuntu systems (BSD's, macOS etc etc)? Is >> it worth adding pkg-config support if we still need a fallback to xml2-config? > > Btw., here is an older thread for the same issue <https://www.postgresql.org/message-id/flat/1358164265.29612.7.camel%40vanquo.pezone.net>. Might be worth reflecting on theissues discussed there. Thanks, didn't realize that the subject had been up for discussion earlier as well. For me, the duplication aspect is the most troubling, since we'd still need the xml2-config fallback and thus won't be able to simplify the code. cheers ./daniel
On Tue, 10 Mar 2020 at 22:41, Daniel Gustafsson wrote: > > > On 10 Mar 2020, at 11:53, Hugh McMaster <hugh.mcmaster@outlook.com> wrote: > > This patch uses pkg-config's PKG_CHECK_MODULES macro to detect libxml2 > > or, if pkg-config is not available, falls back to xml2-confg. > > This was previously discussed in 20200120204715.GA73984@msg.df7cb.de which > ended without a real conclusion on what could/should be done (except that > nothing *had* to be done). > > What is the situation on non-Debian/Ubuntu systems (BSD's, macOS etc etc)? Is > it worth adding pkg-config support if we still need a fallback to xml2-config? To the best of my knowledge, FreeBSD, macOS, OpenSUSE, Solaris etc. all support detection of libxml2 via pkg-config. One way or another, xml2-config is going away, whether by decision of a package maintainer or upstream. freetype-config was deprecated upstream a few years ago. Upstream ICU will also disable the installation of icu-config by default from April this year. Some systems, such as Debian, have not shipped icu-config for a year or so. The PHP project last year switched to using pkg-config by default for all libraries supplying a .pc file. PHP's build scripts do not fall back to legacy scripts. Another reason for switching is that xml2-config incorrectly outputs static libraries when called with `--libs`. You have to call `--libs --dynamic` to output -lxml2 only. Debian patches the script to avoid this unusual behaviour.
On Wed, 11 Mar 2020 at 07:49, Daniel Gustafsson wrote: > > On 10 Mar 2020, at 18:38, Peter Eisentraut wrote: > > Btw., here is an older thread for the same issue <https://www.postgresql.org/message-id/flat/1358164265.29612.7.camel%40vanquo.pezone.net>. Might be worth reflecting on theissues discussed there. > > Thanks, didn't realize that the subject had been up for discussion earlier as > well. Interesting thread. The issue of precedence (e.g. pkg-config over xml2-config) is still relevant, although arguably less so today, due to the far greater availability of pkg-config. Some packages choose to fall back to xml2-config, say, if they need to support old or soon-to-be EOL systems lacking pkg-config. These situations are increasingly rare. The thread is correct on multi-arch header and library directories. That said, pkg-config can handle this easily. > For me, the duplication aspect is the most troubling, since we'd still need the > xml2-config fallback and thus won't be able to simplify the code. configure.in shows that ICU only uses the PKG_CHECK_MODULES macro. libxml2, libxslt and other dependencies could also switch. Using AC_CHECK_LIB to add libraries (such as -lxml2) to $LIBS isn't probably the most ideal method (I'd recommend adding pkg-config's native X_CFLAGS and X_LIBS variables as necessary to $LIBS, $CPPFLAGS etc.), but that's a topic for another thread.
Daniel Gustafsson <daniel@yesql.se> writes: > On 10 Mar 2020, at 18:38, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: >> Btw., here is an older thread for the same issue <https://www.postgresql.org/message-id/flat/1358164265.29612.7.camel%40vanquo.pezone.net>. Might be worth reflecting on theissues discussed there. > Thanks, didn't realize that the subject had been up for discussion earlier as > well. > For me, the duplication aspect is the most troubling, since we'd still need the > xml2-config fallback and thus won't be able to simplify the code. Yeah, but at least it's concentrated in a few lines in configure.in. I think that the main objection to this is the documentation/confusion issues raised by Noah in that old thread. Still, we probably don't have much choice given that some distros are going to remove xml2-config. In that connection, Hugh's patch lacks docs which is entirely not OK, but the doc changes in Peter's old patch look workable. I wonder whether we ought to try to align this with our documented procedure for falling back if you have no icu-config but want to use ICU; that part of the docs suggests setting ICU_CFLAGS and ICU_LIBS manually. The patch as it stands doesn't seem to support manually giving XML2_CFLAGS and XML2_LIBS, but it looks like it could easily be adjusted to allow that. Also, I see that pkg.m4 says dnl Note that if there is a possibility the first call to dnl PKG_CHECK_MODULES might not happen, you should be sure to include an dnl explicit call to PKG_PROG_PKG_CONFIG in your configure.ac which we are not doing. We got away with that as long as there was only one PKG_CHECK_MODULES call ... but with two, I'd expect that the second one will fall over if the first one isn't executed. regards, tom lane
On Thu, 12 Mar 2020 at 03:39, Tom Lane wrote:
Daniel Gustafsson writes:
> For me, the duplication aspect is the most troubling, since we'd still need the
> xml2-config fallback and thus won't be able to simplify the code.
Yeah, but at least it's concentrated in a few lines in configure.in.
I think that the main objection to this is the documentation/confusion
issues raised by Noah in that old thread. Still, we probably don't
have much choice given that some distros are going to remove xml2-config.
In that connection, Hugh's patch lacks docs which is entirely not OK,
but the doc changes in Peter's old patch look workable.
Documentation can be added easily. :-)
I wonder whether we ought to try to align this with our documented
procedure for falling back if you have no icu-config but want to
use ICU; that part of the docs suggests setting ICU_CFLAGS and ICU_LIBS
manually.
Unless your system has ICU installed in a non-standard location, there is no need to set those variables, as PKG_CHECK_MODULES will handle that for you. `./configure --help` also provides the relevant documentation on overriding pkg-config’s X_CFLAGS and X_LIBS.
The patch as it stands doesn't seem to support manually
giving XML2_CFLAGS and XML2_LIBS, but it looks like it could easily
be adjusted to allow that.
What I said for ICU also applies here (in fact, to all use of PKG_CHECK_MODULES). For the fallback, a minor rework is required.
The question is really whether we want to maintain a fallback to xml2-config. To give more context, I gave a more detailed assessment of the situation in an earlier email to this list. (Personally, I don’t think we should.)
Do note also that xslt-config will also be a problem at some point.
Also, I see that pkg.m4 says
dnl Note that if there is a possibility the first call to
dnl PKG_CHECK_MODULES might not happen, you should be sure to include an
dnl explicit call to PKG_PROG_PKG_CONFIG in your configure.ac
which we are not doing. We got away with that as long as there was only
one PKG_CHECK_MODULES call ... but with two, I'd expect that the second
one will fall over if the first one isn't executed.
Yes, that macro needs to be set.
Hugh
Hugh McMaster <hugh.mcmaster@outlook.com> writes:
> The question is really whether we want to maintain a fallback to
> xml2-config. To give more context, I gave a more detailed assessment of the
> situation in an earlier email to this list. (Personally, I don’t think we
> should.)
I think that that is not optional. We try to maintain portability
to old systems as well as new.
regards, tom lane
I poked at this issue a bit more and realized that Hugh's patch
flat-out breaks building --with-libxml in environments without
pkg-config, because PKG_CHECK_MODULES just gives up and dies
if there's no pkg-config (as we'd already found out in connection
with ICU). That won't win us any friends, so the attached revision
doesn't call PKG_CHECK_MODULES unless we found pkg-config. I also
concluded that if the user has set XML2_CONFIG, it's pretty clear
that her intent is to use whatever that is pointing at, so we should
not use pkg-config in that case either.
Also, I'd been going back and forth about whether it was worth
documenting XML2_CFLAGS/XML2_LIBS, but I realized that use of
PKG_CHECK_MODULES(XML2, ...) basically forces the issue for us:
it does AC_ARG_VAR on them, which puts them into configure's
--help output and makes configure picky about caching them.
So we can't really pretend they're boring implementation detail.
So the attached mostly adopts Peter's old suggested docs, but
I added discussion of XML2_CFLAGS/XML2_LIBS and dropped the mention
of forcing matters with --with-libs/--with-libraries (not because
that doesn't work anymore but because it seemed like we were offering
quite enough alternatives already).
I'd originally thought that we might back-patch this, but I'm now of
the opinion that we probably should not. If pkg-config is present,
this can change the default behavior about where we get libxml from,
which seems like something not to do in minor releases. (OTOH, it'd
only matter if the default pkg-config choice is different from the
default xml2-config choice, so maybe the risk of breakage is small
enough to be acceptable?)
regards, tom lane
diff --git a/configure b/configure
index 1a0aca9..253df29 100755
--- a/configure
+++ b/configure
@@ -703,6 +703,8 @@ with_zlib
with_system_tzdata
with_libxslt
with_libxml
+XML2_LIBS
+XML2_CFLAGS
XML2_CONFIG
UUID_EXTRA_OBJS
with_uuid
@@ -719,13 +721,13 @@ with_perl
with_tcl
ICU_LIBS
ICU_CFLAGS
-PKG_CONFIG_LIBDIR
-PKG_CONFIG_PATH
-PKG_CONFIG
with_icu
enable_thread_safety
INCLUDES
autodepend
+PKG_CONFIG_LIBDIR
+PKG_CONFIG_PATH
+PKG_CONFIG
TAS
GCC
CPP
@@ -887,6 +889,8 @@ PKG_CONFIG_LIBDIR
ICU_CFLAGS
ICU_LIBS
XML2_CONFIG
+XML2_CFLAGS
+XML2_LIBS
LDFLAGS_EX
LDFLAGS_SL
PERL
@@ -1589,6 +1593,8 @@ Some influential environment variables:
ICU_CFLAGS C compiler flags for ICU, overriding pkg-config
ICU_LIBS linker flags for ICU, overriding pkg-config
XML2_CONFIG path to xml2-config utility
+ XML2_CFLAGS C compiler flags for XML2, overriding pkg-config
+ XML2_LIBS linker flags for XML2, overriding pkg-config
LDFLAGS_EX extra linker flags for linking executables only
LDFLAGS_SL extra linker flags for linking shared libraries only
PERL Perl program
@@ -7153,6 +7159,129 @@ else
fi
+#
+# Set up pkg_config in case we need it below
+#
+
+
+
+
+
+
+
+if test "x$ac_cv_env_PKG_CONFIG_set" != "xset"; then
+ if test -n "$ac_tool_prefix"; then
+ # Extract the first word of "${ac_tool_prefix}pkg-config", so it can be a program name with args.
+set dummy ${ac_tool_prefix}pkg-config; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_path_PKG_CONFIG+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ case $PKG_CONFIG in
+ [\\/]* | ?:[\\/]*)
+ ac_cv_path_PKG_CONFIG="$PKG_CONFIG" # Let the user override the test with a path.
+ ;;
+ *)
+ as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_path_PKG_CONFIG="$as_dir/$ac_word$ac_exec_ext"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+ ;;
+esac
+fi
+PKG_CONFIG=$ac_cv_path_PKG_CONFIG
+if test -n "$PKG_CONFIG"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $PKG_CONFIG" >&5
+$as_echo "$PKG_CONFIG" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+fi
+if test -z "$ac_cv_path_PKG_CONFIG"; then
+ ac_pt_PKG_CONFIG=$PKG_CONFIG
+ # Extract the first word of "pkg-config", so it can be a program name with args.
+set dummy pkg-config; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_path_ac_pt_PKG_CONFIG+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ case $ac_pt_PKG_CONFIG in
+ [\\/]* | ?:[\\/]*)
+ ac_cv_path_ac_pt_PKG_CONFIG="$ac_pt_PKG_CONFIG" # Let the user override the test with a path.
+ ;;
+ *)
+ as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_path_ac_pt_PKG_CONFIG="$as_dir/$ac_word$ac_exec_ext"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+ ;;
+esac
+fi
+ac_pt_PKG_CONFIG=$ac_cv_path_ac_pt_PKG_CONFIG
+if test -n "$ac_pt_PKG_CONFIG"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_pt_PKG_CONFIG" >&5
+$as_echo "$ac_pt_PKG_CONFIG" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+ if test "x$ac_pt_PKG_CONFIG" = x; then
+ PKG_CONFIG=""
+ else
+ case $cross_compiling:$ac_tool_warned in
+yes:)
+{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: using cross tools not prefixed with host triplet" >&5
+$as_echo "$as_me: WARNING: using cross tools not prefixed with host triplet" >&2;}
+ac_tool_warned=yes ;;
+esac
+ PKG_CONFIG=$ac_pt_PKG_CONFIG
+ fi
+else
+ PKG_CONFIG="$ac_cv_path_PKG_CONFIG"
+fi
+
+fi
+if test -n "$PKG_CONFIG"; then
+ _pkg_min_version=0.9.0
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking pkg-config is at least version $_pkg_min_version" >&5
+$as_echo_n "checking pkg-config is at least version $_pkg_min_version... " >&6; }
+ if $PKG_CONFIG --atleast-pkgconfig-version $_pkg_min_version; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+ else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+ PKG_CONFIG=""
+ fi
+fi
#
# Automatic dependency tracking
@@ -7321,126 +7450,6 @@ $as_echo "$with_icu" >&6; }
if test "$with_icu" = yes; then
-
-
-
-
-
-
-if test "x$ac_cv_env_PKG_CONFIG_set" != "xset"; then
- if test -n "$ac_tool_prefix"; then
- # Extract the first word of "${ac_tool_prefix}pkg-config", so it can be a program name with args.
-set dummy ${ac_tool_prefix}pkg-config; ac_word=$2
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
-$as_echo_n "checking for $ac_word... " >&6; }
-if ${ac_cv_path_PKG_CONFIG+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- case $PKG_CONFIG in
- [\\/]* | ?:[\\/]*)
- ac_cv_path_PKG_CONFIG="$PKG_CONFIG" # Let the user override the test with a path.
- ;;
- *)
- as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
-for as_dir in $PATH
-do
- IFS=$as_save_IFS
- test -z "$as_dir" && as_dir=.
- for ac_exec_ext in '' $ac_executable_extensions; do
- if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
- ac_cv_path_PKG_CONFIG="$as_dir/$ac_word$ac_exec_ext"
- $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
- break 2
- fi
-done
- done
-IFS=$as_save_IFS
-
- ;;
-esac
-fi
-PKG_CONFIG=$ac_cv_path_PKG_CONFIG
-if test -n "$PKG_CONFIG"; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: $PKG_CONFIG" >&5
-$as_echo "$PKG_CONFIG" >&6; }
-else
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
-$as_echo "no" >&6; }
-fi
-
-
-fi
-if test -z "$ac_cv_path_PKG_CONFIG"; then
- ac_pt_PKG_CONFIG=$PKG_CONFIG
- # Extract the first word of "pkg-config", so it can be a program name with args.
-set dummy pkg-config; ac_word=$2
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
-$as_echo_n "checking for $ac_word... " >&6; }
-if ${ac_cv_path_ac_pt_PKG_CONFIG+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- case $ac_pt_PKG_CONFIG in
- [\\/]* | ?:[\\/]*)
- ac_cv_path_ac_pt_PKG_CONFIG="$ac_pt_PKG_CONFIG" # Let the user override the test with a path.
- ;;
- *)
- as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
-for as_dir in $PATH
-do
- IFS=$as_save_IFS
- test -z "$as_dir" && as_dir=.
- for ac_exec_ext in '' $ac_executable_extensions; do
- if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
- ac_cv_path_ac_pt_PKG_CONFIG="$as_dir/$ac_word$ac_exec_ext"
- $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
- break 2
- fi
-done
- done
-IFS=$as_save_IFS
-
- ;;
-esac
-fi
-ac_pt_PKG_CONFIG=$ac_cv_path_ac_pt_PKG_CONFIG
-if test -n "$ac_pt_PKG_CONFIG"; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_pt_PKG_CONFIG" >&5
-$as_echo "$ac_pt_PKG_CONFIG" >&6; }
-else
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
-$as_echo "no" >&6; }
-fi
-
- if test "x$ac_pt_PKG_CONFIG" = x; then
- PKG_CONFIG=""
- else
- case $cross_compiling:$ac_tool_warned in
-yes:)
-{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: using cross tools not prefixed with host triplet" >&5
-$as_echo "$as_me: WARNING: using cross tools not prefixed with host triplet" >&2;}
-ac_tool_warned=yes ;;
-esac
- PKG_CONFIG=$ac_pt_PKG_CONFIG
- fi
-else
- PKG_CONFIG="$ac_cv_path_PKG_CONFIG"
-fi
-
-fi
-if test -n "$PKG_CONFIG"; then
- _pkg_min_version=0.9.0
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking pkg-config is at least version $_pkg_min_version" >&5
-$as_echo_n "checking pkg-config is at least version $_pkg_min_version... " >&6; }
- if $PKG_CONFIG --atleast-pkgconfig-version $_pkg_min_version; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
-$as_echo "yes" >&6; }
- else
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
-$as_echo "no" >&6; }
- PKG_CONFIG=""
- fi
-fi
-
pkg_failed=no
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for icu-uc icu-i18n" >&5
$as_echo_n "checking for icu-uc icu-i18n... " >&6; }
@@ -8161,7 +8170,103 @@ fi
if test "$with_libxml" = yes ; then
- if test -z "$XML2_CONFIG"; then
+ # For backwards compatibility, setting XML2_CONFIG overrides pkg_config
+ have_libxml2_pkg_config=no
+ if test -z "$XML2_CONFIG" -a -n "$PKG_CONFIG"; then
+
+pkg_failed=no
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for libxml-2.0 >= 2.6.23" >&5
+$as_echo_n "checking for libxml-2.0 >= 2.6.23... " >&6; }
+
+if test -n "$XML2_CFLAGS"; then
+ pkg_cv_XML2_CFLAGS="$XML2_CFLAGS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libxml-2.0 >= 2.6.23\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "libxml-2.0 >= 2.6.23") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_XML2_CFLAGS=`$PKG_CONFIG --cflags "libxml-2.0 >= 2.6.23" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+if test -n "$XML2_LIBS"; then
+ pkg_cv_XML2_LIBS="$XML2_LIBS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libxml-2.0 >= 2.6.23\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "libxml-2.0 >= 2.6.23") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_XML2_LIBS=`$PKG_CONFIG --libs "libxml-2.0 >= 2.6.23" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+
+
+
+if test $pkg_failed = yes; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then
+ _pkg_short_errors_supported=yes
+else
+ _pkg_short_errors_supported=no
+fi
+ if test $_pkg_short_errors_supported = yes; then
+ XML2_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "libxml-2.0 >= 2.6.23" 2>&1`
+ else
+ XML2_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "libxml-2.0 >= 2.6.23" 2>&1`
+ fi
+ # Put the nasty error message in config.log where it belongs
+ echo "$XML2_PKG_ERRORS" >&5
+
+ as_fn_error $? "Package requirements (libxml-2.0 >= 2.6.23) were not met:
+
+$XML2_PKG_ERRORS
+
+Consider adjusting the PKG_CONFIG_PATH environment variable if you
+installed software in a non-standard prefix.
+
+Alternatively, you may set the environment variables XML2_CFLAGS
+and XML2_LIBS to avoid the need to call pkg-config.
+See the pkg-config man page for more details." "$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.
+
+Alternatively, you may set the environment variables XML2_CFLAGS
+and XML2_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; }
+else
+ XML2_CFLAGS=$pkg_cv_XML2_CFLAGS
+ XML2_LIBS=$pkg_cv_XML2_LIBS
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+ have_libxml2_pkg_config=yes
+fi
+ fi
+ if test "$have_libxml2_pkg_config" = no ; then
+ if test -z "$XML2_CONFIG"; then
for ac_prog in xml2-config
do
# Extract the first word of "$ac_prog", so it can be a program name with args.
@@ -8216,17 +8321,21 @@ $as_echo "$XML2_CONFIG" >&6; }
fi
if test -n "$XML2_CONFIG"; then
- for pgac_option in `$XML2_CONFIG --cflags`; do
- case $pgac_option in
- -I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
- esac
- done
- for pgac_option in `$XML2_CONFIG --libs`; do
- case $pgac_option in
- -L*) LDFLAGS="$LDFLAGS $pgac_option";;
- esac
- done
+ XML2_CFLAGS=`$XML2_CONFIG --cflags`
+ XML2_LIBS=`$XML2_CONFIG --libs`
+ fi
fi
+ # Note the user could also set XML2_CFLAGS/XML2_LIBS directly
+ for pgac_option in $XML2_CFLAGS; do
+ case $pgac_option in
+ -I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
+ esac
+ done
+ for pgac_option in $XML2_LIBS; do
+ case $pgac_option in
+ -L*) LDFLAGS="$LDFLAGS $pgac_option";;
+ esac
+ done
fi
diff --git a/configure.in b/configure.in
index b27708e..d4e7a87 100644
--- a/configure.in
+++ b/configure.in
@@ -667,6 +667,10 @@ else
fi
AC_SUBST(TAS)
+#
+# Set up pkg_config in case we need it below
+#
+PKG_PROG_PKG_CONFIG
#
# Automatic dependency tracking
@@ -931,20 +935,31 @@ PGAC_ARG_BOOL(with, libxml, no, [build with XML support],
[AC_DEFINE([USE_LIBXML], 1, [Define to 1 to build with XML support. (--with-libxml)])])
if test "$with_libxml" = yes ; then
- PGAC_PATH_PROGS(XML2_CONFIG, xml2-config)
AC_ARG_VAR(XML2_CONFIG, [path to xml2-config utility])dnl
- if test -n "$XML2_CONFIG"; then
- for pgac_option in `$XML2_CONFIG --cflags`; do
- case $pgac_option in
- -I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
- esac
- done
- for pgac_option in `$XML2_CONFIG --libs`; do
- case $pgac_option in
- -L*) LDFLAGS="$LDFLAGS $pgac_option";;
- esac
- done
+ # For backwards compatibility, setting XML2_CONFIG overrides pkg_config
+ have_libxml2_pkg_config=no
+ if test -z "$XML2_CONFIG" -a -n "$PKG_CONFIG"; then
+ PKG_CHECK_MODULES(XML2, [libxml-2.0 >= 2.6.23],
+ [have_libxml2_pkg_config=yes])
+ fi
+ if test "$have_libxml2_pkg_config" = no ; then
+ PGAC_PATH_PROGS(XML2_CONFIG, xml2-config)
+ if test -n "$XML2_CONFIG"; then
+ XML2_CFLAGS=`$XML2_CONFIG --cflags`
+ XML2_LIBS=`$XML2_CONFIG --libs`
+ fi
fi
+ # Note the user could also set XML2_CFLAGS/XML2_LIBS directly
+ for pgac_option in $XML2_CFLAGS; do
+ case $pgac_option in
+ -I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
+ esac
+ done
+ for pgac_option in $XML2_LIBS; do
+ case $pgac_option in
+ -L*) LDFLAGS="$LDFLAGS $pgac_option";;
+ esac
+ done
fi
AC_SUBST(with_libxml)
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index cc242dc..0326ac4 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -1122,16 +1122,26 @@ build-postgresql:
</para>
<para>
- Libxml installs a program <command>xml2-config</command> that
- can be used to detect the required compiler and linker
- options. PostgreSQL will use it automatically if found. To
- specify a libxml installation at an unusual location, you can
- either set the environment variable
- <envar>XML2_CONFIG</envar> to point to the
- <command>xml2-config</command> program belonging to the
- installation, or use the options
- <option>--with-includes</option> and
- <option>--with-libraries</option>.
+ To detect the required compiler and linker options, PostgreSQL will
+ query <command>pkg-config</command>, if that is installed and knows
+ about libxml2. Otherwise the program <command>xml2-config</command>,
+ which is installed by libxml, will be used if it is found. Use
+ of <command>pkg-config</command> is preferred, because it can deal
+ with multi-architecture installations better.
+ </para>
+
+ <para>
+ To use a libxml installation that is in an unusual location, you
+ can set <command>pkg-config</command>-related environment
+ variables (see its documentation), or set the environment variable
+ <envar>XML2_CONFIG</envar> to point to
+ the <command>xml2-config</command> program belonging to the libxml
+ installation, or set the variables <envar>XML2_CFLAGS</envar>
+ and <envar>XML2_LIBS</envar>. (If <command>pkg-config</command> is
+ installed, then to override its idea of where libxml is you must
+ either set <envar>XML2_CONFIG</envar> or set
+ both <envar>XML2_CFLAGS</envar> and <envar>XML2_LIBS</envar> to
+ nonempty strings.)
</para>
</listitem>
</varlistentry>
> On 12 Mar 2020, at 17:39, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I also > concluded that if the user has set XML2_CONFIG, it's pretty clear > that her intent is to use whatever that is pointing at, so we should > not use pkg-config in that case either. +1 > I'd originally thought that we might back-patch this, but I'm now of > the opinion that we probably should not. If pkg-config is present, > this can change the default behavior about where we get libxml from, > which seems like something not to do in minor releases. (OTOH, it'd > only matter if the default pkg-config choice is different from the > default xml2-config choice, so maybe the risk of breakage is small > enough to be acceptable?) I read this is as a preventative patch to stay ahead of future changes to packaging. If these changes do materialize, won't they be equally likely to hit installations for backbranch minors as v13? Changing behavior in a minor release is however not ideal, but perhaps the risk/benefit analysis comes down to releases not compiling being worse? I haven't had the chance to test the patch, but the changes to configure.in reads perfectly fine. In the docs though: > + To use a libxml installation that is in an unusual location, you We refer to both libxml and libxml2 in these paragraphs. Since upstream is consistently referring to it as libxml2, maybe we should take this as opportunity to switch to that for the docs? cheers ./daniel
On Fri, 13 Mar 2020 at 03:39, Tom Lane wrote: > I poked at this issue a bit more and realized that Hugh's patch > flat-out breaks building --with-libxml in environments without > pkg-config, because PKG_CHECK_MODULES just gives up and dies > if there's no pkg-config (as we'd already found out in connection > with ICU). As you found out, that is by design. PKG_CHECK_MODULES actually checks for pkg-config via PKG_PROG_PKG_CONFIG, but only in the first expansion of PKG_CHECK_MODULES. If the first instance is in a conditional, then the check for pkg-config is also in that conditional. Once a build system begins using pkg-config without a fallback (e.g. like for ICU), pkg-config becomes a build dependency (and, yes, I realise ICU isn't mandatory here). That won't win us any friends, so the attached revision > doesn't call PKG_CHECK_MODULES unless we found pkg-config. Did you mean to terminate configure if pkg-config cannot find libxml-2.0 or the library is too old? Your doc changes don't indicate that intent, nor was it prior behaviour, but some projects like that behaviour and others don't. > I also concluded that if the user has set XML2_CONFIG, it's pretty clear > that her intent is to use whatever that is pointing at, so we should > not use pkg-config in that case either. > > Also, I'd been going back and forth about whether it was worth > documenting XML2_CFLAGS/XML2_LIBS, but I realized that use of > PKG_CHECK_MODULES(XML2, ...) basically forces the issue for us: > it does AC_ARG_VAR on them, which puts them into configure's > --help output and makes configure picky about caching them. > So we can't really pretend they're boring implementation detail. You might consider this an edge case, but you override custom XML2_CFLAGS/LIBS if xml2-config is detected. > So the attached mostly adopts Peter's old suggested docs, but > I added discussion of XML2_CFLAGS/XML2_LIBS and dropped the mention > of forcing matters with --with-libs/--with-libraries (not because > that doesn't work anymore but because it seemed like we were offering > quite enough alternatives already). > > I'd originally thought that we might back-patch this, but I'm now of > the opinion that we probably should not. If pkg-config is present, > this can change the default behavior about where we get libxml from, > which seems like something not to do in minor releases. (OTOH, it'd > only matter if the default pkg-config choice is different from the > default xml2-config choice, so maybe the risk of breakage is small > enough to be acceptable?)
Hugh McMaster <hugh.mcmaster@outlook.com> writes:
> On Fri, 13 Mar 2020 at 03:39, Tom Lane wrote:
>> That won't win us any friends, so the attached revision
>> doesn't call PKG_CHECK_MODULES unless we found pkg-config.
> Did you mean to terminate configure if pkg-config cannot find
> libxml-2.0 or the library is too old? Your doc changes don't indicate
> that intent, nor was it prior behaviour, but some projects like that
> behaviour and others don't.
Yeah, a potential edge-case here is that pkg-config is in the PATH
but it has no information about libxml2 (I doubt we need to consider
the risk that it has info about a pre-2.6.23 version). Looking
again at the generated configure code, I realize I shouldn't have
left off the ACTION-IF-NOT-FOUND argument --- the default is to
throw an error, but we'd rather fall through and try to use xml2-config.
The eventual AC_CHECK_LIB(xml2, ...) test will catch the situation
where the library isn't there. Updated patch attached.
> You might consider this an edge case, but you override custom
> XML2_CFLAGS/LIBS if xml2-config is detected.
Yeah, if pkg-config fails and xml2-config is present, that's true.
Since those weren't there before, and we now document them as just
a fallback solution, I think that's fine.
regards, tom lane
diff --git a/configure b/configure
index 1a0aca9..601e6e0 100755
--- a/configure
+++ b/configure
@@ -703,6 +703,8 @@ with_zlib
with_system_tzdata
with_libxslt
with_libxml
+XML2_LIBS
+XML2_CFLAGS
XML2_CONFIG
UUID_EXTRA_OBJS
with_uuid
@@ -719,13 +721,13 @@ with_perl
with_tcl
ICU_LIBS
ICU_CFLAGS
-PKG_CONFIG_LIBDIR
-PKG_CONFIG_PATH
-PKG_CONFIG
with_icu
enable_thread_safety
INCLUDES
autodepend
+PKG_CONFIG_LIBDIR
+PKG_CONFIG_PATH
+PKG_CONFIG
TAS
GCC
CPP
@@ -887,6 +889,8 @@ PKG_CONFIG_LIBDIR
ICU_CFLAGS
ICU_LIBS
XML2_CONFIG
+XML2_CFLAGS
+XML2_LIBS
LDFLAGS_EX
LDFLAGS_SL
PERL
@@ -1589,6 +1593,8 @@ Some influential environment variables:
ICU_CFLAGS C compiler flags for ICU, overriding pkg-config
ICU_LIBS linker flags for ICU, overriding pkg-config
XML2_CONFIG path to xml2-config utility
+ XML2_CFLAGS C compiler flags for XML2, overriding pkg-config
+ XML2_LIBS linker flags for XML2, overriding pkg-config
LDFLAGS_EX extra linker flags for linking executables only
LDFLAGS_SL extra linker flags for linking shared libraries only
PERL Perl program
@@ -7153,6 +7159,129 @@ else
fi
+#
+# Set up pkg_config in case we need it below
+#
+
+
+
+
+
+
+
+if test "x$ac_cv_env_PKG_CONFIG_set" != "xset"; then
+ if test -n "$ac_tool_prefix"; then
+ # Extract the first word of "${ac_tool_prefix}pkg-config", so it can be a program name with args.
+set dummy ${ac_tool_prefix}pkg-config; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_path_PKG_CONFIG+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ case $PKG_CONFIG in
+ [\\/]* | ?:[\\/]*)
+ ac_cv_path_PKG_CONFIG="$PKG_CONFIG" # Let the user override the test with a path.
+ ;;
+ *)
+ as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_path_PKG_CONFIG="$as_dir/$ac_word$ac_exec_ext"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+ ;;
+esac
+fi
+PKG_CONFIG=$ac_cv_path_PKG_CONFIG
+if test -n "$PKG_CONFIG"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $PKG_CONFIG" >&5
+$as_echo "$PKG_CONFIG" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+
+fi
+if test -z "$ac_cv_path_PKG_CONFIG"; then
+ ac_pt_PKG_CONFIG=$PKG_CONFIG
+ # Extract the first word of "pkg-config", so it can be a program name with args.
+set dummy pkg-config; ac_word=$2
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
+$as_echo_n "checking for $ac_word... " >&6; }
+if ${ac_cv_path_ac_pt_PKG_CONFIG+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ case $ac_pt_PKG_CONFIG in
+ [\\/]* | ?:[\\/]*)
+ ac_cv_path_ac_pt_PKG_CONFIG="$ac_pt_PKG_CONFIG" # Let the user override the test with a path.
+ ;;
+ *)
+ as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
+for as_dir in $PATH
+do
+ IFS=$as_save_IFS
+ test -z "$as_dir" && as_dir=.
+ for ac_exec_ext in '' $ac_executable_extensions; do
+ if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
+ ac_cv_path_ac_pt_PKG_CONFIG="$as_dir/$ac_word$ac_exec_ext"
+ $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
+ break 2
+ fi
+done
+ done
+IFS=$as_save_IFS
+
+ ;;
+esac
+fi
+ac_pt_PKG_CONFIG=$ac_cv_path_ac_pt_PKG_CONFIG
+if test -n "$ac_pt_PKG_CONFIG"; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_pt_PKG_CONFIG" >&5
+$as_echo "$ac_pt_PKG_CONFIG" >&6; }
+else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+fi
+
+ if test "x$ac_pt_PKG_CONFIG" = x; then
+ PKG_CONFIG=""
+ else
+ case $cross_compiling:$ac_tool_warned in
+yes:)
+{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: using cross tools not prefixed with host triplet" >&5
+$as_echo "$as_me: WARNING: using cross tools not prefixed with host triplet" >&2;}
+ac_tool_warned=yes ;;
+esac
+ PKG_CONFIG=$ac_pt_PKG_CONFIG
+ fi
+else
+ PKG_CONFIG="$ac_cv_path_PKG_CONFIG"
+fi
+
+fi
+if test -n "$PKG_CONFIG"; then
+ _pkg_min_version=0.9.0
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking pkg-config is at least version $_pkg_min_version" >&5
+$as_echo_n "checking pkg-config is at least version $_pkg_min_version... " >&6; }
+ if $PKG_CONFIG --atleast-pkgconfig-version $_pkg_min_version; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+ else
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+ PKG_CONFIG=""
+ fi
+fi
#
# Automatic dependency tracking
@@ -7321,126 +7450,6 @@ $as_echo "$with_icu" >&6; }
if test "$with_icu" = yes; then
-
-
-
-
-
-
-if test "x$ac_cv_env_PKG_CONFIG_set" != "xset"; then
- if test -n "$ac_tool_prefix"; then
- # Extract the first word of "${ac_tool_prefix}pkg-config", so it can be a program name with args.
-set dummy ${ac_tool_prefix}pkg-config; ac_word=$2
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
-$as_echo_n "checking for $ac_word... " >&6; }
-if ${ac_cv_path_PKG_CONFIG+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- case $PKG_CONFIG in
- [\\/]* | ?:[\\/]*)
- ac_cv_path_PKG_CONFIG="$PKG_CONFIG" # Let the user override the test with a path.
- ;;
- *)
- as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
-for as_dir in $PATH
-do
- IFS=$as_save_IFS
- test -z "$as_dir" && as_dir=.
- for ac_exec_ext in '' $ac_executable_extensions; do
- if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
- ac_cv_path_PKG_CONFIG="$as_dir/$ac_word$ac_exec_ext"
- $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
- break 2
- fi
-done
- done
-IFS=$as_save_IFS
-
- ;;
-esac
-fi
-PKG_CONFIG=$ac_cv_path_PKG_CONFIG
-if test -n "$PKG_CONFIG"; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: $PKG_CONFIG" >&5
-$as_echo "$PKG_CONFIG" >&6; }
-else
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
-$as_echo "no" >&6; }
-fi
-
-
-fi
-if test -z "$ac_cv_path_PKG_CONFIG"; then
- ac_pt_PKG_CONFIG=$PKG_CONFIG
- # Extract the first word of "pkg-config", so it can be a program name with args.
-set dummy pkg-config; ac_word=$2
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for $ac_word" >&5
-$as_echo_n "checking for $ac_word... " >&6; }
-if ${ac_cv_path_ac_pt_PKG_CONFIG+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- case $ac_pt_PKG_CONFIG in
- [\\/]* | ?:[\\/]*)
- ac_cv_path_ac_pt_PKG_CONFIG="$ac_pt_PKG_CONFIG" # Let the user override the test with a path.
- ;;
- *)
- as_save_IFS=$IFS; IFS=$PATH_SEPARATOR
-for as_dir in $PATH
-do
- IFS=$as_save_IFS
- test -z "$as_dir" && as_dir=.
- for ac_exec_ext in '' $ac_executable_extensions; do
- if as_fn_executable_p "$as_dir/$ac_word$ac_exec_ext"; then
- ac_cv_path_ac_pt_PKG_CONFIG="$as_dir/$ac_word$ac_exec_ext"
- $as_echo "$as_me:${as_lineno-$LINENO}: found $as_dir/$ac_word$ac_exec_ext" >&5
- break 2
- fi
-done
- done
-IFS=$as_save_IFS
-
- ;;
-esac
-fi
-ac_pt_PKG_CONFIG=$ac_cv_path_ac_pt_PKG_CONFIG
-if test -n "$ac_pt_PKG_CONFIG"; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_pt_PKG_CONFIG" >&5
-$as_echo "$ac_pt_PKG_CONFIG" >&6; }
-else
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
-$as_echo "no" >&6; }
-fi
-
- if test "x$ac_pt_PKG_CONFIG" = x; then
- PKG_CONFIG=""
- else
- case $cross_compiling:$ac_tool_warned in
-yes:)
-{ $as_echo "$as_me:${as_lineno-$LINENO}: WARNING: using cross tools not prefixed with host triplet" >&5
-$as_echo "$as_me: WARNING: using cross tools not prefixed with host triplet" >&2;}
-ac_tool_warned=yes ;;
-esac
- PKG_CONFIG=$ac_pt_PKG_CONFIG
- fi
-else
- PKG_CONFIG="$ac_cv_path_PKG_CONFIG"
-fi
-
-fi
-if test -n "$PKG_CONFIG"; then
- _pkg_min_version=0.9.0
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking pkg-config is at least version $_pkg_min_version" >&5
-$as_echo_n "checking pkg-config is at least version $_pkg_min_version... " >&6; }
- if $PKG_CONFIG --atleast-pkgconfig-version $_pkg_min_version; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
-$as_echo "yes" >&6; }
- else
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
-$as_echo "no" >&6; }
- PKG_CONFIG=""
- fi
-fi
-
pkg_failed=no
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for icu-uc icu-i18n" >&5
$as_echo_n "checking for icu-uc icu-i18n... " >&6; }
@@ -8161,7 +8170,83 @@ fi
if test "$with_libxml" = yes ; then
- if test -z "$XML2_CONFIG"; then
+ # For backwards compatibility, setting XML2_CONFIG overrides pkg_config
+ have_libxml2_pkg_config=no
+ if test -z "$XML2_CONFIG" -a -n "$PKG_CONFIG"; then
+
+pkg_failed=no
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for libxml-2.0 >= 2.6.23" >&5
+$as_echo_n "checking for libxml-2.0 >= 2.6.23... " >&6; }
+
+if test -n "$XML2_CFLAGS"; then
+ pkg_cv_XML2_CFLAGS="$XML2_CFLAGS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libxml-2.0 >= 2.6.23\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "libxml-2.0 >= 2.6.23") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_XML2_CFLAGS=`$PKG_CONFIG --cflags "libxml-2.0 >= 2.6.23" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+if test -n "$XML2_LIBS"; then
+ pkg_cv_XML2_LIBS="$XML2_LIBS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libxml-2.0 >= 2.6.23\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "libxml-2.0 >= 2.6.23") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_XML2_LIBS=`$PKG_CONFIG --libs "libxml-2.0 >= 2.6.23" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+
+
+
+if test $pkg_failed = yes; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then
+ _pkg_short_errors_supported=yes
+else
+ _pkg_short_errors_supported=no
+fi
+ if test $_pkg_short_errors_supported = yes; then
+ XML2_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "libxml-2.0 >= 2.6.23" 2>&1`
+ else
+ XML2_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "libxml-2.0 >= 2.6.23" 2>&1`
+ fi
+ # Put the nasty error message in config.log where it belongs
+ echo "$XML2_PKG_ERRORS" >&5
+
+ # do nothing
+elif test $pkg_failed = untried; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+ # do nothing
+else
+ XML2_CFLAGS=$pkg_cv_XML2_CFLAGS
+ XML2_LIBS=$pkg_cv_XML2_LIBS
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+ have_libxml2_pkg_config=yes
+fi
+ fi
+ if test "$have_libxml2_pkg_config" = no ; then
+ if test -z "$XML2_CONFIG"; then
for ac_prog in xml2-config
do
# Extract the first word of "$ac_prog", so it can be a program name with args.
@@ -8216,17 +8301,21 @@ $as_echo "$XML2_CONFIG" >&6; }
fi
if test -n "$XML2_CONFIG"; then
- for pgac_option in `$XML2_CONFIG --cflags`; do
- case $pgac_option in
- -I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
- esac
- done
- for pgac_option in `$XML2_CONFIG --libs`; do
- case $pgac_option in
- -L*) LDFLAGS="$LDFLAGS $pgac_option";;
- esac
- done
+ XML2_CFLAGS=`$XML2_CONFIG --cflags`
+ XML2_LIBS=`$XML2_CONFIG --libs`
+ fi
fi
+ # Note the user could also set XML2_CFLAGS/XML2_LIBS directly
+ for pgac_option in $XML2_CFLAGS; do
+ case $pgac_option in
+ -I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
+ esac
+ done
+ for pgac_option in $XML2_LIBS; do
+ case $pgac_option in
+ -L*) LDFLAGS="$LDFLAGS $pgac_option";;
+ esac
+ done
fi
diff --git a/configure.in b/configure.in
index b27708e..cfcf190 100644
--- a/configure.in
+++ b/configure.in
@@ -667,6 +667,10 @@ else
fi
AC_SUBST(TAS)
+#
+# Set up pkg_config in case we need it below
+#
+PKG_PROG_PKG_CONFIG
#
# Automatic dependency tracking
@@ -931,20 +935,31 @@ PGAC_ARG_BOOL(with, libxml, no, [build with XML support],
[AC_DEFINE([USE_LIBXML], 1, [Define to 1 to build with XML support. (--with-libxml)])])
if test "$with_libxml" = yes ; then
- PGAC_PATH_PROGS(XML2_CONFIG, xml2-config)
AC_ARG_VAR(XML2_CONFIG, [path to xml2-config utility])dnl
- if test -n "$XML2_CONFIG"; then
- for pgac_option in `$XML2_CONFIG --cflags`; do
- case $pgac_option in
- -I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
- esac
- done
- for pgac_option in `$XML2_CONFIG --libs`; do
- case $pgac_option in
- -L*) LDFLAGS="$LDFLAGS $pgac_option";;
- esac
- done
+ # For backwards compatibility, setting XML2_CONFIG overrides pkg_config
+ have_libxml2_pkg_config=no
+ if test -z "$XML2_CONFIG" -a -n "$PKG_CONFIG"; then
+ PKG_CHECK_MODULES(XML2, [libxml-2.0 >= 2.6.23],
+ [have_libxml2_pkg_config=yes], [# do nothing])
+ fi
+ if test "$have_libxml2_pkg_config" = no ; then
+ PGAC_PATH_PROGS(XML2_CONFIG, xml2-config)
+ if test -n "$XML2_CONFIG"; then
+ XML2_CFLAGS=`$XML2_CONFIG --cflags`
+ XML2_LIBS=`$XML2_CONFIG --libs`
+ fi
fi
+ # Note the user could also set XML2_CFLAGS/XML2_LIBS directly
+ for pgac_option in $XML2_CFLAGS; do
+ case $pgac_option in
+ -I*|-D*) CPPFLAGS="$CPPFLAGS $pgac_option";;
+ esac
+ done
+ for pgac_option in $XML2_LIBS; do
+ case $pgac_option in
+ -L*) LDFLAGS="$LDFLAGS $pgac_option";;
+ esac
+ done
fi
AC_SUBST(with_libxml)
diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml
index cc242dc..0326ac4 100644
--- a/doc/src/sgml/installation.sgml
+++ b/doc/src/sgml/installation.sgml
@@ -1122,16 +1122,26 @@ build-postgresql:
</para>
<para>
- Libxml installs a program <command>xml2-config</command> that
- can be used to detect the required compiler and linker
- options. PostgreSQL will use it automatically if found. To
- specify a libxml installation at an unusual location, you can
- either set the environment variable
- <envar>XML2_CONFIG</envar> to point to the
- <command>xml2-config</command> program belonging to the
- installation, or use the options
- <option>--with-includes</option> and
- <option>--with-libraries</option>.
+ To detect the required compiler and linker options, PostgreSQL will
+ query <command>pkg-config</command>, if that is installed and knows
+ about libxml2. Otherwise the program <command>xml2-config</command>,
+ which is installed by libxml, will be used if it is found. Use
+ of <command>pkg-config</command> is preferred, because it can deal
+ with multi-architecture installations better.
+ </para>
+
+ <para>
+ To use a libxml installation that is in an unusual location, you
+ can set <command>pkg-config</command>-related environment
+ variables (see its documentation), or set the environment variable
+ <envar>XML2_CONFIG</envar> to point to
+ the <command>xml2-config</command> program belonging to the libxml
+ installation, or set the variables <envar>XML2_CFLAGS</envar>
+ and <envar>XML2_LIBS</envar>. (If <command>pkg-config</command> is
+ installed, then to override its idea of where libxml is you must
+ either set <envar>XML2_CONFIG</envar> or set
+ both <envar>XML2_CFLAGS</envar> and <envar>XML2_LIBS</envar> to
+ nonempty strings.)
</para>
</listitem>
</varlistentry>
Daniel Gustafsson <daniel@yesql.se> writes:
>> On 12 Mar 2020, at 17:39, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'd originally thought that we might back-patch this, but I'm now of
>> the opinion that we probably should not. If pkg-config is present,
>> this can change the default behavior about where we get libxml from,
>> which seems like something not to do in minor releases. (OTOH, it'd
>> only matter if the default pkg-config choice is different from the
>> default xml2-config choice, so maybe the risk of breakage is small
>> enough to be acceptable?)
> I read this is as a preventative patch to stay ahead of future changes to
> packaging. If these changes do materialize, won't they be equally likely to
> hit installations for backbranch minors as v13?
Yeah, that's the argument *for* back-patching. Question is whether it
outweighs the risk of silently breaking somebody's build by linking
to the wrong libxml2 version.
I could go either way, honestly. The risk doesn't seem large, but
it's not zero.
> We refer to both libxml and libxml2 in these paragraphs. Since upstream is
> consistently referring to it as libxml2, maybe we should take this as
> opportunity to switch to that for the docs?
I think we're kind of stuck with "--with-libxml". Conceivably we
could introduce "--with-libxml2", redefine the old switch as an
obsolete alias, and start saying "libxml2" instead of "libxml".
But I'm not sure that's worth the trouble, and it seems like
material for a different patch anyway.
regards, tom lane
On Sat, 14 Mar 2020 at 03:06, Tom Lane wrote: > Looking again at the generated configure code, I realize I shouldn't have > left off the ACTION-IF-NOT-FOUND argument --- the default is to > throw an error, but we'd rather fall through and try to use xml2-config. > The eventual AC_CHECK_LIB(xml2, ...) test will catch the situation > where the library isn't there. Updated patch attached. The updated patch works fine. Thanks for working on this! Hugh
> On 13 Mar 2020, at 17:14, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Daniel Gustafsson <daniel@yesql.se> writes: >> I read this is as a preventative patch to stay ahead of future changes to >> packaging. If these changes do materialize, won't they be equally likely to >> hit installations for backbranch minors as v13? > > Yeah, that's the argument *for* back-patching. Question is whether it > outweighs the risk of silently breaking somebody's build by linking > to the wrong libxml2 version. Correct, my argument is that breakage can be expected equally across branches, so I think back-patching should be seriously considered. >> We refer to both libxml and libxml2 in these paragraphs. Since upstream is >> consistently referring to it as libxml2, maybe we should take this as >> opportunity to switch to that for the docs? > > I think we're kind of stuck with "--with-libxml". Conceivably we > could introduce "--with-libxml2", redefine the old switch as an > obsolete alias, and start saying "libxml2" instead of "libxml". > But I'm not sure that's worth the trouble, and it seems like > material for a different patch anyway. Absolutely, thats why I referred to changing mentions of libxml in the docs only where we refer to the product and not the switch (the latter was not very clear in my email though). Also, shouldn't libxml2 be within <productname> tags like OpenSSL and LLVM et.al? cheers ./daniel
Daniel Gustafsson <daniel@yesql.se> writes:
> On 13 Mar 2020, at 17:14, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, that's the argument *for* back-patching. Question is whether it
>> outweighs the risk of silently breaking somebody's build by linking
>> to the wrong libxml2 version.
> Correct, my argument is that breakage can be expected equally across branches,
> so I think back-patching should be seriously considered.
You're right that the risk of breakage (of either type) is about the same
across branches; but the project's conventions are not. We try to avoid
unnecessary changes in back branches.
Still, after further reflection, I think the odds favor back-patching.
This patch could only break things on systems where
(a) There's more than one libxml2 installation, which is already a
tiny minority use-case. It seems very unlikely to hurt any packagers
following typical build processes, for instance.
AND
(b) the default pkg-config and default xml2-config results differ.
That seems even more unlikely.
Now, breakage is certainly possible. A counterexample to (b) is that
if you wanted to build using a non-default libxml2 installation, you
might've tried to select that by putting its xml2-config into your
PATH ahead of the system version, rather than setting XML2_CONFIG.
Post-patch, we'd consult pkg-config first and presumably end up
with the system libxml2.
Still, I think the number of people who'd get bit by that could be
counted without running out of fingers, while it seems quite likely
that many people will soon need to build our back branches on
platforms that won't have xml2-config.
So I'm now leaning to "back-patch and make sure to mention this in
the next release notes". Barring objections, I'll do that soon.
> Absolutely, thats why I referred to changing mentions of libxml in the docs
> only where we refer to the product and not the switch (the latter was not very
> clear in my email though). Also, shouldn't libxml2 be within <productname>
> tags like OpenSSL and LLVM et.al?
I don't have a problem with s/libxml/libxml2/ in the running text
(without changing the switch name). Can't get too excited about
<productname> though. I think we've only consistently applied
that tag to PostgreSQL itself.
regards, tom lane
> On 16 Mar 2020, at 17:12, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Still, I think the number of people who'd get bit by that could be > counted without running out of fingers, while it seems quite likely > that many people will soon need to build our back branches on > platforms that won't have xml2-config. I agree with this assessment. > So I'm now leaning to "back-patch and make sure to mention this in > the next release notes". Barring objections, I'll do that soon. None from me. > I don't have a problem with s/libxml/libxml2/ in the running text > (without changing the switch name). +1 > Can't get too excited about > <productname> though. I think we've only consistently applied > that tag to PostgreSQL itself. Fair enough. Looking at a random sample it seems we use a bit of a mix of nothing, <application /> and <productname /> (the latter ones being more or less equal in DocBook IIRC) to mark up externals. cheers ./daniel
Daniel Gustafsson <daniel@yesql.se> writes:
>> On 16 Mar 2020, at 17:12, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> So I'm now leaning to "back-patch and make sure to mention this in
>> the next release notes". Barring objections, I'll do that soon.
> None from me.
Done. In the event, it only seemed practical to back-patch as far as
v10. 9.x didn't use pkg-config for anything, so our infrastructure
for it isn't there.
regards, tom lane