clEsperanto / clesperantoj_prototype

A Java Wrapper around CLIc / clesperanto
4 stars 2 forks source link

Progress on maven build stabilization #28

Open carlosuc3m opened 2 months ago

carlosuc3m commented 2 months ago

Hello, this is more of an informative issue to communicate with the clesperanto team and to keep trace of the stabilization of the maven build. If this is not the place to do this, please let me know.

I have been working on improving the pom.xml file to be able to build the clesperantoj_prototype simply with mvn build and no tricks. I am using @StRigaud 's branch use-new-backend-gpu as the base. The code in this branch could not be built directly using mvn, it needed a trick where one execution commenting out some code was executed first and then the whole pom was used for the final execution.

By my understanding, this happened because the maven goal parse from the maven plugin org.bytedeco::javacpp needed the classes net.clesperanto.presets.clesperantoj and net.clesperanto.wrapper.clesperantoj to be compiled. However, the parse goal was executed in a maven phase called generate-sources, which happens before the compile phase.

Changing the phase where this goal is executed to compile solves the error. https://github.com/carlosuc3m/clesperantoj_prototype/commit/adaa4d472a5c68e80deafb590da3fc398daf7601

However, it seems that the parse goal of the javacpp plugin was intended to run on the generate-sources phase. I will keep researching how the javacpp plugin is intended to be used and come back with more.

Also I do not know if this issue might be helpful for https://github.com/clEsperanto/clesperantoj_prototype/issues/4

carlosuc3m commented 2 months ago

It seems that compiling the project before parsing is the way to go. What the official JavaCPP presets repo does is to compile twice, once in the generate-sources phase and another in the default phase: https://github.com/bytedeco/javacpp-presets/blob/34289d4d0c421fa345c9f537bb3afdde4bb4a0c6/pom.xml#L155

bnorthan commented 2 months ago

Hi @carlosuc3m

It looks like the javacpp-parser is executed 3 times in that pom

  1. Here under maven-resources-plugin it is called with <phase>generate-sources</phase> and goal resources.
  2. Here under maven-compiler-plugin it is called with <phase>generate-sources</phase> and goal compile.
  3. Here under java-cpp-pluginit is called with <phase>generate-sources</phase> and goal parse.

I'm not a maven build expert but I would have thought only the third would be needed. Intuitively I would have expected the java-cpp-plugin to be run before maven-compiler-plugin.

In the javacpp step it should parse c header files, create java wrapper files, then compile the native wrapper library. Then the java compiler should be called after javacpp (as it needs the auto-generated java wrapper).

I haven't dived into this in a few months, but my recollection was that the 'hacks' were needed because sometimes the javacpp parser did not execute. Robert mentioned that all java needs to compiled together, however while true it still doesn't make sense to me because the c++ compile should be independent of the java compile (although it does use a java file). Sorry if I've confused the situation, but...

  1. Programmer creates foo.h, and a java file fooWrapperJ.java, the latter just points to where foo.h exists, and which c++ dependency libraries it needs to link to. foo is compiled separately so you have foo.dll (on windows) already.
  2. JavaCpp Parser uses fooWrapperJ.java and foo.h to create a second file (say fooWrapper.java) which is an java API to whatever exists in foo.h.
  3. JavaCpp compiler creates some c++ library (jnifooWrapper.dll on windows)
  4. Java compiler executed to create foo.jar, optionally we can pack in foo.dll, jnifooWrapper.dll, and other dependencies in the jar (this is another issue we will have to discuss eventually, alternatively these can be into some other location on the system search path (like lib folder in Fiji).

Sorry for the long write-up but I am just trying to keep all this straight in my mind... Thanks for finding a solution @carlosuc3m . I still don't understand why 'compiling before parsing' fixes it. I would have though you need to parse, c++ compile, then java compile in that order.

Brian

StRigaud commented 2 months ago

Quick feedback on my side, I tried @carlosuc3m forks on both my M2 sillicon and my Ubuntu: It build right away, no struguling nor modification needed except the maven-compiler-plugin plugin to set the source and target to 1.8 instead of 1.7.

carlosuc3m commented 2 months ago

Hello @bnorthan First of all sorry for the dealy responding.

I completely understand you question about why compiling before parsing is required and honestly I don't think I can provide great answer for that.

My intuition says that the key is here:

JavaCpp Parser uses fooWrapperJ.java and foo.h to create a second file (say fooWrapper.java) which is an java API to whatever exists in foo.h.

In order to use fooWrapperJ.java you need fooWrapperJ.class to be compiled. In that way you can use the InfoMap objects from fooWrapperJ class to create the JNI. And then is when you compile the whole code, including the generated JNI.

Also the JAvaCPP always executes, we just need to be careful on the order in which the plugins are defined, because for goals in the same phase, the one who is defined first is the one executed.

Again, I am still diving into this and trying to get everything to work on Windows too, so I might be wrong. But this is the only explanation that i find.

Carlos

carlosuc3m commented 2 months ago

Glad to hear that! @StRigaud

It build right away, no struguling nor modification needed except the maven-compiler-plugin plugin to set the source and target to 1.8 instead of 1.7.

Yes, sorry I should have changed that but i left the default one that is used in JAvacpp

carlosuc3m commented 1 month ago

Some progress on this. I will create a pull request from my fork as the pom.xml already builds directly both on Windows and on Linux. I have manged to remove the random paths that were needed on Windows for the project to compile. However, the success of this operation depends greatly on the environment variables being properly configured.

The INCLUDE environment variable should contain the OpenCL include directory, which houses the headers for OpenCL. The addition of this folder to the INCLUDE var also removes the need to add the parameter -DOpenCL_INCLUDE_DIR to the CMAKE command on the cppbuild.sh. The other Windows specific parameter -DOpenCL_LIBRARY can also be eliminated if the path to the folder containing opencl.lib is added to the LIB env var https://github.com/carlosuc3m/clesperantoj_prototype/commit/5d7d6da93dcaa804dea3145b81cac32c2705339c

In addition, the Windows Kit Lib folder (for example C:/Program Files (x86)/Windows Kits/10/Lib/10.0.22621.0/um/x64) must be added to the LIB env var.

Another issue that I found on windows is that if i did not link "Shell32" using the properties on the presets clesperantoJ class I was not able to compile on windows. Here is the fix, do you have any idea why? https://github.com/carlosuc3m/clesperantoj_prototype/commit/a6385f1cde0c455f44b5bdbc0d075205cc67165c

Finally, one last question with respect to the compiler parameter for macos, can it be the same as in linux (-std=c++17) or shuuld it be -framework OpenCL as specified here: https://github.com/carlosuc3m/clesperantoj_prototype/blob/5d7d6da93dcaa804dea3145b81cac32c2705339c/pom.xml#L195

Regards, Carlos

StRigaud commented 1 month ago

The INCLUDE environment variable should contain the OpenCL include directory, which houses the headers for OpenCL. The addition of this folder to the INCLUDE var also removes the need to add the parameter -DOpenCL_INCLUDE_DIR to the CMAKE command on the cppbuild.sh. The other Windows specific parameter -DOpenCL_LIBRARY can also be eliminated if the path to the folder containing opencl.lib is added to the LIB env var carlosuc3m@5d7d6da

Ideally this could be solve by having the .hpp in the path, otherwise we can git fetch the header from the khronos repo into the lib folder when building clic. I can integrate this in the cmakelist.txt

In addition, the Windows Kit Lib folder (for example C:/Program Files (x86)/Windows Kits/10/Lib/10.0.22621.0/um/x64) must be added to the LIB env var.

If I understand well this is the VScode runtime executable that need to be install on the computer that build on windows. This should simply be put as a requirement for build but I thinks this is out of out scope. Ideally we do not have to specify the path if install (to be tested)

Another issue that I found on windows is that if i did not link "Shell32" using the properties on the presets clesperantoJ class I was not able to compile on windows. Here is the fix, do you have any idea why? carlosuc3m@a6385f1

The Shell32 lib is required for the cache management on windows. See : https://github.com/clEsperanto/CLIc/blob/2c9375e9a953585faf7a151e21706f585b6b3168/clic/include/cache.hpp#L13

We keep it for now if it does not require complexe install on windows, or untill we find a cleaner way to manage this.

Finally, one last question with respect to the compiler parameter for macos, can it be the same as in linux (-std=c++17) or shuuld it be -framework OpenCL as specified here: https://github.com/carlosuc3m/clesperantoj_prototype/blob/5d7d6da93dcaa804dea3145b81cac32c2705339c/pom.xml#L195

std=c++17 is find for both unix and osx, no issue here. As for the -framework OpenCL it does not seems to affect my OSx build. So we should be able to remove.