Fix kolflib_export
ClosedPublic

Authored by twighk on Oct 11 2019, 8:00 PM.

Details

Reviewers
cullmann
Group Reviewers
KDE Games
Commits
R407:6e91fcaf4391: Fix kolflib_export
Summary

KOLFLIB_EXPORT Macro causes many linking errors when compiling with craft on windows using mingw and MSVC

Diff Detail

Repository
R407 Kolf
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
twighk created this revision.Oct 11 2019, 8:00 PM
Restricted Application added a reviewer: KDE Games. · View Herald TranscriptOct 11 2019, 8:00 PM
Restricted Application added a subscriber: kde-games-devel. · View Herald Transcript
twighk requested review of this revision.Oct 11 2019, 8:00 PM

Cannot be linked by ld with this patch on Linux.

This will probably also break msvc.
I’d suggest reading the doc of cmake generate export macros.
The macros are often used wrong...

twighk retitled this revision from Remove kolflib_export to Fix kolflib_export.Oct 12 2019, 8:49 PM
twighk edited the summary of this revision. (Show Details)
twighk updated this revision to Diff 67801.EditedOct 12 2019, 8:51 PM

Instead of removing KOLFLIB_EXPORT, set a define when building it so it choses the correct dll linkage.

Make the library STATIC instead of SHARED in the cmakelists.txt . It was having different problems where it couldn't find things that were in the library when linking:

Creating library lib\kolf.lib and object lib\kolf.exp                                                                              main.cpp.obj : error LNK2001: unresolved external symbol "public: static struct QMetaObject const KolfGame::staticMetaObject" (?staticMetaObject@KolfGame@@2UQMetaObject@@B)                                                                                                main.cpp.obj : error LNK2001: unresolved external symbol "public: static struct QMetaObject const KolfWindow::staticMetaObject" (?staticMetaObject@KolfWindow@@2UQMetaObject@@B)                                                                                            bin\kolf.exe : fatal error LNK1120: 2 unresolved externals
aacid added a subscriber: aacid.Oct 12 2019, 11:00 PM

If we're going to make the kolfprivate library to be static, which kind of makes sense since we don't install the includes anyway

I think you should remove the exporting altogether since it isn't needed at all anymore then.

twighk updated this revision to Diff 67826.Oct 13 2019, 6:00 AM

Remove KOLFLIB_EXPORT and make kolfprivate library static.

Looks reasonable for me.

cullmann accepted this revision.Oct 13 2019, 11:29 AM

Do you have some developer account to push that yourself? If not, I would go for applying for one, as you seem to care for this stuff and it will be much easier if you can push on your own.

This revision is now accepted and ready to land.Oct 13 2019, 11:29 AM

Do you have some developer account to push that yourself? If not, I would go for applying for one, as you seem to care for this stuff and it will be much easier if you can push on your own.

I don't have an account, but have applied for one now. Thanks

This revision was automatically updated to reflect the committed changes.