Repository: parquet-cpp Updated Branches: refs/heads/master 29c6bff49 -> 616305cb9
PARQUET-662: Compile ParquetException implementation and explicitly export This was causing downstream failure in ARROW-237. Now thirdparty libraries can safely throw `ParquetException` and be properly recognized on both ends. Author: Wes McKinney <[email protected]> Closes #139 from wesm/PARQUET-662 and squashes the following commits: c1458d2 [Wes McKinney] Move ParquetException impl to object code, explicitly export. Add LINKAGE option to ADD_PARQUET_TEST, test throwing ParquetException via shared lib. Build shared library in gcc build. Project: http://git-wip-us.apache.org/repos/asf/parquet-cpp/repo Commit: http://git-wip-us.apache.org/repos/asf/parquet-cpp/commit/616305cb Tree: http://git-wip-us.apache.org/repos/asf/parquet-cpp/tree/616305cb Diff: http://git-wip-us.apache.org/repos/asf/parquet-cpp/diff/616305cb Branch: refs/heads/master Commit: 616305cb9005351f2e0c3cddb075fc06618ec779 Parents: 29c6bff Author: Wes McKinney <[email protected]> Authored: Mon Jul 11 20:22:10 2016 -0700 Committer: Wes McKinney <[email protected]> Committed: Mon Jul 11 20:22:10 2016 -0700 ---------------------------------------------------------------------- .travis.yml | 2 +- CMakeLists.txt | 39 +++++++++++++++++++++++++----- src/parquet/CMakeLists.txt | 4 +++- src/parquet/exception.cc | 48 +++++++++++++++++++++++++++++++++++++ src/parquet/exception.h | 25 ++++++++----------- src/parquet/public-api-test.cc | 10 +++++--- src/parquet/util/logging.h | 6 ++--- src/parquet/util/output.cc | 1 + 8 files changed, 106 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/616305cb/.travis.yml ---------------------------------------------------------------------- diff --git a/.travis.yml b/.travis.yml index e83aeba..780d9f9 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,7 +26,7 @@ matrix: before_script: - source $TRAVIS_BUILD_DIR/ci/before_script_travis.sh - cmake -DCMAKE_CXX_FLAGS="-Werror" -DPARQUET_TEST_MEMCHECK=ON -DPARQUET_BUILD_BENCHMARKS=ON - -DPARQUET_GENERATE_COVERAGE=1 $TRAVIS_BUILD_DIR -DPARQUET_BUILD_SHARED=OFF -DPARQUET_BUILD_STATIC=ON + -DPARQUET_GENERATE_COVERAGE=1 $TRAVIS_BUILD_DIR - export PARQUET_TEST_DATA=$TRAVIS_BUILD_DIR/data - compiler: clang os: linux http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/616305cb/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/CMakeLists.txt b/CMakeLists.txt index 5b594a3..f833f2c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -171,9 +171,32 @@ endfunction() # # Arguments after the test name will be passed to set_tests_properties(). function(ADD_PARQUET_TEST REL_TEST_NAME) - if(NOT PARQUET_BUILD_TESTS OR NOT PARQUET_BUILD_STATIC) + set(options) + set(one_value_args LINKAGE) + set(multi_value_args) + cmake_parse_arguments(ARG "${options}" "${one_value_args}" "${multi_value_args}" ${ARGN}) + + if(NOT PARQUET_BUILD_TESTS) return() endif() + + # TODO(wesm): not very rigorous error checking + if (ARG_LINKAGE AND "${ARG_LINKAGE}" STREQUAL "shared") + if(NOT PARQUET_BUILD_SHARED) + # Skip this test if we are not building the shared library + return() + else() + set(TEST_LINK_LIBS ${PARQUET_TEST_SHARED_LINK_LIBS}) + endif() + else() + if(NOT PARQUET_BUILD_STATIC) + # Skip this test if we are not building the static library + return() + else() + set(TEST_LINK_LIBS ${PARQUET_TEST_LINK_LIBS}) + endif() + endif() + get_filename_component(TEST_NAME ${REL_TEST_NAME} NAME_WE) if(EXISTS ${CMAKE_CURRENT_SOURCE_DIR}/${REL_TEST_NAME}.cc) @@ -192,7 +215,7 @@ function(ADD_PARQUET_TEST REL_TEST_NAME) -DGTEST_USE_OWN_TR1_TUPLE=0) endif() - target_link_libraries(${TEST_NAME} ${PARQUET_TEST_LINK_LIBS}) + target_link_libraries(${TEST_NAME} ${TEST_LINK_LIBS}) else() # No executable, just invoke the test (probably a script) directly. set(TEST_PATH ${CMAKE_CURRENT_SOURCE_DIR}/${REL_TEST_NAME}) @@ -417,10 +440,13 @@ endif() # Test linking set(PARQUET_MIN_TEST_LIBS - parquet_test_main + parquet_test_main) + +set(PARQUET_TEST_LINK_LIBS ${PARQUET_MIN_TEST_LIBS} parquet_static) -set(PARQUET_TEST_LINK_LIBS ${PARQUET_MIN_TEST_LIBS}) +set(PARQUET_TEST_SHARED_LINK_LIBS ${PARQUET_MIN_TEST_LIBS} + parquet_shared) ############################################################# # Benchmark linking @@ -460,6 +486,7 @@ endif() # Library config set(LIBPARQUET_SRCS + src/parquet/exception.cc src/parquet/types.cc src/parquet/column/levels.cc @@ -533,8 +560,8 @@ if (PARQUET_BUILD_STATIC) LIBRARY_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}" OUTPUT_NAME "parquet") target_link_libraries(parquet_static - LINK_PUBLIC ${LIBPARQUET_LINK_LIBS} - LINK_PRIVATE ${LIBPARQUET_PRIVATE_LINK_LIBS}) + LINK_PUBLIC ${LIBPARQUET_LINK_LIBS} + LINK_PRIVATE ${LIBPARQUET_PRIVATE_LINK_LIBS}) endif() add_subdirectory(src/parquet) http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/616305cb/src/parquet/CMakeLists.txt ---------------------------------------------------------------------- diff --git a/src/parquet/CMakeLists.txt b/src/parquet/CMakeLists.txt index a2ebbad..7d4e905 100644 --- a/src/parquet/CMakeLists.txt +++ b/src/parquet/CMakeLists.txt @@ -21,6 +21,8 @@ install(FILES types.h DESTINATION include/parquet) -ADD_PARQUET_TEST(public-api-test) +ADD_PARQUET_TEST(public-api-test + LINKAGE shared) + ADD_PARQUET_TEST(types-test) ADD_PARQUET_TEST(reader-test) http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/616305cb/src/parquet/exception.cc ---------------------------------------------------------------------- diff --git a/src/parquet/exception.cc b/src/parquet/exception.cc new file mode 100644 index 0000000..0973e5a --- /dev/null +++ b/src/parquet/exception.cc @@ -0,0 +1,48 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +#include "parquet/exception.h" + +#include <exception> +#include <sstream> +#include <string> + +namespace parquet { + +void ParquetException::EofException() { + throw ParquetException("Unexpected end of stream."); +} + +void ParquetException::NYI(const std::string& msg) { + std::stringstream ss; + ss << "Not yet implemented: " << msg << "."; + throw ParquetException(ss.str()); +} + +ParquetException::ParquetException(const char* msg) : msg_(msg) {} + +ParquetException::ParquetException(const std::string& msg) : msg_(msg) {} + +ParquetException::ParquetException(const char* msg, std::exception& e) : msg_(msg) {} + +ParquetException::~ParquetException() throw() {} + +const char* ParquetException::what() const throw() { + return msg_.c_str(); +} + +} // namespace parquet http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/616305cb/src/parquet/exception.h ---------------------------------------------------------------------- diff --git a/src/parquet/exception.h b/src/parquet/exception.h index 6e6589e..3851018 100644 --- a/src/parquet/exception.h +++ b/src/parquet/exception.h @@ -19,28 +19,23 @@ #define PARQUET_EXCEPTION_H #include <exception> -#include <sstream> #include <string> #include "parquet/util/visibility.h" namespace parquet { -class ParquetException : public std::exception { +class PARQUET_EXPORT ParquetException : public std::exception { public: - static void EofException() { throw ParquetException("Unexpected end of stream."); } - static void NYI(const std::string& msg) { - std::stringstream ss; - ss << "Not yet implemented: " << msg << "."; - throw ParquetException(ss.str()); - } - - explicit ParquetException(const char* msg) : msg_(msg) {} - explicit ParquetException(const std::string& msg) : msg_(msg) {} - explicit ParquetException(const char* msg, exception& e) : msg_(msg) {} - - virtual ~ParquetException() throw() {} - virtual const char* what() const throw() { return msg_.c_str(); } + static void EofException(); + static void NYI(const std::string& msg); + + explicit ParquetException(const char* msg); + explicit ParquetException(const std::string& msg); + explicit ParquetException(const char* msg, exception& e); + + virtual ~ParquetException() throw(); + virtual const char* what() const throw(); private: std::string msg_; http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/616305cb/src/parquet/public-api-test.cc ---------------------------------------------------------------------- diff --git a/src/parquet/public-api-test.cc b/src/parquet/public-api-test.cc index e307f52..a5c88da 100644 --- a/src/parquet/public-api-test.cc +++ b/src/parquet/public-api-test.cc @@ -22,8 +22,6 @@ #include "parquet/api/schema.h" #include "parquet/api/writer.h" -namespace parquet { - TEST(TestPublicAPI, DoesNotIncludeThrift) { #ifdef _THRIFT_THRIFT_H_ FAIL() << "Thrift headers should not be in the public API"; @@ -36,4 +34,10 @@ TEST(TestPublicAPI, DoesNotExportDCHECK) { #endif } -} // namespace parquet +void ThrowsParquetException() { + throw parquet::ParquetException("This function throws"); +} + +TEST(TestPublicAPI, CanThrowParquetException) { + ASSERT_THROW(ThrowsParquetException(), parquet::ParquetException); +} http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/616305cb/src/parquet/util/logging.h ---------------------------------------------------------------------- diff --git a/src/parquet/util/logging.h b/src/parquet/util/logging.h index 3c873e3..4f2091b 100644 --- a/src/parquet/util/logging.h +++ b/src/parquet/util/logging.h @@ -44,9 +44,9 @@ namespace parquet { #ifdef NDEBUG #define PARQUET_DFATAL PARQUET_WARNING -#define DCHECK(condition) \ - PARQUET_IGNORE_EXPR(condition)\ - while (false) \ +#define DCHECK(condition) \ + PARQUET_IGNORE_EXPR(condition) \ + while (false) \ parquet::internal::NullLog() #define DCHECK_EQ(val1, val2) \ PARQUET_IGNORE_EXPR(val1) \ http://git-wip-us.apache.org/repos/asf/parquet-cpp/blob/616305cb/src/parquet/util/output.cc ---------------------------------------------------------------------- diff --git a/src/parquet/util/output.cc b/src/parquet/util/output.cc index 6fc4ed8..422000f 100644 --- a/src/parquet/util/output.cc +++ b/src/parquet/util/output.cc @@ -19,6 +19,7 @@ #include <cstring> #include <memory> +#include <sstream> #include "parquet/exception.h" #include "parquet/util/buffer.h"
