inexorgame-obsolete / deprecated-cube-engine-inexor

UNMAINTAINED: Please have a look at the vulkan-renderer
https://inexor.org
zlib License
11 stars 1 forks source link

Only build the Boost libraries we do actually use #541

Closed Croydon closed 5 years ago

Croydon commented 6 years ago

This should reduce rare build time dramatically.

Thanks to the amazing work of @bincrafters we can do this now much more easier than we our current Conan boost package.

I can do once again the CI/build part, but I have actually no idea what Boost libs we do require nor how to easily find that out.

@aschaeffer @a-teammate Could somebody of you please provide a list with the Boost libs we are currently using?

a-teammate commented 6 years ago

I think the list is in cmake/require_thirdparty_dependencies.cmake

Croydon commented 6 years ago

Thanks!

find_package(Boost REQUIRED COMPONENTS thread random filesystem regex program_options system )

solvingj commented 6 years ago

I just stumbled across this issue and wanted to try to save you further time if possible. I'm sorry to say but the Bincrafters packages are not currently compatible with the find facilities of CMake. They can only be brought in by using the conanbuildinfo.cmake and linked to with ${CONAN_LIBS}. This is one of a few unfortunate challenges which exist in the space between CMake's design of find_package and systems like Conan which dynamically generate CMake files. We've had many discussions internally and with Conan team, and we were unable to identify any reasonable/supportable solution that would work with CMake find.

Croydon commented 6 years ago

@solvingj We are currently using @lasote's boost package. What are you doing differently that CMake find_package doesn't work?

solvingj commented 6 years ago

Lasote's is one super-package named Boost, which contains all Boost libraries. This package generates a single CMake target named Boost, and represents the individual libraries as CMake "components" under that target.

Bincrafter's is 130 separate packages, each of which contains a single library. These packages each generate a unique CMake targets using the full library name; for example Boost.Algorithm. Thus, our packages do not generate a "Boost" target which is compatible with findBoost scripts.

Does that clarify it a bit?

a-teammate commented 6 years ago

Ah okay good to know! so we need to reference the above list of components individually in our dependencies.py and afterwards import them in cmake as we do it with separate conan libs. Could work :) Good that our list is so short currently :)

Croydon commented 6 years ago

Would it somehow be possible to create an empty super package with some custom findBoost methode, where every single Boost Library package is registering itself there?

-- conan-boost - "empty target", searches for existing Boost.xyz targets, exports custom findBoost.cmake methode

-> Install conan-boost-thread

-- conan-boost - finds Boost.Thread, CMake find methode can find Boost.Thread through findBoost ---- conan-boost-thread

-> Install conan-boost-system

-- conan-boost - finds Boost.Thread, CMake find methode can find Boost.Thread and Boost.System through findBoost ---- conan-boost-thread ---- conan-boost-system

...

a-teammate commented 6 years ago

The problem is that conan install and cmake generation are operating at different stages. You first do conan install (with all boost libs) and afterwards cmake.. (otherwise nothing can be found since nothing was installed) Which would bring us back to the beginning. A solution would be to couple cmaking and conan installing less loosely. i.e. by letting cmake execute conan install each time another boost lib got requested. I just doubt that's worth the effort.

Croydon commented 6 years ago

Yes, but can't CMake just look for other CMake targets at runtime?

solvingj commented 6 years ago

Yes, it's perfectly workable. The unfortunate thing I was pointing out is that we can't satisfy the existing findBoost() stuff that's out there, so our packages don't just "drop in" to that. You have to change your CMake configs to use our packages, and we're a bit disappointed about that.

solvingj commented 6 years ago

Actually, we just had a promising suggestion come in here: https://github.com/bincrafters/community/issues/26

If you have time, please take a look and potentially give it a try. I'll also have some of our team try it out as well, but your feedback would be invaluable.

Croydon commented 6 years ago

Postponed till Bincrafter's Boost recipes get stable.

lasote commented 6 years ago

FYI I released monolithic 1.66.0 recipe to conan-community, will push to promote it to conan center soon. It required a lot of work, tested even with cross-building to ARM.

Croydon commented 6 years ago

@lasote Thanks for your work! It already landed in our master branch :)

https://github.com/inexorgame/inexor-core/blob/f470beb201d1614abe5b2bc9da824ce800ebd138/dependencies.py#L6

Croydon commented 5 years ago

The main motivation to switch to a modularized Boost package was the build time. In the meantime we had several huge changes to our build chain, including that we are now linking in the package directly into our own Conan repository (#599).

While I'm still happy that there is a modularized Boost package I don't think any longer that Inexor would benefit in a meaningful way from it (because of all the changes).

It would be nice to get this PR merged (cc @lasote) https://github.com/conan-community/conan-boost/pull/124 but beyond that it seems quite fine as it is right now.

Thanks for all the input and work! 😄