Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

configury: fix configure --disable-mpi-cxx when no C++ compiler is av… #1860

Closed
wants to merge 1 commit into from
Closed

configury: fix configure --disable-mpi-cxx when no C++ compiler is av… #1860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion config/ompi_setup_cxx.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ AC_DEFUN([_OMPI_SETUP_CXX_COMPILER_BACKEND],[
fi

# Make sure we can link with the C compiler
if test "$ompi_cv_cxx_compiler_vendor" != "microsoft"; then
if test "$ompi_cv_cxx_compiler_vendor" != "microsoft" &&
test "$WANT_MPI_CXX_SUPPORT" -eq 1; then
OPAL_LANG_LINK_WITH_C([C++], [],
[cat <<EOF >&2
**********************************************************************
Expand Down
28 changes: 18 additions & 10 deletions config/opal_config_pthreads.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ AC_DEFUN([OPAL_INTL_POSIX_THREADS_PLAIN_CXX], [
#
# C++ compiler
#
if test "$opal_pthread_cxx_success" = "0"; then
if test "$opal_pthread_cxx_success" = "0" && \
test "$WANT_MPI_CXX_SUPPORT" -eq; then
Copy link
Member

Choose a reason for hiding this comment

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

Missing RHS 1.

AC_MSG_CHECKING([if C++ compiler and POSIX threads work as is])

AC_LANG_PUSH(C++)
Expand Down Expand Up @@ -251,9 +252,10 @@ AC_PROVIDE_IFELSE([AC_PROG_CC],
[OPAL_INTL_POSIX_THREADS_PLAIN_C],
[opal_pthread_c_success=1])

AC_PROVIDE_IFELSE([AC_PROG_CXX],
[OPAL_INTL_POSIX_THREADS_PLAIN_CXX],
[opal_pthread_cxx_success=1])
AS_IF([test "$WANT_MPI_CXX_SUPPORT" = 1],
Copy link
Member

Choose a reason for hiding this comment

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

You use -eq for $WANT_MPI_CXX_SUPPORT above -- should probably use it here, too. Then you don't need to use the quotes for `$WANT_MPI_CXX_SUPPORT, either (because it's guaranteed to be a number).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

on OS X with FreePGI, there is no c++ compiler, so configure falls back to g++, and pgcc does not work out of the box with g++, so pthread tests fail.
the purpose of the test is simply not to run tests that might fail and we should not care about.

makes sense ?

Copy link
Member

Choose a reason for hiding this comment

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

Right -- I understand what $WANT_MPI_CXX_SUPPORT is for, but I want to understand what AC_PROVIDE_IFELSE was supposed to be for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

my understanding is that this is here to support autotools that do not define/provide the AC_PROG_CXX macro.
As you already pointed, that might not be what we want.

Copy link
Member

Choose a reason for hiding this comment

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

I see AC_PROVIDE_IFELSE used in a few places throughout the tree, but aside from opal_config_pthreads.m4, all of them are 3rd-party locations (e.g., libtool, libevent, romio, etc.).

Do you want to get rid of the use of AC_PROVIDE_IFELSE here in this file, and instead, use the proper "if we have FOO support" OMPI shell variables? (e.g., $WANT_MPI_CXX_SUPPORT)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should the test be
Is there a working c++ compiler ?
AND
Do we want to build c++ bindings ?

Which raises the question, how do we test if there is a working c++ compiler ?
test -n $CXX ?
assuming AC_PROG_CXX was already invoked

Copy link
Member

Choose a reason for hiding this comment

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

I think $CXX will be set to no (or empty?) if there is no valid C++ compiler.

/me goes to test...

It looks like configure sets $CXX to g++ if it can't find a valid C++ compiler (!). But then if you try to run it, for example, it will fail (e.g., if it can't be found). 😞

The only place we have C++ in the code base is in the MPI C++ bindings (and examples), but those aren't even built by default any more.

I wonder if this whole C++ section of configure is in need of a bit of revamping / simplification...

[AC_PROVIDE_IFELSE([AC_PROG_CXX],
Copy link
Member

Choose a reason for hiding this comment

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

I actually can't find any documentation on AC_PROVIDE_IFELSE -- I see that we use it all throughout opal_config_pthreads.m4, but I can't quite figure out what it is for. My best guess is that it is "if AC_PROG_CXX has been substituted earlier, then substitute $1, otherwise substitute $2."

But if that's correct, it's been used entirely wrong in this m4 file (and likely should be removed).

Specifically: I'm trying to figure out why your new test on $WANT_MPI_CXX_SUPPORT is necessary. And if it is, is the AC_PROVIDE_IFELSE unnecessary (or incorrect)?

[OPAL_INTL_POSIX_THREADS_PLAIN_CXX],
[opal_pthread_cxx_success=1])])

AC_PROVIDE_IFELSE([AC_PROG_FC],
[OPAL_INTL_POSIX_THREADS_PLAIN_FC],
Expand Down Expand Up @@ -379,9 +381,10 @@ AC_PROVIDE_IFELSE([AC_PROG_CC],
[OPAL_INTL_POSIX_THREADS_SPECIAL_FLAGS_C],
[opal_pthread_c_success=1])

AC_PROVIDE_IFELSE([AC_PROG_CXX],
[OPAL_INTL_POSIX_THREADS_SPECIAL_FLAGS_CXX],
[opal_pthread_cxx_success=1])
AS_IF([test "$WANT_MPI_CXX_SUPPORT" = 1],
[AC_PROVIDE_IFELSE([AC_PROG_CXX],
[OPAL_INTL_POSIX_THREADS_SPECIAL_FLAGS_CXX],
[opal_pthread_cxx_success=1])])

AC_PROVIDE_IFELSE([AC_PROG_FC],
[OPAL_INTL_POSIX_THREADS_SPECIAL_FLAGS_FC],
Expand Down Expand Up @@ -568,9 +571,10 @@ AC_PROVIDE_IFELSE([AC_PROG_CC],
[OPAL_INTL_POSIX_THREADS_LIBS_C],
[opal_pthread_c_success=1])

AC_PROVIDE_IFELSE([AC_PROG_CXX],
[OPAL_INTL_POSIX_THREADS_LIBS_CXX],
[opal_pthread_cxx_success=1])
AS_IF([test "$WANT_MPI_CXX_SUPPORT" = 1],
[AC_PROVIDE_IFELSE([AC_PROG_CXX],
[OPAL_INTL_POSIX_THREADS_LIBS_CXX],
[opal_pthread_cxx_success=1])])

AC_PROVIDE_IFELSE([AC_PROG_FC],
[OPAL_INTL_POSIX_THREADS_LIBS_FC],
Expand Down Expand Up @@ -661,6 +665,10 @@ if test "$OMPI_TRY_FORTRAN_BINDINGS" = "$OMPI_FORTRAN_NO_BINDINGS" || \
opal_pthread_fortran_success=1
fi

if test "$WANT_MPI_CXX_SUPPORT" -ne 1; then
Copy link
Member

Choose a reason for hiding this comment

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

If you use the numeric test operators, you shouldn't need to use quotes on the variable under test (because it had better be guaranteed to be a number).

opal_pthread_cxx_success=1
fi

if test "$opal_pthread_c_success" = "1" && \
test "$opal_pthread_cxx_success" = "1" && \
test "$opal_pthread_fortran_success" = "1"; then
Expand Down
3 changes: 2 additions & 1 deletion config/opal_setup_cxx.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ AC_DEFUN([_OPAL_SETUP_CXX_COMPILER_BACKEND],[
fi

# Make sure we can link with the C compiler
if test "$opal_cv_cxx_compiler_vendor" != "microsoft"; then
if test "$opal_cv_cxx_compiler_vendor" != "microsoft" && \
test "$WANT_MPI_CXX_SUPPORT" -eq 1; then
OPAL_LANG_LINK_WITH_C([C++], [],
[cat <<EOF >&2
**********************************************************************
Expand Down