afritz1 / OpenTESArena

Open-source re-implementation of The Elder Scrolls: Arena.
MIT License
915 stars 68 forks source link

48df428 "Fixed CMake warning with WildMIDI casing" breaks WildMidi detection in Arch Linux #204

Closed noabody closed 3 years ago

noabody commented 3 years ago

On Manjaro (Arch) Linux, 21.0.1 I cannot compile Midi support from commit 48df428 With FIND_PACKAGE(WILDMIDI) I get error output:

CMake Warning at OpenTESArena/CMakeLists.txt:7 (FIND_PACKAGE):
  By not providing "FindWILDMIDI.cmake" in CMAKE_MODULE_PATH this project has
  asked CMake to find a package configuration file provided by "WILDMIDI",
  but CMake did not find one.

  Could not find a package configuration file provided by "WILDMIDI" with any
  of the following names:

    WILDMIDIConfig.cmake
    wildmidi-config.cmake

  Add the installation prefix of "WILDMIDI" to CMAKE_PREFIX_PATH or set
  "WILDMIDI_DIR" to a directory containing one of the above files.  If
  "WILDMIDI" provides a separate development package or SDK, be sure it has
  been installed.

-- WildMidi not found, no MIDI support!

With FIND_PACKAGE(WildMidi), WildMidi is detected properly but I get this output:

CMake Warning (dev) at /usr/share/cmake-3.20/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
  The package name passed to `find_package_handle_standard_args` (WILDMIDI)
  does not match the name of the calling package (WildMidi).  This can lead
  to problems in calling code that expects `find_package` result variables
  (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  cmake/FindWildMidi.cmake:55 (find_package_handle_standard_args)
  OpenTESArena/CMakeLists.txt:7 (FIND_PACKAGE)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Found WILDMIDI: /usr/lib/libWildMidi.so  

I am sorry that I cannot provide more useful information as I don't know how to resolve the WildMidi error message but it is acceptable for me because I can still compile with Midi support.

afritz1 commented 3 years ago

Need to figure out the proper solution for this so we fix both the warning and compiling with it on Arch Linux.

noabody commented 3 years ago

This change seems to work on Linux:

diff --git a/OpenTESArena/CMakeLists.txt b/OpenTESArena/CMakeLists.txt
index 42d7280b..7e3c644d 100644
--- a/OpenTESArena/CMakeLists.txt
+++ b/OpenTESArena/CMakeLists.txt
@@ -4,7 +4,7 @@ SET(OpenTESArena_VERSION "0.13.0")

 FIND_PACKAGE(SDL2 REQUIRED)
 FIND_PACKAGE(OpenAL REQUIRED)
-FIND_PACKAGE(WILDMIDI)
+FIND_PACKAGE(WildMidi)

 SET(EXTERNAL_LIBS ${OPENAL_LIBRARY} ${SDL2_LIBRARY})
 INCLUDE_DIRECTORIES ("${CMAKE_SOURCE_DIR}" ${SDL2_INCLUDE_DIR} ${OPENAL_INCLUDE_DIR})
diff --git a/cmake/FindWildMidi.cmake b/cmake/FindWildMidi.cmake
index a2bebf08..848ea65b 100644
--- a/cmake/FindWildMidi.cmake
+++ b/cmake/FindWildMidi.cmake
@@ -52,7 +52,7 @@ find_library(WILDMIDI_LIBRARY NAMES WildMidi wildmidi_dynamic
 # handle the QUIETLY and REQUIRED arguments and set WILDMIDI_FOUND to TRUE if
 # all listed variables are TRUE
 include(FindPackageHandleStandardArgs)
-find_package_handle_standard_args(WILDMIDI REQUIRED_VARS WILDMIDI_LIBRARY WILDMIDI_INCLUDE_DIR)
+find_package_handle_standard_args(WildMidi REQUIRED_VARS WILDMIDI_LIBRARY WILDMIDI_INCLUDE_DIR)

 if(WILDMIDI_FOUND)
     set(WILDMIDI_LIBRARIES ${WILDMIDI_LIBRARY})
afritz1 commented 3 years ago

So instead of changing the FIND_PACKAGE() argument to all caps, we change the find_package_handle_standard_args() argument to not all caps?

afritz1 commented 3 years ago

Make a pull request and I'll try testing that branch on Ubuntu and Raspberry Pi. Those are the only Linux distros I'm remotely comfortable with, and I think I even intentionally uninstalled libwildmidi-dev because I didn't want it interfering with the "generic" one I thought releases had.

noabody commented 3 years ago

The wildmidi package, in Linux, is very specifically WildMidi so both changes are required.

CMakeLists.txt - FIND_PACKAGE(WildMidi)
FindWildMidi.cmake - find_package_handle_standard_args(WildMidi REQUIRED_VARS WILDMIDI_LIBRARY WILDMIDI_INCLUDE_DIR)

I would love to tell you definitively, but I'm just a hobbyist. I compile projects from source and make my own packages, first on Debian, and now Arch. I have no real background in these things except for drive, troubleshooting and web searching.

Most devs that see me would probably call me a meddler or an annoyance but I do try really hard to help.

afritz1 commented 3 years ago

I do most of development on Windows and only really test on Linux to make sure things aren't broken and optimizations like -Ofast and -flto work. Since the main Linux build maintainer for the project has been inactive for awhile, I'm sort of treating Linux support as "well I can make a binary on Ubuntu and maybe Raspberry Pi", and I get nervous with other distros because it feels like things get too user-specific and opinionated at that point, which I'm not experienced enough to deal with.

noabody commented 3 years ago

I wish I could be a Linux maintainer but fear I'm too late in my career, too close to retirement, to go from power user, to programmer. It seems those days have long since passed.

I'll just have to envy and be in awe of people like you, that make real magic.

afritz1 commented 3 years ago

Getting this fixed has made me slightly more comfortable with compiling with WildMIDI support at releases. Hoping that it'll happen for 0.14.0. Thanks.