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

Fine-grained plugins dependendy upload #32

Merged
merged 52 commits into from
Oct 11, 2024

Conversation

alxvth
Copy link
Contributor

@alxvth alxvth commented Sep 24, 2024

It is very useful to gather all plugin runtime dependencies in a single folder, immediately after building the plugin.

Here, we gather all runtime dependencies and install them to ${ManiVault_INSTALL_DIR}/$<CONFIGURATION>/PluginDependencies/PLUGINNAME where the PLUGINNAME is the current plugin's name.

Depends on ManiVaultStudio/core#692 which loads the runtime dependencies automatically before loading the plugin library.

A plugin developer can use mv_install_dependencies to set up automatic plugin runtime dependency gathering and installation with this commands:

mv_install_dependencies(${PROJECT_NAME} "hwy" "hwy_contrib")

Dependencies that are linked to our plugin ${PROJECT_NAME} and found with find_package are automatically resolve (given that their respective cmake export files are nicely setup). This even works with dependency chains, as shown in this example: we link to Faiss, which in turn depends on Lapack and OpenBlas which in turn depends on other libraries.
Otherwise, we need to pass all targets that we build in our project but do not set up with find_package to mv_install_dependencies, which in this example is the plugin itself and two dependencies, "hwy" and "hwy_contrib".

The mv_install_dependencies is automatically available when using find_package(ManiVault ... CONFIG).

All installed dependencies are listed like this (current CI output on windows):

-- Resolved: D:/.conan/bab46b/1/ExampleDependencies/Release/blake3.dll
-- Resolved: D:/.conan/bab46b/1/ExampleDependencies/Release/faiss.dll
-- Resolved: D:/.conan/bab46b/1/_deps/highway-build/Release/hwy.dll
-- Resolved: D:/.conan/bab46b/1/_deps/highway-build/Release/hwy_contrib.dll
-- Resolved: D:/.conan/bab46b/1/ExampleDependencies/Release/libgcc_s_seh-1.dll
-- Resolved: D:/.conan/bab46b/1/ExampleDependencies/Release/libgfortran-5.dll
-- Resolved: D:/.conan/bab46b/1/ExampleDependencies/Release/liblapack.dll
-- Resolved: D:/.conan/bab46b/1/ExampleDependencies/Release/libquadmath-0.dll
-- Resolved: D:/.conan/bab46b/1/ExampleDependencies/Release/libwinpthread-1.dll
-- Resolved: D:/.conan/bab46b/1/ExampleDependencies/Release/openblas.dll
-- Installing: D:\.conan\bab46b\1\package\Release/PluginDependencies/ExampleDependenciesPlugin/blake3.dll
-- Installing: D:\.conan\bab46b\1\package\Release/PluginDependencies/ExampleDependenciesPlugin/faiss.dll
-- Installing: D:\.conan\bab46b\1\package\Release/PluginDependencies/ExampleDependenciesPlugin/hwy.dll
-- Installing: D:\.conan\bab46b\1\package\Release/PluginDependencies/ExampleDependenciesPlugin/hwy_contrib.dll
-- Installing: D:\.conan\bab46b\1\package\Release/PluginDependencies/ExampleDependenciesPlugin/libgcc_s_seh-1.dll
-- Installing: D:\.conan\bab46b\1\package\Release/PluginDependencies/ExampleDependenciesPlugin/libgfortran-5.dll
-- Installing: D:\.conan\bab46b\1\package\Release/PluginDependencies/ExampleDependenciesPlugin/liblapack.dll
-- Installing: D:\.conan\bab46b\1\package\Release/PluginDependencies/ExampleDependenciesPlugin/libquadmath-0.dll
-- Installing: D:\.conan\bab46b\1\package\Release/PluginDependencies/ExampleDependenciesPlugin/libwinpthread-1.dll
-- Installing: D:\.conan\bab46b\1\package\Release/PluginDependencies/ExampleDependenciesPlugin/openblas.dll

Additionally:

  • vcpkg on the CI:
    • Show how vcpkg can be used on the CI in combination with conan, see build.yml and conanfile.py. This does not replace or remove anything, it only adds functionality.
    • Caching vcpkg dependencies on the CI. This requires setting up a secret as shown here, in our example referred to as secrets.GH_VCPKG_PACKAGES
  • Small clean-up of conanfile.py:
    • Set CMAKE_CXX_STANDARD_REQUIRED on all platforms
    • Do not set CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS on windows
    • Do not call cmake install twice
    • Do not copy ManiVault core into build folder
    • Only set ManiVault_DIR and not MV_INSTALL_DIR which is automatically set by find_package(ManiVault ... CONFIG) and only needs to be manually set when building the core
    • Remove unused variables and functions

@alxvth alxvth force-pushed the feature/FinegrainedDepManagement branch 4 times, most recently from 4ae4066 to 9f1f95b Compare October 9, 2024 08:24
@alxvth alxvth force-pushed the feature/FinegrainedDepManagement branch from 9f1f95b to 4f5ad3d Compare October 9, 2024 08:33
@alxvth alxvth force-pushed the feature/FinegrainedDepManagement branch from 2370aac to ec5f345 Compare October 9, 2024 10:30
@alxvth alxvth force-pushed the feature/FinegrainedDepManagement branch from 811ad9f to 6076125 Compare October 10, 2024 13:00
@alxvth alxvth force-pushed the feature/FinegrainedDepManagement branch from 6076125 to a5b8a00 Compare October 10, 2024 13:23
Copy link
Contributor

@bldrvnlw bldrvnlw left a comment

Choose a reason for hiding this comment

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

.github/workflows/build.yml:
Regarding the vcpkg logic : I recommend putting this into a reusable action hosted in the ManiVault/github-actions project. Project specific values can be passed as parameters. I approve this but will add a separate PR for creating this reusable action.

conanfile.py:
Does the manivault_dir needs to be set on self. (line 105) It is not used outside the generate function. Can this be changed?

@alxvth
Copy link
Contributor Author

alxvth commented Oct 11, 2024

conanfile.py:
Does the manivault_dir needs to be set on self. (line 105) It is not used outside the generate function. Can this be changed?

I changed this. It just was that way before but there is no need to bet set to self.

@alxvth alxvth merged commit 270ebbc into master Oct 11, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants