Skip to content
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.

Sign up for GitHub

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

fix JNI target #26

Merged
merged 11 commits into from
May 7, 2021
Merged

fix JNI target #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
7893d50
update conan install instructions
jothepro Apr 28, 2021
158c5be
improve install instructions
jothepro Apr 30, 2021
531d172
remove mandatory DJINNI_STATIC_LIB option. DJINNI_STATIC_LIB can stil…
jothepro Apr 30, 2021
daa0e0f
make sure BUILD_SHARED_LIB is always set, because the rest of the fil…
jothepro Apr 30, 2021
098f7e9
reduce complexity of library configuration by introducing the variabl…
jothepro May 1, 2021
bab9f04
fix JNI build: always enable -fPIC for JNI, because the resulting sta…
jothepro May 1, 2021
157e16a
add different approach for handling the JNI_OnLoad problem. JNI_OnLoa…
jothepro May 1, 2021
52bad5c
fix JNI build: disable --jni-generate-main for second generator run. …
jothepro May 2, 2021
4c6f601
update djinni generator version to v1.0.0
jothepro May 7, 2021
cc586eb
fix github actions: add new djinni_jni_main.hpp to list of installed …
jothepro May 7, 2021
3e0d40e
Merge branch 'main' of github.com:cross-language-cpp/djinni-support-l…
jothepro May 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .tool-versions
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
djinni 0.5.0
djinni 1.0.0
java zulu-11.41.23
13 changes: 5 additions & 8 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ set(SRC_JNI
"djinni/jni/djinni_support.hpp"
"djinni/jni/Marshal.hpp"
"djinni/jni/djinni_support.cpp"
"djinni/jni/djinni_jni_main.hpp"
"djinni/jni/djinni_jni_main.cpp"
)

set(SRC_OBJC
Expand Down Expand Up @@ -62,22 +64,18 @@ set_target_properties(djinni_support_lib PROPERTIES
CXX_EXTENSIONS false
)


install(
TARGETS djinni_support_lib
LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}"
ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}"
RUNTIME DESTINATION "${CMAKE_INSTALL_BINDIR}"
)


install(
FILES ${SRC_SHARED}
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/djinni/
)



# Objective-C support
option(DJINNI_WITH_OBJC "Include the Objective-C support code in Djinni support library." off)
if(DJINNI_WITH_OBJC)
Expand All @@ -104,11 +102,9 @@ target_include_directories(djinni_support_lib PUBLIC "$<BUILD_INTERFACE:${CMAKE_

# JNI support
option(DJINNI_WITH_JNI "Include the JNI support code in Djinni support library." off)
option(DJINNI_JNI_WITH_MAIN "Include the default minimal JNI_OnLoad and JNI_OnUnload implementation in Djinni support library." on)
if(DJINNI_WITH_JNI)
if(DJINNI_JNI_WITH_MAIN)
list(APPEND SRC_JNI "djinni/jni/djinni_main.cpp")
endif()
set_target_properties(djinni_support_lib PROPERTIES
POSITION_INDEPENDENT_CODE ON)
jothepro marked this conversation as resolved.
Show resolved Hide resolved
target_include_directories(djinni_support_lib PUBLIC "$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/djinni/jni/>")
target_sources(djinni_support_lib PRIVATE ${SRC_JNI})
source_group("jni" FILES ${SRC_JNI})
Expand All @@ -117,6 +113,7 @@ if(DJINNI_WITH_JNI)
FILES
"djinni/jni/djinni_support.hpp"
"djinni/jni/Marshal.hpp"
"djinni/jni/djinni_jni_main.hpp"
DESTINATION
${CMAKE_INSTALL_INCLUDEDIR}/djinni/jni
)
Expand Down
17 changes: 17 additions & 0 deletions djinni/jni/djinni_jni_main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//
// Copyright 2021 cross-language-cpp
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

#include "djinni_jni_main.hpp"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This new files only purpose is to inclued the JNI_OnLoad implementation from djinni_jni_main.hpp.

7 changes: 4 additions & 3 deletions djinni/jni/djinni_main.cpp → djinni/jni/djinni_jni_main.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//
// Copyright 2014 Dropbox, Inc.
// Copyright 2021 cross-language-cpp
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
Expand All @@ -13,9 +14,9 @@
// See the License for the specific language governing permissions and
// limitations under the License.
//

// This provides a minimal JNI_OnLoad and JNI_OnUnload implementation - include it if your
// app doesn't use JNI except through Djinni.
// This provides a minimal JNI_OnLoad and JNI_OnUnload implementation.
// Don't include it if your library does require a custom JNI_OnLoad implementation with custom initialization logic.
#pragma once
Comment on lines +17 to +19
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've transformed djinni_main.cpp to a header file. That way anybody can include it and get the JNI_OnLoad implementation. This only works if the header is included once. Otherwise the linker will complain.


#include "djinni_support.hpp"

Expand Down
4 changes: 2 additions & 2 deletions djinni/jni/djinni_support.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
#include <string>
#include <vector>

#include "../proxy_cache_interface.hpp"
#include "../djinni_common.hpp"
#include "djinni/proxy_cache_interface.hpp"
#include "djinni/djinni_common.hpp"
Comment on lines +26 to +27
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! I see .. in include directives as a smell (even though we had some weird stuff in my previous PRs 😅)

#include <jni.h>

/*
Expand Down
2 changes: 1 addition & 1 deletion mkdocs-requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# dependencies for mkdocs documentation
mkdocs>=1.1
mkdocs-material>=7.1.3
mkdocs-material>=7.1.3
5 changes: 2 additions & 3 deletions test-suite/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ add_djinni_target(DjinniWCharTests
JAVA_NONNULL_ANNOTATION "javax.annotation.Nonnull"
IDENT_JAVA_FIELD "mFooBar"
JNI_OUT "${CMAKE_CURRENT_BINARY_DIR}/generated-src/jni"
JNI_GENERATE_MAIN false
IDENT_JNI_CLASS "NativeFooBar"
IDENT_JNI_FILE "NativeFooBar"
OBJC_OUT "${CMAKE_CURRENT_BINARY_DIR}/generated-src/objc"
Expand Down Expand Up @@ -133,10 +134,7 @@ source_group("generated-objcpp" FILES ${WCHAR_OBJCPP_GENERATED_SRCS})
source_group("generated-yaml" FILES ${WCHAR_YAML_GENERATED_SRCS})

if(DJINNI_WITH_JNI)
file(GLOB_RECURSE SUPPORT_LIB_JNI_SRCS "${CMAKE_SOURCE_DIR}/djinni/jni/*")

add_library(DjinniTestNative SHARED
${SUPPORT_LIB_JNI_SRCS}
${JNI_GENERATED_SRCS}
${CPP_GENERATED_SRCS}
${WCHAR_JNI_GENERATED_SRCS}
Expand All @@ -150,6 +148,7 @@ if(DJINNI_WITH_JNI)
${CMAKE_CURRENT_BINARY_DIR}/generated-src/cpp
${JNI_INCLUDE_DIRS}
)
target_link_libraries(DjinniTestNative PUBLIC djinni-support-lib::djinni-support-lib)

find_package(Java REQUIRED)
include(UseJava)
Expand Down
2 changes: 2 additions & 0 deletions test-suite/Djinni.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ function(add_djinni_target)
JNI_INCLUDE_CPP_PREFIX
JNI_NAMESPACE
JNI_BASE_LIB_INCLUDE_PREFIX
JNI_GENERATE_MAIN
IDENT_JNI_CLASS
IDENT_JNI_FILE

Expand Down Expand Up @@ -233,6 +234,7 @@ function(add_djinni_target)
append_if_defined(DJINNI_GENERATION_COMMAND "--jni-include-cpp-prefix" ${DJINNI_JNI_INCLUDE_CPP_PREFIX})
append_if_defined(DJINNI_GENERATION_COMMAND "--jni-namespace" ${DJINNI_JNI_NAMESPACE})
append_if_defined(DJINNI_GENERATION_COMMAND "--jni-base-lib-include-prefix" ${DJINNI_JNI_BASE_LIB_INCLUDE_PREFIX})
append_if_defined(DJINNI_GENERATION_COMMAND "--jni-generate-main" ${DJINNI_JNI_GENERATE_MAIN})
append_if_defined(DJINNI_GENERATION_COMMAND "--ident-jni-class" ${DJINNI_IDENT_JNI_CLASS})
append_if_defined(DJINNI_GENERATION_COMMAND "--ident-jni-file" ${DJINNI_IDENT_JNI_FILE})

Expand Down
1 change: 1 addition & 0 deletions test/jni_list.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
./usr/local/include/djinni/djinni_common.hpp
./usr/local/include/djinni/jni
./usr/local/include/djinni/jni/Marshal.hpp
./usr/local/include/djinni/jni/djinni_jni_main.hpp
./usr/local/include/djinni/jni/djinni_support.hpp
./usr/local/include/djinni/proxy_cache_impl.hpp
./usr/local/include/djinni/proxy_cache_interface.hpp
Expand Down