-
Notifications
You must be signed in to change notification settings - Fork 355
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
additional Build-System features & fixes #327
Changes from 7 commits
4dc98cc
4e6c541
07fea24
74221c9
138a693
2520075
6092084
d372158
7ca8618
e5e85bc
01fe088
25b7b5c
c44946c
95d4543
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,51 @@ | ||
# Build-System CLI | ||
|
||
## Non-interactive | ||
``` | ||
python build.py -h, --help | ||
|
||
usage: build.py [-h] --target {android,linux,macos,win32} --arch {x86,x64,arm} | ||
[--vendor VENDOR] [--keep-native-libs] [--node-enabled] | ||
[--docker] [--vagrant] [--sys-image SYS_IMAGE] [--no-shutdown] | ||
[--interactive] | ||
[build-steps [build-steps ...]] | ||
``` | ||
``` | ||
python build.py -v alpine -t linux -a x64 -dkr -img openjdk:8u131-alpine -ne j2v8 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know it sounds silly but I would suggest to put more examples here for poor souls that try to understand params. |
||
``` | ||
|
||
## Interactive | ||
``` | ||
python build.py --i, --interactive | ||
|
||
entering interactive mode... | ||
|
||
[0] Docker >> android-x86 >> NODE_ENABLED | ||
[1] Docker >> android-arm >> NODE_ENABLED | ||
[2] Docker >> alpine-linux-x64 >> NODE_ENABLED | ||
[3] Docker >> linux-x64 >> NODE_ENABLED | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did you choose these ones as suggestions? Is it possible to add suggestions? I though interactive would be that I would pick target, vendor, platform, etc, interactively.. also I found a tiny bug, it is -i or --interactive, --i doesn't work There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Because not all combinations of For the current suggestions I added the ones that are working and that were important for me to test the cross-platform build.
Yes, have a look at build_system/build_configs.py
You can use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That being said, I also can imagine many ways to improve upon this feature and see what would be the best DX for the interactive CLI for devs that want to build J2V8 themselves. |
||
[4] Docker >> linux-x86 >> NODE_ENABLED | ||
[5] Vagrant >> macosx-x64 >> NODE_ENABLED | ||
[6] Vagrant >> macosx-x86 >> NODE_ENABLED | ||
[7] Native >> windows-x64 >> NODE_ENABLED | ||
[8] Docker >> windows-x64 >> NODE_ENABLED | ||
[9] Vagrant >> windows-x64 >> NODE_ENABLED | ||
|
||
Select a predefined build-configuration to run: 2 | ||
Building: Docker >> alpine-linux-x64 >> NODE_ENABLED | ||
|
||
Override build-steps ? (leave empty to run pre-configured steps): j2v8 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better use In the script itself the supported steps could be written to the console output, so you know what to choose from at that time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, I'll rather put in |
||
``` | ||
|
||
# Build-Steps | ||
|
||
The J2V8 build-system performs several build steps in a fixed order to produce the final J2V8 packages for usage on the designated target platforms. What follows is a short summary for what each of the executed build-steps does and what output artifacts are produced by each step. | ||
|
||
--- | ||
## Node.js | ||
|
||
Builds the [Node.js](https://nodejs.org/en/) & [V8](https://developers.google.com/v8/) dependency artifacts that are later linked against by the J2V8 native bridge code. | ||
Builds the [Node.js](https://nodejs.org/en/) & [V8](https://developers.google.com/v8/) dependency artifacts that are later linked into the J2V8 native bridge code. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel guilty about commenting that you did so much work but I have to say I didn't learn much from description from nodejs.py:
I still don't know how checkout nodejs source, it used to be python prepare_build.py There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I had to breat something to get commands:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This script is far from finished, I need to first get a better picture of how this would be used in a CI build before I start this kind of polishing 😉 PS: Thanks for reviewing ... once you are done I will start to see what needs fixing and how to prioritze 👍 PPS: looks like there is a feature of the args parser that I could use to improve upon that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that after my successful build I only have a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not all of those directories will be created for all platforms ... the latter three ones are only created on Win32 builds of Node.js IIRC |
||
(only works if the Node.js source was checked out into the J2V8 `./node` directory) | ||
|
||
__Inputs:__ | ||
- Node.js source code | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Checking out the complete node project is about 1.9GB. Maybe you can advise to checkout only required parts, like:
This yields 236MB |
||
|
@@ -22,7 +62,7 @@ __Artifacts:__ | |
--- | ||
## CMake | ||
|
||
Uses [CMake](https://cmake.org/) to generate the native Makefiles / IDE project files to later build the J2V8 C++ native bridge shared libraries (.so/.dylib/.dll) | ||
Uses [CMake](https://cmake.org/) to generate the native Makefiles / IDE project files to later build the J2V8 C++ native bridge shared libraries. | ||
|
||
__Inputs__: | ||
- Node.js / V8 static link libraries | ||
|
@@ -37,7 +77,7 @@ __Artifacts:__ | |
--- | ||
## JNI | ||
|
||
The previously generated Makefiles / IDE project files are used to compile and link the J2V8 C++ source code, which provides the JNI bridge to interop between the Java code and the C++ code of Node.js / V8. | ||
Compile and link the J2V8 C++ shared libraries (.so/.dylib/.dll), which provide the JNI bridge to interop with the C++ code of Node.js / V8. | ||
|
||
__Inputs__: | ||
- CMake generated Makefiles / IDE Project-files | ||
|
@@ -47,27 +87,48 @@ __Inputs__: | |
|
||
__Artifacts:__ | ||
- J2V8 native shared libraries | ||
- `./cmake.out/{platform}.{architecture}/libj2v8_{platform}_{abi}.{ext}` | ||
- e.g. `./cmake.out/linux.x64/libj2v8_linux_x86_64.so` | ||
- The built shared libraries will also be automatically copied to the required Java / Android project directories to be included in the .jar/.aar packages that will be built later. | ||
- `./src/main/resources/` (Java) | ||
- `./src/main/jniLibs/{abi}/libj2v8.so` (Android) | ||
- `./cmake.out/{platform}.{architecture}/libj2v8-[vendor-]{platform}-{abi}.{ext}` | ||
- e.g. `./cmake.out/linux.x64/libj2v8-alpine-linux-x86_64.so` | ||
--- | ||
## Optimize | ||
|
||
The native J2V8 libraries are optimized for performance and/or filesize by using the available tools of the target-platform / compiler-toolchain. | ||
|
||
__Inputs__: | ||
- <u>unoptimized</u> J2V8 native shared libraries | ||
- `./cmake.out/{platform}.{architecture}/libj2v8-[vendor-]{platform}-{abi}.{ext}` | ||
- e.g. `./cmake.out/linux.x64/libj2v8-alpine-linux-x86_64.so` | ||
- platform-specific optimization tools: | ||
- Android: - | ||
- Linux: `execstack`, `strip` | ||
- MacOSX: - | ||
- Windows: - | ||
|
||
__Artifacts:__ | ||
- <u>optimized</u> J2V8 native shared libraries | ||
- `./cmake.out/{platform}.{architecture}/libj2v8-[vendor-]{platform}-{abi}.{ext}` | ||
- e.g. `./cmake.out/linux.x64/libj2v8-alpine-linux-x86_64.so` | ||
--- | ||
## Java / Android | ||
|
||
Compiles the Java source code and packages it, including the previously built native libraries, into the final package artifacts. For the execution of this build-step [Maven](https://maven.apache.org/) (Java) or [Gradle](https://gradle.org/) (Android) are used for the respective target platforms. | ||
|
||
__Inputs__: | ||
- J2V8 native shared libraries | ||
- J2V8 native shared libraries (will be automatically copied to the required Java / Android project directories to be included in the .jar/.aar packages) | ||
- `./src/main/resources/` (Java) | ||
- `./src/main/jniLibs/{abi}/libj2v8.so` (Android) | ||
- J2V8 Java source code | ||
- `./src/main/` | ||
- J2V8 Java test source code | ||
- `./src/test/` | ||
- J2V8 build settings | ||
- `./build_settings.py` | ||
|
||
__Artifacts:__ | ||
- J2V8 platform-specific packages | ||
- Maven platform-specific packages | ||
- `./build.out/j2v8_{platform}_{abi}-{j2v8_version}.jar` | ||
- e.g. `./build.out/j2v8_linux_x86_64-4.8.0-SNAPSHOT.jar` | ||
- Gradle Android packages | ||
- `./build/outputs/aar/j2v8-release.aar` | ||
--- | ||
## JUnit | ||
|
@@ -80,8 +141,8 @@ __Inputs__: | |
- `./src/test/` | ||
|
||
__Artifacts:__ | ||
- Maven Surefire test reports | ||
- Maven Surefire test reports (Desktop platforms) | ||
- `./target/surefire-reports/` | ||
- Gradle connected-test reports | ||
- `./build/outputs/androidTest-results/connected/` | ||
- Gradle Spoon test reports (Android only) | ||
- `./build/spoon/debug/` | ||
--- |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,82 +15,131 @@ endif() | |
cmake_minimum_required(VERSION 3.6) | ||
project(j2v8) | ||
|
||
# adding cmake directory for includes | ||
set(CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake) | ||
|
||
# set up the module path | ||
set(CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake) | ||
set(CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/cmake) | ||
|
||
include(BuildUtils) | ||
include(NodeJsUtils) | ||
include(Policies) | ||
|
||
#----------------------------------------------------------------------- | ||
# DEPENDENCY SETTINGS / CMAKE OPTIONS | ||
#----------------------------------------------------------------------- | ||
|
||
# look for dependencies | ||
find_package(Java) | ||
|
||
# j2v8 dependency options | ||
set(J2V8_JDK_DIR ${Java_ROOT} CACHE STRING "Path to the Java JDK dependency") | ||
set(J2V8_NODEJS_DIR "${CMAKE_SOURCE_DIR}/node" CACHE STRING "Path to the Node.js dependency") | ||
|
||
# get the required Node.js link libraries | ||
get_njs_libs(${J2V8_NODEJS_DIR} "Debug") | ||
get_njs_libs(${J2V8_NODEJS_DIR} "Release") | ||
|
||
# j2v8 build options | ||
set(J2V8_TARGET_ARCH "" CACHE STRING "The target architecture for the build.") | ||
option(J2V8_NODE_ENABLED "Build the J2V8 native bridge with Node.js support enabled" ON) | ||
option(J2V8_BUILD_ONLY_DEBUG_RELEASE "Generate only Debug and Release configurations (exclude RelWithDebInfo and MinSizeRel)" ON) | ||
|
||
if(CMAKE_SYSTEM_NAME STREQUAL "Windows" AND MSVC) | ||
option(J2V8_LINK_WITH_STATIC_MSVCRT "Link against the static version of the Microsoft Visual C++ Common Runtime (will link against the dynamic DLL version if this option is disabled)" ON) | ||
endif() | ||
|
||
#----------------------------------------------------------------------- | ||
# BUILD PLATFORM SETUP & VARIABLES | ||
#----------------------------------------------------------------------- | ||
|
||
# HINT: CMake Multiarchitecture Compilation | ||
# see: https://stackoverflow.com/a/5359572/425532 | ||
|
||
if("${J2V8_TARGET_ARCH}" STREQUAL "") | ||
message (FATAL_ERROR "J2V8_TARGET_ARCH not specified") | ||
endif() | ||
|
||
if(J2V8_TARGET_ARCH STREQUAL "x86_64") | ||
set(J2V8_BUILD_X64 TRUE) | ||
endif() | ||
|
||
if(CMAKE_SYSTEM_NAME STREQUAL "Android") | ||
#{ | ||
set(JAVA_PLATFORM_NAME "android") | ||
|
||
# output library filename | ||
set(J2V8_LIB_PLATFORM_NAME "android") | ||
# output library filename parts | ||
set(J2V8_LIB_PREFIX "") | ||
set(J2V8_LIB_ARCH_NAME ${CMAKE_ANDROID_ARCH_ABI}) | ||
set(J2V8_LIB_VENDOR_NAME "") | ||
set(J2V8_LIB_PLATFORM_NAME "android") | ||
#} | ||
elseif(CMAKE_SYSTEM_NAME STREQUAL "Linux") | ||
#{ | ||
set(JAVA_PLATFORM_NAME "linux") | ||
|
||
# output library filename | ||
set(J2V8_LIB_PLATFORM_NAME "linux") | ||
# output library filename parts | ||
set(J2V8_LIB_PREFIX "") | ||
set(J2V8_LIB_ARCH_NAME "x86") | ||
set(J2V8_LIB_ARCH_NAME ${J2V8_TARGET_ARCH}) | ||
set(J2V8_LIB_VENDOR_NAME "") | ||
set(J2V8_LIB_PLATFORM_NAME "linux") | ||
|
||
if(J2V8_VENDOR) | ||
set(J2V8_LIB_VENDOR_NAME "-${J2V8_VENDOR}") | ||
endif() | ||
|
||
# configure library architecture | ||
if(J2V8_BUILD_X64) | ||
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -m64 ") | ||
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -m64 ") | ||
else() | ||
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -m32 ") | ||
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -m32 ") | ||
endif() | ||
|
||
# -lrt ... see: https://github.com/eclipsesource/J2V8/issues/292 | ||
set (j2v8_Debug_libs "-lrt") | ||
set (j2v8_Release_libs"-lrt") | ||
#} | ||
elseif(CMAKE_SYSTEM_NAME STREQUAL "Darwin") | ||
#{ | ||
set(JAVA_PLATFORM_NAME "darwin") | ||
|
||
# output library filename | ||
set(J2V8_LIB_PLATFORM_NAME "macosx") | ||
# output library filename parts | ||
set(J2V8_LIB_PREFIX "") | ||
set(J2V8_LIB_ARCH_NAME "x86") | ||
set(J2V8_LIB_ARCH_NAME ${J2V8_TARGET_ARCH}) | ||
set(J2V8_LIB_VENDOR_NAME "") | ||
set(J2V8_LIB_PLATFORM_NAME "macosx") | ||
|
||
# configure library architecture | ||
if(J2V8_BUILD_X64) | ||
set(CMAKE_OSX_ARCHITECTURES "x86_64") | ||
else() | ||
set(CMAKE_OSX_ARCHITECTURES "i386") | ||
|
||
# fix for 32-bit linking error "ld: illegal text reloc" | ||
# see: https://stackoverflow.com/a/9322458/425532 | ||
set(CMAKE_SHARED_LINKER_FLAGS "-read_only_relocs suppress") | ||
endif() | ||
#} | ||
elseif(CMAKE_SYSTEM_NAME STREQUAL "Windows") | ||
#{ | ||
set(JAVA_PLATFORM_NAME "win32") | ||
|
||
# output library filename | ||
set(J2V8_LIB_PLATFORM_NAME "win32") | ||
# output library filename parts | ||
set(J2V8_LIB_PREFIX "lib") | ||
set(J2V8_LIB_ARCH_NAME "x86") | ||
set(J2V8_LIB_ARCH_NAME ${J2V8_TARGET_ARCH}) | ||
set(J2V8_LIB_VENDOR_NAME "") | ||
set(J2V8_LIB_PLATFORM_NAME "windows") | ||
#} | ||
endif() | ||
|
||
#----------------------------------------------------------------------- | ||
# DEPENDENCY SETTINGS / CMAKE OPTIONS | ||
#----------------------------------------------------------------------- | ||
|
||
# look for dependencies | ||
find_package(Java) | ||
|
||
# j2v8 dependency options | ||
set(J2V8_JDK_DIR ${Java_ROOT} CACHE STRING "Path to the Java JDK dependency") | ||
set(J2V8_NODEJS_DIR "${CMAKE_SOURCE_DIR}/node" CACHE STRING "Path to the Node.js dependency") | ||
|
||
# get the required Node.js link libraries | ||
get_njs_libs(${J2V8_NODEJS_DIR} "Debug") | ||
get_njs_libs(${J2V8_NODEJS_DIR} "Release") | ||
|
||
# j2v8 build options | ||
option(J2V8_NODE_COMPATIBLE "Build the J2V8 native bridge with Node.js support enabled" ON) | ||
option(J2V8_BUILD_ONLY_DEBUG_RELEASE "Generate only Debug and Release configurations (exclude RelWithDebInfo and MinSizeRel)" ON) | ||
|
||
if(CMAKE_SYSTEM_NAME STREQUAL "Windows" AND MSVC) | ||
#{ | ||
option(J2V8_LINK_WITH_STATIC_MSVCRT "Link against the static version of the Microsoft Visual C++ Common Runtime (will link against the dynamic DLL version if this option is disabled)" ON) | ||
#} | ||
endif() | ||
message("--------------------------------------------------") | ||
message("J2V8_LIB_ARCH_NAME = ${J2V8_LIB_ARCH_NAME}") | ||
message("J2V8_LIB_VENDOR_NAME = ${J2V8_LIB_VENDOR_NAME}") | ||
message("J2V8_LIB_PLATFORM_NAME = ${J2V8_LIB_PLATFORM_NAME}") | ||
message("J2V8_TARGET_ARCH = ${J2V8_TARGET_ARCH}") | ||
message("J2V8_BUILD_X64 = ${J2V8_BUILD_X64}") | ||
message("--------------------------------------------------") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One thing that I am missing, that could be either here or in a 'success' message at the end, is the V8 and NodeJS versions that have been compiled into the final output (e.g. the They are not user-selectable versions, but having them hard-coded makes it more important to report what you'll end up with after build completes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, I will try to include this information even earlier in the build process, so a user can abort as early as possible if he meant to build something else. I am thinking about this, because now that most features that I had planned are done, I am trying to think about the Developer-Experience and one part of that is the verbosity of the build output ... there is already a lot of stuff happening if one runs a complete build for a platform with all the build-steps involved. Two lines for the Node.js/V8 versions of course won't make the big difference there, but at least I wanted to let everyone know that I am thinking about this aspect now also. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would go with a message right at the start. Let's hit those devs with those 2 lines :D |
||
message("J2V8_NODE_ENABLED = ${J2V8_NODE_ENABLED}") | ||
message("--------------------------------------------------") | ||
|
||
#----------------------------------------------------------------------- | ||
# INCLUDE DIRECTORIES & SOURCE FILES | ||
|
@@ -138,9 +187,7 @@ endif() | |
|
||
# remove the MinSizeRel and RelWithDebInfo configurations | ||
if(J2V8_BUILD_ONLY_DEBUG_RELEASE) | ||
#{ | ||
set(CMAKE_CONFIGURATION_TYPES "Debug;Release" CACHE STRING "limited configs" FORCE) | ||
#} | ||
endif() | ||
|
||
# link against the static MS C++ runtime libraries | ||
|
@@ -152,10 +199,12 @@ endif() | |
add_library(j2v8 SHARED ${src_files}) | ||
|
||
# enable Node.js if requested by the build options above | ||
if(J2V8_NODE_COMPATIBLE) | ||
#{ | ||
if(J2V8_NODE_ENABLED) | ||
set_property(TARGET j2v8 PROPERTY COMPILE_DEFINITIONS ${COMPILE_DEFINITIONS} NODE_COMPATIBLE=1) | ||
#} | ||
endif() | ||
|
||
if(CMAKE_SYSTEM_NAME STREQUAL "Windows" AND MSVC) | ||
set_property(TARGET j2v8 APPEND_STRING PROPERTY LINK_FLAGS_RELEASE "/LTCG") | ||
endif() | ||
|
||
# build output directory | ||
|
@@ -166,18 +215,13 @@ include_directories(${include_dirs}) | |
|
||
# link the necessary libraries | ||
target_link_libraries(j2v8 | ||
debug "${njs_Debug_libs}" | ||
optimized "${njs_Release_libs}" | ||
debug "${njs_Debug_libs}" "${j2v8_Debug_libs}" | ||
optimized "${njs_Release_libs}" "${j2v8_Release_libs}" | ||
) | ||
|
||
#----------------------------------------------------------------------- | ||
# OUTPUT SETTINGS & POST-BUILD | ||
#----------------------------------------------------------------------- | ||
|
||
# apply lib suffix if building a 64 bit target | ||
if(CMAKE_CL_64 OR CMAKE_SIZEOF_VOID_P EQUAL 8) | ||
set(J2V8_LIB_ARCH_NAME "${J2V8_LIB_ARCH_NAME}_64") | ||
endif() | ||
|
||
# set library output filename | ||
set_target_properties(j2v8 PROPERTIES OUTPUT_NAME "${PROJECT_NAME}_${J2V8_LIB_PLATFORM_NAME}_${J2V8_LIB_ARCH_NAME}") | ||
set_target_properties(j2v8 PROPERTIES OUTPUT_NAME "${J2V8_LIB_PREFIX}${PROJECT_NAME}${J2V8_LIB_VENDOR_NAME}-${J2V8_LIB_PLATFORM_NAME}-${J2V8_LIB_ARCH_NAME}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see -v in the list of description here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-v is just the short form for the --vendor switch, I haven't yet seen a way to show both in the python args parser generated help