Plugin java jni support #9575
Plugin java jni support #9575
Conversation
There was a problem hiding this comment.
Thanks @alanpaxton for the PR. Mostly LGTM. Since I am not expert at Makefile or RocksJava, will defer to @adamretter to approve.
java/Makefile
Outdated
| $(TEST_SRC)/org/rocksdb/util/*.java\ | ||
| $(TEST_SRC)/org/rocksdb/*.java | ||
| $(AM_V_at)$(JAVAH_CMD) -cp $(MAIN_CLASSES):$(TEST_CLASSES) -d $(NATIVE_INCLUDE) -jni $(NATIVE_JAVA_TEST_CLASSES) | ||
| ifeq ($(shell java -version 2>&1|grep 1.7.0 >/dev/null; printf $$?),0) |
There was a problem hiding this comment.
Should java be $(JAVAC_CMD)?
There was a problem hiding this comment.
Yes. Update incoming.
java/Makefile
Outdated
| $(AM_V_at)$(JAVAC_CMD) $(JAVAC_ARGS) -d $(MAIN_CLASSES)\ | ||
| $(MAIN_SRC)/org/rocksdb/util/*.java\ | ||
| $(MAIN_SRC)/org/rocksdb/*.java | ||
| ifeq ($(shell java -version 2>&1 | grep 1.7.0 > /dev/null; printf $$?), 0) |
There was a problem hiding this comment.
Should java be $(JAVAC_CMD)? Trying to understand the reason for this change.
There was a problem hiding this comment.
Yes it should. Rest of the change is that I've created a single variable SOURCES to gather all the files being compiled.
|
@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@alanpaxton has updated the pull request. You must reimport the pull request before landing. |
|
@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@alanpaxton has updated the pull request. You must reimport the pull request before landing. |
|
@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@alanpaxton @adamretter is the Travis java_test failure related? |
|
I can't see how @riversand963 but I am often stupid. Could it be something to do with changed C++ versions ? See this https://stackoverflow.com/questions/61372954/gcc-with-march-invalid-switch |
|
Makes sense. @alanpaxton could you resolve the conflicts? thanks! |
Java/JNI builds to include plugin java sources plugin build flags pplugin tests
Was invoking java directly
alanpaxton
force-pushed
the
plugin-java-jni-support
branch
from
326f077
to
ff1a747
Compare
|
@alanpaxton has updated the pull request. You must reimport the pull request before landing. |
|
Resolved. Good night. I'll check back in the morning (GMT) if there's anything that's still not right. |
|
@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Extend the plugin architecture to allow for the inclusion, building and testing of Java and JNI components of a plugin. This will cause the JAR built by
$ make rocksdbjavato include the extra functionality provided by the plugin, and will cause$ make jtestto add the java tests provided by the plugin to the tests built and run by Java testing.The plugin's
<plugin>.mkfile can define:The plugin should provide java/src, java/test and java/rocksjni directories. When a plugin is required to be build it must be named in the ROCKSDB_PLUGINS environment variable (as per the plugin architecture). This now has the effect of adding the files specified by the above definitions to the appropriate parts of the build.
An example of a plugin with a Java component can be found as part of the hdfs plugin in https://github.com/riversand963/rocksdb-hdfs-env - at the time of writing the Java part of this fails tests, and needs a little work to complete, but it builds correctly under the plugin model.