You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/04/22 09:02:12 UTC
[GitHub] [arrow] kou commented on a diff in pull request #12914: ARROW-2034: [C++] Filesystem implementation for Azure Blob Storage
kou commented on code in PR #12914:
URL: https://github.com/apache/arrow/pull/12914#discussion_r855924917
##########
cpp/CMakeLists.txt:
##########
@@ -693,7 +693,9 @@ endif()
# Libraries to link statically with libarrow.so
set(ARROW_LINK_LIBS)
+set(ARROW_AZURE_LINK_LIBS)
set(ARROW_STATIC_LINK_LIBS)
+set(ARROW_AZURE_STATIC_LINK_LIBS)
Review Comment:
Could you use `ARROW_LINK_LIBS` and `ARROW_STATIC_LINK_LIBS` instead of add new `ARROW_AZURE_*` variables like S3 and GCS?
##########
cpp/cmake_modules/BuildUtils.cmake:
##########
@@ -46,6 +46,260 @@ if(WIN32 AND CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
list(APPEND ARROW_BOOST_PROCESS_COMPILE_DEFINITIONS "BOOST_USE_WINDOWS_H=1")
endif()
+function(ADD_ARROW_LIB_AZURE LIB_NAME)
Review Comment:
Can we use `add_arrow_lib` instead of defining this?
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -4513,6 +4513,53 @@ if(ARROW_S3)
endif()
endif()
+macro(build_azuresdk)
+ message(STATUS "Building Azure C++ SDK from source")
+
+ set(AZURESDK_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/azuresdk_ep-install")
+ set(AZURESDK_INCLUDE_DIR "${AZURESDK_PREFIX}/include")
+
+ set(AZURESDK_CMAKE_ARGS
+ ${EP_COMMON_CMAKE_ARGS}
+ -DBUILD_TESTING=OFF
+ -DCMAKE_INSTALL_LIBDIR=lib
+ "-DCMAKE_INSTALL_PREFIX=${AZURESDK_PREFIX}"
+ -DCMAKE_PREFIX_PATH=${AZURESDK_PREFIX})
+
+ file(MAKE_DIRECTORY ${AZURESDK_INCLUDE_DIR})
+
+ # Azure C++ SDK related libraries to link statically
+ set(_AZURESDK_LIBS
+ azure-core
+ azure-identity
+ azure-storage-blobs
+ azure-storage-common
+ azure-storage-files-datalake)
+ set(AZURESDK_LIBRARIES)
+ set(AZURESDK_LIBRARIES_CPP)
+ foreach(_AZURESDK_LIB ${_AZURESDK_LIBS})
+ string(TOUPPER ${_AZURESDK_LIB} _AZURESDK_LIB_UPPER)
+ string(REPLACE "-" "_" _AZURESDK_LIB_NAME_PREFIX ${_AZURESDK_LIB_UPPER})
+ list(APPEND AZURESDK_LIBRARIES_CPP "${_AZURESDK_LIB}-cpp")
+ set(_AZURESDK_TARGET_NAME Azure::${_AZURESDK_LIB})
+ list(APPEND AZURESDK_LIBRARIES ${_AZURESDK_TARGET_NAME})
+ endforeach()
+
+ set(AZURESDK_LINK_LIBRARIES ${AZURESDK_LIBRARIES})
+endmacro()
+
+if(ARROW_AZURE)
+ build_azuresdk()
+
+ foreach(AZURESDK_LIBRARY_CPP ${AZURESDK_LIBRARIES_CPP})
+ find_package(${AZURESDK_LIBRARY_CPP} CONFIG REQUIRED)
+ endforeach()
+
+ include_directories(SYSTEM ${AZURESDK_INCLUDE_DIR})
+ message(STATUS "Found AZURE SDK headers: ${AZURESDK_INCLUDE_DIR}")
+ message(STATUS "Found AZURE SDK libraries: ${AZURESDK_LINK_LIBRARIES}")
Review Comment:
```suggestion
message(STATUS "Found Azure SDK headers: ${AZURESDK_INCLUDE_DIR}")
message(STATUS "Found Azure SDK libraries: ${AZURESDK_LINK_LIBRARIES}")
```
##########
cpp/src/arrow/filesystem/azure/CMakeLists.txt:
##########
@@ -0,0 +1,36 @@
+# 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.
+
+set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD_REQUIRED ON)
Review Comment:
Could you use `CXX_STANDARD` and `CXX_STANDARD_REQUIRED` target properties instead?
* https://cmake.org/cmake/help/latest/prop_tgt/CXX_STANDARD.html#prop_tgt:CXX_STANDARD
* https://cmake.org/cmake/help/latest/prop_tgt/CXX_STANDARD_REQUIRED.html#prop_tgt:CXX_STANDARD_REQUIRED
##########
cpp/src/arrow/filesystem/azure/azurefs.cc:
##########
@@ -0,0 +1,1651 @@
+// 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 "arrow/filesystem/azure/azurefs.h"
+
+#include <algorithm>
+#include <atomic>
+#include <azure/core/credentials/credentials.hpp>
+#include <azure/identity/client_secret_credential.hpp>
+#include <azure/identity/managed_identity_credential.hpp>
+#include <azure/storage/blobs.hpp>
+#include <azure/storage/files/datalake.hpp>
+#include <chrono>
+#include <condition_variable>
+#include <functional>
+#include <memory>
+#include <mutex>
+#include <sstream>
+#include <thread>
+#include <unordered_map>
+#include <utility>
+
+#ifdef _WIN32
+// Undefine preprocessor macros that interfere with AWS function / method names
+#ifdef GetMessage
+#undef GetMessage
+#endif
+#ifdef GetObject
+#undef GetObject
+#endif
+#endif
+
+#include "arrow/util/windows_fixup.h"
+
+#include "arrow/buffer.h"
+#include "arrow/filesystem/filesystem.h"
+#include "arrow/filesystem/path_util.h"
+#include "arrow/filesystem/util_internal.h"
+#include "arrow/io/interfaces.h"
+#include "arrow/io/memory.h"
+#include "arrow/io/util_internal.h"
+#include "arrow/result.h"
+#include "arrow/status.h"
+#include "arrow/util/async_generator.h"
+#include "arrow/util/atomic_shared_ptr.h"
+#include "arrow/util/checked_cast.h"
+#include "arrow/util/future.h"
+#include "arrow/util/key_value_metadata.h"
+#include "arrow/util/logging.h"
+#include "arrow/util/optional.h"
+#include "arrow/util/task_group.h"
+#include "arrow/util/thread_pool.h"
+
+namespace arrow {
+
+using internal::Uri;
+
+namespace fs {
+
+static const char kSep = '/';
+
+// -----------------------------------------------------------------------
+// AzureOptions implementation
+
+AzureOptions::AzureOptions() {}
+
+void AzureOptions::ConfigureAnonymousCredentials(const std::string& account_name) {
+ account_dfs_url = "https://" + account_name + ".dfs.core.windows.net/";
+ account_blob_url = "https://" + account_name + ".blob.core.windows.net/";
+ credentials_kind = AzureCredentialsKind::Anonymous;
+}
+
+void AzureOptions::ConfigureAccountKeyCredentials(const std::string& account_name,
+ const std::string& account_key) {
+ account_dfs_url = "https://" + account_name + ".dfs.core.windows.net/";
+ account_blob_url = "https://" + account_name + ".blob.core.windows.net/";
+ storage_credentials_provider =
+ std::make_shared<Azure::Storage::StorageSharedKeyCredential>(account_name,
+ account_key);
+ credentials_kind = AzureCredentialsKind::StorageCredentials;
+}
+
+void AzureOptions::ConfigureConnectionStringCredentials(
+ const std::string& connection_string_uri) {
+ auto account_name =
+ Azure::Storage::_internal::ParseConnectionString(connection_string_uri).AccountName;
Review Comment:
Can we use public API instead of internal API here?
##########
cpp/src/arrow/filesystem/azure/azurefs.cc:
##########
@@ -0,0 +1,1651 @@
+// 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 "arrow/filesystem/azure/azurefs.h"
+
+#include <algorithm>
+#include <atomic>
+#include <azure/core/credentials/credentials.hpp>
+#include <azure/identity/client_secret_credential.hpp>
+#include <azure/identity/managed_identity_credential.hpp>
+#include <azure/storage/blobs.hpp>
+#include <azure/storage/files/datalake.hpp>
+#include <chrono>
+#include <condition_variable>
+#include <functional>
+#include <memory>
+#include <mutex>
+#include <sstream>
+#include <thread>
+#include <unordered_map>
+#include <utility>
+
+#ifdef _WIN32
+// Undefine preprocessor macros that interfere with AWS function / method names
+#ifdef GetMessage
+#undef GetMessage
+#endif
+#ifdef GetObject
+#undef GetObject
+#endif
+#endif
+
+#include "arrow/util/windows_fixup.h"
+
+#include "arrow/buffer.h"
+#include "arrow/filesystem/filesystem.h"
+#include "arrow/filesystem/path_util.h"
+#include "arrow/filesystem/util_internal.h"
+#include "arrow/io/interfaces.h"
+#include "arrow/io/memory.h"
+#include "arrow/io/util_internal.h"
+#include "arrow/result.h"
+#include "arrow/status.h"
+#include "arrow/util/async_generator.h"
+#include "arrow/util/atomic_shared_ptr.h"
+#include "arrow/util/checked_cast.h"
+#include "arrow/util/future.h"
+#include "arrow/util/key_value_metadata.h"
+#include "arrow/util/logging.h"
+#include "arrow/util/optional.h"
+#include "arrow/util/task_group.h"
+#include "arrow/util/thread_pool.h"
+
+namespace arrow {
+
+using internal::Uri;
+
+namespace fs {
+
+static const char kSep = '/';
+
+// -----------------------------------------------------------------------
+// AzureOptions implementation
+
+AzureOptions::AzureOptions() {}
+
+void AzureOptions::ConfigureAnonymousCredentials(const std::string& account_name) {
+ account_dfs_url = "https://" + account_name + ".dfs.core.windows.net/";
+ account_blob_url = "https://" + account_name + ".blob.core.windows.net/";
+ credentials_kind = AzureCredentialsKind::Anonymous;
+}
+
+void AzureOptions::ConfigureAccountKeyCredentials(const std::string& account_name,
+ const std::string& account_key) {
+ account_dfs_url = "https://" + account_name + ".dfs.core.windows.net/";
+ account_blob_url = "https://" + account_name + ".blob.core.windows.net/";
+ storage_credentials_provider =
+ std::make_shared<Azure::Storage::StorageSharedKeyCredential>(account_name,
+ account_key);
+ credentials_kind = AzureCredentialsKind::StorageCredentials;
+}
+
+void AzureOptions::ConfigureConnectionStringCredentials(
+ const std::string& connection_string_uri) {
+ auto account_name =
+ Azure::Storage::_internal::ParseConnectionString(connection_string_uri).AccountName;
+ account_dfs_url = "https://" + account_name + ".dfs.core.windows.net/";
+ account_blob_url = "https://" + account_name + ".blob.core.windows.net/";
+ connection_string = connection_string_uri;
+ credentials_kind = AzureCredentialsKind::ConnectionString;
+}
+
+void AzureOptions::ConfigureServicePrincipleCredentials(
+ const std::string& account_name, const std::string& tenant_id,
+ const std::string& client_id, const std::string& client_secret) {
+ account_dfs_url = "https://" + account_name + ".dfs.core.windows.net/";
+ account_blob_url = "https://" + account_name + ".blob.core.windows.net/";
+ service_principle_credentials_provider =
+ std::make_shared<Azure::Identity::ClientSecretCredential>(tenant_id, client_id,
+ client_secret);
+ credentials_kind = AzureCredentialsKind::ServicePrincipleCredentials;
+}
+
+void AzureOptions::ConfigureSasCredentials(const std::string& uri) {
+ auto src = internal::RemoveTrailingSlash(uri);
+ auto first_sep = src.find_first_of("?");
+ sas_token = std::string(src.substr(first_sep));
+ account_blob_url = std::string(src.substr(0, first_sep));
+ src = internal::RemoveTrailingSlash(account_blob_url);
+ first_sep = src.find("blob.core.windows.net");
+ account_dfs_url = std::string(src.substr(0, first_sep)) + "dfs" +
+ std::string(src.substr(first_sep + 4)) + "/";
+ credentials_kind = AzureCredentialsKind::Sas;
+}
+
+bool AzureOptions::Equals(const AzureOptions& other) const {
+ return (scheme == other.scheme && account_dfs_url == other.account_dfs_url &&
+ account_blob_url == other.account_blob_url &&
+ credentials_kind == other.credentials_kind);
+}
+
+AzureOptions AzureOptions::FromAnonymous(const std::string account_name) {
+ AzureOptions options;
+ options.ConfigureAnonymousCredentials(account_name);
+ return options;
+}
+
+AzureOptions AzureOptions::FromAccountKey(const std::string& account_name,
+ const std::string& account_key) {
+ AzureOptions options;
+ options.ConfigureAccountKeyCredentials(account_name, account_key);
+ return options;
+}
+
+AzureOptions AzureOptions::FromConnectionString(const std::string& connection_string) {
+ AzureOptions options;
+ options.ConfigureConnectionStringCredentials(connection_string);
+ return options;
+}
+
+AzureOptions AzureOptions::FromServicePrincipleCredential(
+ const std::string& account_name, const std::string& tenant_id,
+ const std::string& client_id, const std::string& client_secret) {
+ AzureOptions options;
+ options.ConfigureServicePrincipleCredentials(account_name, tenant_id, client_id,
+ client_secret);
+ return options;
+}
+
+AzureOptions AzureOptions::FromSas(const std::string& uri) {
+ AzureOptions options;
+ options.ConfigureSasCredentials(uri);
+ return options;
+}
+
+Result<AzureOptions> AzureOptions::FromUri(const std::string& uri_string,
+ std::string* out_path) {
+ Uri uri;
+ RETURN_NOT_OK(uri.Parse(uri_string));
+ return FromUri(uri, out_path);
+}
+
+Result<AzureOptions> AzureOptions::FromUri(const Uri& uri, std::string* out_path) {
+ AzureOptions options;
+ AZURE_ASSERT(uri.has_host());
Review Comment:
It seems that this may call `std::abort()`.
Generally, we should not use `std::abort()` in a library.
If we need to use `std::abort()`, please use `ARROW_DCHECK` family.
##########
cpp/src/arrow/filesystem/azure/azurefs_test.cc:
##########
@@ -0,0 +1,1147 @@
+// 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 "arrow/filesystem/azure/azurefs.h"
+
+#include <chrono>
+#include <thread>
+
+#include <gmock/gmock-matchers.h>
+#include <gtest/gtest.h>
+#include <azure/storage/files/datalake.hpp>
+
+#include "arrow/filesystem/test_util.h"
+#include "arrow/testing/future_util.h"
+#include "arrow/testing/gtest_util.h"
+#include "arrow/testing/util.h"
+#include "arrow/util/key_value_metadata.h"
+#include "arrow/util/logging.h"
+#include "arrow/util/uri.h"
+
+namespace arrow {
+
+using internal::Uri;
+
+namespace fs {
+
+class AzureEnvTestMixin {
+ public:
+ static AzureOptions options_;
+ static std::shared_ptr<AzureBlobFileSystem> fs_;
+ static std::shared_ptr<Azure::Storage::Files::DataLake::DataLakeServiceClient>
+ gen2Client_;
+ static std::shared_ptr<Azure::Storage::Blobs::BlobServiceClient> gen1Client_;
+
+ AzureEnvTestMixin() {}
+
+ const std::string& GetAdlsGen2AccountName() {
+ static const std::string accountName = [&]() -> std::string {
+ return std::getenv("ADLS_GEN2_ACCOUNT_NAME");
+ }();
+ return accountName;
+ }
+
+ const std::string& GetAdlsGen2AccountKey() {
+ static const std::string accountKey = [&]() -> std::string {
+ return std::getenv("ADLS_GEN2_ACCOUNT_KEY");
+ }();
+ return accountKey;
+ }
+
+ const std::string& GetAdlsGen2ConnectionString() {
+ static const std::string connectionString = [&]() -> std::string {
+ return std::getenv("ADLS_GEN2_CONNECTION_STRING");
+ }();
+ return connectionString;
+ }
+
+ const std::string& GetAdlsGen2SasUrl() {
+ static const std::string sasUrl = [&]() -> std::string {
+ return std::getenv("ADLS_GEN2_SASURL");
+ }();
+ return sasUrl;
+ }
+
+ const std::string& GetAadTenantId() {
+ static const std::string tenantId = [&]() -> std::string {
+ return std::getenv("AAD_TENANT_ID");
+ }();
+ return tenantId;
+ }
+
+ const std::string& GetAadClientId() {
+ static const std::string clientId = [&]() -> std::string {
+ return std::getenv("AAD_CLIENT_ID");
+ }();
+ return clientId;
+ }
+
+ const std::string& GetAadClientSecret() {
+ static const std::string clientSecret = [&]() -> std::string {
+ return std::getenv("AAD_CLIENT_SECRET");
+ }();
+ return clientSecret;
+ }
+
+ // private:
+ // const std::string& AdlsGen2AccountName = std::getenv("ADLS_GEN2_ACCOUNT_NAME");
+ // const std::string& AdlsGen2AccountKey = std::getenv("ADLS_GEN2_ACCOUNT_KEY");
+ // const std::string& AdlsGen2ConnectionStringValue = std::getenv(
+ // "ADLS_GEN2_CONNECTION_STRING");
+ // const std::string& AdlsGen2SasUrl = std::getenv("ADLS_GEN2_SASURL");
+ // const std::string& AadTenantIdValue = std::getenv("AAD_TENANT_ID");
+ // const std::string& AadClientIdValue = std::getenv("AAD_CLIENT_ID");
+ // const std::string& AadClientSecretValue = std::getenv("AAD_CLIENT_SECRET");
Review Comment:
Should we remove them?
##########
cpp/src/arrow/filesystem/azure/azurefs.cc:
##########
@@ -0,0 +1,1651 @@
+// 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 "arrow/filesystem/azure/azurefs.h"
+
+#include <algorithm>
+#include <atomic>
+#include <azure/core/credentials/credentials.hpp>
+#include <azure/identity/client_secret_credential.hpp>
+#include <azure/identity/managed_identity_credential.hpp>
+#include <azure/storage/blobs.hpp>
+#include <azure/storage/files/datalake.hpp>
+#include <chrono>
+#include <condition_variable>
+#include <functional>
+#include <memory>
+#include <mutex>
+#include <sstream>
+#include <thread>
+#include <unordered_map>
+#include <utility>
+
+#ifdef _WIN32
+// Undefine preprocessor macros that interfere with AWS function / method names
+#ifdef GetMessage
+#undef GetMessage
+#endif
+#ifdef GetObject
+#undef GetObject
+#endif
+#endif
Review Comment:
Do we need this for Azure SDK for C++?
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -4513,6 +4513,53 @@ if(ARROW_S3)
endif()
endif()
+macro(build_azuresdk)
Review Comment:
Could you build Azure C++ SDK by `externalproject_add` in this?
##########
cpp/cmake_modules/BuildUtils.cmake:
##########
@@ -46,6 +46,260 @@ if(WIN32 AND CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
list(APPEND ARROW_BOOST_PROCESS_COMPILE_DEFINITIONS "BOOST_USE_WINDOWS_H=1")
endif()
+function(ADD_ARROW_LIB_AZURE LIB_NAME)
+ set(options)
+ set(one_value_args
+ BUILD_SHARED
+ BUILD_STATIC)
+ set(multi_value_args
+ SOURCES
+ STATIC_LINK_LIBS
+ SHARED_LINK_LIBS
+ DEPENDENCIES
+ SHARED_PRIVATE_LINK_LIBS
+ OUTPUT_PATH)
+ cmake_parse_arguments(ARG
+ "${options}"
+ "${one_value_args}"
+ "${multi_value_args}"
+ ${ARGN})
+ if(ARG_UNPARSED_ARGUMENTS)
+ message(SEND_ERROR "Error: unrecognized arguments: ${ARG_UNPARSED_ARGUMENTS}")
+ endif()
+
+ if(ARG_SOURCES)
+ set(SOURCES ${ARG_SOURCES})
+ else()
+ set(SOURCES "${LIB_NAME}.cc")
+ endif()
+
+ # Allow overriding ARROW_BUILD_SHARED and ARROW_BUILD_STATIC
+ if(DEFINED ARG_BUILD_SHARED)
+ set(BUILD_SHARED ${ARG_BUILD_SHARED})
+ else()
+ set(BUILD_SHARED ${ARROW_BUILD_SHARED})
+ endif()
+ if(DEFINED ARG_BUILD_STATIC)
+ set(BUILD_STATIC ${ARG_BUILD_STATIC})
+ else()
+ set(BUILD_STATIC ${ARROW_BUILD_STATIC})
+ endif()
+ if(ARG_OUTPUT_PATH)
+ set(OUTPUT_PATH ${ARG_OUTPUT_PATH})
+ else()
+ set(OUTPUT_PATH ${BUILD_OUTPUT_ROOT_DIRECTORY})
+ endif()
+
+ if(WIN32 OR (CMAKE_GENERATOR STREQUAL Xcode))
+ # We need to compile C++ separately for each library kind (shared and static)
+ # because of dllexport declarations on Windows.
+ # The Xcode generator doesn't reliably work with Xcode as target names are not
+ # guessed correctly.
+ set(USE_OBJLIB OFF)
+ else()
+ set(USE_OBJLIB ON)
+ endif()
+
+ if(USE_OBJLIB)
+ # Generate a single "objlib" from all C++ modules and link
+ # that "objlib" into each library kind, to avoid compiling twice
+ add_library(${LIB_NAME}_objlib OBJECT ${SOURCES})
+ # Necessary to make static linking into other shared libraries work properly
+ set_property(TARGET ${LIB_NAME}_objlib PROPERTY POSITION_INDEPENDENT_CODE 1)
+ set(LIB_DEPS $<TARGET_OBJECTS:${LIB_NAME}_objlib>)
+ else()
+ set(LIB_DEPS ${ARG_SOURCES})
+ endif()
+
+ set(RUNTIME_INSTALL_DIR bin)
+
+ if(BUILD_SHARED)
+ add_library(${LIB_NAME}_shared SHARED ${LIB_DEPS})
+
+ set_target_properties(${LIB_NAME}_shared
+ PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${OUTPUT_PATH}"
+ RUNTIME_OUTPUT_DIRECTORY "${OUTPUT_PATH}"
+ PDB_OUTPUT_DIRECTORY "${OUTPUT_PATH}"
+ OUTPUT_NAME ${LIB_NAME}
+ VERSION "${ARROW_FULL_SO_VERSION}"
+ SOVERSION "${ARROW_SO_VERSION}")
+
+ target_link_libraries(${LIB_NAME}_shared LINK_PRIVATE ${ARG_SHARED_PRIVATE_LINK_LIBS})
+
+ install(TARGETS ${LIB_NAME}_shared
+ EXPORT ${LIB_NAME}_targets
+ RUNTIME DESTINATION ${RUNTIME_INSTALL_DIR}
+ LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
+ ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
+ INCLUDES
+ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
+ endif()
+
+ if(BUILD_STATIC)
+ add_library(${LIB_NAME}_static SHARED ${LIB_DEPS})
+
+ if(MSVC_TOOLCHAIN)
+ set(LIB_NAME_STATIC ${LIB_NAME}_static)
+ else()
+ set(LIB_NAME_STATIC ${LIB_NAME})
+ endif()
+
+ if(ARROW_BUILD_STATIC AND WIN32)
+ target_compile_definitions(${LIB_NAME}_static PUBLIC ARROW_STATIC)
+ endif()
+
+ set_target_properties(${LIB_NAME}_static
+ PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${OUTPUT_PATH}"
+ OUTPUT_NAME ${LIB_NAME_STATIC})
+
+ install(TARGETS ${LIB_NAME}_static
+ EXPORT ${LIB_NAME}_targets
+ RUNTIME DESTINATION ${RUNTIME_INSTALL_DIR}
+ LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
+ ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
+ INCLUDES
+ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
+ endif()
+endfunction()
+
+function(ADD_TEST_CASE_AZURE REL_TEST_NAME)
Review Comment:
Can we use `add_test_case` instead of defining this?
##########
cpp/src/arrow/CMakeLists.txt:
##########
@@ -68,6 +68,47 @@ function(ADD_ARROW_TEST REL_TEST_NAME)
${ARG_UNPARSED_ARGUMENTS})
endfunction()
+function(ADD_ARROW_TEST_AZURE REL_TEST_NAME)
Review Comment:
Can we use `add_arrow_test` instead of defining this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org