Initial JNI configuration

xevor11 commented 4 months ago

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

GitMensch commented 4 months ago

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.

xevor11 commented 4 months ago

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.

Update: Fixed warnings

xevor11 commented 4 months ago

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?

GitMensch commented 4 months ago

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).

xevor11 commented 3 months ago

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

xevor11 commented 3 months ago

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

GitMensch commented 3 months ago

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 commented 3 months ago

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.

nberth commented 3 months ago

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.

GitMensch commented 3 months ago

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 commented 3 months ago

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?

nberth commented 3 months ago

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).

GitMensch commented 3 months ago

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?

nberth commented 3 months ago

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.

GitMensch commented 3 months ago

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

nberth commented 3 months ago

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

xevor11 commented 3 months ago

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

GitMensch commented 2 months ago

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?

OCamlPro / gnucobol

Initial JNI configuration #157