This is an automated email from the ASF dual-hosted git repository.
dongjoon pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/orc.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new 810f466cf ORC-1732: [C++] Fix detecting Homebrew-installed Protobuf on
MacOS
810f466cf is described below
commit 810f466cf0dfc4026b40bc54b2a99e7ecc34a777
Author: luffy-zh <[email protected]>
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 <[email protected]>
Co-authored-by: Hao Zou <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
(cherry picked from commit a9e0351c800981162bb1435a0fbb541575cb0bc8)
Signed-off-by: Dongjoon Hyun <[email protected]>
---
.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 2de7e6197..d3f0fa53a 100644
--- a/.github/workflows/build_and_test.yml
+++ b/.github/workflows/build_and_test.yml
@@ -194,3 +194,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 cca7c8b87..919bb11c3 100644
--- a/cmake_modules/FindProtobuf.cmake
+++ b/cmake_modules/FindProtobuf.cmake
@@ -32,8 +32,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)
@@ -42,15 +51,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 ecf6b4f12..22f49789a 100644
--- a/tools/test/TestFileScan.cc
+++ b/tools/test/TestFileScan.cc
@@ -211,9 +211,22 @@ void checkForError(const std::string& filename, const
std::string& error_msg) {
EXPECT_NE(std::string::npos, error.find(error_msg)) << 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"),
