Skip to content

Create single JNI shared library #286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

Create single JNI shared library #286

wants to merge 7 commits into from

Conversation

gbburkhardt
Copy link

For deployment, it's a little easier to have one shared library for the JNI interface, and maintenance is bit easier, too, if a binary distribution includes shared libraries for multiple platform. These changes to the makefiles in swig/java create a single library instead of five. I also prefer that all the GDAL code including external libraries be included in the single JNI library, to make sure that all required code is there and it's all compatible. That applies to the static library gdal.lib when it is produced for windows - all the external libraries should be included.

I also noticed that the Java code generated by Swig in the 'org' directory wasn't being deleted for the 'clean' target for Windows. The revised 'swig/java/makefile.vc' fixes that.

… that have been configured. Revise Java JNI shared library generation to produce one library instead of 5. This simplifies deployment. It requires that several functions that are generated for several modules be declared static. For Windows, correct the deletion the 'org' library during 'clean'; was dependent on a file named 'nul' that doesn't get created.
@rouault
Copy link
Member

rouault commented Dec 17, 2017

This fails at runtime on Travis-CI (and likely on Windows too) with

java -Djava.library.path=. -cp gdal.jar:build/apps GDALOverviews tmp_test/byte.tif "NEAREST" 2 4
Native library load failed.
java.lang.UnsatisfiedLinkError: no gdaljni in java.library.path
Exception in thread "main" java.lang.UnsatisfiedLinkError: org.gdal.gdal.gdalJNI.AllRegister()V
	at org.gdal.gdal.gdalJNI.AllRegister(Native Method)
	at org.gdal.gdal.gdal.AllRegister(gdal.java:523)
	at GDALOverviews.main(GDALOverviews.java:70)

…ig/java/GNUmakefile to include all object modules.
@gbburkhardt
Copy link
Author

Load library should be fixed now. The Linux shared JNI library still needs libgdal.so in the LD_LIBRARY_PATH. If the Windows build is without DLLBUILD defined, the final shared library will include gdal.lib, so gdal.dll is not needed. But then the static version of any external libraries are needed.

@gbburkhardt
Copy link
Author

re: 13268.1 check failed
My guess is that it used Swig version 1 for the build - I wasn't able to tell from the log (it would be nice if version info were included in the log). Versions 2 & 3 of Swig declare the SWIG_JavaArrayXXX functions as static, so the duplicate definitions don't appear in the final link. This could be fixed by defining SWIG_NOARRAYS in CFLAGS, since those functions aren't used (and if static, will be omitted when compiled with optimization).

Is there a requirement for Swig version 1 to be supported? I can't even install it on my Linux system from the Ubuntu repository. The last Swig 1.3.40 release is from 2009.

@gbburkhardt
Copy link
Author

Ok, looking at the log more closely, I see that Swig 2.0.4 was used. There was a change somewhere between that release and the Swig 2.0.12 version I used to make the SWIG_JavaArrayXXX functions static. It's not noted in the release notes, but "Lib/java/arrays_java.i" in the Swig code was changed to make the functions static.

For Swig 2.0.4, this can be fixed by adding "CFLAGS += -DSWIG_NOARRAYS" to swig/java/GNUmakefile and some similar change to makefile.vc, but that precludes the use of the Java array functions without additional work, should someone decide to use them in the futures. Suggestions?

@rouault
Copy link
Member

rouault commented Dec 19, 2017

We could potentially drop the Java bindings on the precise_clang target (they are tested in the trusty_clang too)

@gbburkhardt
Copy link
Author

Or upgrade to at least Swig 2.0.5. The use of 'static' for SWIG_JavaArrayXXX was introduced in that version. I suppose putting in the macro SWIG_NOARRAYS isn't so bad. By the time someone changes the code to use any of those functions, the more recent versions of Swig will be in use.

@gbburkhardt
Copy link
Author

re: 13301.8 failed
That doesn't look like it has to do with the changes I've made. Is that correct?
re: 13301.10 The job exceeded the maximum time limit for jobs, and has been terminated.
...also doesn't look like it has to do with the changes I've made...

$(JAVA_MODULES): $(JAVA_OBJECTS)
$(LINK) $(LDFLAGS) $^ $(CONFIG_LIBS) -o $@ $(LINK_EXTRAFLAGS)
$(JAVA_MODULES): $(JAVA_OBJECTS)
$(LINK) $(LDFLAGS) $^ $(GDAL_LIB) $(CONFIG_LIBS) -o $@ $(LINK_EXTRAFLAGS)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is a good idea to incorporate libgdal in the java bindings.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, do you think it will cause a problem? Personally, I prefer to have only one file to deal with. But, certainly, back out the change if you have strong feelings about it.

Note that if the Windows version is built with DLLBUILD undefined, this is what happens with the unmodified code base, with all of the referenced objects being included in each of the DLL's that are built.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well perhaps this makes sense on Windows, but this is less in the Unix philosophy to bundle everything

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let you decide.
My interest is upgrading the GDAL library included with Nasa's WorldWind. The copy of GDAL they are including is version 1.7.2. They have released copies of the JNI interface for multiple platforms with libgdal included in the JNI shared library. It seemed like a good idea to me, but for the life of me I can't see how they included the MrSID libraries in their DLL or .so shared libraries.
They also installed a custom loader. I have a parallel pull request outstanding with them (NASAWorldWind/WorldWindJava#146), but I left the libraries separate, thinking that if they were to be combined that should be part of GDAL.

Happy Holidays!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let you decide.

I'll prefer to not have libgdal incorporated in the libgdaljniall.so, at least not by default. If you can find a way of parameterizing it, why not.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that last change meet with your approval?

@rouault
Copy link
Member

rouault commented Jan 8, 2018

Committed in trunk r41228. Thanks !

@rouault rouault closed this Jan 8, 2018
kwrobot pushed a commit to aashish24/gdal-svn that referenced this pull request Jan 8, 2018
rouault added a commit that referenced this pull request Jan 8, 2018
…patch by gbburkhardt, fixes #286)

git-svn-id: https://svn.osgeo.org/gdal/trunk@41228 f0d54148-0727-0410-94bb-9a71ac55c965
@fer-marino
Copy link

sorry ladies but I have a problem. I've just upgraded GDAL to version 2.3.0 on my windows 10 machine. I use gradle for dependency management, so I've added compile 'org.gdal:gdal:2.3.0' to my gradle build.

It downloads a small jar that does not include any OS dependant libraries, so when I try to use it it look for the gdalalljni in my java.library.path, and it fails. Apparently in the newer distribution, that dll is not included anymore.

I got my gdal package from:
http://www.gisinternals.com/query.html?content=filelist&file=release-1911-x64-gdal-2-3-0-mapserver-7-0-7.zip

and installed the core.

I think you have two options:

  • make at a self containing jar and publish it on maven central. This jar will contains executable for all supported operating systems and does not need anything else installed to run.
  • make a generic gdal.jar, published on maven central, that requires gdal to be installed on the system. In that case anyway no other things shall be installed. This means that you have to ship the uber jni dll/so either inside the jar or with gdal base distrubution. And this is currently not the case.

Thanks for great works.

Fernando

@gbburkhardt
Copy link
Author

That binary distribution has the gdalalljni.dll file in the "bin\gdal\java" directory. Try including that directory in your "java.library.path".

@gbburkhardt
Copy link
Author

It doesn't look like the "gdal/swig/java/README.txt" file was merged to the main branch. Can that be done, or is another pull request needed? Would it be better to have the comments in the make files, too?

@fer-marino
Copy link

I'm sorry but there is no java subdirectory in my gdal root directory. I've performed a clean install selecting a complete install but still I don't see anything.

@gbburkhardt
Copy link
Author

I'm not familiar with the binary distribution you're using, or how it installs. But in the zip file, the gdalalljni.dll file is in the bin\gdal\java directory. Just copy that file to where all the other DLLs have been put by the installation, or change your "java.library.path".

image

nickrobison added a commit to nickrobison/gdal that referenced this pull request Jun 25, 2018
When the new single JNI library introduced in OSGeo#286, the Makefile was not fully updated to copy the new libraries from the .lib dir into the source directory for later copying during the install phase. This patch addresses that.
@nickrobison nickrobison mentioned this pull request Jun 25, 2018
3 tasks
rouault pushed a commit that referenced this pull request Jun 25, 2018
When the new single JNI library introduced in #286, the Makefile was not fully updated to copy the new libraries from the .lib dir into the source directory for later copying during the install phase. This patch addresses that.
@ghost ghost mentioned this pull request Apr 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants