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

Quote the list of patch files in case they have spaces in their paths #463

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

ericniebler
Copy link
Contributor

Description

The rapids cpm scripts fail when populating a dependency with patches if the build directory contains spaces in its path name. The reason is because the paths are not properly quoted in the cmake patch script generated from command_template.cmake.in.

This PR properly quotes the list of patch file names.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.
  • The cmake-format.json is up to date with these changes.
  • I have added new files under rapids-cmake/
    • I have added include guards (include_guard(GLOBAL))
    • I have added the associated docs/ rst file and update the api.rst

@bdice bdice added bug Something isn't working non-breaking Introduces a non-breaking change labels Sep 22, 2023
Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Thanks @ericniebler! Could you add a test for this case?

@robertmaynard
Copy link
Contributor

The current patch override test case ( testing/cpm/cpm_generate_patch_command-current_json_dir.cmake ) should fail now since the set for the files variable has changed.

It should be sufficient to adjust the test to look for the new leading quote

@robertmaynard robertmaynard requested a review from a team as a code owner September 26, 2023 17:30
@robertmaynard
Copy link
Contributor

/merge

@rapids-bot rapids-bot bot merged commit f51861e into rapidsai:branch-23.10 Sep 26, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants