Alexander Neundorf wrote: > On Sunday 11 November 2012, Stephen Kelly wrote: >> Alexander Neundorf wrote: >> > In 2.8.9 automoc did not ask the target for its include dirs, but it >> > asked the directory for its include dirs. This contained and still >> > contains as it seems the CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES. >> > In 2.8.10 the target is asked for its include dirs, and it seems there >> > the implicit include dirs have been stripped. >> >> It seems that the bug is in KDE/Phonon. Why do they populate the >> CMAKE_CXX_IMPLICIT_INCLUDE_DIRECTORIES instead of the INCLUDE_DIRECTORIES >> directory property? >> >> The patch that you added looks like a hack to me which is only for the >> benefit of what KDE/Phonon is doing (but maybe shouldn't be). I'd prefer >> to fix KDE/Phonon instead. > > It still is a backward compatibility break in cmake, for kdelibs, which is > a big user of cmake.
Are you sure? This only affects the CMAKE_AUTOMOC feature, which kdelibs doesn't use. This seems to affect only phonon because they have started to use CMAKE_AUTOMOC instead of automoc4, and because they have a copy of a kdelibs macro. kdelibs should not be affected. KDE Frameworks 5 won't use this trick to populate the implicit include directories, so it is only phonon that is affected, as far as I can tell. Already this feature doesn't work with CMake 2.8.10 and phonon releases since they started to use CMAKE_AUTOMOC. If we patch kdelibs and phonon now to populate INCLUDE_DIRECTORIES instead of the internal system include directories (or some other appropriate fix), then the next phonon release will work with CMake 2.8.10 (right?) instead of having to wait for CMake 2.8.11. It might be appropriate to patch both cmake and kdelibs/phonon so that implicit directories are not appended to anymore. Do you know any reason not to do that? In fact, it was suggested to remove the macro in question earlier this year: http://thread.gmane.org/gmane.comp.kde.devel.buildsystem/5581/focus=7096 when an older thread from 2010 was revived. In that thread, appending to the implicit include dirs was selected as the solution instead of seeing if CMake could be patched. Two years later we hit issues resulting from that choice. For me, that's an argument for finding out what the right fix in the right place is. The reported bugs are side-effects of populating the implicit include directories as was done in 2010. As Raphael notes in the post I linked to, the fix might be simply relying on the user to set up their environment, because we can't re-order include directories for every /opt directory that someone might install to, while they have system includes somewhere else. Your patch in FixAutomocRegression might result in inflexibility or bugs in two years, so I think caution is in order. It might be the only way to re- add 'backward compatibility' while not patching phonon at all (assuming that is the goal), I don't know. I think it's worth considering Raphaels post above, particularly: 'if, say, X11 and Soprano are installed into /opt/custom-prefix, the original problem will show up again' Is making /usr/local an implicit include directory on freebsd the right fix? > > Does moc know actually know about system include dirs ? I don't know. > If it doesn't, this is not a hack, but potentially necessary. I think that depends on how the users environment is set up. Here's what I understand about how the bug occurs: http://cmake.org/gitweb?p=stage/cmake.git;a=commitdiff;h=a9465098c01ace9ead62c04674dc2f4a529962f5 says that /usr/lib/qt/include is usually in QT_INCLUDES, but not passed to moc. The reason /usr/lib/qt/include is not passed to moc is that it is inserted by phonon into the implicit include directories for the language, and therefore filtered out by cmLocalGenerator::AddIncludeDirectories. The reason /usr/lib/qt/include is inserted by phonon into the implicit include directories is that querying g++ returns that path. Is it normal for g++ to return that (Qt specific) path as a system include, or is that resulting from a patch the distros make to g++ based on where they install Qt? The phonon and tomahawk related bug reports you linked to seem to be from people who are using a distro-provided Qt, and FindQt4 finds it appropriately, but phonon causes CMake to remove the directory from what it passes to moc. The bug is the result of the combination of 1) populating the implicit include dirs externally and 2) using CMAKE_AUTOMOC. This to me looks like it's not appropriate for KDE/phonon to populate the implicit include directories. Whatever patch goes into CMake (to handle already-released versions of phonon and others that have copied from FindKDE4Internal), that should probably not be done anymore by kdelibs/phonon. Removing that macro from phonon would fix the reported issues too, I think, without requiring the users of (trunk versions of) phonon to wait for CMake 2.8.11. Can your patch be wrapped in some condition to check if FindPhononInternal is being used? I've seen something similar to check VTK versions, so it's not un-heard-of. Thanks, Steve. -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
