szaszm commented on a change in pull request #979: URL: https://github.com/apache/nifi-minifi-cpp/pull/979#discussion_r566260078
########## File path: extensions/azure/CMakeLists.txt ########## @@ -0,0 +1,37 @@ +# +# 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(${CMAKE_SOURCE_DIR}/extensions/ExtensionHeader.txt) + +set(CMAKE_CXX_STANDARD 14) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +include_directories(controllerservices processors storage ${CMAKE_SOURCE_DIR}/extensions/azure) Review comment: Avoid global commands like `include_directories` and `link_libraries`. Use target equivalents like `target_include_directories` and `target_link_libraries`. For example, you can set the required standard with `target_compile_features(minifi-azure PUBLIC cxx_std_14)` ########## File path: cmake/BundledAzureSdkCpp.cmake ########## @@ -0,0 +1,126 @@ +# 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. + +function(use_bundled_libazure SOURCE_DIR BINARY_DIR) + # Define byproducts + if (WIN32) + set(SUFFIX "lib") + set(PREFIX "") + set(AZURE_CORE_LIB "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/core/azure-core/${CMAKE_BUILD_TYPE}/${PREFIX}azure-core.${SUFFIX}") + set(AZURE_STORAGE_COMMON_LIB "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/storage/azure-storage-common/${CMAKE_BUILD_TYPE}/${PREFIX}azure-storage-common.${SUFFIX}") + set(AZURE_STORAGE_BLOBS_LIB "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/storage/azure-storage-blobs/${CMAKE_BUILD_TYPE}/${PREFIX}azure-storage-blobs.${SUFFIX}") + set(AZURE_IDENTITY_LIB "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/identity/azure-identity/${CMAKE_BUILD_TYPE}/${PREFIX}azure-identity.${SUFFIX}") + else() + set(SUFFIX "a") + set(PREFIX "lib") + set(AZURE_CORE_LIB "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/core/azure-core/${PREFIX}azure-core.${SUFFIX}") + set(AZURE_STORAGE_COMMON_LIB "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/storage/azure-storage-common/${PREFIX}azure-storage-common.${SUFFIX}") + set(AZURE_STORAGE_BLOBS_LIB "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/storage/azure-storage-blobs/${PREFIX}azure-storage-blobs.${SUFFIX}") + set(AZURE_IDENTITY_LIB "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/identity/azure-identity/${PREFIX}azure-identity.${SUFFIX}") + endif() + + set(AZURESDK_LIBRARIES_LIST + "${AZURE_CORE_LIB}" + "${AZURE_STORAGE_COMMON_LIB}" + "${AZURE_STORAGE_BLOBS_LIB}" + "${AZURE_IDENTITY_LIB}") + + set(AZURE_SDK_CMAKE_ARGS ${PASSTHROUGH_CMAKE_ARGS}) + append_third_party_passthrough_args(AZURE_SDK_CMAKE_ARGS "${AZURE_SDK_CMAKE_ARGS}") + + # Build project + ExternalProject_Add( + azure-sdk-cpp-external + GIT_REPOSITORY "https://github.com/Azure/azure-sdk-for-cpp.git"; + GIT_TAG "azure-storage-blobs_12.0.0-beta.6" + BUILD_IN_SOURCE true + SOURCE_DIR "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src" + BUILD_BYPRODUCTS "${AZURESDK_LIBRARIES_LIST}" + EXCLUDE_FROM_ALL TRUE + STEP_TARGETS build + CMAKE_ARGS ${AZURE_SDK_CMAKE_ARGS} + LIST_SEPARATOR % # This is needed for passing semicolon-separated lists + DEPENDS libxml2-external + ) + + # Set dependencies + add_dependencies(azure-sdk-cpp-external LibXml2::LibXml2 CURL::libcurl OpenSSL::Crypto OpenSSL::SSL nlohmann_json::nlohmann_json) + + # Set variables + set(LIBAZURE_FOUND "YES" CACHE STRING "" FORCE) + set(LIBAZURE_INCLUDE_DIRS + "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/core/azure-core/inc/" + "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/storage/azure-storage-blobs/inc/" + "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/storage/azure-storage-common/inc/" + "${BINARY_DIR}/thirdparty/azure-sdk-cpp-src/sdk/identity/azure-identity/inc/" + CACHE STRING "" FORCE) + set(LIBAZURE_LIBRARIES ${AZURESDK_LIBRARIES_LIST} CACHE STRING "" FORCE) + + # Create imported targets + FOREACH(LIBAZURE_INCLUDE_DIR ${LIBAZURE_INCLUDE_DIRS}) + file(MAKE_DIRECTORY ${LIBAZURE_INCLUDE_DIR}) + ENDFOREACH(LIBAZURE_INCLUDE_DIR) + + add_library(AZURE::azure-core STATIC IMPORTED) + set_target_properties(AZURE::azure-core PROPERTIES IMPORTED_LOCATION "${AZURE_CORE_LIB}") + add_dependencies(AZURE::azure-core azure-sdk-cpp-external-build) + set_property(TARGET AZURE::azure-core APPEND PROPERTY INTERFACE_INCLUDE_DIRECTORIES ${LIBAZURE_INCLUDE_DIRS}) + set_property(TARGET AZURE::azure-core APPEND PROPERTY INTERFACE_LINK_LIBRARIES LibXml2::LibXml2 CURL::libcurl OpenSSL::Crypto OpenSSL::SSL Threads::Threads nlohmann_json::nlohmann_json) + if (APPLE) + set_property(TARGET AZURE::azure-core APPEND PROPERTY INTERFACE_LINK_LIBRARIES "-framework CoreFoundation") + endif() + if (WIN32) + set_property(TARGET AZURE::azure-core APPEND PROPERTY INTERFACE_LINK_LIBRARIES winhttp.lib) + endif() Review comment: Instead of setting properties directly, we should prefer using target commands to set them indirectly. This way is shorter, looks better and is more commonly found in examples and docs. ```suggestion target_include_directories(AZURE::azure-core INTERFACE ${LIBAZURE_INCLUDE_DIRS}) target_link_libraries(AZURE::azure-core INTERFACE LibXml2::LibXml2 CURL::libcurl OpenSSL::Crypto OpenSSL::SSL Threads::Threads nlohmann_json::nlohmann_json) if (APPLE) target_link_libraries(AZURE::azure-core INTERFACE "-framework CoreFoundation") endif() if (WIN32) target_link_libraries(AZURE::azure-core INTERFACE winhttp.lib) endif() ``` ########## File path: NOTICE ########## @@ -55,6 +58,7 @@ This software includes third party software subject to the following copyrights: - libsodium - Copyright (c) 2013 - 2018 Frank Denis under the ISC software license - IANA timezone database - public domain - date (HowardHinnant/date) - notices below +- JSON for Modern C++ (nlohmann/json) - Copyright (c) 2013-2021 Niels Lohmann Review comment: Is this a hard dependency of the azure sdk? Because we already have `rapidjson` for json handling, so if it's possible to avoid bringing in another lib for the same purpose, I would try. ########## File path: win_build_vs.bat ########## @@ -47,6 +47,7 @@ for %%x in (%*) do ( if [%%~x] EQU [/M] set installer_merge_modules=ON if [%%~x] EQU [/C] set build_coap=ON if [%%~x] EQU [/A] set build_AWS=ON + if [%%~x] EQU [/Z] set build_azure=ON Review comment: Should probably set it to OFF by default near line 30 ########## File path: LICENSE ########## @@ -2958,3 +2958,53 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. -------------------------------------------------------------------------- This product bundles the IANA timezone database which is in the public domain. + +-------------------------------------------------------------------------- + +This project bundles 'azure-sdk-for-cpp', which is available under an MIT License: +MIT License + +Copyright (c) Microsoft Corporation. + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE + +-------------------------------------------------------------------------- + +This product bundles 'JSON for Modern C++' which is available under a MIT license: +MIT License + +Copyright (c) 2013-2021 Niels Lohmann + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. Review comment: `msi/LICENSE.txt` should be a copy of this file. I've just submitted a pull request #983 to synchronize it, because it is out of date. Please consider updating it in this PR as well, so that the merge conflict will remind us to check both while merging. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
