[cmake-developers] Correct way to get include directories for a target (automoc regression) ?

[Alexander Neundorf]

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