[cmake-developers] cmake automoc breaks kde

[Alexander Neundorf]

On Tuesday 22 November 2011, Stephen Kelly wrote:
> On 11/22/2011 06:20 PM, Alexander Neundorf wrote:
> > Using Qt5 or Qt4 ?
> 
> 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.
> 
> Sort of. There are cases where the moc file must be included in the cpp
> file (see below).
> 
> But this removal of includes is what we should aim for in KDE. I think
> if the options are:
> 
> 1) Introduce odd hacky KDE specific workarounds to CMake
> 2) Remove the problematic includes from KDE files in the frameworks branch
> 
> I will side with option 2). CMAKE_AUTOMOC is not used in KDE4. The
> feature is only really relevant to non KDE Qt4 users. Let's just 'fix
> the bug' in the frameworks iteration of kdelibs.
> 
> > * 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.
> 
> Yes.
> 
> > * 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
> 
> Sort of. There are cases where the moc file must be included in the cpp
> file, because it needs definitions from the .h file. (eg, use of the
> Q_PRIVATE_SLOT macro).

I don't know that one.
Are there docs somewhere ?
I have also seen a I think Q_GADGET macro mentioned somewhere, but also didn't 
find docs.
If they exist and require special handling, adding this special handling is 
fine IMO.

> If a KDE developer included foo.moc expecting that to be the result of
> moc'ing the .h file, the build will fail.
> 
> In this case, foo.cpp needs to be fixed to include "moc_foo.cpp"
> instead. I don't think CMake should introduce a hack for this.
>
> > * 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
> 
> I think this is fine.
> 
> > ** 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.
> 
> This is not uncommon in both KDE and in Qt itself, or any other project
> where it makes sense to put a QObject-inherited class in the _p.h as an
> internally used class. See for example, qplaintextedit.cpp:
> 
> #include "moc_qplaintextedit.cpp"
> #include "moc_qplaintextedit_p.cpp"
> 
> This should be supported by the automoc feature.

Yes, it should work.
I'm adding a test.

> I have also seen places (I think in QtDeclarative) where a foo.cpp file
> included the file moc_bar.cpp, though that is very uncommon. The logic
> that I would use would be something like:
> 
> find #include "moc_(.*).cpp" -> generate moc for "\\1.h"

This should work right now.
I thought there is a test for this. Checking.

> find #include "(.*).moc" -> generate moc for "\\1.cpp"
> Was there a reason for preventing this?
 
We can do that, but how can that work ?
I mean, this will generate code for a class which is declated in a different 
source file, so it is unknown in the including file, no ?

> >> 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.
> 
> Nearly. As you can see, there are quite some differences compared to the
> qmake behavior.
> 
> >> 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).
> 
> Currently the warning is also issued for Qt5, and the containsQ_OBJECT
> is called for Qt5 too. I would change that to also not search for
> Q_OBJECT or the warning for Qt5.

I think the error (it errors out, it's not a warning) is a good thing, 
otherwise it will break during compiling with a less descriptive error 
message.

Alex