gnucobol icon indicating copy to clipboard operation
gnucobol copied to clipboard

Initial JNI configuration

Open xevor11 opened this issue 1 year ago • 18 comments

Cleaned up java.c, added the requested changes from PR#150, ready for feedback and comments

xevor11 avatar Jul 09 '24 13:07 xevor11

I hope my commit to java.c did not break anything... (I just felt that the explanations would take much longer and those are more related to libcob in general than the changes of this project itself). ... if not: thank you for cleaning it up.

And yes, once there is a running simple test, we should update the branch from gcos4 which will then also include the CI. for further commits.

GitMensch avatar Jul 11 '24 10:07 GitMensch

I hope my commit to java.c did not break anything... (I just felt that the explanations would take much longer and those are more related to libcob in general than the changes of this project itself). ... if not: thank you for cleaning it up.

And yes, once there is a running simple test, we should update the branch from gcos4 which will then also include the CI. for further commits.

image

Update: Fixed warnings

xevor11 avatar Jul 12 '24 12:07 xevor11

Addressing this specific system call:

System.getProperty("java.version")

I wrote a simple method call:

if ((cob_global_exception & 0x0b00) == 0x0b00) cob_global_exception = 0;
    if (call_java_class_method_call.funcvoid == NULL || cob_glob_ptr->cob_physical_cancel)
  {
    call_java_class_method_call.funcvoid = cob_resolve_cobol ("java_class_method_call", 0, 0);
  }

however internally it must be create the VM and retrieve the methods from the JNI Function Table?

xevor11 avatar Jul 30 '24 00:07 xevor11

however internally it must be create the VM and retrieve the methods from the JNI Function Table?

Yes the first fist to be done in java.c once and the other is done by the cob_resolve_java (which will also set the exception and possible internal registers if resolving it does not work).

GitMensch avatar Jul 30 '24 11:07 GitMensch

Additional changes got added due to new state of jni-config

xevor11 avatar Aug 07 '24 00:08 xevor11

Yes we have a test that verifies the version, I was wondering for git diff I can compare my local against upstream/java-interop?

xevor11 avatar Aug 07 '24 15:08 xevor11

The later would match the design we have in fileio best and would also be easily prepared now by just doing a direct call for the time being - the delay load addition is then only minimal.

For now I'd primarily like to see the working java calls in the testsuite and running in CI - simple static one, followed by more complex examples similar to the one from IBM.

GitMensch avatar Aug 13 '24 03:08 GitMensch

I suggest to give it a go with gitpod and see hows the result there - as you can use the debugger if it doesn't work.

GitMensch avatar Aug 14 '24 13:08 GitMensch

I suggest to give it a go with gitpod and see hows the result there - as you can use the debugger if it doesn't work.

The new tests seem to pass the CI (cf https://github.com/OCamlPro/gnucobol/actions/runs/10389348104/job/28767156138?pr=157#step:11:2637). @xevor11 I suspect there might be something fishy with your setup… no idea what that could be, however.

nberth avatar Aug 14 '24 14:08 nberth

As seen in the Test for the MacOS CI:

  • configure.ac: has to be adjusted COB_HAVE_JNI->COB_HAS_JNI (already fixed)
  • configure.ac: --with-java should abort if no applicable jni.h can be found
  • either a part of the configure macros does not work (for the MacOS CI just issue a find -name to look for installations of jni.h) or needs an extra install (I'd prefer to get the current path working first, before supporting the Homebrew installation next)
  • it will be interesting to see Win32 builds - I guess they need an OpenJDK installation outside of pacman/vcpkg

GitMensch avatar Aug 14 '24 15:08 GitMensch

As noted in the other PR where we had the last commit before: please adjust libcob/Changelog for that and revert the definition of lt_dladdsearchdir and the changes to the defines above; the use of JAVA_HOME should definitely be documented in gnucobol.texi. Please adjust the new CI files as well (no install.log needed, adding the || make check TESTSUITEFLAGS="--recheck --verbose" for getting details on the failing testcase runs, replacing the commented NIST tests by a single note that those are not useful to be executed with this build as the java parts are not used there).

Afterwards (and after integrating the Windows CI that is work in progress) - what would be next for this PR? Checking that the arguments are one of the cobol java types and handle those?

GitMensch avatar Aug 21 '24 08:08 GitMensch

Afterwards (and after integrating the Windows CI that is work in progress) - what would be next for this PR? Checking that the arguments are one of the cobol java types and handle those?

I was thinking, it may be more manageable to restrict this PR to only deal with void (void) methods, and leave handling of parameters and returned values in a dedicated PR. In this case, what is missing for this PR are:

  • proper detection and reporting of calls to missing methods (one of the new tests should not pass as is);
  • reporting/erroring on CALL "Java.…" with parameters and/or returned values (maybe with a CB_PENDING, probably to be added in parser.y);
  • moving detection of malformed method names to the parsing phase (CALL "Java.someMethodWihtoutClass" should be reported before codegen);
  • adjustments to the ChangeLogs and the documentation;
  • (probably some other missing items).

nberth avatar Aug 21 '24 12:08 nberth

It will be quite beneficial if we use the x86 MSYS2 version for testing an old JDK (version 8).

Can you please try to put a cached download of https://github.com/adoptium/temurin8-binaries/releases/download/jdk8u422-b05/OpenJDK8U-jdk_x86-32_windows_hotspot_8u422b05.zip in use - only for mingw32?

GitMensch avatar Aug 21 '24 14:08 GitMensch

It will be quite beneficial if we use the x86 MSYS2 version for testing an old JDK (version 8).

Can you please try to put a cached download of https://github.com/adoptium/temurin8-binaries/releases/download/jdk8u422-b05/OpenJDK8U-jdk_x86-32_windows_hotspot_8u422b05.zip in use - only for mingw32?

Yes that's a good idea.

Note Java 8 from Oracle seems to be the default on the Windows CIs (https://github.com/actions/runner-images/blob/main/images/windows/Windows2022-Readme.md#java) — although I might not be aware of all intricacies behind versioning of Java VMs/JDKs.

nberth avatar Aug 21 '24 14:08 nberth

That's an interesting doc and we may should reset the JAVA_HOME to the 21 one for all other Windows environments then.

GitMensch avatar Aug 21 '24 18:08 GitMensch

That's an interesting doc and we may should reset the JAVA_HOME to the 21 one for all other Windows environments then.

cf #173

nberth avatar Aug 22 '24 08:08 nberth

There is another PR #174 which includes the non-void method handling as well

xevor11 avatar Aug 23 '24 13:08 xevor11

I've got word from the original author of the java related macros used and that's (part of) his answer:

I'm afraid I cannot help and that it would be better to make these macros as deprecated/abandoned.

I'm likely to adjust the part configure for that (but that likely takes some days until I get to this), @nberth can you please check the review related to the delay-load part and @xevor11 could you please check on the rest?

GitMensch avatar Sep 14 '24 17:09 GitMensch