communi / libcommuni

A cross-platform IRC framework for Qt
https://communi.github.io
BSD 3-Clause "New" or "Revised" License
83 stars 38 forks source link

Move <Qt5.14.0 definition of Qt::SkipEmptyParts out of communi namespace #96

Closed pajlada closed 3 years ago

pajlada commented 3 years ago

Relevant snipped from our CI when trying to build Chatterino which includes libcommuni with Qt 5.12.10

2021-01-09T13:13:57.8815318Z g++ -c -include chatterino -pipe -std=c++17 -O2 -std=gnu++1z -pthread -Wall -Wno-unused-function -Wno-switch -Wno-deprecated-declarations -Wno-sign-compare -Wno-unused-variable -Wno-strict-aliasing -Werror=return-type -Wno-class-memaccess -D_REENTRANT -fPIC -DCHATTERINO -DAB_CUSTOM_THEME -DAB_CUSTOM_SETTINGS -DNDEBUG -DIRC_NAMESPACE=Communi -DBUILD_IRC_CORE -DBUILD_IRC_MODEL -DBUILD_IRC_UTIL -DPAJLADA_SETTINGS_BOOST_FILESYSTEM -DQTKEYCHAIN_NO_EXPORT -DLIBSECRET_SUPPORT -DHAVE_LIBSECRET -DCHATTERINO_GIT_COMMIT=\"4eee27518585b80932f85241ec7398dd1e6218a5\" -DCHATTERINO_GIT_RELEASE=\"\" -DCHATTERINO_GIT_HASH=\"4eee27518\" -DDEBUG_OFF -DQT_NO_DEBUG -DQT_SVG_LIB -DQT_WIDGETS_LIB -DQT_MULTIMEDIA_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_CONCURRENT_LIB -DQT_DBUS_LIB -DQT_CORE_LIB -I../../chatterino2 -I. -I../src -I../lib/humanize/include -I../lib/libcommuni/src/core -I../lib/libcommuni/include/IrcCore -I../lib/libcommuni/src/model -I../lib/libcommuni/include/IrcModel -I../lib/libcommuni/src/util -I../lib/libcommuni/include/IrcUtil -I../lib/websocketpp -I../lib/signals/include -I../lib/settings/include -I../lib/serialize/include -I../lib/rapidjson/include -I../lib -I../lib/qtkeychain -isystem /usr/include/libsecret-1 -isystem /usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -I../../Qt/5.12.10/gcc_64/include -I../../Qt/5.12.10/gcc_64/include/QtSvg -I../../Qt/5.12.10/gcc_64/include/QtWidgets -I../../Qt/5.12.10/gcc_64/include/QtMultimedia -I../../Qt/5.12.10/gcc_64/include/QtGui -I../../Qt/5.12.10/gcc_64/include/QtNetwork -I../../Qt/5.12.10/gcc_64/include/QtConcurrent -I../../Qt/5.12.10/gcc_64/include/QtDBus -I../../Qt/5.12.10/gcc_64/include/QtCore -I. -isystem /usr/include/libdrm -I. -I../../Qt/5.12.10/gcc_64/mkspecs/linux-g++ -o ircmessage.o ../lib/libcommuni/src/core/ircmessage.cpp
2021-01-09T13:13:58.2611114Z ../lib/libcommuni/src/core/ircconnection.cpp: In member function ‘void Communi::IrcConnection::installMessageFilter(QObject*)’:
2021-01-09T13:13:58.2613520Z ../lib/libcommuni/src/core/ircconnection.cpp:1527:102: error: ‘UniqueConnection’ is not a member of ‘Communi::Qt’
2021-01-09T13:13:58.2614802Z          connect(filter, SIGNAL(destroyed(QObject*)), this, SLOT(_irc_filterDestroyed(QObject*)), Qt::UniqueConnection);
2021-01-09T13:13:58.2615646Z                                                                                                       ^~~~~~~~~~~~~~~~
2021-01-09T13:13:58.2626086Z ../lib/libcommuni/src/core/ircconnection.cpp:1527:102: note: suggested alternative:
2021-01-09T13:13:58.2678153Z In file included from ../../Qt/5.12.10/gcc_64/include/QtCore/qobjectdefs.h:48:0,
2021-01-09T13:13:58.2678874Z                  from ../../Qt/5.12.10/gcc_64/include/QtCore/qobject.h:46,
2021-01-09T13:13:58.2679474Z                  from ../lib/libcommuni/include/IrcCore/irccommand.h:33,
2021-01-09T13:13:58.2680136Z                  from ../src/PrecompiledHeader.hpp:2:
2021-01-09T13:13:58.2681712Z ../../Qt/5.12.10/gcc_64/include/QtCore/qnamespace.h:1307:9: note:   ‘UniqueConnection’
2021-01-09T13:13:58.2682354Z          UniqueConnection =  0x80
2021-01-09T13:13:58.2682755Z          ^~~~~~~~~~~~~~~~

Wherever Qt::SkipEmptyParts was used, the rest of the Qt namespace usages got confused and couldn't find other definitions like UniqueConnection

Full link to our CI logs: https://github.com/Chatterino/chatterino2/runs/1673554206#step:13:406

jpnurmi commented 3 years ago

Thanks, looks good! Do you want to have it in the 3.6 branch? We could do a bugfix release

pajlada commented 3 years ago

Thanks, looks good! Do you want to have it in the 3.6 branch? We could do a bugfix release

Whatever is most convenient for you - we build libcommuni statically into our application using git submodules so we generally just select a commit hash and go from there, so we're good to just go from latest master once this has been accepted and merged in. Right now we're on ff79c7292b9e45cffcbe5b717ef55c3fc7d5ef01

pajlada commented 3 years ago

I went ahead and made a PR in the Chatterino2 project that temporarily uses my branch to make sure it's fixed in CI too (I only tested this locally) https://github.com/Chatterino/chatterino2/pull/2349/checks?check_run_id=1673767199 Should be done in 15 minutes or so if you want another layer of confidence for the fix

EDIT: Confirmed it works as expected in our CI suite

jpnurmi commented 3 years ago

Rebased on 3.6