On Sunday 11 November 2012, Stephen Kelly wrote: > 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?
I don't know, maybe. > > Does moc know actually know about system include dirs ? > > I don't know. Interestingly, moc seems to fail on some systems if all system include dirs are given to it, see the Raphaels comment ( http://public.kitware.com/Bug/view.php?id=13667#c31553) > > 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=a9465098c01ace9ea > d62c04674dc2f4a529962f5 > > 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. Yes. > 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? At least it's also the case on my Slackware 13.37, and I think I didn't manually change this. ...this could have the interesting effect that I also get /usr/lib/qt/include when using some other Qt installation :-/ > 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. Yes. > 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. I have put a FixAutomocRegression2 branch now on stage, which is less intrusive than the first fix. It now checks whether ${QT_QTCORE_INCLUDE_DIR} is being given to moc as an include dir, and if this is the case, but ${QT_INCLUDE_DIR} is defined and not given to moc, ${QT_INCLUDE_DIR} is added. I think for Qt4 this should be safe. The QtCore include dir should always be used when using anything from Qt4, right ? It should also always be a subdir of ${QT_INCLUDE_DIR} and so usually not be part of the system-default include dirs. I don't know about Qt5, but at least for Qt5 nothing can't be broken (since it is not yet released), right ? Alex -- 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
