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

[Stephen Kelly]

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.