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

[R-package] changed FindLibR to take advantage of CMake cache #5427

Merged
merged 1 commit into from
Mar 19, 2020
Merged

[R-package] changed FindLibR to take advantage of CMake cache #5427

merged 1 commit into from
Mar 19, 2020

Conversation

jameslamb
Copy link
Contributor

I've been using this project's FindLibR.cmake as a reference, and noticed something that I think could be improved.

According to the cmake docs:

This command is used to find a program. A cache entry named by is created to store the result of this command. If the program is found the result is stored in the variable and the search will not be repeated unless the variable is cleared. If nothing is found, the result will be -NOTFOUND, and the search will be attempted again the next time find_program is invoked with the same variable.

As far as I can tell right now, this script's uses of find_program() for gendef and dlltool isn't taking advantage of that caching, and is repeating the search for those programs when it calls execute_process(). In this PR, I propose directly reading the path to those executables from the cache.

Thanks for your time and consideration!

@trivialfis trivialfis merged commit 3cf665d into dmlc:master Mar 19, 2020
@trivialfis
Copy link
Member

Thanks for the PR!

@terrytangyuan
Copy link
Member

@jameslamb Thanks James! 👍

@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
@jameslamb jameslamb deleted the r/find-libr branch April 19, 2021 04:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants