Refactoring CMake support

Tue Aug 27 15:37:36 UTC 2013

On Tue, Aug 27, 2013 at 10:48 AM, Milian Wolff <mail at milianw.de> wrote:

> On Tuesday 27 August 2013 02:10:36 Aleix Pol wrote:
> > I haven't had problems with it so far, I'll merge it later this week. I'm
> > not doing it yet because I just merged the noAreaTabs branch and I'd
> prefer
> > to break systems one at a time.
> >
> > In any case, please test :)
>
> I've successfully loaded my big kdev* session with 10 cmake projects in
> it. So
> it works - cool!
>
> Switching branches in kdevplatform from master to 1.4 triggers a crash
> though:
> Can you reproduce this?
>
> Thread 1 (Thread 0x7fc905a927c0 (LWP 17518)):
> [KCrash Handler]
> #5  0x00007fc86938b978 in CMakeManager::isReloading (this=0x30131b0,
> p=0x0) at
>
> /ssd/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:649
> #6  0x00007fc86938be10 in CMakeManager::realDirectoryChanged
> (this=0x30131b0,
> dir=...) at
>
> /ssd/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:685
> #7  0x00007fc86938bcf7 in CMakeManager::filesystemBuffererTimeout
> (this=0x30131b0) at
>
> /ssd/milian/projects/kde4/kdevelop/projectmanagers/cmake/cmakemanager.cpp:676
> #8  0x00007fc86937b83c in CMakeManager::qt_static_metacall (_o=0x30131b0,
> _c=QMetaObject::InvokeMetaMethod, _id=6, _a=0x7fffad95a410) at
>
> /ssd/milian/projects/.build/kde4/kdevelop/projectmanagers/cmake/moc_cmakemanager.cpp:71
> #9  0x00007fc916068ba8 in QMetaObject::activate(QObject*, QMetaObject
> const*,
> int, void**) () from /usr/lib/libQtCore.so.4
> #10 0x00007fc91606cd71 in QObject::event(QEvent*) () from
> /usr/lib/libQtCore.so.4
> #11 0x00007fc9151db12c in QApplicationPrivate::notify_helper(QObject*,
> QEvent*) () from /usr/lib/libQtGui.so.4
> #12 0x00007fc9151e16f0 in QApplication::notify(QObject*, QEvent*) () from
> /usr/lib/libQtGui.so.4
> #13 0x00007fc91687472a in KApplication::notify(QObject*, QEvent*) () from
> /usr/lib/libkdeui.so.5
> #14 0x00007fc916054efd in QCoreApplication::notifyInternal(QObject*,
> QEvent*)
> () from /usr/lib/libQtCore.so.4
> #15 0x00007fc916084633 in ?? () from /usr/lib/libQtCore.so.4
> #16 0x00007fc916081989 in ?? () from /usr/lib/libQtCore.so.4
> #17 0x00007fc90f11dd96 in g_main_context_dispatch () from
> /usr/lib/libglib-2.0.so.0
> #18 0x00007fc90f11e0e8 in ?? () from /usr/lib/libglib-2.0.so.0
> #19 0x00007fc90f11e18c in g_main_context_iteration () from
> /usr/lib/libglib-2.0.so.0
> #20 0x00007fc916081b85 in
> QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>)
> ()
> from /usr/lib/libQtCore.so.4
> #21 0x00007fc9152782e6 in ?? () from /usr/lib/libQtGui.so.4
> #22 0x00007fc916053b5f in
> QEventLoop::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () from
> /usr/lib/libQtCore.so.4
> #23 0x00007fc916053e55 in
> QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () from
> /usr/lib/libQtCore.so.4
> #24 0x00007fc916058f8b in QCoreApplication::exec() () from
> /usr/lib/libQtCore.so.4
> #25 0x00000000004127db in main (argc=3, argv=0x7fffad95c998) at
> /ssd/milian/projects/kde4/kdevelop/app/main.cpp:561
>

I cannot reproduce, but I'd say this should be fixed now anyway, the fix
was quite obvious.

>
> Furthermore looking at the code I have some things to note:
>
> a) Don't use QList in new code.
> http://marcmutz.wordpress.com/effective-qt/containers/ Instead use
> QVector and mark custom structs as Q_MOVABLE_TYPE.
>
> b) Don't use QMap if you don't need sorting (which you probably don't). Use
> QHash. http://woboq.com/blog/qmap_qhash_benchmark.html Yes, for small sets
> QMap may be faster but then operations are really fast anyways on small
> scale.
>
> c) Remove trailing spaces - come on Aleix ;-) Do me that favor pretty
> please?
> You can just enable the "remove trailing spaces on edited lines" feature in
> Kate - it's perfect!
>
> d) Some other style nitpicks like spaces around operators sometimes missing
> and placement of * or & not always following the style we use in other
> places
> (i.e. attach it to the type name).
>
Will look into those tonight.

>
> But don't get me wrong - the code looks really good all in all! The above
> is
> just nitpicks, which you could also ignore and I'll refactor eventually.
> Take
> it more as an educative post-review :)
>
> Many many thanks for working on this! /me is so happy that "grep -R QMutex"
> returns zero matches in the cmake folder now :) You are my hero Aleix!
>
> > On Tue, Aug 20, 2013 at 2:56 AM, Aleix Pol <aleixpol at kde.org> wrote:
> > > Hi!
> > > It's not news that we've had some threading issues with the project
> model,
> > > especially from the cmake plugin. This was mostly because the project
> was
> > > parsed in a different thread and the results where being added from
> that
> > > thread to the ProjectsModel. It worked, but killing a bug made another
> one
> > > to appear, so that was no fun.
> > >
> > > With this change, I'm making it so the project will be parsed from a
> > > separate thread and will pass the data to another thread that will do
> the
> > > tree creation.
> > >
> > > I haven't tested it (at all, despite small tests with test projects)
> but
> > > the unit tests pass and I'm feeling quite positive about it. I'll be
> > > testing it in my system making sure all's in order during the following
> > > weeks, but for now, if somebody is curious enough, I'm interested in
> any
> > > kind of feedback.
> > >
> > > Cheers!
> > > Aleix
> > >
> > > PS: I'm applying this patch in my kdevplatform, for further safety ;)
> > > http://paste.opensuse.org/63054558
>
> --
> Milian Wolff
> mail at milianw.de
> http://milianw.de
>

None taken Milian, don't worry so much.

Aleix, aka Milian's hero ;)

PS: If you "grep -R Mutex" you'll find some entries about the duchain... ;)
can't wait to figure those out some day :P
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130827/e243c24c/attachment.html>