Add flag for whether patches should be committed (#1653)

Signed-off-by: John Mazanec <[email protected]>

DEVELOPER_GUIDE.md

@@ -237,6 +237,13 @@ If you want to make a custom patch on JNI library
 3. Place the patch file under `jni/patches`
 4. Make a change in `jni/CmakeLists.txt`, `.github/workflows/CI.yml` to apply the patch during build
 
+By default, in the cmake build system, these patches will be applied and committed to the native libraries. In order to 
+successfully make the commits the `user.name` and `user.email` git configurations need to be setup. If you cannot set 
+these in your environment, you can disable committing the changes to the library by passing gradle this flag: 
+`build.lib.commit_patches=false`. For example, `gradlew build -Dbuild.lib.commit_patches=false`. If the patches are 
+not committed, then the full library build process will run each time `cmake` is invoked. In a development environment, 
+it is recommended to setup the user git configuration to avoid this cost.
+
 ### Enable SIMD Optimization
 SIMD(Single Instruction/Multiple Data) Optimization is enabled by default on Linux and Mac which boosts the performance
 by enabling `AVX2` on `x86 architecture` and `NEON` on `ARM64 architecture` while building the Faiss library. But to enable SIMD, the underlying processor

build.gradle

@@ -18,6 +18,9 @@ buildscript {
         opensearch_group = "org.opensearch"
         isSnapshot = "true" == System.getProperty("build.snapshot", "true")
         simd_enabled = System.getProperty("simd.enabled", "true")
+        // Flag to determine whether cmake build system should apply patches and commit. In automated build environments
+        // set this to false. In dev environments, set to true
+        commit_lib_patches = System.getProperty("build.lib.commit_patches", "true")
 
         version_tokens = opensearch_version.tokenize('-')
         opensearch_build = version_tokens[0] + '.0'
@@ -303,10 +306,10 @@ task cmakeJniLib(type:Exec) {
     workingDir 'jni'
     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", "-DSIMD_ENABLED=${simd_enabled}"
+        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", "-DSIMD_ENABLED=${simd_enabled}", "-DCOMMIT_LIB_PATCHES=${commit_lib_patches}"
     }
     else {
-        commandLine 'cmake', '.', "-DKNN_PLUGIN_VERSION=${opensearch_version}", "-DSIMD_ENABLED=${simd_enabled}"
+        commandLine 'cmake', '.', "-DKNN_PLUGIN_VERSION=${opensearch_version}", "-DSIMD_ENABLED=${simd_enabled}", "-DCOMMIT_LIB_PATCHES=${commit_lib_patches}"
     }
 }
 

jni/CMakeLists.txt

@@ -31,6 +31,18 @@ else()
     set(CONFIG_ALL OFF)
 endif ()
 
+# `git am` will create commits from the patches in the native libraries. This is ideal for development envs
+# because it prevents full lib rebuild everytime cmake is run. However, for build systems that will run the
+# build workflow once, it can cause issues because git commits require that the user and the user's email be set.
+# See https://github.com/opensearch-project/k-NN/issues/1651. So, we provide a flag that allows users to select between
+# the two
+if(NOT DEFINED COMMIT_LIB_PATCHES OR ${COMMIT_LIB_PATCHES} STREQUAL true)
+    set(GIT_PATCH_COMMAND am)
+else()
+    set(GIT_PATCH_COMMAND apply)
+endif()
+message(STATUS "Using the following git patch command: \"${GIT_PATCH_COMMAND}\"")
+
 # Set OS specific variables
 if (${CMAKE_SYSTEM_NAME} STREQUAL Darwin)
     set(CMAKE_MACOSX_RPATH 1)

jni/cmake/init-faiss.cmake

@@ -18,8 +18,8 @@ find_path(PATCH_FILE NAMES 0001-Custom-patch-to-support-multi-vector.patch 0002-
 # If it exists, apply patches
 if (EXISTS ${PATCH_FILE})
     message(STATUS "Applying custom patches.")
-    execute_process(COMMAND git am --3way ${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0001-Custom-patch-to-support-multi-vector.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE)
-    execute_process(COMMAND git am --3way ${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0002-Enable-precomp-table-to-be-shared-ivfpq.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE)
+    execute_process(COMMAND git ${GIT_PATCH_COMMAND} --3way --ignore-space-change --ignore-whitespace ${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0001-Custom-patch-to-support-multi-vector.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE)
+    execute_process(COMMAND git ${GIT_PATCH_COMMAND} --3way --ignore-space-change --ignore-whitespace ${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0002-Enable-precomp-table-to-be-shared-ivfpq.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE)
     if(RESULT_CODE)
         message(FATAL_ERROR "Failed to apply patch:\n${ERROR_MSG}")
     endif()

jni/cmake/init-nmslib.cmake

@@ -18,7 +18,7 @@ find_path(PATCH_FILE NAMES 0001-Initialize-maxlevel-during-add-from-enterpoint-l
 # If it exists, apply patches
 if (EXISTS ${PATCH_FILE})
     message(STATUS "Applying custom patches.")
-    execute_process(COMMAND git am --3way ${CMAKE_CURRENT_SOURCE_DIR}/patches/nmslib/0001-Initialize-maxlevel-during-add-from-enterpoint-level.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE)
+    execute_process(COMMAND git ${GIT_PATCH_COMMAND} --3way --ignore-space-change --ignore-whitespace ${CMAKE_CURRENT_SOURCE_DIR}/patches/nmslib/0001-Initialize-maxlevel-during-add-from-enterpoint-level.patch WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib ERROR_VARIABLE ERROR_MSG RESULT_VARIABLE RESULT_CODE)
 
     if(RESULT_CODE)
         message(FATAL_ERROR "Failed to apply patch:\n${ERROR_MSG}")

scripts/build.sh

@@ -118,12 +118,12 @@ fi
 
 # Build k-NN lib and plugin through gradle tasks
 cd $work_dir
-./gradlew build --no-daemon --refresh-dependencies -x integTest -x test -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER
-./gradlew :buildJniLib -Dsimd.enabled=false
+./gradlew build --no-daemon --refresh-dependencies -x integTest -x test -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER -Dbuild.lib.commit_patches=false
+./gradlew :buildJniLib -Dsimd.enabled=false -Dbuild.lib.commit_patches=false
 
 if [ "$PLATFORM" != "windows" ] && [ "$ARCHITECTURE" = "x64" ]; then
   echo "Building k-NN library after enabling AVX2"
-  ./gradlew :buildJniLib -Dsimd.enabled=true
+  ./gradlew :buildJniLib -Dsimd.enabled=true -Dbuild.lib.commit_patches=false
 fi
 
 ./gradlew publishPluginZipPublicationToZipStagingRepository -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER