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

[package] assimp/5.4.3: Undefined minizip symbols on assimp static build #25765

Open
EstebanDugueperoux2 opened this issue Oct 29, 2024 · 9 comments
Labels
bug Something isn't working

Comments

@EstebanDugueperoux2
Copy link
Contributor

Description

Having a package consuming assimp and openimageio as dependencies to do a static build, I encounter 2 issues:

See https://github.com/EstebanDugueperoux2/conan_assimp_test to reproduce.

I reproduce these issues only when adding openimageio requirement in addition to assimp.

Package and Environment Details

  • Package Name/Version: assimp/5.43
  • Operating System+version: Linux Ubuntu 24.04.1 LTS
  • Compiler+version: GCC 13
  • Docker image: conanio/gcc13
  • Conan version: conan 2.7.0
  • Python version: Python 3.12.3

Conan profile

[settings]
os=Linux
arch=x86_64
compiler=gcc
compiler.cppstd=17
compiler.version=10
compiler.libcxx=libstdc++11
build_type=Release

Steps to reproduce

conan create . --build missing

Logs

https://github.com/EstebanDugueperoux2/conan_assimp_test/blob/main/static_build_with_assimp_and_openimageio_with_cmakedeps_property_workaround.log

@EstebanDugueperoux2 EstebanDugueperoux2 added the bug Something isn't working label Oct 29, 2024
@SpaceIm
Copy link
Contributor

SpaceIm commented Oct 30, 2024

It's not a issue coming from assimp recipe.

You have two libraries with conflicting CMake config files in your dependency graph: minizip (dependency of assimp) and minizip-ng (dependency of opencolorio which is a dependency of openimageio).

minizip-ng generates minizipConfig.cmake config file with MINIZIP::minizip target, and minizip generates minizipConfig.cmake config file with minizip::minizip target (or MINIZIP::minizip with your trick). The conflict is here, one config file overrides the other one, so it breaks all the CMake linkage stuff.

As a workaround, you can override cmake_file_name (and cmake_target_name eventually, I don't remember whether CMake is case sensitive regarding target names) of either minizip or minizip-ng recipe to avoid this conflict, in a way that names can be different.

To summarize the root cause of this issue at conancenter recipe level: it's a cmake_file_name conflict in package_info() between minizip & minizip-ng recipes.
Fix:
Since minizip-ng has config files upstream, this recipe just follows upstream names. On the other side, minizip has no config file upstream (and therefore cmake_file_name is not even forced to a specific value in package_info(), it's just inherited from recipe name), so we could change its cmake names since there is no upstream convention to follow (though it would be a breaking change for consumers of minizip recipe).

@EstebanDugueperoux2
Copy link
Contributor Author

Hi @SpaceIm and thanks for your feedback.

About the workaround to change the cmake_file_name generated by CMakeDeps for minizip-ng requirement of opencolorio consumer, If I do that the find_package of opencolorio CMake scripts will not works anymore?
The only workaround I see is to disable opencolorio requirement with 'self.requires("openimageio/2.5.16.0", options={"with_opencolorio": False})'

Is this kind of issue could be detected by CMakeDeps and produce an explicit error log? Because this kind of issue is very time consuming to detect.

Does I understand correctly if i say that minizip-ng is an API compatible replacement of minizip and this last is no more maintained?
If correct, could we deprecate minizip recipe and update all consuming recipes to use minizip-ng?
Or theses recipes use "provides" attributes? (https://docs.conan.io/2/reference/conanfile/attributes.html#provides)

Regards.

@SpaceIm
Copy link
Contributor

SpaceIm commented Oct 30, 2024

About the workaround to change the cmake_file_name generated by CMakeDeps for minizip-ng requirement of opencolorio consumer, If I do that the find_package of opencolorio CMake scripts will not works anymore?

Not sure to understand. Are you afraid of some side effect in the build of opencolorio? There is no reason. When you override cmake_file_name of a dependency in your own conanfile (not the one of opencolorio), it doesn't change cmake_file_name in the internal conan build of your dependencies, each build logic is isolated. Anyway, I would change cmake_file_name of minizip, not minizip-ng.

Does I understand correctly if i say that minizip-ng is an API compatible replacement of minizip and this last is no more maintained?

I'm not an expert of this lib. If compatibility is enabled (mz_compatibility option in minizip-ng recipe), yes it's compatible I guess. The problem being that it's disabled by default in minizip-ng recipe. And I don't know if users would be always happy to get minizip-ng in their dependency graph, instead of minizip, since it has many dependencies by default (zlib, bzip2, xz_utils, zstd, openssl, libiconv), while minizip has just zlib & bzip2.

@EstebanDugueperoux2
Copy link
Contributor Author

About the workaround to change the cmake_file_name generated by CMakeDeps for minizip-ng requirement of opencolorio consumer, If I do that the find_package of opencolorio CMake scripts will not works anymore?

Not sure to understand. Are you afraid of some side effect in the build of opencolorio? There is no reason. When you override cmake_file_name of a dependency in your own conanfile (not the one of opencolorio), it doesn't change cmake_file_name in the internal conan build of your dependencies, each build logic is isolated. Anyway, I would change cmake_file_name of minizip, not minizip-ng.

Ok then it is not a workaround if we need to fix the minizip recipe.
Do you want I make a pull request on the minizip recipe to add a cmake_file_name CMakeDeps property?
And also some pull requests on consuming recipes?

To come back to mean to have CMakeDeps give a better feedback in case of such conflict, what do you think?
Could we have an evoluton of CMakeDeps about that?

Does I understand correctly if i say that minizip-ng is an API compatible replacement of minizip and this last is no more maintained?

I'm not an expert of this lib. If compatibility is enabled (mz_compatibility option in minizip-ng recipe), yes it's compatible I guess. The problem being that it's disabled by default in minizip-ng recipe. And I don't know if users would be always happy to get minizip-ng in their dependency graph, instead of minizip, since it has many dependencies by default (zlib, bzip2, xz_utils, zstd, openssl, libiconv), while minizip has just zlib & bzip2.

Hum interesting this mz_compatibility option .
According to official documentation, this is enabled by default but not in conan-center.

Regards.

@SpaceIm
Copy link
Contributor

SpaceIm commented Nov 1, 2024

Ok then it is not a workaround if we need to fix the minizip recipe.

Sorry I don't understand what you mean or you misunderstood what I said. You can override in your own package cmake_file_name of minizip, it should fix build issue of your package, and it will not affect opencolorio build at all (which is working fine I guess). But consumers of your package may encounter this issue (if they use CMakeDeps), since overriding cmake_file_name is something isolated in each package.

@EstebanDugueperoux2
Copy link
Contributor Author

Hi @SpaceIm,

Sorry for the misunderstanding, to take again https://github.com/EstebanDugueperoux2/conan_assimp_test example, I have added:

deps.set_property("minizip", "cmake_target_name", "MINIZIP::minizip") deps.set_property("minizip", "cmake_file_name", "MINIZIP")

to CMakeDeps in my recipe in despite this last doesn't consume minizip directly.
And indeed with that my build works but I don't understand because according to https://docs.conan.io/2/reference/tools/cmake/cmakedeps.html I should have generated files from CMakeDeps only for conan_assimp_test requirements not for transitive ones.

Regards.

@EstebanDugueperoux2
Copy link
Contributor Author

Now I have a segfault at runtime when my final static executable invoke "oiio::ColorConfig colorConfig(configOCIOFilePath);" :

(gdb) where #0 __strncasecmp_l_avx2 () at ../sysdeps/x86_64/multiarch/strcmp-avx2.S:215 #1 0x000055555692ee9c in OpenImageIO_v2_5::Strutil::strncasecmp ( a=0x5555668ce340 "/home/esteban/Documents/git/conan_assimp_test/config.ocio", b=0x55555a89023e "ocio://", size=7) at /home/esteban/.conan2/p/b/openi3375862115c4c/b/src/src/libutil/strutil.cpp:471 #2 0x0000555556929f32 in OpenImageIO_v2_5::Strutil::istarts_with (a=..., b=...) at /home/esteban/.conan2/p/b/openi3375862115c4c/b/src/src/libutil/strutil.cpp:526 #3 0x00005555561e46e5 in OpenImageIO_v2_5::ColorConfig::Impl::init (this=0x5555668ce390, filename=...) at /home/esteban/.conan2/p/b/openi3375862115c4c/b/src/src/libOpenImageIO/color_ocio.cpp:838 #4 0x00005555561e5481 in OpenImageIO_v2_5::ColorConfig::reset (this=0x7fffffffd4a8, filename=...) at /home/esteban/.conan2/p/b/openi3375862115c4c/b/src/src/libOpenImageIO/color_ocio.cpp:909 #5 0x00005555561e4358 in OpenImageIO_v2_5::ColorConfig::ColorConfig (this=0x7fffffffd4a8, filename=...) at /home/esteban/.conan2/p/b/openi3375862115c4c/b/src/src/libOpenImageIO/color_ocio.cpp:793 #6 0x0000555555a56555 in aliceVision::image::getDefaultColorConfigFilePath[abi:cxx11]() () at /home/esteban/.conan2/p/b/conan9c3d7421a84e0/b/src/colorspace.cpp:42 #7 0x0000555555a56708 in __static_initialization_and_destruction_0 () at /home/esteban/.conan2/p/b/conan9c3d7421a84e0/b/src/colorspace.cpp:20 #8 0x0000555555a567e3 in _GLOBAL__sub_I__ZN11aliceVision5image24getGlobalColorConfigOCIOEv () at /home/esteban/.conan2/p/b/conan9c3d7421a84e0/b/src/colorspace.cpp:53 #9 0x00007ffff7892304 in call_init (env=<optimized out>, argv=0x7fffffffd618, argc=1) at ../csu/libc-start.c:145 #10 __libc_start_main_impl (main=0x555555a56159 <main(int, char**)>, argc=1, argv=0x7fffffffd618, init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffd608) at ../csu/libc-start.c:347 #11 0x0000555555a56095 in _start ()

I permit to ping @irieger which know well openimageio and opencolorio.
@irieger do you have already encounter this error when invoking ColorConfig?

Regards.

@EstebanDugueperoux2
Copy link
Contributor Author

To make an addition, I have tried to reproduce with a shared build with following command because opencolorio requires static minizip-ng as requirement:

conan create . -o "*/*:shared=True" -o "minizip-ng/*:shared=False"

but got linking error on openimageio:

... /usr/bin/ld: /home/esteban/.conan2/p/b/openc9be125abbd8c0/p/lib/libOpenColorIO.so.2.3: undefined reference topystring::os::path::splitext(std::__cxx11::basic_string<char, std::char_traits, std::allocator >&, std::__cxx11::basic_string<char, std::char_traits, std::allocator >&, std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)'
/usr/bin/ld: /home/esteban/.conan2/p/b/openc9be125abbd8c0/p/lib/libOpenColorIO.so.2.3: undefined reference to YAML::Emitter::SetLocalValue(YAML::EMITTER_MANIP)' /usr/bin/ld: /home/esteban/.conan2/p/b/openc9be125abbd8c0/p/lib/libOpenColorIO.so.2.3: undefined reference to YAML::convert::decode(YAML::Node const&, bool&)'
/usr/bin/ld: /home/esteban/.conan2/p/b/openc9be125abbd8c0/p/lib/libOpenColorIO.so.2.3: undefined reference to vtable for YAML::BadSubscript' ...

also tried to do a full shared build with #24395 fix to have opencolorio works with a shared build of minizip-ng dependency, but links correctly only with gold linker and running also.

Also tried with a shared build without conan, i.e. only with cmake using assimp and openimageio requirements from ubuntu package manager and it build and run without error.
Was not able to do a static build because ubuntu doesn't seems to provides static libs for openimageio and assimp.

I would like to confirm if it is an openimageio recipe issue but doing same static build scenario without conan but I'm not able to have a static lib in ubuntu/debian repos.

Regards.

@irieger
Copy link
Contributor

irieger commented Nov 8, 2024

Hey,

I have to admit I have never called into any of the OCIO related sections of OIIO. Don't know if it calls any function when just using it to read/write files but wouldn't assume so. However I now tried the OIIO::ColorConfig initialization and had it work in several scenarios. Somehow your sample code doesn't work also here, but I created a kind of derived sample and there I don't get any segfaults. Have to admit I have no clue why it would work in one case but not the other as to me the logic looks quite the same.

See: https://github.com/irieger/conan_assimp_test, I added a folder oiio-only where I first started to reproduce just the color part and added back some code to include assimp (just copying parts from you. None of the situations failed. Also I was curious what an invalid path to OIIO::ColorConfig does but it seems to just load some defaults, no error.

I really don't see why my code should work while yours is failing in this way.

Also tested the OIIO ColorConfig part by hacking in a test to my project where I use OIIO and thus already link OIIO, shared all the way (using my https://github.com/irieger/conan-center-index/tree/with-non-upstream-changes branch where I have removed the config error on shared builds, requires gold or mold on Linux to build). So not really able to reproduce it besides your code where I'm not sure in why it could fail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants