From 84ff2beddf87e1017981fb795a012d44d05baf51 Mon Sep 17 00:00:00 2001 From: "opensearch-trigger-bot[bot]" <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Date: Thu, 18 Jul 2024 11:07:09 -0700 Subject: [PATCH] Fix build failure due to patch conflict (#1851) (#1852) Signed-off-by: Heemin Kim (cherry picked from commit 881364f73b5f06761a1c4beffea6fb182e9512d5) Co-authored-by: Heemin Kim --- .gitignore | 1 + build.gradle | 27 ++++++++++---- jni/cmake/init-faiss.cmake | 71 +++++++++++++++++++------------------ jni/cmake/init-nmslib.cmake | 56 +++++++++++++++-------------- scripts/build.sh | 4 ++- 5 files changed, 92 insertions(+), 67 deletions(-) diff --git a/.gitignore b/.gitignore index 4cb4ee61c..7ff764056 100644 --- 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 index 09851f65f..73f11933b 100644 --- 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 index c2ec24a3b..08f3ccc40 100644 --- 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 index 56c52bb69..b2c16f1fe 100644 --- 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 index 38998d66a..12798633b 100644 --- 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