You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "kou (via GitHub)" <gi...@apache.org> on 2023/10/10 05:40:26 UTC
[PR] GH-37510: [C++] Don't install bundled Azure SDK for C++ [arrow]
kou opened a new pull request, #38176:
URL: https://github.com/apache/arrow/pull/38176
### Rationale for this change
It's an internal bundled library. We should not install it as a part of Arrow.
### What changes are included in this PR?
Exclude all Azure SDK for C++ jobs including install jobs aren't executed by default. Building jobs are executed because they are required to build Arrow.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Yes.
--
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
Re: [PR] GH-37510: [C++] Don't install bundled Azure SDK for C++ [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #38176:
URL: https://github.com/apache/arrow/pull/38176#issuecomment-1758498643
I'll merge this for 14.0.0.
--
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
Re: [PR] GH-37510: [C++] Don't install bundled Azure SDK for C++ [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #38176:
URL: https://github.com/apache/arrow/pull/38176#issuecomment-1754424451
:warning: GitHub issue #37510 **has been automatically assigned in GitHub** to PR creator.
--
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
Re: [PR] GH-37510: [C++] Don't install bundled Azure SDK for C++ [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #38176:
URL: https://github.com/apache/arrow/pull/38176#discussion_r1354107080
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -5070,8 +5070,20 @@ endif()
# Azure SDK for C++
function(build_azure_sdk)
+ if(CMAKE_VERSION VERSION_LESS 3.22)
+ # We can't disable installing Azure SDK for C++ by
+ # "set_property(DIRECTORY ${azure_sdk_SOURCE_DIR} PROPERTY
+ # EXCLUDE_FROM_ALL TRUE)" with CMake 3.16.
+ #
+ # At least CMake 3.22 on Ubuntu 22.04 works. So we use 3.22
+ # here. We may be able to use more earlier version here.
+ message(FATAL_ERROR "Building Azure SDK for C++ requires at least CMake 3.22. "
+ "(At least we can't use CMake 3.16)")
+ endif()
message(STATUS "Building Azure SDK for C++ from source")
fetchcontent_declare(azure_sdk
+ # EXCLUDE_FROM_ALL is available since CMake 3.28
+ # EXCLUDE_FROM_ALL TRUE
Review Comment:
Unfortunately, CMake reports an error for this case...
--
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
Re: [PR] GH-37510: [C++] Don't install bundled Azure SDK for C++ [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on PR #38176:
URL: https://github.com/apache/arrow/pull/38176#issuecomment-1756823405
> A future improvement might be installing the sdk into the build tree instead of the host system by modifing:
Does this approach apply a patch to Azure SDK for C++? I want to avoid having a patch...
> Or setting EXCLUDE_FROM_ALL for each of the azure targets might also work? Not sure if dir scope is needed to suppress the installation...
It will work but I want to maintain target list for Azure SDK for C++... It'll increase maintenance cost like Abseil...
--
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
Re: [PR] GH-37510: [C++] Don't install bundled Azure SDK for C++ [arrow]
Posted by "assignUser (via GitHub)" <gi...@apache.org>.
assignUser commented on code in PR #38176:
URL: https://github.com/apache/arrow/pull/38176#discussion_r1354008015
##########
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##########
@@ -5070,8 +5070,20 @@ endif()
# Azure SDK for C++
function(build_azure_sdk)
+ if(CMAKE_VERSION VERSION_LESS 3.22)
+ # We can't disable installing Azure SDK for C++ by
+ # "set_property(DIRECTORY ${azure_sdk_SOURCE_DIR} PROPERTY
+ # EXCLUDE_FROM_ALL TRUE)" with CMake 3.16.
+ #
+ # At least CMake 3.22 on Ubuntu 22.04 works. So we use 3.22
+ # here. We may be able to use more earlier version here.
+ message(FATAL_ERROR "Building Azure SDK for C++ requires at least CMake 3.22. "
+ "(At least we can't use CMake 3.16)")
+ endif()
message(STATUS "Building Azure SDK for C++ from source")
fetchcontent_declare(azure_sdk
+ # EXCLUDE_FROM_ALL is available since CMake 3.28
+ # EXCLUDE_FROM_ALL TRUE
Review Comment:
iirc you could just leave that there and cmake will ignore it if it doesn't know it
--
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
Re: [PR] GH-37510: [C++] Don't install bundled Azure SDK for C++ [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #38176:
URL: https://github.com/apache/arrow/pull/38176
--
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
Re: [PR] GH-37510: [C++] Don't install bundled Azure SDK for C++ [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #38176:
URL: https://github.com/apache/arrow/pull/38176#issuecomment-1763417089
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit 7047e63f6f5fca43f6f5f58cf0f711b4590f92b4.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/17716860957) has more details. It also includes information about 2 possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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