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

Update to use rapids-export(COMPONENTS) feature. #12959

Merged

Conversation

robertmaynard
Copy link
Contributor

@robertmaynard robertmaynard commented Mar 16, 2023

Description

Not only does this simplify the cudf CMakeLists.txt it removes a bug where we failed to install the testing dependencies file since it relied on internal rapids-cmake details.

Expected to fix the following:

  • cuspatial issues with not finding GTest targets
  • spark-rapids-jni issues with not finding GTest targets

Fixes #12976

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Not only does this simplify the `cudf` CMakeLists.txt it
removes a bug where we failed to install the testing
dependencies file since it relied on internal rapids-cmake
details.
@robertmaynard robertmaynard added bug Something isn't working 3 - Ready for Review Ready for review by team non-breaking Non-breaking change labels Mar 16, 2023
@robertmaynard robertmaynard requested a review from a team as a code owner March 16, 2023 19:42
@github-actions github-actions bot added CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. labels Mar 16, 2023
@vyasr
Copy link
Contributor

vyasr commented Mar 16, 2023

Is the export name being set here clashing with the "nice" export name in rapids-cmake, leading to trying to install an export set that doesn't actually exist?

@robertmaynard
Copy link
Contributor Author

Is the export name being set here clashing with the "nice" export name in rapids-cmake, leading to trying to install an export set that doesn't actually exist?

Maybe? What failures are you looking at. I can't see what is going wrong in the wheel builds

@vyasr
Copy link
Contributor

vyasr commented Mar 17, 2023

It's failing with

    -- Configuring done (22.5s)
    CMake Error: INSTALL(EXPORT) given unknown export "cudf-testing-exports"
    -- Generating done (0.2s)

(you have to expand some in-text dropdowns or open the raw log).

@robertmaynard
Copy link
Contributor Author

It's failing with


    -- Configuring done (22.5s)

    CMake Error: INSTALL(EXPORT) given unknown export "cudf-testing-exports"

    -- Generating done (0.2s)

(you have to expand some in-text dropdowns or open the raw log).

The "nice" name is only used for the files rapids-CMake generates.

What this looks like is that for some builds we disable cudf test util from being built, but we are always trying to export it.
We will need to update the PR to only add the COMPONENTS arguments to rapids_export when cudf test util is being built.

@vyasr
Copy link
Contributor

vyasr commented Mar 17, 2023

Good catch. Pushed the fix.

Copy link
Contributor

@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.

Verified that pulling in this fix resolves #12976.

@robertmaynard
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit d171fda into rapidsai:branch-23.04 Mar 20, 2023
@robertmaynard robertmaynard deleted the bug/use_rapids-cmake_components branch March 20, 2023 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 - Ready for Review Ready for review by team bug Something isn't working CMake CMake build issue libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Cannot find GTest::gmock GTest::gtest after installing libcudf
5 participants