search

conda-forge / opencv-feedstock

A conda-smithy repository for opencv.
BSD 3-Clause "New" or "Revised" License
65 stars 60 forks source link

Rebuild for protobuf423 with additional fixes #363

Closed traversaro closed 1 year ago

traversaro commented 1 year ago

Checklist

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

traversaro commented 1 year ago

The additional fixes seems to be working fine.

traversaro commented 1 year ago

@conda-forge-admin, please rerender

traversaro commented 1 year ago

Windows failures were probably fixed by https://github.com/conda-forge/libprotobuf-feedstock/pull/162 .

traversaro commented 1 year ago

I think the PR is ready for review, the only two remaining failures are temporary network failures.

hmaarrfk commented 1 year ago

Hmmm. Seems good to me

h-vetinari commented 1 year ago

Completely unrelated, but small enough to consider it nevertheless: could you add dev_url: https://github.com/opencv/opencv under extras:? I find it nice to be able to navigate to the upstream repo directly from the feedstock README.

traversaro commented 1 year ago

Completely unrelated, but small enough to consider it nevertheless: could you add dev_url: https://github.com/opencv/opencv under extras:? I find it nice to be able to navigate to the upstream repo directly from the feedstock README.

Done: https://github.com/conda-forge/opencv-feedstock/pull/363/commits/0011f24924e585fc03a9a25c58df6b25a6d52515 .

traversaro commented 1 year ago

@conda-forge-admin, please rerender

mshabunin commented 1 year ago

I'm wondering whether it is really necessary to raise C++ standard level. Conda packages can be used by C++ developers and sudden change of C++ ABI can break their applications. Does conda-forge have predefined set of toolchains which have stable ABI for all packages in the repository? OpenCV wouldn't override C++ standard, but it will set it to 11 if it was not explicitly defined.

It means that OpenCV have failed to find Lapack-compatible library (OpenBLAS, MKL, ...):

CMake Error at cmake/OpenCVUtils.cmake:797 (message):
  Some dependencies have not been found or have been forced, unset
  ENABLE_CONFIG_VERIFICATION option to ignore these failures or change
  following options:

  WITH_LAPACK

Technically Lapack is not required and can be disabled if it gives troubles on this platform (explicitly set WITH_LAPACK=OFF). Package dependencies will be changed in this case though, I'm not sure if it's a good thing.

h-vetinari commented 1 year ago

I'm wondering whether it is really necessary to raise C++ standard level.

Yes, because of abseil, see here.

Conda packages can be used by C++ developers and sudden change of C++ ABI can break their applications.

Where we know of ABI breaks based on the standard version (like abseil), we take great care to rebuild all dependent packages so that everything in conda-forge uses a consistent ABI. If people compile C++ on top of conda-forge dependencies and update any ABI-relevant library in their environment (basically anything in the global pinning), it's up to them to recompile (effectively a local execution of our migration machinery).

Does conda-forge have predefined set of toolchains which have stable ABI for all packages in the repository?

The answer is yes, but (aside from compilers and fundamental support libraries) that baseline is constantly in flux, as new versions with new ABIs get released, and we migrate all dependents over to that, before we upgrade the baseline to that. You can see the ongoing migrations here.

As I mentioned, abseil is special here. And the problem is made worse by protobuf using abseil in their public API. This also makes the protobuf migration special - because we expect problems along the way, we actually double the build matrix on all feedstocks, until we can build everything with protobuf 4 (normal migrations rely on the assumption that they can be done relatively quickly across the ecosystem, which is not the case here)

mshabunin commented 1 year ago

Thank you for reply. Currently I can see that the only issue is missing Lapack dependency, it could be that try_compile in the OpenCV here is missing <LANG>_STANDARD <std> or can not be compiled using C++ 17. I suggest removing all sed patching and checking <build>/CMakeFiles/CMakeError.log to determine what is wrong with Lapack. Alternatively we can disable Lapack temporarily and reintroduce it later.

h-vetinari commented 1 year ago

Alternatively we can disable Lapack temporarily and reintroduce it later.

I think we should get lapack to work again. Not sure what broke though. In any case, Lapack is Fortran (and a C interface), so shouldn't be affected by the C++ standard. More than that, it shouldn't be rebuilt at all (if we're talking about the actual Lapack and not OpenCV's interface to it), but taken from the one we have already.

mshabunin commented 1 year ago

FYI, I've been able to build OpenCV with protobuf@v3.23.3 with minor OpenCV patching and raising CXX_STANDARD to 17 (https://github.com/opencv/opencv/issues/23791#issuecomment-1597579393). Official new protobuf support in the upstream will require slightly more effort though.

traversaro commented 1 year ago

FYI, I've been able to build OpenCV with protobuf@v3.23.3 with minor OpenCV patching and raising CXX_STANDARD to 17 (opencv/opencv#23791 (comment)). Official new protobuf support in the upstream will require slightly more effort though.

A related patch can be found in https://github.com/conda-forge/opencv-feedstock/blob/1925cac9c12099a6c710eee3f79c847f76166f75/recipe/fix_protobuf23.patch .

traversaro commented 1 year ago

@conda-forge-admin, please rerender

traversaro commented 1 year ago

I was not able to reproduce locally the C++17 problem, but I guess the compilation problem in the Lapack test code are related to:

traversaro commented 1 year ago

The build error is:

LAPACK(LAPACK/Generic) check FAILED:
    LAPACK_INCLUDE_DIR: 'C:/Users/STraversaro/AppData/Local/mambaforge/conda-bld/libopencv_1687556262317/_h_env/Library/include'
    LAPACK_LIBRARIES: 'C:/Users/STraversaro/AppData/Local/mambaforge/conda-bld/libopencv_1687556262317/_h_env/Library/lib/lapack.lib;C:/Users/STraversaro/AppData/Local/mambaforge/conda-bld/libopencv_1687556262317/_h_env/Library/lib/blas.lib'
    LAPACK_LINK_LIBRARIES: ''
    Output:
Change Dir: C:/Users/STraversaro/AppData/Local/mambaforge/conda-bld/libopencv_1687556262317/work/build/CMakeFiles/CMakeTmp

Run Build Command(s):C:/Users/STraversaro/AppData/Local/mambaforge/conda-bld/libopencv_1687556262317/_build_env/Library/bin/ninja.exe -v cmTC_c3fad && [1/2] C:\PROGRA~2\MIB055~1\2019\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe  /nologo /TP  -IC:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include -IC:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\work\build /DWIN32 /D_WINDOWS /W3 /GR  /D _CRT_SECURE_NO_DEPRECATE /D _CRT_NONSTDC_NO_DEPRECATE /D _SCL_SECURE_NO_WARNINGS /Gy /bigobj /Oi  /fp:precise /FS     /EHa /wd4127 /wd4251 /wd4324 /wd4275 /wd4512 /wd4589 /wd4819   /MD /O2 /Ob2 /DNDEBUG  -std:c++17 /showIncludes /FoCMakeFiles\cmTC_c3fad.dir\lapack_check.cpp.obj /FdCMakeFiles\cmTC_c3fad.dir\ /FS -c C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\work\cmake\checks\lapack_check.cpp

FAILED: CMakeFiles/cmTC_c3fad.dir/lapack_check.cpp.obj 

C:\PROGRA~2\MIB055~1\2019\ENTERP~1\VC\Tools\MSVC\1429~1.301\bin\Hostx64\x64\cl.exe  /nologo /TP  -IC:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include -IC:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\work\build /DWIN32 /D_WINDOWS /W3 /GR  /D _CRT_SECURE_NO_DEPRECATE /D _CRT_NONSTDC_NO_DEPRECATE /D _SCL_SECURE_NO_WARNINGS /Gy /bigobj /Oi  /fp:precise /FS     /EHa /wd4127 /wd4251 /wd4324 /wd4275 /wd4512 /wd4589 /wd4819   /MD /O2 /Ob2 /DNDEBUG  -std:c++17 /showIncludes /FoCMakeFiles\cmTC_c3fad.dir\lapack_check.cpp.obj /FdCMakeFiles\cmTC_c3fad.dir\ /FS -c C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\work\cmake\checks\lapack_check.cpp
C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\VC\Tools\MSVC\14.29.30133\include\ccomplex(22): warning C4996: '_Header_ccomplex': warning STL4004: <ccomplex>, <cstdalign>, <cstdbool>, and <ctgmath> are deprecated in C++17. You can define _SILENCE_CXX17_C_HEADER_DEPRECATION_WARNING or _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS to acknowledge that you have received this warning.
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(104): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(106): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(107): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(109): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(125): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(188): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(235): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(271): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(325): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(365): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(379): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(379): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(382): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(382): error C2086: 'float _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(412): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(412): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(415): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(415): error C2086: 'double _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(422): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(422): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(455): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(455): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(466): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(466): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(499): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(499): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(511): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(511): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(512): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(512): error C2086: 'float _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(512): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(513): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(513): error C2086: 'float _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(513): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(514): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(514): error C2086: 'float _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(553): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(553): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(554): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(554): error C2086: 'double _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(554): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(555): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(555): error C2086: 'double _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(555): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(556): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(556): error C2086: 'double _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(567): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(567): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(568): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(568): error C2086: 'float _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(568): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(571): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(571): error C2086: 'float _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(571): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(572): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(572): error C2086: 'float _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(624): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(624): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(625): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(625): error C2086: 'double _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(625): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(628): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(628): error C2086: 'double _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(628): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(629): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(629): error C2086: 'double _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(642): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(663): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(671): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(725): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(743): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(806): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(826): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(844): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(851): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(851): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(852): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(852): error C2086: 'float _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(875): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(875): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(876): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(876): error C2086: 'double _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(884): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(908): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(915): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(939): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(946): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(979): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(991): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(991): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(994): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(994): error C2086: 'float _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1024): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1024): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1027): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1027): error C2086: 'double _Complex': redefinition
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1034): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1034): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1067): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1067): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1078): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1078): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1111): error C2143: syntax error: missing ',' before 'const'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1111): error C4430: missing type specifier - int assumed. Note: C++ does not support default-int
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1123): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1156): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1167): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1208): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1221): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1257): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1269): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1319): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1334): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1358): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1367): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1404): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1416): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1440): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1448): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1472): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1480): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1504): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1513): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1540): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1548): error C2143: syntax error: missing ',' before '*'
C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1582): error C2143: syntax error: missing ',' before '*'

C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1594): error C2143: syntax error: missing ',' before '*'

C:\Users\STraversaro\AppData\Local\mambaforge\conda-bld\libopencv_1687556262317\_h_env\Library\include\lapack.h(1594): fatal error C1003: error count exceeds 100; stopping compilation
traversaro commented 1 year ago

Other related issue: https://github.com/Reference-LAPACK/lapack/issues/683 .

h-vetinari commented 1 year ago

Other related issue: Reference-LAPACK/lapack#683 .

Interesting, thanks for digging that up. There clearly seems to be something going wrong with the LAPACK detection. We could backport patches, but from what I can see in that issue, there are none (at least none directly linked)

traversaro commented 1 year ago

Defining _CRT_USE_C_COMPLEX_H to restore the pre-C++17 behaviour seems to be working fine.

traversaro commented 1 year ago

@conda-forge-admin, please rerender

traversaro commented 1 year ago

@conda-forge-admin, please rerender

traversaro commented 1 year ago

So, I think the sleuthing of @traversaro is amazing and I'm grateful this is fixed.

That said, I think the commit history is too much of a mess to merge, but more importantly, I'd like to ask for a few days to see if I can get MicrosoftDocs/cpp-docs#365 to work. I think I have most of what it'll take to make libopencv independent of python and collapse the CI matrix.

Once that works, I'd be happy to pick the relevant changes from this PR on top of it, which would reduce the eye-popping 136 jobs here down to a more manageable 24.

Great! I will wait then, I can also clean the history of this PR but at this point I guess it is a not useful effort.

hmaarrfk commented 1 year ago

I think cleaning the history will help with "cherry-picking" (helping maintain your authorship) or at least organizing the useful changes to transfer them over.

rebase, removing all the rerenders, and squashing the ones where you authored would go a long way (I think)

traversaro commented 1 year ago

Let's do whatever it is easier, having the changes with a different author is not a big problem for me.

mshabunin commented 1 year ago

Regarding LAPACK issue, I can see that you've already fixed it, but still :slightly_smiling_face:. OpenCV uses custom generated header wrapper over lapack.h (because it can be named differently, e.g. mkl_lapack.h), so there is another possible way to do a workaround - patch this generated header (opencv_lapack.h) - see https://github.com/opencv/opencv/blob/c982be39248dc01a8c1f3f79fab22292e94c6b8d/cmake/OpenCVFindLAPACK.cmake#L32-L76 , it even have some MSVC-specific fix which probably should be extended for C++17 case.

Regarding multi-python dependency, there is also another way to reduce binaries number. OpenCV supports Python Limited API, it means that it is possible to build single cv2.abi3.so for all Python>=3.4 versions. There is special cmake option for this - PYTHON3_LIMITED_API=ON. As I understand, opencv-python pip package have already switched to this scheme (https://github.com/opencv/opencv-python/pull/595). See also https://github.com/opencv/opencv/pull/14736 and https://www.python.org/dev/peps/pep-0384 .

traversaro commented 1 year ago

Regarding LAPACK issue, I can see that you've already fixed it, but still 🙂. OpenCV uses custom generated header wrapper over lapack.h (because it can be named differently, e.g. mkl_lapack.h), so there is another possible way to do a workaround - patch this generated header (opencv_lapack.h) - see https://github.com/opencv/opencv/blob/c982be39248dc01a8c1f3f79fab22292e94c6b8d/cmake/OpenCVFindLAPACK.cmake#L32-L76 , it even have some MSVC-specific fix which probably should be extended for C++17 case.

Thanks for the hint! I probably iterate more on this in the future, as for example I suspect (but I need to understand more) that this is something that could be solved in lapack upstream.

h-vetinari commented 1 year ago

Regarding multi-python dependency, there is also another way to reduce binaries number. OpenCV supports Python Limited API, it means that it is possible to build single cv2.abi3.so for all Python>=3.4 versions. There is special cmake option for this - PYTHON3_LIMITED_API=ON.

That's perhaps not ideal from a performance side, but I did see those options floating around, so maybe...? 🤔

I'm having more trouble than anticipated to split the build into library and python bits (in MicrosoftDocs/cpp-docs#365), because there's a generation step that runs when the library is built, and when I build against that again later (even when I point to the right opencv_python_config.cmake) the generated headers aren't found. I tried regenerating (with OPENCV_INITIAL_PASS=ON, but then that doesn't load all necessary CMake modules).

It's probably still possible (and I haven't yet stopped hacking away at it), but the question becomes whether the trade-off is worth it. If it's too fragile and too much maintenance effort, perhaps using the limited API makes sense.

hmaarrfk commented 1 year ago

lets merge this for the HDF5 1.14.1 migration. Feel free to cleanup the commits if you want. but I'll try to merge in a few hours.

h-vetinari commented 1 year ago

lets merge this for the HDF5 1.14.1 migration. Feel free to cleanup the commits if you want. but I'll try to merge in a few hours.

I had asked for a couple of days (and MicrosoftDocs/cpp-docs#365 turned out a bit gnarlier than expected), but if the urgency is high, I guess we can do this one first.

I'm willing to do the clean-up, but cannot push into this PR. If you want, you can change that with https://github.com/conda-forge/opencv-feedstock/pull/369.

hmaarrfk commented 1 year ago

its mostly the fact that hdf5 is super painful to migrate.....

h-vetinari commented 1 year ago

Here's the aggregate of my changes. Hopefully the commits make sense individually. I've picked a few obvious improvements from MicrosoftDocs/cpp-docs#365 as well.

h-vetinari commented 1 year ago

Not sure where this is coming from now; perhaps because I'm triggering not just on error-code 1, but NEQ 0? 🤔 

Verifying WITH_IPP=ON => 'HAVE_IPP'=FALSE
h-vetinari commented 1 year ago

Pretty sure that wasn't me (osx / win):

  File "C:\Miniforge\lib\site-packages\conda_index\index\__init__.py", line 347, in _get_resolve_object
    sd._process_raw_repodata(repodata_copy)  # type: ignore
TypeError: SubdirData._process_raw_repodata() missing 1 required positional argument: 'state'
hmaarrfk commented 1 year ago

Alright great! Thanks all! very exciting!