sezero / mikmod

Mikmod Sound System (mirror of git repo at https://sf.net/projects/mikmod/)
http://mikmod.sourceforge.net/
69 stars 21 forks source link

[build issue] libmikmod uses CMake-relative directories for certain files instead of project-relative, causes CMake errors #76

Closed frank2 closed 1 year ago

frank2 commented 1 year ago

Hello! I'm not sure if this is the right venue for this bug in particular since this is a GitHub mirror, not necessarily the official project source, but I noticed during the build step that libmikmod assumes it's at the root of the CMake project, which in my case it isn't, it's being used as a subdirectory.

I can make a PR if it goes upstream to the SourceForge repository, it's simply a matter of replacing some instances of ${CMAKE_SOURCE_DIR} with ${PROJECT_SOURCE_DIR}. I don't know if this is ideal since the CMake version that's being targeted is so old, and ${PROJECT_SOURCE_DIR} might be a new feature. I couldn't find a way to report this issue on the main project page either.

Let me know what you think!

frank2 commented 1 year ago

Here's an example of the errors caused by the incorrect relative path. I'm building on Windows:

-- Selecting Windows SDK version 10.0.19041.0 to target Windows 10.0.19044.
-- The C compiler identification is MSVC 19.28.29914.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.28.29910/bin/Hostx64/x64/cl.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
CMake Error at C:/Program Files/CMake/share/cmake-3.24/Modules/CPack.cmake:646 (message):
  CPack package description file:
  "C:/cygwin64/home/purple/local/doc/git/github/frank2/sostav/README" could
  not be found.
Call Stack (most recent call first):
  C:/Program Files/CMake/share/cmake-3.24/Modules/CPack.cmake:650 (cpack_check_file_exists)
  lib/mikmod/libmikmod/CMakeLists.txt:168 (include)

CMake Error at C:/Program Files/CMake/share/cmake-3.24/Modules/CPack.cmake:646 (message):
  CPack license resource file:
  "C:/cygwin64/home/purple/local/doc/git/github/frank2/sostav/COPYING.LESSER"
  could not be found.
Call Stack (most recent call first):
  C:/Program Files/CMake/share/cmake-3.24/Modules/CPack.cmake:651 (cpack_check_file_exists)
  lib/mikmod/libmikmod/CMakeLists.txt:168 (include)
sezero commented 1 year ago

I don't know cmake much. @madebr: should I change ${CMAKE_SOURCE_DIR} to ${PROJECT_SOURCE_DIR} or ${CMAKE_CURRENT_SOURCE_DIR} or what?

madebr commented 1 year ago

In your case it does not really matter because CMAKE_CURRENT_SOURCE_DIR == PROJECT_SOURCE_DIR. But I suppose it is clearer to use PROJECT_SOURCE_DIR so it will always be the root directory of your project.

sezero commented 1 year ago

OK thanks. Will make changes soon.

frank2 commented 1 year ago

FWIW, this diff fixed my build issues.

diff --git a/libmikmod/CMakeLists.txt b/libmikmod/CMakeLists.txt
index 6f45f1c..b3d953e 100644
--- a/libmikmod/CMakeLists.txt
+++ b/libmikmod/CMakeLists.txt
@@ -41,8 +41,8 @@ SET(CPACK_PACKAGE_VERSION_PATCH ${LIBMIKMOD_MICRO_VERSION})
 SET(CPACK_PACKAGE_NAME "libmikmod")
 SET(CPACK_PACKAGE_DESCRIPTION_SUMMARY "libmikmod - a module file playing and sound library")
 SET(CPACK_PACKAGE_VENDOR "Shlomi Fish")
-SET(CPACK_PACKAGE_DESCRIPTION_FILE "${CMAKE_SOURCE_DIR}/README")
-SET(CPACK_RESOURCE_FILE_LICENSE "${CMAKE_SOURCE_DIR}/COPYING.LESSER")
+SET(CPACK_PACKAGE_DESCRIPTION_FILE "${PROJECT_SOURCE_DIR}/README")
+SET(CPACK_RESOURCE_FILE_LICENSE "${PROJECT_SOURCE_DIR}/COPYING.LESSER")

 SET(CPACK_PACKAGE_INSTALL_DIRECTORY "${CPACK_PACKAGE_DESCRIPTION_SUMMARY} ${CPACK_PACKAGE_VERSION_MAJOR}.${CPACK_PACKAGE_VERSION_MINOR}.${CPACK_PACKAGE_VERSION_PATCH}")
 SET(CPACK_SOURCE_PACKAGE_FILE_NAME "${CPACK_PACKAGE_NAME}-${CPACK_PACKAGE_VERSION_MAJOR}.${CPACK_PACKAGE_VERSION_MINOR}.${CPACK_PACKAGE_VERSION_PATCH}")
@@ -245,8 +245,8 @@ ENDIF()
 TEST_BIG_ENDIAN(WORDS_BIGENDIAN)

 # to find mikmod_internals.h:
-INCLUDE_DIRECTORIES(BEFORE ${CMAKE_SOURCE_DIR})
-INCLUDE_DIRECTORIES(BEFORE "${CMAKE_SOURCE_DIR}/include")
+INCLUDE_DIRECTORIES(BEFORE ${PROJECT_SOURCE_DIR})
+INCLUDE_DIRECTORIES(BEFORE "${PROJECT_SOURCE_DIR}/include")

 # to find config.h:
 INCLUDE_DIRECTORIES(BEFORE ${CMAKE_BINARY_DIR})
@@ -713,14 +713,14 @@ IF (ENABLE_DL)
   LIST (APPEND EXTRA_LIBS ${CMAKE_DL_LIBS})
 ENDIF()

-CONFIGURE_FILE("${CMAKE_SOURCE_DIR}/config.h.cmake" "${CMAKE_BINARY_DIR}/config.h")
+CONFIGURE_FILE("${PROJECT_SOURCE_DIR}/config.h.cmake" "${CMAKE_BINARY_DIR}/config.h")

 SET(prefix ${CMAKE_INSTALL_PREFIX})
 SET(exec_prefix "\${prefix}")
 SET(libdir "\${exec_prefix}/lib${LIB_SUFFIX}")
 SET(includedir "\${prefix}/include")
-CONFIGURE_FILE("${CMAKE_SOURCE_DIR}/libmikmod-config.in" "${CMAKE_BINARY_DIR}/libmikmod-config" @ONLY)
-CONFIGURE_FILE("${CMAKE_SOURCE_DIR}/libmikmod.pc.in" "${CMAKE_BINARY_DIR}/libmikmod.pc" @ONLY)
+CONFIGURE_FILE("${PROJECT_SOURCE_DIR}/libmikmod-config.in" "${CMAKE_BINARY_DIR}/libmikmod-config" @ONLY)
+CONFIGURE_FILE("${PROJECT_SOURCE_DIR}/libmikmod.pc.in" "${CMAKE_BINARY_DIR}/libmikmod.pc" @ONLY)

 FOREACH (TGT ${LIBMIKMOD_LIBS})
     TARGET_LINK_LIBRARIES ("${TGT}"
madebr commented 1 year ago

You might also look into replacing CMAKE_BINARY_DIR with PROJECT_BINARY_DIR because multiple projects might put files in the root folder. config.h is a common file name.

sezero commented 1 year ago

Should be fixed as of commit 096d0711ca3e294564a5c6ec18f5bbc3a2aac016

Thanks to you both.