You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Crystrix (via GitHub)" <gi...@apache.org> on 2023/04/13 12:08:53 UTC
[GitHub] [arrow] Crystrix opened a new pull request, #35109: GH-35101: [C++] Update deprecated LOCATION target property in ArrowConfig.cmake.in
Crystrix opened a new pull request, #35109:
URL: https://github.com/apache/arrow/pull/35109
### Rationale for this change
Issue: https://github.com/apache/arrow/issues/35101
Application in release mode is linked to the debug version `libarrow_bundled_dependencies.a` if Arrow is installed by vcpkg. As th e deprecated `LOCATION` property in `ArrowConfig.cmake.in` always returns the debug lib's path. (https://cmake.org/cmake/help/latest/policy/CMP0026.html)
### What changes are included in this PR?
Use `IMPORTED_LOCATION_DEBUG` and `IMPORTED_LOCATION_DEBUG` to replace the deprecated `LOCATION` property in `ArrowConfig.cmake.in`.
### Are these changes tested?
It's a fix of vcpkg build.
### Are there any user-facing changes?
No
--
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
[GitHub] [arrow] ursabot commented on pull request #35109: GH-35101: [C++] Update deprecated LOCATION target property in ArrowConfig.cmake.in
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #35109:
URL: https://github.com/apache/arrow/pull/35109#issuecomment-1511462446
Benchmark runs are scheduled for baseline = d4f73322a093c053c60ec0eee2a7bd08f335f7f0 and contender = b63463c62881d098e9e262e52ca2bd29a416e55d. b63463c62881d098e9e262e52ca2bd29a416e55d is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/b2ac32d1046a4109b10da880c9e888ef...1dac385aad754a90bd2303a632d6e480/)
[Failed] [test-mac-arm](https://conbench.ursa.dev/compare/runs/f55fa939d21c44659d2990d55fa982cc...b0a87ca85a3e4dafae4d04a5e3bf133d/)
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/10e11e6d22fb4950beb800383a1acc20...0784af810f54448bb6353fa119c031bc/)
[Finished :arrow_down:0.25% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/ab68c2006d894c18b2b5767ef8113b18...9ab58e69a88f44d2ad57b28ffcb13ead/)
Buildkite builds:
[Finished] [`b63463c6` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2720)
[Failed] [`b63463c6` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2754)
[Finished] [`b63463c6` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2718)
[Finished] [`b63463c6` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2745)
[Finished] [`d4f73322` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/2719)
[Failed] [`d4f73322` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/2753)
[Finished] [`d4f73322` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/2717)
[Finished] [`d4f73322` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/2744)
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
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
[GitHub] [arrow] kou commented on a diff in pull request #35109: GH-35101: [C++] Update deprecated LOCATION target property in ArrowConfig.cmake.in
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #35109:
URL: https://github.com/apache/arrow/pull/35109#discussion_r1166008766
##########
cpp/src/arrow/ArrowConfig.cmake.in:
##########
@@ -96,11 +96,20 @@ include("${CMAKE_CURRENT_LIST_DIR}/ArrowTargets.cmake")
if(TARGET Arrow::arrow_static AND NOT TARGET Arrow::arrow_bundled_dependencies)
add_library(Arrow::arrow_bundled_dependencies STATIC IMPORTED)
- get_target_property(arrow_static_location Arrow::arrow_static LOCATION)
- get_filename_component(arrow_lib_dir "${arrow_static_location}" DIRECTORY)
+ get_target_property(arrow_static_debug_location Arrow::arrow_static
+ IMPORTED_LOCATION_DEBUG)
+ get_filename_component(arrow_debug_lib_dir "${arrow_static_debug_location}" DIRECTORY)
set_target_properties(Arrow::arrow_bundled_dependencies
PROPERTIES IMPORTED_LOCATION
- "${arrow_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ "${arrow_debug_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ )
+ get_target_property(arrow_static_release_location Arrow::arrow_static
+ IMPORTED_LOCATION_RELEASE)
+ get_filename_component(arrow_release_lib_dir "${arrow_static_release_location}"
+ DIRECTORY)
+ set_target_properties(Arrow::arrow_bundled_dependencies
+ PROPERTIES IMPORTED_LOCATION_RELEASE
+ "${arrow_release_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
Review Comment:
Can we use `IMPORTED_CONFIGURATIONS` instead of explicit `DEBUG` and `RELEASE`?
```diff
diff --git a/cpp/src/arrow/ArrowConfig.cmake.in b/cpp/src/arrow/ArrowConfig.cmake.in
index 5bd710893b..b2097a2f31 100644
--- a/cpp/src/arrow/ArrowConfig.cmake.in
+++ b/cpp/src/arrow/ArrowConfig.cmake.in
@@ -96,12 +96,16 @@ include("${CMAKE_CURRENT_LIST_DIR}/ArrowTargets.cmake")
if(TARGET Arrow::arrow_static AND NOT TARGET Arrow::arrow_bundled_dependencies)
add_library(Arrow::arrow_bundled_dependencies STATIC IMPORTED)
- get_target_property(arrow_static_location Arrow::arrow_static LOCATION)
- get_filename_component(arrow_lib_dir "${arrow_static_location}" DIRECTORY)
- set_target_properties(Arrow::arrow_bundled_dependencies
- PROPERTIES IMPORTED_LOCATION
- "${arrow_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
- )
+ get_target_property(arrow_static_configurations Arrow::arrow_static IMPORTED_CONFIGURATIONS)
+ foreach(CONFIGURATION ${arrow_static_configurations})
+ string(TOUPPER "${CONFIGURATION}" CONFIGURATION)
+ get_target_property(arrow_static_location Arrow::arrow_static LOCATION_${CONFIGURATION})
+ get_filename_component(arrow_lib_dir "${arrow_static_location}" DIRECTORY)
+ set_target_properties(Arrow::arrow_bundled_dependencies
+ PROPERTIES IMPORTED_LOCATION_${CONFIGURATION}
+ "${arrow_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ )
+ endforeach()
# CMP0057: Support new if() IN_LIST operator.
# https://cmake.org/cmake/help/latest/policy/CMP0057.html
```
--
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
[GitHub] [arrow] kou merged pull request #35109: GH-35101: [C++] Update deprecated LOCATION target property in ArrowConfig.cmake.in
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #35109:
URL: https://github.com/apache/arrow/pull/35109
--
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
[GitHub] [arrow] kou commented on a diff in pull request #35109: GH-35101: [C++] Update deprecated LOCATION target property in ArrowConfig.cmake.in
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #35109:
URL: https://github.com/apache/arrow/pull/35109#discussion_r1166384103
##########
cpp/src/arrow/ArrowConfig.cmake.in:
##########
@@ -96,11 +96,20 @@ include("${CMAKE_CURRENT_LIST_DIR}/ArrowTargets.cmake")
if(TARGET Arrow::arrow_static AND NOT TARGET Arrow::arrow_bundled_dependencies)
add_library(Arrow::arrow_bundled_dependencies STATIC IMPORTED)
- get_target_property(arrow_static_location Arrow::arrow_static LOCATION)
- get_filename_component(arrow_lib_dir "${arrow_static_location}" DIRECTORY)
+ get_target_property(arrow_static_debug_location Arrow::arrow_static
+ IMPORTED_LOCATION_DEBUG)
+ get_filename_component(arrow_debug_lib_dir "${arrow_static_debug_location}" DIRECTORY)
set_target_properties(Arrow::arrow_bundled_dependencies
PROPERTIES IMPORTED_LOCATION
- "${arrow_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ "${arrow_debug_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ )
+ get_target_property(arrow_static_release_location Arrow::arrow_static
+ IMPORTED_LOCATION_RELEASE)
+ get_filename_component(arrow_release_lib_dir "${arrow_static_release_location}"
+ DIRECTORY)
+ set_target_properties(Arrow::arrow_bundled_dependencies
+ PROPERTIES IMPORTED_LOCATION_RELEASE
+ "${arrow_release_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
Review Comment:
Thanks.
Could you confirm this change with vcpkg?
--
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
[GitHub] [arrow] Crystrix commented on a diff in pull request #35109: GH-35101: [C++] Update deprecated LOCATION target property in ArrowConfig.cmake.in
Posted by "Crystrix (via GitHub)" <gi...@apache.org>.
Crystrix commented on code in PR #35109:
URL: https://github.com/apache/arrow/pull/35109#discussion_r1166375931
##########
cpp/src/arrow/ArrowConfig.cmake.in:
##########
@@ -96,11 +96,20 @@ include("${CMAKE_CURRENT_LIST_DIR}/ArrowTargets.cmake")
if(TARGET Arrow::arrow_static AND NOT TARGET Arrow::arrow_bundled_dependencies)
add_library(Arrow::arrow_bundled_dependencies STATIC IMPORTED)
- get_target_property(arrow_static_location Arrow::arrow_static LOCATION)
- get_filename_component(arrow_lib_dir "${arrow_static_location}" DIRECTORY)
+ get_target_property(arrow_static_debug_location Arrow::arrow_static
+ IMPORTED_LOCATION_DEBUG)
+ get_filename_component(arrow_debug_lib_dir "${arrow_static_debug_location}" DIRECTORY)
set_target_properties(Arrow::arrow_bundled_dependencies
PROPERTIES IMPORTED_LOCATION
- "${arrow_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ "${arrow_debug_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ )
+ get_target_property(arrow_static_release_location Arrow::arrow_static
+ IMPORTED_LOCATION_RELEASE)
+ get_filename_component(arrow_release_lib_dir "${arrow_static_release_location}"
+ DIRECTORY)
+ set_target_properties(Arrow::arrow_bundled_dependencies
+ PROPERTIES IMPORTED_LOCATION_RELEASE
+ "${arrow_release_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
Review Comment:
Sure, this PR is updated to use `IMPORTED_CONFIGURATIONS`
--
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
[GitHub] [arrow] Crystrix commented on a diff in pull request #35109: GH-35101: [C++] Update deprecated LOCATION target property in ArrowConfig.cmake.in
Posted by "Crystrix (via GitHub)" <gi...@apache.org>.
Crystrix commented on code in PR #35109:
URL: https://github.com/apache/arrow/pull/35109#discussion_r1166801034
##########
cpp/src/arrow/ArrowConfig.cmake.in:
##########
@@ -96,11 +96,20 @@ include("${CMAKE_CURRENT_LIST_DIR}/ArrowTargets.cmake")
if(TARGET Arrow::arrow_static AND NOT TARGET Arrow::arrow_bundled_dependencies)
add_library(Arrow::arrow_bundled_dependencies STATIC IMPORTED)
- get_target_property(arrow_static_location Arrow::arrow_static LOCATION)
- get_filename_component(arrow_lib_dir "${arrow_static_location}" DIRECTORY)
+ get_target_property(arrow_static_debug_location Arrow::arrow_static
+ IMPORTED_LOCATION_DEBUG)
+ get_filename_component(arrow_debug_lib_dir "${arrow_static_debug_location}" DIRECTORY)
set_target_properties(Arrow::arrow_bundled_dependencies
PROPERTIES IMPORTED_LOCATION
- "${arrow_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ "${arrow_debug_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ )
+ get_target_property(arrow_static_release_location Arrow::arrow_static
+ IMPORTED_LOCATION_RELEASE)
+ get_filename_component(arrow_release_lib_dir "${arrow_static_release_location}"
+ DIRECTORY)
+ set_target_properties(Arrow::arrow_bundled_dependencies
+ PROPERTIES IMPORTED_LOCATION_RELEASE
+ "${arrow_release_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
Review Comment:
Yes, I've tested it both on Mac and CentOS with vcpkg
--
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
[GitHub] [arrow] kou commented on a diff in pull request #35109: GH-35101: [C++] Update deprecated LOCATION target property in ArrowConfig.cmake.in
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #35109:
URL: https://github.com/apache/arrow/pull/35109#discussion_r1167269124
##########
cpp/src/arrow/ArrowConfig.cmake.in:
##########
@@ -96,11 +96,20 @@ include("${CMAKE_CURRENT_LIST_DIR}/ArrowTargets.cmake")
if(TARGET Arrow::arrow_static AND NOT TARGET Arrow::arrow_bundled_dependencies)
add_library(Arrow::arrow_bundled_dependencies STATIC IMPORTED)
- get_target_property(arrow_static_location Arrow::arrow_static LOCATION)
- get_filename_component(arrow_lib_dir "${arrow_static_location}" DIRECTORY)
+ get_target_property(arrow_static_debug_location Arrow::arrow_static
+ IMPORTED_LOCATION_DEBUG)
+ get_filename_component(arrow_debug_lib_dir "${arrow_static_debug_location}" DIRECTORY)
set_target_properties(Arrow::arrow_bundled_dependencies
PROPERTIES IMPORTED_LOCATION
- "${arrow_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ "${arrow_debug_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ )
+ get_target_property(arrow_static_release_location Arrow::arrow_static
+ IMPORTED_LOCATION_RELEASE)
+ get_filename_component(arrow_release_lib_dir "${arrow_static_release_location}"
+ DIRECTORY)
+ set_target_properties(Arrow::arrow_bundled_dependencies
+ PROPERTIES IMPORTED_LOCATION_RELEASE
+ "${arrow_release_lib_dir}/${CMAKE_STATIC_LIBRARY_PREFIX}arrow_bundled_dependencies${CMAKE_STATIC_LIBRARY_SUFFIX}"
Review Comment:
Thanks!
--
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
[GitHub] [arrow] github-actions[bot] commented on pull request #35109: GH-35101: [C++] Update deprecated LOCATION target property in ArrowConfig.cmake.in
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35109:
URL: https://github.com/apache/arrow/pull/35109#issuecomment-1506855561
:warning: GitHub issue #35101 **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
[GitHub] [arrow] github-actions[bot] commented on pull request #35109: GH-35101: [C++] Update deprecated LOCATION target property in ArrowConfig.cmake.in
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35109:
URL: https://github.com/apache/arrow/pull/35109#issuecomment-1506855426
* Closes: #35101
--
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