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.

Sign up for GitHub

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

Jump to bottom

Add version suffix to DLL when compiling for MinGW target #5701

Merged
merged 1 commit into from
Jun 13, 2022
Merged

Add version suffix to DLL when compiling for MinGW target #5701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion autotest/postinstall/test_pkg-config.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ case "$UNAME,$2" in
;;
MINGW*,)
alias ldd="sh -c 'objdump -x \$1.exe' --"
LDD_SUBSTR="DLL Name: libgdal.dll"
LDD_SUBSTR="DLL Name: libgdal"
export PATH="$prefix/bin:$PATH"
;;
*,--static)
Expand Down
2 changes: 1 addition & 1 deletion cmake/helpers/GdalVersion.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ add_custom_target(generate_gdal_version_h
-P "${PROJECT_SOURCE_DIR}/cmake/helpers/generate_gdal_version_h.cmake"
VERBATIM)

if (WIN32)
if (WIN32 AND NOT MINGW)
set(GDAL_SOVERSION "")
set(GDAL_ABI_FULL_VERSION "${GDAL_VERSION_MAJOR}${GDAL_VERSION_MINOR}")
else()
Expand Down
4 changes: 4 additions & 0 deletions gdal.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,10 @@ if (MSVC)
CACHE STRING "Postfix to add to the GDAL dll name for debug builds")
set_target_properties(${GDAL_LIB_TARGET_NAME} PROPERTIES DEBUG_POSTFIX "${GDAL_DEBUG_POSTFIX}")
endif ()
if (MINGW AND BUILD_SHARED_LIBS)
set_target_properties(${GDAL_LIB_TARGET_NAME} PROPERTIES SUFFIX "-${GDAL_SOVERSION}${CMAKE_SHARED_LIBRARY_SUFFIX}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overriding SUFFIX in this way breaks import lib (.dll.a) and static lib (.a).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm where do you see this? The suffix isn't added to the import library here from what I can see, and generally I've always seen it added only to the runtime library even in projects which build both static and shared libraries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reworked this to only set RUNTIME_OUTPUT_NAME which should be safer - I've verified building with both BUILD_SHARED_LIBS=ON|OFF that only the .dll gets the suffix this way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I was wrong about the import lib but not about the static lib.

RUNTIME_OUTPUT_NAME is an alternative. However, this will put the version before CMAKE_<CONFIG>_POSTFIX...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

POSTFIXES like d etc though are only used in the MSVC world? Alternatively I could go back to setting SUFFIX, but conditionally on BUILD_SHARED_LIBS, so that static libraries are not affected.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

POSTFIXES like d etc though are only used in the MSVC world?

Possibly yes, but it is a user option (passed to CMake at configuration time). And with the Ninja multi-config generator and exported config, there may be valid use cases outside the MSVC world.

This just illustrates the key conflict with re-implementing autotools features in CMake: It often works against CMake features (here: multi-config support, user/toolchain control). Most often I meet this conflict with simultaneous builds of static and shared.

To be clear, I'm not against adding the version suffix. I just want to make sure it doesn't cause trouble for other reasonable (CMake) use cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you see any potential issues with overriding SUFFIX just for BUILD_SHARED_LIBS=TRUE?

endif ()


if (MSVC AND NOT BUILD_SHARED_LIBS)
target_compile_definitions(${GDAL_LIB_TARGET_NAME} PUBLIC CPL_DISABLE_DLL=)
Expand Down