conan-io / conan-center-index

Recipes for the ConanCenter repository
https://conan.io/center
MIT License
972 stars 1.79k forks source link

[package] opencv/4.5.3: Missing internal static library dependency for internal implementation details #8326

Open Lorac opened 3 years ago

Lorac commented 3 years ago

From: https://github.com/friendlyanon/cmake-init-multi-target

CMake does not combine the .a or .lib files, which means that if your public static library target depends on one as an internal dependency, you must also install the internal dependency, which will show up in the install interface as a `$ generator expression. The purpose of this genex is that the public static library target does not contain the object code from the internal static library dependency, so it must also be linked into a consuming final target.

If CMake didn't do this for you, then you would end up with linker errors due to missing symbols related to an implementation detail.

The current understanding is that Conan generates CMakeFiles that do not provide such internal static library dependency.

Environment Details (include every applicable attribute)

Steps to reproduce (Include if Applicable)

Static build of OpenCV with OpenMP then OpenMP is not present in the generated CMake package config.

Logs (Executed commands with output) (Include/Attach if Applicable)

~/.conan/data/opencv/4.5.3/_/_/package/5e7bf59bf32f70dc05eed7e4ba38ed2c788a5976/lib/libopencv_core.a(parallel.cpp.o): in function `cv::(anonymous namespace)::_initMaxThreads()':
~/.conan/data/opencv/4.5.3/_/_/build/5e7bf59bf32f70dc05eed7e4ba38ed2c788a5976/source_subfolder/modules/core/src/parallel.cpp:445: undefined reference to `omp_get_max_threads'
~/.conan/data/opencv/4.5.3/_/_/build/5e7bf59bf32f70dc05eed7e4ba38ed2c788a5976/source_subfolder/modules/core/src/parallel.cpp:448: undefined reference to `omp_set_dynamic'
[requires]
opencv/4.5.3

[generators]
cmake_find_package_multi

[options]
opencv:contrib=False
opencv:parallel=openmp
opencv:dnn=False
opencv:with_v4l=True
opencv:with_jpeg=libjpeg-turbo
opencv:with_ade=False
opencv:with_cublas=False
opencv:with_quirc=False
opencv:with_cuda=False
opencv:with_ffmpeg=False
opencv:with_gtk=False
opencv:with_openexr=False
opencv:with_tiff=False
opencv:with_webp=False
memsharded commented 3 years ago

Just to make sure, you are doing a conan install conanfile.txt --build=opencv over that conanfile? That should be enough to reproduce?

I have just tried in Windows, and didn't work because of the gtk option (seems only Linux). It would be fantastic if this could be formulated in terms of very simple libraries, for example the one generated with conan new hello/0.1 --template=cmake_lib, that really helps in debugging and fixing issues.

Lorac commented 3 years ago

This is enough to reproduce the problem, should work on windows:

conan install opencv/4.5.3@ -if conan -s build_type=Release -o opencv:parallel=openmp -g cmake_find_package_multi --build missing

It generates build files that are missing OpenMP.

memsharded commented 3 years ago

Oh, now I think that I see what you mean. It seems the current opencv recipe doesn't implement support for openmp as a Conan package. Only:

        if self.options.parallel == "tbb":
            self.requires("tbb/2020.3")

Is there, but no equivalent for openmp. Then I guess the recipe assumes openmp is in the environment and working, and opencv will manage to use it. But Conan will not generate build files for it, because it is not a Conan package. Could this be the issue?

Lorac commented 3 years ago

The compiler provides OpenMP support, so if your compiler has the support, it will "just" work.

This is why there is no self.requires("some_openmp")

        if self.options.parallel:
            self._cmake.definitions["WITH_TBB"] = self.options.parallel == "tbb"
            self._cmake.definitions["WITH_OPENMP"] = self.options.parallel == "openmp"

Then CMake will verify that OpenMP is supported by the compiler you are using. But Conan doesn't generate build files that take the dependency into account. The static library opencv::opencv_core doesn't contain the symbols for OpenMP, so if my executable links to opencv_core I get linker errors due to missing symbols.

When installing OpenCV with OpenMP as an internal dependency I have to specify to link with OpenMP, but that's incorrect since it's an implementation detail of OpenCV, OpenCV/Conan should generate build files that when linked with, adds OpenMP into the final target.

I don't believe it's because OpenMP is not a Conan package, because I don't think it could be one anyway?

memsharded commented 3 years ago

But Conan doesn't generate build files that take the dependency into account.

Maybe the thing is that I am a bit confused by the terminology. Indeed openmp is not a "dependency" in the Conan terms, we usually associate that with a package

don't believe it's because OpenMP is not a Conan package, because I don't think it could be one anyway?

Yes, I meant it might be possible to wrap the necessary flags into a Conan package, or something like that, no I don't think it is possible to have a real openmp package

Then, what you are probably suggesting is that opencv recipe, when used with option.parallel=openmp should define in its package_info() some cpp_info.cxxflags like -fopenmp or whatever flags are necessary in the downstream consumers to link with the openmp? If that is the case, then this would be a recipe issue, not a Conan client issue, and we should transfer this to the conan-center-index repo.

@uilianries @SSE4 is there any specific reason why opencv recipe does not define flags for consumers if parallel=openmp?

Lorac commented 3 years ago

I think you are right, and we should move the issue to the CCI repo.

It probably makes sense. Looking at the soxr recipe, they add the flags at link time.

https://github.com/conan-io/conan-center-index/blob/ed79d61bc1a560b6384b868cd83f11da3c2cbce4/recipes/soxr/all/conanfile.py#L102-L109

ohunter commented 1 year ago

I don't mean to necrobump this, but it does not seem to be fixed after 1.5 years.

uilianries commented 1 year ago

@ohunter The CMake generator cmake_find_package_multi is almost deprecated in favor of CMakeToolchain + CMakeDeps, plus, the OpenCV recipe was updated many times since this issue was created. Do you have a log that proves that this error persists?

ohunter commented 1 year ago
[ 50%] Building CXX object CMakeFiles/demo.dir/src/main.cpp.o
[100%] Linking CXX executable demo
/usr/bin/ld: /root/.conan2/p/b/openc546a19206cedc/p/lib/libopencv_calib3d.a(essential_solver.cpp.o): in function `Eigen::internal::manage_multi_threading(Eigen::Action, int*)':
/root/.conan2/p/eigen3d88c0279cc26/p/include/eigen3/Eigen/src/Core/products/Parallelizer.h:39: undefined reference to `omp_get_max_threads'
/usr/bin/ld: /root/.conan2/p/b/openc546a19206cedc/p/lib/libopencv_calib3d.a(essential_solver.cpp.o): in function `Eigen::internal::general_matrix_matrix_product<long, double, 1, false, double, 1, false, 0, 1>::run(long, long, long, double const*, long, double const*, long, double*, long, long, double, Eigen::internal::level3_blocking<double, double>&, Eigen::internal::GemmParallelInfo<long>*)':
/root/.conan2/p/eigen3d88c0279cc26/p/include/eigen3/Eigen/src/Core/products/GeneralMatrixMatrix.h:88: undefined reference to `omp_get_thread_num'
/usr/bin/ld: /root/.conan2/p/eigen3d88c0279cc26/p/include/eigen3/Eigen/src/Core/products/GeneralMatrixMatrix.h:89: undefined reference to `omp_get_num_threads'
/usr/bin/ld: /root/.conan2/p/b/openc546a19206cedc/p/lib/libopencv_core.a(parallel.cpp.o): in function `cv::(anonymous namespace)::_initMaxThreads()':
/root/.conan2/p/b/openc546a19206cedc/b/src/modules/core/src/parallel.cpp:445: undefined reference to `omp_get_max_threads'
/usr/bin/ld: /root/.conan2/p/b/openc546a19206cedc/b/src/modules/core/src/parallel.cpp:448: undefined reference to `omp_set_dynamic'
/usr/bin/ld: /root/.conan2/p/b/openc546a19206cedc/p/lib/libopencv_core.a(parallel.cpp.o): in function `cv::parallel_for_impl(cv::Range const&, cv::ParallelLoopBody const&, double)':
/root/.conan2/p/b/openc546a19206cedc/b/src/modules/core/src/parallel.cpp:567: undefined reference to `GOMP_parallel'
/usr/bin/ld: /root/.conan2/p/b/openc546a19206cedc/p/lib/libopencv_core.a(parallel.cpp.o): in function `cv::getThreadNum()':
/root/.conan2/p/b/openc546a19206cedc/b/src/modules/core/src/parallel.cpp:778: undefined reference to `omp_get_thread_num'
/usr/bin/ld: /root/.conan2/p/b/openc546a19206cedc/p/lib/libopencv_core.a(parallel.cpp.o): in function `cv::parallel_for_impl(cv::Range const&, cv::ParallelLoopBody const&, double) [clone ._omp_fn.0]':
/root/.conan2/p/b/openc546a19206cedc/b/src/modules/core/src/parallel.cpp:568: undefined reference to `GOMP_loop_nonmonotonic_dynamic_start'
/usr/bin/ld: /root/.conan2/p/b/openc546a19206cedc/b/src/modules/core/src/parallel.cpp:569: undefined reference to `GOMP_loop_nonmonotonic_dynamic_next'
/usr/bin/ld: /root/.conan2/p/b/openc546a19206cedc/b/src/modules/core/src/parallel.cpp:569: undefined reference to `GOMP_loop_end_nowait'
collect2: error: ld returned 1 exit status
gmake[2]: *** [CMakeFiles/demo.dir/build.make:281: demo] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:83: CMakeFiles/demo.dir/all] Error 2
gmake: *** [Makefile:91: all] Error 2

Not sure if there is a specific log file you're looking for, but I am using a recipe like this:

from conan import ConanFile

class DemoApp(ConanFile):
    settings = "os", "compiler", "build_type", "arch"
    generators = "CMakeToolchain", "CMakeDeps"

    def requirements(self):
        opencv_options = {
            "shared": False,
            "fPIC": True,
            "parallel": "openmp",
            "with_gtk": False,
            "with_ffmpeg": True,
            "cpu_baseline": None,
            "cpu_dispatch": None,
        }

        self.requires("onnxruntime/1.15.1")
        self.requires("opencv/4.5.5", options=opencv_options)
        # OpenCV has a version conflict with something that isn't reported
        self.requires("protobuf/3.21.9", override=True)

and a cmake setup like this:

cmake_minimum_required(VERSION 3.25)

project(Demo LANGUAGES CXX C)

find_package(onnxruntime REQUIRED)
find_package(OpenCV REQUIRED)

add_executable(demo
    src/main.cpp
)

target_include_directories(demo
    PUBLIC $<TARGET_PROPERTY:onnxruntime::onnxruntime,INTERFACE_INCLUDE_DIRECTORIES>
    PUBLIC $<TARGET_PROPERTY:opencv::opencv,INTERFACE_INCLUDE_DIRECTORIES>
)

target_link_libraries(demo 
    PUBLIC onnxruntime::onnxruntime
    PUBLIC opencv::opencv
)