Add flag for whether patches should be committed

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

.github/workflows/CI.yml

@@ -38,12 +38,6 @@ jobs:
         with:
           submodules: true
 
-      # Setup git user so that patches for native libraries can be applied and committed
-      - name: Setup git user
-        run: |
-          su `id -un 1000` -c 'git config --global user.name "github-actions[bot]"'
-          su `id -un 1000` -c 'git config --global user.email "github-actions[bot]@users.noreply.github.com"'
-
       - name: Setup Java ${{ matrix.java }}
         uses: actions/setup-java@v1
         with:
@@ -56,10 +50,10 @@ jobs:
           if lscpu  | grep -i avx2
           then
             echo "avx2 available on system"
-            su `id -un 1000` -c "whoami && java -version && ./gradlew build"
+            su `id -un 1000` -c "whoami && java -version && ./gradlew build -Dbuild.lib.commit_patches=false"
           else
             echo "avx2 not available on system"
-            su `id -un 1000` -c "whoami && java -version && ./gradlew build -Dsimd.enabled=false"
+            su `id -un 1000` -c "whoami && java -version && ./gradlew build -Dsimd.enabled=false -Dbuild.lib.commit_patches=false"
           fi  
           
 
@@ -81,12 +75,6 @@ jobs:
       - name: Checkout k-NN
         uses: actions/checkout@v1
 
-      # Setup git user so that patches for native libraries can be applied and committed
-      - name: Setup git user
-        run: |
-          git config --global user.name "github-actions[bot]"
-          git config --global user.email "github-actions[bot]@users.noreply.github.com"
-
       - name: Setup Java ${{ matrix.java }}
         uses: actions/setup-java@v1
         with:
@@ -102,10 +90,10 @@ jobs:
           if sysctl -n machdep.cpu.features machdep.cpu.leaf7_features | grep -i AVX2
           then
               echo "avx2 available on system"
-              ./gradlew build 
+              ./gradlew build -Dbuild.lib.commit_patches=false
           else
               echo "avx2 not available on system"
-              ./gradlew build -Dsimd.enabled=false
+              ./gradlew build -Dsimd.enabled=false -Dbuild.lib.commit_patches=false
           fi
 
   Build-k-NN-Windows:
@@ -121,12 +109,6 @@ jobs:
       - name: Checkout k-NN
         uses: actions/checkout@v1
 
-      # Setup git user so that patches for native libraries can be applied and committed
-      - name: Setup git user
-        run: |
-          git config --global user.name "github-actions[bot]"
-          git config --global user.email "github-actions[bot]@users.noreply.github.com"
-
       - name: Setup Java ${{ matrix.java }}
         uses: actions/setup-java@v1
         with:
@@ -165,5 +147,5 @@ jobs:
 
       - name: Run build
         run: |
-          ./gradlew.bat build -D'simd.enabled=false'
+          ./gradlew.bat build -D'simd.enabled=false' -D'build.lib.commit_patches=false'
 

.github/workflows/backwards_compatibility_tests_workflow.yml

@@ -36,12 +36,6 @@ jobs:
       - name: Checkout k-NN
         uses: actions/checkout@v1
 
-      # Setup git user so that patches for native libraries can be applied and committed
-      - name: Setup git user
-        run: |
-          git config --global user.name "github-actions[bot]"
-          git config --global user.email "github-actions[bot]@users.noreply.github.com"
-
       - name: Setup Java ${{ matrix.java }}
         uses: actions/setup-java@v1
         with:
@@ -80,13 +74,13 @@ jobs:
         name: Run k-NN Restart-Upgrade BWC Tests from BWCVersion-${{ matrix.bwc_version }} to OpenSearch Version-${{ matrix.opensearch_version }} on Windows
         run: |
           echo "Running restart-upgrade backwards compatibility tests ..."
-          ./gradlew :qa:restart-upgrade:testRestartUpgrade -D'tests.bwc.version=${{ matrix.bwc_version }}'
+          ./gradlew :qa:restart-upgrade:testRestartUpgrade -D'tests.bwc.version=${{ matrix.bwc_version }}' -D'build.lib.commit_patches=false'
 
       - if: startsWith(matrix.os,'ubuntu')
         name: Run k-NN Restart-Upgrade BWC Tests from BWCVersion-${{ matrix.bwc_version }} to OpenSearch Version-${{ matrix.opensearch_version }} on Ubuntu
         run: |
           echo "Running restart-upgrade backwards compatibility tests ..."
-          ./gradlew :qa:restart-upgrade:testRestartUpgrade -Dtests.bwc.version=$BWC_VERSION_RESTART_UPGRADE
+          ./gradlew :qa:restart-upgrade:testRestartUpgrade -Dtests.bwc.version=$BWC_VERSION_RESTART_UPGRADE -Dbuild.lib.commit_patches=false
 
 
   Rolling-Upgrade-BWCTests-k-NN:
@@ -106,12 +100,6 @@ jobs:
       - name: Checkout k-NN
         uses: actions/checkout@v1
 
-      # Setup git user so that patches for native libraries can be applied and committed
-      - name: Setup git user
-        run: |
-          git config --global user.name "github-actions[bot]"
-          git config --global user.email "github-actions[bot]@users.noreply.github.com"
-
       - name: Setup Java ${{ matrix.java }}
         uses: actions/setup-java@v1
         with:
@@ -150,10 +138,10 @@ jobs:
         name: Run k-NN Rolling-Upgrade BWC Tests from BWCVersion-${{ matrix.bwc_version }} to OpenSearch Version-${{ matrix.opensearch_version }} on Windows
         run: |
           echo "Running rolling-upgrade backwards compatibility tests ..."
-          ./gradlew :qa:rolling-upgrade:testRollingUpgrade -D'tests.bwc.version=${{ matrix.bwc_version }}'
+          ./gradlew :qa:rolling-upgrade:testRollingUpgrade -D'tests.bwc.version=${{ matrix.bwc_version }}' -D'build.lib.commit_patches=false'
 
       - if: startsWith(matrix.os,'ubuntu')
         name: Run k-NN Rolling-Upgrade BWC Tests from BWCVersion-${{ matrix.bwc_version }} to OpenSearch Version-${{ matrix.opensearch_version }} on Ubuntu
         run: |
           echo "Running rolling-upgrade backwards compatibility tests ..."
-          ./gradlew :qa:rolling-upgrade:testRollingUpgrade -Dtests.bwc.version=$BWC_VERSION_ROLLING_UPGRADE
+          ./gradlew :qa:rolling-upgrade:testRollingUpgrade -Dtests.bwc.version=$BWC_VERSION_ROLLING_UPGRADE -Dbuild.lib.commit_patches=false

.github/workflows/test_security.yml

@@ -37,11 +37,6 @@ jobs:
         uses: actions/checkout@v1
         with:
           submodules: true
-      # Setup git user so that patches for native libraries can be applied and committed
-      - name: Setup git user
-        run: |
-          su `id -un 1000` -c 'git config --global user.name "github-actions[bot]"'
-          su `id -un 1000` -c 'git config --global user.email "github-actions[bot]@users.noreply.github.com"'
 
       - name: Setup Java ${{ matrix.java }}
         uses: actions/setup-java@v1
@@ -52,4 +47,4 @@ jobs:
         # switching the user, as OpenSearch cluster can only be started as root/Administrator on linux-deb/linux-rpm/windows-zip.
         run: |
           chown -R 1000:1000 `pwd`
-          su `id -un 1000` -c "whoami && java -version && ./gradlew integTest -Dsecurity.enabled=true -Dsimd.enabled=true"
+          su `id -un 1000` -c "whoami && java -version && ./gradlew integTest -Dsecurity.enabled=true -Dsimd.enabled=true -Dbuild.lib.commit_patches=false"

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

@@ -12,14 +12,18 @@ if (NOT EXISTS ${FAISS_REPO_DIR})
     execute_process(COMMAND git submodule update --init -- external/faiss WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
 endif ()
 
+# This is a bit of a hack for the CI. For whatever reason, git status needs to be run on the submodule directory for
+# patches to be applied correctly.
+execute_process(COMMAND git status WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib)
+
 # Check if patch exist, this is to skip git apply during CI build. See CI.yml with ubuntu.
 find_path(PATCH_FILE NAMES 0001-Custom-patch-to-support-multi-vector.patch 0002-Enable-precomp-table-to-be-shared-ivfpq.patch PATHS ${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss NO_DEFAULT_PATH)
 
 # 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

@@ -15,10 +15,14 @@ endif ()
 # Check if patch exist, this is to skip git apply during CI build. See CI.yml with ubuntu.
 find_path(PATCH_FILE NAMES 0001-Initialize-maxlevel-during-add-from-enterpoint-level.patch PATHS ${CMAKE_CURRENT_SOURCE_DIR}/patches/nmslib NO_DEFAULT_PATH)
 
+# This is a bit of a hack for the CI. For whatever reason, git status needs to be run on the submodule directory for
+# patches to be applied correctly.
+execute_process(COMMAND git status WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib)
+
 # 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