configury: use javac vs javah whenever possible. #5001

ggouaillardet · 2018-04-02T05:07:43Z

javah has been removed from Java 10, so use javac -h instead
(that is already usable in Java 8)

Signed-off-by: Gilles Gouaillardet [email protected]

ggouaillardet · 2018-04-02T05:08:20Z

@hppritcha @bwbarrett FYI, this PR might break JDK older than 8.

kawashima-fj · 2018-04-02T11:43:06Z

@hppritcha @bwbarrett @jsquyres This change is required to support recently-released OpenJDK 10. But it causes a make-time error with OpenJDK 7 and older. JDK 7 was released in 2011. This problem appears only in the non-default --enable-mpi-java build. Supported JDK versions are described in README.JAVA.txt Can I merge it to master (and release branches)?

jsquyres

Works for me.

hppritcha · 2018-04-02T13:50:23Z

looks good to me. jenkins 32bit builds still not happy but that has nothing to do with this PR.

jjhursey · 2018-04-02T17:34:53Z

bot:ibm:gnu:retest

bwbarrett · 2018-04-02T19:12:20Z

bot:ompi:retest

Brian broke the 32bit builds...

hppritcha · 2018-04-02T19:24:45Z

@kawashima-fj have you tried to build the java bindings with java 7? I vaguely recall that some more recent additions to the bindings prevent compiling with java7. Can you check this?

kawashima-fj · 2018-04-03T00:32:42Z

@hppritcha Yes, I tried it.

Without this PR:

OpenJDK 7 on CentOS 6.9: can build and run

With this PR:

OpenJDK 7 on CentOS 6.9: cannot build Open MPI (make-time error)
OpenJDK 8 on CentOS 7.4: can build and run
OpenJDK 9 on CentOS 7.4: can build and run
OpenJDK 10 on CentOS 7.4: can build and run

I confirmed the following things on x86_64.

Open MPI can be built with --enable-mpi-java
examples/Ring.java can be built by mpijavac
examples/Ring.java can be run by mpiexec

A possible related issue is #3705/#4121. I will recall it...

RandomDSdevel · 2018-04-03T01:50:33Z

As I mentioned here on the developer mailing list, I'll rebase an attempt to fix how Open mPI's build process handles finding JDK tools on OS X/macOS that I'm working on (still only locally at the moment, but it will become a PR soon) against this after it's merged.

ggouaillardet · 2018-04-03T01:53:34Z

let me know if we want to support JDK < 8.
In this case, I will also check javah -version is working and use some macros to handle the legacy case

bwbarrett · 2018-04-03T02:34:22Z

For 3.0 and 3.1, we definitely want to keep supporting Java 7, because otherwise we'd have a fairly major backwards compatibility problem. I think 4.0 (aka master) is an open question.

ggouaillardet · 2018-04-03T06:29:15Z

@bwbarrett I reworked this PR so tries javah first, and then fallback to javac -h.
Not ideal imho, but much simpler than doing it the other way around.

kawashima-fj · 2018-04-03T11:37:04Z

@ggouaillardet If you have JDK 8 in /usr/bin and JDK 10 in another directory specified in --with-jdk-dir, Open MPI make fails with Could not find class file for 'mpi.MPI' error.

In this case, $JDK10/bin/javac is set in JAVAC and /usr/bin/javah is set in JAVAH. JDK 10 javac generates *.class. JDK 8 javah tries to generates *.h from *.class but it cannot because JDK 8 cannot read *.class generated by JDK 10 (class file version incompatibility).

ggouaillardet · 2018-04-03T12:46:16Z

Good point, I will think about a solution.

ggouaillardet · 2018-04-04T02:40:10Z

@kawashima-fj it should be good now

first, try javac -h
if fails, then ensure javah is available

kawashima-fj · 2018-04-04T09:33:03Z

@ggouaillardet I confirmed almost successful build & run with OpenJDK 7/8/9/10 and javah is used only with OpenJDK 7. But I sometimes see an error when I use parallel make.

$ cd build/ompi/mpi/java/java
$ make -j32
  JAVAC    mpi_MPI.h
  JAVAC    mpi_Comm.h
  JAVAC    mpi_Count.h
  JAVAC    mpi_CartParms.h
  JAVAC    mpi_Datatype.h
  JAVAC    mpi_CartComm.h
  JAVAC    mpi_Constant.h
  JAVAC    mpi_Errhandler.h
  JAVAC    mpi_File.h
  JAVAC    mpi_GraphComm.h
  JAVAC    mpi_GraphParms.h
  JAVAC    mpi_Group.h
  JAVAC    mpi_Info.h
  JAVAC    mpi_Op.h
  JAVAC    mpi_Intercomm.h
  JAVAC    mpi_Intracomm.h
  JAVAC    mpi_Message.h
  JAVAC    mpi_Prequest.h
  JAVAC    mpi_ShiftParms.h
  JAVAC    mpi_Request.h
  JAVAC    mpi_Status.h
  JAVAC    mpi_Version.h
JAVADOC    doc
  JAVAC    mpi_Win.h
../../../../../ompi/mpi/java/java/Intracomm.java:134: error: cannot access Info
        @Override public Intracomm dupWithInfo(Info info) throws MPIException
                                               ^
  bad class file: ./mpi/Info.class
    class file contains wrong class: mpi.MPIException
    Please remove or make sure it appears in the correct subdirectory of the classpath.
1 error
make: *** [mpi_Intracomm.h] Error 1
make: *** Waiting for unfinished jobs....

I finally find that javac -h . -d . Foo.java creates Foo.h and Foo.class at the same time and javac -h . -d . Bar.java reads Foo.class if Bar.java uses Foo class. It causes a race. One possible solution will be adding -h . to the rule for mpi/MPI.class and delete the rule for $(JAVA_H) if OPAL_HAVE_JAVAH_SUPPORT is true.

ggouaillardet · 2018-04-04T10:31:21Z

@kawashima-fj thanks for the insight ! I suspected such an issue might occur, but was unable to evidence it. I will fully review the process and update the PR

Signed-off-by: Gilles Gouaillardet <[email protected]>

javah is no more available from Java 10, so try javac -h first (available since Java 8) and fallback on javah Refs. open-mpi#5000 Signed-off-by: Gilles Gouaillardet <[email protected]>

ggouaillardet · 2018-04-05T02:50:42Z

@kawashima-fj we should be good now, can you please give this PR a final review ?

ggouaillardet · 2018-04-05T02:50:53Z

:bot:mellanox:retest

kawashima-fj · 2018-04-05T05:58:52Z

@ggouaillardet I'll review and test later today or tomorrow.

kawashima-fj · 2018-04-06T02:30:04Z

@ggouaillardet I reviewed and tested it on OpenJDK 7/8/9/10. Perfect.

For our record:
This change affects Open MPI only when the non-default --enable-mpi-java option (Open MPI Java binding) is specified to configure.
Without this change, building Open MPI with OpenJDK 10 (and future versions) fails because OpenJDK 10 dropped javah command.
With this change, all recent OpenJDK versions (at least 7, 8, 9, and 10) are supported.
I don't know other vendor's JDK. But if Open MPI can be built without this change, it can also be built with this change.
The combination of OpenJDK 10 and OS X/macOS has still the issue #5000. The PR #5015 is proposed for it.

…acOS. Also avoid picking up Apple's Java shims via the sym. links to them in `/usr/bin` on systems where any one of them could possibly exhibit behavior that is erratic and, to some extent, likely to be incorrect nowadays (cf.: - https://www.mail-archive.com/[email protected]/msg20551.html - open-mpi#5015 (comment) - the last part of open-mpi#5015 (comment) - open-mpi#5015 (comment) for more detailed context.) Works alongside open-mpi#5001 to close open-mpi#5000. Signed-off-by: Bryce Glover <[email protected]> (cherry picked from commit open-mpi/ompi@8c32cd8)

bosilca · 2018-05-09T16:48:27Z

This broke OMPI on OSX systems without Java SDK. The configure exits with the following:

*** Java compiler
Unable to find any JVMs matching version "(null)".
No Java runtime present, try --request to install.
checking OS X/macOS locations... found ()
configure: WARNING: No recognized directory structure found under
configure: error: Cannot continue

RandomDSdevel · 2018-05-09T18:59:46Z

@bosilca:

Are you sure that this PR is to blame? That looks like the part of the build infrastructure that goes through the logic I changed in #5015. I'm not sure how what I did could have caused this to break, though, at least at first glance; I'm pretty sure I didn't touch anything having to do with whether the Java bindings are enabled or not, and I would suspect that those always required you to have a JDK installed…

bosilca · 2018-05-09T19:04:26Z

I am not 100% sure this PR was to be blamed, but I thought it was the last thing that touched the java support.

If I don't want java bindings, and especially if I did not requested them specifically, why should I have an SDK installed ?

jsquyres · 2018-05-09T19:16:45Z

George is right -- we do not require a Java SDK to build/run OMPI. Regardless of whether it was this PR or #5001, both of them touched the Java configury recently, and we apparently got it wrong. 😦

RandomDSdevel · 2018-05-09T19:34:53Z

@jsquyres:

Well, back to the drawing board it is, then. Upon further investigation, I believe my inserting this line into that file as a blind copy of the suggestion that you requested near the bottom of the code sketch you put in list item 2 of this review comment of yours is to blame; one of us should have noticed that this would cause this part of the build scripting to abort and thus make the fallback logic below unreachable. Removing that line should fix the resulting problem that @bosilca reported; I'll file a new PR to do just that against master shortly, and then that can get backported to the other branches (unless you want me to muck with Git history, although that…doesn't sound like a very good idea.)

RandomDSdevel · 2018-05-09T20:29:22Z

@jsquyres: Submitted as #5160.

jsquyres · 2018-05-09T21:32:41Z

@RandomDSdevel Perfect -- thanks! (and correct: mucking with public git history in a large multi-developer project -- just say no!)

@bosilca

That PR accidentally changed Open MPI's build configuration infrastruc- ture's Java toolchain detection logic so that it would, as reported by @bosilca in open-mpi#5001 (comment) and tracked down by me in open-mpi#5001 (comment), abort your entire in-progress Open MPI build when it failed to find an OS X/macOS JDK instead of simply falling back to checking for a JDK in locations where it would be found on other platforms. _Oops…!_ Signed-off-by: Bryce Glover <[email protected]> (cherry picked from commit 4a05c7e)

@bosilca

That PR accidentally changed Open MPI's build configuration infrastruc- ture's Java toolchain detection logic so that it would, as reported by @bosilca in open-mpi#5001 (comment) and tracked down by me in open-mpi#5001 (comment), abort your entire in-progress Open MPI build when it failed to find an OS X/macOS JDK instead of simply falling back to checking for a JDK in locations where it would be found on other platforms. _Oops…!_ Signed-off-by: Bryce Glover <[email protected]> (cherry picked from commit open-mpi/ompi@4a05c7e)

@bosilca

That PR accidentally changed Open MPI's build configuration infrastruc- ture's Java toolchain detection logic so that it would, as reported by @bosilca in open-mpi#5001 (comment) and tracked down by me in open-mpi#5001 (comment), abort your entire in-progress Open MPI build when it failed to find an OS X/macOS JDK instead of simply falling back to checking for a JDK in locations where it would be found on other platforms. _Oops…!_ Signed-off-by: Bryce Glover <[email protected]> (cherry picked from commit 4a05c7e)

…acOS. Also avoid picking up Apple's Java shims via the sym. links to them in `/usr/bin` on systems where any one of them could possibly exhibit behavior that is erratic and, to some extent, likely to be incorrect nowadays (cf.: - https://www.mail-archive.com/[email protected]/msg20551.html - #5015 (comment) - the last part of #5015 (comment) - #5015 (comment) for more detailed context.) Works alongside #5001 to close #5000. Signed-off-by: Bryce Glover <[email protected]> (cherry picked from commit 8c32cd8) Signed-off-by: KAWASHIMA Takahiro <[email protected]>

ggouaillardet added enhancement Target: main labels Apr 2, 2018

ggouaillardet assigned kawashima-fj Apr 2, 2018

ggouaillardet requested a review from kawashima-fj April 2, 2018 05:07

kawashima-fj approved these changes Apr 2, 2018

View reviewed changes

jsquyres approved these changes Apr 2, 2018

View reviewed changes

hppritcha added the NEWS label Apr 2, 2018

open-mpi deleted a comment from ibm-ompi Apr 2, 2018

RandomDSdevel mentioned this pull request Apr 3, 2018

[Formula/open-mpi] Open MPI Fails to Build and Install Under OS X v10.11.6 'El Capitan' Homebrew/homebrew-core#26009

Closed

6 tasks

ggouaillardet force-pushed the topic/javah branch from a936f91 to 045af83 Compare April 3, 2018 05:07

ggouaillardet changed the title ~~configury: replace javah with javac -h~~ configury: use javac -h when javah cannot be used. Apr 3, 2018

ggouaillardet force-pushed the topic/javah branch from 045af83 to 0be7b86 Compare April 4, 2018 02:37

ggouaillardet force-pushed the topic/javah branch from 0be7b86 to bc9d571 Compare April 4, 2018 04:07

ggouaillardet changed the title ~~configury: use javac -h when javah cannot be used.~~ configury: use javac -h when possible. Apr 4, 2018

ggouaillardet added 2 commits April 5, 2018 09:27

java: cleanup the list of automatically generated header files

132ea1a

Signed-off-by: Gilles Gouaillardet <[email protected]>

configury: use javac vs javah whenever possible

5370586

javah is no more available from Java 10, so try javac -h first (available since Java 8) and fallback on javah Refs. open-mpi#5000 Signed-off-by: Gilles Gouaillardet <[email protected]>

ggouaillardet force-pushed the topic/javah branch from bc9d571 to 5370586 Compare April 5, 2018 02:46

ggouaillardet changed the title ~~configury: use javac -h when possible.~~ configury: use javac vs javah whenever possible. Apr 5, 2018

kawashima-fj mentioned this pull request Apr 5, 2018

config/opal_setup_java.m4: Improve JDK Tool Path Resolution on OS X/macOS #5015

Merged

kawashima-fj merged commit 5e12e0f into open-mpi:master Apr 6, 2018

RandomDSdevel mentioned this pull request Apr 6, 2018

Open MPI Fails to Build Under OS X v10.11.6 'El Capitan' Due to the Absence of javah in Java 10 #5000

Closed

This was referenced May 1, 2018

v3.1: config/opal_setup_java.m4: Improve JDK tool path resolution on OS X/… #5118

Merged

v3.0: config/opal_setup_java.m4: Improve JDK tool path resolution on OS X/… #5119

Merged

RandomDSdevel mentioned this pull request May 9, 2018

open-mpi: Fix brew installing Open MPI --with-java. Homebrew/homebrew-core#27577

Closed

4 tasks

RandomDSdevel mentioned this pull request May 9, 2018

config/opal_setup_java.m4: Fix #5015. #5160

Merged

jsquyres mentioned this pull request May 9, 2018

v3.0.x: Fix config/opal_setup_java.m4 from commit merged in #5119 #5161

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

configury: use javac vs javah whenever possible. #5001

configury: use javac vs javah whenever possible. #5001

ggouaillardet commented Apr 2, 2018

ggouaillardet commented Apr 2, 2018

kawashima-fj commented Apr 2, 2018

jsquyres left a comment

hppritcha commented Apr 2, 2018

jjhursey commented Apr 2, 2018

bwbarrett commented Apr 2, 2018

hppritcha commented Apr 2, 2018

kawashima-fj commented Apr 3, 2018

RandomDSdevel commented Apr 3, 2018

ggouaillardet commented Apr 3, 2018

bwbarrett commented Apr 3, 2018

ggouaillardet commented Apr 3, 2018

kawashima-fj commented Apr 3, 2018

ggouaillardet commented Apr 3, 2018

ggouaillardet commented Apr 4, 2018

kawashima-fj commented Apr 4, 2018

ggouaillardet commented Apr 4, 2018

ggouaillardet commented Apr 5, 2018

ggouaillardet commented Apr 5, 2018

kawashima-fj commented Apr 5, 2018

kawashima-fj commented Apr 6, 2018

bosilca commented May 9, 2018

RandomDSdevel commented May 9, 2018

bosilca commented May 9, 2018

jsquyres commented May 9, 2018

RandomDSdevel commented May 9, 2018

RandomDSdevel commented May 9, 2018

jsquyres commented May 9, 2018

configury: use javac vs javah whenever possible. #5001

configury: use javac vs javah whenever possible. #5001

Conversation

ggouaillardet commented Apr 2, 2018

ggouaillardet commented Apr 2, 2018

kawashima-fj commented Apr 2, 2018

jsquyres left a comment

Choose a reason for hiding this comment

hppritcha commented Apr 2, 2018

jjhursey commented Apr 2, 2018

bwbarrett commented Apr 2, 2018

hppritcha commented Apr 2, 2018

kawashima-fj commented Apr 3, 2018

RandomDSdevel commented Apr 3, 2018

ggouaillardet commented Apr 3, 2018

bwbarrett commented Apr 3, 2018

ggouaillardet commented Apr 3, 2018

kawashima-fj commented Apr 3, 2018

ggouaillardet commented Apr 3, 2018

ggouaillardet commented Apr 4, 2018

kawashima-fj commented Apr 4, 2018

ggouaillardet commented Apr 4, 2018

ggouaillardet commented Apr 5, 2018

ggouaillardet commented Apr 5, 2018

kawashima-fj commented Apr 5, 2018

kawashima-fj commented Apr 6, 2018

bosilca commented May 9, 2018

RandomDSdevel commented May 9, 2018

bosilca commented May 9, 2018

jsquyres commented May 9, 2018

RandomDSdevel commented May 9, 2018

RandomDSdevel commented May 9, 2018

jsquyres commented May 9, 2018