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 <wesm@apache.org>

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.

Author: wesm

.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

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)

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)

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

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_;

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);
+}

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)   \

src/parquet/util/output.cc

@@ -19,6 +19,7 @@
 
 #include <cstring>
 #include <memory>
+#include <sstream>
 
 #include "parquet/exception.h"
 #include "parquet/util/buffer.h"