monocasual / giada

Your Hardcore Loop Machine.
https://www.giadamusic.com
GNU General Public License v3.0
1.73k stars 99 forks source link

Fails to find FLTK despite cmake supporting FLTK #537

Closed yurivict closed 11 months ago

yurivict commented 2 years ago

Environment

Describe the bug

CMake Error at CMakeLists.txt:310 (find_package):
  Could not find a package configuration file provided by "FLTK" with any of
  the following names:

    FLTKConfig.cmake
    fltk-config.cmake

  Add the installation prefix of "FLTK" to CMAKE_PREFIX_PATH or set
  "FLTK_DIR" to a directory containing one of the above files.  If "FLTK"
  provides a separate development package or SDK, be sure it has been
  installed.

cmake-3.22.1 has files to find FLTK:

$ pkg info -l cmake | grep -i fltk
    /usr/local/share/cmake/Help/command/fltk_wrap_ui.rst
    /usr/local/share/cmake/Help/module/FindFLTK.rst
    /usr/local/share/cmake/Help/module/FindFLTK2.rst
    /usr/local/share/cmake/Modules/FLTKCompatibility.cmake
    /usr/local/share/cmake/Modules/FindFLTK.cmake
    /usr/local/share/cmake/Modules/FindFLTK2.cmake

but giada still fails to find it.

fltk-1.3.8

gvnnz commented 2 years ago

Happy new year @yurivict ! :tada:

We are still stuck with FLTK 1.3.4 in our build workflow: that could be the reason why we don't get such error. I'll update FLTK to the latest version for Giada v.0.20 and report back.

danfe commented 2 years ago

This is pretty easy to fix, just follow the documented procedure. The following simple patch allows the program to find and correctly link FLTK (supposedly any version).

@@ -307,9 +307,8 @@ endif()

 set(FLTK_SKIP_FLUID TRUE)  # Don't search for FLTK's fluid
 set(FLTK_SKIP_OPENGL TRUE) # Don't search for FLTK's OpenGL
-find_package(FLTK CONFIG REQUIRED)
-list(APPEND LIBRARIES fltk fltk_gl fltk_forms fltk_images)
-message("FLTK library found in " ${FLTK_DIR})
+find_package(FLTK REQUIRED)
+list(APPEND LIBRARIES ${FLTK_LIBRARIES})
gvnnz commented 2 years ago

@danfe your patch works, but the official FLTK documentation from version 1.3.8 suggests the following:

find_package(FLTK REQUIRED NO_MODULE)
list(APPEND LIBRARIES fltk)
list(APPEND INCLUDE_DIRS ${FLTK_INCLUDE_DIRS})
danfe commented 2 years ago

but the official FLTK documentation from version 1.3.8 suggests the following

Any (working) approach is okay so long as it does not use FLTKConfig.cmake and friends, but just the CMake glue.

gvnnz commented 2 years ago

Fixed in 67bab1865cc340f5df556016542e05b821677915. @yurivict does it work for you?

yurivict commented 2 years ago

0.20.0 still has this problem.

gvnnz commented 2 years ago

@yurivict in 0.20.0 we follow the official instructions coming from the FLTK documentation. Maybe there's something wrong with their CMake script?

gvnnz commented 2 years ago

@yurivict any feedback on this?

yurivict commented 2 years ago

I think it would be best if giada would be able to find FLTK even if FLTK wouldn't have cmake files installed. cmake officially supplies FLTK-discovering macros:

$ pkg info -l cmake | grep -i fltk | grep "\.cmake"
    /usr/local/share/cmake/Modules/FLTKCompatibility.cmake
    /usr/local/share/cmake/Modules/FindFLTK.cmake
    /usr/local/share/cmake/Modules/FindFLTK2.cmake
gvnnz commented 2 years ago

@yurivict but as far as I understand that's exactly the deprecated approach @danfe warned against?

yurivict commented 2 years ago

No, @danfe for some reason doesn't want to add cmake files to FLTK package ans asks to support both ways to detect FLTK.

gvnnz commented 2 years ago

OK so I'm officially lost at this point. @yurivict please feel free to submit a patch/PR that fixes the problem you are describing. I'm moving this issue to 1.0 milestone.

danfe commented 2 years ago

I understand that's exactly the deprecated approach @danfe warned against?

There's nothing deprecated about using CMake native scripts, where did I warn against it?

No, @danfe for some reason doesn't want to add cmake files to FLTK package ans asks to support both ways to detect FLTK.

Well, the reasons are rather sound to me: it would break certain things and that would require additional fixing, including generated CMake files themselves (e.g. $WRKSRC gets embedded in them), for no real benefits. CMake provides full support for locating FLTK bits by itself.

OK so I'm officially lost at this point.

I've provided working variant in the earlier comment. I might pull the current code and adjust my patch, but there really should not be anything hard about it.

gvnnz commented 2 years ago

I might pull the current code and adjust my patch [...]

A PR would be very welcome.

gvnnz commented 11 months ago

Follow-up: currently we are fetching FLTK via CMake's fetch content, so this issue should not be relevant anymore - at least until we switch back to the original method, when FLTK 1.4 will be out. Closing for now.