-
Notifications
You must be signed in to change notification settings - Fork 160
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
[Unit tests] skip BLAS and LAPACK unit tests if Netlib is not found #577
Conversation
f9703ec
to
de0e6ba
Compare
LGTM! |
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.
Look good, thanks for the changes!
@@ -29,7 +36,7 @@ if(BUILD_SHARED_LIBS) | |||
PUBLIC ${PROJECT_SOURCE_DIR}/include | |||
PUBLIC ${PROJECT_SOURCE_DIR}/deps/googletest/include | |||
PUBLIC ${CMAKE_BINARY_DIR}/bin | |||
PUBLIC ${CBLAS_INCLUDE} | |||
$<$<BOOL:${CBLAS_FOUND}>:${CBLAS_INCLUDE}> |
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 don't really follow this syntax, but are we changing this argument from PUBLIC
to non-public?
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.
New logic helps us to drop CBLAS_INCLUDE
from the compilation line when CBLAS header file is not found. Otherwise it might appear in the compilation line as -ICBLAS_INCLUDE_not_found
and cause test compilation issue.
As for PUBLIC
keyword since each BLAS test is a final binary, not library, we don't need to mark this dependency as needed to be propagated to other targets that depend on this target, because we don't have such targets.
Description
This PR implements one more possible solution for the problem described in #557
In case Netlib libraries are not available it shouldn't block completely oneMKL Interfaces building, instead we can generate warning that BLAS and LAPACK unit tests will be skipped because of Netlib absence.
This PR also includes some pre-work for adding BLAS and LAPACK tests that won't require Netlib.
Checklist
All Submissions