This is an archive of the discontinued LLVM Phabricator instance.

Differential D136065

[CMake] Fix Findzstd module for shared DLL on Windows
ClosedPublic

Authored by andrewng on Oct 17 2022, 3:25 AM.

Details

Reviewers
phosek
mgorny
MaskRay
Commits
rGe7fc7540daa9: [CMake] Fix Findzstd module for shared DLL on Windows

Diff Detail

Event Timeline

andrewng created this revision.Oct 17 2022, 3:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2022, 3:25 AM
Herald added a subscriber: StephenFan. · View Herald Transcript
andrewng requested review of this revision.Oct 17 2022, 3:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 17 2022, 3:25 AM
Harbormaster completed remote builds in B192457: Diff 468147.Oct 17 2022, 4:31 AM
phosek added inline comments.Oct 17 2022, 10:29 AM
llvm/cmake/modules/Findzstd.cmake
42

I'd use CMAKE_LINK_LIBRARY_SUFFIX here, I find it easier to understand that way.

andrewng updated this revision to Diff 468256.Oct 17 2022, 10:50 AM

Update for review comment.

andrewng marked an inline comment as done.Oct 17 2022, 10:50 AM
phosek accepted this revision.Oct 17 2022, 11:27 AM

LGTM

This revision is now accepted and ready to land.Oct 17 2022, 11:27 AM
Harbormaster completed remote builds in B192535: Diff 468256.Oct 17 2022, 12:26 PM
Closed by commit rGe7fc7540daa9: [CMake] Fix Findzstd module for shared DLL on Windows (authored by andrewng). · Explain WhyOct 18 2022, 4:39 AM
This revision was automatically updated to reflect the committed changes.
andrewng added a commit: rGe7fc7540daa9: [CMake] Fix Findzstd module for shared DLL on Windows.

Revision Contents

PathSize
llvm/
cmake/
modules/
16 lines

Diff 468480

llvm/cmake/modules/Findzstd.cmake

Show All 28 Linesfind_package_handle_standard_args(
zstd DEFAULT_MSG zstd DEFAULT_MSG
zstd_LIBRARY zstd_INCLUDE_DIR zstd_LIBRARY zstd_INCLUDE_DIR
) )
if(zstd_FOUND) if(zstd_FOUND)
if(zstd_LIBRARY MATCHES "${zstd_SHARED_LIBRARY_SUFFIX}$" AND if(zstd_LIBRARY MATCHES "${zstd_SHARED_LIBRARY_SUFFIX}$" AND
NOT TARGET zstd::libzstd_shared) NOT TARGET zstd::libzstd_shared)
add_library(zstd::libzstd_shared SHARED IMPORTED) add_library(zstd::libzstd_shared SHARED IMPORTED)
if(MSVC)
# IMPORTED_LOCATION is the path to the DLL and IMPORTED_IMPLIB is the "library".
get_filename_component(zstd_DIRNAME "${zstd_LIBRARY}" DIRECTORY)
string(REGEX REPLACE "${CMAKE_INSTALL_LIBDIR}$" "${CMAKE_INSTALL_BINDIR}" zstd_DIRNAME "${zstd_DIRNAME}")
get_filename_component(zstd_BASENAME "${zstd_LIBRARY}" NAME)
string(REGEX REPLACE "\\${CMAKE_LINK_LIBRARY_SUFFIX}$" "${CMAKE_SHARED_LIBRARY_SUFFIX}" zstd_BASENAME "${zstd_BASENAME}")
phosekUnsubmitted
Done
ReplyInline Actions
get_filename_component(zstd_BASENAME "${zstd_LIBRARY}" NAME)
- string(REGEX REPLACE "${zstd_SHARED_LIBRARY_SUFFIX}$" "${CMAKE_SHARED_LIBRARY_SUFFIX}" zstd_BASENAME "${zstd_BASENAME}")
+ string(REGEX REPLACE "${CMAKE_LINK_LIBRARY_SUFFIX}$" "${CMAKE_SHARED_LIBRARY_SUFFIX}" zstd_BASENAME "${zstd_BASENAME}")
set_target_properties(zstd::libzstd_shared PROPERTIES

I'd use CMAKE_LINK_LIBRARY_SUFFIX here, I find it easier to understand that way.

phosek: I'd use `CMAKE_LINK_LIBRARY_SUFFIX` here, I find it easier to understand that way.
set_target_properties(zstd::libzstd_shared PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES "${zstd_INCLUDE_DIR}"
IMPORTED_LOCATION "${zstd_DIRNAME}/${zstd_BASENAME}"
IMPORTED_IMPLIB "${zstd_LIBRARY}")
unset(zstd_DIRNAME)
unset(zstd_BASENAME)
else()
set_target_properties(zstd::libzstd_shared PROPERTIES set_target_properties(zstd::libzstd_shared PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES "${zstd_INCLUDE_DIR}" INTERFACE_INCLUDE_DIRECTORIES "${zstd_INCLUDE_DIR}"
IMPORTED_LOCATION "${zstd_LIBRARY}") IMPORTED_LOCATION "${zstd_LIBRARY}")
endif()
else() else()
set(zstd_STATIC_LIBRARY "${zstd_LIBRARY}") set(zstd_STATIC_LIBRARY "${zstd_LIBRARY}")
endif() endif()
if(zstd_STATIC_LIBRARY MATCHES "${zstd_STATIC_LIBRARY_SUFFIX}$" AND if(zstd_STATIC_LIBRARY MATCHES "${zstd_STATIC_LIBRARY_SUFFIX}$" AND
NOT TARGET zstd::libzstd_static) NOT TARGET zstd::libzstd_static)
add_library(zstd::libzstd_static STATIC IMPORTED) add_library(zstd::libzstd_static STATIC IMPORTED)
set_target_properties(zstd::libzstd_static PROPERTIES set_target_properties(zstd::libzstd_static PROPERTIES
INTERFACE_INCLUDE_DIRECTORIES "${zstd_INCLUDE_DIR}" INTERFACE_INCLUDE_DIRECTORIES "${zstd_INCLUDE_DIR}"
IMPORTED_LOCATION "${zstd_STATIC_LIBRARY}") IMPORTED_LOCATION "${zstd_STATIC_LIBRARY}")
endif() endif()
endif() endif()
unset(zstd_SHARED_LIBRARY_SUFFIX) unset(zstd_SHARED_LIBRARY_SUFFIX)
unset(zstd_STATIC_LIBRARY_SUFFIX) unset(zstd_STATIC_LIBRARY_SUFFIX)
mark_as_advanced(zstd_INCLUDE_DIR zstd_LIBRARY zstd_STATIC_LIBRARY) mark_as_advanced(zstd_INCLUDE_DIR zstd_LIBRARY zstd_STATIC_LIBRARY)