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

[magnum] Builds for emsdk #7412

Closed
wants to merge 7 commits into from
Closed

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented Sep 25, 2021

Specify library name and version: lib/1.0

After: #7410

With these options disabled, it builds (running Macos)... I'm not sure how to organize it. There are options that don't make sense in Emscripten, others that just fail, others that might fail because some requirement is not prepared. Asking for help about how to organize it (ping @SSE4 ).

conan create magnum/all/conanfile.py magnum/2020.06@ --profile:host=emsdk --profile:build=default

profile:host

[settings]
os=Emscripten
arch=wasm
compiler=clang
compiler.version=14
build_type=Release
[options]
[conf]
[build_requires]
*: emsdk/2.0.30
[env]

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@SSE4
Copy link
Contributor

SSE4 commented Sep 26, 2021

There are options that don't make sense in Emscripten, others that just fail, others that might fail because some requirement is not prepared.

I am not sure I understand what's the problem.

  • if some option doesn't make sense for Emscripten, remove it the same way we remove Apple-specific options for Windows (
    del self.options.egl_context
    )
  • if some just fail (do you mean some code just doesn't compile for Emscripten?) need to check, if it's an upstream issue. maybe will require a patch or new version
  • if some requirement is not prepared, we should prepare it first

in cases 2 and 3 we always can raise ConanInvalidConfiguration with meaningful comment (FIXME/TODO).
it's not needed to support all the possible combinations of options in the first iteration.
some reasonable default is already better than nothing.

@@ -391,6 +434,12 @@ def _patch_sources(self):
"EGL::EGL",
"egl::egl")

# Copy FindOpenGLES2/3.cmake to build folder
shutil.copy(os.path.join(self._source_subfolder, "modules", "FindOpenGLES2.cmake"), "FindOpenGLES2.cmake")
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't we remove them? we don't package CMake modules.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not being packaged. I´m just reusing the file that the library provides. I really don't know if/how to create a Conan package for it, I don't have enough experience with it... is this a different package? Is the same opengl package we have?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think GLES are considered different entities from OpenGL.
why doesn't it find them if they aren't copied?
isn't it enough to adjust CMAKE_MODULE_PATH/CMAKE_PREFIX_PATH?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why doesn't it find them if they aren't copied?
isn't it enough to adjust CMAKE_MODULE_PATH/CMAKE_PREFIX_PATH?

In the sources repo, these files are in a directory together with other FindXXXX files (https://github.com/mosra/magnum/tree/master/modules), that directory is removed from CMAKE_MODULE_PATH/CMAKE_PREFIX_PATH in the recipe. We don't need the rest of them as we are providing those files with Conan.

set(MAGNUM_TARGET_VK 1)
endif()

-if(CORRADE_TARGET_EMSCRIPTEN)
Copy link
Contributor

Choose a reason for hiding this comment

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

why not needed? what does UseEmscripten do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The source of the patch is documented, and the reasons are explained by the library author himself: mosra/magnum#490. Basically he was providing his own Emscripten toolchain in the past because the official ones were quite unstable. New versions already remove this custom toolchain.

SSE4
SSE4 previously approved these changes Sep 26, 2021
@SSE4 SSE4 requested a review from uilianries September 26, 2021 11:54
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

Failure in build 7 (63a0dce58dee45b3a22b64c07eef303dba4a9684):

  • magnum/2020.06@:
    CI failed to create some packages (All logs)

    Logs for packageID c0517e26f93a269ac16b5cec7ee841ad836134c3:
    [settings]
    arch=x86_64
    arch_build=x86_64
    build_type=Release
    compiler=gcc
    compiler.libcxx=libstdc++
    compiler.version=5
    os=Linux
    os_build=Linux
    [options]
    magnum:shared=False
    
    [...]
    -- Installing: /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/package/c0517e26f93a269ac16b5cec7ee841ad836134c3/lib/magnum/importers/TgaImporter.so
    -- Set runtime path of "/home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/package/c0517e26f93a269ac16b5cec7ee841ad836134c3/lib/magnum/importers/TgaImporter.so" to ""
    -- Installing: /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/package/c0517e26f93a269ac16b5cec7ee841ad836134c3/lib/magnum/importers/TgaImporter.conf
    -- Installing: /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/package/c0517e26f93a269ac16b5cec7ee841ad836134c3/./include/MagnumPlugins/TgaImporter/TgaImporter.h
    -- Installing: /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/package/c0517e26f93a269ac16b5cec7ee841ad836134c3/./include/MagnumPlugins/TgaImporter/configure.h
    -- Installing: /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/package/c0517e26f93a269ac16b5cec7ee841ad836134c3/lib/magnum/audioimporters/WavAudioImporter.so
    -- Set runtime path of "/home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/package/c0517e26f93a269ac16b5cec7ee841ad836134c3/lib/magnum/audioimporters/WavAudioImporter.so" to ""
    -- Installing: /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/package/c0517e26f93a269ac16b5cec7ee841ad836134c3/lib/magnum/audioimporters/WavAudioImporter.conf
    -- Installing: /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/package/c0517e26f93a269ac16b5cec7ee841ad836134c3/./include/MagnumPlugins/WavAudioImporter/WavImporter.h
    -- Installing: /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/package/c0517e26f93a269ac16b5cec7ee841ad836134c3/./include/MagnumPlugins/WavAudioImporter/configure.h
    [HOOK - conan-center.py] post_package(): [PACKAGE LICENSE (KB-H012)] OK
    [HOOK - conan-center.py] post_package(): [DEFAULT PACKAGE LAYOUT (KB-H013)] If you are trying to package a tool put all the contents under the 'bin' folder
    [HOOK - conan-center.py] post_package(): [MATCHING CONFIGURATION (KB-H014)] OK
    [HOOK - conan-center.py] post_package(): [SHARED ARTIFACTS (KB-H015)] OK
    [HOOK - conan-center.py] post_package(): [PC-FILES (KB-H020)] OK
    [HOOK - conan-center.py] post_package(): [CMAKE-MODULES-CONFIG-FILES (KB-H016)] OK
    [HOOK - conan-center.py] post_package(): [PDB FILES NOT ALLOWED (KB-H017)] OK
    [HOOK - conan-center.py] post_package(): [LIBTOOL FILES PRESENCE (KB-H018)] OK
    [HOOK - conan-center.py] post_package(): [MS RUNTIME FILES (KB-H021)] OK
    CMake Warning:
      Manually-specified variables were not used by the project:
    
        CMAKE_EXPORT_NO_PACKAGE_REGISTRY
        CMAKE_INSTALL_BINDIR
        CMAKE_INSTALL_DATAROOTDIR
        CMAKE_INSTALL_INCLUDEDIR
        CMAKE_INSTALL_LIBDIR
        CMAKE_INSTALL_LIBEXECDIR
        CMAKE_INSTALL_OLDINCLUDEDIR
        CMAKE_INSTALL_SBINDIR
        WITH_VULKANTESTER
    
    
    /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/build/c0517e26f93a269ac16b5cec7ee841ad836134c3/source_subfolder/src/Magnum/Trade/PhongMaterialData.cpp:71:58: warning: unused parameter ‘other’ [-Wunused-parameter]
     PhongMaterialData::PhongMaterialData(PhongMaterialData&& other) noexcept = default;
                                                              ^
    /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/build/c0517e26f93a269ac16b5cec7ee841ad836134c3/source_subfolder/src/Magnum/Trade/PhongMaterialData.cpp:71:76: note: synthesized method ‘Magnum::Trade::PhongMaterialData::PhongMaterialData(Magnum::Trade::PhongMaterialData&&)’ first required here 
     PhongMaterialData::PhongMaterialData(PhongMaterialData&& other) noexcept = default;
                                                                                ^
    /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/build/c0517e26f93a269ac16b5cec7ee841ad836134c3/source_subfolder/src/MagnumPlugins/ObjImporter/ObjImporter.cpp: In member function ‘virtual Corrade::Containers::Optional<Magnum::Trade::MeshData> Magnum::Trade::ObjImporter::doMesh(Magnum::UnsignedInt, Magnum::UnsignedInt)’:
    /home/conan/w/BuildSingleReference/.conan/data/magnum/2020.06/_/_/build/c0517e26f93a269ac16b5cec7ee841ad836134c3/source_subfolder/src/MagnumPlugins/ObjImporter/ObjImporter.cpp:483:56: warning: ‘primitive’ may be used uninitialized in this function [-Wmaybe-uninitialized]
             std::move(vertexData), std::move(attributeData)};
                                                            ^
    [HOOK - conan-center.py] post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] The *.cmake files have to be placed in a folder declared as `cpp_info.builddirs`. Currently folders declared: ['', '', '', '', '', '', '']
    [HOOK - conan-center.py] post_package_info(): WARN: [CMAKE FILE NOT IN BUILD FOLDERS (KB-H019)] Found files: ./lib/cmake/CorradeLibSuffix.cmake; ./lib/cmake/UseCorrade.cmake; ./lib/cmake/conan-corrade-vars.cmake
    vulkan-headers/1.2.190: WARN: Lib folder doesn't exist, can't collect libraries: /home/conan/w/BuildSingleReference/.conan/data/vulkan-headers/1.2.190/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/lib
    wayland-protocols/1.21: WARN: Lib folder doesn't exist, can't collect libraries: /home/conan/w/BuildSingleReference/.conan/data/wayland-protocols/1.21/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/lib
    xorg/system: WARN: Lib folder doesn't exist, can't collect libraries: /home/conan/w/BuildSingleReference/.conan/data/xorg/system/_/_/package/5ab84d6acfe1f23c4fae0ab88f26e3a396351ac9/lib
    [HOOK - conan-center.py] post_package(): ERROR: [DEFAULT PACKAGE LAYOUT (KB-H013)] Unknown folder 'share' in the package (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H013) 
    ERROR: [HOOK - conan-center.py] post_package(): Some checks failed running the hook, check the output
    

Note: To save resources, CI tries to finish as soon as an error is found. For this reason you might find that not all the references have been launched or not all the configurations for a given reference. Also, take into account that we cannot guarantee the order of execution as it depends on CI workload and workers availability.

@@ -433,7 +482,7 @@ def package(self):
endif()
""".format(target=target, library=library)))

tools.rmdir(os.path.join(self.package_folder, "share"))
tools.rmdir(os.path.join(self.package_folder, "share", "cmake"))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tools.rmdir(os.path.join(self.package_folder, "share", "cmake"))
tools.rmdir(os.path.join(self.package_folder, "share", "cmake"))
tools.rmdir(os.path.join(self.package_folder, "share"))

Copy link
Contributor

Choose a reason for hiding this comment

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

[HOOK - conan-center.py] post_package(): ERROR: [DEFAULT PACKAGE LAYOUT (KB-H013)] Unknown folder 'share' in the package (https://github.com/conan-io/conan-center-index/blob/master/docs/error_knowledge_base.md#KB-H013) 
ERROR: [HOOK - conan-center.py] post_package(): Some checks failed running the hook, check the output

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Inside share there is a folder magnum with some files that are distributed together with the library. I can move them to a different place, or modify the hook to accept them. Wdyt @uilianries ?

Copy link
Member

Choose a reason for hiding this comment

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

I would suggest res folder, as it's managed by Conan. Of course, need to check if does not break the project with some hardcoded expected path.

@uilianries
Copy link
Member

BTW great job with Magnum, it's not an usual recipe and require a lot of effort.

@jgsogo jgsogo mentioned this pull request Oct 5, 2021
4 tasks
@stale
Copy link

stale bot commented Oct 27, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale
Copy link

stale bot commented Nov 27, 2021

This pull request has been automatically closed because it has not had recent activity. Thank you for your contributions.

@stale stale bot closed this Nov 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants