jkriege2 / JKQtPlotter

an extensive Qt5 & Qt6 Plotter framework (including a feature-richt plotter widget, a speed-optimized, but limited variant and a LaTeX equation renderer!), written fully in C/C++ and without external dependencies
http://jkriege2.github.io/JKQtPlotter/index.html
GNU Lesser General Public License v2.1
889 stars 190 forks source link

Fix CMake usage for downstream consumers. #104

Closed Neumann-A closed 10 months ago

Neumann-A commented 1 year ago

Simply said: Your CMake users will have a horrible experience with the current intended usage of jkqtplotter since they actively have to care about the fact if jkqtplotter was build as a static or shared library.

Your current usage looks like this:

find_package(JKQtPlotterSharedLib CONIFG QUIET)
set(jkplottertarget "")
if(NOT JKQtPlotterSharedLib)
  find_package(JKQtPlotterLib CONIFG REQUIRED)
  set(jkplottertarget "<some_imported_target_with_Lib_in_name>")
else()
  set(jkplottertarget "<some_imported_target_with_SharedLib_in_name>")
endif()
target_link_libraries(mytarget PRIVATE "${jkplottertarget}"  )

instead of a simple:

find_package(JKQtPlotter CONFIG REQUIRED)
target_link_libraries(mytarget PRIVATE JKQtPlotter::JKQtPlotter  )

So please provide a common entry point to find JKQtPlotter and also provide an lib agnostic target for downstream consumers.

jkriege2 commented 1 year ago

Yes, that's the current status quo ... it was designed to allow for having static and shared lib side by side.

This might go onto the TODO list for future improvements.

Remember: Currently I'm the a single main developer, whoch works in this in his spare time!

If you want to propose a better solution: Please just post a PR!

Neumann-A commented 1 year ago

it was designed to allow for having static and shared lib side by side.

Maybe that design is wrong since it ignores BUILD_SHARED_LIBS. Since you are not using OBJECT libraries to build both everything is just getting compiled twice so it is not even saving time (except for "configuration time"). What was your argument to allow building both?

jkriege2 commented 1 year ago

As I said: "Yes, that's the current status quo" ... I'm open to better solutions, but my time is currently limited.

jkriege2 commented 10 months ago

I'm working on this right now ... please have a look at https://github.com/jkriege2/JKQtPlotter/commit/b0df7a1fd7ccea4c3e097e8d42b81d5c228aff1b ... and possible bugfix-commmits that follow!

jkriege2 commented 10 months ago

I think my changes from yesterday should adress your suggestions. Could you have a look at the changed CMake-Build-System? Can I close this issue?

See https://jkriege2.github.io/JKQtPlotter/page_buildinstructions__c_m_a_k_e.html for a detailed description.

Neumann-A commented 10 months ago

Its better, not ideal since you encoded the Qt version in the targets without providing also an unversioned target. Downstream users shouldn't need to care which version of Qt to use since it is a hard requirement the moment the library itself is build and installed. Furthermore there is no way to use 5 and 6 in the same project without running into other issues.