[cmake-developers] cmake automoc breaks kde

[Alexander Neundorf]

On Tuesday 22 November 2011, Stephen Kelly wrote:
> 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.

I think I merged it into next in the meantime.

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

I haven't actually tried, I was relying on the frameworks developers (e.g. 
you) for that :-)
In CMake/Tests/QtAutomoc/ there are several cases tested, actually I think all 
I could think of.

Using Qt5 or Qt4 ?

> 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.

Yes.

> * 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.

I thought about this. There are 4 constellations, and only one constellation 
where things can break:

* foo.cpp includes neither foo.moc nor moc_foo.cpp. No problem.

* if foo.cpp includes both foo.moc and moc_foo.cpp, then both foo.h and 
foo.cpp will be moc'ed, and it doesn't matter which one is for which. No 
problem.
It seems there is a test missing for this case.

* if foo.cpp includes only foo.moc, then foo.cpp will be moc'ed. This may be 
unnecessary, but shouldn't create problems. If foo.h also needs to be moc'ed, 
it will be, and the resulting moc_foo.cpp will be included in the 
target_automoc.cpp file. So it should also work.
This is Tests/QtAutomoc/codeeditor.cpp

* problematic case: if foo.cpp includes only moc_foo.cpp, then cmake will 
check whether foo.cpp contains "Q_OBJECT". If it does, it will:
** fail with Qt5, stating that foo.cpp contains Q_OBJECT but does not include 
foo.moc, so it can't work
** warn with Qt4 that it will run moc on foo.cpp instead of foo.h to generate 
moc_foo.cpp
This is Tests/QtAutomoc/blub.cpp
 

> * 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.

Yes.

> * 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.

Yes.
I did not expect somebody would be doing this.
I don't want to support this for Qt5.
I can add a fix with a warning for Qt4.

> * The code is already getting quite complex trying to guess all kinds of
> things despite inconsistency.

Yes.
 
> I think we need to have a list of exactly what cases should be
> supported, 

This is the current documentation of automoc in cmake. I was hoping it would 
be relatively clear:

"If an #include statement like #include \"moc_foo.cpp\" is found, "
"the Q_OBJECT class declaration is expected in the header, and moc is "
"run on the header file. "
"If an #include statement like #include \"foo.moc\" is found, "
"then a Q_OBJECT is expected in the current source file and moc "
"is run on the file itself. "
"Additionally, all header files are parsed for Q_OBJECT macros, "
"and if found, moc is also executed on those files. The resulting "
"moc files, which are not included as shown above in any of the source "
"files are included in a generated <targetname>_automoc.cpp file, "
"which is compiled as part of the target."

This should be compatible to qmake, and this is what I'd like to support for 
Qt5.

> 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.

Since it has been released with cmake 2.8.6, it can only be turned off by 
default using a cmake policy. I'd like to avoid that, and instead support it 
and issue a warning (for Qt4).

Alex