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.

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

Merged
merged 14 commits into from
Aug 29, 2017
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 8 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,17 @@ hs_err*.log
*.iws
.idea

# python binaries
# Python binaries
*.pyc

# Build input/output.
# build input/output
node
build.out
cmake.out
node.out

# test input/output
test-mockup-os-release

# generated dependency packages
*.tar.*
87 changes: 74 additions & 13 deletions BUILDING.md
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]
Copy link
Contributor

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...

Copy link
Contributor Author

@drywolf drywolf Aug 2, 2017

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

[--interactive]
[build-steps [build-steps ...]]
```
```
python build.py -v alpine -t linux -a x64 -dkr -img openjdk:8u131-alpine -ne j2v8
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why did you choose these ones as suggestions?
I though interactive would be that I would pick target, vendor, platform, etc, interactively..

Because not all combinations of platforms * architectures * vendors * virtualization-tech * feature-variations[N] are currently technically possible nor would it be sensible to try and support them all.

For the current suggestions I added the ones that are working and that were important for me to test the cross-platform build.

Is it possible to add suggestions?

Yes, have a look at build_system/build_configs.py

also I found a tiny bug, it is -i or --interactive, --i doesn't work

You can use -i or --interactive ... they are both doing the same thing (they are aliases)

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
But that would be stuff for a next PR ... this one is already bloated enough 😆

[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
Copy link

Choose a reason for hiding this comment

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

Better use all instead of j2v8 because you didn't explain yet what that means.

In the script itself the supported steps could be written to the console output, so you know what to choose from at that time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, I'll rather put in nodejs j2v8 test as an example, as it better illustrates that one can specify multiple steps here.

```

# 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.
Copy link
Contributor

Choose a reason for hiding this comment

The 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:

➜  J2V8 git:(drywolf-build-sys-features) ✗ python nodejs.py -h
usage: nodejs.py [-h] command

positional arguments:
  command

optional arguments:
  -h, --help  show this help message and exit

I still don't know how checkout nodejs source, it used to be python prepare_build.py

Copy link
Contributor

Choose a reason for hiding this comment

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

I had to breat something to get commands:

➜  J2V8 git:(drywolf-build-sys-features) ✗ python nodejs.py 7.4.0
usage: nodejs.py [-h] command
nodejs.py: error: argument command: invalid choice: '7.4.0' (choose from 'flush-cache', 'fc', 'git-init', 'gi', 'package', 'pkg', 'store-diff', 'sd', 'apply-diff', 'ad')

Copy link
Contributor Author

@drywolf drywolf Aug 2, 2017

Choose a reason for hiding this comment

The 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 😉
But I agree that this help output is pretty useless even at this point, I will add some preliminary help for now.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Choose a reason for hiding this comment

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

Note that after my successful build I only have a ./node/out directory. The other ones listed here are non-existing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
I will add some remarks about that to the documentation 👍

(only works if the Node.js source was checked out into the J2V8 `./node` directory)

__Inputs:__
- Node.js source code

Choose a reason for hiding this comment

The 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:

git clone --depth=2 -b v8.x https://github.com/nodejs/node.git

This yields 236MB

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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/`
---
148 changes: 96 additions & 52 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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("--------------------------------------------------")

Choose a reason for hiding this comment

The 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 .aar).

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.

Copy link
Contributor Author

@drywolf drywolf Aug 17, 2017

Choose a reason for hiding this comment

The 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 not sure though if this information should always be displayed or if it should be only displayed when an extra command line switch is present or some kind of verbosity level is set.

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.

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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}")