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-17575: [Docs][C++] Update build document to follow new CMake package #14097

Merged
merged 3 commits into from
Sep 13, 2022

Conversation

raulcd
Copy link
Member

@raulcd raulcd commented Sep 12, 2022

This is a follow up to update our build documentation from the changes on #13892

@github-actions
Copy link

@raulcd
Copy link
Member Author

raulcd commented Sep 12, 2022

@github-actions crossbow submit preview-docs

@github-actions
Copy link

Revision: 1baab6a

Submitted crossbow builds: ursacomputing/crossbow @ actions-a611c23cf2

Task Status
preview-docs Github Actions

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.

Thanks!

docs/source/cpp/build_system.rst Outdated Show resolved Hide resolved

In most cases, it is recommended to use the Arrow shared libraries.

If Arrow is installed on a custom path instead of a common system one you
will have to add the path where Arrow is installed to ``CMAKE_PREFIX_PATH``.
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 add examples how to add the path to CMAKE_PREFIX_PATH?

There are 2 ways to add:

  1. cmake ... -DCMAKE_PREFIX_PATH=...: CMake variable: https://cmake.org/cmake/help/latest/variable/CMAKE_PREFIX_PATH.html
  2. export CMAKE_PREFIX_PATH=...: Environment variable: https://cmake.org/cmake/help/latest/envvar/CMAKE_PREFIX_PATH.html

I think the latter is better for most cases because the CMAKE_PREFIX_PATH environment variable may be defined in other place.

If we specify cmake -DCMAKE_PREFIX_PATH=${ARROW_ROOT}, the pre-defined CMAKE_PREFIX_PATH environment variable value is ignored. (I didn't confirm it yet but I think so. Sorry.) We can use the pre-defined CMAKE_PREFIX_PATH environment variable value by cmake -DCMAKE_PREFIX_PATH=${ARROW_ROOT}${CMAKE_PREFIX_PATH:+:${CMAKE_PREFIX_PATH}. (${X:+...} is a variable substitution syntax. See also ${parameter:+word} in https://www.gnu.org/software/bash/manual/html_node/Shell-Parameter-Expansion.html . This is used here to not include an empty path such as cmake -DCMAKE_PREFIX_PATH=${ARROW_ROOT}:.)

We can use the variable substitution syntax with export CMAKE_PREFIX_PATH=... case:

export CMAKE_PREFIX_PATH=${ARROW_ROOT}${CMAKE_PREFIX_PATH:+:${CMAKE_PREFIX_PATH}}

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks a lot! I've added a couple of examples, let me know if it makes sense or you think there's something more missing.

@raulcd
Copy link
Member Author

raulcd commented Sep 13, 2022

@github-actions crossbow submit preview-docs

@github-actions
Copy link

Revision: 3f2c76d

Submitted crossbow builds: ursacomputing/crossbow @ actions-d4495da68e

Task Status
preview-docs Github Actions

@raulcd
Copy link
Member Author

raulcd commented Sep 13, 2022

This is how the changes will look like: https://crossbow.voltrondata.com/pr_docs/14097/cpp/build_system.html

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
Copy link
Member

kou commented Sep 13, 2022

Can we make this pull request "Ready for review"?

@raulcd raulcd marked this pull request as ready for review September 13, 2022 13:28
@kou kou merged commit 6fc33e7 into apache:master Sep 13, 2022
@ursabot
Copy link

ursabot commented Sep 13, 2022

Benchmark runs are scheduled for baseline = 6cea486 and contender = 6fc33e7. 6fc33e7 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.41% ⬆️0.0%] test-mac-arm
[Failed ⬇️5.07% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.14% ⬆️0.04%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 6fc33e74 ec2-t3-xlarge-us-east-2
[Failed] 6fc33e74 test-mac-arm
[Failed] 6fc33e74 ursa-i9-9960x
[Finished] 6fc33e74 ursa-thinkcentre-m75q
[Finished] 6cea4868 ec2-t3-xlarge-us-east-2
[Failed] 6cea4868 test-mac-arm
[Failed] 6cea4868 ursa-i9-9960x
[Finished] 6cea4868 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

@ursabot
Copy link

ursabot commented Sep 13, 2022

['Python', 'R'] benchmarks have high level of regressions.
ursa-i9-9960x

zagto pushed a commit to zagto/arrow that referenced this pull request Oct 7, 2022
…ckage (apache#14097)

This is a follow up to update our build documentation from the changes on apache#13892

Authored-by: Raúl Cumplido <raulcumplido@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
fatemehp pushed a commit to fatemehp/arrow that referenced this pull request Oct 17, 2022
…ckage (apache#14097)

This is a follow up to update our build documentation from the changes on apache#13892

Authored-by: Raúl Cumplido <raulcumplido@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
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.

None yet

3 participants