-
Notifications
You must be signed in to change notification settings - Fork 7.3k
[intel-mkl] Fix for dynamic and static #49216
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
Conversation
|
I manage to fix it for windows and add all the dll. For linux create the But it still don't find the mkl on linux: |
4f7a5fd to
91976d2
Compare
|
I manage to fix the error |
dg0yt
left a comment
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.
Maybe this needs more attention on how to actually deal with VCPKG_LIBRARY_LINKAGE.
| if(VCPKG_LIBRARY_LINKAGE STREQUAL "dynamic") | ||
| file(COPY "${mkl_dir}/${runtime_dir}/" DESTINATION "${CURRENT_PACKAGES_DIR}/bin/") | ||
| endif() |
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.
IIUC the port now offers DLL in addition to static libs for MSVC.
And it probably always offered shared objects in addition to static libs for non-Windows.
IOW installed binaries do not follow VCPKG_LIBRARY_LINKAGE.
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.
IIUC the port now offers DLL in addition to static libs for MSVC.
Only of msvc dynamic.
Currently for non windows:
In static it offer only static libs without shared object.
In dynamic it offer only shared object without static libs.
| vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/share/mkl/MKLConfig.cmake" "redist/\${MKL_ARCH}" "bin") | ||
| if(${VCPKG_LIBRARY_LINKAGE} STREQUAL "static") | ||
| vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/share/mkl/MKLConfig.cmake" "define_param(MKL_LINK DEFAULT_MKL_LINK MKL_LINK_LIST)" | ||
| [[define_param(MKL_LINK DEFAULT_MKL_LINK MKL_LINK_LIST) | ||
| set(MKL_LINK "static") | ||
| ]]) | ||
| if(VCPKG_LIBRARY_LINKAGE STREQUAL "static") | ||
| vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/share/mkl/MKLConfig.cmake" "set(DEFAULT_MKL_LINK dynamic)" "set(DEFAULT_MKL_LINK static)") | ||
| vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/share/mkl/MKLConfig.cmake" "set(LIB_EXT \".so\")" "set(LIB_EXT \".a\")") | ||
| endif() |
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.
IIUC the port used to implement VCPKG_LIBRARY_LINKAGE by forcing MKL_LINK.
After this change, it only changes the DEFAULT_MKL_LINK instead, but it overwrites LIB_EXT.
IMO the original variant was more vcpkg-ish.
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.
The previous change didn't change link to static, and this change is cleaner.
You are welcome to offer practical changes that are working.
|
@dg0yt Are you interesting to continue this work and change as you see fit? |
|
@vicroms @BillyONeal |
./vcpkg x-add-version --alland committing the result.