Skip to content

Add cmake#10

Closed
keneanung wants to merge 3 commits intoedbee:masterfrom
keneanung:AddCmake
Closed

Add cmake#10
keneanung wants to merge 3 commits intoedbee:masterfrom
keneanung:AddCmake

Conversation

@keneanung
Copy link
Copy Markdown
Contributor

This adds the use of the cmake build system parallel to the qmake system already in place.

In order to make sure cmake can compile the files that included the Q_OBJECT macro, I had to explicitly include the created moc_ files into the correct one. Without those includes, the DECLARE_TEST macro didn't work correctly.

keneanung and others added 2 commits May 18, 2017 09:49
Qmake somehow correctly added the moc files to the source files. Cmake needs a
little help by adding the #include "moc_*.cpp" declaration. If not all are added
to a single file which creates issues with the DECLARE_TEST define.
@keneanung
Copy link
Copy Markdown
Contributor Author

I should mention that I was unable to test the windows development environment because I don't have one of those. macOS should work according to travis.

@gamecreature
Copy link
Copy Markdown
Member

Thanks for your work!

I'm going to investigate it further. If the .cpp files weren't changed, I've would have merged it directly. I don't like the requirement for manually including the moc_*.c files.
(There has to be another solution for the problem)

Btw. I had recently had an issue with including '.c' files in (k-takata/Onigmo#88 )

@keneanung
Copy link
Copy Markdown
Contributor Author

I didn't like it either, but I didn't find another way (without changing the DECLARE_TEST macro, but I am no C++-dev, so I shied away from that).

Here is what the cmake manual says about the AUTOMOC regarding file includes:

[...] 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. [...] 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 _automoc.cpp file, which is compiled as part of the target.

The <targetname>_automoc.cpp file is what created the problem: The generated includes resulted in multiple different declarations of the class t in the same object file due to macro expansion.

It might be possible to create a custom build step that runs moc on explicitly stated headers and add the results to the linker command in the end, but that sounds like an unreasonable and difficult way to do it.

@keneanung
Copy link
Copy Markdown
Contributor Author

I found another way to create and set the moc-files for cmake, but personally I prefer the explicit inclusion as it is now.

Have a look at https://github.com/Mudlet/Mudlet/blob/development/3rdparty/communi/CMakeLists.txt#L80 and the block below, maybe that's more to your preference?

@gamecreature
Copy link
Copy Markdown
Member

http://doc.qt.io/qt-5/cmake-manual.html

# Instruct CMake to run moc automatically when needed.
set(CMAKE_AUTOMOC ON)

I havent investigated this, but is this an option?

@keneanung
Copy link
Copy Markdown
Contributor Author

That's already set, otherwise I'd have to manually tell cmake which files to run moc on.

@gamecreature
Copy link
Copy Markdown
Member

I've took your pull-request and changed it to use a different DECLARE_TEST macro.
Could you please check out the solution? (Travis tells me it runs ok)

https://github.com/edbee/edbee-lib/tree/pr/10

@keneanung
Copy link
Copy Markdown
Contributor Author

I'll have another look at our setup later, to see if it works.

But you forgot to include a commit I added a bit later (more or less silently) to be able to compile dependent targets.

@gamecreature
Copy link
Copy Markdown
Member

Oops, just added the commit

@gamecreature gamecreature force-pushed the master branch 2 times, most recently from d03ba75 to d523932 Compare May 23, 2017 09:39
@keneanung
Copy link
Copy Markdown
Contributor Author

The new branch works for us as well, so I'm fine with going ahead and merging

gamecreature added a commit that referenced this pull request May 23, 2017
@gamecreature
Copy link
Copy Markdown
Member

Merged via branch/pr10

@keneanung keneanung closed this May 26, 2017
@keneanung keneanung deleted the AddCmake branch May 26, 2017 09:47
SlySven pushed a commit to SlySven/edbee-lib that referenced this pull request Oct 29, 2025
Update to latest upstream for ctrl support on macos
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants