Improve handling of libs in wrapper compilers

bwbarrett · bwbarrett · commit 3edf9f1799e2 · 2021-11-20T17:08:41.000Z
Improve the logic of handling library (-l) references in the
wrapper compilers.  Previously, a duplicate in one of the LIB
strings would leave the left-most reference, which can lead to
missing symbols if a more right library also depends on the
duplicated library.  Instead, keep only the right-most reference.

Add a helper macro for list appending to avoid duplicating the
same 3 lines of shell code multiple times in opal_functions.m4

Signed-off-by: Brian Barrett &lt;bbarrett@amazon.com&gt;
diff --git a/config/opal_config_libevent.m4 b/config/opal_config_libevent.m4
@@ -101,7 +101,7 @@ AC_DEFUN([OPAL_CONFIG_LIBEVENT], [
            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])],
+           OPAL_FLAGS_APPEND_MOVE([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"])
diff --git a/config/opal_functions.m4 b/config/opal_functions.m4
@@ -16,6 +16,8 @@ dnl Copyright (c) 2009-2020 Cisco Systems, Inc.  All rights reserved.
 dnl Copyright (c) 2014      Intel, Inc. All rights reserved.
 dnl Copyright (c) 2015-2017 Research Organization for Information Science
 dnl                         and Technology (RIST). All rights reserved.
+dnl Copyright (c) 2021      Amazon.com, Inc. or its affiliates.  All Rights
+dnl                         reserved.
 dnl
 dnl $COPYRIGHT$
 dnl
@@ -317,6 +319,18 @@ dnl #######################################################################
 dnl #######################################################################
 dnl #######################################################################
 
+# OPAL_APPEND(variable, new_argument)
+# ----------------------------------------
+# Append new_argument to variable, assuming a space separated list.
+#
+AC_DEFUN([OPAL_APPEND], [
+  AS_IF([test -z "$$1"], [$1="$2"], [$1="$$1 $2"])
+])
+
+dnl #######################################################################
+dnl #######################################################################
+dnl #######################################################################
+
 # OPAL_APPEND_UNIQ(variable, new_argument)
 # ----------------------------------------
 # Append new_argument to variable if not already in variable.  This assumes a
@@ -333,11 +347,7 @@ for arg in $2; do
         fi
     done
     if test "$opal_found" = "0" ; then
-        if test -z "$$1"; then
-            $1="$arg"
-        else
-            $1="$$1 $arg"
-        fi
+        OPAL_APPEND([$1], [$arg])
     fi
 done
 unset opal_found
@@ -463,7 +473,50 @@ AC_DEFUN([OPAL_FLAGS_APPEND_UNIQ], [
                    AS_IF([test "x$val" = "x$arg"], [opal_append=0])
                done])
         AS_IF([test "$opal_append" = "1"],
-              [AS_IF([test -z "$$1"], [$1=$arg], [$1="$$1 $arg"])])
+              [OPAL_APPEND([$1], [$arg])])
+    done
+
+    OPAL_VAR_SCOPE_POP
+])
+
+dnl #######################################################################
+dnl #######################################################################
+dnl #######################################################################
+
+# OPAL_FLAGS_APPEND_MOVE(variable, new_argument)
+# ----------------------------------------------
+# add new_arguments to the end of variable.
+#
+# If an argument in new_arguments does not begin with -I, -L, or -l OR
+# the argument begins with -I, -L, or -l and it is not already in
+# variable, it is appended to variable.
+#
+# If an argument in new_argument begins with a -l and is already in
+# variable, the existing occurances of the argument are removed from
+# variable and the argument is appended to variable.  This behavior
+# is most useful in LIBS, where ordering matters and being rightmost
+# is usually the right behavior.
+#
+# This macro assumes a space separated list.
+AC_DEFUN([OPAL_FLAGS_APPEND_MOVE], [
+    OPAL_VAR_SCOPE_PUSH([opal_tmp_variable opal_tmp opal_append])
+
+    for arg in $2; do
+        AS_CASE([$arg],
+                [-I*|-L*],
+                [opal_append=1
+                 for val in ${$1} ; do
+                     AS_IF([test "x$val" = "x$arg"], [opal_append=0])
+                 done
+                 AS_IF([test $opal_append -eq 1], [OPAL_APPEND([$1], [$arg])])],
+                [-l*],
+                [opal_tmp_variable=
+                 for val in ${$1}; do
+                     AS_IF([test "x$val" != "x$arg"],
+                           [OPAL_APPEND([opal_tmp_variable], [$val])])
+                 done
+                 OPAL_APPEND([opal_tmp_variable], [$arg])])
+                 $1="$opal_tmp_variable"
     done
 
     OPAL_VAR_SCOPE_POP
diff --git a/config/opal_mca.m4 b/config/opal_mca.m4
@@ -850,7 +850,7 @@ AC_MSG_ERROR([*** $2 component $3 was supposed to be direct-called, but
               [AS_VAR_COPY([tmp_flags], [$2_$3_WRAPPER_EXTRA_LIBS])],
               [AS_VAR_COPY([tmp_all_flags], [$2_$3_LIBS])
                OPAL_MCA_STRIP_LAFILES([tmp_flags], [$tmp_all_flags])])
-           OPAL_FLAGS_APPEND_UNIQ([mca_wrapper_extra_libs], [$tmp_flags])])
+           OPAL_FLAGS_APPEND_MOVE([mca_wrapper_extra_libs], [$tmp_flags])])
 
     # WRAPPER_EXTRA_CPPFLAGS are only needed for STOP_AT_FIRST
     # components, as all other components are not allowed to leak
diff --git a/config/opal_setup_wrappers.m4 b/config/opal_setup_wrappers.m4
@@ -43,7 +43,7 @@ AC_DEFUN([OPAL_WRAPPER_FLAGS_ADD], [
           [$1], [CXXFLAGS], [OPAL_FLAGS_APPEND_UNIQ([wrapper_extra_cxxflags], [$2])],
           [$1], [FCFLAGS], [OPAL_FLAGS_APPEND_UNIQ([wrapper_extra_fcflags], [$2])],
           [$1], [LDFLAGS], [OPAL_FLAGS_APPEND_UNIQ([wrapper_extra_ldflags], [$2])],
-          [$1], [LIBS], [OPAL_FLAGS_APPEND_UNIQ([wrapper_extra_libs], [$2])],
+          [$1], [LIBS], [OPAL_FLAGS_APPEND_MOVE([wrapper_extra_libs], [$2])],
           [m4_fatal([Unknown wrapper flag type $1])])
 ])
 
@@ -386,9 +386,9 @@ AC_DEFUN([OPAL_SETUP_WRAPPER_FINAL],[
        # asked for, as they know better than us.
        AC_MSG_CHECKING([for OPAL LIBS])
        OPAL_WRAPPER_EXTRA_LIBS="$opal_mca_wrapper_extra_libs"
-       OPAL_FLAGS_APPEND_UNIQ([OPAL_WRAPPER_EXTRA_LIBS], [$wrapper_extra_libs])
-       OPAL_WRAPPER_EXTRA_LIBS="$OPAL_WRAPPER_EXTRA_LIBS $with_wrapper_libs"
-       OPAL_FLAGS_APPEND_UNIQ([OMPI_WRAPPER_EXTRA_LIBS], [$LIBS])
+       OPAL_FLAGS_APPEND_MOVE([OPAL_WRAPPER_EXTRA_LIBS], [$wrapper_extra_libs])
+       OPAL_FLAGS_APPEND_MOVE([OPAL_WRAPPER_EXTRA_LIBS], [$with_wrapper_libs])
+       OPAL_FLAGS_APPEND_MOVE([OMPI_WRAPPER_EXTRA_LIBS], [$LIBS])
        AC_SUBST([OPAL_WRAPPER_EXTRA_LIBS])
        AC_MSG_RESULT([$OPAL_WRAPPER_EXTRA_LIBS])
     ])
@@ -451,9 +451,9 @@ AC_DEFUN([OPAL_SETUP_WRAPPER_FINAL],[
 
        AC_MSG_CHECKING([for OMPI LIBS])
        OMPI_WRAPPER_EXTRA_LIBS="$ompi_mca_wrapper_extra_libs"
-       OPAL_FLAGS_APPEND_UNIQ([OMPI_WRAPPER_EXTRA_LIBS], [$wrapper_extra_libs])
-       OMPI_WRAPPER_EXTRA_LIBS="$OMPI_WRAPPER_EXTRA_LIBS $with_wrapper_libs"
-       OPAL_FLAGS_APPEND_UNIQ([OMPI_WRAPPER_EXTRA_LIBS], [$LIBS])
+       OPAL_FLAGS_APPEND_MOVE([OMPI_WRAPPER_EXTRA_LIBS], [$wrapper_extra_libs])
+       OPAL_FLAGS_APPEND_MOVE([OMPI_WRAPPER_EXTRA_LIBS], [$with_wrapper_libs])
+       OPAL_FLAGS_APPEND_MOVE([OMPI_WRAPPER_EXTRA_LIBS], [$LIBS])
        AC_SUBST([OMPI_WRAPPER_EXTRA_LIBS])
        AC_MSG_RESULT([$OMPI_WRAPPER_EXTRA_LIBS])
 
