[cmake-developers] cmake automoc breaks kde

[Stephen Kelly]

On 11/10/2011 10:16 PM, Alexander Neundorf wrote:
> On Wednesday 02 November 2011, David Faure wrote:
>> On Monday 31 October 2011 21:47:31 Alexander Neundorf wrote:
>>> On Monday 31 October 2011, David Faure wrote:
>>>> This is a typical (kde) case where the .cpp incudes the .moc, for the
>>>> object defined in the .h.
>>> Shouldn't it include moc_kauthactionwatcher.cpp ?
>> No, that's for the case where the qobject is in the .cpp file.
>>
>>> Is this really a typical case, i.e. is that done in many places ?
>> Yes. Just grep for moc includes in kde...
>>
>>> I thought the rule is that if there is a include foo.moc, the Q_OBJECT is
>>> in the same cpp file.
>> No, it's the other way around, in KDE.
>>
>> $ grep Q_OBJECT kautosavefile.*
>> kautosavefile.h:    Q_OBJECT
>> $ grep moc kautosavefile.cpp
>> #include "kautosavefile.moc"
>>
>>> If it did additionally other things, this was more or less accidentially.
>> But it was the way kdesupport/automoc always worked, on purpose, not
>> accidentally.
>>
>> OK, more precisely: it didn't care what the name of the included moc file
>> was, what mattered was, where is Q_OBJECT being used.
>>
>> As the kde4automoc.cpp code says: when parsing the .cpp file:
>>
>> // If the moc include is of the moc_foo.cpp style we expect the Q_OBJECT
>> class // declaration in a header file.
>> // If the moc include is of the foo.moc style we need to look for a
>> Q_OBJECT // macro in the current source file, if it contains the macro we
>> generate the // moc file from the source file, else from the header.
>>
>>> The logic which is currently implemented is:
>>>
>>> 1. if foo.cpp includes foo.moc, run moc on foo.cpp and generate foo.moc
>> That should depend on if foo.cpp actually says Q_OBJECT or not. Otherwise
>> this .moc should be created from the header file. Not much point in running
>> moc over a .cpp that doesn't define a Q_OBJECT :)
>>
>>> 2. run moc on all header files bar.h which contain "Q_OBJECT" and
>>> generate moc_bar.cpp from them
>> That could be moc_bar.cpp or bar.moc depending on which one is included.
>>
>>> (which in detail means:
>>> 2.1 for every included moc_xyz.cpp (no matter in which file), search for
>>> a corresponsing xyz.h/hxx/hpp file and run moc on it
>>>
>>> 2.2 for every bar.cpp file, check whether there is a corresponding
>>> bar.h/hpp/hxx file and collect it
>>>
>>> 2.3 check for a Q_OBJECT macro in all collected header files and all
>>> header files listed explicitely as a source for the target, and run moc
>>> on them. If the resulting moc_xyz.cpp file has not been included in any
>>> source file, include it in<targetname>_automoc.cpp, which is built as
>>> part of the target )
>>>
>>> Step 2.2 already involves guessing, which I don't like.
>>> Beside that, IMO these are clear rules, which are easy to understand.
>>>
>>> With the old behaviour it was actually ambigous:
>>>
>>> foo.h:
>>> class Foo
>>> {
>>>
>>>    Q_OBJECT
>>>
>>> };
>>>
>>>
>>> foo.cpp:
>>>
>>> Foo::Foo() {}
>>>
>>> #include "foo.moc"
>>> #include "moc_foo.cpp"
>>>
>>> This would have generated twice the same moc file, I think. IMO this is
>>> really confusing.
>> Well there is no reason to include both, unless you have Q_OBJECT in the
>> .cpp file too :-)
>>
>>> Now it is simple: foo.moc from foo.cpp, moc_foo.cpp from foo.h.
>>>
>>> If this is really done in many places in KDE, I'll add some workaround,
>>> but I think the default behaviour should stay as it is now in cmake git.
>>>
>>> How does qmake handle such cases ?
>> Checking....
>> Indeed qmake expects moc_foo.cpp for the standard case (Q_OBJECT in
>> header). This is why kde4automoc was made to support both moc filenames,
>> so that it could be compatible with the kde way of doing it (foo.moc) and
>> with the qmake way of doing it (moc_foo.cpp).
>>
>> Oh well, if you want to stick to that we'll change all of KDE, for the
>> benefit of a clearer future indeed... At least it won't be an source
>> incompatible change for app developers since they will have to enable
>> CMAKE_AUTOMOC to get into this situation. Merely upgrading cmake or KDE
>> (even to frameworks 5) won't trigger it. OK not exactly,
>> find_package(kde4) will stop working so they will -have- to port to
>> CMAKE_AUTOMOC, it will be the only solution available...
> Please give the RestoreAutmocKDECompatibility branch on cmake stage a try.
> It should work again, but print a warning if a file includes a moc_foo.cpp,
> but no foo.moc, and contains a Q_OBJECT macro.
>
> For Qt5 I'd prefer to not support that anymore.
> I.e. moc_foo.cpp ->  header, foo.moc ->  source file.
>
> Alex

I tried that branch. It doesn't build the frameworks branch for me. Does 
it work for you?

There seems to be several problems:

* KDE does include "foo.moc" if it wants the header file to be moc'd. 
This is 'incorrect' compared to what qmake expects, which would be a 
"moc_foo.cpp" include.
* Some places in KDE use include "moc_foo.cpp" 'correctly', but there 
are only about 10 occurrences. The current automoc tool seems to handle 
that, as does cmake 2.8.6 apparently.
* KDE does include "foo_test.moc" in foo_test.cpp to moc the test object 
in foo_test.cpp. This is 'correct' compared to what qmake does.
* KDE does eg, include "kjob_p.moc" in kjob.cpp to cause kjob_p.h to be 
moc'd. The RestoreAutmocKDECompatibility branch introduces a commit 
which makes this not work because the source file has a different basename.
* The code is already getting quite complex trying to guess all kinds of 
things despite inconsistency.

I think we need to have a list of exactly what cases should be 
supported, and fix KDE to do the right thing. I think in many cases, 
fixing KDE will just be removing the explicit include "foo.moc" which is 
intended to run moc on foo.h, and let the automatic moc'ing do the work.

If any other 'KDE compatilbility' features are in cmQtAutomoc which 
differ from qmake behavior, I really think it should be behind an option 
which is off by default, which KDE turns on, and then actively ports 
away from. Currently the 'KDE compatilbility' stuff is always on, which 
is not an incentive to do things right or consistently.

Thanks,

Steve.