Fix Libs.private in .pc file.

[mcmtroffaes]

This patch ensures the proper libraries are specified for static linking in the .pc file, for both linux and windows. Note that the user32 library is required on windows due to use of wsprintf.

[sezero]

You are neglecting that autotools can perfectly be used to build for windows, with mingw or cygwin. Surely not msvc, but msvc isn't mingw or cygwin.

As for cmake: what is the point of generating and installing the pkgconfig file for msvc (visual studio)?

[mcmtroffaes]

I reverted part of the patch that (presumably?) already worked with mingw/cygwin. If it is broken I'll try to see if I can fix it along with the cmake build script.

There are at least two use cases: more generally, meson uses the .pc files on windows with msvc, and more specifically, ffmpeg uses pkgconfig to detect libmodplug when building on windows with msvc. My primary aim is to fix the .pc file so ffmpeg can link statically against libmodplug on windows with vcpkg.

[sezero]

FWIW, I plan to apply something like the following to my fork (https://github.com/sezero/libmodplug)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 41ff174..c1b0fe0 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -176,7 +176,15 @@ install(FILES
   ${CMAKE_INSTALL_INCLUDEDIR}/libmodplug
 )
 
-if (NOT WIN32)
+if (NOT MSVC)
+  if(MINGW OR CYGWIN)
+    set(LIBS_PRIVATE "-luser32 -lstdc++")
+  else()
+    set(LIBS_PRIVATE "-lstdc++")
+    if(MATH_LIB)
+      set(LIBS_PRIVATE "${LIBS_PRIVATE} -lm")
+    endif()
+  endif()
   set(prefix ${CMAKE_INSTALL_PREFIX})
   set(exec_prefix ${CMAKE_INSTALL_PREFIX})
   set(libdir ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR})
@@ -187,4 +195,4 @@ if (NOT WIN32)
   install(FILES "${PROJECT_BINARY_DIR}/libmodplug.pc"
     DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig
   )
-endif (NOT WIN32)
+endif ()
diff --git a/configure.ac b/configure.ac
index 7195eac..20df932 100644
--- a/configure.ac
+++ b/configure.ac
@@ -29,7 +29,6 @@ LT_INIT([win32-dll])
 
 AC_HEADER_STDC
 AC_CHECK_HEADERS([inttypes.h stdint.h malloc.h])
-AC_CHECK_FUNCS(sinf)
 
 CXXFLAGS="$CXXFLAGS -fno-exceptions -Wall -ffast-math -fno-common -D_REENTRANT"
 
@@ -44,6 +43,30 @@ case "$host" in
 esac
 AC_SUBST(LT_LDFLAGS)
 
+LIBS_PRIVATE=-lstdc++
+LIBM=
+case "${host_os}" in
+dnl Djgpp has all c89 math funcs in libc.a
+*djgpp)
+  ;;
+dnl These systems don't have libm or don't need it (list based on libtool)
+darwin*|haiku*|beos*|cegcc*|pw32*)
+  ;;
+dnl MinGW and Cygwin don't need libm, either
+mingw*|cygwin*)
+  LIBS_PRIVATE="-luser32 ${LIBS_PRIVATE}"
+  ;;
+dnl All others:
+*) AC_CHECK_LIB(m, pow, LIBM="-lm")
+  if test x$LIBM != x; then
+    LIBS="${LIBS} ${LIBM}"
+    LIBS_PRIVATE="${LIBS_PRIVATE} ${LIBM}"
+  fi
+  ;;
+esac
+AC_SUBST(LIBS_PRIVATE)
+AC_CHECK_FUNCS(sinf)
+
 # symbol visibility
 ac_save_CXXFLAGS="$CXXFLAGS"
 CXXFLAGS="$CXXFLAGS -fvisibility=hidden -Werror"
diff --git a/libmodplug.pc.in b/libmodplug.pc.in
index bbf05f9..5de90e1 100644
--- a/libmodplug.pc.in
+++ b/libmodplug.pc.in
@@ -7,6 +7,6 @@ Name: libmodplug
 Description: The ModPlug mod file playing library.
 Version: @VERSION@
 Requires: 
-Libs: -L${libdir} -lmodplug 
-Libs.private: -lstdc++ -lm
+Libs: -L${libdir} -lmodplug
+Libs.private: @LIBS_PRIVATE@
 Cflags: -I${includedir}
diff --git a/src/Makefile.am b/src/Makefile.am
index d9a0ba3..e920d66 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -6,7 +6,6 @@ AM_CPPFLAGS = -I$(top_srcdir)/src/libmodplug
 
 lib_LTLIBRARIES = libmodplug.la
 libmodplug_la_LDFLAGS = -version-info $(MODPLUG_LIBRARY_VERSION) $(LT_LDFLAGS)
-libmodplug_la_LIBADD = -lm
 libmodplug_la_SOURCES = tables.h         \
                         sndmix.cpp         \
                         sndfile.cpp        \

[mcmtroffaes]

Looks great! Note that mingw doesn't appear to need -luser32 (I guess either because it doesn't use the wsprintf function, or because it is internally provided by mingw itself).

[sezero]

mingw doesn't appear to need -luser32

It's among its default link libraries, but not if one uses -nostdlib

[mcmtroffaes]

Thanks for sharing your insight. The PR is ready for final review as far as I am concerned.

[sezero]

Pushed https://github.com/sezero/libmodplug/commit/ff19d10348ca6ebe995d00f6880b79a5b34b2e4a in my fork.

[sezero]

One bad thing about this (also with my patch in my fork, and also with the existing unpatched situation) is that -lstdc++ may actually be the wrong library: think of clang, especially on new macOS. What is the fool- proof solution here?

[Konstanty]

ensuring you can include the stdlib that you want is one of the major uses of "mmacosx-version-min". Here is a discussion of a similar/but slightly different problem in another library re c++ library [ref] https://github.com/pure-data/pd-lib-builder/issues/51

[sezero]

ensuring you can include the stdlib that you want is one of the major uses of "mmacosx-version-min". Here is a discussion of a similar/but slightly different problem in another library re c++ library [ref] pure-data/pd-lib-builder#51

AFAICS, the issue shouldn't be limited to mac. As I said in #78, that should be a subject for another discussion and patch.

[dg0yt]

One bad thing about this (also with my patch in my fork, and also with the existing unpatched situation) is that -lstdc++ may actually be the wrong library: think of clang, especially on new macOS. What is the fool- proof solution here?

Not foolproof but somewhat robust: Using the difference of CMAKE_CXX_IMPLICIT_LINK_LIBRARIES and CMAKE_C_IMPLICIT_LINK_LIBRARIES. Implemented in https://github.com/microsoft/vcpkg/pull/39703/files#diff-01fb13749f143558b72753435861d8f036d26883407d7095ef841e519e451db3