Improve pkg-config wrapper compiler handling

bwbarrett · bwbarrett · commit f4796a872fbb · 2021-11-20T17:06:52.000Z
Rewrite the pkg-config code to split LDFLAGS and LIBS detection
as well as to return the flags in an environment variable instead
of updating the wrapper compiler flags directly.  The split
keeps us from adding -L flags to LIBS (as we had been doing) and
the environment variable means that we can consider using
pkg-config in OPAL_CHECK_PACKAGE, since some of our dependencies
(PMIX) are getting more complicated over time and our package
checks will struggle to keep up.

Rewrite the wrapper compiler code in the hwloc, libevent, and
pmix checks to handle the new pkg-config interface.

Signed-off-by: Brian Barrett &lt;bbarrett@amazon.com&gt;
diff --git a/config/opal_check_pc.m4 b/config/opal_check_pc.m4
@@ -1,7 +1,8 @@
-dnl -*- shell-script -*-
+dnl -*- autoconf -*-
 dnl
 dnl Copyright (c) 2021 IBM Corporation.  All rights reserved.
-dnl
+dnl Copyright (c) 2021      Amazon.com, Inc. or its affiliates.  All Rights
+dnl                         reserved.
 dnl $COPYRIGHT$
 dnl
 dnl Additional copyrights may follow
@@ -10,96 +11,62 @@ dnl $HEADER$
 dnl
 
 
-dnl OPAL_CHECK_PKG_CONFIG
+dnl OPAL_CHECK_PKG_CONFIG()
+dnl
 dnl Check for availability of pkg-config and store the result.
 dnl If it is not available, store any passed in libs from the
 dnl --with-extra-libs configure option, or the known defaults.
 dnl
 dnl If it is available, allow configury to check for .pc files
 dnl and append to OPAL_WRAPPER_EXTRA_LIBS.
 AC_DEFUN([OPAL_CHECK_PKG_CONFIG], [
-  AS_IF([test -z "$OPAL_HAS_PKG_CONFIG"],
-        [
-          AC_MSG_CHECKING([If pkg-config is available])
-          which pkg-config > /dev/null 2>&1
-          AS_IF([test "$?" -eq "0"],
-                [
-                  AC_MSG_RESULT([yes])
-                  OPAL_HAS_PKG_CONFIG="yes"
-                ],
-                [
-                  AC_MSG_RESULT([no])
-                  OPAL_HAS_PKG_CONFIG="no"
-                  OPAL_FLAGS_APPEND_UNIQ([OPAL_WRAPPER_EXTRA_LIBS], [$OPAL_EXTRA_LIBS])
-                ]
-          )
-        ]
-  )
+  AC_CHECK_PROG([PKG_CONFIG], [pkg-config], [pkg-config])
 ])
 
-dnl OPAL_GET_LFLAGS_FROM_PC
-dnl Get the -l flags using pkg-config on the passed in .pc file, and
-dnl append them to OPAL_WRAPPER_EXTRA_LIBS.
-dnl
-dnl Requires 1 or more arguments.
-dnl
-dnl Arg 1 (Required): the name of the .pc file to look for (hwloc/pmix/libevent_core).pc
-dnl 
-dnl Arg 2..n (Optional): The search path for the .pc file above and any dependencies.
-dnl                      These search will be appended and set in the env variable
-dnl                      PKG_CONFIG_PATH to tell pkg-config where to locate the .pc
-dnl                      file from argument 1, and its dependencies.
-dnl
-AC_DEFUN([OPAL_GET_LFLAGS_FROM_PC], [
+dnl OPAL_GET_LDFLAGS_FROM_PC(library name, variable-to-set-if-found,
+dnl                          action-if-not-found)
+AC_DEFUN([OPAL_GET_LDFLAGS_FROM_PC], [
+  OPAL_VAR_SCOPE_PUSH([pkg_config_results happy])
 
   AC_REQUIRE([OPAL_CHECK_PKG_CONFIG])
-  AS_IF([test "$OPAL_HAS_PKG_CONFIG" = "yes"],
-        [
-          AC_MSG_NOTICE([Looking for pc file for $1])
-          OPAL_VAR_SCOPE_PUSH([opal_pkg_config_name opal_pkg_config_env_to_set opal_pkg_config_args opal_pkg_config_cmd opal_pkg_config_result])
-          opal_pkg_config_name=$1
-          opal_pkg_config_env_to_set=""
-          opal_pkg_config_cmd=""
 
-          AS_LITERAL_WORD_IF([$1], [], [m4_fatal([Non-literal argument $1])], [])
-          $1_OMPI_PC_DIR=""
+  AC_MSG_CHECKING([for ldflags from pkg-config file $1])
+
+  happy=1
+  AS_IF([test "$PKG_CONFIG" = ""],
+        [happy=0],
+        [OPAL_LOG_COMMAND([pkg_config_results=`$PKG_CONFIG --static --libs-only-L --libs-only-other $1`],
+             [AS_VAR_COPY([$2], [pkg_config_results])],
+             [happy=0])])
+  AS_IF([test $happy -eq 0],
+        [pkg_config_results="none"
+         $3])
 
-          # Tell pkg-config where to find the .pc file from argument 1
-          # as well as any/all dependencies.
-          # The following block will append the path
-          # to those .pc files into the pkg-config command.
-          # num_paths > 1 implies there is at least one dependent .pc file
-          # to append.
-          AS_IF([test $# -gt 1],
-                [
-                  $1_OMPI_PC_DIR=$2
+  AC_MSG_RESULT([$pkg_config_results])
+
+  OPAL_VAR_SCOPE_POP
+])
+
+dnl OPAL_GET_LIBS_FROM_PC(library name, variable-to-set-if-found,
+dnl                       action-if-not-found)
+AC_DEFUN([OPAL_GET_LIBS_FROM_PC], [
+  OPAL_VAR_SCOPE_PUSH([pkg_config_results happy])
+
+  AC_REQUIRE([OPAL_CHECK_PKG_CONFIG])
 
-                  # Shift the arguments by one to get to the actual paths.
-                  opal_pkg_config_args=m4_shift($@)
+  AC_MSG_CHECKING([for libs from pkg-config file $1])
 
-                  # Iterate over the comma seperated arguments, and replace the ','
-                  for i in $(echo $opal_pkg_config_args | sed "s/,/ /g"); do
-                    opal_pkg_config_env_to_set="$i:${opal_pkg_config_env_to_set}"
-                  done
-                  opal_pkg_config_cmd="env PKG_CONFIG_PATH=$opal_pkg_config_env_to_set pkg-config --libs $opal_pkg_config_name"
-                ],
-                [opal_pkg_config_cmd="pkg-config --libs $opal_pkg_config_name"]
-          )
+  happy=1
+  AS_IF([test "$PKG_CONFIG" = ""],
+        [happy=0],
+        [OPAL_LOG_COMMAND([pkg_config_results=`$PKG_CONFIG --libs-only-l $1`],
+             [AS_VAR_COPY([$2], [pkg_config_results])],
+             [happy=0])])
+  AS_IF([test $happy -eq 0],
+        [pkg_config_results="none"
+         $3])
 
-          OPAL_LOG_MSG([pkg-config cmd for $opal_pkg_config_name: $opal_pkg_config_cmd])
+  AC_MSG_RESULT([$pkg_config_results])
 
-          opal_pkg_config_result=$($opal_pkg_config_cmd)
-          AS_IF([test -z "$opal_pkg_config_result"],
-                [
-                  AC_MSG_WARN([Could not find viable $opal_pkg_config_name.pc])
-                ],
-                [
-                  AC_MSG_NOTICE([pkg-config result $opal_pkg_config_result])
-                  OPAL_FLAGS_APPEND_UNIQ([OPAL_WRAPPER_EXTRA_LIBS], [$opal_pkg_config_result])
-                ]
-          )
-          OPAL_VAR_SCOPE_POP
-        ],
-        []
-  )
+  OPAL_VAR_SCOPE_POP
 ])
diff --git a/config/opal_config_hwloc.m4 b/config/opal_config_hwloc.m4
@@ -3,7 +3,7 @@ dnl
 dnl Copyright (c) 2009-2017 Cisco Systems, Inc.  All rights reserved
 dnl Copyright (c) 2014-2018 Research Organization for Information Science
 dnl                         and Technology (RIST). All rights reserved.
-dnl Copyright (c) 2020      Amazon.com, Inc. or its affiliates.  All Rights
+dnl Copyright (c) 2020-2021 Amazon.com, Inc. or its affiliates.  All Rights
 dnl                         reserved.
 dnl Copyright (c) 2020      Intel, Inc.  All rights reserved.
 dnl $COPYRIGHT$
@@ -39,10 +39,16 @@ dnl         uses Hwloc.  Cannot be added to LIBS yet, because then
 dnl         other execution tests later in configure (there are sadly
 dnl         some) would fail if the path in LDFLAGS was not added to
 dnl         LD_LIBRARY_PATH.
+dnl   * opal_hwloc_WRAPPER_LDFLAGS - the linker flags necessary to
+dnl         add to the wrapper compilers in order to link an opal
+dnl         application when opal is built as a static library.
+dnl   * opal_hwloc_WRAPPER_LIBS - the linker flags necessary to
+dnl         add to the wrapper compilers in order to link an opal
+dnl         application when opal is built as a static library.
 dnl   * CPPFLAGS, LDFLAGS - Updated opal_hwloc_CPPFLAGS and
 dnl         opal_hwloc_LDFLAGS.
 AC_DEFUN([OPAL_CONFIG_HWLOC], [
-    OPAL_VAR_SCOPE_PUSH([external_hwloc_happy internal_hwloc_happy])
+    OPAL_VAR_SCOPE_PUSH([external_hwloc_happy internal_hwloc_happy pkg_config_file pkg_config_happy pkg_config_ldflags pkg_config_libs])
 
     opal_show_subtitle "Configuring hwloc"
 
@@ -70,13 +76,27 @@ AC_DEFUN([OPAL_CONFIG_HWLOC], [
     AS_IF([test "$external_hwloc_happy" = "0" -a "$internal_hwloc_happy" = "0"],
           [AC_MSG_ERROR([Could not find viable hwloc build.])])
 
-    AS_IF([test "$opal_hwloc_mode" = "external"],
-        [AS_IF([test -n "$with_hwloc"],
-               [OPAL_GET_LFLAGS_FROM_PC(hwloc, $with_hwloc/lib/pkgconfig)],
-               [OPAL_GET_LFLAGS_FROM_PC(hwloc)]
-         )],
-        [OPAL_GET_LFLAGS_FROM_PC(hwloc, $OMPI_TOP_SRCDIR/3rd-party/hwloc_directory)]
-    )
+    AS_IF([test "$opal_hwloc_mode" = "internal"],
+          [pkg_config_file="${OMPI_TOP_BUILDDIR}/3rd-party/hwloc_directory/hwloc.pc"
+           PKG_CONFIG_PATH="${OMPI_TOP_BUILDDIR}/3rd-party/hwloc_directory:${PKG_CONFIG_PATH}"],
+          [test -n "$with_hwloc"],
+          [pkg_config_file="${with_hwloc}/lib/pkgconfig/hwloc.pc"
+           PKG_CONFIG_PATH="${with_hwloc}/lib/pkgconfig:${PKG_CONFIG_PATH}"],
+          [pkg_config_file="hwloc"])
+
+    pkg_config_happy=1
+    OPAL_GET_LDFLAGS_FROM_PC([$pkg_config_file], [pkg_config_ldflags], [pkg_config_happy=0])
+    OPAL_GET_LIBS_FROM_PC([$pkg_config_file], [pkg_config_libs], [pkg_config_happy=0])
+
+    AS_IF([test $pkg_config_happy -ne 0],
+          [opal_hwloc_WRAPPER_LDFLAGS="$pkg_config_ldflags"
+           opal_hwloc_WRAPPER_LIBS="$pkg_config_libs"],
+          [# guess that what we have from compiling OMPI is good enough
+           opal_hwloc_WRAPPER_LDFLAGS="$opal_hwloc_LDFLAGS"
+           opal_hwloc_WRAPPER_LIBS="$opal_hwloc_LIBS"])
+
+    OPAL_WRAPPER_FLAGS_ADD([LDFLAGS], [$opal_hwloc_WRAPPER_LDFLAGS])
+    OPAL_WRAPPER_FLAGS_ADD([LIBS], [$opal_hwloc_WRAPPER_LIBS])
 
     # this will work even if there is no hwloc package included,
     # because hwloc_tarball and hwloc_directory will evaluate to an
diff --git a/config/opal_config_libevent.m4 b/config/opal_config_libevent.m4
@@ -4,7 +4,7 @@ dnl Copyright (c) 2009-2018 Cisco Systems, Inc.  All rights reserved
 dnl Copyright (c) 2013      Los Alamos National Security, LLC.  All rights reserved.
 dnl Copyright (c) 2015-2018 Research Organization for Information Science
 dnl                         and Technology (RIST). All rights reserved.
-dnl Copyright (c) 2020      Amazon.com, Inc. or its affiliates.  All Rights
+dnl Copyright (c) 2020-2021 Amazon.com, Inc. or its affiliates.  All Rights
 dnl                         reserved.
 dnl $COPYRIGHT$
 dnl
@@ -39,10 +39,16 @@ dnl         uses Libevent.  Cannot be added to LIBS yet, because then
 dnl         other execution tests later in configure (there are sadly
 dnl         some) would fail if the path in LDFLAGS was not added to
 dnl         LD_LIBRARY_PATH.
+dnl   * opal_libevent_WRAPPER_LDFLAGS - the linker flags necessary to
+dnl         add to the wrapper compilers in order to link an opal
+dnl         application when opal is built as a static library.
+dnl   * opal_libevent_WRAPPER_LIBS - the linker flags necessary to
+dnl         add to the wrapper compilers in order to link an opal
+dnl         application when opal is built as a static library.
 dnl   * CPPFLAGS, LDFLAGS - Updated opal_libevent_CPPFLAGS and
 dnl         opal_libevent_LDFLAGS.
 AC_DEFUN([OPAL_CONFIG_LIBEVENT], [
-    OPAL_VAR_SCOPE_PUSH([internal_libevent_happy external_libevent_happy])
+    OPAL_VAR_SCOPE_PUSH([internal_libevent_happy external_libevent_happy pkg_config_core pkg_config_pthreads pkg_config_core_ldflags pkg_config_pthreads_ldflags pkg_config_core_libs pkg_config_pthreads_libs pkg_config_happy])
 
     opal_show_subtitle "Configuring Libevent"
 
@@ -70,30 +76,38 @@ AC_DEFUN([OPAL_CONFIG_LIBEVENT], [
     AS_IF([test "$external_libevent_happy" = "0" -a "$internal_libevent_happy" = "0"],
           [AC_MSG_ERROR([Could not find viable libevent build.])])
 
-    AS_IF([test "$opal_libevent_mode" = "external"],
-        # The libevent.pc file only returns -levent, however we want
-        # only -libevent_core and -libevent_pthreads, so ompi needs to
-        # do this twice, once for each.
-        [AS_IF([test -n "$with_libevent"],
-               [
-                 OPAL_GET_LFLAGS_FROM_PC(libevent_core, $with_libevent/lib/pkgconfig)
-                 OPAL_GET_LFLAGS_FROM_PC(libevent_pthreads, $with_libevent/lib/pkgconfig)
-               ],
-               [
-                 OPAL_GET_LFLAGS_FROM_PC(libevent_core)
-                 OPAL_GET_LFLAGS_FROM_PC(libevent_pthreads)
-               ]
-         )],
-        [
-          OPAL_GET_LFLAGS_FROM_PC(libevent_core, $OMPI_TOP_SRCDIR/3rd-party/libevent_directory)
-          OPAL_GET_LFLAGS_FROM_PC(libevent_pthreads, $OMPI_TOP_SRCDIR/3rd-party/libevent_directory)
-        ]
-    )
-
-    # Strip -levent as we don't want it. We only want -levent_core/-levent_pthreads.
-    # See https://github.com/open-mpi/ompi/pull/8792
-    OPAL_WRAPPER_EXTRA_LIBS=$(echo $OPAL_WRAPPER_EXTRA_LIBS | sed "s/\\-levent\b//g");
-    AC_MSG_NOTICE([OPAL_WRAPPER_EXTRA_LIBS stripped -levent: $OPAL_WRAPPER_EXTRA_LIBS])
+    AS_IF([test "$opal_libevent_mode" = "internal"],
+          [pkg_config_core="${OMPI_TOP_BUILDDIR}/3rd-party/libevent_directory/libevent_core.pc"
+           pkg_config_pthreads="${OMPI_TOP_BUILDDIR}/3rd-party/libevent_directory/libevent_pthreads.pc"
+           PKG_CONFIG_PATH="${OMPI_TOP_BUILDDIR}/3rd-party/libevent_directory:${PKG_CONFIG_PATH}"],
+          [test -n "$with_libevent"],
+          [pkg_config_core="${with_libevent}/lib/pkgconfig/libevent_core.pc"
+           pkg_config_pthreads="${with_libevent}/lib/pkgconfig/libevent_pthreads.pc"
+           PKG_CONFIG_PATH="${with_libevent}/lib/pkgconfig:${PKG_CONFIG_PATH}"],
+          [pkg_config_core="libevent_core"
+           pkg_config_pthreads="libevent_pthreads"])
+
+    pkg_config_happy=1
+    OPAL_GET_LDFLAGS_FROM_PC([$pkg_config_core], [pkg_config_core_ldflags], [pkg_config_happy=0])
+    OPAL_GET_LDFLAGS_FROM_PC([$pkg_config_pthreads], [pkg_config_pthreads_ldflags], [pkg_config_happy=0])
+    OPAL_GET_LIBS_FROM_PC([$pkg_config_core], [pkg_config_core_libs], [pkg_config_happy=0])
+    OPAL_GET_LIBS_FROM_PC([$pkg_config_pthreads], [pkg_config_pthreads_libs], [pkg_config_happy=0])
+
+    AS_IF([test $pkg_config_happy -ne 0],
+          [# Strip -levent from pkg_config_pthreads_libs, since we
+           # only want to link against libevent_core.  We'll pick up
+           # the core library from pkg_config_core_libs.
+           pkg_config_pthreads_libs=`echo $pkg_config_pthreads_libs | sed "s/\\-levent\b//g"`
+           opal_libevent_WRAPPER_LDFLAGS="$pkg_config_core_ldflags"
+           OPAL_FLAGS_APPEND_UNIQ([opal_libevent_WRAPPER_LDFLAGS], [$pkg_config_pthreads_ldflags])
+           opal_libevent_WRAPPER_LIBS="$pkg_config_pthreads_libs"
+           OPAL_FLAGS_APPEND_UNIQ([opal_libevent_WRAPPER_LIBS], [$pkg_config_core_libs])],
+          [# guess that what we have from compiling OMPI is good enough
+           opal_libevent_WRAPPER_LDFLAGS="$opal_libevent_LDFLAGS"
+           opal_libevent_WRAPPER_LIBS="$opal_libevent_LIBS"])
+
+    OPAL_WRAPPER_FLAGS_ADD([LDFLAGS], [$opal_libevent_WRAPPER_LDFLAGS])
+    OPAL_WRAPPER_FLAGS_ADD([LIBS], [$opal_libevent_WRAPPER_LIBS])
 
     # this will work even if there is no libevent package included,
     # because libevent_tarball and libevent_directory will evaluate to
diff --git a/config/opal_config_pmix.m4 b/config/opal_config_pmix.m4
@@ -55,6 +55,12 @@ dnl         uses PMIx.  Cannot be added to LIBS yet, because then
 dnl         other execution tests later in configure (there are sadly
 dnl         some) would fail if the path in LDFLAGS was not added to
 dnl         LD_LIBRARY_PATH.
+dnl   * opal_pmix_WRAPPER_LDFLAGS - the linker flags necessary to
+dnl         add to the wrapper compilers in order to link an opal
+dnl         application when opal is built as a static library.
+dnl   * opal_pmix_WRAPPER_LIBS - the linker flags necessary to
+dnl         add to the wrapper compilers in order to link an opal
+dnl         application when opal is built as a static library.
 dnl   * CPPFLAGS, LDFLAGS - Updated opal_pmix_CPPFLAGS and
 dnl         opal_pmix_LDFLAGS.
 AC_DEFUN([OPAL_CONFIG_PMIX], [
@@ -142,13 +148,27 @@ AC_DEFUN([OPAL_CONFIG_PMIX], [
     AS_IF([test "$external_pmix_happy" = "0" -a "$internal_pmix_happy" = "0"],
           [AC_MSG_ERROR([Could not find viable pmix build.])])
 
-   AS_IF([test "$external_pmix_happy" = "1"],
-         [AS_IF([test -n "$with_pmix"],
-               [OPAL_GET_LFLAGS_FROM_PC(pmix, $with_pmix/lib/pkgconfig, $libevent_core_OMPI_PC_DIR, $hwloc_OMPI_PC_DIR)],
-               [OPAL_GET_LFLAGS_FROM_PC(pmix, "", $libevent_core_OMPI_PC_DIR, $hwloc_OMPI_PC_DIR)]
-         )],
-        [OPAL_GET_LFLAGS_FROM_PC(pmix, $OMPI_TOP_SRCDIR/3rd-party/openpmix/maint, $libevent_core_OMPI_PC_DIR, $hwloc_OMPI_PC_DIR)]
-    )
+    AS_IF([test "$opal_pmix_mode" = "internal"],
+          [pkg_config_file="${OMPI_TOP_BUILDDIR}/3rd-party/openpmix/maint/pmix.pc"
+           PKG_CONFIG_PATH="${OMPI_TOP_BUILDDIR}/3rd-party/openpmix/maint:${PKG_CONFIG_PATH}"],
+          [test -n "$with_hwloc"],
+          [pkg_config_file="${with_pmix}/lib/pkgconfig/pmix.pc"
+           PKG_CONFIG_PATH="${with_pmix}/lib/pkgconfig:${PKG_CONFIG_PATH}"],
+          [pkg_config_file="pmix"])
+
+    pkg_config_happy=1
+    OPAL_GET_LDFLAGS_FROM_PC([$pkg_config_file], [pkg_config_ldflags], [pkg_config_happy=0])
+    OPAL_GET_LIBS_FROM_PC([$pkg_config_file], [pkg_config_libs], [pkg_config_happy=0])
+
+    AS_IF([test $pkg_config_happy -ne 0],
+          [opal_pmix_WRAPPER_LDFLAGS="$pkg_config_ldflags"
+           opal_pmix_WRAPPER_LIBS="$pkg_config_libs"],
+          [# guess that what we have from compiling OMPI is good enough
+           opal_pmix_WRAPPER_LDFLAGS="$opal_hwloc_LDFLAGS"
+           opal_pmix_WRAPPER_LIBS="$opal_hwloc_LIBS"])
+
+    OPAL_WRAPPER_FLAGS_ADD([LDFLAGS], [$opal_pmix_WRAPPER_LDFLAGS])
+    OPAL_WRAPPER_FLAGS_ADD([LIBS], [$opal_pmix_WRAPPER_LIBS])
 
     AC_DEFINE_UNQUOTED([OPAL_USING_INTERNAL_PMIX],
                        [$OPAL_USING_INTERNAL_PMIX],
