diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index f9e914099..438a9ac49 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -17,7 +17,7 @@ jobs: matrix: java: [11, 17] - name: Build and Test k-NN Plugin + name: Build and Test k-NN Plugin on Ubuntu runs-on: ubuntu-latest steps: @@ -42,6 +42,42 @@ jobs: with: token: ${{ secrets.CODECOV_TOKEN }} + Build-k-NN-Windows: + strategy: + matrix: + java: [ 11, 17 ] + + name: Build and Test k-NN Plugin on Windows + runs-on: windows-latest + + steps: + - name: Checkout k-NN + uses: actions/checkout@v1 + + - name: Setup Java ${{ matrix.java }} + uses: actions/setup-java@v1 + with: + java-version: ${{ matrix.java }} + + - name: Install MinGW Using Scoop + run: | + iex "& {$(irm get.scoop.sh)} -RunAsAdmin" + scoop bucket add main + scoop install mingw + + - name: Add MinGW to PATH + run: | + echo "C:/Users/runneradmin/scoop/apps/mingw/current/bin" >> $env:GITHUB_PATH + refreshenv + + - name: Run build + run: | + ./gradlew.bat build + - name: Upload Coverage Report + uses: codecov/codecov-action@v1 + with: + token: ${{ secrets.CODECOV_TOKEN }} + # - name: Pull and Run Docker for security tests # run: | # plugin=`ls build/distributions/*.zip` diff --git a/build-tools/knnplugin-coverage.gradle b/build-tools/knnplugin-coverage.gradle index 57a0eeeec..a304e5255 100644 --- a/build-tools/knnplugin-coverage.gradle +++ b/build-tools/knnplugin-coverage.gradle @@ -16,6 +16,8 @@ * cluster is stopped and dump it to a file. Luckily our current security policy seems to allow this. This will also probably * break if there are multiple nodes in the integTestCluster. But for now... it sorta works. */ + +import org.apache.tools.ant.taskdefs.condition.Os apply plugin: 'jacoco' // Get gradle to generate the required jvm agent arg for us using a dummy tasks of type Test. Unfortunately Elastic's @@ -63,7 +65,14 @@ afterEvaluate { jacocoTestReport.dependsOn integTest testClusters.integTest { - jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}".replace('javaagent:','javaagent:/') + if (Os.isFamily(Os.FAMILY_WINDOWS)) { + + // Replacing build with absolute path to fix the error "error opening zip file or JAR manifest missing : /build/tmp/expandedArchives/..../jacocoagent.jar" + jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}".replace('build',"${buildDir}") + } else { + jvmArgs " ${dummyIntegTest.jacoco.getAsJvmArg()}".replace('javaagent:','javaagent:/') + } + systemProperty 'com.sun.management.jmxremote', "true" systemProperty 'com.sun.management.jmxremote.authenticate', "false" systemProperty 'com.sun.management.jmxremote.port', "7777" diff --git a/build.gradle b/build.gradle index fe1762fb3..00f6644bc 100644 --- a/build.gradle +++ b/build.gradle @@ -4,6 +4,7 @@ */ import org.opensearch.gradle.test.RestIntegTestTask +import org.apache.tools.ant.taskdefs.condition.Os buildscript { ext { @@ -170,9 +171,19 @@ dependencies { def opensearch_tmp_dir = rootProject.file('build/private/opensearch_tmp').absoluteFile opensearch_tmp_dir.mkdirs() +task windowsPatches(type:Exec) { + commandLine 'cmd', '/c', "Powershell -File $rootDir\\scripts\\windowsScript.ps1" +} + task cmakeJniLib(type:Exec) { workingDir 'jni' - commandLine 'cmake', '.', "-DKNN_PLUGIN_VERSION=${opensearch_version}" + if (Os.isFamily(Os.FAMILY_WINDOWS)) { + dependsOn windowsPatches + commandLine 'cmake', '.', "-G", "Unix Makefiles", "-DKNN_PLUGIN_VERSION=${opensearch_version}", "-DBLAS_LIBRARIES=$rootDir\\src\\main\\resources\\windowsDependencies\\libopenblas.dll", "-DLAPACK_LIBRARIES=$rootDir\\src\\main\\resources\\windowsDependencies\\libopenblas.dll" + } + else { + commandLine 'cmake', '.', "-DKNN_PLUGIN_VERSION=${opensearch_version}" + } } task buildJniLib(type:Exec) { @@ -185,6 +196,11 @@ test { dependsOn buildJniLib systemProperty 'tests.security.manager', 'false' systemProperty "java.library.path", "$rootDir/jni/release" + if (Os.isFamily(Os.FAMILY_WINDOWS)) { + + // Add the paths of built JNI libraries and its dependent libraries to PATH variable in System variables + environment('PATH', System.getenv('PATH') + ";$rootDir/jni/release" + ";$rootDir/src/main/resources/windowsDependencies") + } } def _numNodes = findProperty('numNodes') as Integer ?: 1 @@ -225,6 +241,12 @@ integTest { testClusters.integTest { testDistribution = "ARCHIVE" plugin(project.tasks.bundlePlugin.archiveFile) + if (Os.isFamily(Os.FAMILY_WINDOWS)) { + + // Add the paths of built JNI libraries and its dependent libraries to PATH variable in System variables + environment('PATH', System.getenv('PATH') + ";$rootDir/jni/release" + ";$rootDir/src/main/resources/windowsDependencies") + } + // Cluster shrink exception thrown if we try to set numberOfNodes to 1, so only apply if > 1 if (_numNodes > 1) numberOfNodes = _numNodes // When running integration tests it doesn't forward the --debug-jvm to the cluster anymore diff --git a/patches/windows/CMakeLists.patch b/patches/windows/CMakeLists.patch new file mode 100644 index 000000000..7c8390fae --- /dev/null +++ b/patches/windows/CMakeLists.patch @@ -0,0 +1,119 @@ +Index: jni/CMakeLists.txt +<+>UTF-8 +=================================================================== +Description: +We are making the following changes to jni/CMakeLists.txt file to build it on Windows OS +1. Set CXX_COMPILER_VERSION and CMAKE_CXX_FLAGS. +2. Add a new if else case when the CMAKE_SYSTEM_NAME matches Windows and set flags JVM_OS_TYPE, prefix and extension for the target libraries that are built. +3. Replace LIBRARY_OUTPUT_DIRECTORY with RUNTIME_OUTPUT_DIRECTORY, to build the target libraries(opensearchknn_common, opensearchknn_nmslib and opensearchknn_faiss) in the +specified directory at runtime. +4. Comment the TESTS for now because the tests are failing(failing to build jni_tests.exe) if we are building our target libraries as SHARED libraries. + +TODO: Fix the failing JNI TESTS + +=================================================================== +diff --git a/jni/CMakeLists.txt b/jni/CMakeLists.txt +--- a/jni/CMakeLists.txt (revision 78a2b5b2a83db52aded56ab74940a62c94a88b50) ++++ b/jni/CMakeLists.txt (date 1666128659873) +@@ -14,8 +14,10 @@ + set(TARGET_LIB_FAISS opensearchknn_faiss) # faiss JNI + set(TARGET_LIBS "") # Libs to be installed + ++set(CXX_COMPILER_VERSION ${CMAKE_CXX_COMPILER_VERSION}) + set(CMAKE_CXX_STANDARD 11) + set(CMAKE_CXX_STANDARD_REQUIRED True) ++set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fpermissive") + + option(CONFIG_FAISS "Configure faiss library build when this is on") + option(CONFIG_NMSLIB "Configure nmslib library build when this is on") +@@ -35,6 +37,11 @@ + elseif(${CMAKE_SYSTEM_NAME} STREQUAL Linux) + set(JVM_OS_TYPE linux) + set(LIB_EXT .so) ++elseif(${CMAKE_SYSTEM_NAME} STREQUAL Windows) ++ set(JVM_OS_TYPE win32) ++ set(LIB_EXT .dll) ++ set(CMAKE_SHARED_LIBRARY_PREFIX "") ++ set(CMAKE_STATIC_LIBRARY_PREFIX "") + else() + message(FATAL_ERROR "Unable to run on system: ${CMAKE_SYSTEM_NAME}") + endif() +@@ -57,7 +64,7 @@ + target_include_directories(${TARGET_LIB_COMMON} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include $ENV{JAVA_HOME}/include $ENV{JAVA_HOME}/include/${JVM_OS_TYPE}) + set_target_properties(${TARGET_LIB_COMMON} PROPERTIES SUFFIX ${LIB_EXT}) + set_target_properties(${TARGET_LIB_COMMON} PROPERTIES POSITION_INDEPENDENT_CODE ON) +-set_target_properties(${TARGET_LIB_COMMON} PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release) ++set_target_properties(${TARGET_LIB_COMMON} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release) + list(APPEND TARGET_LIBS ${TARGET_LIB_COMMON}) + # ---------------------------------------------------------------------------- + +@@ -79,7 +86,7 @@ + target_include_directories(${TARGET_LIB_NMSLIB} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include $ENV{JAVA_HOME}/include $ENV{JAVA_HOME}/include/${JVM_OS_TYPE} ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib/similarity_search/include) + set_target_properties(${TARGET_LIB_NMSLIB} PROPERTIES SUFFIX ${LIB_EXT}) + set_target_properties(${TARGET_LIB_NMSLIB} PROPERTIES POSITION_INDEPENDENT_CODE ON) +- set_target_properties(${TARGET_LIB_NMSLIB} PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release) ++ set_target_properties(${TARGET_LIB_NMSLIB} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release) + + list(APPEND TARGET_LIBS ${TARGET_LIB_NMSLIB}) + endif () +@@ -130,7 +137,7 @@ + target_include_directories(${TARGET_LIB_FAISS} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/include $ENV{JAVA_HOME}/include $ENV{JAVA_HOME}/include/${JVM_OS_TYPE} ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss) + set_target_properties(${TARGET_LIB_FAISS} PROPERTIES SUFFIX ${LIB_EXT}) + set_target_properties(${TARGET_LIB_FAISS} PROPERTIES POSITION_INDEPENDENT_CODE ON) +- set_target_properties(${TARGET_LIB_FAISS} PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release) ++ set_target_properties(${TARGET_LIB_FAISS} PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/release) + + list(APPEND TARGET_LIBS ${TARGET_LIB_FAISS}) + endif () +@@ -138,51 +145,7 @@ + # --------------------------------------------------------------------------- + + # --------------------------------- TESTS ----------------------------------- +-if (${CONFIG_ALL} STREQUAL ON OR ${CONFIG_TEST} STREQUAL ON) +- # Reference - https://crascit.com/2015/07/25/cmake-gtest/ +- configure_file(CMakeLists.txt.in googletest-download/CMakeLists.txt) +- execute_process(COMMAND "${CMAKE_COMMAND}" -G "${CMAKE_GENERATOR}" . +- WORKING_DIRECTORY "${CMAKE_BINARY_DIR}/googletest-download" +- ) +- execute_process(COMMAND "${CMAKE_COMMAND}" --build . +- WORKING_DIRECTORY "${CMAKE_BINARY_DIR}/googletest-download" +- ) +- set(gtest_force_shared_crt ON CACHE BOOL "" FORCE) +- +- add_subdirectory("${CMAKE_BINARY_DIR}/googletest-src" +- "${CMAKE_BINARY_DIR}/googletest-build" EXCLUDE_FROM_ALL +- ) +- add_executable( +- jni_test +- tests/faiss_wrapper_test.cpp +- tests/nmslib_wrapper_test.cpp +- tests/test_util.cpp) +- +- target_link_libraries( +- jni_test +- gtest_main +- gmock_main +- faiss +- NonMetricSpaceLib +- OpenMP::OpenMP_CXX +- ${TARGET_LIB_FAISS} +- ${TARGET_LIB_NMSLIB} +- ${TARGET_LIB_COMMON} +- ) +- +- target_include_directories(jni_test PRIVATE +- ${CMAKE_CURRENT_SOURCE_DIR}/tests +- ${CMAKE_CURRENT_SOURCE_DIR}/include +- $ENV{JAVA_HOME}/include +- $ENV{JAVA_HOME}/include/${JVM_OS_TYPE} +- ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss +- ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib/similarity_search/include +- ${gtest_SOURCE_DIR}/include +- ${gmock_SOURCE_DIR}/include) + +- +- set_target_properties(jni_test PROPERTIES RUNTIME_OUTPUT_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/bin) +-endif () + # --------------------------------------------------------------------------- + + # -------------------------------- INSTALL ---------------------------------- diff --git a/scripts/windowsScript.ps1 b/scripts/windowsScript.ps1 new file mode 100644 index 000000000..c271c5219 --- /dev/null +++ b/scripts/windowsScript.ps1 @@ -0,0 +1,23 @@ +# +# Copyright OpenSearch Contributors +# SPDX-License-Identifier: Apache-2.0 +# + +git submodule update --init -- jni/external/nmslib +git submodule update --init -- jni/external/faiss + +git apply patches/windows/CMakeLists.patch --verbose + +# Validating if the CMakeLists patch is applied +echo "Validating if the CMakeLists patch is applied" +type jni/CMakeLists.txt | Select-String "Windows" + +# Replace '_MSC_VER' with '__MINGW32__' +(Get-Content jni/external/faiss/faiss/impl/index_read.cpp).replace('_MSC_VER', '__MINGW32__') | Set-Content jni/external/faiss/faiss/impl/index_read.cpp +(Get-Content jni/external/faiss/faiss/impl/index_write.cpp).replace('_MSC_VER', '__MINGW32__') | Set-Content jni/external/faiss/faiss/impl/index_write.cpp + +# Replace '#include ' with +# #ifndef __MINGW32__ +# #include +# #endif +(Get-Content jni/external/faiss/faiss/OnDiskInvertedLists.cpp).replace('#include ', "#ifndef __MINGW32__`n#include `n#endif") | Set-Content jni/external/faiss/faiss/OnDiskInvertedLists.cpp diff --git a/src/main/resources/windowsDependencies/libopenblas.dll b/src/main/resources/windowsDependencies/libopenblas.dll new file mode 100644 index 000000000..f5db4624f Binary files /dev/null and b/src/main/resources/windowsDependencies/libopenblas.dll differ