microsoft / vcpkg

C++ Library Manager for Windows, Linux, and MacOS
MIT License
22.2k stars 6.16k forks source link

[speex] Currently lacks a cmake wrapper #37412

Open dsvensson opened 3 months ago

dsvensson commented 3 months ago

Is your feature request related to a problem? Please describe.

Currently have this nasty chunk in my CMakeFile:

set(VCPKG_ARCH_DIR "${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}")
find_path(SPEEX_INCLUDE_DIR NAMES speex/speex.h PATHS "${VCPKG_ARCH_DIR}/include" NO_DEFAULT_PATH REQUIRED)
find_library(SPEEX_LIB_RELEASE NAMES speex PATHS "${VCPKG_ARCH_DIR}/lib" NO_DEFAULT_PATH)
find_library(SPEEX_LIB_DEBUG NAMES speex PATHS "${VCPKG_ARCH_DIR}/debug/lib" NO_DEFAULT_PATH)
if(NOT SPEEX_LIB_RELEASE AND NOT SPEEX_LIB_DEBUG)
    message(FATAL_ERROR "Speex library not found")
endif()
add_library(SPEEX::SPEEX STATIC IMPORTED)
set_target_properties(SPEEX::SPEEX PROPERTIES
        IMPORTED_CONFIGURATIONS "Debug;Release"
        IMPORTED_LOCATION_RELEASE "${SPEEX_LIB_RELEASE}"
        IMPORTED_LOCATION_DEBUG "${SPEEX_LIB_DEBUG}"
        INTERFACE_INCLUDE_DIRECTORIES "${SPEEX_INCLUDE_DIR}"
)
find_path(SPEEXDSP_INCLUDE_DIR NAMES speex/speexdsp_types.h PATHS "${VCPKG_ARCH_DIR}/include" NO_DEFAULT_PATH REQUIRED)
find_library(SPEEXDSP_LIB_RELEASE NAMES speexdsp PATHS "${VCPKG_ARCH_DIR}/lib" NO_DEFAULT_PATH)
find_library(SPEEXDSP_LIB_DEBUG NAMES speexdsp PATHS "${VCPKG_ARCH_DIR}/debug/lib" NO_DEFAULT_PATH)
if(NOT SPEEXDSP_LIB_RELEASE AND NOT SPEEXDSP_LIB_DEBUG)
    message(FATAL_ERROR "SpeexDSP library not found")
endif()
add_library(SPEEXDSP::SPEEXDSP STATIC IMPORTED)
set_target_properties(SPEEXDSP::SPEEXDSP PROPERTIES
        IMPORTED_CONFIGURATIONS "Debug;Release"
        IMPORTED_LOCATION_RELEASE "${SPEEXDSP_LIB_RELEASE}"
        IMPORTED_LOCATION_DEBUG "${SPEEXDSP_LIB_DEBUG}"
        INTERFACE_INCLUDE_DIRECTORIES "${SPEEXDSP_INCLUDE_DIR}"
)

Proposed solution

Would be nice if the port gained a proper cmake-wrapper so that one can short that down to find_package(speex). The same applies to speexdsp port.

Here is how libjpeg-turbo does it:

https://github.com/microsoft/vcpkg/blob/2024.02.14/ports/libjpeg-turbo/vcpkg-cmake-wrapper.cmake

Describe alternatives you've considered

No response

Additional context

No response

jimwang118 commented 3 months ago

If you want to use this port simply, you can use the following usage.

find_package(PkgConfig)
pkg_check_modules(speex REQUIRED IMPORTED_TARGET speex)
target_link_libraries(main PRIVATE PkgConfig::speex)
dsvensson commented 3 months ago

@jimwang118 Nice. How do I get it to pick the release version? Your suggestion above leads to linking with vcpkg_installed\x64-windows-static\debug\lib\speexdsp.lib when passing "configuration": "Release" to the build preset. I'm using Ninja Multi-Config, but it's the same for Visual Studio generator.

dsvensson commented 3 months ago

Seems like pkg-config is broken with multi-config. Is there any workaround for that?

jimwang118 commented 3 months ago

Seems like pkg-config is broken with multi-config. Is there any workaround for that?

You can try to solve the problem by uninstalling it.

dg0yt commented 3 months ago

Seems like pkg-config is broken with multi-config. Is there any workaround for that?

No.

You can try to solve the problem by uninstalling it.

No.

jimwang118 commented 2 months ago

@dsvensson Can you use speex normally now?

dsvensson commented 2 months ago

Doesn't look like cmake helpers have been added, so no. Using pkg-config is not compatible with multi config cmake as it fails to select correct static library for release vs debug builds, so I'm using a workaround now by having FindSpeex.cmake and FindSpeexDSP.cmake in my project. They're not fleshed out enough to be upstreamed but will have to do for now. Looking at other ports, it seems fairly common for vcpkg to introduce helpers like this.

# Can be removed once vcpkg speexdsp package has gained a proper cmake helpers.
set(_VCPKG_ARCH_DIR "${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}")
find_path(SPEEX_INCLUDE_DIR NAMES speex/speex.h PATHS "${_VCPKG_ARCH_DIR}/include" NO_DEFAULT_PATH REQUIRED)
find_library(SPEEX_LIB_RELEASE NAMES speex PATHS "${_VCPKG_ARCH_DIR}/lib" NO_DEFAULT_PATH)
find_library(SPEEX_LIB_DEBUG NAMES speex PATHS "${_VCPKG_ARCH_DIR}/debug/lib" NO_DEFAULT_PATH)
if(NOT SPEEX_LIB_RELEASE AND NOT SPEEX_LIB_DEBUG)
    message(FATAL_ERROR "Speex library not found")
endif()
add_library(SPEEX::SPEEX STATIC IMPORTED)
set_target_properties(SPEEX::SPEEX PROPERTIES
        IMPORTED_CONFIGURATIONS "Debug;Release"
        IMPORTED_LOCATION_RELEASE "${SPEEX_LIB_RELEASE}"
        IMPORTED_LOCATION_DEBUG "${SPEEX_LIB_DEBUG}"
        INTERFACE_INCLUDE_DIRECTORIES "${SPEEX_INCLUDE_DIR}"
)

and

# Can be removed once vcpkg speexdsp package has gained a proper cmake helpers.
set(_VCPKG_ARCH_DIR "${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}")
find_path(SPEEXDSP_INCLUDE_DIR NAMES speex/speexdsp_types.h PATHS "${_VCPKG_ARCH_DIR}/include" NO_DEFAULT_PATH REQUIRED)
find_library(SPEEXDSP_LIB_RELEASE NAMES speexdsp PATHS "${_VCPKG_ARCH_DIR}/lib" NO_DEFAULT_PATH)
find_library(SPEEXDSP_LIB_DEBUG NAMES speexdsp PATHS "${_VCPKG_ARCH_DIR}/debug/lib" NO_DEFAULT_PATH)
if(NOT SPEEXDSP_LIB_RELEASE AND NOT SPEEXDSP_LIB_DEBUG)
    message(FATAL_ERROR "SpeexDSP library not found")
endif()
add_library(SPEEX::SPEEXDSP STATIC IMPORTED)
set_target_properties(SPEEX::SPEEXDSP PROPERTIES
        IMPORTED_CONFIGURATIONS "Debug;Release"
        IMPORTED_LOCATION_RELEASE "${SPEEXDSP_LIB_RELEASE}"
        IMPORTED_LOCATION_DEBUG "${SPEEXDSP_LIB_DEBUG}"
        INTERFACE_INCLUDE_DIRECTORIES "${SPEEXDSP_INCLUDE_DIR}"
)
jimwang118 commented 3 days ago

Thanks for posting this issue. Please reopen this issue if this is still a problem for you.

dsvensson commented 2 days ago

This is still an issue. The above two cmake snippets are likely a nice start for wrappers, and without them multi-config is broken as explained above.

dg0yt commented 2 days ago

"Wrapper" is the wrong label. There is nothing to wrap.

You actually want find_package support. This isn't vcpkg's responsibility (although some ports add it) but something which could be asked from upstream.

CMake Find modules as used by find_package are little more than find_path and find_library. You have that already.

dsvensson commented 2 days ago

I agree that upstream ought to provide it, but upstream speex and speexdsp are not using cmake, and not having the workarounds for these ports, that other ports have, makes vcpkg less useful. I'm just using the vocabulary found in those other ports that do add it.

@jimwang118 So Think either this one should be reopened, or the cmake wrappers, packages, fixups, whatever the name is from the other ports should be removed.

Like delete stuff like this (which would be inconvenient for vcpkg users), or keep this issue open until speex/speexdsp has received similar treatment:

https://github.com/microsoft/vcpkg/blob/2024.02.14/ports/libjpeg-turbo/vcpkg-cmake-wrapper.cmake

dg0yt commented 2 days ago

libjpeg-turbo illustrates the valid use case for wrappers: Adapt an official CMake Find module to the multi-config installation layout and port setup in vcpkg. The wrapper calls the official module. The official module defines the interface.

upstream speex and speexdsp are not using cmake

This doesn't mean they couldn't provide cmake package (Find module or Config). It isn't more inconvenient than providing it in vcpkg.

I didn't mentioned pgk-config because I know it doesn't work well with CMake multi-config. But TBH I think this could be "wrapped", at least for the imported target interface of pkg_check_modules. This would solve the problem for more than one port in vcpkg.

Alternatively, unofficial cmake config could be generated out of pkg-config at the end of the port build. But any form of unofficial cmake package means vcpkg lock-in.

dsvensson commented 2 days ago

This doesn't mean they couldn't provide cmake package (Find module or Config). It isn't more inconvenient than providing it in vcpkg.

Few project maintainers who doesn't use CMake already would add such a thing to their repo I expect.

As for handling it within vcpkg -> In the case of speexdsp the vcpkg project already creates a (broken) cmake configuration for it:

https://github.com/microsoft/vcpkg/blob/master/ports/speexdsp/CMakeLists.txt

So it's just weird that the same is not done for speex, plus ofc the CheckCSourceCompiles instead of the yolo-arch-feature-selection that currently breaks cross compilation.