Skip to content
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

fltk-config needs improvements when created by CMake #129

Open
Albrecht-S opened this issue Aug 24, 2020 · 34 comments
Open

fltk-config needs improvements when created by CMake #129

Albrecht-S opened this issue Aug 24, 2020 · 34 comments
Assignees
Labels
active Somebody is working on it bug Something isn't working CMake CMake bug or needs CMake support

Comments

@Albrecht-S
Copy link
Member

Albrecht-S commented Aug 24, 2020

Please add all your observations regarding fltk-config (created by CMake) to this issue so we have a record and fltk-config can be fixed.

@Albrecht-S Albrecht-S added bug Something isn't working CMake CMake bug or needs CMake support labels Aug 24, 2020
@Albrecht-S Albrecht-S self-assigned this Aug 24, 2020
@Albrecht-S
Copy link
Member Author

@imaclmaca wrote in #115 (comment):

I find that it [ fltk-config ] doesn’t return anything for the "fltk-config --optim” case, it is just blank. ...
I do not know if this is just that the fltk-config is not set, or if the build really is running without any optimisations enabled!
The autoconf version says:

ian$ ./fltk-config --optim
-Os -Wno-deprecated-declarations -Wall -Wunused -Wno-format-y2k -fno-exceptions -fno-strict-aliasing

I believe it's both:

  1. we don't set all the additional compiler options we set with configure/make
  2. even if users set their own option with CMake (OPTION_OPTIM) this is not yet reflected in fltk-config

Note however that CMake uses some default debug and optimization switches to actually build FLTK IF you set an appropriate CMAKE_BUILD_TYPE for standard (single config) build types like Makefiles and also for the selected build type when building in multi config builds (VS, Xcode).

Multi config builds (VS, Xcode) have another "special problem": since there are multiple configurations, fltk-config can't be uniquely configured during the CMake configuration phase because the compiler switches differ WRT the chosen build type. Hence we might need to create fltk-config during the build process (per build configuration) and/or at installation time. I need to check options how to do this...

@ManoloFLTK
Copy link
Contributor

ManoloFLTK commented Aug 25, 2020

About the situation on the macOS platform:

The script BUILD/fltk-config created by cmake fails in my hands for 2 reasons :

  1. the compilation option "-isysroot xxx" which determines what SDK version is used is missing;
  2. the -lfontconfig present in LDLIBS must be removed.

My belief is that these 2 lines of BUILD/fltk-config
CC="/Applications/Xcode11.5.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc"
CXX="/Applications/Xcode11.5.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++

should be rewritten as follows (and for my setup):
CC="/Applications/Xcode11.5.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc"
-isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk
CXX="/Applications/Xcode11.5.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
-isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk

That is, the two compiler paths should be extended with " -isysroot ${CMAKE_OSX_SYSROOT}"
or whatever is the syntax to catch the value of the CMAKE_OSX_SYSROOT variable.

The same 2 errors apply to the other fltk-config script CMake generates, BUILD/bin/fltk-config

@Albrecht-S
Copy link
Member Author

@ManoloFLTK Thanks for your report. I'll take a look into the "-lfontconfig" issue. If it's not present in the build process (is it?) then it shouldn't be in fltk-config.

Regarding CC and CXX: These variables return the compiler names and not flags to compile FLTK and/or applications, see fltk-config --help:

  [--cc]            return C compiler used to compile FLTK
  [--cxx]           return C++ compiler used to compile FLTK
  [--cflags]        return flags to compile C using FLTK
  [--cxxflags]      return flags to compile C++ using FLTK

I believe that additional flags like -isysroot belong in --cflags and/or --cxxflags (likely in both of them) so users can use them to compile their applications and fltk-config --compile would use them as well.

@Albrecht-S
Copy link
Member Author

@ManoloFLTK Can you edit your BUILD/fltk-config file as proposed by me and test how that works? If it works, can you post a diff here? I can then find a way to do it correctly with CMake.

@Albrecht-S
Copy link
Member Author

Regarding -lfontconfig: AFAICT it should only be in the LDLIBS variable if libfontconfig was found on the build system (no matter if it is really necessary). Can you answer these questions?

  • Why should -lfontconfig be removed?
  • What happens if it is present?

Thanks, and I apologize if these questions sound unnecessary. I'm trying to understand the issue.

@Albrecht-S Albrecht-S added the active Somebody is working on it label Aug 25, 2020
@ManoloFLTK
Copy link
Contributor

I attach a patch that fixes both problems, at least in my hands :
cmake.patch.txt

-lfontconfig must be removed because it means absolutey nothing under macOS.
If present, the link operation fails with this error:
ld: library not found for -lfontconfig

@ManoloFLTK
Copy link
Contributor

More details about fontconfig under macOS:
if the XQuartz optional software giving X11 support is installed on macOS, the cmake command
find_library (LIB_fontconfig fontconfig)
apparently finds the fontconfig library. But then the -lfontconfig option is added to the link command
and makes it fail.

I believe the solution is to run
find_library (LIB_fontconfig fontconfig)
only if OPTION_APPLE_X11 is ON.

@Albrecht-S
Copy link
Member Author

I believe the solution is to run find_library (LIB_fontconfig fontconfig) only if OPTION_APPLE_X11 is ON.

I'm not really happy with this solution (as shown in your patch) but I agree that we can live with it, at least for now, until we find a better solution. Please feel free to commit the fontconfig part of your patch, or let me know that I shall do it. I can do it later tonight or tomorrow.

@Albrecht-S
Copy link
Member Author

Albrecht-S commented Aug 25, 2020

I'm not yet convinced that the -isysroot part of your patch is the correct solution. I'm wondering how this all works with CMake on macOS. A quick search showed these two CMake documentation pages:

Both pages state that the behavior depends on environment variables, hence my next question: Are the mentioned environment variables SDKROOT and MACOSX_DEPLOYMENT_TARGET defined in your build environment, and if yes, what are their values?

The first page says:

CMake uses this value to compute the value of the -isysroot flag or equivalent and to help the find_* commands locate files in the SDK.

If this is true (and it's very likely), then the build should work w/o adding the -isysroot flag to the CMAKE_*_FLAGS variables and I believe we shouldn't do it. I need to investigate more, but I can't do that before tomorrow.

Both pages state:

The value of this variable should be set prior to the first project() or enable_language() command invocation because it may influence configuration of the toolchain and flags. It is intended to be set locally by the user creating a build tree.

IMHO the last sentence is questionable. Did you know that you should set any of these variables? Would other users know this? If they don't know, how can we do this in the FLTK CMake files? I don't know yet, this is something we need to find out.

@Albrecht-S
Copy link
Member Author

Note: although your patch works for you we need to separate two things:

  • the build itself which works and is influenced by the CMAKE_*_FLAGS and other variables (see above)
  • the generation of fltk-config which has some deficiencies

We should not mix both, i.e. we should not modify CMake variables that influence the build to make changes in the generation of fltk-config or we might later be hit by this. That's why I'm trying to find a better and more consistent way.

@ManoloFLTK
Copy link
Contributor

OK. I have committed the fontconfig part of my patch (at d944965).
The other part, which adds the -isysroot option to compiler and linker flags, is still to be committed.
Please, notice this option is also required at the link step.

More details about the -isysroot option:

A C/C++ compiler is installed on macOS (ignoring more exotic ways) in 2 ways, which can also co-exist:

  1. installing Xcode (notice several Xcode versions targetting various macOS versions can co-exist)
  2. installing the so-called "Command Line Tools".

With 1) compilers are called by a long path going inside the Xcode app, e.g.:
/Applications/Xcode11.5.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++

With 2) the compiler is called with unix-like syntax: g++, or c++

With 2) the -isysroot option is not required. In my understanding, that's because 2) targets only one
software development kit (SDK) version, that of the running OS.

With 1) the -isysroot option is required because within a given Xcode can sit several distinct SDK versions,
and that option identifies which version will be used by the compilation.

cmakes computes a default value of CMAKE_OSX_SYSROOT which is exactly what is needed for
the -isysroot option.

Environment variables SDKROOT and MACOSX_DEPLOYMENT_TARGET are not defined in my build environment.
Cmake finds itself the compiler and its default SDK version. With cmake-gui, it's possible to change
these to other values. cmake-gui also offers the opportunity to select what compiler is to be used.

I will try to put myself in a 2) configuration and seek what cmake does in terms of -isysroot option.

@ManoloFLTK
Copy link
Contributor

ManoloFLTK commented Aug 26, 2020

In situation 2) above, that is, using "Command Line Tools", the problems in fltk-config are the same.

Cmake sets this as compiler paths in BUILD/fltk-config :
CC="/Library/Developer/CommandLineTools/usr/bin/cc"
CXX="/Library/Developer/CommandLineTools/usr/bin/c++"

cmake-gui shows the computed value of CMAKE_OSX_SYSROOT :
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk

but that value is not used in a -isysroot option within BUILD/fltk-config,
so that "fltk-config --compile hello.cxx" fails because the -isysroot argument is missing.

In contrast, the configure-based building process sets this in file makeinclude:
CXX = g++
CC = gcc

No -isysroot option is used either, but when the compiler is called that way, it does not require
that option. That's confirmed modifying file BUILD/fltk-config (produced by cmake) by replacing
the long paths to compiler by short forms (CXX = g++). After that, "BUILD/fltk-config --compile hello.cxx"
runs correctly.

In summary, my understanding at that point is

  1. Cmake uses long paths to specify the compiler to be used such as
    CXX="/Library/Developer/CommandLineTools/usr/bin/c++"
    when using "Command Line Tools", and
    CXX="/Applications/Xcode11.5.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++"
    when using Xcode.
    In both cases, BUILD/fltk-config fails when run by lack of the -isysroot compilation (and link also) option.
    In both cases, cmake-gui shows that a value has been computed for variable CMAKE_OSX_SYSROOT
    and if that value is used to add a "-isysroot XXX" option to CFLAGS and CXXFLAGS in
    BUILD/fltk-config, then "fltk-config --compile hello.cxx" runs OK.

  2. configure-based build uses the short form to specify the compiler to be used (CXX = g++).
    With it, the "-isysroot XXX" option is not required, and fltk-config runs OK without it.

@Albrecht-S
Copy link
Member Author

Hi Manolo, thanks for your continuing tests and feedback. The information you provided is very helpful.

AFAICT CMake always stores the compiler names as full paths to make sure they don't change in different build executions if users change their PATH or something else. I wonder why you sometimes need the -isysroot switch and why you don't need it (apparently) with the short forms (gcc and g++) of the compiler commands. Are these different compiler instances than those found by CMake in the long forms? To find this out, can you please execute the following commands with gcc and g++, respectively and post results?

echo $PATH
which g++
type g++
file g++

If these commands don't reveal useful results, can you provide additional info? TIA

@ManoloFLTK
Copy link
Contributor

ManoloFLTK commented Aug 26, 2020

% echo $PATH
.:/Users/XXX/bin:/Library/Frameworks/Python.framework/Versions/3.7/bin:/opt/sw/lib/perl5/ExtUtils:/opt/sw/bin:/opt/sw/sbin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/texbin:/opt/X11/bin:/Library/Apple/usr/bin:/Users/XXX/bin:/usr/local/bin:/usr/local/git/bin:/Users/XXX/keep/wine_root/bin:/opt/local/bin:/Volumes/Mavericks_10_9/Android/sdk/cmake/bin

% which g++
/usr/bin/g++

% type g++
g++ is /usr/bin/g++

% file /usr/bin/g++
/usr/bin/g++: Mach-O 64-bit executable x86_64

Other information:
/usr/bin/g++ is a small file (31 KB)
/Library/Developer/CommandLineTools/usr/bin/c++ is a link to /Library/Developer/CommandLineTools/usr/bin/clang
which is much bigger (88 MB).

My understanding is that the 2 behaviours don't differ because of the compiler version but because
of the way they are run. Both ways ultimately call the same version of the same compiler
as suggested by these 2 queries:

% /usr/bin/g++ --version
Configured with: --prefix=/Library/Developer/CommandLineTools/usr --with-gxx-include-dir=/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/4.2.1
Apple clang version 11.0.3 (clang-1103.0.32.29)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

% /Library/Developer/CommandLineTools/usr/bin/c++ --version
Configured with: --prefix=/Library/Developer/CommandLineTools/usr --with-gxx-include-dir=/usr/include/c++/4.2.1
Apple clang version 11.0.3 (clang-1103.0.32.29)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

and as proven by these 3 compilation + link attempts:

Attempt 1 produces no error:
% /usr/bin/g++ -I/Volumes/Extra -I/Users/XXX/Documents/fltk/fltk-git -U__APPLE__ -D_LIBCPP_HAS_THREAD_API_PTHREAD -I/opt/X11/include -I/sw/include -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_THREAD_SAFE -D_REENTRANT -o 'hello' 'hello.cxx' -L/opt/X11/lib -L/opt/sw/lib /Volumes/Extra/lib/libfltk.a -lm -lX11 -lXext -lpthread -lXinerama -lXfixes -lXcursor -lpango-1.0 -lpangoxft-1.0 -lgobject-2.0 -lXft -lXrender -lm -lfontconfig

Attempt 2 fails :
% /Library/Developer/CommandLineTools/usr/bin/c++ -I/Volumes/Extra -I/Users/XXX/Documents/fltk/fltk-git -U__APPLE__ -D_LIBCPP_HAS_THREAD_API_PTHREAD -I/opt/X11/include -I/sw/include -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_THREAD_SAFE -D_REENTRANT -o 'hello' 'hello.cxx' -L/opt/X11/lib -L/opt/sw/lib /Volumes/Extra/lib/libfltk.a -lm -lX11 -lXext -lpthread -lXinerama -lXfixes -lXcursor -lpango-1.0 -lpangoxft-1.0 -lgobject-2.0 -lXft -lXrender -lm -lfontconfig
In file included from hello.cxx:17:
In file included from /Users/XXX/Documents/fltk/fltk-git/FL/Fl.H:26:
/Users/XXX/Documents/fltk/fltk-git/FL/platform_types.h:146:10: fatal error: 'sys/stat.h' file not
found
#include <sys/stat.h>
^~~~~~~~~~~~
1 error generated.

Attempt 3 runs OK
% /Library/Developer/CommandLineTools/usr/bin/g++ -I/Volumes/Extra -I/Users/XXX/Documents/fltk/fltk-git -U__APPLE__ -D_LIBCPP_HAS_THREAD_API_PTHREAD -I/opt/X11/include -I/sw/include -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -D_THREAD_SAFE -D_REENTRANT -o 'hello' 'hello.cxx' -L/opt/X11/lib -L/opt/sw/lib /Volumes/Extra/lib/libfltk.a -lm -lX11 -lXext -lpthread -lXinerama -lXfixes -lXcursor -lpango-1.0 -lpangoxft-1.0 -lgobject-2.0 -lXft -lXrender -lm -lfontconfig

These attempts differ by how the compiler is named and all lack the -isysroot XXX option.
If it's called g++ (as by configure) the -isysroot XXX option is not required
If it's called c++ (as by cmake) the -isysroot XXX option is required

@ManoloFLTK
Copy link
Contributor

Here is a new proposal to have Cmake build fltk-config under macOS
This should solve the question of -isysroot
and support OPTION_APPLE_X11 and OPTION_USE_PANGO :

cmake-patch2.txt

@Albrecht-S
Copy link
Member Author

Albrecht-S commented Aug 26, 2020

Thanks, patch looks basically good. Some comments and questions:

+  list (APPEND FLTK_CFLAGS " -isysroot ${CMAKE_OSX_SYSROOT}")
  • This is inside if (APPLE). Does it apply under all circumstances, no matter if OPTION_APPLE_X11 and/or OPTION_APPLE_SDL are set?
  • the leading space in " -isysroot" can be removed (AFAICT)
+    if (${CMAKE_SYSTEM_VERSION} VERSION_GREATER 16.9.0)
+      list (APPEND FLTK_CFLAGS "-D_LIBCPP_HAS_THREAD_API_PTHREAD")
+    endif (${CMAKE_SYSTEM_VERSION} VERSION_GREATER 16.9.0)

I don't understand version "16.9.0". CMAKE_SYSTEM_VERSION is the OS version. What macOS version is "16.9.0"?

-    list (APPEND FLTK_CFLAGS "-I${X11_INCLUDE_DIR}")
+    set(X11_INCLUDE_DIR_2 ${X11_INCLUDE_DIR})
+    list(TRANSFORM X11_INCLUDE_DIR_2 PREPEND "-I")
+    list (APPEND FLTK_CFLAGS "${X11_INCLUDE_DIR_2}")
  • is X11_INCLUDE_DIR really a list of directories (not a single dir)?
  • the local variable X11_INCLUDE_DIR_2 is a bad choice. Variables in the namespace X11_* should be reserved to the FindX11 module, I'd prefer another name.

BTW: indenting is not correct and space after set and list is missing.

+    if (APPLE)
+      get_filename_component(PANGO_L_PATH ${HAVE_LIB_PANGO} PATH)
+      set (LDFLAGS "${LDFLAGS} -L${PANGO_L_PATH}")
+    endif (APPLE)

Again, is this really always under condition if (APPLE) or ... ?

Other than that the handling of FLTK_CFLAGS and LDFLAGS seems to be "correct" now, AFAICT.

@ManoloFLTK
Copy link
Contributor

Thanks, patch looks basically good. Some comments and questions:

  • list (APPEND FLTK_CFLAGS " -isysroot ${CMAKE_OSX_SYSROOT}")

    This is inside if (APPLE). Does it apply under all circumstances, no matter if OPTION_APPLE_X11 and/or OPTION_APPLE_SDL are set?

Yes it does. I'm not sure if OPTION_APPLE_SDL is still alive. That's Mathias stuff related to the pico driver.

the leading space in " -isysroot" can be removed (AFAICT)

OK

  • if (${CMAKE_SYSTEM_VERSION} VERSION_GREATER 16.9.0)
  •  list (APPEND FLTK_CFLAGS "-D_LIBCPP_HAS_THREAD_API_PTHREAD")
    
  • endif (${CMAKE_SYSTEM_VERSION} VERSION_GREATER 16.9.0)

I don't understand version "16.9.0". CMAKE_SYSTEM_VERSION is the OS version. What macOS version is "16.9.0"?

Another numbering system is also present under macOS which is used in its Unix layer.
It appears for example when using "uname -a". It seems that Cmake wants that other numbering.
See columns "Version" and "Darwin version" there: https://en.wikipedia.org/wiki/MacOS#Release_history
Thus, Darwin version 16 corresponds to macOS 10.12.

  • list (APPEND FLTK_CFLAGS "-I${X11_INCLUDE_DIR}")
  • set(X11_INCLUDE_DIR_2 ${X11_INCLUDE_DIR})
  • list(TRANSFORM X11_INCLUDE_DIR_2 PREPEND "-I")
  • list (APPEND FLTK_CFLAGS "${X11_INCLUDE_DIR_2}")
is X11_INCLUDE_DIR really a list of directories (not a single dir)?

At least, it can be so in some cases, because it is in my case.

the local variable X11_INCLUDE_DIR_2 is a bad choice. Variables in the namespace X11_* should be reserved to the FindX11 module, I'd prefer another name.

OK. I'll change that.

BTW: indenting is not correct and space after set and list is missing.

OK.

  • if (APPLE)
  •  get_filename_component(PANGO_L_PATH ${HAVE_LIB_PANGO} PATH)
    
  •  set (LDFLAGS "${LDFLAGS} -L${PANGO_L_PATH}")
    
  • endif (APPLE)

Again, is this really always under condition if (APPLE) or ... ?

This if (APPLE) is itself in if (X11_Xft_FOUND AND OPTION_USE_PANGO)
and in if (PANGOXFT_FOUND).

@ManoloFLTK
Copy link
Contributor

That new patch incorporates all changes mentionned above:
cmake-patch3.txt

@ManoloFLTK
Copy link
Contributor

That is unrelated to fltk-config, but to cmake under macOS:

When OPTION_APPLE_X11 is ON , BUILD/config.h contains
#define HAVE_GL_GLU_H 0
and therefore GLU-using test programs (glpuzzle, fractals) fail.

The cause is that statement at line #196 of CMake/options.cmake
find_file (HAVE_GL_GLU_H GL/glu.h PATHS /opt/X11/include)
fails to assign a value to variable HAVE_GL_GLU_H.
I can't understand why. File /opt/X11/include/GL/glu.h is present and readable.
find_file () is successfully used in several other places.

Any hint why that is so?

@Albrecht-S
Copy link
Member Author

@ManoloFLTK Patch cmake-patch3.txt looks good, I can't see any issues, but as you know I can't test it. If it works for you I'd suggest to commit it. Thanks for your help.

Regarding HAVE_GL_GLU_H: I see two instances of searches for GL/glu.h:

CMake/options.cmake:190:    find_file (HAVE_GL_GLU_H GL/glu.h PATHS /opt/X11/include)
CMake/resources.cmake:42:fl_find_header (HAVE_GL_GLU_H GL/glu.h)

This is questionable and should probably be fixed. I suggest to add a debug statement after both of these statements like: fl_debug_var (HAVE_GL_GLU_H) which prints the variable with its contents.

I'd prefer fl_find_header() for header file searches because that had better results in the past. fl_find_header() can take a list of header files as argument where order matters and the former headers are required to successfully find and use (compile) the header, hence this test is usually better because it performs a compile test (IIRC).

I don't know why we have these two searches in different files, but some parts of this code are old and need refactoring anyway. HTH and good luck!

@Albrecht-S
Copy link
Member Author

PS: only an educated guess, but hopefully helpful:

ISTR that you can't use the same CMake variable twice in searches because CMake caches the result and does not execute the second search if the variable is already cached. Note that resources.cmake is executed first, hence it can be that the 2nd search is not executed at all. The search in resources.cmake may return false because it doesn't search in the path specified in the search in options.cmake. Maybe.

Can you comment out the search in resources.cmake and see if this works?

@ManoloFLTK
Copy link
Contributor

cmake-patch3.txt is now committed (1ace96e).

@ManoloFLTK
Copy link
Contributor

ISTR that you can't use the same CMake variable twice in searches because CMake caches the result and does not execute the second search if the variable is already cached.

Beautiful: that was it!

@ManoloFLTK
Copy link
Contributor

The fix for cmake to find GLU when present under macOS + OPTION_APPLE_X11 is committed.

@ManoloFLTK
Copy link
Contributor

We've seen that HAVE_GL_GLU_H is processed 1st in resources.cmake and 2nd in options.cmake

What about doing it only once in options.cmake, as in this small patch ?
cmake-glu-patch.txt

@Albrecht-S
Copy link
Member Author

Albrecht-S commented Aug 27, 2020

What about doing it only once in options.cmake, ...

  1. I'm always for simplification, and having two searches for the same file is always bad.
  2. My plan is to refactor (big) parts of the CMake files anyway.
  3. I don't know why PATHS /opt/X11/include is included in the search in options.cmake.

Regarding (3): where is GL/glu.h located on macOS systems? If that additional PATHS argument is not necessary (as your patch suggests), then the patch would IMHO work. It seems to be fine to search for GL/glu.h only if OPTION_USE_GL is ON, but why did the old code not work which should also have found GL/glu.h in resources.cmake (without the PATHS argument as you propose now). That's strange.

That said, does your patch work on macOS with OPTION_APPLE_X11=ON?

@ManoloFLTK
Copy link
Contributor

ManoloFLTK commented Aug 27, 2020

For macOS + OPTION_APPLE_X11:
The file to be found is /opt/X11/include/GL/glu.h.
The point is that /opt/X11/include is put in the include path by the findX11 macro.
That's why GL/glu.h is not found in resources.cmake
because findX11 has not run yet. That's also why in the patched form, "/opt/X11/include"
disappears, because findX11 adds it to where fl_find_header searches.

For plain macOS, file glu.h is located in another, macOS-style, place that macro FindOpenGL discovers
and assigns to HAVE_OPENGL_GLU_H which is then copied to HAVE_GL_GLU_H.

The patch works both with and without OPTION_APPLE_X11.

@Albrecht-S
Copy link
Member Author

The point is that /opt/X11/include is put in the include path by the findX11 macro.
That's why GL/glu.h is not found in resources.cmake because findX11 has not run yet.

Ooh, that's something I didn't know. Thanks for this info.

The patch works both with and without OPTION_APPLE_X11.

Then I'd say, go for it.

@ManoloFLTK
Copy link
Contributor

The issue with GLU apps under macOS + OPTION_APPLE_X11 should be fixed with commit
95799bd.

@Albrecht-S
Copy link
Member Author

Hmm, just for curiosity and because I'd like to understand: can you explain why you chose another solution than in cmake-glu-patch.txt suggested before?

@ManoloFLTK
Copy link
Contributor

Because I tried again cmake-glu-patch.txt before committing it, and found it didn't work.
The proposal in cmake-glu-patch.txt was to search for GL/glu.h with
fl_find_header (HAVE_GL_GLU_H GL/glu.h)
after FindX11. This was not correct because FindX11 computes X11_INCLUDE_DIR but fl_find_header
doesn't search in X11_INCLUDE_DIR.

The new commit uses unset(HAVE_GL_GLU_H CACHE) to forget that HAVE_GL_GLU_H had
been computed before in resources.cmake, and does (at line #199 of options.cmake)
find_file (HAVE_GL_GLU_H GL/glu.h PATHS ${X11_INCLUDE_DIR})
which uses X11_INCLUDE_DIR, computed earlier by FindX11, to newly search for GL/glu.h,
and that second search is successful.
Both the new commit and cmake-glu-patch.txt use the key fact that the findX11 macro
detects where GL/glu.h can be found. Both rely on FindX11 to decide what directory that is,
and prevent from writing a directory name explicitly in the cmake file.

@Albrecht-S
Copy link
Member Author

OK, thanks. Understood.

Note to myself: we need an additional PATHS attribute/argument in fl_find_header() or a similar solution so we can use this function to find header files even in extra search paths like ${X11_INCLUDE_DIR}.

@Albrecht-S
Copy link
Member Author

Another note regarding fltk-config deficiencies: currently (c74a482) fltk-config doesn't seem to honor building the bundled image libs and produces wrong --ldflags and --ldstaticflags output.

See this comment: #425 (comment).

@Albrecht-S
Copy link
Member Author

Reminder to myself: see also issue #639.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
active Somebody is working on it bug Something isn't working CMake CMake bug or needs CMake support
Projects
None yet
Development

No branches or pull requests

2 participants