-
Notifications
You must be signed in to change notification settings - Fork 332
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
Make it available as vcpkg package #40
Comments
I do want to take on this but I would need some feedback first, before I'm going down the path until the end. I think it would take a bit of time and there isn't a sure functional outcome.
Now comes my real question:
|
@mathisloge Thanks for looking into this. As to vcpkg, I am very open to adding the project to the registry, but I can't say I have a complete understanding of how vcpkg references dependencies and what exactly needs to be done to make this work - can you clarify this (or send a link to the documentation as I was not able to find a good one)? I looked at your changes and I can't immediately tell why they are necessary. So the answer to your first question is yes, unless this requires major changes to CMake, which will result in inconsistent dependencies and major build breakage risks.
Depending which platform you build on and for, and which components you enable, CMake requirements may vary. While it is easy to update CMake on Windows, Android Studio, for example, bundles CMake and it is usually quite out-of-date. We are trying to keep CMake requirements low when possible.
If you have a PR that makes it whitespace-compatible - please submit it. But please keep the PR scope narrow to not mix unrelated changes in one commit. |
@TheMostDiligent thanks for the fast response.
unfortunately there is currently no good documentation
In the world of vcpkg, the goal and the requirement is that all libraries build together. That is, if a library has dependency A and Diligent also has dependency A, both must build together with the same version in the catalogue. As a result, every dependency is considered an external dependency and must be based on the finding mechanism of cmake. Submodules are not allowed. In particular, with shared libraries, submodules can cause problems if libraries bring in their own and both use different versions of the dependency A.
if Diligent is consumed in the current way, i.e. without vcpkg, the submodules are still used. Therefore, nothing should change here, except for the way the include paths are introduced into Diligent. |
Is my understanding correct that basically vcpkg is a catalogue of different libraries at specific versions. If one library depends on another, the dependency must be present in the catalogue and its version is defined by vcpkg?
It may build at this very moment, but like I said in the beginning, there is no guarantee it will consistently work in the future is there is no way to control the version of dependency packages.
Does Microsoft team do that? Is there a continuous process that makes ensures that a library still builds after it has been addded? |
yes thats correct. If a dependency is not in the catalogue, one has to add it.
Diligent is pinpointed to a specifc version. If a dependency of Diligent is updated and breaks the build, the person who updates the dependency must also make sure, that Diligent builds again. Typically this would result in a PR for Diligent to fix those problems. See microsoft/vcpkg#19665 or microsoft/vcpkg#16494 for PRs that had to fix a lot of issues in other projects before updating.
not in the main catalogue. that would break the whole design decision of "everything can be build together with the current catalogue you have". There is versioning for the final application that consumes vcpkg https://github.com/microsoft/vcpkg/blob/master/docs/specifications/versioning.md
the microsoft team reviews every single PR. CIs are building every PR with windows-x86, windows-x64, windows-x64-static, x64-uwp, arm64, arm-uwp, x64-osx and x64-linux. Before a update is merged, all CIs have to be green. As said above, if dependencies gets updated, all projects are build which depend on that dependency. If there are still runtime issues, those will be fixed with a new PRs when reported from users. It is commonly to redirect users from diligent issues to vcpkg issues, if they have dependency issues that are related to vcpkg. |
just as a side note: it could be a longer journey, but I guess that it is worth it. e.g. for mapnik I needed one and a half year until it was added in vcpkg microsoft/vcpkg#14628 and microsoft/vcpkg#18849 but it also contributed a lot to the original repository (mapnik/mapnik#4191 , mapnik/mapnik#4286) just to mention a few. |
OK, things are a bit more clearer now. Since code stability and quality is paramount for us, we can't switch the main build system to the one where dependencies are not controlled by us. My current thinking to make vcpkg work is this: change CMake files to check if every 3rd party target already exists (it is already done in a couple of places). This way, when building with vcpkg, a special CMake script will import all targets or initialize them as necessary before running Diligent CMake files, and Diligent will use these existing targets instead of using its submodules. This should make the system flexible enough to support vcpkg (as well as other scenarios where dependencies are provided by the app) and also to keep the default build approach intact. What do you think? |
sound good. I've planned that, too. I've added a CMake option Don't know how fast I'm progressing this and the next week but I would estimate that I'm having a working build by the end of april. vcpkg working branch: https://github.com/mathisloge/vcpkg/tree/diligent |
Good, that sounds like a good plan. I want to make it more granular - to check every single project if it is already present in the system. This way an application may provide its own version of any specific third-party dependency. |
@TheMostDiligent sounds good. I will do that. Starting with core. |
@mathisloge I updated core, tools and samples to allow their dependencies to be provided externally. If required targets already exist, Diligent will use those instead of the bundled - nothing extra needs to be done. In my test I put all submodule dependencies into one folder ThirdParty:
CMake file for this folder: set(SPIRV-Headers_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/SPIRV-Headers")
add_subdirectory(SPIRV-Tools)
add_subdirectory(glslang)
add_subdirectory(SPIRV-Cross)
set(gtest_force_shared_crt ON CACHE BOOL "Use shared (DLL) run-time lib even when Google Test is built as static lib.")
add_subdirectory(googletest)
add_subdirectory(glfw)
option(BUILD_SHARED_LIBS "Build shared library" OFF)
add_subdirectory(xxHash/cmake_unofficial) Some of the dependencies do not create targets. For them a add_subdirectory(ThirdParty)
set(DILIGENT_VULKAN_HEADERS_DIR "${CMAKE_CURRENT_SOURCE_DIR}/ThirdParty/Vulkan-Headers")
set(DILIGENT_SPIRV_HEADERS_DIR "${CMAKE_CURRENT_SOURCE_DIR}/ThirdParty/SPIRV-Headers")
set(DILIGENT_VOLK_DIR "${CMAKE_CURRENT_SOURCE_DIR}/ThirdParty/volk")
set(DILIGENT_ARGS_DIR "${CMAKE_CURRENT_SOURCE_DIR}/ThirdParty/args")
set(DILIGENT_DEAR_IMGUI_PATH "${CMAKE_CURRENT_SOURCE_DIR}/ThirdParty/imgui")
set(DILIGENT_NUKLEAR_DIR "${CMAKE_CURRENT_SOURCE_DIR}/ThirdParty/nuklear") Oveall, this allows all third-party dependencies to be defined outside of the Diligent CMake file and it will transparently pick them up. There are few other dependencies that are checked in into the tree: Core:
Tools:
|
@TheMostDiligent oh great. I will look into it and see if there are some bits not working as expected with vcpkg. about volk: if(PLATFORM_LINUX)
target_link_libraries(Diligent-GPUTestFramework
PUBLIC
dl # Required by Volk
xcb
)
endif() aren't needed. in ThirdParty/CMakeLists.txt: set(VOLK_PULL_IN_VULKAN OFF)
add_subdirectory(volk)
set_target_properties(volk PROPERTIES EXCLUDE_FROM_ALL 1 EXCLUDE_FROM_DEFAULT_BUILD 1) # do not build volk library target - we only need the header target All changes for volk: DiligentGraphics/DiligentCore@63efbd5 Regarding the other listed targets: Core:
Tools:
So to support those option(USE_EXTERNAL_XXXXX OFF)
if(USE_EXTERNAL_XXXXX)
find_package(XXXX [CONFIG] REQUIRED)
else()
add_library(myCustomLibOfXXXX ....)
add_library(XXXX::XXXX ALIAS myCustomLibOfXXXX)
endif()
add_executable(DiligentCore ...)
target_link_libraries(DiligentCore ... XXXX::XXXX) |
@mathisloge I committed a bunch of updates:
CMake files now first try to check if the target already exists. If so, they use the existing target, otherwise add required subdirectories and create targets as necessary. As a result, all external decencies can now be loaded outside of Diligent's CMake files and will be automatically picked up. No need for The way it should work now is that all required external packages are found with
I also think that there should be two options: to install DiligentCore only, and the whole project. |
for vcpkg I need to make seperate targets, since it is discouraged to add submodules to ports. so there will be Maybe we could also add a optional file include where one can hook in a cmake script with all the find_package things? e.g.: if(EXISTS "${DILIGENT_FIND_EXTERNAL_LIBS}")
include("${DILIGENT_FIND_EXTERNAL_LIBS}")
endif() |
Are they going to be separate packages? Only
Yes, I think that every submodule can have a script to load its required dependencies.
Where this file will be supplied from? I mean, why would it not exist? I would expect something like if(DILIGENT_FIND_EXTERNAL_LIBS)
include("find_external_libs.cmake")
endif() BTW, how are you debugging the set up before proposing a change to vcpkg? |
yes.
IIUC you want people to enable to find some external packages and some submodule packages. By enabling users to specify a custom script file somewhere in the project or on disk, they could add the paths or find_package calls as they liked. for vpckg purposes it would be this way:
I will setup a test project which tries to find the core lib, tools etc. and for runtime test copying the samples. propably I will need to add a cmake wrapper to find all diligent libs |
What I don't understand in case of vcpkg is who is responsible for downloading these decencies? Should we assume that if we specify these dependencies, then they all are present when our CMake runs? I created a test project where all third-party libs were in a separate folder and everything built and ran fine. Here is the CMake file for my folder with the libs: add_subdirectory(glew)
add_subdirectory(Vulkan-Headers)
add_subdirectory(SPIRV-Headers)
add_subdirectory(SPIRV-Tools)
add_subdirectory(SPIRV-Cross)
add_subdirectory(glslang)
add_subdirectory(volk)
set(gtest_force_shared_crt ON CACHE BOOL "Use shared (DLL) run-time lib even when Google Test is built as static lib.")
add_subdirectory(googletest)
add_subdirectory(glfw)
option(BUILD_SHARED_LIBS "Build shared library" OFF)
add_subdirectory(xxHash/cmake_unofficial)
add_subdirectory(libjpeg-9a)
add_subdirectory(libtiff)
add_subdirectory(lpng-1.6.17)
add_subdirectory(zlib-1.2.8)
set(DILIGENT_ARGS_DIR "${CMAKE_CURRENT_SOURCE_DIR}/args" CACHE PATH "" FORCE)
set(DILIGENT_DEAR_IMGUI_PATH "${CMAKE_CURRENT_SOURCE_DIR}/imgui" CACHE PATH "" FORCE)
set(DILIGENT_NUKLEAR_DIR "${CMAKE_CURRENT_SOURCE_DIR}/nuklear" CACHE PATH "" FORCE) With the remaining path variables I think it should be possible to remove them too, if really necessary. So everything should be good to go. |
In short: yes, all needed dependencies are installed when running the build step of diligent-xxx vcpkg has a catalogue of many libraries, each entry is called
So if my project called XY now wants to consume diligent-core, the vcpkg-tool will look at its vcpkg.json and scans it dependencies. Then it will look at the dependencies of the dependencies and so on. It will construct a build graph to build all deps in the correct order. Hope it is a bit clearer now. The thing with vcpkg is that it won't take anything specified variables in our projects CMakeLists.txt to build any dependencies. |
Yes, that makes it clearer - thanks.
Is it possible to also configure the dependent projects? Diligent sets some required options for third-party libs and also disables unnecessary targets to reduce clutter.
Or did you mean that third-party libs can't be configured?
I actually now think that it may not make a lot of sense to install the engine itself. While core has a very defined API and produces dynamic/static libs that can be easily used, other modules are mostly indented to be used as source. So |
no. The way it works is that each port defines a set of possible features. Imagine Port A B and C C needs vcpkg will instruct the buildsystems to build the following configuration: So there is only build what is needed to build everything that was requested.
if that were possible, the goal of vcpkg of building everything together with everything would be impossible to achieve.
yes, sorry. those can only be configured via features (defined in a vcpkg.json)
I will first implement diligent-core. And then looking for the other tools/libraries. but as I said at the beginning, I don't have much time this week and next week. So I don't expect any results until the end of april. |
@mathisloge Any update on this? |
Is this still a goal or has it been officially ruled impossible? |
There is nothing impossible, but we don't have bandwidth to work on this now. |
Cool no problem.
…On Thu, Sep 5, 2024, 11:32 PM Assiduous ***@***.***> wrote:
There is nothing impossible, but we don't have bandwidth to work on this
now.
—
Reply to this email directly, view it on GitHub
<#40 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAM5USLRRQXZRGQEASSHSPTZVFD6JAVCNFSM6AAAAABNXHUJPOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZTGM2DCOJQHA>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
Make it available through vcpkg
The text was updated successfully, but these errors were encountered: