Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-17632: [Python][C++] Add details of where libarrow is being found during build #14059

Merged

Conversation

dhruv9vats
Copy link
Contributor

This PR aims to add back message(STATUS ...) statements that printed some details about Arrow being found, its version, and the paths. These were refactored away as part of #13892.

@github-actions
Copy link

github-actions bot commented Sep 7, 2022

message(STATUS "Found the Arrow core shared library: ${ARROW_SHARED_LIB}")
message(STATUS "Found the Arrow core import library: ${ARROW_IMPORT_LIB}")
message(STATUS "Found the Arrow core static library: ${ARROW_STATIC_LIB}")
endif()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you define a function arrow_show_details(package_name) or something that shows these status messages and use it in all *Config.cmake.in?

cpp/src/arrow/ArrowConfig.cmake.in Outdated Show resolved Hide resolved
cpp/src/arrow/ArrowConfig.cmake.in Outdated Show resolved Hide resolved
cpp/src/arrow/ArrowConfig.cmake.in Outdated Show resolved Hide resolved
cpp/src/arrow/ArrowConfig.cmake.in Outdated Show resolved Hide resolved
if(NOT ${PACKAGE_NAME}_FIND_QUIETLY)
string(TOUPPER ${PACKAGE_NAME} package_name_upper)
message(STATUS "${PACKAGE_NAME} version: ${${package_name_upper}_VERSION}")
message(STATUS "Found the ${PACKAGE_NAME} shared library: ${${package_name_upper}_SHARED_LIB}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this work with ArrowDataset?

It seems that we need to pass 2 arguments to this function: arrow_show_details(package_name variable_prefix) (e.g.: arrow_show_details(ArrowDataset ARROW_DATASET))

Copy link
Contributor Author

@dhruv9vats dhruv9vats Sep 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The signature now is macro(arrow_show_details package_name variable_prefix), but does it make sense to use target_base_name instead of variable_prefix as in arrow_keep_backward_compatibility, and do something like:

macro(arrow_show_details package_name target_base_name)
  string(TOUPPER ${target_base_name} target_base_name_upper)
  .
  .
  .
endmacro()

cpp/src/arrow/ArrowTestingConfig.cmake.in Outdated Show resolved Hide resolved
cpp/src/arrow/flight/ArrowFlightConfig.cmake.in Outdated Show resolved Hide resolved
python/pyarrow/src/ArrowPythonConfig.cmake.in Outdated Show resolved Hide resolved
Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@kou kou merged commit 3aee08b into apache:master Sep 12, 2022
@ursabot
Copy link

ursabot commented Sep 12, 2022

Benchmark runs are scheduled for baseline = f42f3df and contender = 3aee08b. 3aee08b is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.81% ⬆️0.0%] test-mac-arm
[Failed ⬇️0.0% ⬆️0.28%] ursa-i9-9960x
[Finished ⬇️0.18% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 3aee08b4 ec2-t3-xlarge-us-east-2
[Finished] 3aee08b4 test-mac-arm
[Failed] 3aee08b4 ursa-i9-9960x
[Finished] 3aee08b4 ursa-thinkcentre-m75q
[Finished] f42f3df0 ec2-t3-xlarge-us-east-2
[Failed] f42f3df0 test-mac-arm
[Failed] f42f3df0 ursa-i9-9960x
[Finished] f42f3df0 ursa-thinkcentre-m75q
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

zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
…nd during build (apache#14059)

This PR aims to add back `message(STATUS ...)` statements that printed some details about Arrow being found, its version, and the paths. These were refactored away as part of apache#13892.

Authored-by: Dhruv Vats <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
fatemehp pushed a commit to fatemehp/arrow that referenced this pull request Oct 17, 2022
…nd during build (apache#14059)

This PR aims to add back `message(STATUS ...)` statements that printed some details about Arrow being found, its version, and the paths. These were refactored away as part of apache#13892.

Authored-by: Dhruv Vats <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants