search

lxqt / lxqt-session

The LXQt session manager
https://lxqt.github.io
GNU Lesser General Public License v2.1
57 stars 52 forks source link

Fixes a time interval related FTBFS #381

Closed luis-pereira closed 3 years ago

luis-pereira commented 3 years ago

Fixes https://github.com/lxqt/lxqt-session/issues/380.

palinek commented 3 years ago

Hm... the "std::chrono::duration" QDeadlineTimer constructor is in Qt for a long time (since v5.8) https://doc.qt.io/qt-5/qdeadlinetimer.html#QDeadlineTimer-4, https://github.com/qt/qtbase/blame/dev/src/corelib/kernel/qdeadlinetimer.h#L145

The question for me is why isn't the QDeadlineTimer object implicitly constructed? Maybe in qt5.15 the \<QDeadlineTimer> is somehow transitively included and in 5.12 not. @luis-pereira @darkshram Can you, please, check if only including the QDeadlineTimer makes the trick for 5.12?

$ git diff
diff --git a/lxqt-session/src/procreaper.cpp b/lxqt-session/src/procreaper.cpp
index a45afeb..89cf1d5 100644
--- a/lxqt-session/src/procreaper.cpp
+++ b/lxqt-session/src/procreaper.cpp
@@ -34,6 +34,7 @@
 #include <cstring>
 #include <cerrno>
 #include <sys/wait.h>
+#include <QDeadlineTimer>

 ProcReaper::ProcReaper()
     : mShouldRun{true}
luis-pereira commented 3 years ago

@palinek If I find the time to build the dependencies with 5.12 I will do it. I wanted to do it yesterday, but I was time constrained.

The question for me is why isn't the QDeadlineTimer object implicitly constructed? Maybe in qt5.15 the is somehow transitively included and in 5.12 not.

That's a possible explanation.

tsujan commented 3 years ago

@palinek Since Qt 5.12 doesn't have QThread::wait(QDeadlineTimer), I don't think adding of #include <QDeadlineTimer> would change anything.

palinek commented 3 years ago

Qt 5.12 doesn't have QThread::wait(QDeadlineTimer)

You're right. I missed that... I checked only QWaitCondition::wait.

tsujan commented 3 years ago

Sadly, Qt doc isn't completely reliable when it comes to checking versions.

@palinek So, can this be merged?

palinek commented 3 years ago

So, can this be merged?

Yes, it can... but I personally would stick with:

$ git diff
diff --git a/lxqt-session/src/procreaper.cpp b/lxqt-session/src/procreaper.cpp
index a45afeb..169dae9 100644
--- a/lxqt-session/src/procreaper.cpp
+++ b/lxqt-session/src/procreaper.cpp
@@ -35,6 +35,8 @@
 #include <cerrno>
 #include <sys/wait.h>

+namespace sc = std::chrono;
+
 ProcReaper::ProcReaper()
     : mShouldRun{true}
 {
@@ -58,7 +60,7 @@ void ProcReaper::run()
         if (pid <= 0)
         {
             QMutexLocker guard{&mMutex};
-            mWait.wait(&mMutex, std::chrono::seconds(1));
+            mWait.wait(&mMutex, sc::duration_cast<sc::milliseconds>(sc::seconds(1)));
         }

         int status;
@@ -111,5 +113,5 @@ void ProcReaper::stop(const std::set<int64_t> & excludedPids)
         QMutexLocker guard{&mMutex};
         mShouldRun = false;
     }
-    QThread::wait(std::chrono::seconds(5));
+    QThread::wait(sc::duration_cast<sc::milliseconds>(sc::seconds(5)));
 }

(avoiding plain numbers misinterpretation where possible)

darkshram commented 3 years ago

I have tested the suggested patch by @palinek with Qt 5.12:

diff -Naur lxqt-session-0.17.0.orig/lxqt-session/src/procreaper.cpp lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp
--- lxqt-session-0.17.0.orig/lxqt-session/src/procreaper.cpp    2021-04-13 12:37:02.000000000 -0500
+++ lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp 2021-04-16 06:36:28.361113762 -0500
@@ -34,6 +34,7 @@
 #include <cstring>
 #include <cerrno>
 #include <sys/wait.h>
+#include <QDeadlineTimer>

 ProcReaper::ProcReaper()
     : mShouldRun{true}

But fails to build with this output:

 99%] Built target lxqt-leave
gmake[2]: Leaving directory '/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu'
[ 99%] Built target lxqt-config-session
/builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp: In member function 'void ProcReaper::stop(const std::set<long long int>&)':
/builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp:115:42: error: no matching function for call to 'ProcReaper::wait(std::chrono::seconds)'
     QThread::wait(std::chrono::seconds(5));
                                          ^
In file included from /usr/include/qt5/QtCore/QThread:1:0,
                 from /builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.h:28,
                 from /builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp:28:
/usr/include/qt5/QtCore/qthread.h:139:10: note: candidate: bool QThread::wait(long unsigned int)
     bool wait(unsigned long time = ULONG_MAX);
          ^~~~
/usr/include/qt5/QtCore/qthread.h:139:10: note:   no known conversion for argument 1 from 'std::chrono::seconds {aka std::chrono::duration<long long int>}' to 'long unsigned int'
gmake[2]: Leaving directory '/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu'
gmake[1]: Leaving directory '/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu'
gmake[2]: *** [lxqt-session/CMakeFiles/lxqt-session.dir/build.make:386: lxqt-session/CMakeFiles/lxqt-session.dir/src/procreaper.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:224: lxqt-session/CMakeFiles/lxqt-session.dir/all] Error 2
gmake: *** [Makefile:152: all] Error 2

The second proposed patch:

diff --git a/lxqt-session/src/procreaper.cpp b/lxqt-session/src/procreaper.cpp
index a45afeb..169dae9 100644
--- a/lxqt-session/src/procreaper.cpp
+++ b/lxqt-session/src/procreaper.cpp
@@ -35,6 +35,8 @@
 #include <cerrno>
 #include <sys/wait.h>

+namespace sc = std::chrono;
+
 ProcReaper::ProcReaper()
     : mShouldRun{true}
 {
@@ -58,7 +60,7 @@ void ProcReaper::run()
         if (pid <= 0)
         {
             QMutexLocker guard{&mMutex};
-            mWait.wait(&mMutex, std::chrono::seconds(1));
+            mWait.wait(&mMutex, sc::duration_cast<sc::milliseconds>(sc::seconds(1)));
         }

         int status;
@@ -111,5 +113,5 @@ void ProcReaper::stop(const std::set<int64_t> & excludedPids)                                                                                                                                                            
         QMutexLocker guard{&mMutex};                                                                                                                                                                                                        
         mShouldRun = false;                                                                                                                                                                                                                 
     }                                                                                                                                                                                                                                       
-    QThread::wait(std::chrono::seconds(5));                                                                                                                                                                                                 
+    QThread::wait(sc::duration_cast<sc::milliseconds>(sc::seconds(5)));                                                                                                                                                                     
 }

Also fails to build with this other output:

[ 99%] Built target lxqt-leave
gmake[2]: Leaving directory '/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu'
[ 99%] Built target lxqt-config-session
/builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp: In member function 'virtual void ProcReaper::run()':
/builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp:63:84: error: no matching function for call to 'QWaitCondition::wait(QMutex*, std::enable_if<true, std::chrono::duration<long long int, std::ratio<1, 1000> > >::type)'
             mWait.wait(&mMutex, sc::duration_cast<sc::milliseconds>(sc::seconds(1)));
                                                                                    ^
In file included from /usr/include/qt5/QtCore/QWaitCondition:1:0,
                 from /builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.h:30,
                 from /builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp:28:
/usr/include/qt5/QtCore/qwaitcondition.h:63:10: note: candidate: bool QWaitCondition::wait(QMutex*, long unsigned int)
     bool wait(QMutex *lockedMutex, unsigned long time = ULONG_MAX);
          ^~~~
/usr/include/qt5/QtCore/qwaitcondition.h:63:10: note:   no known conversion for argument 2 from 'std::enable_if<true, std::chrono::duration<long long int, std::ratio<1, 1000> > >::type {aka std::chrono::duration<long long int, std::ratio<1, 1000> >}' to 'long unsigned int'
/usr/include/qt5/QtCore/qwaitcondition.h:64:10: note: candidate: bool QWaitCondition::wait(QMutex*, QDeadlineTimer)
     bool wait(QMutex *lockedMutex, QDeadlineTimer deadline);
          ^~~~
/usr/include/qt5/QtCore/qwaitcondition.h:64:10: note:   no known conversion for argument 2 from 'std::enable_if<true, std::chrono::duration<long long int, std::ratio<1, 1000> > >::type {aka std::chrono::duration<long long int, std::ratio<1, 1000> >}' to 'QDeadlineTimer'
/usr/include/qt5/QtCore/qwaitcondition.h:65:10: note: candidate: bool QWaitCondition::wait(QReadWriteLock*, long unsigned int)
     bool wait(QReadWriteLock *lockedReadWriteLock, unsigned long time = ULONG_MAX);
          ^~~~
/usr/include/qt5/QtCore/qwaitcondition.h:65:10: note:   no known conversion for argument 1 from 'QMutex*' to 'QReadWriteLock*'
/usr/include/qt5/QtCore/qwaitcondition.h:66:10: note: candidate: bool QWaitCondition::wait(QReadWriteLock*, QDeadlineTimer)
     bool wait(QReadWriteLock *lockedReadWriteLock, QDeadlineTimer deadline);
          ^~~~
/usr/include/qt5/QtCore/qwaitcondition.h:66:10: note:   no known conversion for argument 1 from 'QMutex*' to 'QReadWriteLock*'
/builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp: In member function 'void ProcReaper::stop(const std::set<long long int>&)':
/builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp:116:70: error: no matching function for call to 'ProcReaper::wait(std::enable_if<true, std::chrono::duration<long long int, std::ratio<1, 1000> > >::type)'
     QThread::wait(sc::duration_cast<sc::milliseconds>(sc::seconds(5)));
                                                                      ^
In file included from /usr/include/qt5/QtCore/QThread:1:0,
                 from /builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.h:28,
                 from /builddir/build/BUILD/lxqt-session-0.17.0/lxqt-session/src/procreaper.cpp:28:
/usr/include/qt5/QtCore/qthread.h:139:10: note: candidate: bool QThread::wait(long unsigned int)
     bool wait(unsigned long time = ULONG_MAX);
          ^~~~
/usr/include/qt5/QtCore/qthread.h:139:10: note:   no known conversion for argument 1 from 'std::enable_if<true, std::chrono::duration<long long int, std::ratio<1, 1000> > >::type {aka std::chrono::duration<long long int, std::ratio<1, 1000> >}' to 'long unsigned int'
gmake[2]: Leaving directory '/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu'
gmake[1]: Leaving directory '/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu'
gmake[2]: *** [lxqt-session/CMakeFiles/lxqt-session.dir/build.make:386: lxqt-session/CMakeFiles/lxqt-session.dir/src/procreaper.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:224: lxqt-session/CMakeFiles/lxqt-session.dir/all] Error 2
gmake: *** [Makefile:152: all] Error 2

For me, c7271cd60adf8d21da5a8e79865156dd6c3584d3 fixes the the build with Qt 5.12:

From c7271cd60adf8d21da5a8e79865156dd6c3584d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lu=C3=ADs=20Pereira?= <luis.artur.pereira@gmail.com>
Date: Thu, 15 Apr 2021 19:00:11 +0100
Subject: [PATCH] Fixes a time interval related FTBFS

Fixes https://github.com/lxqt/lxqt-session/issues/380.
---
 lxqt-session/src/procreaper.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lxqt-session/src/procreaper.cpp b/lxqt-session/src/procreaper.cpp
index a45afeb..1ccbc78 100644
--- a/lxqt-session/src/procreaper.cpp
+++ b/lxqt-session/src/procreaper.cpp
@@ -58,7 +58,7 @@ void ProcReaper::run()
         if (pid <= 0)
         {
             QMutexLocker guard{&mMutex};
-            mWait.wait(&mMutex, std::chrono::seconds(1));
+            mWait.wait(&mMutex, 1000); // 1 second
         }

         int status;
@@ -111,5 +111,5 @@ void ProcReaper::stop(const std::set<int64_t> & excludedPids)
         QMutexLocker guard{&mMutex};
         mShouldRun = false;
     }
-    QThread::wait(std::chrono::seconds(5));
+    QThread::wait(5000000); // 5 seconds
 }
tsujan commented 3 years ago

For me, c7271cd fixes the the build with Qt 5.12

@darkshram Yes, but please use the final revision, namely https://github.com/lxqt/lxqt-session/commit/f0df56a39e62b1ce8916141ab78aee2e7a2b4a86 — "5000000" is fixed in it.

palinek commented 3 years ago

Also fails to build with this other output:

Heh... of course, the count() is missing there

$ git diff
diff --git a/lxqt-session/src/procreaper.cpp b/lxqt-session/src/procreaper.cpp
index a45afeb..169dae9 100644
--- a/lxqt-session/src/procreaper.cpp
+++ b/lxqt-session/src/procreaper.cpp
@@ -35,6 +35,8 @@
 #include <cerrno>
 #include <sys/wait.h>

+namespace sc = std::chrono;
+
 ProcReaper::ProcReaper()
     : mShouldRun{true}
 {
@@ -58,7 +60,7 @@ void ProcReaper::run()
         if (pid <= 0)
         {
             QMutexLocker guard{&mMutex};
-            mWait.wait(&mMutex, std::chrono::seconds(1));
+            mWait.wait(&mMutex, sc::duration_cast<sc::milliseconds>(sc::seconds(1)).count());
         }

         int status;
@@ -111,5 +113,5 @@ void ProcReaper::stop(const std::set<int64_t> & excludedPids)
         QMutexLocker guard{&mMutex};
         mShouldRun = false;
     }
-    QThread::wait(std::chrono::seconds(5));
+    QThread::wait(sc::duration_cast<sc::milliseconds>(sc::seconds(5)).count());
 }

(avoiding plain numbers misinterpretation where possible)

darkshram commented 3 years ago

@darkshram Yes, but please use the final revision, namely f0df56a — "5000000" is fixed in it.

Done.

Will proceed testing https://github.com/lxqt/lxqt-session/pull/381#issuecomment-821131943 in a few minutes.

darkshram commented 3 years ago

Also fails to build with this other output:

Heh... of course, the count() is missing there ...

(avoiding plain numbers misinterpretation where possible)

Built successfully without warnings.

I post the final output just as reference:

[ 98%] Building CXX object lxqt-config-session/CMakeFiles/lxqt-config-session.dir/LXQtAppTranslationLoader.cpp.o
cd /builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu/lxqt-config-session && /usr/lib/ccache/c++ -DLXQT_DATA_DIR=\"/usr/share\" -DLXQT_ETC_XDG_DIR=\"/etc/xdg\" -DLXQT_GRAPHICS_DIR=\"/usr/share/lxqt/graphics\" -DLXQT_MAJOR_VERSION=\"0\" -DLXQT_MINOR_VERSION=\"17\" -DLXQT_PATCH_VERSION=\"0\" -DLXQT_RELATIVE_SHARE_DIR=\"lxqt\" -DLXQT_RELATIVE_SHARE_TRANSLATIONS_DIR=\"lxqt/translations\" -DLXQT_SESSION_VERSION=\"0.17.0\" -DLXQT_SHARE_DIR=\"/usr/share/lxqt\" -DLXQT_SHARE_TRANSLATIONS_DIR=\"/usr/share/lxqt/translations\" -DLXQT_VERSION=\"0.17.0\" -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NO_CAST_FROM_ASCII -DQT_NO_CAST_FROM_BYTEARRAY -DQT_NO_CAST_TO_ASCII -DQT_NO_DEBUG -DQT_NO_FOREACH -DQT_NO_URL_CAST_FROM_STRING -DQT_SVG_LIB -DQT_USE_QSTRINGBUILDER -DQT_WIDGETS_LIB -DQT_X11EXTRAS_LIB -DQT_XML_LIB -I/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu/lxqt-config-session -I/builddir/build/BUILD/lxqt-session-0.17.0/lxqt-config-session -I/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu/lxqt-config-session/lxqt-config-session_autogen/include -isystem /usr/include/KF5/KWindowSystem -isystem /usr/include/KF5 -isystem /usr/include/qt5 -isystem /usr/include/qt5/QtGui -isystem /usr/include/qt5/QtCore -isystem /usr/lib/qt5/./mkspecs/linux-g++ -isystem /usr/include/qt5/QtWidgets -isystem /usr/include/qt5/QtDBus -isystem /usr/include/qt5/QtX11Extras -isystem /usr/include/lxqt -isystem /usr/include/lxqt/LXQt -isystem /usr/include/qt5xdg -isystem /usr/include/qt5/QtXml -isystem /usr/include/qt5xdgiconloader -isystem /usr/include/qt5xdgiconloader/3.7.0 -isystem /usr/include/qt5/QtSvg -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches   -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables -fno-exceptions -Wall -Wextra -Wchar-subscripts -Wno-long-long -Wpointer-arith -Wundef -Wformat-security -Wnon-virtual-dtor -Woverloaded-virtual -Wpedantic -DNDEBUG -fPIE -fvisibility=hidden -fvisibility-inlines-hidden -fPIC -std=c++14 -o CMakeFiles/lxqt-config-session.dir/LXQtAppTranslationLoader.cpp.o -c /builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu/lxqt-config-session/LXQtAppTranslationLoader.cpp
[ 99%] Linking CXX executable lxqt-config-session
cd /builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu/lxqt-config-session && /usr/bin/cmake -E cmake_link_script CMakeFiles/lxqt-config-session.dir/link.txt --verbose=1
/usr/lib/ccache/c++ -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches   -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables -fno-exceptions -Wall -Wextra -Wchar-subscripts -Wno-long-long -Wpointer-arith -Wundef -Wformat-security -Wnon-virtual-dtor -Woverloaded-virtual -Wpedantic -DNDEBUG -Wl,-Bsymbolic-functions -Wl,-Bsymbolic -Wl,-z,relro -rdynamic CMakeFiles/lxqt-config-session.dir/lxqt-config-session_autogen/mocs_compilation.cpp.o CMakeFiles/lxqt-config-session.dir/main.cpp.o CMakeFiles/lxqt-config-session.dir/sessionconfigwindow.cpp.o CMakeFiles/lxqt-config-session.dir/basicsettings.cpp.o CMakeFiles/lxqt-config-session.dir/modulemodel.cpp.o CMakeFiles/lxqt-config-session.dir/autostartpage.cpp.o CMakeFiles/lxqt-config-session.dir/autostartmodel.cpp.o CMakeFiles/lxqt-config-session.dir/autostartitem.cpp.o CMakeFiles/lxqt-config-session.dir/autostartedit.cpp.o CMakeFiles/lxqt-config-session.dir/autostartutils.cpp.o CMakeFiles/lxqt-config-session.dir/environmentpage.cpp.o CMakeFiles/lxqt-config-session.dir/userlocationspage.cpp.o CMakeFiles/lxqt-config-session.dir/__/lxqt-session/src/windowmanager.cpp.o CMakeFiles/lxqt-config-session.dir/LXQtAppTranslationLoader.cpp.o -o lxqt-config-session  /usr/lib/liblxqt.so.0.17.0 /usr/lib/libKF5WindowSystem.so.5.76.0 /usr/lib/libQt5X11Extras.so.5.12.10 /usr/lib/libQt5Xdg.so.3.7.0 /usr/lib/libQt5DBus.so.5.12.10 /usr/lib/libQt5Xml.so.5.12.10 /usr/lib/libQt5XdgIconLoader.so.3.7.0 /usr/lib/libQt5Svg.so.5.12.10 /usr/lib/libQt5Widgets.so.5.12.10 /usr/lib/libQt5Gui.so.5.12.10 /usr/lib/libQt5Core.so.5.12.10 -lglib-2.0 -lgobject-2.0 -lgio-2.0 
gmake[2]: Leaving directory '/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu'
[ 99%] Built target lxqt-leave
gmake[2]: Leaving directory '/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu'
[ 99%] Built target lxqt-config-session
[100%] Linking CXX executable lxqt-session
cd /builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu/lxqt-session && /usr/bin/cmake -E cmake_link_script CMakeFiles/lxqt-session.dir/link.txt --verbose=1
/usr/lib/ccache/c++ -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches   -m32 -march=i686 -mtune=atom -fasynchronous-unwind-tables -fno-exceptions -Wall -Wextra -Wchar-subscripts -Wno-long-long -Wpointer-arith -Wundef -Wformat-security -Wnon-virtual-dtor -Woverloaded-virtual -Wpedantic -DNDEBUG -Wl,-Bsymbolic-functions -Wl,-Bsymbolic -Wl,-z,relro -rdynamic CMakeFiles/lxqt-session.dir/lxqt-session_autogen/mocs_compilation.cpp.o CMakeFiles/lxqt-session.dir/src/main.cpp.o CMakeFiles/lxqt-session.dir/src/lxqtmodman.cpp.o CMakeFiles/lxqt-session.dir/src/wmselectdialog.cpp.o CMakeFiles/lxqt-session.dir/src/windowmanager.cpp.o CMakeFiles/lxqt-session.dir/src/sessionapplication.cpp.o CMakeFiles/lxqt-session.dir/src/lockscreenmanager.cpp.o CMakeFiles/lxqt-session.dir/src/numlock.cpp.o CMakeFiles/lxqt-session.dir/src/log.cpp.o CMakeFiles/lxqt-session.dir/src/procreaper.cpp.o CMakeFiles/lxqt-session.dir/src/UdevNotifier.cpp.o CMakeFiles/lxqt-session.dir/LXQtAppTranslationLoader.cpp.o -o lxqt-session  /usr/lib/liblxqt.so.0.17.0 -lX11 -lXext /usr/lib/libKF5WindowSystem.so.5.76.0 -lprocps -ludev /usr/lib/libQt5X11Extras.so.5.12.10 /usr/lib/libQt5Xdg.so.3.7.0 /usr/lib/libQt5DBus.so.5.12.10 /usr/lib/libQt5Xml.so.5.12.10 /usr/lib/libQt5XdgIconLoader.so.3.7.0 /usr/lib/libQt5Svg.so.5.12.10 /usr/lib/libQt5Widgets.so.5.12.10 /usr/lib/libQt5Gui.so.5.12.10 /usr/lib/libQt5Core.so.5.12.10 -lglib-2.0 -lgobject-2.0 -lgio-2.0 
gmake[2]: Leaving directory '/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu'
[100%] Built target lxqt-session
gmake[1]: Leaving directory '/builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu'
/usr/bin/cmake -E cmake_progress_start /builddir/build/BUILD/lxqt-session-0.17.0/i686-redhat-linux-gnu/CMakeFiles 0

Either https://github.com/lxqt/lxqt-session/commit/f0df56a39e62b1ce8916141ab78aee2e7a2b4a86 or https://github.com/lxqt/lxqt-session/pull/381#issuecomment-821131943 fixes build for Qt 5.12. Which patch will be used for 0.17.1?

tsujan commented 3 years ago

Either f0df56a or #381 (comment) fixes build for Qt 5.12. Which patch will be used for 0.17.1?

Both are correct.

@palinek Any practical reason for complicating the code by using sc::duration_cast<sc::milliseconds>(sc::seconds(1)).count() instead of 1000?

palinek commented 3 years ago

Any practical reason for complicating the code

No, just the one reason I wrote up there: ...avoiding plain numbers misinterpretation where possible...

Note: As the expressions are constexpr there is no runtime overhead even if the notation might seem long and/or complicated...

static_assert(std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::seconds(1)).count() == 1000, "test");
tsujan commented 3 years ago

there is no runtime overhead

OK but 1000 and 5000 cannot be misinterpreted in any way and Qt 5.15 does have overloads. Your patch may frighten code readers ;)

palinek commented 3 years ago

OK but 1000 and 5000 cannot be misinterpreted in any way

Did you see the first commit by Luis?

You know my opinion, but merge whatever you find more suitable.

tsujan commented 3 years ago

IMHO, this place isn't different from other places where we use milliseconds inside a Qt method. The problem arose only because Qt doc wasn't specific in this regard.

luis-pereira commented 3 years ago

Did you see the first commit by Luis?

QThread::wait(unsigned long time) documentation doesn't state in what units time is. That's the source of my mistake.

tsujan commented 3 years ago

Let's not delay lxqt-session-0.17.1; some users may need it.