This is an automated email from the ASF dual-hosted git repository. dongjoon pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/orc.git
The following commit(s) were added to refs/heads/main by this push: new a9e0351c8 ORC-1732: [C++] Fix detecting Homebrew-installed Protobuf on MacOS a9e0351c8 is described below commit a9e0351c800981162bb1435a0fbb541575cb0bc8 Author: luffy-zh <zhn...@outlook.com> AuthorDate: Thu Jul 11 08:44:39 2024 -0700 ORC-1732: [C++] Fix detecting Homebrew-installed Protobuf on MacOS ### What changes were proposed in this pull request? Detect Protobuf installed by Homebrew on macOS. ### Why are the changes needed? Deal with the issue discussed [here](https://github.com/apache/arrow/issues/42149) ### How was this patch tested? Test it locally. ### Was this patch authored or co-authored using generative AI tooling? NO Closes #1963 from luffy-zh/ORC-1732. Lead-authored-by: luffy-zh <zhn...@outlook.com> Co-authored-by: Hao Zou <34559830+luffy...@users.noreply.github.com> Signed-off-by: Dongjoon Hyun <dongj...@apache.org> --- .github/workflows/build_and_test.yml | 22 ++++++++++++++++++++++ cmake_modules/FindProtobuf.cmake | 34 +++++++++++++++++++++++++++++++--- tools/test/TestFileScan.cc | 15 ++++++++++++++- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 30d374e75..89460a543 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -197,3 +197,25 @@ jobs: with: config: .github/.licenserc.yaml + macos-cpp-check: + name: "C++ Test on macOS" + strategy: + fail-fast: false + matrix: + version: [12, 13, 14] + runs-on: macos-${{ matrix.version }} + steps: + - name: Checkout repository + uses: actions/checkout@v4 + - name: Install dependencies + run: | + brew update + brew install protobuf + - name: Test + run: | + CMAKE_PREFIX_PATH=$(brew --prefix protobuf) + mkdir -p build + cd build + cmake .. -DBUILD_JAVA=OFF -DPROTOBUF_HOME=${CMAKE_PREFIX_PATH} + make package test-out + diff --git a/cmake_modules/FindProtobuf.cmake b/cmake_modules/FindProtobuf.cmake index 82429a23f..ca91fb5ad 100644 --- a/cmake_modules/FindProtobuf.cmake +++ b/cmake_modules/FindProtobuf.cmake @@ -45,8 +45,17 @@ endif() message (STATUS "PROTOBUF_HOME: ${PROTOBUF_HOME}") +if (NOT DEFINED CMAKE_STATIC_LIBRARY_SUFFIX) + if (WIN32) + set (CMAKE_STATIC_LIBRARY_SUFFIX ".lib") + else () + set (CMAKE_STATIC_LIBRARY_SUFFIX ".a") + endif () +endif () + find_package (Protobuf CONFIG) if (Protobuf_FOUND) + if (TARGET protobuf::libprotobuf) set (PROTOBUF_LIBRARY protobuf::libprotobuf) set (PROTOBUF_STATIC_LIB PROTOBUF_STATIC_LIB-NOTFOUND) set (PROTOC_LIBRARY protobuf::libprotoc) @@ -55,15 +64,34 @@ if (Protobuf_FOUND) get_target_property (target_type protobuf::libprotobuf TYPE) if (target_type STREQUAL "STATIC_LIBRARY") - set(PROTOBUF_STATIC_LIB protobuf::libprotobuf) + set (PROTOBUF_STATIC_LIB protobuf::libprotobuf) endif () get_target_property (target_type protobuf::libprotoc TYPE) if (target_type STREQUAL "STATIC_LIBRARY") - set (PROTOC_STATIC_LIB protobuf::libprotoc) + set (PROTOC_STATIC_LIB protobuf::libprotoc) + endif () + + get_target_property (PROTOBUF_INCLUDE_DIR protobuf::libprotobuf INTERFACE_INCLUDE_DIRECTORIES) + if (NOT PROTOBUF_INCLUDE_DIR) + set (PROTOBUF_INCLUDE_DIR ${Protobuf_INCLUDE_DIRS}) + if (NOT PROTOBUF_INCLUDE_DIR) + message (FATAL_ERROR "Cannot determine Protobuf include directory from protobuf::libprotobuf and Protobuf_INCLUDE_DIRS.") + endif () + endif () + else () + set (PROTOBUF_LIBRARY ${Protobuf_LIBRARIES}) + set (PROTOBUF_INCLUDE_DIR ${Protobuf_INCLUDE_DIRS}) + if (NOT PROTOBUF_INCLUDE_DIR) + message (FATAL_ERROR "Cannot determine Protobuf include directory.") endif () - get_target_property (PROTOBUF_INCLUDE_DIR protobuf::libprotoc INTERFACE_INCLUDE_DIRECTORIES) + if (Protobuf_LIBRARIES MATCHES "\\${CMAKE_STATIC_LIBRARY_SUFFIX}$") + set (PROTOBUF_STATIC_LIB ${Protobuf_LIBRARIES}) + else () + set (PROTOBUF_STATIC_LIB PROTOBUF_STATIC_LIB-NOTFOUND) + endif () + endif () else() find_path (PROTOBUF_INCLUDE_DIR google/protobuf/io/zero_copy_stream.h HINTS diff --git a/tools/test/TestFileScan.cc b/tools/test/TestFileScan.cc index 2e35d86ef..5fb43d5d2 100644 --- a/tools/test/TestFileScan.cc +++ b/tools/test/TestFileScan.cc @@ -211,9 +211,22 @@ void checkForError(const std::string& filename, const std::string& errorMsg) { EXPECT_NE(std::string::npos, error.find(errorMsg)) << error; } +void checkForError(const std::string& filename, const std::string& errorMsg1, + const std::string& errorMsg2) { + const std::string pgm = findProgram("tools/src/orc-scan"); + std::string output; + std::string error; + EXPECT_EQ(1, runProgram({pgm, filename}, output, error)); + EXPECT_EQ("", output); + if (error.find(errorMsg1) == std::string::npos && error.find(errorMsg2) == std::string::npos) { + FAIL() << error; + } +} + TEST(TestFileScan, testErrorHandling) { checkForError(findExample("corrupt/stripe_footer_bad_column_encodings.orc"), - "bad number of ColumnEncodings in StripeFooter: expected=6, actual=0"); + "bad number of ColumnEncodings in StripeFooter: expected=6, actual=0", + "bad StripeFooter from zlib"); checkForError(findExample("corrupt/negative_dict_entry_lengths.orc"), "Negative dictionary entry length"); checkForError(findExample("corrupt/missing_length_stream_in_string_dict.orc"),