From 8d09baf49f03ff8a50e30f1c662ea706ab657bb5 Mon Sep 17 00:00:00 2001 From: Scott McKay Date: Wed, 8 May 2024 08:30:11 +1000 Subject: [PATCH] Clarify when protobuf dependency builds protoc (#20542) ### Description Currently figuring out if the protobuf dependency is building protoc it is a little obtuse and inconsistent * in some places we directly set protobuf_BUILD_PROTOC_BINARIES to OFF to indicate the protobuf dependency is not building protoc * e.g. macOS/iOS/visionOS builds * for a user provided protoc path we don't set protobuf_BUILD_PROTOC_BINARIES, and inside protobuf_function.cmake that determines if `protobuf::protoc` is added as a dependency or not * https://github.com/microsoft/onnxruntime/blob/0dda8b0c449009dee5e9cdf3ccaa4fbbacf04dd0/cmake/external/protobuf_function.cmake#L40-L45 To be more consistent/explicit, set protobuf_BUILD_PROTOC_BINARIES to OFF when ONNX_CUSTOM_PROTOC_EXECUTABLE set and valid. Remove outdated script that built and external protoc binary which was used in later builds. The build setup will fetch a pre-built protoc so there's no need for this additional build. ### Motivation and Context Make it easier to figure out if protoc is coming from the protobuf dependency. --- .../external/onnxruntime_external_deps.cmake | 47 +++++++++++++------ .../github/apple/build_host_protoc.sh | 40 ---------------- ...ndroid-x86_64-crosscompile-ci-pipeline.yml | 8 ---- .../azure-pipelines/templates/c-api-cpu.yml | 8 ---- 4 files changed, 32 insertions(+), 71 deletions(-) delete mode 100755 tools/ci_build/github/apple/build_host_protoc.sh diff --git a/cmake/external/onnxruntime_external_deps.cmake b/cmake/external/onnxruntime_external_deps.cmake index 8839dbc8fd..775576a771 100644 --- a/cmake/external/onnxruntime_external_deps.cmake +++ b/cmake/external/onnxruntime_external_deps.cmake @@ -97,7 +97,6 @@ FetchContent_Declare( ) - # Flatbuffers # We do not need to build flatc for iOS or Android Cross Compile if (CMAKE_SYSTEM_NAME STREQUAL "iOS" OR CMAKE_SYSTEM_NAME STREQUAL "Android" OR CMAKE_SYSTEM_NAME STREQUAL "Emscripten") @@ -122,6 +121,19 @@ FetchContent_Declare( FIND_PACKAGE_ARGS 23.5.9 NAMES Flatbuffers ) + +#Protobuf depends on utf8_range +FetchContent_Declare( + utf8_range + URL ${DEP_URL_utf8_range} + URL_HASH SHA1=${DEP_SHA1_utf8_range} + FIND_PACKAGE_ARGS NAMES utf8_range +) + +set(utf8_range_ENABLE_TESTS OFF CACHE BOOL "Build test suite" FORCE) +set(utf8_range_ENABLE_INSTALL OFF CACHE BOOL "Configure installation" FORCE) + + # Download a protoc binary from Internet if needed if(NOT ONNX_CUSTOM_PROTOC_EXECUTABLE) # This part of code is only for users' convenience. The code couldn't handle all cases. Users always can manually @@ -148,6 +160,7 @@ if(NOT ONNX_CUSTOM_PROTOC_EXECUTABLE) FetchContent_Declare(protoc_binary URL ${DEP_URL_protoc_win32} URL_HASH SHA1=${DEP_SHA1_protoc_win32}) FetchContent_Populate(protoc_binary) endif() + if(protoc_binary_SOURCE_DIR) message("Use prebuilt protoc") set(ONNX_CUSTOM_PROTOC_EXECUTABLE ${protoc_binary_SOURCE_DIR}/bin/protoc.exe) @@ -164,15 +177,30 @@ if(NOT ONNX_CUSTOM_PROTOC_EXECUTABLE) FetchContent_Declare(protoc_binary URL ${DEP_URL_protoc_linux_aarch64} URL_HASH SHA1=${DEP_SHA1_protoc_linux_aarch64}) FetchContent_Populate(protoc_binary) endif() + if(protoc_binary_SOURCE_DIR) message("Use prebuilt protoc") set(ONNX_CUSTOM_PROTOC_EXECUTABLE ${protoc_binary_SOURCE_DIR}/bin/protoc) set(PROTOC_EXECUTABLE ${ONNX_CUSTOM_PROTOC_EXECUTABLE}) endif() endif() + + if(NOT ONNX_CUSTOM_PROTOC_EXECUTABLE) + message(FATAL_ERROR "ONNX_CUSTOM_PROTOC_EXECUTABLE must be set to cross-compile.") + endif() endif() endif() +# if ONNX_CUSTOM_PROTOC_EXECUTABLE is set we don't need to build the protoc binary +if (ONNX_CUSTOM_PROTOC_EXECUTABLE) + if (NOT EXISTS "${ONNX_CUSTOM_PROTOC_EXECUTABLE}") + message(FATAL_ERROR "ONNX_CUSTOM_PROTOC_EXECUTABLE is set to '${ONNX_CUSTOM_PROTOC_EXECUTABLE}' " + "but protoc executable was not found there.") + endif() + + set(protobuf_BUILD_PROTOC_BINARIES OFF CACHE BOOL "Build protoc" FORCE) +endif() + #Here we support two build mode: #1. if ONNX_CUSTOM_PROTOC_EXECUTABLE is set, build Protobuf from source, except protoc.exe. This mode is mainly # for cross-compiling @@ -183,17 +211,6 @@ else() set(ONNXRUNTIME_PROTOBUF_PATCH_COMMAND "") endif() -FetchContent_Declare( - utf8_range - URL ${DEP_URL_utf8_range} - URL_HASH SHA1=${DEP_SHA1_utf8_range} - FIND_PACKAGE_ARGS NAMES utf8_range -) - -set(utf8_range_ENABLE_TESTS OFF CACHE BOOL "Build test suite" FORCE) -set(utf8_range_ENABLE_INSTALL OFF CACHE BOOL "Configure installation" FORCE) - - #Protobuf depends on absl and utf8_range FetchContent_Declare( Protobuf @@ -211,10 +228,10 @@ set(protobuf_BUILD_TESTS OFF CACHE BOOL "Build protobuf tests" FORCE) #set(protobuf_INSTALL OFF CACHE BOOL "Install protobuf binaries and files" FORCE) set(protobuf_USE_EXTERNAL_GTEST ON CACHE BOOL "" FORCE) -if (CMAKE_SYSTEM_NAME STREQUAL "Android") - set(protobuf_BUILD_PROTOC_BINARIES OFF CACHE BOOL "Build protobuf tests" FORCE) - set(protobuf_WITH_ZLIB OFF CACHE BOOL "Build with zlib support" FORCE) +if (ANDROID) + set(protobuf_WITH_ZLIB OFF CACHE BOOL "Build protobuf with zlib support" FORCE) endif() + if (onnxruntime_DISABLE_RTTI) set(protobuf_DISABLE_RTTI ON CACHE BOOL "Remove runtime type information in the binaries" FORCE) endif() diff --git a/tools/ci_build/github/apple/build_host_protoc.sh b/tools/ci_build/github/apple/build_host_protoc.sh deleted file mode 100755 index 6d90e6a504..0000000000 --- a/tools/ci_build/github/apple/build_host_protoc.sh +++ /dev/null @@ -1,40 +0,0 @@ -#!/bin/bash - -# Note: This script is intended to be called from a macOS pipeline to build the host protoc -# See tools/ci_build/github/azure-pipelines/mac-ios-ci-pipeline.yml -# The host_protoc can be found as $PROTOC_INSTALL_PATH/bin/protoc - -set -e - -if [ $# -ne 3 ] -then - echo "Usage: ${0} " - exit 1 -fi - -set -x - -ORT_REPO_ROOT=$1 -PROTOC_BUILD_PATH=$2 -PROTOC_INSTALL_PATH=$3 - -pushd . -mkdir -p "$PROTOC_BUILD_PATH" -cd "$PROTOC_BUILD_PATH" -DEP_FILE_PATH="$ORT_REPO_ROOT/cmake/deps.txt" -PATCH_FILE_PATH="$ORT_REPO_ROOT/cmake/patches/protobuf/protobuf_cmake.patch" -protobuf_url=$(grep '^protobuf' "$DEP_FILE_PATH" | cut -d ';' -f 2 | sed 's/\.zip$/\.tar.gz/') -curl -sSL --retry 5 --retry-delay 10 --create-dirs --fail -L -o protobuf_src.tar.gz "$protobuf_url" -tar -zxf protobuf_src.tar.gz --strip=1 -patch --binary --ignore-whitespace -p1 < "$PATCH_FILE_PATH" -# The second 'cmake' is a folder name -cmake cmake \ - -DCMAKE_POSITION_INDEPENDENT_CODE=ON \ - -Dprotobuf_BUILD_TESTS=OFF \ - -Dprotobuf_WITH_ZLIB_DEFAULT=OFF \ - -Dprotobuf_BUILD_SHARED_LIBS=OFF \ - -DCMAKE_BUILD_TYPE=Release \ - "-DCMAKE_INSTALL_PREFIX=$PROTOC_INSTALL_PATH" -make -j $(getconf _NPROCESSORS_ONLN) -make install -popd diff --git a/tools/ci_build/github/azure-pipelines/android-x86_64-crosscompile-ci-pipeline.yml b/tools/ci_build/github/azure-pipelines/android-x86_64-crosscompile-ci-pipeline.yml index d0a22aae07..54e83b03aa 100644 --- a/tools/ci_build/github/azure-pipelines/android-x86_64-crosscompile-ci-pipeline.yml +++ b/tools/ci_build/github/azure-pipelines/android-x86_64-crosscompile-ci-pipeline.yml @@ -394,14 +394,6 @@ stages: - script: brew install coreutils ninja displayName: Install coreutils and ninja - # We build the host protoc to /protobuf_install - - script: | - /bin/bash $(Build.SourcesDirectory)/tools/ci_build/github/apple/build_host_protoc.sh \ - $(Build.SourcesDirectory) \ - $(Build.BinariesDirectory)/protobuf \ - $(Build.SourcesDirectory)/protobuf_install - displayName: Build Host Protoc - - template: templates/use-android-emulator.yml parameters: create: true diff --git a/tools/ci_build/github/azure-pipelines/templates/c-api-cpu.yml b/tools/ci_build/github/azure-pipelines/templates/c-api-cpu.yml index f8c0009f5a..c04cc67036 100644 --- a/tools/ci_build/github/azure-pipelines/templates/c-api-cpu.yml +++ b/tools/ci_build/github/azure-pipelines/templates/c-api-cpu.yml @@ -110,13 +110,6 @@ stages: parameters: xcodeVersion: 14.2 - - script: | - /bin/bash $(Build.SourcesDirectory)/tools/ci_build/github/apple/build_host_protoc.sh \ - $(Build.SourcesDirectory) \ - $(Build.BinariesDirectory)/protobuf \ - $(Build.BinariesDirectory)/protobuf_install - displayName: Build Host Protoc - - template: download-deps.yml - task: PythonScript@0 @@ -130,7 +123,6 @@ stages: set -e -x python3 tools/ci_build/github/apple/build_apple_framework.py \ --build_dir "$(Build.BinariesDirectory)/ios_framework" \ - --path_to_protoc_exe $(Build.BinariesDirectory)/protobuf_install/bin/protoc \ tools/ci_build/github/apple/default_full_ios_framework_build_settings.json mkdir $(Build.BinariesDirectory)/artifacts mkdir -p $(Build.BinariesDirectory)/artifacts_staging/onnxruntime-ios-xcframework-$(OnnxRuntimeVersion)