java: clean up MPI Java configury #5170

jsquyres · 2018-05-14T20:50:02Z

The Java configury is split into two parts:

Determine if we want MPI Java bindings.
Find the Java compiler (and related).

This commit does a few things:

Move the "Find the Java compiler" step from OPAL to OMPI (because
there is no Java in OPAL, and there doesn't appear to be any
immanent danger that there will be).
- As a direct consequence, remove the --enable-java CLI option
  (--enable-mpi-java still remains). Enabling the MPI Java bindings
  and enabling Java are now considered the same thing (since there
  is no Java elsewhere in the code base, the different was
  meaningless).
Only invoke the "Find the Java compiler" step if we actually want
the MPI Java bindings.
A few miscellaneous Java-related cleanups in configury (E.g., change
testing "$foo" == "1" to $foo -eq 1, etc.

This commit is mostly s/opal/ompi/gi in many places in configury and
shifting code around. But it looks bigger than it actually is because
of two reasons:

Some files were renamed:
- ompi_setup_java.m4 -> ompi_setup_mpi_java.m4 (setup MPI Java bindings)
- opal_setup_java.m4 -> ompi_setup_java.m4 (setup Java compiler)
Indenting level changed in (the new) ompi_setup_java.m4.

Signed-off-by: Jeff Squyres jsquyres@cisco.com

For the end user, this commit should effectively only be a cosmetic change.

No need for this to go to release branches -- this is just cleanup. It can just be slated for v4.0.0.

@RandomDSdevel FYI

ggouaillardet · 2018-05-15T00:50:30Z

config/ompi_setup_mpi_java.m4

+                       [do we want java mpi bindings])
+    AM_CONDITIONAL(OMPI_WANT_JAVA_BINDINGS, test "$WANT_MPI_JAVA_BINDINGS" = "1")
+
+   # Are we happy?


minor indentation issue

ggouaillardet · 2018-05-15T00:52:29Z

config/ompi_setup_java.m4

+    AS_IF([test "$1" = "yes"],
+          [_OMPI_SETUP_JAVA])
+
+    # JMS TO BE DELETED


just to be clear, this only applies to OMPI_HAVE_JAVA_SUPPORTBOGUS_TO_BE_DELETEDp.
I also noticed the initial OPAL_HAVE_HAVE_SUPPORT macro is unused, so as far as I am concerned, you can remove it now.

Good catch; I meant to delete that before creating the PR. Thanks.

ggouaillardet · 2018-05-15T00:55:34Z

config/ompi_setup_java.m4

+    AC_MSG_CHECKING([if Java support available])
+    AS_IF([test "$ompi_java_happy" = "no"],
+          [AC_MSG_RESULT([no])],
+          [AC_MSG_RESULT([yes])])


since this macro is only invoked when --enable-mpi-java is used, shouldn't you simply abort here ?
(or at least abort somewhere else at a higher level)

ggouaillardet · 2018-05-15T00:57:21Z

config/ompi_setup_mpi_java.m4

+# Do everything required to setup the Java MPI bindings.  Safe to AC_REQUIRE
+# this macro.
+AC_DEFUN([OMPI_SETUP_JAVA_BINDINGS],[
+    AC_REQUIRE([OMPI_SETUP_JAVA_BINDINGS_BANNER])


any reason why you AC_REQUIRE() instead of simply inlining the (one line) macro ?

I think this is a holdover -- there is a prior comment saying that it is required to get the title output in order. But I think that's from prior days when OMPI_SETUP_JAVA_BINDINGS itself was AC_REQUIRED. Doesn't seem like this is needed any more; I'll remove.

jsquyres · 2018-05-15T22:00:44Z

@ggouaillardet New commit pushed with fixes.

ibm-ompi · 2018-05-15T22:12:24Z

The IBM CI (XL Compiler) build failed! Please review the log, linked below.

Gist: https://gist.github.com/e882b208539784cabcacd2434afe1cad

ibm-ompi · 2018-05-15T22:12:25Z

The IBM CI (GNU Compiler) build failed! Please review the log, linked below.

Gist: https://gist.github.com/423c4ef1e8aa37b6cfa2f68f75ae47b4

ibm-ompi · 2018-05-15T22:12:33Z

The IBM CI (PGI Compiler) build failed! Please review the log, linked below.

Gist: https://gist.github.com/fcfac2d9a9dbe159e2d258ca508590af

The Java configury is split into two parts: 1. Determine if we want MPI Java bindings. 2. Find the Java compiler (and related). This commit does a few things: - Move the "Find the Java compiler" step from OPAL to OMPI (because there is no Java in OPAL, and there doesn't appear to be any immanent danger that there will be). - As a direct consequence, remove the --enable-java CLI option (--enable-mpi-java still remains). Enabling the MPI Java bindings and enabling Java are now considered the same thing (since there is no Java elsewhere in the code base, the different was meaningless). - Only invoke the "Find the Java compiler" step if we actually want the MPI Java bindings. - A few miscellaneous Java-related cleanups in configury (E.g., change testing "$foo" == "1" to $foo -eq 1, etc. This commit is mostly s/opal/ompi/gi in many places in configury and shifting code around. But it looks bigger than it actually is because of two reasons: 1. Some files were renamed: * ompi_setup_java.m4 -> ompi_setup_mpi_java.m4 (setup MPI Java bindings) * opal_setup_java.m4 -> ompi_setup_java.m4 (setup Java compiler) 2. Indenting level changed in (the new) ompi_setup_java.m4. Signed-off-by: Jeff Squyres <jsquyres@cisco.com>

jsquyres · 2018-05-15T22:15:49Z

This is why we have CI. Pushed a fix...

jsquyres · 2018-05-16T00:00:24Z

Looks like a false Mellanox failure:

02:02:20 + taskset -c 8,9 timeout -s SIGSEGV 17m /var/lib/jenkins/jobs/gh-ompi-master-pr/workspace/ompi_install1/bin/mpirun -np 8 -bind-to none -mca orte_tmpdir_base /tmp/tmp.RXpsTLcd1E --report-state-on-timeout --get-stack-traces --timeout 900 -mca coll '^hcoll' -mca pml ob1 -mca btl self,vader taskset -c 8,9 /var/lib/jenkins/jobs/gh-ompi-master-pr/workspace/ompi_install1/examples/hello_c
02:02:20 --------------------------------------------------------------------------
02:02:20 A system call failed during shared memory initialization that should
02:02:20 not have.  It is likely that your MPI job will now either abort or
02:02:20 experience performance degradation.
02:02:20 
02:02:20   Local host:  jenkins03
02:02:20   System call: open(2) 
02:02:20   Error:       No such file or directory (errno 2)
02:02:20 --------------------------------------------------------------------------

bot:mellanox:retest

jsquyres added enhancement code cleanup labels May 14, 2018

jsquyres added this to the v4.0.0 milestone May 14, 2018

jsquyres requested a review from ggouaillardet May 14, 2018 20:50

ggouaillardet reviewed May 15, 2018

View reviewed changes

bwbarrett added the Target: main label May 15, 2018

jsquyres force-pushed the pr/moar-java-updates-but-mostly-cosmetic-this-time branch from 5a321fa to 2d34443 Compare May 15, 2018 22:00

jsquyres force-pushed the pr/moar-java-updates-but-mostly-cosmetic-this-time branch from 2d34443 to 9f21ea4 Compare May 15, 2018 22:15

jsquyres merged commit 74257aa into open-mpi:master May 16, 2018

jsquyres deleted the pr/moar-java-updates-but-mostly-cosmetic-this-time branch May 16, 2018 13:42

java: clean up MPI Java configury #5170

java: clean up MPI Java configury #5170

Uh oh!

Conversation

jsquyres commented May 14, 2018

Uh oh!

ggouaillardet May 15, 2018

Choose a reason for hiding this comment

Uh oh!

ggouaillardet May 15, 2018

Choose a reason for hiding this comment

Uh oh!

jsquyres May 15, 2018

Choose a reason for hiding this comment

Uh oh!

ggouaillardet May 15, 2018

Choose a reason for hiding this comment

Uh oh!

ggouaillardet May 15, 2018

Choose a reason for hiding this comment

Uh oh!

jsquyres May 15, 2018

Choose a reason for hiding this comment

Uh oh!

jsquyres commented May 15, 2018

Uh oh!

ibm-ompi commented May 15, 2018

Uh oh!

ibm-ompi commented May 15, 2018

Uh oh!

ibm-ompi commented May 15, 2018

Uh oh!

jsquyres commented May 15, 2018

Uh oh!

jsquyres commented May 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants