-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
GH-36707: [C++] Use ARROW_PACKAGE_PREFIX for OPENSSL_ROOT_DIR too #36710
Conversation
|
is fixed by this. Others will be fixed by r-windows/rtools-packages#285 . |
@github-actions crossbow submit -g nightly-tests -g nightly-packaging -g nightly-release |
Revision: 7a0a398 Submitted crossbow builds: ursacomputing/crossbow @ actions-3da5b12a61 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm admittedly not familiar with this part of the build system but the change seems reasonable. Thank you!
I am also not familiar with the current state of nightly build failures...from R's end, I know that the r-devdocs failure is unrelated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
None of the CI failures seem related and the change looks good to me.
…6710) ### Rationale for this change In general, a CMake package uses `${PACKAGE}_ROOT` variable to detect `PACKAGE` but `FindOpenSSL.cmake` uses `OPENSSL_ROOT_DIR` not `OpenSSL_ROOT`. ### What changes are included in this PR? Set `OPENSSL_ROOT_DIR` explicitly. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * Closes: #36707 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
…oo (apache#36710) ### Rationale for this change In general, a CMake package uses `${PACKAGE}_ROOT` variable to detect `PACKAGE` but `FindOpenSSL.cmake` uses `OPENSSL_ROOT_DIR` not `OpenSSL_ROOT`. ### What changes are included in this PR? Set `OPENSSL_ROOT_DIR` explicitly. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * Closes: apache#36707 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit fb7fb0d. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit fb7fb0d. There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
…oo (apache#36710) ### Rationale for this change In general, a CMake package uses `${PACKAGE}_ROOT` variable to detect `PACKAGE` but `FindOpenSSL.cmake` uses `OPENSSL_ROOT_DIR` not `OpenSSL_ROOT`. ### What changes are included in this PR? Set `OPENSSL_ROOT_DIR` explicitly. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * Closes: apache#36707 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Raúl Cumplido <raulcumplido@gmail.com>
Rationale for this change
In general, a CMake package uses
${PACKAGE}_ROOT
variable to detectPACKAGE
butFindOpenSSL.cmake
usesOPENSSL_ROOT_DIR
notOpenSSL_ROOT
.What changes are included in this PR?
Set
OPENSSL_ROOT_DIR
explicitly.Are these changes tested?
Yes.
Are there any user-facing changes?
Yes.