Fix build failure due to patch conflict (opensearch-project#1851) (op…

…ensearch-project#1852) Signed-off-by: Heemin Kim <[email protected]> (cherry picked from commit 881364f) Co-authored-by: Heemin Kim <[email protected]>
naveentatikonda · Jul 20, 2024 · 84ff2be · 84ff2be
1 parent e9e42eb
commit 84ff2be
Show file tree

Hide file tree

Showing 5 changed files with 92 additions and 67 deletions.
diff --git a/.gitignore b/.gitignore
@@ -28,6 +28,7 @@ jni/bin/
 jni/lib/
 jni/jni_test*
 jni/googletest*
+jni/cmake/*.cmake-e
 
 benchmarks/perf-tool/okpt/output
 benchmarks/perf-tool/okpt/dev

diff --git a/build.gradle b/build.gradle
@@ -18,8 +18,15 @@ 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
+        // This flag determines whether the CMake build system should apply a custom patch. It prevents build failures
+        // when the cmakeJniLib task is run multiple times. If the build.lib.commit_patches is true, the CMake build
+        // system skips applying the patch if the patches have been applied already. If build.lib.commit_patches is
+        // false, the patches are always applied. To avoid patch conflicts, disable this flag manually after the first
+        // run of buildJniLib
+        apply_lib_patches = System.getProperty("build.lib.apply_patches", "true")
+        // Flag to determine whether cmake build system should commit the patch or not. In automated build environments
+        // set this to false. In dev environments, set to true. If false, repetitive execution of cmakeJniLib may fail.
+        // To prevent this, set build.lib.apply_patches to false after the first cmakeJniLib run.
         commit_lib_patches = System.getProperty("build.lib.commit_patches", "true")
 
         version_tokens = opensearch_version.tokenize('-')
@@ -304,13 +311,21 @@ task windowsPatches(type:Exec) {
 
 task cmakeJniLib(type:Exec) {
     workingDir 'jni'
+    def args = []
+    args.add("cmake")
+    args.add(".")
+    args.add("-DKNN_PLUGIN_VERSION=${opensearch_version}")
+    args.add("-DSIMD_ENABLED=${simd_enabled}")
+    args.add("-DCOMMIT_LIB_PATCHES=${commit_lib_patches}")
+    args.add("-DAPPLY_LIB_PATCHES=${apply_lib_patches}")
     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}", "-DCOMMIT_LIB_PATCHES=${commit_lib_patches}"
-    }
-    else {
-        commandLine 'cmake', '.', "-DKNN_PLUGIN_VERSION=${opensearch_version}", "-DSIMD_ENABLED=${simd_enabled}", "-DCOMMIT_LIB_PATCHES=${commit_lib_patches}"
+        args.add("-G")
+        args.add("Unix Makefiles")
+        args.add("-DBLAS_LIBRARIES=$rootDir\\src\\main\\resources\\windowsDependencies\\libopenblas.dll")
+        args.add("-DLAPACK_LIBRARIES=$rootDir\\src\\main\\resources\\windowsDependencies\\libopenblas.dll")
     }
+    commandLine args
 }
 
 task buildJniLib(type:Exec) {

diff --git a/jni/cmake/init-faiss.cmake b/jni/cmake/init-faiss.cmake
@@ -12,44 +12,47 @@ if (NOT EXISTS ${FAISS_REPO_DIR})
     execute_process(COMMAND git submodule update --init -- external/faiss WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
 endif ()
 
-# Define list of patch files
-set(PATCH_FILE_LIST)
-list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0001-Custom-patch-to-support-multi-vector.patch")
-list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0002-Enable-precomp-table-to-be-shared-ivfpq.patch")
-list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0003-Custom-patch-to-support-range-search-params.patch")
-list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0004-Custom-patch-to-support-binary-vector.patch")
+# Apply patches
+if(NOT DEFINED APPLY_LIB_PATCHES OR "${APPLY_LIB_PATCHES}" STREQUAL true)
+    # Define list of patch files
+    set(PATCH_FILE_LIST)
+    list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0001-Custom-patch-to-support-multi-vector.patch")
+    list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0002-Enable-precomp-table-to-be-shared-ivfpq.patch")
+    list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0003-Custom-patch-to-support-range-search-params.patch")
+    list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/faiss/0004-Custom-patch-to-support-binary-vector.patch")
 
-# Get patch id of the last commit
-execute_process(COMMAND sh -c "git --no-pager show HEAD | git patch-id --stable" OUTPUT_VARIABLE PATCH_ID_OUTPUT_FROM_COMMIT WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss)
-string(REPLACE " " ";" PATCH_ID_LIST_FROM_COMMIT ${PATCH_ID_OUTPUT_FROM_COMMIT})
-list(GET PATCH_ID_LIST_FROM_COMMIT 0 PATCH_ID_FROM_COMMIT)
+    # Get patch id of the last commit
+    execute_process(COMMAND sh -c "git --no-pager show HEAD | git patch-id --stable" OUTPUT_VARIABLE PATCH_ID_OUTPUT_FROM_COMMIT WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/faiss)
+    string(REPLACE " " ";" PATCH_ID_LIST_FROM_COMMIT ${PATCH_ID_OUTPUT_FROM_COMMIT})
+    list(GET PATCH_ID_LIST_FROM_COMMIT 0 PATCH_ID_FROM_COMMIT)
 
-# Find all patch files need to apply
-list(SORT PATCH_FILE_LIST ORDER DESCENDING)
-set(PATCH_FILES_TO_APPLY)
-foreach(PATCH_FILE IN LISTS PATCH_FILE_LIST)
-    # Get patch id of a patch file
-    execute_process(COMMAND sh -c "cat ${PATCH_FILE} | git patch-id --stable" OUTPUT_VARIABLE PATCH_ID_OUTPUT)
-    string(REPLACE " " ";" PATCH_ID_LIST ${PATCH_ID_OUTPUT})
-    list(GET PATCH_ID_LIST 0 PATCH_ID)
+    # Find all patch files need to apply
+    list(SORT PATCH_FILE_LIST ORDER DESCENDING)
+    set(PATCH_FILES_TO_APPLY)
+    foreach(PATCH_FILE IN LISTS PATCH_FILE_LIST)
+        # Get patch id of a patch file
+        execute_process(COMMAND sh -c "cat ${PATCH_FILE} | git patch-id --stable" OUTPUT_VARIABLE PATCH_ID_OUTPUT)
+        string(REPLACE " " ";" PATCH_ID_LIST ${PATCH_ID_OUTPUT})
+        list(GET PATCH_ID_LIST 0 PATCH_ID)
 
-    # Add the file to patch list if patch id does not match
-    if (${PATCH_ID} STREQUAL ${PATCH_ID_FROM_COMMIT})
-        break()
-    else()
-        list(APPEND PATCH_FILES_TO_APPLY ${PATCH_FILE})
-    endif()
-endforeach()
+        # Add the file to patch list if patch id does not match
+        if (${PATCH_ID} STREQUAL ${PATCH_ID_FROM_COMMIT})
+            break()
+        else()
+            list(APPEND PATCH_FILES_TO_APPLY ${PATCH_FILE})
+        endif()
+    endforeach()
 
-# Apply patch files
-list(SORT PATCH_FILES_TO_APPLY)
-foreach(PATCH_FILE IN LISTS PATCH_FILES_TO_APPLY)
-    message(STATUS "Applying patch of ${PATCH_FILE}")
-    execute_process(COMMAND git ${GIT_PATCH_COMMAND} --3way --ignore-space-change --ignore-whitespace ${PATCH_FILE} 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()
-endforeach()
+    # Apply patch files
+    list(SORT PATCH_FILES_TO_APPLY)
+    foreach(PATCH_FILE IN LISTS PATCH_FILES_TO_APPLY)
+        message(STATUS "Applying patch of ${PATCH_FILE}")
+        execute_process(COMMAND git ${GIT_PATCH_COMMAND} --3way --ignore-space-change --ignore-whitespace ${PATCH_FILE} 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()
+    endforeach()
+endif()
 
 if (${CMAKE_SYSTEM_NAME} STREQUAL Darwin)
     if(CMAKE_C_COMPILER_ID MATCHES "Clang\$")

diff --git a/jni/cmake/init-nmslib.cmake b/jni/cmake/init-nmslib.cmake
@@ -12,32 +12,36 @@ if (NOT EXISTS ${NMS_REPO_DIR})
     execute_process(COMMAND git submodule update --init -- external/nmslib WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR})
 endif ()
 
-# Define list of patch files
-set(PATCH_FILE_LIST)
-list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/nmslib/0001-Initialize-maxlevel-during-add-from-enterpoint-level.patch")
-list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/nmslib/0002-Adds-ability-to-pass-ef-parameter-in-the-query-for-h.patch")
-
-# Get patch id of the last commit
-execute_process(COMMAND sh -c "git --no-pager show HEAD | git patch-id --stable" OUTPUT_VARIABLE PATCH_ID_OUTPUT_FROM_COMMIT WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib)
-string(REPLACE " " ";" PATCH_ID_LIST_FROM_COMMIT ${PATCH_ID_OUTPUT_FROM_COMMIT})
-list(GET PATCH_ID_LIST_FROM_COMMIT 0 PATCH_ID_FROM_COMMIT)
-
-# Find all patch files need to apply
-list(SORT PATCH_FILE_LIST ORDER DESCENDING)
-set(PATCH_FILES_TO_APPLY)
-foreach(PATCH_FILE IN LISTS PATCH_FILE_LIST)
-    # Get patch id of a patch file
-    execute_process(COMMAND sh -c "cat ${PATCH_FILE} | git patch-id --stable" OUTPUT_VARIABLE PATCH_ID_OUTPUT)
-    string(REPLACE " " ";" PATCH_ID_LIST ${PATCH_ID_OUTPUT})
-    list(GET PATCH_ID_LIST 0 PATCH_ID)
-
-    # Add the file to patch list if patch id does not match
-    if (${PATCH_ID} STREQUAL ${PATCH_ID_FROM_COMMIT})
-        break()
-    else()
-        list(APPEND PATCH_FILES_TO_APPLY ${PATCH_FILE})
-    endif()
-endforeach()
+
+# Apply patches
+if(NOT DEFINED APPLY_LIB_PATCHES OR "${APPLY_LIB_PATCHES}" STREQUAL true)
+    # Define list of patch files
+    set(PATCH_FILE_LIST)
+    list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/nmslib/0001-Initialize-maxlevel-during-add-from-enterpoint-level.patch")
+    list(APPEND PATCH_FILE_LIST "${CMAKE_CURRENT_SOURCE_DIR}/patches/nmslib/0002-Adds-ability-to-pass-ef-parameter-in-the-query-for-h.patch")
+
+    # Get patch id of the last commit
+    execute_process(COMMAND sh -c "git --no-pager show HEAD | git patch-id --stable" OUTPUT_VARIABLE PATCH_ID_OUTPUT_FROM_COMMIT WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/external/nmslib)
+    string(REPLACE " " ";" PATCH_ID_LIST_FROM_COMMIT ${PATCH_ID_OUTPUT_FROM_COMMIT})
+    list(GET PATCH_ID_LIST_FROM_COMMIT 0 PATCH_ID_FROM_COMMIT)
+
+    # Find all patch files need to apply
+    list(SORT PATCH_FILE_LIST ORDER DESCENDING)
+    set(PATCH_FILES_TO_APPLY)
+    foreach(PATCH_FILE IN LISTS PATCH_FILE_LIST)
+        # Get patch id of a patch file
+        execute_process(COMMAND sh -c "cat ${PATCH_FILE} | git patch-id --stable" OUTPUT_VARIABLE PATCH_ID_OUTPUT)
+        string(REPLACE " " ";" PATCH_ID_LIST ${PATCH_ID_OUTPUT})
+        list(GET PATCH_ID_LIST 0 PATCH_ID)
+
+        # Add the file to patch list if patch id does not match
+        if (${PATCH_ID} STREQUAL ${PATCH_ID_FROM_COMMIT})
+            break()
+        else()
+            list(APPEND PATCH_FILES_TO_APPLY ${PATCH_FILE})
+        endif()
+    endforeach()
+endif()
 
 # Apply patch files
 list(SORT PATCH_FILES_TO_APPLY)

diff --git a/scripts/build.sh b/scripts/build.sh
@@ -122,7 +122,9 @@ cd $work_dir
 
 if [ "$PLATFORM" != "windows" ] && [ "$ARCHITECTURE" = "x64" ]; then
   echo "Building k-NN library after enabling AVX2"
-  ./gradlew :buildJniLib -Dsimd.enabled=true -Dbuild.lib.commit_patches=false
+  # Skip applying patches as patches were applied already from previous :buildJniLib task
+  # If we apply patches again, it fails with conflict
+  ./gradlew :buildJniLib -Dsimd.enabled=true -Dbuild.lib.commit_patches=false -Dbuild.lib.apply_patches=false
 fi
 
 ./gradlew publishPluginZipPublicationToZipStagingRepository -Dopensearch.version=$VERSION -Dbuild.snapshot=$SNAPSHOT -Dbuild.version_qualifier=$QUALIFIER