GH-26394: [Python] Don't use target_include_directories() for imported target #33606
Conversation
…mported target It requires CMake 3.11 or later.
|
|
|
@github-actions crossbow submit verify-rc-source-python-* |
|
Revision: 851e235 Submitted crossbow builds: ursacomputing/crossbow @ actions-4dd4c2aaf3
|
|
@github-actions crossbow submit verify-rc-source-python-* |
|
Revision: 04ffaef Submitted crossbow builds: ursacomputing/crossbow @ actions-207bedcc18
|
assignUser
left a comment
assignUser
left a comment
There was a problem hiding this comment.
Thanks for fixing this, looking forward to April :D
In FindNumpy something got moixed up :)
cpp/cmake_modules/FindNumPy.cmake
Outdated
| target_include_directories(Python3::NumPy PROPERTIES INTERFACE_INCLUDE_DIRECTORIES | ||
| ${NUMPY_INCLUDE_DIRS}) | ||
| set_target_properties(Python3::Module PROPERTIES INTERFACE_INCLUDE_DIRECTORIES |
There was a problem hiding this comment.
Should this be:
| target_include_directories(Python3::NumPy PROPERTIES INTERFACE_INCLUDE_DIRECTORIES | |
| ${NUMPY_INCLUDE_DIRS}) | |
| set_target_properties(Python3::Module PROPERTIES INTERFACE_INCLUDE_DIRECTORIES | |
| set_target_properties(Python3::Module PROPERTIES INTERFACE_INCLUDE_DIRECTORIES | |
| ${NUMPY_INCLUDE_DIRS}) |
There was a problem hiding this comment.
Oh, this is broken code...
We should use set_target_properties() + Python3::NumPy here.
|
@github-actions crossbow submit verify-rc-source-python-* |
|
Revision: a909789 Submitted crossbow builds: ursacomputing/crossbow @ actions-674f83daa6
|
|
@github-actions crossbow submit verify-rc-source-python-* |
|
Revision: 2175f99 Submitted crossbow builds: ursacomputing/crossbow @ actions-5106bd145f
|
|
+1 |
|
Benchmark runs are scheduled for baseline = 427a865 and contender = 2e3683b. 2e3683b is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
|
['Python', 'R'] benchmarks have high level of regressions. |
3 participants
Which issue does this PR close?
Closes #26394
Rationale for this change
We require at least CMake 3.5 for now.
What changes are included in this PR?
Add workaround for CMake < 3.11.
Are these changes tested?
Yes.
Are there any user-facing changes?
No.