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

fix Arrow CMake file #7358

Merged
merged 2 commits into from
Feb 10, 2021
Merged

Conversation

rongou
Copy link
Contributor

@rongou rongou commented Feb 9, 2021

With the latest cmake (built from head), it crashes with -DARROW_STATIC_LIB=ON. Adding cmake_minimum_required to match what's in the top-level file.

@rongou rongou requested a review from a team as a code owner February 9, 2021 20:51
@github-actions github-actions bot added CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. labels Feb 9, 2021
@rongou rongou added 3 - Ready for Review Ready for review by team bug Something isn't working non-breaking Non-breaking change Spark Functionality that helps Spark RAPIDS and removed libcudf Affects libcudf (C++/CUDA) code. labels Feb 9, 2021
@rongou rongou requested review from jlowe and kkraus14 February 9, 2021 20:53
Copy link
Member

@jlowe jlowe left a comment

Choose a reason for hiding this comment

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

This needs to target branch-0.19.

@harrism harrism changed the base branch from branch-0.18 to branch-0.19 February 9, 2021 22:41
@harrism harrism requested a review from a team as a code owner February 9, 2021 22:41
@github-actions github-actions bot added conda libcudf Affects libcudf (C++/CUDA) code. labels Feb 9, 2021
@harrism
Copy link
Member

harrism commented Feb 9, 2021

Is there a good reason to force push?

@harrism harrism added 5 - Ready to Merge Testing and reviews complete, ready to merge and removed 3 - Ready for Review Ready for review by team labels Feb 9, 2021
@rongou
Copy link
Contributor Author

rongou commented Feb 9, 2021

@harrism I had to rebase on branch-0.19.

@kkraus14
Copy link
Collaborator

@rongou do you happen to have a stacktrace of how it was crashing? Would like to understand what CMake policy introduced in 3.18+ is giving it issues.

@codecov
Copy link

codecov bot commented Feb 10, 2021

Codecov Report

❗ No coverage uploaded for pull request base (branch-0.19@c0282e6). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff               @@
##             branch-0.19    #7358   +/-   ##
==============================================
  Coverage               ?   82.22%           
==============================================
  Files                  ?      100           
  Lines                  ?    16969           
  Branches               ?        0           
==============================================
  Hits                   ?    13953           
  Misses                 ?     3016           
  Partials               ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c0282e6...e014ca3. Read the comment docs.

@rongou
Copy link
Contributor Author

rongou commented Feb 10, 2021

@kkraus14

CMake Error in CMakeLists.txt:
  No cmake_minimum_required command is present.  A line of code such as

    cmake_minimum_required(VERSION 3.19)

  should be added at the top of the file.  The version specified may be lower
  if you wish to support older CMake versions for this project.  For more
  information run "cmake --help-policy CMP0000".


-- Configuring incomplete, errors occurred!
See also "/home/rou/src/cudf/cpp/build/arrow/CMakeFiles/CMakeOutput.log".
CMake Error at cmake/Modules/ConfigureArrow.cmake:39 (message):
  Configuring Arrow failed: 1
Call Stack (most recent call first):
  CMakeLists.txt:205 (include)

@kkraus14 kkraus14 removed the request for review from a team February 10, 2021 16:11
@kkraus14
Copy link
Collaborator

@rongou how are you invoking CMake / using the ConfigureArrow module? It should be inheriting the cmake_minimum_required from the parent CMakeLists.txt as far as I know.

@kkraus14
Copy link
Collaborator

cc @robertmaynard for visibility

@rongou
Copy link
Contributor Author

rongou commented Feb 10, 2021

Currently this is done through execute_process (https://github.com/rapidsai/cudf/blob/branch-0.19/cpp/cmake/Modules/ConfigureArrow.cmake#L33). This module can probably use some refactoring, e.g. switching to CPM.

@kkraus14
Copy link
Collaborator

🤦 well that explains it. This should be fixed in #7107 then, but this is a good bandaid until then.

Is this broken for 0.18 as well? Does this require a patch for the JNI build to work properly?

@rongou
Copy link
Contributor Author

rongou commented Feb 10, 2021

This is only a problem if you build cmake from head. I've been testing the cmake nvcc symlink change, thus was using the head version. The current release version of cmake still works. Yeah #7107 would be a more permanent fix.

@kkraus14
Copy link
Collaborator

Going to merge this in as is then, thanks!

@kkraus14
Copy link
Collaborator

@gpucibot merge

@rapids-bot rapids-bot bot merged commit efcd52d into rapidsai:branch-0.19 Feb 10, 2021
@rongou rongou deleted the fix-arrow-cmake branch November 23, 2021 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5 - Ready to Merge Testing and reviews complete, ready to merge bug Something isn't working CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change Spark Functionality that helps Spark RAPIDS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants