Skip to content

Commit

Permalink
Switch to Ninja for building the native runtime
Browse files Browse the repository at this point in the history
The version of `make` provided in the MSYS2 does not work well with CMake on
Windows. This makes it impossible to do incremental nativeruntme builds on
Windows.

See https://stackoverflow.com/questions/2401976 for more details on the issue.

Switch to using Ninja everywhere, which seems to be more reliable across
platforms.

PiperOrigin-RevId: 413717789
  • Loading branch information
hoisie authored and copybara-robolectric committed Dec 2, 2021
1 parent bcb31db commit ac62e00
Show file tree
Hide file tree
Showing 7 changed files with 1,412 additions and 30 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/build_native_runtime.yml
Expand Up @@ -61,8 +61,8 @@ jobs:
run: |
mkdir build
cd build
ICU_ROOT_DIR=$HOME/icu-bin cmake -B . -S ../nativeruntime/cpp
make
ICU_ROOT_DIR=$HOME/icu-bin cmake -B . -S ../nativeruntime/cpp -G Ninja
ninja
- name: Rename libnativeruntime for Linux
if: runner.os == 'Linux'
Expand Down
4 changes: 2 additions & 2 deletions nativeruntime/build.gradle
Expand Up @@ -29,7 +29,7 @@ task cmakeNativeRuntime {
mkdir "$buildDir/cpp"
exec {
workingDir "$buildDir/cpp"
commandLine 'cmake', "-B", ".", "-S","$projectDir/cpp/"
commandLine 'cmake', "-B", ".", "-S","$projectDir/cpp/", "-G", "Ninja"
}
}
}
Expand Down Expand Up @@ -80,7 +80,7 @@ task makeNativeRuntime {
doLast {
exec {
workingDir "$buildDir/cpp"
commandLine 'make'
commandLine 'ninja'
}
}
}
Expand Down
9 changes: 2 additions & 7 deletions nativeruntime/cpp/CMakeLists.txt
Expand Up @@ -3,11 +3,6 @@ cmake_minimum_required(VERSION 3.10)
# This is needed to ensure that static libraries can be linked into shared libraries.
set(CMAKE_POSITION_INDEPENDENT_CODE ON)

# Building the nativeruntime does not work with GCC due to libstddc++ linker errors.
# TODO: figure out which linker args are needed to build with GCC.
set(CMAKE_C_COMPILER "clang")
set(CMAKE_CXX_COMPILER "clang++")

# Some libutils headers require C++17
set (CMAKE_CXX_STANDARD 17)

Expand Down Expand Up @@ -100,6 +95,8 @@ target_link_libraries(androidsqlite
${STATIC_ICUI18N_LIBRARY}
${STATIC_ICUUC_LIBRARY}
${STATIC_ICUDATA_LIBRARY}
-ldl
-lpthread
)

include_directories(${JNI_INCLUDE_DIRS})
Expand Down Expand Up @@ -142,8 +139,6 @@ if (CMAKE_HOST_SYSTEM_NAME MATCHES "Linux")
target_link_libraries(nativeruntime
-static-libgcc
-static-libstdc++
-ldl
-lpthread
-Wl,--no-undefined # print an error if there are any undefined symbols
)
endif()

0 comments on commit ac62e00

Please sign in to comment.