-
Notifications
You must be signed in to change notification settings - Fork 262
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
CMake's INCLUDES adds CMAKE_PREFIX_PATH before staging area leading to problems between .H and .cxx files on API changes #982
Comments
I looked into the FLTK file structure a bit, and I see the problem. All of FLTK's built-in dependencies (png, zlib, jpeg, libdecor and nanosvg) are in the same directory as FL directory includes. |
An alternative solution that might be easier to implement might be to remove the FL directory from CMAKE_INSTALL_PREFIX/include on an install target. |
@ggarra13 Hi Gonzalo, thanks for the report. I can see the problem but I can't imagine a proper solution in FLTK's build system. The basic problem is that you set This is not an FLTK problem but an issue of your build procedure. The same problem you are describing would happen if any other dependency of your project would be changed in an incompatible way - and would "see" its own header files of the previous build. This depends on how you set up all dependencies in your build. If you want to avoid cleaning the build directory before new builds of your project's dependencies, then the solution I'm thinking of is:
This way each dependency wouldn't "know" of its own install path and thus wouldn't include outdated headers of a previous build. You could also easily delete the build folders of any dependency it you change the GIT tag of that dependency. A tiny change in your build setup made it "work" for me. This is only a proof of concept, not the final solution. The only change in my proof of concept is to set diff --git a/SuperBuild/cmake/Build/BuildFLTK.cmake b/SuperBuild/cmake/Build/BuildFLTK.cmake
index f4ec429..46449c6 100755
--- a/SuperBuild/cmake/Build/BuildFLTK.cmake
+++ b/SuperBuild/cmake/Build/BuildFLTK.cmake
@@ -50,7 +50,7 @@ ExternalProject_Add(
-DCMAKE_MODULE_PATH=${CMAKE_MODULE_PATH}
-DCMAKE_BUILD_TYPE=${FLTK_BUILD_TYPE}
-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
- -DCMAKE_PREFIX_PATH=${CMAKE_PREFIX_PATH}
+ -DCMAKE_PREFIX_PATH=""
-DCMAKE_C_FLAGS=${FLTK_C_FLAGS}
-DCMAKE_CXX_FLAGS=${FLTK_CXX_FLAGS}
-DCMAKE_INSTALL_MESSAGE=${CMAKE_INSTALL_MESSAGE} The only other solution I can imagine is to clean the build if you change any of the dependencies, i.e. if you update one of the GIT tags. This should be rare while you develop your own application. Please let me know if I missed anything or if anything is unclear -- and please close the issue if you agree with my suggestion. |
@Albrecht-S Hi, Albrecht. Thanks for taking the time with this problem, which is difficult to solve.
I disagree. AFAICT, it happens only with FLTK, as it reads dependencies and include files all from the root directory, which is the issue.. Basically, what I imagine FLTK's CMAKE_INCLUDE_PATH or whatever is called (I can't remember how it is named in cmake now) should be: ${FLTK_SOURCE_DIRECTORY_where_FL_and_src_are} {chosen_built_in_dependencies} ${CMAKE_PREFIX_PATH} So, for example, if FLTK is installed in libs/FLTK (I omit the full path to it), with built-in png, zlib and libdecor the include path should be: libs/FLTK libs/FLTK/deps/png libs/FLTK/deps/zlib libs/FLTK/deps/libdecor ${CMAKE_PREFIX_PATH} and lets say CMAKE_PREFIX_PATH contains the system dependency for libjpeg, which is not listed as one of the deps/, but would get picked last. The dependencies would be moved to two additional directories (so they can be chosen individually based on system/built-in setting), like: With that approach, you would not have any issues with header files getting out of sync. It would be, indeed, a major restructuring of the current distribution of directories in FLTK, so it would need some heavy testing to make sure nothing breaks. |
This comment is off-topic, please read but don't reply to these statements. @ggarra13 Gonzalo, I agree that there are issues with the bundled libraries and you made some good points in your latest comment. It is however even more complicated than you described, and I won't change this directory structure in FLTK 1.4.x, i.e. as long as we have to maintain both Please understand that I can't spend time on this "bundled library problem" now and risk the long overdue release of FLTK 1.4.0. I'm really aware of this "bundled library problem" but if this concerns you in any way, please start a discussion thread in fltk.coredev where I may spend some time to explain the issue in more detail. But don't expect big changes regarding this in FLTK 1.4.0. Please read further in my follow-up comment... |
Back on-topic. Gonzalo, please let's focus on the topic of your report:
This happens in your build with or w/o bundled libraries, I believe you could even remove the external ZLIB project and it would still happen. The FLTK build in the second run - after changing the Git commit hash of the FLTK library - fails because the compiler "sees" the old FLTK header files and misses the new method added in the updated Git commit hash. I hope we can both agree with this analysis so far. Where we don't agree yet is the reason why this happens. You wrote, among others:
At this point I don't know exactly what happens in your build process, but what I definitely know is that our FLTK (CMake) build files don't use OTOH your build process sets Let's take a look at the CMake docs on CMAKE_PREFIX_PATH:
"It is to be set by the project": what does this mean? Basically I interpret this that "the project" (i.e. in this case the FLTK project) is intended to set this path that affects searches. I do also believe that "the user building the project" can legitimately set it to affect the build of "the project (FLTK)" like you do, for instance to find a particular ZLIB installation or anything else. However, I really don't know what happens if you set At this point I'm stuck. I can't test and investigate further because I need more time for other FLTK issues to be able to release 1.4.0. I'll come back to this later - unless you decide to close this issue yourself. |
Describe the bug
When using system libs, FLTK's CMake scripts are adding CMAKE_PREFIX_PATH/include first before its staging area.
This is a problem as it leads to subsequent compilations to fail when there's a discrepancy in the API.
To Reproduce
fltk_cmake_build_bug.zip
Just run:
./runme.sh
The first compilation will work, the second run (where there was an API changing) without cleaning the directory will fail.
For more info, please refer to the included README.txt file.
Expected behavior
Compilation should not fail. Cmake is smart enough to re-compile only what's needed. The CMAKE_PREFIX_PATH should be added later or the FLTK's FL includes first. There should not be any need to clean the directory.
Screenshots
N/A.
FLTK Version
1.4.0 tags in runme.sh script
FLTK Configure / Build Options
See runme.sh script
Operating System / Platform:
All OSes.
The text was updated successfully, but these errors were encountered: