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]

This is wrong: mingw and cygwin are gcc-based and need -lstdc++ (same with the cmake side.)

[mcmtroffaes]

Good catch. What should be the correct host_os that corresponds to cmake's WIN32 an msvc build?

[sezero]

I think cmake sets MSVC for msvc environment.

If it were me, I'd do something like the following for win32 case in cmake:

if(WIN32)
  set(LIBS_PRIVATE "-luser32")
  if(MINGW OR CYGWIN)
    set(LIBS_PRIVATE "${LIBS_PRIVATE} "-lstdc++")
  endif()
else()
  [....]

[mcmtroffaes]

Ok, perhaps we can assume that autotools won't be used with msvc (although it's possible with msys2). For cmake, I've updated the patch to check with MSVC. I'll see if I can run a test with mingw.

[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 sezero@ff19d10 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] pure-data/pd-lib-builder#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