search

stlab / libraries

ASL libraries will be migrated here in the stlab namespace, new libraries will be created here.
https://stlab.cc
Boost Software License 1.0
660 stars 65 forks source link

Restore Qt5 compatibility #524

Closed touraill-adobe closed 1 year ago

touraill-adobe commented 1 year ago

Fixes #518

touraill-adobe commented 1 year ago

I tried to keep things simple, I'm not sure if an option or something like that is needed; if a project wants to ensure Qt5 is used over Qt6, it can disable the find_package or inject a dummy FindQt6.cmake so I think this approach is sufficient. Let me know what you think!

touraill-adobe commented 1 year ago

Sounds like a good plan, I'll update the PR accordingly!

Another improvement could be to do the various find_package only as needed (for instance there is no need to search Qt if STLAB_MAIN_EXECUTOR is already set to, say, libdispatch), but this is a bit orthogonal to this PR topic, if you think it would be useful I could make a follow-up PR.

camio commented 1 year ago

IIUC, conditionally calling find_package would be a configure-time performance benefit. It doesn't seem like the benefit there outweights the additional complexity.

touraill-adobe commented 1 year ago

IIUC, conditionally calling find_package would be a configure-time performance benefit. It doesn't seem like the benefit there outweights the additional complexity.

Some projects will download dependencies automatically when find_package(X) is called, so the cost may actually be quite high (like downloading two Qt binary packages for instance, or cloning git repos). This use case is not far-fetched: some Adobe projects do it and it is even an example in the cmake doc (google test is fetched on demand in a dependency provider).

touraill-adobe commented 1 year ago

I updated the PR, is this what you had in mind?

camio commented 1 year ago

Thanks for the PR @touraill-adobe!