Skip to content

Commit

Permalink
Stealing some pieces of Josh Hursey's PR #1583 and modifying a bit, a…
Browse files Browse the repository at this point in the history
…llow the opal/pmix external component to handle both PMIx 1.1.4 and PMIx 2.0 versions. Automatically detect the version of the target external library and adjust the only two APIs that changed (PMIx_Init and PMIx_Finalize)

Rename temp vars in .m4 to avoid conflict with Travis
  • Loading branch information
Ralph Castain committed May 27, 2016
1 parent d25b846 commit 55923ea
Show file tree
Hide file tree
Showing 6 changed files with 176 additions and 22 deletions.
90 changes: 73 additions & 17 deletions config/opal_check_pmi.m4
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@
# Copyright (c) 2009-2015 Cisco Systems, Inc. All rights reserved.
# Copyright (c) 2011-2014 Los Alamos National Security, LLC. All rights
# reserved.
# Copyright (c) 2014 Intel, Inc. All rights reserved.
# Copyright (c) 2014-2016 Intel, Inc. All rights reserved.
# Copyright (c) 2014-2016 Research Organization for Information Science
# and Technology (RIST). All rights reserved.
# Copyright (c) 2016 IBM Corporation. All rights reserved.
# $COPYRIGHT$
#
# Additional copyrights may follow
Expand Down Expand Up @@ -155,16 +156,16 @@ AC_DEFUN([OPAL_CHECK_PMI],[
default_pmi_loc=yes])
AS_IF([test ! -z "$with_pmi_libdir"],
[check_pmi_lib_dir=$with_pmi_libdir
default_pmi_libloc=no],
default_pmi_libloc=no],
[check_pmi_lib_dir=$check_pmi_install_dir
AS_IF([test "$default_pmi_loc" = "no"],
[default_pmi_libloc=no],
[default_pmi_libloc=yes])])
AS_IF([test "$default_pmi_loc" = "no"],
[default_pmi_libloc=no],
[default_pmi_libloc=yes])])
# check for pmi-1 lib */
slurm_pmi_found=no
OPAL_CHECK_PMI_LIB([$check_pmi_install_dir],
[$check_pmi_lib_dir],
[$check_pmi_lib_dir],
[pmi], [PMI_Init],
[slurm_pmi_found=yes],
[opal_enable_pmi1=yes
Expand All @@ -174,10 +175,10 @@ AC_DEFUN([OPAL_CHECK_PMI],[
AS_IF([test "$opal_enable_pmi1" = "yes"],
[AS_IF([test "$default_pmi_loc" = "no" || test "$slurm_pmi_found" = "yes"],
[opal_pmi1_CPPFLAGS="$pmi_CPPFLAGS"
[opal_pmi1_CPPFLAGS="$pmi_CPPFLAGS"
AC_SUBST(opal_pmi1_CPPFLAGS)])
AS_IF([test "$default_pmi_libloc" = "no" || test "$slurm_pmi_found" = "yes"],
[opal_pmi1_LDFLAGS="$pmi_LDFLAGS"
[opal_pmi1_LDFLAGS="$pmi_LDFLAGS"
AC_SUBST(opal_pmi1_LDFLAGS)
opal_pmi1_rpath="$pmi_rpath"
AC_SUBST(opal_pmi1_rpath)])])
Expand All @@ -195,25 +196,25 @@ AC_DEFUN([OPAL_CHECK_PMI],[
AS_IF([test "$opal_enable_pmi2" = "yes"],
[AS_IF([test "$default_pmi_loc" = "no" || test "$slurm_pmi_found" = "yes"],
[opal_pmi2_CPPFLAGS="$pmi2_CPPFLAGS"
[opal_pmi2_CPPFLAGS="$pmi2_CPPFLAGS"
AC_SUBST(opal_pmi2_CPPFLAGS)])
AS_IF([test "$default_pmi_libloc" = "no" || test "$slurm_pmi_found" = "yes"],
[opal_pmi2_LDFLAGS="$pmi2_LDFLAGS"
[opal_pmi2_LDFLAGS="$pmi2_LDFLAGS"
AC_SUBST(opal_pmi2_LDFLAGS)
opal_pmi2_rpath="$pmi2_rpath"
AC_SUBST(opal_pmi2_rpath)])])
# since support was explicitly requested, then we should error out
# if we didn't find the required support
AC_MSG_CHECKING([can PMI support be built])
AC_MSG_CHECKING([can PMI support be built])
AS_IF([test "$opal_enable_pmi1" != "yes" && test "$opal_enable_pmi2" != "yes"],
[AC_MSG_RESULT([no])
AC_MSG_WARN([PMI support requested (via --with-pmi) but neither pmi.h])
AC_MSG_WARN([nor pmi2.h were found under locations:])
AC_MSG_WARN([nor pmi2.h were found under locations:])
AC_MSG_WARN([ $check_pmi_install_dir])
AC_MSG_WARN([ $check_pmi_install_dir/slurm])
AC_MSG_WARN([Specified path: $with_pmi])
AC_MSG_WARN([OR neither libpmi nor libpmi2 were found under:])
AC_MSG_WARN([OR neither libpmi nor libpmi2 were found under:])
AC_MSG_WARN([ $check_pmi_lib_dir/lib])
AC_MSG_WARN([ $check_pmi_lib_dir/lib64])
AC_MSG_WARN([Specified path: $with_pmi_libdir])
Expand All @@ -230,7 +231,7 @@ AC_DEFUN([OPAL_CHECK_PMIX],[
opal_pmix_ext_LDFLAGS=
opal_pmix_ext_LIBS=
OPAL_VAR_SCOPE_PUSH([pmix_ext_install_dir])
OPAL_VAR_SCOPE_PUSH([pmix_ext_install_dir opal_pmix_CPPFLAGS_save opal_pmix_LDFLAGS_save opal_pmix_LIBS_save opal_pmix_LD_LIBRARY_PATH_save])
AC_ARG_WITH([pmix],
[AC_HELP_STRING([--with-pmix(=DIR)],
Expand All @@ -251,21 +252,76 @@ AC_DEFUN([OPAL_CHECK_PMIX],[
AS_IF([test "$with_pmix" = "external"],
[pmix_ext_install_dir=/usr],
[pmix_ext_install_dir=$with_pmix])
# Make sure we have the headers and libs in the correct location
OPAL_CHECK_WITHDIR([external-pmix], [$pmix_ext_install_dir/include], [pmix.h])
OPAL_CHECK_WITHDIR([external-libpmix], [$pmix_ext_install_dir/lib], [libpmix.*])
AC_MSG_CHECKING([if external component can be used])
OPAL_CHECK_PACKAGE([opal_pmix_ext],
[pmix.h],
[pmix],
[PMIx_Init],
[],
[$pmix_ext_install_dir],
[],
[$pmix_ext_install_dir/lib],
[AC_MSG_RESULT([PMIx external support will be built])
opal_external_pmix_happy=yes],
[AC_MSG_RESULT([no])
[opal_external_pmix_happy="no"
AC_MSG_RESULT([no])
AC_MSG_WARN([External PMIx support was requested but failed])
AC_MSG_WARN([as explained above.])
AC_MSG_ERROR([Cannot continue])])
])
# Check the version
opal_external_pmix_version="unknown"
opal_pmix_CPPFLAGS_save=$CPPFLAGS
opal_pmix_LDFLAGS_save=$LDFLAGS
opal_pmix_LIBS_save=$LIBS
LD_LIBRARY_PATH_orig=$opal_pmix_LD_LIBRARY_PATH_save
CPPFLAGS=$opal_pmix_ext_CPPFLAGS
LDFLAGS=$opal_pmix_ext_LDFLAGS
LIBS=$opal_pmix_ext_LIBS
LD_LIBRARY_PATH=$pmix_ext_install_dir/lib
AC_MSG_CHECKING([PMIx library version])
AC_TRY_RUN([
#include <stdio.h>
#include <stdlib.h>
#include <pmix.h>
#include <pmix/pmix_common.h>
int main(int argc, char **argv)
{
const char * version = NULL;
FILE *f = NULL;
f = fopen("conftestval", "w");
if( !f ) exit(1);
version = PMIx_Get_version();
fprintf(f, "%s", version);
fclose(f);
return 0;
}
], [
eval opal_external_pmix_version=`cat conftestval`
AC_MSG_RESULT([$opal_external_pmix_version])
], [
LD_LIBRARY_PATH=$opal_pmix_LD_LIBRARY_PATH_save
opal_external_pmix_happy="no"
AC_MSG_ERROR([External PMIx support requested but could not build/run a test program. Aborting])
], [
LD_LIBRARY_PATH=$opal_pmix_LD_LIBRARY_PATH_save
opal_external_pmix_happy="no"
AC_MSG_ERROR([External PMIx disabled for cross compile. Aborting])
])
CPPFLAGS=$opal_pmix_CPPFLAGS_save
LDFLAGS=$opal_pmix_LDFLAGS_save
LIBS=$opal_pmix_LIBS_save
LD_LIBRARY_PATH=$opal_pmix_LD_LIBRARY_PATH_save
opal_external_pmix_happy="yes"
])
AC_SUBST(opal_pmix_ext_CPPFLAGS)
AC_SUBST(opal_pmix_ext_LDFLAGS)
AC_SUBST(opal_pmix_ext_LIBS)
Expand Down
79 changes: 79 additions & 0 deletions config/opal_check_version.m4
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
dnl -*- shell-script -*-
dnl
dnl Copyright (c) 2016 IBM Corporation. All rights reserved.
dnl $COPYRIGHT$
dnl
dnl Additional copyrights may follow
dnl
dnl $HEADER$
dnl

# Parameters: (use a version "1.1.4rc2" as the example)
# * prefix
# Will export a variable $prefix_version_cmp
# - action_if_less => "less"
# - action_if_equal => "equal"
# - action_if_equal_series => "series"
# - action_if_greater => "greater"
# * version_actual
# Actual version string
# * version_desired
# Desired version string to check against
# * action_if_less
# Action to take if the version is strictly less than
# "1.1.3" < "1.1.4rc2"
# * action_if_equal
# Action to take if the version matches exactly
# "1.1.4rc2" = "1.1.4rc2"
# * action_if_equal_series
# Action to take if the version matches to this series
# "1.1.4rc1" ~=~ "1.1.4rc2"
# "1.1.4" ~=~ "1.1.4rc2"
# * action_if_greater
# Action to take if the version is strictly greater than
# "1.1.5" > "1.1.4rc2"
# "2.0" > "1.1.4rc2"
#
# See documentation on m4_version_compare and AS_VERSION_COMPARE for more
# precise definitions
# OPAL_CHECK_VERSION(prefix, version_actual, version_desired,
# action_if_less, action_if_equal, action_if_equal_series,
# action_if_greater)
# ----------------------------------------------------
AC_DEFUN([OPAL_CHECK_VERSION],[
version_actual=$2
version_desired=$3

AC_MSG_CHECKING([Checking library version is $version_desired])
#
# Example: If version_desired=1.1.4 and
# version_actual=1.1.3 -> -1
# version_actual=1.1.4 -> 0
# version_actual=1.1.4rc1 -> 1
# version_actual=1.1.5 -> 1 (need further check)
#
AS_VERSION_COMPARE(["$version_actual"], [$version_desired],
[AC_MSG_RESULT([Earlier than expected ($version_actual < $$version_desired)])
$1_version_cmp="less"
$4],
[AC_MSG_RESULT([Equal])
$1_version_cmp="equal"
$5],
[
# Need further check to make sure we are < 1.1.5
# version_actual=1.1.4rc1 -> -1
# version_actual=1.1.4 -> 0 (caught above)
# version_actual=1.1.5 -> 1
AS_VERSION_COMPARE(["$version_actual"], [$version_desired"zzzz"],
[AC_MSG_RESULT([Within release series ($version_actual)])
$1_version_cmp="series"
$6],
[AC_MSG_RESULT([Within release series ($version_actual)])
$1_version_cmp="series"
$6],
[AC_MSG_RESULT([Later than expected ($version_actual > $$version_desired)])
$1_version_cmp="greater"
$7]
)]
)
])dnl
4 changes: 1 addition & 3 deletions opal/mca/pmix/external/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ mca_pmix_external_la_SOURCES = $(sources)
mca_pmix_external_la_CFLAGS =
mca_pmix_external_la_CPPFLAGS = $(opal_pmix_ext_CPPFLAGS)
mca_pmix_external_la_LDFLAGS = -module -avoid-version $(opal_pmix_ext_LDFLAGS)
mca_pmix_external_la_LIBADD = $(opal_pmix_ext_LIBS) \
$(OPAL_TOP_BUILDDIR)/opal/mca/event/lib@OPAL_LIB_PREFIX@mca_event.la \
$(OPAL_TOP_BUILDDIR)/opal/mca/hwloc/lib@OPAL_LIB_PREFIX@mca_hwloc.la
mca_pmix_external_la_LIBADD = $(opal_pmix_ext_LIBS)

noinst_LTLIBRARIES = $(component_noinst)
libmca_pmix_external_la_SOURCES =$(sources)
Expand Down
12 changes: 12 additions & 0 deletions opal/mca/pmix/external/configure.m4
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ AC_DEFUN([MCA_opal_pmix_external_CONFIG],[
AC_MSG_WARN([TO BUILD PMIX OR ELSE UNPREDICTABLE BEHAVIOR MAY RESULT])
AC_MSG_ERROR([PLEASE CORRECT THE CONFIGURE COMMAND LINE AND REBUILD])])
external_WRAPPER_EXTRA_CPPFLAGS='-I${includedir}/openmpi/$opal_pmix_external_basedir/pmix -I${includedir}/openmpi/$opal_pmix_external_basedir/pmix/include'
# check the version
AC_MSG_CHECKING([pmix version])
OPAL_CHECK_VERSION([opal_pmix_external],
[$opal_external_pmix_version],
["2.0"],
[opal_external_pmix_version_flag=1.1],
[opal_external_pmix_version_flag=2.0],
[opal_external_pmix_version_flag=2.0],
[opal_external_pmix_version_flag=2.0])
AC_MSG_RESULT([$opal_external_pmix_version])
AS_IF([test "$opal_external_pmix_version_flag" = "1.1"],
[AC_DEFINE([OPAL_PMIX_VERSION_11], [1], [PMIx external version])])
$1],
[$2])
])dnl
9 changes: 9 additions & 0 deletions opal/mca/pmix/external/pmix_ext_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,12 @@ int pmix1_client_init(void)
asprintf(&dbgvalue, "PMIX_DEBUG=%d", dbg);
putenv(dbgvalue);
}

#ifdef OPAL_PMIX_VERSION_11
rc = PMIx_Init(&my_proc);
#else
rc = PMIx_Init(&my_proc, NULL, 0);
#endif
if (PMIX_SUCCESS != rc) {
return pmix1_convert_rc(rc);
}
Expand Down Expand Up @@ -154,7 +159,11 @@ int pmix1_client_finalize(void)
/* deregister the errhandler */
PMIx_Deregister_errhandler(errhdler_ref, NULL, NULL);

#ifdef OPAL_PMIX_VERSION_11
rc = PMIx_Finalize();
#else
rc = PMIx_Finalize(NULL, 0);
#endif

return pmix1_convert_rc(rc);
}
Expand Down
4 changes: 2 additions & 2 deletions opal/mca/pmix/external/pmix_ext_server_south.c
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* -*- Mode: C; c-basic-offset:4 ; indent-tabs-mode:nil -*- */
/*
* Copyright (c) 2014-2015 Intel, Inc. All rights reserved.
* Copyright (c) 2014-2016 Intel, Inc. All rights reserved.
* Copyright (c) 2014-2016 Research Organization for Information Science
* and Technology (RIST). All rights reserved.
* Copyright (c) 2014-2015 Intel, Inc. All rights reserved.
Expand Down Expand Up @@ -231,7 +231,7 @@ int pmix1_server_register_nspace(opal_jobid_t jobid,
pmapinfo = (opal_list_t*)kv->data.ptr;
szmap = opal_list_get_size(pmapinfo);
PMIX_INFO_CREATE(pmap, szmap);
pinfo[n].value.data.array.array = (struct pmix_info*)pmap;
pinfo[n].value.data.array.array = (struct pmix_info_t*)pmap;
pinfo[n].value.data.array.size = szmap;
m = 0;
OPAL_LIST_FOREACH(k2, pmapinfo, opal_value_t) {
Expand Down

3 comments on commit 55923ea

@ggouaillardet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jjhursey @rhc54 @jsquyres
i got some error because of this commit.
the root cause is external pmix is not in my LD_LIBRARY_PATH in my environment

the inline patch

  • prepends pmix libdir to LD_LIBRARY_PATH (vs overwrite)
  • export LD_LIBRARY_PATH
  • uses AC_RUN_IFELSE instead of deprecated AC_TRY_RUN

it might be easier/better to check pmix version by compiling some test programs (with #if and #error) instead of building one and running it. no more need to set/restore LD_LIBRARY_PATH and no more error when cross compiling.
bottom line, i did not commit that patch.

any thoughts ?

diff --git a/config/opal_check_pmi.m4 b/config/opal_check_pmi.m4
index 57850a1..92407af 100644
--- a/config/opal_check_pmi.m4
+++ b/config/opal_check_pmi.m4
@@ -280,10 +280,12 @@ AC_DEFUN([OPAL_CHECK_PMIX],[
            CPPFLAGS=$opal_pmix_ext_CPPFLAGS
            LDFLAGS=$opal_pmix_ext_LDFLAGS
            LIBS=$opal_pmix_ext_LIBS
-           LD_LIBRARY_PATH=$pmix_ext_install_dir/lib
+           LD_LIBRARY_PATH=$pmix_ext_install_dir/lib:$LD_LIBRARY_PATH
+           export LD_LIBRARY_PATH

           AC_MSG_CHECKING([PMIx library version])
-          AC_TRY_RUN([
+          AC_RUN_IFELSE([
+              AC_LANG_SOURCE([
 #include <stdio.h>
 #include <stdlib.h>
 #include <pmix.h>
@@ -302,15 +304,17 @@ int main(int argc, char **argv)

     return 0;
 }
-             ], [
+             ])], [
                  eval opal_external_pmix_version=`cat conftestval`
                  AC_MSG_RESULT([$opal_external_pmix_version])
              ], [
                  LD_LIBRARY_PATH=$opal_pmix_LD_LIBRARY_PATH_save
+                 export LD_LIBRARY_PATH
                  opal_external_pmix_happy="no"
                  AC_MSG_ERROR([External PMIx support requested but could not build/run a test program. Aborting])
              ], [
                  LD_LIBRARY_PATH=$opal_pmix_LD_LIBRARY_PATH_save
+                 export LD_LIBRARY_PATH
                  opal_external_pmix_happy="no"
                  AC_MSG_ERROR([External PMIx disabled for cross compile. Aborting])
          ])

@rhc54
Copy link
Contributor

@rhc54 rhc54 commented on 55923ea May 30, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went ahead and created a new PR #1730 with this patch. There are two alternative approaches we have considered:

  • detect the PMIX_VERSION string itself as each version library is tagged with it. I haven't quite figured out how to do this in autoconf yet, but am convinced it is doable.
  • do a typical check_lib once 2.0 has a distinct function. This is expected to happen soon, at which point we will need a distinct external component for PMIx 2.0 vs 1.1.4 anyway

So hopefully this will work for now, and we'll go with one of the above fairly soon.

@ggouaillardet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here is a patch that uses PMIX_VERSION
only PMIx include files are required, and there is no need to link with PMIx libs.
that being said, a program has to run, and hence, it still does not work when cross compiling.

diff --git a/config/opal_check_pmi.m4 b/config/opal_check_pmi.m4
index 92407af..2319b10 100644
--- a/config/opal_check_pmi.m4
+++ b/config/opal_check_pmi.m4
@@ -275,21 +275,16 @@ AC_DEFUN([OPAL_CHECK_PMIX],[
            opal_pmix_CPPFLAGS_save=$CPPFLAGS
            opal_pmix_LDFLAGS_save=$LDFLAGS
            opal_pmix_LIBS_save=$LIBS
-           LD_LIBRARY_PATH_orig=$opal_pmix_LD_LIBRARY_PATH_save

            CPPFLAGS=$opal_pmix_ext_CPPFLAGS
-           LDFLAGS=$opal_pmix_ext_LDFLAGS
-           LIBS=$opal_pmix_ext_LIBS
-           LD_LIBRARY_PATH=$pmix_ext_install_dir/lib:$LD_LIBRARY_PATH
-           export LD_LIBRARY_PATH

           AC_MSG_CHECKING([PMIx library version])
           AC_RUN_IFELSE([
               AC_LANG_SOURCE([
 #include <stdio.h>
 #include <stdlib.h>
-#include <pmix.h>
-#include <pmix/pmix_common.h>
+
+#include <pmix/include/private/autogen/config.h>

 int main(int argc, char **argv)
 {
@@ -298,8 +293,7 @@ int main(int argc, char **argv)

     f = fopen("conftestval", "w");
     if( !f ) exit(1);
-    version = PMIx_Get_version();
-    fprintf(f, "%s", version);
+    fprintf(f, PMIX_VERSION);
     fclose(f);

     return 0;
@@ -308,20 +302,13 @@ int main(int argc, char **argv)
                  eval opal_external_pmix_version=`cat conftestval`
                  AC_MSG_RESULT([$opal_external_pmix_version])
              ], [
-                 LD_LIBRARY_PATH=$opal_pmix_LD_LIBRARY_PATH_save
-                 export LD_LIBRARY_PATH
                  opal_external_pmix_happy="no"
                  AC_MSG_ERROR([External PMIx support requested but could not build/run a test program. Aborting])
              ], [
-                 LD_LIBRARY_PATH=$opal_pmix_LD_LIBRARY_PATH_save
-                 export LD_LIBRARY_PATH
                  opal_external_pmix_happy="no"
                  AC_MSG_ERROR([External PMIx disabled for cross compile. Aborting])
          ])
          CPPFLAGS=$opal_pmix_CPPFLAGS_save
-         LDFLAGS=$opal_pmix_LDFLAGS_save
-         LIBS=$opal_pmix_LIBS_save
-         LD_LIBRARY_PATH=$opal_pmix_LD_LIBRARY_PATH_save

          opal_external_pmix_happy="yes"
     ])

Please sign in to comment.