Rewrite FindOpenAL so it works with add_subdirectory'd OpenAL Soft sources

[mosra]

Requested by @hsdk123. Currently the forked upstream FindOpenAL only works with bundled OpenAL Soft binaries, but not with sources.

EDIT: And also allow DLL copying like with FindSDL and FindGLFW.

[hsdk123]

Nice, looking forward to this!

[mosra]

Aaand there is an issue with a static build of OpenAL Soft as well (microsoft/vcpkg#10158 (comment)), so this also needs to correctly handle that, either by finding & linking to the exported target or adding winmm to the list of libraries to link to on Windows.

[mosra]

Done in 25f7d79:

• works with add_subdirectory()'d OpenAL Soft
• works with statically built OpenAL Soft on Linux (and should also fix [magnum/magnum-plugins] Fix patches for --head builds microsoft/vcpkg#10158 (comment), assuming vcpkg installs OpenALConfig.cmake)
• exposes OPENAL_DLL_RELEASE on Windows and copies it to output location if you use Magnum via a subproject, similarly to GLFW and SDL DLLs

Everything seems to work on my end (and on all the CIs), please complain loudly if it doesn't behave as desired.

[hsdk123]

Nice to see progress on this! It seems to fail with my current setup.
I add the paths to openal32.dll and openal32.lib to CMAKE_PREFIX_PATH, and while cmakelists succeeds to find the .lib, it doesn't copy the .dll into the output directory automatically.

[hsdk123]

It looks as if OPENAL_DLL_RELEASE doesn't get populated with my current flow. (whereas OPENAL_LIBRARY does)

[mosra]

OpenAL Soft, right? Built by hand or using the binary distribution? Where is the dll located? the Find script expects it in bin/Win32 or bin/Win64 and (at least what I see in the binary distribution) it's named soft_oal.dll. Is it openal32.dll in your case? If so, can you try with this?

diff --git a/modules/FindOpenAL.cmake b/modules/FindOpenAL.cmake
index d4cd66170..042021da1 100644
--- a/modules/FindOpenAL.cmake
+++ b/modules/FindOpenAL.cmake
@@ -130,7 +130,7 @@ find_path(OPENAL_INCLUDE_DIR NAMES al.h
 if(CORRADE_TARGET_WINDOWS)
     # TODO: debug?
     find_file(OPENAL_DLL_RELEASE
-        NAMES soft_oal.dll
+        NAMES OpenAL32.dll soft_oal.dll
         PATH_SUFFIXES ${_OPENAL_DLL_PATH_SUFFIX})
 endif()
 

[hsdk123]

I've tried the NAMES OpenAL32.dll soft_oal.dll change, it seems it doesn't work.

OpenAL Soft

Yup! To give a bit more info, I've renamed the output directory to be a custom name, and I'm building it by hand through the cmakelists the repo provides, I'm guessing the custom name would be the issue?

[hsdk123]

Using the repo here: https://github.com/kcat/openal-soft

[mosra]

Can you show me the directory layout, where the lib and dll is? In case of the binary distribution it's like this:

bin/
  Win32/
    soft_oal.dll
  Win64/
    soft_oal.dll
include/
  AL/
    ...
libs/
  Win32/
    OpenAL32.lib
  Win64/
    OpenAL32.lib

[hsdk123]

extlibs/
    install-<custom_name depending on bits>/
        bin/OpenAL32.dll
        lib/OpenAL32.lib
        cmake/
        include/
        share/

[mosra]

Ok, then this patch could do it I hope?

diff --git a/modules/FindOpenAL.cmake b/modules/FindOpenAL.cmake
index d4cd66170..d933f4041 100644
--- a/modules/FindOpenAL.cmake
+++ b/modules/FindOpenAL.cmake
@@ -130,8 +130,8 @@ find_path(OPENAL_INCLUDE_DIR NAMES al.h
 if(CORRADE_TARGET_WINDOWS)
     # TODO: debug?
     find_file(OPENAL_DLL_RELEASE
-        NAMES soft_oal.dll
-        PATH_SUFFIXES ${_OPENAL_DLL_PATH_SUFFIX})
+        NAMES OpenAL32.dll soft_oal.dll
+        PATH_SUFFIXES bin ${_OPENAL_DLL_PATH_SUFFIX})
 endif()
 
 include(FindPackageHandleStandardArgs)

[hsdk123]

Wait a sec... so it seems that the FindOpenAL.cmake file bundled in magnum/modules isn't being used at all, do I need to copy it in to my local modules/?

[hsdk123]

Oh wait, nvm....

[hsdk123]

I think I've found the reason. It looks as if the cmake file is hitting here
https://github.com/mosra/magnum/blob/master/modules/FindOpenAL.cmake#L89

and just returning, never reaching the dll find logic.

[mosra]

Ah, yep, that's the TODO there, haha.

Your installation should have OpenALConfig.cmake there somewhere, can you paste its contents here? Ideally it could be listing the DLL location also.

[hsdk123]

OpenALConfig.cmake:

# Generated by CMake

if("${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION}" LESS 2.5)
   message(FATAL_ERROR "CMake >= 2.6.0 required")
endif()
cmake_policy(PUSH)
cmake_policy(VERSION 2.6)
#----------------------------------------------------------------
# Generated CMake target import file.
#----------------------------------------------------------------

# Commands may need to know the format version.
set(CMAKE_IMPORT_FILE_VERSION 1)

# Protect against multiple inclusion, which would fail when already imported targets are added once more.
set(_targetsDefined)
set(_targetsNotDefined)
set(_expectedTargets)
foreach(_expectedTarget OpenAL::OpenAL)
  list(APPEND _expectedTargets ${_expectedTarget})
  if(NOT TARGET ${_expectedTarget})
    list(APPEND _targetsNotDefined ${_expectedTarget})
  endif()
  if(TARGET ${_expectedTarget})
    list(APPEND _targetsDefined ${_expectedTarget})
  endif()
endforeach()
if("${_targetsDefined}" STREQUAL "${_expectedTargets}")
  unset(_targetsDefined)
  unset(_targetsNotDefined)
  unset(_expectedTargets)
  set(CMAKE_IMPORT_FILE_VERSION)
  cmake_policy(POP)
  return()
endif()
if(NOT "${_targetsDefined}" STREQUAL "")
  message(FATAL_ERROR "Some (but not all) targets in this export set were already defined.\nTargets Defined: ${_targetsDefined}\nTargets not yet defined: ${_targetsNotDefined}\n")
endif()
unset(_targetsDefined)
unset(_targetsNotDefined)
unset(_expectedTargets)


# Compute the installation prefix relative to this file.
get_filename_component(_IMPORT_PREFIX "${CMAKE_CURRENT_LIST_FILE}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)
if(_IMPORT_PREFIX STREQUAL "/")
  set(_IMPORT_PREFIX "")
endif()

# Create imported target OpenAL::OpenAL
add_library(OpenAL::OpenAL SHARED IMPORTED)

set_target_properties(OpenAL::OpenAL PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "${_IMPORT_PREFIX}/include;${_IMPORT_PREFIX}/include;${_IMPORT_PREFIX}/include/AL"
)

# Load information for each installed configuration.
get_filename_component(_DIR "${CMAKE_CURRENT_LIST_FILE}" PATH)
file(GLOB CONFIG_FILES "${_DIR}/OpenALConfig-*.cmake")
foreach(f ${CONFIG_FILES})
  include(${f})
endforeach()

# Cleanup temporary variables.
set(_IMPORT_PREFIX)

# Loop over all imported files and verify that they actually exist
foreach(target ${_IMPORT_CHECK_TARGETS} )
  foreach(file ${_IMPORT_CHECK_FILES_FOR_${target}} )
    if(NOT EXISTS "${file}" )
      message(FATAL_ERROR "The imported target \"${target}\" references the file
   \"${file}\"
but this file does not exist.  Possible reasons include:
* The file was deleted, renamed, or moved to another location.
* An install or uninstall procedure did not complete successfully.
* The installation package was faulty and contained
   \"${CMAKE_CURRENT_LIST_FILE}\"
but not all the files it references.
")
    endif()
  endforeach()
  unset(_IMPORT_CHECK_FILES_FOR_${target})
endforeach()
unset(_IMPORT_CHECK_TARGETS)

# This file does not depend on other imported targets which have
# been exported from the same project but in a separate export set.

# Commands beyond this point should not need to know the version.
set(CMAKE_IMPORT_FILE_VERSION)
cmake_policy(POP)

OpenALConfig-relwithdebinfo.cmake

#----------------------------------------------------------------
# Generated CMake target import file for configuration "RelWithDebInfo".
#----------------------------------------------------------------

# Commands may need to know the format version.
set(CMAKE_IMPORT_FILE_VERSION 1)

# Import target "OpenAL::OpenAL" for configuration "RelWithDebInfo"
set_property(TARGET OpenAL::OpenAL APPEND PROPERTY IMPORTED_CONFIGURATIONS RELWITHDEBINFO)
set_target_properties(OpenAL::OpenAL PROPERTIES
  IMPORTED_IMPLIB_RELWITHDEBINFO "${_IMPORT_PREFIX}/lib/OpenAL32.lib"
  IMPORTED_LOCATION_RELWITHDEBINFO "${_IMPORT_PREFIX}/bin/OpenAL32.dll"
  )

list(APPEND _IMPORT_CHECK_TARGETS OpenAL::OpenAL )
list(APPEND _IMPORT_CHECK_FILES_FOR_OpenAL::OpenAL "${_IMPORT_PREFIX}/lib/OpenAL32.lib" "${_IMPORT_PREFIX}/bin/OpenAL32.dll" )

# Commands beyond this point should not need to know the version.
set(CMAKE_IMPORT_FILE_VERSION)

[mosra]

Great! Last patch to try, hopefully:

diff --git a/modules/FindOpenAL.cmake b/modules/FindOpenAL.cmake
index d4cd66170..456b19544 100644
--- a/modules/FindOpenAL.cmake
+++ b/modules/FindOpenAL.cmake
@@ -69,6 +69,17 @@ if(TARGET OpenAL OR TARGET OpenAL::OpenAL)
         # that ourselves.
         get_target_property(_OPENAL_SOURCE_DIR OpenAL SOURCE_DIR)
         set_target_properties(OpenAL::OpenAL PROPERTIES INTERFACE_INCLUDE_DIRECTORIES ${_OPENAL_SOURCE_DIR}/include/AL)
+
+    # For the imported target get the DLL location
+    else()
+        if(CORRADE_TARGET_WINDOWS)
+            get_target_property(OPENAL_DLL_DEBUG OpenAL::OpenAL IMPORTED_LOCATION_DEBUG)
+            get_target_property(OPENAL_DLL_RELEASE OpenAL::OpenAL IMPORTED_LOCATION_RELEASE)
+            # Release not found, fall back to RelWithDebInfo
+            if(NOT OPENAL_DLL_RELEASE)
+                get_target_property(OPENAL_DLL_RELEASE OpenAL::OpenAL IMPORTED_LOCATION_RELWITHDEBINFO)
+            endif()
+        endif()
     endif()
 
     # Just to make FPHSA print some meaningful location, nothing else.
@@ -81,11 +92,6 @@ if(TARGET OpenAL OR TARGET OpenAL::OpenAL)
     find_package_handle_standard_args("OpenAL" DEFAULT_MSG
         _OPENAL_INTERFACE_INCLUDE_DIRECTORIES)
 
-    if(CORRADE_TARGET_WINDOWS)
-        # TODO: investigate if OpenAL Soft has IMPORTED_LOCATION_ / IMPLIB like
-        #   GLFW does so we can provide OPENAL_DLL
-    endif()
-
     return()
 endif()
 

[hsdk123]

Looks great!

[mosra]

Awesome, thanks a lot for the quick replies. Commited as 1b1347f, will update the Find module in other projects accordingly.