Re: [PATCH] Use PKG_CHECK_MODULES to detect the libxml2 library

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>