ARROW-699: [C++] Resolve Arrow and Arrow IPC build issues on Windows;

appveyor.yml

@@ -23,17 +23,19 @@ environment:
     - GENERATOR: Visual Studio 14 2015 Win64
     # - GENERATOR: Visual Studio 14 2015
   MSVC_DEFAULT_OPTIONS: ON
-  BOOST_ROOT: C:\Libraries\boost_1_59_0
-  BOOST_LIBRARYDIR: C:\Libraries\boost_1_59_0\lib64-msvc-14.0
+  BOOST_ROOT: C:\Libraries\boost_1_63_0
+  BOOST_LIBRARYDIR: C:\Libraries\boost_1_63_0\lib64-msvc-14.0
 
 build_script:
  - cd cpp
  - mkdir build
  - cd build
  # A lot of features are still deactivated as they do not build on Windows
  #  * gbenchmark doesn't build with MSVC
- - cmake -G "%GENERATOR%" -DARROW_BOOST_USE_SHARED=OFF -DARROW_IPC=OFF -DARROW_HDFS=OFF -DARROW_BUILD_BENCHMARKS=OFF -DARROW_JEMALLOC=OFF ..
- - cmake --build . --config Debug
+ - cmake -G "%GENERATOR%" -DARROW_BOOST_USE_SHARED=OFF -DARROW_BUILD_BENCHMARKS=OFF -DARROW_JEMALLOC=OFF -DCMAKE_BUILD_TYPE=Release ..
+ - cmake --build . --config Release
 
 # test_script:
-#  - ctest -VV
+ - ctest -VV
+
+

cpp/CMakeLists.txt

@@ -500,7 +500,11 @@ if(ARROW_BUILD_TESTS)
     set(GFLAGS_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/gflags_ep-prefix/src/gflags_ep")
     set(GFLAGS_HOME "${GFLAGS_PREFIX}")
     set(GFLAGS_INCLUDE_DIR "${GFLAGS_PREFIX}/include")
-    set(GFLAGS_STATIC_LIB "${GFLAGS_PREFIX}/lib/libgflags.a")
+    if(MSVC)
+      set(GFLAGS_STATIC_LIB "${GFLAGS_PREFIX}/lib/gflags_static.lib")
+    else()
+      set(GFLAGS_STATIC_LIB "${GFLAGS_PREFIX}/lib/libgflags.a")
+    endif()
     set(GFLAGS_VENDORED 1)
     set(GFLAGS_CMAKE_ARGS -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
                           -DCMAKE_INSTALL_PREFIX=${GFLAGS_PREFIX}
@@ -536,6 +540,11 @@ if(ARROW_BUILD_TESTS)
   include_directories(SYSTEM ${GFLAGS_INCLUDE_DIR})
   ADD_THIRDPARTY_LIB(gflags
     STATIC_LIB ${GFLAGS_STATIC_LIB})
+  if(MSVC)
+    set_target_properties(gflags
+      PROPERTIES
+      IMPORTED_LINK_INTERFACE_LIBRARIES "shlwapi.lib")
+  endif()
 
   if(GFLAGS_VENDORED)
     add_dependencies(gflags gflags_ep)

cpp/cmake_modules/BuildUtils.cmake

@@ -125,6 +125,8 @@ function(ADD_ARROW_LIB LIB_NAME)
     set_target_properties(${LIB_NAME}_shared
       PROPERTIES
       LIBRARY_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}"
+      RUNTIME_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}"
+      PDB_OUTPUT_DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}"
       LINK_FLAGS "${ARG_SHARED_LINK_FLAGS}"
       OUTPUT_NAME ${LIB_NAME}
       VERSION "${ARROW_ABI_VERSION}"

cpp/src/arrow/io/file.cc

@@ -152,20 +152,30 @@ static inline int64_t lseek64_compat(int fd, int64_t pos, int whence) {
 #endif
 }
 
+#if defined(_MSC_VER)
+static inline Status ConvertToUtf16(const std::string& input, std::wstring* result) {
+  if (result == nullptr) { return Status::Invalid("Pointer to result is not valid"); }
+
+  if (input.empty()) {
+    *result = std::wstring();
+    return Status::OK();
+  }
+
+  std::wstring_convert<std::codecvt_utf8_utf16<wchar_t>> utf16_converter;
+  *result = utf16_converter.from_bytes(input);
+  return Status::OK();
+}
+#endif
+
 static inline Status FileOpenReadable(const std::string& filename, int* fd) {
   int ret;
   errno_t errno_actual = 0;
 #if defined(_MSC_VER)
-  // https://msdn.microsoft.com/en-us/library/w64k0ytk.aspx
-
-  // See GH #209. Here we are assuming that the filename has been encoded in
-  // utf-16le so that unicode filenames can be supported
-  const int nwchars = static_cast<int>(filename.size()) / sizeof(wchar_t);
-  std::vector<wchar_t> wpath(nwchars + 1);
-  memcpy(wpath.data(), filename.data(), filename.size());
-  memcpy(wpath.data() + nwchars, L"\0", sizeof(wchar_t));
+  std::wstring wide_filename;
+  RETURN_NOT_OK(ConvertToUtf16(filename, &wide_filename));
 
-  errno_actual = _wsopen_s(fd, wpath.data(), _O_RDONLY | _O_BINARY, _SH_DENYNO, _S_IREAD);
+  errno_actual =
+      _wsopen_s(fd, wide_filename.c_str(), _O_RDONLY | _O_BINARY, _SH_DENYNO, _S_IREAD);
   ret = *fd;
 #else
   ret = *fd = open(filename.c_str(), O_RDONLY | O_BINARY);
@@ -181,16 +191,12 @@ static inline Status FileOpenWriteable(
   errno_t errno_actual = 0;
 
 #if defined(_MSC_VER)
-  // https://msdn.microsoft.com/en-us/library/w64k0ytk.aspx
-  // Same story with wchar_t as above
-  const int nwchars = static_cast<int>(filename.size()) / sizeof(wchar_t);
-  std::vector<wchar_t> wpath(nwchars + 1);
-  memcpy(wpath.data(), filename.data(), filename.size());
-  memcpy(wpath.data() + nwchars, L"\0", sizeof(wchar_t));
+  std::wstring wide_filename;
+  RETURN_NOT_OK(ConvertToUtf16(filename, &wide_filename));
 
   int oflag = _O_CREAT | _O_BINARY;
-  int sh_flag = _S_IWRITE;
-  if (!write_only) { sh_flag |= _S_IREAD; }
+  int pmode = _S_IWRITE;
+  if (!write_only) { pmode |= _S_IREAD; }
 
   if (truncate) { oflag |= _O_TRUNC; }
 
@@ -200,7 +206,7 @@ static inline Status FileOpenWriteable(
     oflag |= _O_RDWR;
   }
 
-  errno_actual = _wsopen_s(fd, wpath.data(), oflag, _SH_DENYNO, sh_flag);
+  errno_actual = _wsopen_s(fd, wide_filename.c_str(), oflag, _SH_DENYNO, pmode);
   ret = *fd;
 
 #else

cpp/src/arrow/io/io-file-test.cc

@@ -41,14 +41,29 @@ static bool FileExists(const std::string& path) {
   return std::ifstream(path.c_str()).good();
 }
 
+#if defined(_MSC_VER)
+void InvalidParamHandler(const wchar_t* expr, const wchar_t* func,
+    const wchar_t* source_file, unsigned int source_line, uintptr_t reserved) {
+  wprintf(L"Invalid parameter in funcion %s. Source: %s line %d expression %s", func,
+      source_file, source_line, expr);
+}
+#endif
+
 static bool FileIsClosed(int fd) {
-#ifdef _MSC_VER
-  // Close file a second time, this should set errno to EBADF
-  close(fd);
+#if defined(_MSC_VER)
+  // Disables default behavior on wrong params which causes the application to crash
+  // https://msdn.microsoft.com/en-us/library/ksazx244.aspx
+  _set_invalid_parameter_handler(InvalidParamHandler);
+
+  // Disables possible assertion alert box on invalid input arguments
+  _CrtSetReportMode(_CRT_ASSERT, 0);
+
+  int ret = static_cast<int>(_close(fd));
+  return (ret == -1);
 #else
   if (-1 != fcntl(fd, F_GETFD)) { return false; }
-#endif
   return errno == EBADF;
+#endif
 }
 
 class FileTestFixture : public ::testing::Test {

cpp/src/arrow/io/io-hdfs-test.cc

@@ -78,8 +78,9 @@ class TestHdfsClient : public ::testing::Test {
     LibHdfsShim* driver_shim;
 
     client_ = nullptr;
-    scratch_dir_ =
-        boost::filesystem::unique_path("/tmp/arrow-hdfs/scratch-%%%%").string();
+    scratch_dir_ = boost::filesystem::unique_path(
+        boost::filesystem::temp_directory_path() / "arrow-hdfs/scratch-%%%%")
+                       .string();
 
     loaded_driver_ = false;
 

cpp/src/arrow/io/test-common.h

@@ -69,13 +69,15 @@ class MemoryMapFixture {
 
   void CreateFile(const std::string path, int64_t size) {
     FILE* file = fopen(path.c_str(), "w");
-    if (file != nullptr) { tmp_files_.push_back(path); }
+    if (file != nullptr) {
+      tmp_files_.push_back(path);
 #ifdef _MSC_VER
-    _chsize(fileno(file), static_cast<size_t>(size));
+      _chsize(fileno(file), static_cast<size_t>(size));
 #else
-    ftruncate(fileno(file), static_cast<size_t>(size));
+      ftruncate(fileno(file), static_cast<size_t>(size));
 #endif
-    fclose(file);
+      fclose(file);
+    }
   }
 
   Status InitMemoryMap(

cpp/src/arrow/ipc/CMakeLists.txt

@@ -85,6 +85,15 @@ if (ARROW_BUILD_TESTS)
       dl)
     set_target_properties(json-integration-test
       PROPERTIES LINK_FLAGS "-undefined dynamic_lookup")
+  elseif (MSVC)
+    target_link_libraries(json-integration-test
+      arrow_ipc_static
+      arrow_io_static
+      arrow_static
+      gflags
+      gtest
+      ${BOOST_FILESYSTEM_LIBRARY}
+      ${BOOST_SYSTEM_LIBRARY})
   else()
     target_link_libraries(json-integration-test
       arrow_ipc_static
@@ -156,14 +165,22 @@ install(
   FILES "${CMAKE_CURRENT_BINARY_DIR}/arrow-ipc.pc"
   DESTINATION "${CMAKE_INSTALL_LIBDIR}/pkgconfig/")
 
-
-set(UTIL_LINK_LIBS
-  arrow_ipc_static
-  arrow_io_static
-  arrow_static
-  ${BOOST_FILESYSTEM_LIBRARY}
-  ${BOOST_SYSTEM_LIBRARY}
-  dl)
+if(MSVC)
+  set(UTIL_LINK_LIBS
+    arrow_ipc_static
+    arrow_io_static
+    arrow_static
+    ${BOOST_FILESYSTEM_LIBRARY}
+    ${BOOST_SYSTEM_LIBRARY})
+else()
+  set(UTIL_LINK_LIBS
+    arrow_ipc_static
+    arrow_io_static
+    arrow_static
+    ${BOOST_FILESYSTEM_LIBRARY}
+    ${BOOST_SYSTEM_LIBRARY}
+    dl)
+endif()
 
 if (ARROW_BUILD_UTILITIES)
   add_executable(file-to-stream file-to-stream.cc)

cpp/src/arrow/ipc/metadata.cc

@@ -768,6 +768,8 @@ Message::Message(const std::shared_ptr<Buffer>& buffer, int64_t offset) {
   impl_.reset(new MessageImpl(buffer, offset));
 }
 
+Message::~Message() {}
+
 Status Message::Open(const std::shared_ptr<Buffer>& buffer, int64_t offset,
     std::shared_ptr<Message>* out) {
   // ctor is private

cpp/src/arrow/ipc/metadata.h

@@ -140,6 +140,7 @@ struct ARROW_EXPORT BufferMetadata {
 
 class ARROW_EXPORT Message {
  public:
+  ~Message();
   enum Type { NONE, SCHEMA, DICTIONARY_BATCH, RECORD_BATCH };
 
   static Status Open(const std::shared_ptr<Buffer>& buffer, int64_t offset,

cpp/src/arrow/ipc/writer.cc

@@ -663,6 +663,8 @@ Status StreamWriter::WriteRecordBatch(const RecordBatch& batch, bool allow_64bit
   return impl_->WriteRecordBatch(batch, allow_64bit);
 }
 
+StreamWriter::~StreamWriter() {}
+
 void StreamWriter::set_memory_pool(MemoryPool* pool) {
   impl_->set_memory_pool(pool);
 }
@@ -719,6 +721,8 @@ FileWriter::FileWriter() {
   impl_.reset(new FileWriterImpl());
 }
 
+FileWriter::~FileWriter() {}
+
 Status FileWriter::Open(io::OutputStream* sink, const std::shared_ptr<Schema>& schema,
     std::shared_ptr<FileWriter>* out) {
   *out = std::shared_ptr<FileWriter>(new FileWriter());  // ctor is private

cpp/src/arrow/ipc/writer.h

@@ -82,7 +82,7 @@ Status GetRecordBatchSize(const RecordBatch& batch, int64_t* size);
 
 class ARROW_EXPORT StreamWriter {
  public:
-  virtual ~StreamWriter() = default;
+  virtual ~StreamWriter();
 
   static Status Open(io::OutputStream* sink, const std::shared_ptr<Schema>& schema,
       std::shared_ptr<StreamWriter>* out);
@@ -105,6 +105,8 @@ class ARROW_EXPORT StreamWriter {
 
 class ARROW_EXPORT FileWriter : public StreamWriter {
  public:
+  virtual ~FileWriter();
+
   static Status Open(io::OutputStream* sink, const std::shared_ptr<Schema>& schema,
       std::shared_ptr<FileWriter>* out);