Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add soxr library #7728

Merged
merged 15 commits into from
Oct 25, 2021
7 changes: 7 additions & 0 deletions recipes/soxr/all/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
cmake_minimum_required(VERSION 2.8.11)
project(cmake_wrapper)

include(conanbuildinfo.cmake)
conan_basic_setup()

add_subdirectory("source_subfolder")
10 changes: 10 additions & 0 deletions recipes/soxr/all/conandata.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
sources:
"0.1.3":
url: https://sourceforge.net/projects/soxr/files/soxr-0.1.3-Source.tar.xz
sha1: "32ea46b1a8c0c15f835422892d02fce8286aec3c"
patches:
"0.1.3":
- base_path: source_subfolder
patch_file: "patches/findpackage-openmp.patch"
- base_path: source_subfolder
patch_file: "patches/cmake-source-dir.patch"
116 changes: 116 additions & 0 deletions recipes/soxr/all/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
from conans import ConanFile, CMake, tools
import os
import re

required_conan_version = ">=1.33.0"


class SoxrConan(ConanFile):
name = "soxr"
description = "The SoX Resampler library libsoxr performs fast, high-quality one-dimensional sample rate conversion."
homepage = "https://sourceforge.net/projects/soxr/"
topics = ("resampling", "audio", "sample-rate", "conversion")
license = "LGPL-2.1-or-later"
url = "https://github.com/conan-io/conan-center-index"
settings = "os", "compiler", "build_type", "arch"
options = {
"shared": [True, False],
"fPIC": [True, False],
"with_openmp": [True, False],
"with_lsr_bindings": [True, False]
Copy link
Contributor

@madebr madebr Oct 23, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option will create a soxr-lsr library (see bottom of this cmake script)
It will also create a soxr-lsr.pc file.

So you will probably need to use modules in package_info.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I split it up into two components "core" and "lsr". Please have a look: fc13703

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for your contribution!

}
default_options = {
"shared": False,
"fPIC": True,
"with_openmp": False,
"with_lsr_bindings": True
}
generators = "cmake"
exports_sources = ["CMakeLists.txt", "patches/**"]

_cmake = None

@property
def _source_subfolder(self):
return "source_subfolder"

@property
def _build_subfolder(self):
return "build_subfolder"

def config_options(self):
if self.settings.os == "Windows":
del self.options.fPIC

def configure(self):
if self.options.shared:
del self.options.fPIC
del self.settings.compiler.cppstd
del self.settings.compiler.libcxx

def source(self):
tools.get(**self.conan_data["sources"][self.version],
destination=self._source_subfolder, strip_root=True)

def _configure_cmake(self):
if self._cmake:
return self._cmake
self._cmake = CMake(self)
if self.settings.compiler == "Visual Studio":
self._cmake.definitions["BUILD_SHARED_RUNTIME"] = "MD" in self.settings.compiler.runtime
elif self.settings.compiler == "msvc":
self._cmake.definitions["BUILD_SHARED_RUNTIME"] = self.settings.compiler.runtime == "dynamic"
self._cmake.definitions["BUILD_TESTS"] = False
self._cmake.definitions["WITH_OPENMP"] = self.options.with_openmp
self._cmake.definitions["WITH_LSR_BINDINGS"] = self.options.with_lsr_bindings
self._cmake.configure(build_folder=self._build_subfolder)
return self._cmake

def build(self):
for patch in self.conan_data.get("patches", {}).get(self.version, []):
tools.patch(**patch)
cmake = self._configure_cmake()
cmake.build()

def _extract_pffft_license(self):
# extract license header from pffft.c and store it in the package folder
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

@chausner chausner Oct 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer. I have rewritten it using conans.tools and regex: 5865a82

pffft_c = tools.load(os.path.join(self._source_subfolder, "src", "pffft.c"))
license_header = re.search(r"/\* (Copyright.*?)\*/", pffft_c, re.DOTALL).group(1)
license_header = "\n".join(line.lstrip() for line in license_header.splitlines())
tools.save(os.path.join(self.package_folder, "licenses", "pffft"), license_header)

def package(self):
self.copy("LICENCE", dst="licenses", src=self._source_subfolder)
self._extract_pffft_license()
cmake = self._configure_cmake()
cmake.install()
tools.rmdir(os.path.join(self.package_folder, "doc"))
tools.rmdir(os.path.join(self.package_folder, "lib", "pkgconfig"))
tools.rmdir(os.path.join(self.package_folder, "share"))

def package_info(self):
# core component
self.cpp_info.components["core"].names["pkg_config"] = "soxr"
self.cpp_info.components["core"].libs = ["soxr"]
if self.settings.os in ("FreeBSD", "Linux"):
self.cpp_info.components["core"].system_libs = ["m"]
if self.settings.os == "Windows" and self.options.shared:
self.cpp_info.components["core"].defines.append("SOXR_DLL")
if not self.options.shared and self.options.with_openmp:
chausner marked this conversation as resolved.
Show resolved Hide resolved
if self.settings.compiler in ("Visual Studio", "msvc"):
openmp_flags = ["-openmp"]
elif self.settings.compiler in ("gcc", "clang"):
openmp_flags = ["-fopenmp"]
elif self.settings.compiler == "apple-clang":
openmp_flags = ["-Xpreprocessor", "-fopenmp"]
else:
openmp_flags = []
self.cpp_info.components["core"].exelinkflags = openmp_flags
self.cpp_info.components["core"].sharedlinkflags = openmp_flags
# lsr component
if self.options.with_lsr_bindings:
self.cpp_info.components["lsr"].names["pkg_config"] = "soxr-lsr"
self.cpp_info.components["lsr"].libs = ["soxr-lsr"]
if self.settings.os == "Windows" and self.options.shared:
self.cpp_info.components["lsr"].defines.append("SOXR_DLL")
self.cpp_info.components["lsr"].requires = ["core"]
12 changes: 12 additions & 0 deletions recipes/soxr/all/patches/cmake-source-dir.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
Patch required for proper resolving of CMake modules when using the CMake conan helper wrapper.
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -84,7 +84,7 @@ mark_as_advanced (WITH_HI_PREC_CLOCK WITH_FLOAT_STD_PREC_CLOCK

# Introspection:

-list (APPEND CMAKE_MODULE_PATH ${CMAKE_SOURCE_DIR}/cmake/Modules)
+list (APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules)

include (CheckFunctionExists)
include (CheckIncludeFiles)
15 changes: 15 additions & 0 deletions recipes/soxr/all/patches/findpackage-openmp.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
Adds the REQUIRED flag to find_package for OpenMP so that
we can be sure in the conanfile that if with_openmp is set
that OpenMP is indeed used. This is important since we need to add
compiler flags in package_info() in this case.
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -105,7 +105,7 @@ if (${BUILD_EXAMPLES})
endif ()

if (WITH_OPENMP)
- find_package (OpenMP)
+ find_package (OpenMP REQUIRED)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it required? It is an option in the recipe, what happens if soxr:with_openmp=False?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comment at the top of recipes/soxr/all/patches/findpackage-openmp.patch. I am setting the "REQUIRED" flag because I want CMake to fail early if users set "with_openmp=True" and CMake cannot find the package. Otherwise, the build would succeed and later builds would fail when attempting to link to libsoxr because the compiler flags are still getting set for OpenMP in package_info.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be even better to check it on Conan side, the message to the user could be better and the check performed even before trying to build anything. Also, Conan can run the check even if the binary is already available, when a consumer tries to use this recipe (and the compiler flag is propagated), not just when it is being built.

Is this some library we can build from sources (we have llvm-openmp)? Is this one use-case for a openmp/system package?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be even better to check it on Conan side,

Yes, I agree it would be better to check on the Conan side. But I don't think there is a reliable way to check for the presence of a CMake package from within Conan (without invoking CMake itself), is there? What do you suggest in this case?

Is this some library we can build from sources (we have llvm-openmp)? Is this one use-case for a openmp/system package?

Hmm, normally OpenMP is not built from sources. On Linux and macOS, it's commonly installed as a system package. On Windows, it comes with Visual Studio. I am not familiar with llvm-openmp but I doubt this would be the package you normally would want to use on Windows, for example.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be even better to check it on Conan side,

Yes, I agree it would be better to check on the Conan side. But I don't think there is a reliable way to check for the presence of a CMake package from within Conan (without invoking CMake itself), is there? What do you suggest in this case?

The build system should follow instructions from the package-manager. If the package manager is configured with with_openmp=True, it should pass the option to the build-system and this one should find and use it or fail (the REQUIRED is a good thing). We try to avoid auto-detection as much as possible, because it makes the process uncertain and not reproducible.

If OpenMP is expected to be in the system, then the best way to go is to create a system package, Conan will check if the package is available in the system (using some underlying tool like pkg_config) and generate the proper FindOpenMP.cmake to use it from system libraries.... or fail if the package is required and not available.

It would work like any other requirement.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(the REQUIRED is a good thing)

Okay, yes I agree and might have misunderstood you here.

If OpenMP is expected to be in the system, then the best way to go is to create a system package,

I see. Unfortunately, I will not be able to contribute that one. I am surprised it does not exist yet, as there should be more existing packages in Conan Center that depend on OpenMP in some way.

For now, I suggest to either go with the current solution in this PR, or if you prefer, I can also completely remove the with_openmp option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see there are other packages in ConanCenter with a with_openmp option and all of them default to False, like yours, and it is ok. We can leave it as it is, probably it is something to be addressed in the future and, at that point, we can improve all the recipes.

My initial concern about this issue was if creating (and maintaining) a patch to add the REQUIRED keyword was worth it or not, because it is the only patch in this recipe and the only line changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change looks fine to me.

if (OPENMP_FOUND)
set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${OpenMP_C_FLAGS}")
set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${OpenMP_CXX_FLAGS}")
15 changes: 15 additions & 0 deletions recipes/soxr/all/test_package/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
cmake_minimum_required(VERSION 3.1)
project(test_package C)

include(${CMAKE_BINARY_DIR}/conanbuildinfo.cmake)
conan_basic_setup(TARGETS)

find_package(soxr CONFIG REQUIRED)

add_executable(test_package_core test_package_core.c)
target_link_libraries(test_package_core soxr::core)

if(TARGET soxr::lsr)
add_executable(test_package_lsr test_package_lsr.c)
target_link_libraries(test_package_lsr soxr::lsr)
endif()
21 changes: 21 additions & 0 deletions recipes/soxr/all/test_package/conanfile.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import os
from conans import ConanFile, CMake, tools

class TestPackageConan(ConanFile):
settings = "os", "compiler", "build_type", "arch"
generators = "cmake", "cmake_find_package_multi"

def build(self):
cmake = CMake(self)
cmake.configure()
cmake.build()

def test(self):
if not tools.cross_building(self):
# core component
bin_path = os.path.join("bin", "test_package_core")
self.run(bin_path, run_environment=True)
# lsr component
if self.options["soxr"].with_lsr_bindings:
bin_path = os.path.join("bin", "test_package_lsr")
self.run(bin_path, run_environment=True)
7 changes: 7 additions & 0 deletions recipes/soxr/all/test_package/test_package_core.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <soxr.h>
#include <stdio.h>

int main() {
printf("soxr version: %s\n", soxr_version());
return 0;
}
7 changes: 7 additions & 0 deletions recipes/soxr/all/test_package/test_package_lsr.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#include <soxr-lsr.h>
#include <stdio.h>

int main() {
printf("soxr-lsr version: %s\n", src_get_version());
return 0;
}
3 changes: 3 additions & 0 deletions recipes/soxr/config.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
versions:
"0.1.3":
folder: all